Commit 73468e14 by Ivana Huckova Committed by GitHub

Jaeger/Zipkin: URL-encode service names and trace ids for API calls (#26115)

* Encode services and id

* Encode URL for Zipkin API calls

* Add test coverage
parent c136f7da
......@@ -18,6 +18,55 @@ describe('JaegerQueryField', function() {
expect(wrapper.find(ButtonCascader).props().options[0].label).toBe('No traces found');
});
it('uses URL encoded service name in metadataRequest request', async function() {
const wrapper = mount(
<JaegerQueryField
history={[]}
datasource={makeDatasourceMock({
'service/test': {
op1: [
{
traceID: '12345',
spans: [
{
spanID: 's2',
operationName: 'nonRootOp',
references: [{ refType: 'CHILD_OF', traceID: '12345', spanID: 's1' }],
duration: 10,
},
{
operationName: 'rootOp',
spanID: 's1',
references: [],
duration: 99,
},
],
},
],
},
})}
query={{ query: '1234' } as JaegerQuery}
onRunQuery={() => {}}
onChange={() => {}}
/>
);
// Simulating selection options. We need this as the function depends on the intermediate state of the component
await wrapper
.find(ButtonCascader)
.props()
.loadData([{ value: 'service/test', label: 'service/test' }]);
wrapper.update();
expect(wrapper.find(ButtonCascader).props().options[0].label).toEqual('service/test');
expect(wrapper.find(ButtonCascader).props().options[0].value).toEqual('service/test');
expect(wrapper.find(ButtonCascader).props().options[0].children[1]).toEqual({
isLeaf: false,
label: 'op1',
value: 'op1',
});
});
it('shows root span as 3rd level in cascader', async function() {
const wrapper = mount(
<JaegerQueryField
......@@ -73,9 +122,11 @@ function makeDatasourceMock(data: { [service: string]: { [operation: string]: an
if (url.match(/\/services$/)) {
return Promise.resolve(Object.keys(data));
}
let match = url.match(/\/services\/(\w+)\/operations/);
let match = url.match(/\/services\/(.*)\/operations/);
if (match) {
return Promise.resolve(Object.keys(data[match[1]]));
const decodedService = decodeURIComponent(match[1]);
expect(decodedService).toBe(Object.keys(data)[0]);
return Promise.resolve(Object.keys(data[decodedService]));
}
if (url.match(/\/traces?/)) {
......
......@@ -148,7 +148,7 @@ export class JaegerQueryField extends React.PureComponent<Props, State> {
findOperations = async (service: string) => {
const { datasource } = this.props;
const url = `/api/services/${service}/operations`;
const url = `/api/services/${encodeURIComponent(service)}/operations`;
try {
return await datasource.metadataRequest(url);
} catch (error) {
......
......@@ -16,6 +16,28 @@ describe('JaegerDatasource', () => {
});
});
it('returns trace when traceId with special characters is queried', async () => {
await withMockedBackendSrv(makeBackendSrvMock('a/b'), async () => {
const ds = new JaegerDatasource(defaultSettings);
const query = {
...defaultQuery,
targets: [
{
query: 'a/b',
refId: '1',
},
],
};
const response = await ds.query(query).toPromise();
const field = response.data[0].fields[0];
expect(field.name).toBe('trace');
expect(field.type).toBe(FieldType.trace);
expect(field.values.get(0)).toEqual({
traceId: 'a/b',
});
});
});
it('returns empty response if trace id is not specified', async () => {
const ds = new JaegerDatasource(defaultSettings);
const response = await ds
......@@ -34,7 +56,9 @@ describe('JaegerDatasource', () => {
function makeBackendSrvMock(traceId: string) {
return {
datasourceRequest(options: BackendSrvRequest): Promise<any> {
expect(options.url.substr(options.url.length - 17, options.url.length)).toBe(`/api/traces/${traceId}`);
expect(options.url.substr(options.url.length - 17, options.url.length)).toBe(
`/api/traces/${encodeURIComponent(traceId)}`
);
return Promise.resolve({
data: {
data: [
......
......@@ -36,7 +36,7 @@ export class JaegerDatasource extends DataSourceApi<JaegerQuery> {
const id = options.targets[0]?.query;
if (id) {
// TODO: this api is internal, used in jaeger ui. Officially they have gRPC api that should be used.
return this._request(`/api/traces/${id}`).pipe(
return this._request(`/api/traces/${encodeURIComponent(id)}`).pipe(
map(response => {
return {
data: [
......
......@@ -11,6 +11,12 @@ describe('ZipkinDatasource', () => {
const response = await ds.query({ targets: [{ query: '12345' }] } as DataQueryRequest<ZipkinQuery>).toPromise();
expect(response.data[0].fields[0].values.get(0)).toEqual(jaegerTrace);
});
it('runs query with traceId that includes special characters', async () => {
setupBackendSrv({ url: '/api/datasources/proxy/1/api/v2/trace/a%2Fb', response: zipkinResponse });
const ds = new ZipkinDatasource(defaultSettings);
const response = await ds.query({ targets: [{ query: 'a/b' }] } as DataQueryRequest<ZipkinQuery>).toPromise();
expect(response.data[0].fields[0].values.get(0)).toEqual(jaegerTrace);
});
});
describe('metadataRequest', () => {
......
......@@ -27,7 +27,9 @@ export class ZipkinDatasource extends DataSourceApi<ZipkinQuery> {
query(options: DataQueryRequest<ZipkinQuery>): Observable<DataQueryResponse> {
const traceId = options.targets[0]?.query;
if (traceId) {
return this.request<ZipkinSpan[]>(`${apiPrefix}/trace/${traceId}`).pipe(map(responseToDataQueryResponse));
return this.request<ZipkinSpan[]>(`${apiPrefix}/trace/${encodeURIComponent(traceId)}`).pipe(
map(responseToDataQueryResponse)
);
} else {
return of(emptyDataQueryResponse);
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment