From 5bd2fac9c84c8b081b0b087533c2b3e6426b68e3 Mon Sep 17 00:00:00 2001 From: Ludovic Viaud Date: Thu, 2 Mar 2023 15:05:24 +0100 Subject: [PATCH] InfluxDB: Update InfluxQL querybuilder to qualify identifiers (#62230) --- pkg/tsdb/influxdb/query.go | 12 ++++- pkg/tsdb/influxdb/query_part.go | 17 ++++++- .../VisualInfluxQLEditor/Editor.tags.test.tsx | 45 ++++++++++++++----- .../VisualInfluxQLEditor/Editor.tsx | 30 ++++++++----- .../influxdb/influxQLMetadataQuery.ts | 5 +++ .../datasource/influxdb/influx_query_model.ts | 12 ++++- .../datasource/influxdb/query_builder.ts | 20 ++++++++- .../plugins/datasource/influxdb/query_part.ts | 17 ++++++- 8 files changed, 127 insertions(+), 31 deletions(-) diff --git a/pkg/tsdb/influxdb/query.go b/pkg/tsdb/influxdb/query.go index 8ee02dda628..cc8d2bccdec 100644 --- a/pkg/tsdb/influxdb/query.go +++ b/pkg/tsdb/influxdb/query.go @@ -78,7 +78,17 @@ func (query *Query) renderTags() []string { textValue = fmt.Sprintf("'%s'", strings.ReplaceAll(tag.Value, `\`, `\\`)) } - res = append(res, fmt.Sprintf(`%s"%s" %s %s`, str, tag.Key, tag.Operator, textValue)) + escapedKey := fmt.Sprintf(`"%s"`, tag.Key) + + if strings.HasSuffix(tag.Key, "::tag") { + escapedKey = fmt.Sprintf(`"%s"::tag`, strings.TrimSuffix(tag.Key, "::tag")) + } + + if strings.HasSuffix(tag.Key, "::field") { + escapedKey = fmt.Sprintf(`"%s"::field`, strings.TrimSuffix(tag.Key, "::field")) + } + + res = append(res, fmt.Sprintf(`%s%s %s %s`, str, escapedKey, tag.Operator, textValue)) } return res diff --git a/pkg/tsdb/influxdb/query_part.go b/pkg/tsdb/influxdb/query_part.go index d6d96cae168..eba7b8a5553 100644 --- a/pkg/tsdb/influxdb/query_part.go +++ b/pkg/tsdb/influxdb/query_part.go @@ -98,10 +98,23 @@ func init() { } func fieldRenderer(query *Query, queryContext *backend.QueryDataRequest, part *QueryPart, innerExpr string) string { - if part.Params[0] == "*" { + param := part.Params[0] + + if param == "*" { return "*" } - return fmt.Sprintf(`"%s"`, part.Params[0]) + + escapedParam := fmt.Sprintf(`"%s"`, param) + + if strings.HasSuffix(param, "::tag") { + escapedParam = fmt.Sprintf(`"%s"::tag`, strings.TrimSuffix(param, "::tag")) + } + + if strings.HasSuffix(param, "::field") { + escapedParam = fmt.Sprintf(`"%s"::field`, strings.TrimSuffix(param, "::field")) + } + + return escapedParam } func functionRenderer(query *Query, queryContext *backend.QueryDataRequest, part *QueryPart, innerExpr string) string { diff --git a/public/app/plugins/datasource/influxdb/components/VisualInfluxQLEditor/Editor.tags.test.tsx b/public/app/plugins/datasource/influxdb/components/VisualInfluxQLEditor/Editor.tags.test.tsx index a2f4b6a043f..6e399782261 100644 --- a/public/app/plugins/datasource/influxdb/components/VisualInfluxQLEditor/Editor.tags.test.tsx +++ b/public/app/plugins/datasource/influxdb/components/VisualInfluxQLEditor/Editor.tags.test.tsx @@ -1,4 +1,4 @@ -import { render, screen } from '@testing-library/react'; +import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; @@ -10,11 +10,16 @@ import { Editor } from './Editor'; jest.mock('../../influxQLMetadataQuery', () => { return { + __esModule: true, + getFieldKeysForMeasurement: jest + .fn() + .mockReturnValueOnce(Promise.resolve(['free', 'total'])) + .mockReturnValueOnce(Promise.resolve([])), getTagKeysForMeasurementAndTags: jest .fn() // first time we are called when the widget mounts, - // we respond by saying `cpu, host` are the real tags - .mockReturnValueOnce(Promise.resolve(['cpu', 'host'])) + // we respond by saying `cpu, host, device` are the real tags + .mockReturnValueOnce(Promise.resolve(['cpu', 'host', 'device'])) // afterwards we will be called once when we click // on a tag-key in the WHERE section. // it does not matter what we return, as long as it is @@ -43,6 +48,7 @@ jest.mock('@grafana/runtime', () => { beforeEach(() => { (mockedMeta.getTagKeysForMeasurementAndTags as jest.Mock).mockClear(); + (mockedMeta.getFieldKeysForMeasurement as jest.Mock).mockClear(); }); const ONLY_TAGS = [ @@ -57,6 +63,12 @@ const ONLY_TAGS = [ operator: '=', value: 'host2', }, + { + condition: 'AND', + key: 'device::tag', + operator: '=', + value: 'sdd', + }, ]; const query: InfluxQuery = { @@ -76,10 +88,22 @@ const query: InfluxQuery = { }, { condition: 'AND', - key: 'field1', + key: 'device::tag', + operator: '=', + value: 'sdd', + }, + { + condition: 'AND', + key: 'free', operator: '=', value: '45', }, + { + condition: 'AND', + key: 'total::field', + operator: '=', + value: '200', + }, ], select: [ [ @@ -101,17 +125,14 @@ describe('InfluxDB InfluxQL Visual Editor field-filtering', () => { } as unknown as InfluxDatasource; render(); + await waitFor(() => {}); + + // when the editor-widget mounts, it calls getFieldKeysForMeasurement + expect(mockedMeta.getFieldKeysForMeasurement).toHaveBeenCalledTimes(1); + // when the editor-widget mounts, it calls getTagKeysForMeasurementAndTags expect(mockedMeta.getTagKeysForMeasurementAndTags).toHaveBeenCalledTimes(1); - // we click the WHERE/cpu button - await userEvent.click(screen.getByRole('button', { name: 'cpu' })); - - // and verify getTagKeysForMeasurementAndTags was called again, - // and in the tags-param we did not receive the `field1` part. - expect(mockedMeta.getTagKeysForMeasurementAndTags).toHaveBeenCalledTimes(2); - expect((mockedMeta.getTagKeysForMeasurementAndTags as jest.Mock).mock.calls[1][2]).toStrictEqual(ONLY_TAGS); - // now we click on the WHERE/host2 button await userEvent.click(screen.getByRole('button', { name: 'host2' })); diff --git a/public/app/plugins/datasource/influxdb/components/VisualInfluxQLEditor/Editor.tsx b/public/app/plugins/datasource/influxdb/components/VisualInfluxQLEditor/Editor.tsx index 7935f6819c0..03e1c73b866 100644 --- a/public/app/plugins/datasource/influxdb/components/VisualInfluxQLEditor/Editor.tsx +++ b/public/app/plugins/datasource/influxdb/components/VisualInfluxQLEditor/Editor.tsx @@ -63,7 +63,7 @@ function withTemplateVariableOptions(optionsPromise: Promise, filter?: // it is possible to add fields into the `InfluxQueryTag` structures, and they do work, // but in some cases, when we do metadata queries, we have to remove them from the queries. function filterTags(parts: InfluxQueryTag[], allTagKeys: Set): InfluxQueryTag[] { - return parts.filter((t) => allTagKeys.has(t.key)); + return parts.filter((t) => t.key.endsWith('::tag') || allTagKeys.has(t.key + '::tag')); } export const Editor = (props: Props): JSX.Element => { @@ -76,10 +76,16 @@ export const Editor = (props: Props): JSX.Element => { const { datasource } = props; const { measurement, policy } = query; - const allTagKeys = useMemo(() => { - return getTagKeysForMeasurementAndTags(measurement, policy, [], datasource).then((tags) => { - return new Set(tags); - }); + const allTagKeys = useMemo(async () => { + const tagKeys = (await getTagKeysForMeasurementAndTags(measurement, policy, [], datasource)).map( + (tag) => `${tag}::tag` + ); + + const fieldKeys = (await getFieldKeysForMeasurement(measurement || '', policy, datasource)).map( + (field) => `${field}::field` + ); + + return new Set([...tagKeys, ...fieldKeys]); }, [measurement, policy, datasource]); const selectLists = useMemo(() => { @@ -98,12 +104,14 @@ export const Editor = (props: Props): JSX.Element => { // the following function is not complicated enough to memoize, but it's result // is used in both memoized and un-memoized parts, so we have no choice - const getTagKeys = useMemo(() => { - return () => - allTagKeys.then((keys) => - getTagKeysForMeasurementAndTags(measurement, policy, filterTags(query.tags ?? [], keys), datasource) - ); - }, [measurement, policy, query.tags, datasource, allTagKeys]); + const getTagKeys = useMemo( + () => async () => { + const selectedTagKeys = new Set(query.tags?.map((tag) => tag.key)); + + return [...(await allTagKeys)].filter((tagKey) => !selectedTagKeys.has(tagKey)); + }, + [query.tags, allTagKeys] + ); const groupByList = useMemo(() => { const dynamicGroupByPartOptions = new Map([['tag_0', getTagKeys]]); diff --git a/public/app/plugins/datasource/influxdb/influxQLMetadataQuery.ts b/public/app/plugins/datasource/influxdb/influxQLMetadataQuery.ts index edd35d3f0ac..1429d00c197 100644 --- a/public/app/plugins/datasource/influxdb/influxQLMetadataQuery.ts +++ b/public/app/plugins/datasource/influxdb/influxQLMetadataQuery.ts @@ -49,6 +49,11 @@ export async function getTagValues( datasource: InfluxDatasource ): Promise { const target = { tags, measurement, policy }; + + if (tagKey.endsWith('::field')) { + return []; + } + const data = await runExploreQuery('TAG_VALUES', tagKey, undefined, target, datasource); return data.map((item) => item.text); } diff --git a/public/app/plugins/datasource/influxdb/influx_query_model.ts b/public/app/plugins/datasource/influxdb/influx_query_model.ts index 2aa00ee797f..9a0bb3b46d4 100644 --- a/public/app/plugins/datasource/influxdb/influx_query_model.ts +++ b/public/app/plugins/datasource/influxdb/influx_query_model.ts @@ -170,7 +170,17 @@ export default class InfluxQueryModel { value = this.templateSrv.replace(value, this.scopedVars, 'regex'); } - return str + '"' + tag.key + '" ' + operator + ' ' + value; + let escapedKey = `"${tag.key}"`; + + if (tag.key.endsWith('::tag')) { + escapedKey = `"${tag.key.slice(0, -5)}"::tag`; + } + + if (tag.key.endsWith('::field')) { + escapedKey = `"${tag.key.slice(0, -7)}"::field`; + } + + return str + escapedKey + ' ' + operator + ' ' + value; } getMeasurementAndPolicy(interpolate: any) { diff --git a/public/app/plugins/datasource/influxdb/query_builder.ts b/public/app/plugins/datasource/influxdb/query_builder.ts index 66b06b15f16..e62d4c818cf 100644 --- a/public/app/plugins/datasource/influxdb/query_builder.ts +++ b/public/app/plugins/datasource/influxdb/query_builder.ts @@ -25,7 +25,17 @@ function renderTagCondition(tag: { operator: any; value: string; condition: any; value = "'" + value.replace(/\\/g, '\\\\').replace(/\'/g, "\\'") + "'"; } - return str + '"' + tag.key + '" ' + operator + ' ' + value; + let escapedKey = `"${tag.key}"`; + + if (tag.key.endsWith('::tag')) { + escapedKey = `"${tag.key.slice(0, -5)}"::tag`; + } + + if (tag.key.endsWith('::field')) { + escapedKey = `"${tag.key.slice(0, -7)}"::field`; + } + + return str + escapedKey + ' ' + operator + ' ' + value; } export class InfluxQueryBuilder { @@ -83,7 +93,13 @@ export class InfluxQueryBuilder { } if (withKey) { - query += ' WITH KEY = "' + withKey + '"'; + let keyIdentifier = withKey; + + if (keyIdentifier.endsWith('::tag')) { + keyIdentifier = keyIdentifier.slice(0, -5); + } + + query += ' WITH KEY = "' + keyIdentifier + '"'; } if (this.target.tags && this.target.tags.length > 0) { diff --git a/public/app/plugins/datasource/influxdb/query_part.ts b/public/app/plugins/datasource/influxdb/query_part.ts index b8914ecabb8..cf8ab68a852 100644 --- a/public/app/plugins/datasource/influxdb/query_part.ts +++ b/public/app/plugins/datasource/influxdb/query_part.ts @@ -34,10 +34,23 @@ function aliasRenderer(part: { params: string[] }, innerExpr: string) { } function fieldRenderer(part: { params: string[] }, innerExpr: any) { - if (part.params[0] === '*') { + const param = part.params[0]; + + if (param === '*') { return '*'; } - return '"' + part.params[0] + '"'; + + let escapedParam = `"${param}"`; + + if (param.endsWith('::tag')) { + escapedParam = `"${param.slice(0, -5)}"::tag`; + } + + if (param.endsWith('::field')) { + escapedParam = `"${param.slice(0, -7)}"::field`; + } + + return escapedParam; } function replaceAggregationAddStrategy(selectParts: any[], partModel: { def: { type: string } }) {