From 9b9e41e0366857edc152017e34b1fed0c6e9ed97 Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Fri, 12 Nov 2021 12:49:06 +0100 Subject: [PATCH] Prometheus: Disable exemplar queries for alerting (#41607) * Prometheus: Dont include empty exempalr frame in results * Prometheus: Never run exemplar queries for alerting * Remove exemplar field from alerting and set exemplar to false * Add tests for frontend * Add test for backend --- packages/grafana-data/src/types/app.ts | 1 + pkg/tsdb/prometheus/time_series_query.go | 8 +- pkg/tsdb/prometheus/time_series_query_test.go | 31 +++ .../components/rule-editor/QueryWrapper.tsx | 2 + .../components/PromExemplarField.tsx | 5 +- .../components/PromQueryEditor.test.tsx | 46 ++-- .../prometheus/components/PromQueryEditor.tsx | 17 +- .../PromQueryEditor.test.tsx.snap | 209 ------------------ 8 files changed, 91 insertions(+), 228 deletions(-) delete mode 100644 public/app/plugins/datasource/prometheus/components/__snapshots__/PromQueryEditor.test.tsx.snap diff --git a/packages/grafana-data/src/types/app.ts b/packages/grafana-data/src/types/app.ts index d0617adeaa2..d2cee306a28 100644 --- a/packages/grafana-data/src/types/app.ts +++ b/packages/grafana-data/src/types/app.ts @@ -9,6 +9,7 @@ import { PluginMeta, GrafanaPlugin, PluginIncludeType } from './plugin'; * */ export enum CoreApp { CloudAlerting = 'cloud-alerting', + UnifiedAlerting = 'unified-alerting', Dashboard = 'dashboard', Explore = 'explore', Unknown = 'unknown', diff --git a/pkg/tsdb/prometheus/time_series_query.go b/pkg/tsdb/prometheus/time_series_query.go index 1fbbb47b8d1..fbb2497cc8e 100644 --- a/pkg/tsdb/prometheus/time_series_query.go +++ b/pkg/tsdb/prometheus/time_series_query.go @@ -195,6 +195,12 @@ func (s *Service) parseTimeSeriesQuery(queryContext *backend.QueryDataRequest, d rangeQuery = true } + // We never want to run exemplar query for alerting + exemplarQuery := model.ExemplarQuery + if queryContext.Headers["FromAlert"] == "true" { + exemplarQuery = false + } + qs = append(qs, &PrometheusQuery{ Expr: expr, Step: interval, @@ -204,7 +210,7 @@ func (s *Service) parseTimeSeriesQuery(queryContext *backend.QueryDataRequest, d RefId: query.RefID, InstantQuery: model.InstantQuery, RangeQuery: rangeQuery, - ExemplarQuery: model.ExemplarQuery, + ExemplarQuery: exemplarQuery, UtcOffsetSec: model.UtcOffsetSec, }) } diff --git a/pkg/tsdb/prometheus/time_series_query_test.go b/pkg/tsdb/prometheus/time_series_query_test.go index 86dd5cc7ee5..30360f7ea17 100644 --- a/pkg/tsdb/prometheus/time_series_query_test.go +++ b/pkg/tsdb/prometheus/time_series_query_test.go @@ -59,6 +59,37 @@ func TestPrometheus_timeSeriesQuery_parseTimeSeriesQuery(t *testing.T) { intervalCalculator: intervalv2.NewCalculator(), } + t.Run("parsing query from unified alerting", func(t *testing.T) { + timeRange := backend.TimeRange{ + From: now, + To: now.Add(12 * time.Hour), + } + + queryJson := `{ + "expr": "go_goroutines", + "refId": "A", + "exemplar": true + }` + + query := &backend.QueryDataRequest{ + Queries: []backend.DataQuery{ + { + JSON: []byte(queryJson), + TimeRange: timeRange, + RefID: "A", + }, + }, + Headers: map[string]string{ + "FromAlert": "true", + }, + } + + dsInfo := &DatasourceInfo{} + models, err := service.parseTimeSeriesQuery(query, dsInfo) + require.NoError(t, err) + require.Equal(t, false, models[0].ExemplarQuery) + }) + t.Run("parsing query model with step", func(t *testing.T) { timeRange := backend.TimeRange{ From: now, diff --git a/public/app/features/alerting/unified/components/rule-editor/QueryWrapper.tsx b/public/app/features/alerting/unified/components/rule-editor/QueryWrapper.tsx index be09fb6b12d..cb2d669533f 100644 --- a/public/app/features/alerting/unified/components/rule-editor/QueryWrapper.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/QueryWrapper.tsx @@ -2,6 +2,7 @@ import React, { FC, ReactNode, useState } from 'react'; import { css } from '@emotion/css'; import { cloneDeep } from 'lodash'; import { + CoreApp, DataQuery, DataSourceInstanceSettings, getDefaultRelativeTimeRange, @@ -83,6 +84,7 @@ export const QueryWrapper: FC = ({ onRunQuery={onRunQueries} queries={queries} renderHeaderExtras={() => renderTimePicker(query, index)} + app={CoreApp.UnifiedAlerting} visualization={ data.state !== LoadingState.NotStarted ? ( void; datasource: PrometheusDatasource; query: PromQuery; + 'data-testid'?: string; } -export function PromExemplarField({ datasource, onChange, query }: Props) { +export function PromExemplarField({ datasource, onChange, query, ...rest }: Props) { const [error, setError] = useState(null); const styles = useStyles2(getStyles); const prevError = usePrevious(error); @@ -41,7 +42,7 @@ export function PromExemplarField({ datasource, onChange, query }: Props) { ); return ( - +
Exemplars diff --git a/public/app/plugins/datasource/prometheus/components/PromQueryEditor.test.tsx b/public/app/plugins/datasource/prometheus/components/PromQueryEditor.test.tsx index bfef5789a79..988b25419de 100644 --- a/public/app/plugins/datasource/prometheus/components/PromQueryEditor.test.tsx +++ b/public/app/plugins/datasource/prometheus/components/PromQueryEditor.test.tsx @@ -1,8 +1,8 @@ import React from 'react'; -import { shallow } from 'enzyme'; -import { dateTime } from '@grafana/data'; +import { dateTime, CoreApp } from '@grafana/data'; +import { screen, render } from '@testing-library/react'; -import { PromQueryEditor } from './PromQueryEditor'; +import { PromQueryEditor, testIds } from './PromQueryEditor'; import { PrometheusDatasource } from '../datasource'; import { PromQuery } from '../types'; @@ -17,10 +17,24 @@ jest.mock('app/features/dashboard/services/TimeSrv', () => { }; }); +jest.mock('./monaco-query-field/MonacoQueryFieldWrapper', () => { + const fakeQueryField = () =>
prometheus query field
; + return { + MonacoQueryFieldWrapper: fakeQueryField, + }; +}); + const setup = (propOverrides?: object) => { const datasourceMock: unknown = { createQuery: jest.fn((q) => q), getPrometheusTime: jest.fn((date, roundup) => 123), + languageProvider: { + start: () => Promise.resolve([]), + syntax: () => {}, + getLabelKeys: () => [], + metrics: [], + }, + getInitHints: () => [], }; const datasource: PrometheusDatasource = datasourceMock as PrometheusDatasource; const onRunQuery = jest.fn(); @@ -36,18 +50,24 @@ const setup = (propOverrides?: object) => { Object.assign(props, propOverrides); - const wrapper = shallow(); - const instance = wrapper.instance() as PromQueryEditor; - - return { - instance, - wrapper, - }; + return render(); }; describe('Render PromQueryEditor with basic options', () => { - it('should render', () => { - const { wrapper } = setup(); - expect(wrapper).toMatchSnapshot(); + it('should render editor', () => { + setup(); + expect(screen.getByTestId(testIds.editor)).toBeInTheDocument(); + }); + + it('should render exemplar editor for dashboard', () => { + setup({ app: CoreApp.Dashboard }); + expect(screen.getByTestId(testIds.editor)).toBeInTheDocument(); + expect(screen.getByTestId(testIds.exemplar)).toBeInTheDocument(); + }); + + it('should not render exemplar editor for unified alerting', () => { + setup({ app: CoreApp.UnifiedAlerting }); + expect(screen.getByTestId(testIds.editor)).toBeInTheDocument(); + expect(screen.queryByTestId(testIds.exemplar)).not.toBeInTheDocument(); }); }); diff --git a/public/app/plugins/datasource/prometheus/components/PromQueryEditor.tsx b/public/app/plugins/datasource/prometheus/components/PromQueryEditor.tsx index e5c5c648a4c..a8fa2f0f6c3 100644 --- a/public/app/plugins/datasource/prometheus/components/PromQueryEditor.tsx +++ b/public/app/plugins/datasource/prometheus/components/PromQueryEditor.tsx @@ -3,7 +3,7 @@ import React, { PureComponent } from 'react'; // Types import { InlineFormLabel, LegacyForms, Select } from '@grafana/ui'; -import { SelectableValue } from '@grafana/data'; +import { CoreApp, SelectableValue } from '@grafana/data'; import { PromQuery } from '../types'; import PromQueryField from './PromQueryField'; @@ -44,7 +44,8 @@ export class PromQueryEditor extends PureComponent expr: '', legendFormat: '', interval: '', - exemplar: true, + // Set exemplar to false for alerting queries + exemplar: props.app === CoreApp.UnifiedAlerting ? false : true, }; const query = Object.assign({}, defaultQuery, props.query); this.query = query; @@ -111,6 +112,8 @@ export class PromQueryEditor extends PureComponent render() { const { datasource, query, range, data } = this.props; const { formatOption, instant, interval, intervalFactorOption, legendFormat } = this.state; + //We want to hide exemplar field for unified alerting as exemplars in alerting don't make sense and are source of confusion + const showExemplarField = this.props.app !== CoreApp.UnifiedAlerting; return ( />
- + {showExemplarField && ( + + )} } /> @@ -207,4 +217,5 @@ export class PromQueryEditor extends PureComponent export const testIds = { editor: 'prom-editor', + exemplar: 'exemplar-editor', }; 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 deleted file mode 100644 index 3ef0088f8d7..00000000000 --- a/public/app/plugins/datasource/prometheus/components/__snapshots__/PromQueryEditor.test.tsx.snap +++ /dev/null @@ -1,209 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`Render PromQueryEditor with basic options should render 1`] = ` - -
- - Legend - - -
-
- - An additional lower limit for the step parameter of the Prometheus query and for the - - - $__interval - - and - - $__rate_interval - - variables. The limit is absolute and not modified by the "Resolution" setting. - - } - width={7} - > - Min step - - -
-
-
- Resolution -
- - - - - -
- - - } - data-testid="prom-editor" - datasource={ - Object { - "createQuery": [MockFunction], - "getPrometheusTime": [MockFunction], - } - } - history={Array []} - onChange={[Function]} - onRunQuery={[Function]} - query={ - Object { - "expr": "", - "refId": "A", - } - } -/> -`;