From 8bf914ac0b61c32b71191d7ce701e951c033564b Mon Sep 17 00:00:00 2001 From: Andre Pereira Date: Mon, 9 Oct 2023 14:22:39 +0100 Subject: [PATCH] Tempo: Fix service graph menu item links (#75748) * Only call preventDefault if it exists * Change "View Traces" link to use traceQLSearch instead of the deprecated nativeSearch * Thank you again test. Update tests * Update test * Update betterer * Type fix * Small type change * Update betterer --- .betterer.results | 5 +- .../dataquery/x/TempoDataQuery_types.gen.ts | 2 +- .../kinds/dataquery/types_dataquery_gen.go | 2 +- pkg/tsdb/tempo/trace.go | 8 ++- .../SearchTraceQLEditor/SearchField.test.tsx | 1 - .../tempo/SearchTraceQLEditor/SearchField.tsx | 3 + .../plugins/datasource/tempo/dataquery.cue | 2 +- .../plugins/datasource/tempo/dataquery.gen.ts | 2 +- .../datasource/tempo/datasource.test.ts | 60 +++++++++++++++---- .../plugins/datasource/tempo/datasource.ts | 44 +++++++++----- .../datasource/tempo/traceql/QueryEditor.tsx | 2 +- .../app/plugins/datasource/tempo/tracking.ts | 2 +- 12 files changed, 96 insertions(+), 37 deletions(-) diff --git a/.betterer.results b/.betterer.results index be66d2848e2..3fb02eb3288 100644 --- a/.betterer.results +++ b/.betterer.results @@ -7027,11 +7027,10 @@ exports[`better eslint`] = { [0, 0, 0, "Do not use any type assertions.", "10"], [0, 0, 0, "Do not use any type assertions.", "11"], [0, 0, 0, "Do not use any type assertions.", "12"], - [0, 0, 0, "Do not use any type assertions.", "13"], + [0, 0, 0, "Unexpected any. Specify a different type.", "13"], [0, 0, 0, "Unexpected any. Specify a different type.", "14"], [0, 0, 0, "Unexpected any. Specify a different type.", "15"], - [0, 0, 0, "Unexpected any. Specify a different type.", "16"], - [0, 0, 0, "Unexpected any. Specify a different type.", "17"] + [0, 0, 0, "Unexpected any. Specify a different type.", "16"] ], "public/app/plugins/datasource/tempo/language_provider.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"] diff --git a/packages/grafana-schema/src/raw/composable/tempo/dataquery/x/TempoDataQuery_types.gen.ts b/packages/grafana-schema/src/raw/composable/tempo/dataquery/x/TempoDataQuery_types.gen.ts index 721b556f4df..91f136e78f6 100644 --- a/packages/grafana-schema/src/raw/composable/tempo/dataquery/x/TempoDataQuery_types.gen.ts +++ b/packages/grafana-schema/src/raw/composable/tempo/dataquery/x/TempoDataQuery_types.gen.ts @@ -34,7 +34,7 @@ export interface TempoQuery extends common.DataQuery { /** * TraceQL query or trace ID */ - query: string; + query?: string; /** * @deprecated Logfmt query to filter traces by their tags. Example: http.status_code=200 error=true */ diff --git a/pkg/tsdb/tempo/kinds/dataquery/types_dataquery_gen.go b/pkg/tsdb/tempo/kinds/dataquery/types_dataquery_gen.go index 6c6c80f5883..e635751839d 100644 --- a/pkg/tsdb/tempo/kinds/dataquery/types_dataquery_gen.go +++ b/pkg/tsdb/tempo/kinds/dataquery/types_dataquery_gen.go @@ -109,7 +109,7 @@ type TempoQuery struct { MinDuration *string `json:"minDuration,omitempty"` // TraceQL query or trace ID - Query string `json:"query"` + Query *string `json:"query,omitempty"` // Specify the query flavor // TODO make this required and give it a default diff --git a/pkg/tsdb/tempo/trace.go b/pkg/tsdb/tempo/trace.go index 271d7146720..9bd4ff33f6e 100644 --- a/pkg/tsdb/tempo/trace.go +++ b/pkg/tsdb/tempo/trace.go @@ -28,7 +28,11 @@ func (s *Service) getTrace(ctx context.Context, pCtx backend.PluginContext, quer return nil, err } - request, err := s.createRequest(ctx, dsInfo, model.Query, query.TimeRange.From.Unix(), query.TimeRange.To.Unix()) + if model.Query == nil || *model.Query == "" { + return result, fmt.Errorf("trace id is required") + } + + request, err := s.createRequest(ctx, dsInfo, *model.Query, query.TimeRange.From.Unix(), query.TimeRange.To.Unix()) if err != nil { return result, err } @@ -50,7 +54,7 @@ func (s *Service) getTrace(ctx context.Context, pCtx backend.PluginContext, quer } if resp.StatusCode != http.StatusOK { - result.Error = fmt.Errorf("failed to get trace with id: %s Status: %s Body: %s", model.Query, resp.Status, string(body)) + result.Error = fmt.Errorf("failed to get trace with id: %v Status: %s Body: %s", model.Query, resp.Status, string(body)) return result, nil } diff --git a/public/app/plugins/datasource/tempo/SearchTraceQLEditor/SearchField.test.tsx b/public/app/plugins/datasource/tempo/SearchTraceQLEditor/SearchField.test.tsx index 7fbd097a83a..89bb1ad8be6 100644 --- a/public/app/plugins/datasource/tempo/SearchTraceQLEditor/SearchField.test.tsx +++ b/public/app/plugins/datasource/tempo/SearchTraceQLEditor/SearchField.test.tsx @@ -90,7 +90,6 @@ describe('SearchField', () => { }); const filter: TraceqlFilter = { id: 'test1', - value: 'old', valueType: 'string', tag: 'test-tag', }; diff --git a/public/app/plugins/datasource/tempo/SearchTraceQLEditor/SearchField.tsx b/public/app/plugins/datasource/tempo/SearchTraceQLEditor/SearchField.tsx index 3f4858b5ed4..d8b4a741ce1 100644 --- a/public/app/plugins/datasource/tempo/SearchTraceQLEditor/SearchField.tsx +++ b/public/app/plugins/datasource/tempo/SearchTraceQLEditor/SearchField.tsx @@ -81,6 +81,9 @@ const SearchField = ({ setError, query, ]); + if (filter.value && options && !options.find((o) => o === filter.value)) { + options.push({ label: filter.value.toString(), value: filter.value.toString(), type: filter.valueType }); + } useEffect(() => { if (Array.isArray(filter.value) && filter.value.length > 1 && filter.operator !== '=~') { diff --git a/public/app/plugins/datasource/tempo/dataquery.cue b/public/app/plugins/datasource/tempo/dataquery.cue index 358b88d9c6d..c94c9a16254 100644 --- a/public/app/plugins/datasource/tempo/dataquery.cue +++ b/public/app/plugins/datasource/tempo/dataquery.cue @@ -27,7 +27,7 @@ composableKinds: DataQuery: { schema: { #TempoQuery: common.DataQuery & { // TraceQL query or trace ID - query: string + query?: string // @deprecated Logfmt query to filter traces by their tags. Example: http.status_code=200 error=true search?: string // @deprecated Query traces by service name diff --git a/public/app/plugins/datasource/tempo/dataquery.gen.ts b/public/app/plugins/datasource/tempo/dataquery.gen.ts index b1e743f4bb2..7cfa4c022b4 100644 --- a/public/app/plugins/datasource/tempo/dataquery.gen.ts +++ b/public/app/plugins/datasource/tempo/dataquery.gen.ts @@ -31,7 +31,7 @@ export interface TempoQuery extends common.DataQuery { /** * TraceQL query or trace ID */ - query: string; + query?: string; /** * @deprecated Logfmt query to filter traces by their tags. Example: http.status_code=200 error=true */ diff --git a/public/app/plugins/datasource/tempo/datasource.test.ts b/public/app/plugins/datasource/tempo/datasource.test.ts index 91c8db8bd50..7a41119d45e 100644 --- a/public/app/plugins/datasource/tempo/datasource.test.ts +++ b/public/app/plugins/datasource/tempo/datasource.test.ts @@ -539,8 +539,8 @@ describe('Tempo service graph view', () => { expect(response.data[0].fields[6].config.links[0].url).toBe(''); expect(response.data[0].fields[6].config.links[0].title).toBe('Tempo'); - expect(response.data[0].fields[6].config.links[0].internal.query.queryType).toBe('nativeSearch'); - expect(response.data[0].fields[6].config.links[0].internal.query.spanName).toBe('${__data.fields[0]}'); + expect(response.data[0].fields[6].config.links[0].internal.query.queryType).toBe('traceqlSearch'); + expect(response.data[0].fields[6].config.links[0].internal.query.filters[0].value).toBe('${__data.fields[0]}'); // Service graph expect(response.data[1].name).toBe('Nodes'); @@ -683,8 +683,18 @@ describe('Tempo service graph view', () => { title: 'View traces', internal: { query: { - queryType: 'nativeSearch', - serviceName: '${__data.fields.target}', + refId: 'A', + queryType: 'traceqlSearch', + filters: [ + { + id: 'service-name', + operator: '=', + scope: 'resource', + tag: 'service.name', + value: '${__data.fields.target}', + valueType: 'string', + }, + ], }, datasourceUid: 'EbPO1fYnz', datasourceName: '', @@ -764,8 +774,18 @@ describe('Tempo service graph view', () => { title: 'View traces', internal: { query: { - queryType: 'nativeSearch', - serviceName: '${__data.fields.target}', + queryType: 'traceqlSearch', + refId: 'A', + filters: [ + { + id: 'service-name', + operator: '=', + scope: 'resource', + tag: 'service.name', + value: '${__data.fields.target}', + valueType: 'string', + }, + ], }, datasourceUid: 'EbPO1fYnz', datasourceName: '', @@ -842,8 +862,18 @@ describe('Tempo service graph view', () => { title: 'Tempo', internal: { query: { - queryType: 'nativeSearch', - spanName: '"${__data.fields[0]}"', + queryType: 'traceqlSearch', + refId: 'A', + filters: [ + { + id: 'span-name', + operator: '=', + scope: 'span', + tag: 'name', + value: '"${__data.fields[0]}"', + valueType: 'string', + }, + ], }, datasourceUid: 'gdev-tempo', datasourceName: 'Tempo', @@ -1169,8 +1199,18 @@ const serviceGraphLinks = [ title: 'View traces', internal: { query: { - queryType: 'nativeSearch', - serviceName: '${__data.fields[0]}', + refId: 'A', + queryType: 'traceqlSearch', + filters: [ + { + id: 'service-name', + operator: '=', + scope: 'resource', + tag: 'service.name', + value: '${__data.fields[0]}', + valueType: 'string', + }, + ], } as TempoQuery, datasourceUid: 'gdev-tempo', datasourceName: 'Tempo', diff --git a/public/app/plugins/datasource/tempo/datasource.ts b/public/app/plugins/datasource/tempo/datasource.ts index 9019cca45d1..47b6190227e 100644 --- a/public/app/plugins/datasource/tempo/datasource.ts +++ b/public/app/plugins/datasource/tempo/datasource.ts @@ -1,4 +1,4 @@ -import { identity, pick, pickBy, groupBy, startCase } from 'lodash'; +import { groupBy, identity, pick, pickBy, startCase } from 'lodash'; import { EMPTY, from, lastValueFrom, merge, Observable, of, throwError } from 'rxjs'; import { catchError, concatMap, map, mergeMap, toArray } from 'rxjs/operators'; import semver from 'semver'; @@ -20,13 +20,13 @@ import { ScopedVars, } from '@grafana/data'; import { - config, BackendSrvRequest, + config, DataSourceWithBackend, getBackendSrv, + getTemplateSrv, reportInteraction, TemplateSrv, - getTemplateSrv, } from '@grafana/runtime'; import { BarGaugeDisplayMode, TableCellDisplayMode, VariableFormatID } from '@grafana/schema'; import { NodeGraphOptions } from 'app/core/components/NodeGraphSettings'; @@ -43,27 +43,27 @@ import { generateQueryFromFilters } from './SearchTraceQLEditor/utils'; import { TempoVariableQuery, TempoVariableQueryType } from './VariableQueryEditor'; import { TraceqlFilter, TraceqlSearchScope } from './dataquery.gen'; import { + defaultTableFilter, + durationMetric, + errorRateMetric, failedMetric, histogramMetric, mapPromMetricsToServiceMap, + rateMetric, serviceMapMetrics, totalsMetric, - rateMetric, - durationMetric, - errorRateMetric, - defaultTableFilter, } from './graphTransform'; import TempoLanguageProvider from './language_provider'; import { createTableFrameFromMetricsSummaryQuery, emptyResponse, MetricsSummary } from './metricsSummary'; import { + createTableFrameFromSearch, + transformFromOTLP as transformFromOTEL, transformTrace, transformTraceList, - transformFromOTLP as transformFromOTEL, - createTableFrameFromSearch, formatTraceQLResponse, } from './resultTransformer'; import { doTempoChannelStream } from './streaming'; -import { SearchQueryParams, TempoQuery, TempoJsonData } from './types'; +import { SearchQueryParams, TempoJsonData, TempoQuery } from './types'; import { getErrorMessage } from './utils'; import { TempoVariableSupport } from './variables'; @@ -608,7 +608,7 @@ export class TempoDatasource extends DataSourceWithBackend, targets: TempoQuery[]): Observable { const validTargets = targets .filter((t) => t.query) - .map((t): TempoQuery => ({ ...t, query: t.query.trim(), queryType: 'traceId' })); + .map((t): TempoQuery => ({ ...t, query: t.query?.trim(), queryType: 'traceId' })); if (!validTargets.length) { return EMPTY; } @@ -713,7 +713,7 @@ export class TempoDatasource extends DataSourceWithBackend { +const hasTemplateVariables = (val?: string): boolean => { return getTemplateSrv().containsTemplate(val); };