From a742149687ed944986ab202417ecefedd4b217f6 Mon Sep 17 00:00:00 2001 From: ismail simsek Date: Mon, 26 Jun 2023 20:20:03 +0300 Subject: [PATCH] InfluxDB: Refactor annotation transform logic (#69866) * Reformatting and restructuring * Update unit test * Always send the default retention policy as first element * Fix typo * Update test * Update test once more * Field names start with capital letters * Simplify the condition * Case-insensitive checks * Fix typo * Update response_parser test * Update imports * Refactor annotation transform logic * More types * Moore types * Fix table rendering issue * Use it as raw query * Migrate annotations * Set default retention policy when there is no policy in the query --------- Co-authored-by: Ludovic Viaud --- .betterer.results | 13 +- .../editor/annotation/AnnotationEditor.tsx | 1 + .../influxql/visual/VisualInfluxQLEditor.tsx | 14 +- .../plugins/datasource/influxdb/datasource.ts | 40 ++-- .../influxdb/response_parser.test.ts | 210 ++++++++++-------- .../datasource/influxdb/response_parser.ts | 143 ++++++------ 6 files changed, 222 insertions(+), 199 deletions(-) diff --git a/.betterer.results b/.betterer.results index 07bb4a249ff..6c886a6c36f 100644 --- a/.betterer.results +++ b/.betterer.results @@ -4146,18 +4146,7 @@ exports[`better eslint`] = { ], "public/app/plugins/datasource/influxdb/response_parser.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], - [0, 0, 0, "Unexpected any. Specify a different type.", "1"], - [0, 0, 0, "Unexpected any. Specify a different type.", "2"], - [0, 0, 0, "Unexpected any. Specify a different type.", "3"], - [0, 0, 0, "Do not use any type assertions.", "4"], - [0, 0, 0, "Unexpected any. Specify a different type.", "5"], - [0, 0, 0, "Unexpected any. Specify a different type.", "6"], - [0, 0, 0, "Unexpected any. Specify a different type.", "7"], - [0, 0, 0, "Unexpected any. Specify a different type.", "8"], - [0, 0, 0, "Unexpected any. Specify a different type.", "9"], - [0, 0, 0, "Unexpected any. Specify a different type.", "10"], - [0, 0, 0, "Unexpected any. Specify a different type.", "11"], - [0, 0, 0, "Unexpected any. Specify a different type.", "12"] + [0, 0, 0, "Unexpected any. Specify a different type.", "1"] ], "public/app/plugins/datasource/influxdb/specs/datasource.test.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], diff --git a/public/app/plugins/datasource/influxdb/components/editor/annotation/AnnotationEditor.tsx b/public/app/plugins/datasource/influxdb/components/editor/annotation/AnnotationEditor.tsx index 38c4a14e361..d8ba3b533f6 100644 --- a/public/app/plugins/datasource/influxdb/components/editor/annotation/AnnotationEditor.tsx +++ b/public/app/plugins/datasource/influxdb/components/editor/annotation/AnnotationEditor.tsx @@ -18,6 +18,7 @@ export const AnnotationEditor = (props: QueryEditorProps { const { measurement, policy } = query; const { retentionPolicies } = useRetentionPolicies(datasource); + useEffect(() => { + if (!policy) { + props.onChange({ + ...query, + policy: retentionPolicies[0], + }); + props.onRunQuery(); + } + }, [policy, props, query, retentionPolicies]); + const allTagKeys = useMemo(async () => { const tagKeys = (await getTagKeysForMeasurementAndTags(datasource, [], measurement, policy)).map( (tag) => `${tag}::tag` @@ -121,7 +131,7 @@ export const VisualInfluxQLEditor = (props: Props): JSX.Element => {
withTemplateVariableOptions( diff --git a/public/app/plugins/datasource/influxdb/datasource.ts b/public/app/plugins/datasource/influxdb/datasource.ts index 709aac4a7ec..8f6f78aef1e 100644 --- a/public/app/plugins/datasource/influxdb/datasource.ts +++ b/public/app/plugins/datasource/influxdb/datasource.ts @@ -149,6 +149,26 @@ export default class InfluxDatasource extends DataSourceWithBackend t.hide !== true), }; + // migrate annotations + if (filteredRequest.targets.some((target: InfluxQuery) => target.fromAnnotations)) { + const streams: Array> = []; + + for (const target of filteredRequest.targets) { + if (target.query) { + streams.push( + new Observable((subscriber) => { + this.annotationEvents(filteredRequest, target) + .then((events) => subscriber.next({ data: [toDataFrame(events)] })) + .catch((ex) => subscriber.error(new Error(ex))) + .finally(() => subscriber.complete()); + }) + ); + } + } + + return merge(...streams); + } + if (this.isFlux) { return super.query(filteredRequest); } @@ -560,26 +580,6 @@ export default class InfluxDatasource extends DataSourceWithBackend { - // migrate annotations - if (options.targets.some((target: InfluxQuery) => target.fromAnnotations)) { - const streams: Array> = []; - - for (const target of options.targets) { - if (target.query) { - streams.push( - new Observable((subscriber) => { - this.annotationEvents(options, target) - .then((events) => subscriber.next({ data: [toDataFrame(events)] })) - .catch((ex) => subscriber.error(new Error(ex))) - .finally(() => subscriber.complete()); - }) - ); - } - } - - return merge(...streams); - } - let timeFilter = this.getTimeFilter(options); const scopedVars = options.scopedVars; const targets = cloneDeep(options.targets); diff --git a/public/app/plugins/datasource/influxdb/response_parser.test.ts b/public/app/plugins/datasource/influxdb/response_parser.test.ts index 1cfc07450b2..a5d9be396fd 100644 --- a/public/app/plugins/datasource/influxdb/response_parser.test.ts +++ b/public/app/plugins/datasource/influxdb/response_parser.test.ts @@ -1,7 +1,7 @@ import { size } from 'lodash'; import { of } from 'rxjs'; -import { AnnotationEvent, DataQueryRequest, FieldType, MutableDataFrame } from '@grafana/data'; +import { AnnotationEvent, DataQueryRequest, dateTime, FieldType, MutableDataFrame } from '@grafana/data'; import { FetchResponse } from '@grafana/runtime'; import config from 'app/core/config'; import { backendSrv } from 'app/core/services/backend_srv'; // will use the version in __mocks__ @@ -312,114 +312,132 @@ describe('influxdb response parser', () => { tagsColumn: 'host,path', }; - const queryOptions = { + const queryOptions: DataQueryRequest = { + app: 'explore', + interval: '', + intervalMs: 0, + requestId: '', + scopedVars: {}, + startTime: 0, + timezone: '', targets: [annotation], range: { - from: '2018-01-01T00:00:00Z', - to: '2018-01-02T00:00:00Z', + from: dateTime().subtract(1, 'h'), + to: dateTime(), + raw: { from: '1h', to: 'now' }, }, - } as unknown as DataQueryRequest; + }; let response: AnnotationEvent[]; beforeEach(async () => { - fetchMock.mockImplementation(() => { - return of({ - data: { - results: { - metricFindQuery: { - frames: [ - { - schema: { - name: 'logs.host', - fields: [ - { - name: 'time', - type: 'time', - }, - { - name: 'value', - type: 'string', - }, - ], - }, - data: { - values: [ - [1645208701000, 1645208702000], - ['cbfa07e0e3bb 1', 'cbfa07e0e3bb 2'], - ], - }, + const mockResponse: FetchResponse = { + config: { url: '' }, + headers: new Headers(), + ok: false, + redirected: false, + status: 0, + statusText: '', + type: 'basic', + url: '', + data: { + results: { + metricFindQuery: { + frames: [ + { + schema: { + name: 'logs.host', + fields: [ + { + name: 'time', + type: 'time', + }, + { + name: 'value', + type: 'string', + }, + ], }, - { - schema: { - name: 'logs.message', - fields: [ - { - name: 'time', - type: 'time', - }, - { - name: 'value', - type: 'string', - }, - ], - }, - data: { - values: [ - [1645208701000, 1645208702000], - [ - 'Station softwareupdated[447]: Adding client 1', - 'Station softwareupdated[447]: Adding client 2', - ], - ], - }, + data: { + values: [ + [1645208701000, 1645208702000], + ['cbfa07e0e3bb 1', 'cbfa07e0e3bb 2'], + ], }, - { - schema: { - name: 'logs.path', - fields: [ - { - name: 'time', - type: 'time', - }, - { - name: 'value', - type: 'string', - }, - ], - }, - data: { - values: [ - [1645208701000, 1645208702000], - ['/var/log/host/install.log 1', '/var/log/host/install.log 2'], - ], - }, + }, + { + schema: { + name: 'logs.message', + fields: [ + { + name: 'time', + type: 'time', + }, + { + name: 'value', + type: 'string', + }, + ], }, - { - schema: { - name: 'textColumn', - fields: [ - { - name: 'time', - type: 'time', - }, - { - name: 'value', - type: 'string', - }, + data: { + values: [ + [1645208701000, 1645208702000], + [ + 'Station softwareupdated[447]: Adding client 1', + 'Station softwareupdated[447]: Adding client 2', ], - }, - data: { - values: [ - [1645208701000, 1645208702000], - ['text 1', 'text 2'], - ], - }, + ], }, - ], - }, + }, + { + schema: { + name: 'logs.path', + fields: [ + { + name: 'time', + type: 'time', + }, + { + name: 'value', + type: 'string', + }, + ], + }, + data: { + values: [ + [1645208701000, 1645208702000], + ['/var/log/host/install.log 1', '/var/log/host/install.log 2'], + ], + }, + }, + { + schema: { + name: 'textColumn', + fields: [ + { + name: 'time', + type: 'time', + }, + { + name: 'value', + type: 'string', + }, + ], + }, + data: { + values: [ + [1645208701000, 1645208702000], + ['text 1', 'text 2'], + ], + }, + }, + ], }, }, - } as FetchResponse); + }, + }; + + fetchMock.mockImplementation(() => { + return of(mockResponse); }); config.featureToggles.influxdbBackendMigration = true; diff --git a/public/app/plugins/datasource/influxdb/response_parser.ts b/public/app/plugins/datasource/influxdb/response_parser.ts index 138f2eec867..b34e8f91bc6 100644 --- a/public/app/plugins/datasource/influxdb/response_parser.ts +++ b/public/app/plugins/datasource/influxdb/response_parser.ts @@ -1,7 +1,7 @@ import { each, flatten, groupBy, isArray } from 'lodash'; -import { AnnotationEvent, DataFrame, DataQuery, FieldType, QueryResultMeta } from '@grafana/data'; -import { toDataQueryResponse } from '@grafana/runtime'; +import { AnnotationEvent, DataFrame, FieldType, QueryResultMeta } from '@grafana/data'; +import { BackendDataSourceResponse, FetchResponse, toDataQueryResponse } from '@grafana/runtime'; import TableModel from 'app/core/TableModel'; import { InfluxQuery } from './types'; @@ -84,14 +84,14 @@ export default class ResponseParser { // if group by tag(s) added if (dfs[0].fields[1] && dfs[0].fields[1].labels) { - let dfsByLabels: any = groupBy(dfs, (df: DataFrame) => + let dfsByLabels = groupBy(dfs, (df: DataFrame) => df.fields[1].labels ? Object.values(df.fields[1].labels!) : null ); const labels = Object.keys(dfsByLabels); - dfsByLabels = Object.values(dfsByLabels); + const dfsByLabelValues = Object.values(dfsByLabels); - for (let i = 0; i < dfsByLabels.length; i++) { - table = getTableRows(dfsByLabels[i], table, [...labels[i].split(',')]); + for (let i = 0; i < dfsByLabelValues.length; i++) { + table = getTableRows(dfsByLabelValues[i], table, [...labels[i].split(',')]); } } else { table = getTableRows(dfs, table, []); @@ -101,74 +101,79 @@ export default class ResponseParser { return table; } - async transformAnnotationResponse(annotation: any, data: any, target: InfluxQuery): Promise { - const rsp = toDataQueryResponse(data, [target] as DataQuery[]); + async transformAnnotationResponse( + annotation: InfluxQuery, + data: FetchResponse, + target: InfluxQuery + ): Promise { + const rsp = toDataQueryResponse(data, [target]); - if (rsp) { - const table = this.getTable(rsp.data, target, {}); - const list: any[] = []; - let titleCol: any = null; - let timeCol: any = null; - let timeEndCol: any = null; - const tagsCol: any = []; - let textCol: any = null; - - each(table.columns, (column, index) => { - if (column.text.toLowerCase() === 'time') { - timeCol = index; - return; - } - if (column.text === annotation.titleColumn) { - titleCol = index; - return; - } - if (colContainsTag(column.text, annotation.tagsColumn)) { - tagsCol.push(index); - return; - } - if (column.text.includes(annotation.textColumn)) { - textCol = index; - return; - } - if (column.text === annotation.timeEndColumn) { - timeEndCol = index; - return; - } - // legacy case - if (!titleCol && textCol !== index) { - titleCol = index; - } - }); - - each(table.rows, (value) => { - const data = { - annotation: annotation, - time: +new Date(value[timeCol]), - title: value[titleCol], - timeEnd: value[timeEndCol], - // Remove empty values, then split in different tags for comma separated values - tags: flatten( - tagsCol - .filter((t: any) => { - return value[t]; - }) - .map((t: any) => { - return value[t].split(','); - }) - ), - text: value[textCol], - }; - - list.push(data); - }); - - return list; + if (!rsp) { + return []; } - return []; + + const table = this.getTable(rsp.data, target, {}); + const list: any[] = []; + let titleColIndex = 0; + let timeColIndex = 0; + let timeEndColIndex = 0; + let textColIndex = 0; + const tagsColIndexes: number[] = []; + + each(table.columns, (column, index) => { + if (column.text.toLowerCase() === 'time') { + timeColIndex = index; + return; + } + if (column.text === annotation.titleColumn) { + titleColIndex = index; + return; + } + if (colContainsTag(column.text, annotation.tagsColumn)) { + tagsColIndexes.push(index); + return; + } + if (annotation.textColumn && column.text.includes(annotation.textColumn)) { + textColIndex = index; + return; + } + if (column.text === annotation.timeEndColumn) { + timeEndColIndex = index; + return; + } + // legacy case + if (!titleColIndex && textColIndex !== index) { + titleColIndex = index; + } + }); + + each(table.rows, (value) => { + const data = { + annotation: annotation, + time: +new Date(value[timeColIndex]), + title: value[titleColIndex], + timeEnd: value[timeEndColIndex], + // Remove empty values, then split in different tags for comma separated values + tags: flatten( + tagsColIndexes + .filter((t) => { + return value[t]; + }) + .map((t) => { + return value[t].split(','); + }) + ), + text: value[textColIndex], + }; + + list.push(data); + }); + + return list; } } -function colContainsTag(colText: string, tagsColumn: string): boolean { +function colContainsTag(colText: string, tagsColumn?: string): boolean { const tags = (tagsColumn || '').replace(' ', '').split(','); for (const tag of tags) { if (tag !== '' && colText.includes(tag)) {