From a4c580144080625f7d8ec80c339bfa8ba6b15328 Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Fri, 7 Oct 2022 16:21:59 +0200 Subject: [PATCH] Loki: Fix redundant escaping in adhoc filter with regex match (#56447) * Loki: Fix redundant escaping in adhoc filter with regex match * Update data.js * Simplify test * Simplify test * Update * Add more tests --- devenv/docker/blocks/loki/data/data.js | 4 +- .../datasource/loki/datasource.test.ts | 57 ++++++++++++++----- .../app/plugins/datasource/loki/datasource.ts | 30 ++++++---- 3 files changed, 65 insertions(+), 26 deletions(-) diff --git a/devenv/docker/blocks/loki/data/data.js b/devenv/docker/blocks/loki/data/data.js index 07572a18efd..fed60c9e70b 100644 --- a/devenv/docker/blocks/loki/data/data.js +++ b/devenv/docker/blocks/loki/data/data.js @@ -125,8 +125,8 @@ async function main() { await sleep(getNextSineWaveSleepDuration()); const timestampMs = new Date().getTime(); const item = getRandomLogItem(step + 1) - lokiSendLogLine(timestampMs, JSON.stringify(item), {place:'moon', source: 'data'}); - lokiSendLogLine(timestampMs, logFmtLine(item), {place:'luna', source: 'data'}); + lokiSendLogLine(timestampMs, JSON.stringify(item), {place:'moon', source: 'data', instance: 'server\\1', job: '"grafana/data"'}); + lokiSendLogLine(timestampMs, logFmtLine(item), {place:'luna', source: 'data', instance: 'server\\2', job: '"grafana/data"'}); } } diff --git a/public/app/plugins/datasource/loki/datasource.test.ts b/public/app/plugins/datasource/loki/datasource.test.ts index 6acd8d32f4b..dc5cc8a57d0 100644 --- a/public/app/plugins/datasource/loki/datasource.test.ts +++ b/public/app/plugins/datasource/loki/datasource.test.ts @@ -201,7 +201,7 @@ describe('LokiDatasource', () => { }, ]); expect(ds.applyTemplateVariables(query, {}).expr).toBe( - 'rate({bar="baz", job="foo", k1=~"v\\\\.\\\\*", k2=~"v\'\\\\.\\\\*"} |= "bar" [5m])' + 'rate({bar="baz", job="foo", k1=~"v.*", k2=~"v\\\\\'.*"} |= "bar" [5m])' ); }); }); @@ -665,10 +665,15 @@ describe('LokiDatasource', () => { describe('addAdHocFilters', () => { let ds: LokiDatasource; - let adHocFilters: AdHocFilter[]; + const createTemplateSrvMock = (options: { adHocFilters: AdHocFilter[] }) => { + return { + getAdhocFilters: (): AdHocFilter[] => options.adHocFilters, + replace: (a: string) => a, + } as unknown as TemplateSrv; + }; describe('when called with "=" operator', () => { beforeEach(() => { - adHocFilters = [ + const defaultAdHocFilters: AdHocFilter[] = [ { condition: '', key: 'job', @@ -676,11 +681,7 @@ describe('LokiDatasource', () => { value: 'grafana', }, ]; - const templateSrvMock = { - getAdhocFilters: (): AdHocFilter[] => adHocFilters, - replace: (a: string) => a, - } as unknown as TemplateSrv; - ds = createLokiDatasource(templateSrvMock); + ds = createLokiDatasource(createTemplateSrvMock({ adHocFilters: defaultAdHocFilters })); }); describe('and query has no parser', () => { it('then the correct label should be added for logs query', () => { @@ -706,6 +707,21 @@ describe('LokiDatasource', () => { it('then the correct label should be added for metrics query with empty selector and variable', () => { assertAdHocFilters('rate({}[$__interval])', 'rate({job="grafana"}[$__interval])', ds); }); + it('should correctly escape special characters in ad hoc filter', () => { + const ds = createLokiDatasource( + createTemplateSrvMock({ + adHocFilters: [ + { + condition: '', + key: 'instance', + operator: '=', + value: '"test"', + }, + ], + }) + ); + assertAdHocFilters('{job="grafana"}', '{job="grafana", instance="\\"test\\""}', ds); + }); }); describe('and query has parser', () => { it('then the correct label should be added for logs query', () => { @@ -719,7 +735,7 @@ describe('LokiDatasource', () => { describe('when called with "!=" operator', () => { beforeEach(() => { - adHocFilters = [ + const defaultAdHocFilters: AdHocFilter[] = [ { condition: '', key: 'job', @@ -727,11 +743,7 @@ describe('LokiDatasource', () => { value: 'grafana', }, ]; - const templateSrvMock = { - getAdhocFilters: (): AdHocFilter[] => adHocFilters, - replace: (a: string) => a, - } as unknown as TemplateSrv; - ds = createLokiDatasource(templateSrvMock); + ds = createLokiDatasource(createTemplateSrvMock({ adHocFilters: defaultAdHocFilters })); }); describe('and query has no parser', () => { it('then the correct label should be added for logs query', () => { @@ -751,6 +763,23 @@ describe('LokiDatasource', () => { }); }); }); + + describe('when called with regex operator', () => { + beforeEach(() => { + const defaultAdHocFilters: AdHocFilter[] = [ + { + condition: '', + key: 'instance', + operator: '=~', + value: '.*', + }, + ]; + ds = createLokiDatasource(createTemplateSrvMock({ adHocFilters: defaultAdHocFilters })); + }); + it('should not escape special characters in ad hoc filter', () => { + assertAdHocFilters('{job="grafana"}', '{job="grafana", instance=~".*"}', ds); + }); + }); }); describe('prepareLogRowContextQueryTarget', () => { diff --git a/public/app/plugins/datasource/loki/datasource.ts b/public/app/plugins/datasource/loki/datasource.ts index 766413eef79..58384148e0e 100644 --- a/public/app/plugins/datasource/loki/datasource.ts +++ b/public/app/plugins/datasource/loki/datasource.ts @@ -49,7 +49,7 @@ import LanguageProvider from './LanguageProvider'; import { LiveStreams, LokiLiveTarget } from './LiveStreams'; import { transformBackendResult } from './backendResultTransformer'; import { LokiAnnotationsQueryEditor } from './components/AnnotationsQueryEditor'; -import { escapeLabelValueInSelector } from './languageUtils'; +import { escapeLabelValueInSelector, isRegexSelector } from './languageUtils'; import { labelNamesRegex, labelValuesRegex } from './migrations/variableQueryMigrations'; import { addLabelFormatToQuery, @@ -456,13 +456,15 @@ export class LokiDatasource switch (action.type) { case 'ADD_FILTER': { if (action.options?.key && action.options?.value) { - expression = this.addLabelToQuery(expression, action.options.key, '=', action.options.value); + const value = escapeLabelValueInSelector(action.options.value); + expression = addLabelToQuery(expression, action.options.key, '=', value); } break; } case 'ADD_FILTER_OUT': { if (action.options?.key && action.options?.value) { - expression = this.addLabelToQuery(expression, action.options.key, '!=', action.options.value); + const value = escapeLabelValueInSelector(action.options.value); + expression = addLabelToQuery(expression, action.options.key, '!=', value); } break; } @@ -747,18 +749,23 @@ export class LokiDatasource let expr = replaceVariables(queryExpr); expr = adhocFilters.reduce((acc: string, filter: { key: string; operator: string; value: string }) => { - const { key, operator, value } = filter; - return this.addLabelToQuery(acc, key, operator, value); + const { key, operator } = filter; + let { value } = filter; + if (isRegexSelector(operator)) { + // Adhoc filters don't support multiselect, therefore if user selects regex operator + // we are going to consider value to be regex filter and use lokiRegularEscape + // that does not escape regex special characters (e.g. .*test.* => .*test.*) + value = lokiRegularEscape(value); + } else { + // Otherwise, we want to escape special characters in value + value = escapeLabelValueInSelector(value, operator); + } + return addLabelToQuery(acc, key, operator, value); }, expr); return returnVariables(expr); } - addLabelToQuery(queryExpr: string, key: string, operator: string, value: string) { - const escapedValue = escapeLabelValueInSelector(value, operator); - return addLabelToQuery(queryExpr, key, operator, escapedValue); - } - // Used when running queries through backend filterQuery(query: LokiQuery): boolean { if (query.hide || query.expr === '') { @@ -794,6 +801,9 @@ export class LokiDatasource } } +// NOTE: these two functions are very similar to the escapeLabelValueIn* functions +// in language_utils.ts, but they are not exactly the same algorithm, and we found +// no way to reuse one in the another or vice versa. export function lokiRegularEscape(value: any) { if (typeof value === 'string') { return value.replace(/'/g, "\\\\'");