From 4d2decfa0c0c9bb4df9e21d6c3eba1708a139a6d Mon Sep 17 00:00:00 2001 From: Brendan O'Handley Date: Mon, 25 Aug 2025 14:43:22 -0500 Subject: [PATCH] Tempo: automatic native histogram check for service graph (#108394) * add new dataquery field for identifying which histogram type * run make gen-cue to update tempo dataquery * add native histogram check function & look for dataquery flag * remove console log * check for native histograms in queryfield * only run query when histograms have not been migrated yet * use less resource intensive query, just check if series are there * fix type issue * use series as better for detecting existence of a metric, fix betterer things * fix type * fix import * remove metric name from func * use datasrource func and don't make a new one * remove string for boolean check in service graph functions * fix bug for switching tempo data sources * handle race condition for unmounting before async call is completed * fix imports * add todo to implement getNativeHistograms in Prometheus data source * add todo to remove automatic check once tempo fully migrates to native histograms * add todo to remove the native histogram config option --- .../dataquery/x/TempoDataQuery_types.gen.ts | 4 + .../kinds/dataquery/types_dataquery_gen.go | 2 + .../plugins/datasource/tempo/QueryField.tsx | 37 ++++++ .../configuration/ServiceGraphSettings.tsx | 1 + .../plugins/datasource/tempo/dataquery.cue | 2 + .../plugins/datasource/tempo/dataquery.gen.ts | 4 + .../datasource/tempo/datasource.test.ts | 26 +--- .../plugins/datasource/tempo/datasource.ts | 111 ++++++++++++------ public/app/plugins/datasource/tempo/types.ts | 1 + 9 files changed, 133 insertions(+), 55 deletions(-) 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 9f48e49018d..803b6edf1c4 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 @@ -54,6 +54,10 @@ export interface TempoQuery extends common.DataQuery { * Filters to be included in a PromQL query to select data for the service graph. Example: {client="app",service="app"}. Providing multiple values will produce union of results for each filter, using PromQL OR operator internally. */ serviceMapQuery?: (string | Array); + /** + * Whether to use native histograms for service map queries + */ + serviceMapUseNativeHistograms?: boolean; /** * @deprecated Query traces by service name */ diff --git a/pkg/tsdb/tempo/kinds/dataquery/types_dataquery_gen.go b/pkg/tsdb/tempo/kinds/dataquery/types_dataquery_gen.go index 8edddb0710a..b5f417e90b9 100644 --- a/pkg/tsdb/tempo/kinds/dataquery/types_dataquery_gen.go +++ b/pkg/tsdb/tempo/kinds/dataquery/types_dataquery_gen.go @@ -42,6 +42,8 @@ type TempoQuery struct { ServiceMapQuery *StringOrArrayOfString `json:"serviceMapQuery,omitempty"` // Use service.namespace in addition to service.name to uniquely identify a service. ServiceMapIncludeNamespace *bool `json:"serviceMapIncludeNamespace,omitempty"` + // Whether to use native histograms for service map queries + ServiceMapUseNativeHistograms *bool `json:"serviceMapUseNativeHistograms,omitempty"` // Defines the maximum number of traces that are returned from Tempo Limit *int64 `json:"limit,omitempty"` // Defines the maximum number of spans per spanset that are returned from Tempo diff --git a/public/app/plugins/datasource/tempo/QueryField.tsx b/public/app/plugins/datasource/tempo/QueryField.tsx index c0a47c8f79c..b3ec936518e 100644 --- a/public/app/plugins/datasource/tempo/QueryField.tsx +++ b/public/app/plugins/datasource/tempo/QueryField.tsx @@ -36,6 +36,8 @@ interface State { const DEFAULT_QUERY_TYPE: TempoQueryType = 'traceql'; class TempoQueryFieldComponent extends PureComponent { + private _isMounted = false; + constructor(props: Props) { super(props); this.state = { @@ -48,12 +50,47 @@ class TempoQueryFieldComponent extends PureComponent { // otherwise if the user changes the query type and refreshes the page, no query type will be selected // which is inconsistent with how the UI was originally when they selected the Tempo data source. async componentDidMount() { + this._isMounted = true; + if (!this.props.query.queryType || this.props.query.queryType === 'clear') { this.props.onChange({ ...this.props.query, queryType: DEFAULT_QUERY_TYPE, }); } + // TODO: Remove this automatic check for native histograms once Tempo only supports native histograms https://github.com/grafana/grafana/issues/109708 + // indentify the service map can use native histograms + const timeRange = this.props.range; + const nativeHistograms = await this.props.datasource.getNativeHistograms(timeRange); + + // Only update if component is still mounted + if (!this._isMounted) { + return; + } + + this.props.onChange({ + ...this.props.query, + serviceMapUseNativeHistograms: nativeHistograms, + }); + // Migrate to native histograms + // this will ensure that on navigating to the query option service map from a url, + // the service map will be rendered with the native histograms when + // querytype is serviceMap + // the serviceMapUseNativeHistograms is undefined + // and nativeHistograms is true + if ( + this.props.query.queryType === 'serviceMap' && + this.props.query.serviceMapUseNativeHistograms === undefined && + // switch from tempo with native histograms to tempo without native histograms + this.props.query.serviceMapUseNativeHistograms !== nativeHistograms && + nativeHistograms + ) { + this.props.onRunQuery(); + } + } + + componentWillUnmount() { + this._isMounted = false; } onClearResults = () => { diff --git a/public/app/plugins/datasource/tempo/configuration/ServiceGraphSettings.tsx b/public/app/plugins/datasource/tempo/configuration/ServiceGraphSettings.tsx index 4eebfdaffca..bc7cdea6a72 100644 --- a/public/app/plugins/datasource/tempo/configuration/ServiceGraphSettings.tsx +++ b/public/app/plugins/datasource/tempo/configuration/ServiceGraphSettings.tsx @@ -94,6 +94,7 @@ export function ServiceGraphSettings({ options, onOptionsChange }: Props) { ) : null} + {/* TODO: Remove this in favor of automatic detection of native histograms https://github.com/grafana/grafana/issues/109709 */} ); + /** + * Whether to use native histograms for service map queries + */ + serviceMapUseNativeHistograms?: boolean; /** * @deprecated Query traces by service name */ diff --git a/public/app/plugins/datasource/tempo/datasource.test.ts b/public/app/plugins/datasource/tempo/datasource.test.ts index 72c82f356b0..437264cf282 100644 --- a/public/app/plugins/datasource/tempo/datasource.test.ts +++ b/public/app/plugins/datasource/tempo/datasource.test.ts @@ -1267,7 +1267,7 @@ describe('histogram type functionality', () => { const target = 'server="${__data.fields.target}"'; const serverSumBy = 'server'; - const links = makeHistogramLink(datasourceUid, source, target, serverSumBy); + const links = makeHistogramLink(datasourceUid, source, target, serverSumBy, false); expect(links).toHaveLength(1); expect(links[0].title).toBe('Request classic histogram'); expect(links[0].internal.query.expr).toBe( @@ -1281,7 +1281,7 @@ describe('histogram type functionality', () => { const target = 'server="${__data.fields.target}"'; const serverSumBy = 'server'; - const links = makeHistogramLink(datasourceUid, source, target, serverSumBy, 'native'); + const links = makeHistogramLink(datasourceUid, source, target, serverSumBy, true); expect(links).toHaveLength(1); expect(links[0].title).toBe('Request native histogram'); expect(links[0].internal.query.expr).toBe( @@ -1289,24 +1289,6 @@ describe('histogram type functionality', () => { ); }); - it('should create correct histogram links for both histogram types', () => { - const datasourceUid = 'prom'; - const source = 'client="${__data.fields.source}",'; - const target = 'server="${__data.fields.target}"'; - const serverSumBy = 'server'; - - const links = makeHistogramLink(datasourceUid, source, target, serverSumBy, 'both'); - expect(links).toHaveLength(2); - expect(links[0].title).toBe('Request classic histogram'); - expect(links[1].title).toBe('Request native histogram'); - expect(links[0].internal.query.expr).toBe( - 'histogram_quantile(0.9, sum(rate(traces_service_graph_request_server_seconds_bucket{client="${__data.fields.source}",server="${__data.fields.target}"}[$__rate_interval])) by (le, client, server))' - ); - expect(links[1].internal.query.expr).toBe( - 'histogram_quantile(0.9, sum(rate(traces_service_graph_request_server_seconds{client="${__data.fields.source}",server="${__data.fields.target}"}[$__rate_interval])) by (le, client, server))' - ); - }); - it('should include histogram type in field config', () => { const datasourceUid = 'prom'; const tempoDatasourceUid = 'tempo'; @@ -1321,7 +1303,7 @@ describe('histogram type functionality', () => { tempoField, sourceField, undefined, - 'native' + true ); const histogramLink = fieldConfig.links.find((link) => link.title === 'Request native histogram'); expect(histogramLink).toBeDefined(); @@ -1339,7 +1321,7 @@ describe('histogram type functionality', () => { targets: [{ serviceMapQuery: '{service="test"}' }], range: getDefaultTimeRange(), } as DataQueryRequest, - 'native' + true ); const bucketMetric = request.targets.find((t: PromQuery) => t.expr.includes('_bucket')); diff --git a/public/app/plugins/datasource/tempo/datasource.ts b/public/app/plugins/datasource/tempo/datasource.ts index b7c3af81613..6143950be47 100644 --- a/public/app/plugins/datasource/tempo/datasource.ts +++ b/public/app/plugins/datasource/tempo/datasource.ts @@ -289,6 +289,52 @@ export class TempoDatasource extends DataSourceWithBackend { + if (!this.serviceMap?.datasourceUid) { + return false; + } + + // remove _bucket from the metric name to get the native histogram metric name + const metricName = histogramMetric.replace('_bucket', ''); + + try { + // Get the Prometheus datasource instance + const promDs = await getDataSourceSrv().get(this.serviceMap.datasourceUid); + // Use provided time range or default to last hour + const from = timeRange?.from || dateTime().subtract(1, 'hour'); + const to = timeRange?.to || dateTime(); + + // Convert to Unix timestamps (seconds since epoch) + const start = Math.floor(from.valueOf() / 1000); + const end = Math.floor(to.valueOf() / 1000); + + // Use the series endpoint to check if native histogram metrics exist + // this has a 90% chance of returning correctly due to sparse data + if (!('metadataRequest' in promDs) || typeof promDs.metadataRequest !== 'function') { + return false; + } + + const seriesResult = await promDs.metadataRequest('/api/v1/series', { + 'match[]': metricName, + limit: 1, + start: start, + end: end, + }); + + // Check if any native histogram series exist + const seriesData = seriesResult?.data?.data; + if (seriesData && Array.isArray(seriesData)) { + // If the series array has any entries, native histograms exist + return seriesData.length > 0; + } + + return false; + } catch (error) { + console.warn('Failed to check for native histograms:', error); + return false; + } + } /** * Check, for the given feature, whether it is available in Grafana. @@ -539,13 +585,20 @@ export class TempoDatasource extends DataSourceWithBackend rateQuery(options, result, datasourceUid).pipe( - concatMap((result) => errorAndDurationQuery(options, result, datasourceUid, tempoDsUid, histogramType)) + concatMap((result) => + errorAndDurationQuery(options, result, datasourceUid, tempoDsUid, useNativeHistogram) + ) ) ) ) @@ -974,9 +1027,9 @@ function serviceMapQuery( request: DataQueryRequest, datasourceUid: string, tempoDatasourceUid: string, - histogramType?: string + useNativeHistogram?: boolean ): Observable { - const serviceMapRequest = makePromServiceMapRequest(request, histogramType); + const serviceMapRequest = makePromServiceMapRequest(request, useNativeHistogram); return queryPrometheus(serviceMapRequest, datasourceUid).pipe( // Just collect all the responses first before processing into node graph data @@ -1014,7 +1067,7 @@ function serviceMapQuery( '__data.fields[0]', // tempoField undefined, // sourceField { targetNamespace: '__data.fields.subtitle' }, - histogramType + useNativeHistogram ); edges.fields[0].config = getFieldConfig( @@ -1024,7 +1077,7 @@ function serviceMapQuery( '__data.fields.target', // tempoField '__data.fields.sourceName', // sourceField { targetNamespace: '__data.fields.targetNamespace', sourceNamespace: '__data.fields.sourceNamespace' }, - histogramType + useNativeHistogram ); } else { nodes.fields[0].config = getFieldConfig( @@ -1034,7 +1087,7 @@ function serviceMapQuery( '__data.fields[0]', undefined, undefined, - histogramType + useNativeHistogram ); edges.fields[0].config = getFieldConfig( datasourceUid, @@ -1043,7 +1096,7 @@ function serviceMapQuery( '__data.fields.target', '__data.fields.source', undefined, - histogramType + useNativeHistogram ); } @@ -1060,9 +1113,9 @@ function rateQuery( request: DataQueryRequest, serviceMapResponse: ServiceMapQueryResponse, datasourceUid: string, - histogramType?: string + useNativeHistogram?: boolean ): Observable { - const serviceMapRequest = makePromServiceMapRequest(request, histogramType); + const serviceMapRequest = makePromServiceMapRequest(request, useNativeHistogram); serviceMapRequest.targets = makeServiceGraphViewRequest([buildExpr(rateMetric, defaultTableFilter, request)]); return queryPrometheus(serviceMapRequest, datasourceUid).pipe( @@ -1088,7 +1141,7 @@ function errorAndDurationQuery( rateResponse: ServiceMapQueryResponseWithRates, datasourceUid: string, tempoDatasourceUid: string, - histogramType?: string + useNativeHistogram?: boolean ) { let serviceGraphViewMetrics = []; let errorRateBySpanName = ''; @@ -1114,14 +1167,14 @@ function errorAndDurationQuery( errorRateBySpanName = buildExpr(errorRateMetric, 'span_name=~"' + spanNames.join('|') + '"', request); serviceGraphViewMetrics.push(errorRateBySpanName); spanNames.map((name: string) => { - const checkedDurationMetric = histogramType === 'native' ? nativeHistogramDurationMetric : durationMetric; + const checkedDurationMetric = useNativeHistogram ? nativeHistogramDurationMetric : durationMetric; const metric = buildExpr(checkedDurationMetric, 'span_name=~"' + name + '"', request); durationsBySpanName.push(metric); serviceGraphViewMetrics.push(metric); }); } - const serviceMapRequest = makePromServiceMapRequest(request, histogramType); + const serviceMapRequest = makePromServiceMapRequest(request, useNativeHistogram); serviceMapRequest.targets = makeServiceGraphViewRequest(serviceGraphViewMetrics); return queryPrometheus(serviceMapRequest, datasourceUid).pipe( @@ -1141,7 +1194,7 @@ function errorAndDurationQuery( durationsBySpanName, datasourceUid, tempoDatasourceUid, - histogramType + useNativeHistogram ); if (serviceGraphView.fields.length === 0) { @@ -1193,7 +1246,7 @@ export function getFieldConfig( tempoField: string, sourceField?: string, namespaceFields?: { targetNamespace: string; sourceNamespace?: string }, - histogramType?: string + useNativeHistogram?: boolean ) { let source = sourceField ? `client="\${${sourceField}}",` : ''; let target = `server="\${${targetField}}"`; @@ -1219,7 +1272,7 @@ export function getFieldConfig( datasourceUid, false ), - ...makeHistogramLink(datasourceUid, source, target, serverSumBy, histogramType), + ...makeHistogramLink(datasourceUid, source, target, serverSumBy, useNativeHistogram), makePromLink( 'Failed request rate', `sum by (client, ${serverSumBy})(rate(${failedMetric}{${source}${target}}[$__rate_interval]))`, @@ -1241,7 +1294,7 @@ export function makeHistogramLink( source: string, target: string, serverSumBy: string, - histogramType?: string + useNativeHistogram?: boolean ) { const createHistogramLink = (metric: string, title: string) => makePromLink( @@ -1250,18 +1303,10 @@ export function makeHistogramLink( datasourceUid, false ); - - switch (histogramType) { - case 'both': - return [ - createHistogramLink(histogramMetric, 'Request classic histogram'), - createHistogramLink(nativeHistogramMetric, 'Request native histogram'), - ]; - case 'native': - return [createHistogramLink(nativeHistogramMetric, 'Request native histogram')]; - default: - return [createHistogramLink(histogramMetric, 'Request classic histogram')]; + if (useNativeHistogram) { + return [createHistogramLink(nativeHistogramMetric, 'Request native histogram')]; } + return [createHistogramLink(histogramMetric, 'Request classic histogram')]; } export function makeTempoLink( @@ -1372,13 +1417,13 @@ function makeTempoLinkServiceMap( export function makePromServiceMapRequest( options: DataQueryRequest, - histogramType?: string + useNativeHistogram?: boolean ): DataQueryRequest { return { ...options, targets: serviceMapMetrics .map((metric) => { - if (histogramType === 'native' && metric.includes('_bucket')) { + if (useNativeHistogram) { metric = metric.replace('_bucket', ''); } const { serviceMapQuery, serviceMapIncludeNamespace: serviceMapIncludeNamespace } = options.targets[0]; @@ -1422,7 +1467,7 @@ function getServiceGraphViewDataFrames( durationsBySpanName: string[], datasourceUid: string, tempoDatasourceUid: string, - histogramType?: string + useNativeHistogram?: boolean ) { let df: any = { fields: [] }; @@ -1547,7 +1592,7 @@ function getServiceGraphViewDataFrames( } }); if (Object.keys(durationObj).length > 0) { - const checkedDurationMetric = histogramType === 'native' ? nativeHistogramDurationMetric : durationMetric; + const checkedDurationMetric = useNativeHistogram ? nativeHistogramDurationMetric : durationMetric; df.fields.push({ ...duration[0].fields[1], name: 'Duration (p90)', diff --git a/public/app/plugins/datasource/tempo/types.ts b/public/app/plugins/datasource/tempo/types.ts index 502f64ab8a7..ef7eaae06ac 100644 --- a/public/app/plugins/datasource/tempo/types.ts +++ b/public/app/plugins/datasource/tempo/types.ts @@ -31,6 +31,7 @@ export interface TempoJsonData extends DataSourceJsonData { export interface TempoQuery extends TempoBase { queryType: TempoQueryType; + serviceMapUseNativeHistograms?: boolean; } export interface MyDataSourceOptions extends DataSourceJsonData {}