From 70d49b017d2776138cd481f3705406ecd33d14b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zolt=C3=A1n=20Bedi?= Date: Tue, 6 Apr 2021 18:35:00 +0200 Subject: [PATCH] Prometheus: Fix instant query to run two times when exemplars enabled (#32508) * Prometheus: Fix instant query to run two times when exemplars enabled * Update exemplar messages * Tempo: show empty response if response is empty --- .../components/PromExemplarField.tsx | 22 +++++++--------- .../components/PromExploreExtraField.tsx | 6 ++++- .../prometheus/components/PromQueryEditor.tsx | 13 +++++++--- .../PromQueryEditor.test.tsx.snap | 12 ++------- .../datasource/prometheus/datasource.test.ts | 26 +++++++++++++++++++ .../datasource/prometheus/datasource.ts | 22 ++++++++++------ .../plugins/datasource/tempo/datasource.ts | 23 ++++++++++++++++ 7 files changed, 90 insertions(+), 34 deletions(-) diff --git a/public/app/plugins/datasource/prometheus/components/PromExemplarField.tsx b/public/app/plugins/datasource/prometheus/components/PromExemplarField.tsx index 1c66354ec51..645238a857e 100644 --- a/public/app/plugins/datasource/prometheus/components/PromExemplarField.tsx +++ b/public/app/plugins/datasource/prometheus/components/PromExemplarField.tsx @@ -1,49 +1,47 @@ import { GrafanaTheme } from '@grafana/data'; -import { FetchError } from '@grafana/runtime'; import { IconButton, InlineLabel, Tooltip, useStyles } from '@grafana/ui'; import { css, cx } from '@emotion/css'; import React, { useEffect, useState } from 'react'; import { PrometheusDatasource } from '../datasource'; -import { PromQuery } from '../types'; interface Props { - query: PromQuery; - onChange: (value: PromQuery) => void; + isEnabled: boolean; + onChange: (isEnabled: boolean) => void; datasource: PrometheusDatasource; } -export function PromExemplarField(props: Props) { - const [error, setError] = useState(); +export function PromExemplarField({ datasource, onChange, isEnabled }: Props) { + const [error, setError] = useState(); const styles = useStyles(getStyles); useEffect(() => { - const subscription = props.datasource.exemplarErrors.subscribe((err) => { + const subscription = datasource.exemplarErrors.subscribe((err) => { setError(err); }); return () => { subscription.unsubscribe(); }; - }, [props]); + }, [datasource]); const iconButtonStyles = cx( { - [styles.activeIcon]: !!props.query.exemplar, + [styles.activeIcon]: isEnabled, }, styles.eyeIcon ); return ( - +
Exemplars { - props.onChange({ ...props.query, exemplar: !props.query.exemplar }); + onChange(!isEnabled); }} />
diff --git a/public/app/plugins/datasource/prometheus/components/PromExploreExtraField.tsx b/public/app/plugins/datasource/prometheus/components/PromExploreExtraField.tsx index 4ca04e831a8..283b96e2077 100644 --- a/public/app/plugins/datasource/prometheus/components/PromExploreExtraField.tsx +++ b/public/app/plugins/datasource/prometheus/components/PromExploreExtraField.tsx @@ -77,7 +77,11 @@ export const PromExploreExtraField: React.FC = memo( /> - + onChange({ ...query, exemplar: isEnabled })} + datasource={datasource} + /> ); } diff --git a/public/app/plugins/datasource/prometheus/components/PromQueryEditor.tsx b/public/app/plugins/datasource/prometheus/components/PromQueryEditor.tsx index 6bfb30523de..e12f2293962 100644 --- a/public/app/plugins/datasource/prometheus/components/PromQueryEditor.tsx +++ b/public/app/plugins/datasource/prometheus/components/PromQueryEditor.tsx @@ -32,6 +32,7 @@ interface State { interval?: string; intervalFactorOption: SelectableValue; instant: boolean; + exemplar: boolean; } export class PromQueryEditor extends PureComponent { @@ -55,6 +56,7 @@ export class PromQueryEditor extends PureComponent { INTERVAL_FACTOR_OPTIONS.find((option) => option.value === query.intervalFactor) || INTERVAL_FACTOR_OPTIONS[0], // Switch options instant: Boolean(query.instant), + exemplar: Boolean(query.exemplar), }; } @@ -90,6 +92,11 @@ export class PromQueryEditor extends PureComponent { this.setState({ legendFormat }); }; + onExemplarChange = (isEnabled: boolean) => { + this.query.exemplar = isEnabled; + this.setState({ exemplar: isEnabled }, this.onRunQuery); + }; + onRunQuery = () => { const { query } = this; // Change of query.hide happens outside of this component and is just passed as prop. We have to update it when running queries. @@ -99,8 +106,8 @@ export class PromQueryEditor extends PureComponent { }; render() { - const { datasource, query, range, data, onChange } = this.props; - const { formatOption, instant, interval, intervalFactorOption, legendFormat } = this.state; + const { datasource, query, range, data } = this.props; + const { formatOption, instant, interval, intervalFactorOption, legendFormat, exemplar } = this.state; return (
@@ -186,7 +193,7 @@ export class PromQueryEditor extends PureComponent {
- + ); diff --git a/public/app/plugins/datasource/prometheus/components/__snapshots__/PromQueryEditor.test.tsx.snap b/public/app/plugins/datasource/prometheus/components/__snapshots__/PromQueryEditor.test.tsx.snap index 1929959c069..4b6beed89eb 100644 --- a/public/app/plugins/datasource/prometheus/components/__snapshots__/PromQueryEditor.test.tsx.snap +++ b/public/app/plugins/datasource/prometheus/components/__snapshots__/PromQueryEditor.test.tsx.snap @@ -189,16 +189,8 @@ exports[`Render PromQueryEditor with basic options should render 1`] = ` "getPrometheusTime": [MockFunction], } } - onChange={[MockFunction]} - query={ - Object { - "exemplar": true, - "expr": "", - "interval": "", - "legendFormat": "", - "refId": "A", - } - } + isEnabled={true} + onChange={[Function]} /> diff --git a/public/app/plugins/datasource/prometheus/datasource.test.ts b/public/app/plugins/datasource/prometheus/datasource.test.ts index fec60510b27..4a9f9263d7e 100644 --- a/public/app/plugins/datasource/prometheus/datasource.test.ts +++ b/public/app/plugins/datasource/prometheus/datasource.test.ts @@ -1746,6 +1746,32 @@ describe('prepareTargets', () => { }); expect(activeTargets[0]).toEqual(target); }); + it('should give back 2 targets when exemplar enabled', () => { + const target: PromQuery = { + refId: 'A', + expr: 'up', + exemplar: true, + }; + + const { queries, activeTargets } = getPrepareTargetsContext(target); + expect(queries).toHaveLength(2); + expect(activeTargets).toHaveLength(2); + expect(activeTargets[0].exemplar).toBe(true); + expect(activeTargets[1].exemplar).toBe(false); + }); + it('should give back 1 target when exemplar and instant are enabled', () => { + const target: PromQuery = { + refId: 'A', + expr: 'up', + exemplar: true, + instant: true, + }; + + const { queries, activeTargets } = getPrepareTargetsContext(target); + expect(queries).toHaveLength(1); + expect(activeTargets).toHaveLength(1); + expect(activeTargets[0].instant).toBe(true); + }); }); describe('when run from Explore', () => { diff --git a/public/app/plugins/datasource/prometheus/datasource.ts b/public/app/plugins/datasource/prometheus/datasource.ts index 4f143323c96..5486588207d 100644 --- a/public/app/plugins/datasource/prometheus/datasource.ts +++ b/public/app/plugins/datasource/prometheus/datasource.ts @@ -44,7 +44,8 @@ import { PrometheusVariableSupport } from './variables'; import PrometheusMetricFindQuery from './metric_find_query'; export const ANNOTATION_QUERY_STEP_DEFAULT = '60s'; -const GET_AND_POST_MEDATADATA_ENDPOINTS = ['api/v1/query', 'api/v1/query_range', 'api/v1/series', 'api/v1/labels']; +const EXEMPLARS_NOT_AVAILABLE = 'Exemplars for this data source are not available.'; +const GET_AND_POST_METADATA_ENDPOINTS = ['api/v1/query', 'api/v1/query_range', 'api/v1/series', 'api/v1/labels']; export class PrometheusDatasource extends DataSourceApi { type: string; @@ -62,7 +63,7 @@ export class PrometheusDatasource extends DataSourceApi exemplarTraceIdDestinations: ExemplarTraceIdDestination[] | undefined; lookupsDisabled: boolean; customQueryParameters: any; - exemplarErrors: Subject = new Subject(); + exemplarErrors: Subject = new Subject(); constructor( instanceSettings: DataSourceInstanceSettings, @@ -147,7 +148,7 @@ export class PrometheusDatasource extends DataSourceApi } // If URL includes endpoint that supports POST and GET method, try to use configured method. This might fail as POST is supported only in v2.10+. - if (GET_AND_POST_MEDATADATA_ENDPOINTS.some((endpoint) => url.includes(endpoint))) { + if (GET_AND_POST_METADATA_ENDPOINTS.some((endpoint) => url.includes(endpoint))) { try { return await this._request(url, data, { method: this.httpMethod, hideFromInspector: true }).toPromise(); } catch (err) { @@ -238,12 +239,17 @@ export class PrometheusDatasource extends DataSourceApi queries.push(this.createQuery(instantTarget, options, start, end)); activeTargets.push(instantTarget); } else { - if (target.exemplar) { + // It doesn't make sense to query for exemplars in dashboard if only instant is selected + if (target.exemplar && !target.instant) { const exemplarTarget = cloneDeep(target); exemplarTarget.requestId += '_exemplar'; target.exemplar = false; queries.push(this.createQuery(exemplarTarget, options, start, end)); activeTargets.push(exemplarTarget); + this.exemplarErrors.next(); + } + if (target.exemplar && target.instant) { + this.exemplarErrors.next('Exemplars are not available for instant queries.'); } queries.push(this.createQuery(target, options, start, end)); activeTargets.push(target); @@ -310,8 +316,8 @@ export class PrometheusDatasource extends DataSourceApi if (query.exemplar) { return this.getExemplars(query).pipe( - catchError((err: FetchError) => { - this.exemplarErrors.next(err); + catchError(() => { + this.exemplarErrors.next(EXEMPLARS_NOT_AVAILABLE); return of({ data: [], state: LoadingState.Done, @@ -357,8 +363,8 @@ export class PrometheusDatasource extends DataSourceApi if (query.exemplar) { return this.getExemplars(query).pipe( - catchError((err: FetchError) => { - this.exemplarErrors.next(err); + catchError(() => { + this.exemplarErrors.next(EXEMPLARS_NOT_AVAILABLE); return of({ data: [], state: LoadingState.Done, diff --git a/public/app/plugins/datasource/tempo/datasource.ts b/public/app/plugins/datasource/tempo/datasource.ts index 0def688bb95..665eae1b07b 100644 --- a/public/app/plugins/datasource/tempo/datasource.ts +++ b/public/app/plugins/datasource/tempo/datasource.ts @@ -7,6 +7,7 @@ import { DataSourceInstanceSettings, Field, FieldType, + MutableDataFrame, } from '@grafana/data'; import { DataSourceWithBackend } from '@grafana/runtime'; import { Observable } from 'rxjs'; @@ -32,6 +33,11 @@ export class TempoDatasource extends DataSourceWithBackend { // Seems like we can't just map the values as the frame we got from backend has some default processing // and will stringify the json back when we try to set it. So we create a new field and swap it instead. const frame: DataFrame = response.data[0]; + + if (!frame) { + return emptyDataQueryResponse; + } + for (const fieldName of ['serviceTags', 'logs', 'tags']) { const field = frame.fields.find((f) => f.name === fieldName); if (field) { @@ -70,3 +76,20 @@ export class TempoDatasource extends DataSourceWithBackend { return query.query; } } + +const emptyDataQueryResponse = { + data: [ + new MutableDataFrame({ + fields: [ + { + name: 'trace', + type: FieldType.trace, + values: [], + }, + ], + meta: { + preferredVisualisationType: 'trace', + }, + }), + ], +};