From 6c3efb0c88201ff4ace29c85e2dbb9a1f8b48f5b Mon Sep 17 00:00:00 2001 From: Brendan O'Handley Date: Wed, 27 Jul 2022 16:27:42 -0400 Subject: [PATCH] Graphite Plugin: Remove angular dependencies for graphite annotations (#52261) * fix merge conflict * fix betterer * handle new creating annotations * add h5 'or' tag to annotation editor * fix annotation regression looking for tags before target * remove angular annotation partial * change ann tags type to string[] and use TagsInput to create ann * remove GraphiteEventsType, return annotations targets setting 'textEditor': true * fix yarn typecheck errors * add dateTime for yarn fix to tests * fix incorrect merge conflict resolution * fix betterer * making changes for PR approval resolutions * fix prettier issue * fix prettier --- .betterer.results | 3 + .../graphite/components/AnnotationsEditor.tsx | 56 +++++++++ .../datasource/graphite/datasource.test.ts | 28 +++-- .../plugins/datasource/graphite/datasource.ts | 115 +++++++++++------- .../plugins/datasource/graphite/migrations.ts | 39 ++++++ .../app/plugins/datasource/graphite/module.ts | 7 +- .../graphite/partials/annotations.editor.html | 13 -- .../app/plugins/datasource/graphite/types.ts | 10 +- 8 files changed, 198 insertions(+), 73 deletions(-) create mode 100644 public/app/plugins/datasource/graphite/components/AnnotationsEditor.tsx create mode 100644 public/app/plugins/datasource/graphite/migrations.ts delete mode 100644 public/app/plugins/datasource/graphite/partials/annotations.editor.html diff --git a/.betterer.results b/.betterer.results index fc0766fa8a9..888007e28e7 100644 --- a/.betterer.results +++ b/.betterer.results @@ -7215,6 +7215,9 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "5"], [0, 0, 0, "Unexpected any. Specify a different type.", "6"] ], + "public/app/plugins/datasource/graphite/migrations.ts:5381": [ + [0, 0, 0, "Unexpected any. Specify a different type.", "0"] + ], "public/app/plugins/datasource/graphite/parser.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"], diff --git a/public/app/plugins/datasource/graphite/components/AnnotationsEditor.tsx b/public/app/plugins/datasource/graphite/components/AnnotationsEditor.tsx new file mode 100644 index 00000000000..28ba6d834ba --- /dev/null +++ b/public/app/plugins/datasource/graphite/components/AnnotationsEditor.tsx @@ -0,0 +1,56 @@ +import React, { useState } from 'react'; + +import { QueryEditorProps } from '@grafana/data'; +import { InlineFormLabel, Input, TagsInput } from '@grafana/ui'; + +import { GraphiteDatasource } from '../datasource'; +import { GraphiteQuery, GraphiteOptions } from '../types'; + +export const AnnotationEditor = (props: QueryEditorProps) => { + const { query, onChange } = props; + const [target, setTarget] = useState(query.target ?? ''); + const [tags, setTags] = useState(query.tags ?? []); + const updateValue = (key: K, val: V) => { + if (key === 'tags') { + onChange({ + ...query, + [key]: val, + fromAnnotations: true, + queryType: key, + }); + } else { + onChange({ + ...query, + [key]: val, + fromAnnotations: true, + textEditor: true, + }); + } + }; + + const onTagsChange = (tagsInput: string[]) => { + setTags(tagsInput); + updateValue('tags', tagsInput); + }; + + return ( +
+
+ Graphite Query + setTarget(e.currentTarget.value || '')} + onBlur={() => updateValue('target', target)} + placeholder="Example: statsd.application.counters.*.count" + /> +
+ +
Or
+ +
+ Graphite events tags + +
+
+ ); +}; diff --git a/public/app/plugins/datasource/graphite/datasource.test.ts b/public/app/plugins/datasource/graphite/datasource.test.ts index 4cc4d1fc22e..9910231df87 100644 --- a/public/app/plugins/datasource/graphite/datasource.test.ts +++ b/public/app/plugins/datasource/graphite/datasource.test.ts @@ -2,7 +2,7 @@ import { isArray } from 'lodash'; import { of } from 'rxjs'; import { createFetchResponse } from 'test/helpers/createFetchResponse'; -import { AbstractLabelMatcher, AbstractLabelOperator, dateTime, getFrameDisplayName } from '@grafana/data'; +import { AbstractLabelMatcher, AbstractLabelOperator, getFrameDisplayName, dateTime } from '@grafana/data'; import { backendSrv } from 'app/core/services/backend_srv'; // will use the version in __mocks__ import { TemplateSrv } from 'app/features/templating/template_srv'; @@ -196,13 +196,21 @@ describe('graphiteDatasource', () => { }); const options = { - annotation: { - tags: 'tag1', - }, + targets: [ + { + fromAnnotations: true, + tags: ['tag1'], + queryType: 'tags', + }, + ], + range: { - from: dateTime(1432288354), - to: dateTime(1432288401), - raw: { from: 'now-24h', to: 'now' }, + from: '2022-06-06T07:03:03.109Z', + to: '2022-06-07T07:03:03.109Z', + raw: { + from: '2022-06-06T07:03:03.109Z', + to: '2022-06-07T07:03:03.109Z', + }, }, }; @@ -221,7 +229,7 @@ describe('graphiteDatasource', () => { fetchMock.mockImplementation((options: any) => { return of(createFetchResponse(response)); }); - await ctx.ds.annotationQuery(options).then((data: any) => { + await ctx.ds.annotationEvents(options.range, options.targets[0]).then((data: any) => { results = data; }); }); @@ -250,7 +258,7 @@ describe('graphiteDatasource', () => { return of(createFetchResponse(response)); }); - await ctx.ds.annotationQuery(options).then((data: any) => { + await ctx.ds.annotationEvents(options.range, options.targets[0]).then((data: any) => { results = data; }); }); @@ -267,7 +275,7 @@ describe('graphiteDatasource', () => { fetchMock.mockImplementation((options: any) => { return of(createFetchResponse('zzzzzzz')); }); - await ctx.ds.annotationQuery(options).then((data: any) => { + await ctx.ds.annotationEvents(options.range, options.targets[0]).then((data: any) => { results = data; }); expect(results).toEqual([]); diff --git a/public/app/plugins/datasource/graphite/datasource.ts b/public/app/plugins/datasource/graphite/datasource.ts index 99e15e610f2..e529c9290cb 100644 --- a/public/app/plugins/datasource/graphite/datasource.ts +++ b/public/app/plugins/datasource/graphite/datasource.ts @@ -1,5 +1,5 @@ import { each, indexOf, isArray, isString, map as _map } from 'lodash'; -import { lastValueFrom, Observable, of, OperatorFunction, pipe, throwError } from 'rxjs'; +import { lastValueFrom, merge, Observable, of, OperatorFunction, pipe, throwError } from 'rxjs'; import { catchError, map } from 'rxjs/operators'; import { @@ -26,15 +26,18 @@ import { getRollupNotice, getRuntimeConsolidationNotice } from 'app/plugins/data import { getSearchFilterScopedVar } from '../../../features/variables/utils'; +import { AnnotationEditor } from './components/AnnotationsEditor'; import { convertToGraphiteQueryObject } from './components/helpers'; import gfunc, { FuncDefs, FuncInstance } from './gfunc'; import GraphiteQueryModel from './graphite_query'; +import { prepareAnnotation } from './migrations'; // Types import { GraphiteLokiMapping, GraphiteMetricLokiMatcher, GraphiteOptions, GraphiteQuery, + GraphiteQueryRequest, GraphiteQueryImportConfiguration, GraphiteQueryType, GraphiteType, @@ -97,6 +100,10 @@ export class GraphiteDatasource this.funcDefs = null; this.funcDefsPromise = null; this._seriesRefLetters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'; + this.annotations = { + QueryEditor: AnnotationEditor, + prepareAnnotation, + }; } getQueryOptionsInfo() { @@ -182,40 +189,62 @@ export class GraphiteDatasource } query(options: DataQueryRequest): Observable { - const graphOptions = { - from: this.translateTime(options.range.from, false, options.timezone), - until: this.translateTime(options.range.to, true, options.timezone), - targets: options.targets, - format: (options as any).format ?? 'json', - cacheTimeout: options.cacheTimeout || this.cacheTimeout, - maxDataPoints: options.maxDataPoints, - }; + const streams: Array> = []; - const params = this.buildGraphiteParams(graphOptions, options.scopedVars); - if (params.length === 0) { + for (const target of options.targets) { + // hiding target is handled in buildGraphiteParams + if (target.fromAnnotations) { + streams.push( + new Observable((subscriber) => { + this.annotationEvents(options.range, target) + .then((events) => subscriber.next({ data: [toDataFrame(events)] })) + .catch((ex) => subscriber.error(new Error(ex))) + .finally(() => subscriber.complete()); + }) + ); + } else { + // handle the queries here + const graphOptions = { + from: this.translateTime(options.range.from, false, options.timezone), + until: this.translateTime(options.range.to, true, options.timezone), + targets: options.targets, + format: (options as GraphiteQueryRequest).format, + cacheTimeout: options.cacheTimeout || this.cacheTimeout, + maxDataPoints: options.maxDataPoints, + }; + + const params = this.buildGraphiteParams(graphOptions, options.scopedVars); + if (params.length === 0) { + return of({ data: [] }); + } + + if (this.isMetricTank) { + params.push('meta=true'); + } + + const httpOptions: any = { + method: 'POST', + url: '/render', + data: params.join('&'), + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + }, + }; + + this.addTracingHeaders(httpOptions, options); + + if (options.panelId) { + httpOptions.requestId = this.name + '.panelId.' + options.panelId; + } + + streams.push(this.doGraphiteRequest(httpOptions).pipe(map(this.convertResponseToDataFrames))); + } + } + + if (streams.length === 0) { return of({ data: [] }); } - - if (this.isMetricTank) { - params.push('meta=true'); - } - - const httpOptions: any = { - method: 'POST', - url: '/render', - data: params.join('&'), - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', - }, - }; - - this.addTracingHeaders(httpOptions, options); - - if (options.panelId) { - httpOptions.requestId = this.name + '.panelId.' + options.panelId; - } - - return this.doGraphiteRequest(httpOptions).pipe(map(this.convertResponseToDataFrames)); + return merge(...streams); } addTracingHeaders(httpOptions: { headers: any }, options: { dashboardId?: number; panelId?: number }) { @@ -330,13 +359,13 @@ export class GraphiteDatasource return expandedQueries; } - annotationQuery(options: any) { - // Graphite metric as annotation - if (options.annotation.target) { - const target = this.templateSrv.replace(options.annotation.target, {}, 'glob'); + annotationEvents(range: any, target: any) { + if (target.target) { + // Graphite query as target as annotation + const targetAnnotation = this.templateSrv.replace(target.target, {}, 'glob'); const graphiteQuery = { - range: options.range, - targets: [{ target: target }], + range: range, + targets: [{ target: targetAnnotation }], format: 'json', maxDataPoints: 100, } as unknown as DataQueryRequest; @@ -358,7 +387,7 @@ export class GraphiteDatasource } list.push({ - annotation: options.annotation, + annotation: target, time, title: target.name, }); @@ -370,9 +399,9 @@ export class GraphiteDatasource ) ); } else { - // Graphite event as annotation - const tags = this.templateSrv.replace(options.annotation.tags); - return this.events({ range: options.range, tags: tags }).then((results: any) => { + // Graphite event/tag as annotation + const tags = this.templateSrv.replace(target.tags?.join(' ')); + return this.events({ range: range, tags: tags }).then((results: any) => { const list = []; if (!isArray(results.data)) { console.error(`Unable to get annotations from ${results.url}.`); @@ -387,7 +416,7 @@ export class GraphiteDatasource } list.push({ - annotation: options.annotation, + annotation: target, time: e.when * 1000, title: e.what, tags: tags, diff --git a/public/app/plugins/datasource/graphite/migrations.ts b/public/app/plugins/datasource/graphite/migrations.ts new file mode 100644 index 00000000000..2f91bf3bdfe --- /dev/null +++ b/public/app/plugins/datasource/graphite/migrations.ts @@ -0,0 +1,39 @@ +type LegacyAnnotation = { + target?: string; + tags?: string; +}; + +// this becomes the target in the migrated annotations +const migrateLegacyAnnotation = (json: LegacyAnnotation) => { + // return the target annotation + if (typeof json.target === 'string' && json.target) { + return { + fromAnnotations: true, + target: json.target, + textEditor: true, + }; + } + + // return the tags annotation + return { + queryType: 'tags', + tags: (json.tags || '').split(' '), + fromAnnotations: true, + }; +}; + +// eslint-ignore-next-line +export const prepareAnnotation = (json: any) => { + // annotation attributes are either 'tags' or 'target'(a graphite query string) + // because the new annotations will also have a target attribute, {} + // we need to handle the ambiguous 'target' when migrating legacy annotations + // so, to migrate legacy annotations + // we check that target is a string + // or + // there is a tags attribute with no target + const resultingTarget = json.target && typeof json.target !== 'string' ? json.target : migrateLegacyAnnotation(json); + + json.target = resultingTarget; + + return json; +}; diff --git a/public/app/plugins/datasource/graphite/module.ts b/public/app/plugins/datasource/graphite/module.ts index 5b92a1decc2..8f6b31dcb85 100644 --- a/public/app/plugins/datasource/graphite/module.ts +++ b/public/app/plugins/datasource/graphite/module.ts @@ -6,13 +6,8 @@ import { MetricTankMetaInspector } from './components/MetricTankMetaInspector'; import { ConfigEditor } from './configuration/ConfigEditor'; import { GraphiteDatasource } from './datasource'; -class AnnotationsQueryCtrl { - static templateUrl = 'partials/annotations.editor.html'; -} - export const plugin = new DataSourcePlugin(GraphiteDatasource) .setQueryEditor(GraphiteQueryEditor) .setConfigEditor(ConfigEditor) .setVariableQueryEditor(GraphiteVariableEditor) - .setMetadataInspector(MetricTankMetaInspector) - .setAnnotationQueryCtrl(AnnotationsQueryCtrl); + .setMetadataInspector(MetricTankMetaInspector); diff --git a/public/app/plugins/datasource/graphite/partials/annotations.editor.html b/public/app/plugins/datasource/graphite/partials/annotations.editor.html deleted file mode 100644 index 9d228b8e4f9..00000000000 --- a/public/app/plugins/datasource/graphite/partials/annotations.editor.html +++ /dev/null @@ -1,13 +0,0 @@ -
-
- Graphite query - -
- -
Or
- -
- Graphite events tags - -
-
diff --git a/public/app/plugins/datasource/graphite/types.ts b/public/app/plugins/datasource/graphite/types.ts index efe19aeb345..0bdd7039ebc 100644 --- a/public/app/plugins/datasource/graphite/types.ts +++ b/public/app/plugins/datasource/graphite/types.ts @@ -1,4 +1,4 @@ -import { DataQuery, DataSourceJsonData, TimeRange } from '@grafana/data'; +import { DataQuery, DataQueryRequest, DataSourceJsonData, TimeRange } from '@grafana/data'; import { TemplateSrv } from '../../../features/templating/template_srv'; @@ -11,7 +11,11 @@ export enum GraphiteQueryType { } export interface GraphiteQuery extends DataQuery { + queryType?: string; + textEditor?: boolean; target?: string; + tags?: string[]; + fromAnnotations?: boolean; } export interface GraphiteOptions extends DataSourceJsonData { @@ -93,3 +97,7 @@ export type GraphiteQueryEditorDependencies = { // schedule onChange/onRunQuery after the reducer actions finishes refresh: () => void; }; + +export interface GraphiteQueryRequest extends DataQueryRequest { + format: string; +}