From 379249fc601f71fcf7484d2455ac56af44d79cf6 Mon Sep 17 00:00:00 2001 From: Joey <90795735+joey-grafana@users.noreply.github.com> Date: Mon, 12 Aug 2024 14:12:22 +0100 Subject: [PATCH] Tempo: Tidy up and organise (#90649) * Move test files to folder * Update paths * Tidy up --- .../SearchTraceQLEditor/GroupByField.test.tsx | 2 +- .../SearchTraceQLEditor/SearchField.test.tsx | 2 +- .../SearchTraceQLEditor/TagsInput.test.tsx | 2 +- .../TraceQLSearch.test.tsx | 2 +- .../tempo/VariableQueryEditor.test.tsx | 2 +- .../datasource/tempo/datasource.test.ts | 28 +- .../plugins/datasource/tempo/datasource.ts | 270 +++++++++--------- .../datasource/tempo/graphTransform.test.ts | 2 +- .../tempo/resultTransformer.test.ts | 2 +- .../tempo/{ => test}/mockJsonResponse.json | 0 .../tempo/{ => test}/mockServiceGraph.json | 0 .../datasource/tempo/{ => test}/mocks.ts | 4 +- .../tempo/{ => test}/testResponse.ts | 0 .../datasource/tempo/{ => test}/test_utils.ts | 0 .../datasource/tempo/variables.test.ts | 2 +- 15 files changed, 158 insertions(+), 160 deletions(-) rename public/app/plugins/datasource/tempo/{ => test}/mockJsonResponse.json (100%) rename public/app/plugins/datasource/tempo/{ => test}/mockServiceGraph.json (100%) rename public/app/plugins/datasource/tempo/{ => test}/mocks.ts (94%) rename public/app/plugins/datasource/tempo/{ => test}/testResponse.ts (100%) rename public/app/plugins/datasource/tempo/{ => test}/test_utils.ts (100%) diff --git a/public/app/plugins/datasource/tempo/SearchTraceQLEditor/GroupByField.test.tsx b/public/app/plugins/datasource/tempo/SearchTraceQLEditor/GroupByField.test.tsx index 02e8b7190fc..b3a8dae79ae 100644 --- a/public/app/plugins/datasource/tempo/SearchTraceQLEditor/GroupByField.test.tsx +++ b/public/app/plugins/datasource/tempo/SearchTraceQLEditor/GroupByField.test.tsx @@ -5,7 +5,7 @@ import { useState } from 'react'; import { TraceqlSearchScope } from '../dataquery.gen'; import { TempoDatasource } from '../datasource'; import TempoLanguageProvider from '../language_provider'; -import { initTemplateSrv } from '../test_utils'; +import { initTemplateSrv } from '../test/test_utils'; import { TempoQuery } from '../types'; import { GroupByField } from './GroupByField'; diff --git a/public/app/plugins/datasource/tempo/SearchTraceQLEditor/SearchField.test.tsx b/public/app/plugins/datasource/tempo/SearchTraceQLEditor/SearchField.test.tsx index 55207d53415..7330956c63d 100644 --- a/public/app/plugins/datasource/tempo/SearchTraceQLEditor/SearchField.test.tsx +++ b/public/app/plugins/datasource/tempo/SearchTraceQLEditor/SearchField.test.tsx @@ -6,7 +6,7 @@ import { LanguageProvider } from '@grafana/data'; import { TraceqlFilter, TraceqlSearchScope } from '../dataquery.gen'; import { TempoDatasource } from '../datasource'; import TempoLanguageProvider from '../language_provider'; -import { initTemplateSrv } from '../test_utils'; +import { initTemplateSrv } from '../test/test_utils'; import { keywordOperators, numberOperators, operators, stringOperators } from '../traceql/traceql'; import SearchField from './SearchField'; diff --git a/public/app/plugins/datasource/tempo/SearchTraceQLEditor/TagsInput.test.tsx b/public/app/plugins/datasource/tempo/SearchTraceQLEditor/TagsInput.test.tsx index 6133badd652..b7834c57774 100644 --- a/public/app/plugins/datasource/tempo/SearchTraceQLEditor/TagsInput.test.tsx +++ b/public/app/plugins/datasource/tempo/SearchTraceQLEditor/TagsInput.test.tsx @@ -4,7 +4,7 @@ import userEvent from '@testing-library/user-event'; import { TraceqlFilter, TraceqlSearchScope } from '../dataquery.gen'; import { TempoDatasource } from '../datasource'; import TempoLanguageProvider from '../language_provider'; -import { initTemplateSrv } from '../test_utils'; +import { initTemplateSrv } from '../test/test_utils'; import { Scope } from '../types'; import TagsInput from './TagsInput'; diff --git a/public/app/plugins/datasource/tempo/SearchTraceQLEditor/TraceQLSearch.test.tsx b/public/app/plugins/datasource/tempo/SearchTraceQLEditor/TraceQLSearch.test.tsx index 5bf430ba53e..ae2e26ed67f 100644 --- a/public/app/plugins/datasource/tempo/SearchTraceQLEditor/TraceQLSearch.test.tsx +++ b/public/app/plugins/datasource/tempo/SearchTraceQLEditor/TraceQLSearch.test.tsx @@ -7,7 +7,7 @@ import { config } from '@grafana/runtime'; import { TraceqlSearchScope } from '../dataquery.gen'; import { TempoDatasource } from '../datasource'; import TempoLanguageProvider from '../language_provider'; -import { initTemplateSrv } from '../test_utils'; +import { initTemplateSrv } from '../test/test_utils'; import { TempoQuery } from '../types'; import TraceQLSearch from './TraceQLSearch'; diff --git a/public/app/plugins/datasource/tempo/VariableQueryEditor.test.tsx b/public/app/plugins/datasource/tempo/VariableQueryEditor.test.tsx index 700c2defc36..de0e63fb112 100644 --- a/public/app/plugins/datasource/tempo/VariableQueryEditor.test.tsx +++ b/public/app/plugins/datasource/tempo/VariableQueryEditor.test.tsx @@ -10,7 +10,7 @@ import { TempoVariableQueryType, } from './VariableQueryEditor'; import { selectOptionInTest } from './_importedDependencies/test/helpers/selectOptionInTest'; -import { createTempoDatasource } from './mocks'; +import { createTempoDatasource } from './test/mocks'; const refId = 'TempoDatasourceVariableQueryEditor-VariableQuery'; diff --git a/public/app/plugins/datasource/tempo/datasource.test.ts b/public/app/plugins/datasource/tempo/datasource.test.ts index 43dbb94b2a7..6d0cb41785e 100644 --- a/public/app/plugins/datasource/tempo/datasource.test.ts +++ b/public/app/plugins/datasource/tempo/datasource.test.ts @@ -41,10 +41,10 @@ import { getFieldConfig, getEscapedSpanNames, } from './datasource'; -import mockJson from './mockJsonResponse.json'; -import mockServiceGraph from './mockServiceGraph.json'; -import { createMetadataRequest, createTempoDatasource } from './mocks'; -import { initTemplateSrv } from './test_utils'; +import mockJson from './test/mockJsonResponse.json'; +import mockServiceGraph from './test/mockServiceGraph.json'; +import { createMetadataRequest, createTempoDatasource } from './test/mocks'; +import { initTemplateSrv } from './test/test_utils'; import { TempoJsonData, TempoQuery } from './types'; let mockObservable: () => Observable; @@ -68,7 +68,7 @@ describe('Tempo data source', () => { describe('runs correctly', () => { config.featureToggles.traceQLStreaming = true; jest.spyOn(TempoDatasource.prototype, 'isFeatureAvailable').mockImplementation(() => true); - const handleStreamingSearch = jest.spyOn(TempoDatasource.prototype, 'handleStreamingSearch'); + const handleStreamingQuery = jest.spyOn(TempoDatasource.prototype, 'handleStreamingQuery'); const request = jest.spyOn(TempoDatasource.prototype, '_request'); const templateSrv: TemplateSrv = { replace: (s: string) => s } as unknown as TemplateSrv; @@ -104,7 +104,7 @@ describe('Tempo data source', () => { config.liveEnabled = true; const ds = new TempoDatasource(defaultSettings, templateSrv); await lastValueFrom(ds.query(traceqlQuery as DataQueryRequest)); - expect(handleStreamingSearch).toHaveBeenCalledTimes(1); + expect(handleStreamingQuery).toHaveBeenCalledTimes(1); expect(request).toHaveBeenCalledTimes(0); }); @@ -112,7 +112,7 @@ describe('Tempo data source', () => { config.liveEnabled = true; const ds = new TempoDatasource(defaultSettings, templateSrv); await lastValueFrom(ds.query(traceqlSearchQuery as DataQueryRequest)); - expect(handleStreamingSearch).toHaveBeenCalledTimes(1); + expect(handleStreamingQuery).toHaveBeenCalledTimes(1); expect(request).toHaveBeenCalledTimes(0); }); @@ -120,7 +120,7 @@ describe('Tempo data source', () => { config.liveEnabled = false; const ds = new TempoDatasource(defaultSettings, templateSrv); await lastValueFrom(ds.query(traceqlQuery as DataQueryRequest)); - expect(handleStreamingSearch).toHaveBeenCalledTimes(1); + expect(handleStreamingQuery).toHaveBeenCalledTimes(1); expect(request).toHaveBeenCalledTimes(1); }); @@ -128,7 +128,7 @@ describe('Tempo data source', () => { config.liveEnabled = false; const ds = new TempoDatasource(defaultSettings, templateSrv); await lastValueFrom(ds.query(traceqlSearchQuery as DataQueryRequest)); - expect(handleStreamingSearch).toHaveBeenCalledTimes(1); + expect(handleStreamingQuery).toHaveBeenCalledTimes(1); expect(request).toHaveBeenCalledTimes(1); }); }); @@ -365,14 +365,14 @@ describe('Tempo data source', () => { describe('test the testDatasource function', () => { it('should return a success msg if response.ok is true', async () => { mockObservable = () => of({ ok: true }); - const handleStreamingSearch = jest - .spyOn(TempoDatasource.prototype, 'handleStreamingSearch') + const handleStreamingQuery = jest + .spyOn(TempoDatasource.prototype, 'handleStreamingQuery') .mockImplementation(() => of({ data: [] })); const ds = new TempoDatasource(defaultSettings); const response = await ds.testDatasource(); expect(response.status).toBe('success'); - expect(handleStreamingSearch).toHaveBeenCalled(); + expect(handleStreamingQuery).toHaveBeenCalled(); }); }); @@ -397,7 +397,7 @@ describe('Tempo data source', () => { raw: { from: 'now-15m', to: 'now' }, }; - const request = ds.traceIdQueryRequest( + const request = ds.makeTraceIdRequest( { requestId: 'test', interval: '', @@ -426,7 +426,7 @@ describe('Tempo data source', () => { jsonData: { traceQuery: { timeShiftEnabled: false, spanStartTimeShift: '2m', spanEndTimeShift: '4m' } }, }); - const request = ds.traceIdQueryRequest( + const request = ds.makeTraceIdRequest( { requestId: 'test', interval: '', diff --git a/public/app/plugins/datasource/tempo/datasource.ts b/public/app/plugins/datasource/tempo/datasource.ts index da625c8b8bc..44cd4f14169 100644 --- a/public/app/plugins/datasource/tempo/datasource.ts +++ b/public/app/plugins/datasource/tempo/datasource.ts @@ -283,6 +283,19 @@ export class TempoDatasource extends DataSourceWithBackend): Observable { const subQueries: Array> = []; const filteredTargets = options.targets.filter((target) => !target.hide); @@ -358,7 +371,7 @@ export class TempoDatasource extends DataSourceWithBackend) => { + formatGroupBy = (groupBy: TraceqlFilter[]) => { + return groupBy + ?.filter((f) => f.tag) + .map((f) => { + if (f.scope === TraceqlSearchScope.Unscoped) { + return `.${f.tag}`; + } + return f.scope !== TraceqlSearchScope.Intrinsic ? `${f.scope}.${f.tag}` : f.tag; + }) + .join(', '); + }; + + hasGroupBy = (query: TempoQuery) => { + return query.groupBy?.find((gb) => gb.tag); + }; + + /** + * Handles the simplest of the queries where we have just a trace id and return trace data for it. + * @param options + * @param targets + * @private + */ + handleTraceIdQuery(options: DataQueryRequest, targets: TempoQuery[]): Observable { + const validTargets = targets + .filter((t) => t.query) + .map((t): TempoQuery => ({ ...t, query: t.query?.trim(), queryType: 'traceId' })); + if (!validTargets.length) { + return EMPTY; + } + + const request = this.makeTraceIdRequest(options, validTargets); + return super.query(request).pipe( + map((response) => { + if (response.error) { + return response; + } + return transformTrace(response, this.instanceSettings, this.nodeGraph?.enabled); + }) + ); + } + + handleTraceQlQuery = ( + options: DataQueryRequest, + targets: { + [type: string]: TempoQuery[]; + }, + queryValue: string + ): Observable => { + if (this.isStreamingSearchEnabled()) { + return this.handleStreamingQuery(options, targets.traceql, queryValue); + } else { + return this._request('/api/search', { + q: queryValue, + limit: options.targets[0].limit ?? DEFAULT_LIMIT, + spss: options.targets[0].spss ?? DEFAULT_SPSS, + start: options.range.from.unix(), + end: options.range.to.unix(), + }).pipe( + map((response) => { + return { + data: formatTraceQLResponse(response.data.traces, this.instanceSettings, targets.traceql[0].tableType), + }; + }), + catchError((err) => { + return of({ error: { message: getErrorMessage(err.data.message) }, data: [] }); + }) + ); + } + }; + + handleTraceQlMetricsQuery = ( + options: DataQueryRequest, + queryValue: string + ): Observable => { + const requestData = { + query: queryValue, + start: options.range.from.unix(), + end: options.range.to.unix(), + step: options.targets[0].step, + }; + + if (!requestData.step) { + delete requestData.step; + } + + return this._request('/api/metrics/query_range', requestData).pipe( + map((response) => { + return { + data: formatTraceQLMetrics(queryValue, response.data), + }; + }), + catchError((err) => { + return of({ error: { message: getErrorMessage(err.data.message) }, data: [] }); + }) + ); + }; + + handleMetricsSummaryQuery = (target: TempoQuery, query: string, options: DataQueryRequest) => { reportInteraction('grafana_traces_metrics_summary_queried', { datasourceType: 'tempo', app: options.app ?? '', @@ -574,105 +671,30 @@ export class TempoDatasource extends DataSourceWithBackend { - return groupBy - ?.filter((f) => f.tag) - .map((f) => { - if (f.scope === TraceqlSearchScope.Unscoped) { - return `.${f.tag}`; - } - return f.scope !== TraceqlSearchScope.Intrinsic ? `${f.scope}.${f.tag}` : f.tag; - }) - .join(', '); - }; - - hasGroupBy = (query: TempoQuery) => { - return query.groupBy?.find((gb) => gb.tag); - }; - - /** - * Handles the simplest of the queries where we have just a trace id and return trace data for it. - * @param options - * @param targets - * @private - */ - handleTraceIdQuery(options: DataQueryRequest, targets: TempoQuery[]): Observable { - const validTargets = targets - .filter((t) => t.query) - .map((t): TempoQuery => ({ ...t, query: t.query?.trim(), queryType: 'traceId' })); - if (!validTargets.length) { + // This function can probably be simplified by avoiding passing both `targets` and `query`, + // since `query` is built from `targets`, if you look at how this function is currently called + handleStreamingQuery( + options: DataQueryRequest, + targets: TempoQuery[], + query: string + ): Observable { + if (query === '') { return EMPTY; } - const traceRequest = this.traceIdQueryRequest(options, validTargets); - - return super.query(traceRequest).pipe( - map((response) => { - if (response.error) { - return response; - } - return transformTrace(response, this.instanceSettings, this.nodeGraph?.enabled); - }) + return merge( + ...targets.map((target) => + doTempoChannelStream( + { ...target, query }, + this, // the datasource + options, + this.instanceSettings + ) + ) ); } - handleTraceQlQuery = ( - options: DataQueryRequest, - targets: { - [type: string]: TempoQuery[]; - }, - queryValue: string - ): Observable => { - if (this.isStreamingSearchEnabled()) { - return this.handleStreamingSearch(options, targets.traceql, queryValue); - } else { - return this._request('/api/search', { - q: queryValue, - limit: options.targets[0].limit ?? DEFAULT_LIMIT, - spss: options.targets[0].spss ?? DEFAULT_SPSS, - start: options.range.from.unix(), - end: options.range.to.unix(), - }).pipe( - map((response) => { - return { - data: formatTraceQLResponse(response.data.traces, this.instanceSettings, targets.traceql[0].tableType), - }; - }), - catchError((err) => { - return of({ error: { message: getErrorMessage(err.data.message) }, data: [] }); - }) - ); - } - }; - - handleTraceQlMetricsQuery = ( - options: DataQueryRequest, - queryValue: string - ): Observable => { - const requestData = { - query: queryValue, - start: options.range.from.unix(), - end: options.range.to.unix(), - step: options.targets[0].step, - }; - - if (!requestData.step) { - delete requestData.step; - } - - return this._request('/api/metrics/query_range', requestData).pipe( - map((response) => { - return { - data: formatTraceQLMetrics(queryValue, response.data), - }; - }), - catchError((err) => { - return of({ error: { message: getErrorMessage(err.data.message) }, data: [] }); - }) - ); - }; - - traceIdQueryRequest(options: DataQueryRequest, targets: TempoQuery[]): DataQueryRequest { + makeTraceIdRequest(options: DataQueryRequest, targets: TempoQuery[]): DataQueryRequest { const request = { ...options, targets, @@ -697,29 +719,6 @@ export class TempoDatasource extends DataSourceWithBackend, - targets: TempoQuery[], - query: string - ): Observable { - if (query === '') { - return EMPTY; - } - - return merge( - ...targets.map((target) => - doTempoChannelStream( - { ...target, query }, - this, // the datasource - options, - this.instanceSettings - ) - ) - ); - } - async metadataRequest(url: string, params = {}) { return await lastValueFrom(this._request(url, params, { method: 'GET', hideFromInspector: true })); } @@ -728,7 +727,6 @@ export class TempoDatasource extends DataSourceWithBackend): DataQ }; } -function getServiceGraphView( +function getServiceGraphViewDataFrames( request: DataQueryRequest, rateResponse: ServiceMapQueryResponseWithRates, secondResponse: DataQueryResponse, diff --git a/public/app/plugins/datasource/tempo/graphTransform.test.ts b/public/app/plugins/datasource/tempo/graphTransform.test.ts index efebee60cd6..7a5ab7ccac8 100644 --- a/public/app/plugins/datasource/tempo/graphTransform.test.ts +++ b/public/app/plugins/datasource/tempo/graphTransform.test.ts @@ -1,7 +1,7 @@ import { DataFrameView, dateTime, createDataFrame, FieldType } from '@grafana/data'; import { createGraphFrames, mapPromMetricsToServiceMap } from './graphTransform'; -import { bigResponse } from './testResponse'; +import { bigResponse } from './test/testResponse'; describe('createGraphFrames', () => { it('transforms basic response into nodes and edges frame', async () => { diff --git a/public/app/plugins/datasource/tempo/resultTransformer.test.ts b/public/app/plugins/datasource/tempo/resultTransformer.test.ts index 0c9c4750755..fc47f88dab2 100644 --- a/public/app/plugins/datasource/tempo/resultTransformer.test.ts +++ b/public/app/plugins/datasource/tempo/resultTransformer.test.ts @@ -14,7 +14,7 @@ import { otlpDataFrameFromResponse, otlpResponse, traceQlResponse, -} from './testResponse'; +} from './test/testResponse'; import { TraceSearchMetadata } from './types'; const defaultSettings: DataSourceInstanceSettings = { diff --git a/public/app/plugins/datasource/tempo/mockJsonResponse.json b/public/app/plugins/datasource/tempo/test/mockJsonResponse.json similarity index 100% rename from public/app/plugins/datasource/tempo/mockJsonResponse.json rename to public/app/plugins/datasource/tempo/test/mockJsonResponse.json diff --git a/public/app/plugins/datasource/tempo/mockServiceGraph.json b/public/app/plugins/datasource/tempo/test/mockServiceGraph.json similarity index 100% rename from public/app/plugins/datasource/tempo/mockServiceGraph.json rename to public/app/plugins/datasource/tempo/test/mockServiceGraph.json diff --git a/public/app/plugins/datasource/tempo/mocks.ts b/public/app/plugins/datasource/tempo/test/mocks.ts similarity index 94% rename from public/app/plugins/datasource/tempo/mocks.ts rename to public/app/plugins/datasource/tempo/test/mocks.ts index 8a9c568ac81..28fe7c66f91 100644 --- a/public/app/plugins/datasource/tempo/mocks.ts +++ b/public/app/plugins/datasource/tempo/test/mocks.ts @@ -1,8 +1,8 @@ import { DataSourceInstanceSettings, PluginType, toUtc } from '@grafana/data'; import { TemplateSrv } from '@grafana/runtime'; -import { TempoDatasource } from './datasource'; -import { TempoJsonData } from './types'; +import { TempoDatasource } from '../datasource'; +import { TempoJsonData } from '../types'; const rawRange = { from: toUtc('2018-04-25 10:00'), diff --git a/public/app/plugins/datasource/tempo/testResponse.ts b/public/app/plugins/datasource/tempo/test/testResponse.ts similarity index 100% rename from public/app/plugins/datasource/tempo/testResponse.ts rename to public/app/plugins/datasource/tempo/test/testResponse.ts diff --git a/public/app/plugins/datasource/tempo/test_utils.ts b/public/app/plugins/datasource/tempo/test/test_utils.ts similarity index 100% rename from public/app/plugins/datasource/tempo/test_utils.ts rename to public/app/plugins/datasource/tempo/test/test_utils.ts diff --git a/public/app/plugins/datasource/tempo/variables.test.ts b/public/app/plugins/datasource/tempo/variables.test.ts index 1b5f9fce94c..4a2215c4aae 100644 --- a/public/app/plugins/datasource/tempo/variables.test.ts +++ b/public/app/plugins/datasource/tempo/variables.test.ts @@ -3,7 +3,7 @@ import { lastValueFrom } from 'rxjs'; import { DataQueryRequest, TimeRange } from '@grafana/data'; import { TempoVariableQuery } from './VariableQueryEditor'; -import { createMetadataRequest, createTempoDatasource } from './mocks'; +import { createMetadataRequest, createTempoDatasource } from './test/mocks'; import { TempoVariableSupport } from './variables'; describe('TempoVariableSupport', () => {