From 062fa2daa2ec25fa75b401e2ee9d115ddf5a6dc7 Mon Sep 17 00:00:00 2001 From: Sven Grossmann Date: Wed, 14 Feb 2024 17:18:46 +0100 Subject: [PATCH] Loki Log Context: Always show label filters with at least one parsed label (#82211) * Loki: Always show label filter for parsed labels * add label as structured metadata * change contextfilter `fromParser` to `nonIndexed` * rename variable * simplify * fix test * fix test * rename more properties --- .../loki/LogContextProvider.test.ts | 86 +++++++++---------- .../datasource/loki/LogContextProvider.ts | 37 +++++--- .../loki/components/LokiContextUi.test.tsx | 8 +- .../loki/components/LokiContextUi.tsx | 27 +++--- public/app/plugins/datasource/loki/types.ts | 3 +- 5 files changed, 85 insertions(+), 76 deletions(-) diff --git a/public/app/plugins/datasource/loki/LogContextProvider.test.ts b/public/app/plugins/datasource/loki/LogContextProvider.test.ts index 0b03f5fa7c1..0eb962a89df 100644 --- a/public/app/plugins/datasource/loki/LogContextProvider.test.ts +++ b/public/app/plugins/datasource/loki/LogContextProvider.test.ts @@ -57,7 +57,7 @@ describe('LogContextProvider', () => { describe('getLogRowContext', () => { it('should call getInitContextFilters if no cachedContextFilters', async () => { logContextProvider.getInitContextFilters = jest.fn().mockResolvedValue({ - contextFilters: [{ value: 'baz', enabled: true, fromParser: false, label: 'bar' }], + contextFilters: [{ value: 'baz', enabled: true, nonIndexed: false, label: 'bar' }], preservedFiltersApplied: false, }); @@ -89,11 +89,11 @@ describe('LogContextProvider', () => { it('should not call getInitContextFilters if cachedContextFilters', async () => { logContextProvider.getInitContextFilters = jest .fn() - .mockResolvedValue([{ value: 'baz', enabled: true, fromParser: false, label: 'bar' }]); + .mockResolvedValue([{ value: 'baz', enabled: true, nonIndexed: false, label: 'bar' }]); logContextProvider.cachedContextFilters = [ - { value: 'baz', enabled: true, fromParser: false, label: 'bar' }, - { value: 'abc', enabled: true, fromParser: false, label: 'xyz' }, + { value: 'baz', enabled: true, nonIndexed: false, label: 'bar' }, + { value: 'abc', enabled: true, nonIndexed: false, label: 'xyz' }, ]; await logContextProvider.getLogRowContext(defaultLogRow, { limit: 10, @@ -107,7 +107,7 @@ describe('LogContextProvider', () => { describe('getLogRowContextQuery', () => { it('should call getInitContextFilters if no cachedContextFilters', async () => { logContextProvider.getInitContextFilters = jest.fn().mockResolvedValue({ - contextFilters: [{ value: 'baz', enabled: true, fromParser: false, label: 'bar' }], + contextFilters: [{ value: 'baz', enabled: true, nonIndexed: false, label: 'bar' }], preservedFiltersApplied: false, }); @@ -121,12 +121,12 @@ describe('LogContextProvider', () => { it('should also call getInitContextFilters if cacheFilters is not set', async () => { logContextProvider.getInitContextFilters = jest.fn().mockResolvedValue({ - contextFilters: [{ value: 'baz', enabled: true, fromParser: false, label: 'bar' }], + contextFilters: [{ value: 'baz', enabled: true, nonIndexed: false, label: 'bar' }], preservedFiltersApplied: false, }); logContextProvider.cachedContextFilters = [ - { value: 'baz', enabled: true, fromParser: false, label: 'bar' }, - { value: 'abc', enabled: true, fromParser: false, label: 'xyz' }, + { value: 'baz', enabled: true, nonIndexed: false, label: 'bar' }, + { value: 'abc', enabled: true, nonIndexed: false, label: 'xyz' }, ]; await logContextProvider.getLogRowContextQuery( defaultLogRow, @@ -158,11 +158,11 @@ describe('LogContextProvider', () => { expect(result.query.expr).toEqual('{}'); }); - it('should not apply parsed labels', async () => { + it('should apply parsed label as structured metadata', async () => { logContextProvider.cachedContextFilters = [ - { value: 'baz', enabled: true, fromParser: false, label: 'bar' }, - { value: 'abc', enabled: true, fromParser: false, label: 'xyz' }, - { value: 'uniqueParsedLabel', enabled: true, fromParser: true, label: 'foo' }, + { value: 'baz', enabled: true, nonIndexed: false, label: 'bar' }, + { value: 'abc', enabled: true, nonIndexed: false, label: 'xyz' }, + { value: 'uniqueParsedLabel', enabled: true, nonIndexed: true, label: 'foo' }, ]; const contextQuery = await logContextProvider.prepareLogRowContextQueryTarget( defaultLogRow, @@ -171,15 +171,15 @@ describe('LogContextProvider', () => { query ); - expect(contextQuery.query.expr).toEqual('{bar="baz",xyz="abc"}'); + expect(contextQuery.query.expr).toEqual('{bar="baz",xyz="abc"} | foo=`uniqueParsedLabel`'); }); }); describe('query with parser', () => { it('should apply parser', async () => { logContextProvider.cachedContextFilters = [ - { value: 'baz', enabled: true, fromParser: false, label: 'bar' }, - { value: 'abc', enabled: true, fromParser: false, label: 'xyz' }, + { value: 'baz', enabled: true, nonIndexed: false, label: 'bar' }, + { value: 'abc', enabled: true, nonIndexed: false, label: 'xyz' }, ]; const contextQuery = await logContextProvider.prepareLogRowContextQueryTarget( defaultLogRow, @@ -196,9 +196,9 @@ describe('LogContextProvider', () => { it('should apply parser and parsed labels', async () => { logContextProvider.cachedContextFilters = [ - { value: 'baz', enabled: true, fromParser: false, label: 'bar' }, - { value: 'abc', enabled: true, fromParser: false, label: 'xyz' }, - { value: 'uniqueParsedLabel', enabled: true, fromParser: true, label: 'foo' }, + { value: 'baz', enabled: true, nonIndexed: false, label: 'bar' }, + { value: 'abc', enabled: true, nonIndexed: false, label: 'xyz' }, + { value: 'uniqueParsedLabel', enabled: true, nonIndexed: true, label: 'foo' }, ]; const contextQuery = await logContextProvider.prepareLogRowContextQueryTarget( defaultLogRow, @@ -216,8 +216,8 @@ describe('LogContextProvider', () => { it('should not apply parser and parsed labels if more parsers in original query', async () => { logContextProvider.cachedContextFilters = [ - { value: 'baz', enabled: true, fromParser: false, label: 'bar' }, - { value: 'uniqueParsedLabel', enabled: true, fromParser: true, label: 'foo' }, + { value: 'baz', enabled: true, nonIndexed: false, label: 'bar' }, + { value: 'uniqueParsedLabel', enabled: true, nonIndexed: true, label: 'foo' }, ]; const contextQuery = await logContextProvider.prepareLogRowContextQueryTarget( defaultLogRow, @@ -229,11 +229,11 @@ describe('LogContextProvider', () => { } ); - expect(contextQuery.query.expr).toEqual(`{bar="baz"}`); + expect(contextQuery.query.expr).toEqual(`{bar="baz"} | foo=\`uniqueParsedLabel\``); }); it('should not apply line_format if flag is not set by default', async () => { - logContextProvider.cachedContextFilters = [{ value: 'baz', enabled: true, fromParser: false, label: 'bar' }]; + logContextProvider.cachedContextFilters = [{ value: 'baz', enabled: true, nonIndexed: false, label: 'bar' }]; const contextQuery = await logContextProvider.prepareLogRowContextQueryTarget( defaultLogRow, 10, @@ -249,7 +249,7 @@ describe('LogContextProvider', () => { it('should not apply line_format if flag is not set', async () => { window.localStorage.setItem(SHOULD_INCLUDE_PIPELINE_OPERATIONS, 'false'); - logContextProvider.cachedContextFilters = [{ value: 'baz', enabled: true, fromParser: false, label: 'bar' }]; + logContextProvider.cachedContextFilters = [{ value: 'baz', enabled: true, nonIndexed: false, label: 'bar' }]; const contextQuery = await logContextProvider.prepareLogRowContextQueryTarget( defaultLogRow, 10, @@ -265,7 +265,7 @@ describe('LogContextProvider', () => { it('should apply line_format if flag is set', async () => { window.localStorage.setItem(SHOULD_INCLUDE_PIPELINE_OPERATIONS, 'true'); - logContextProvider.cachedContextFilters = [{ value: 'baz', enabled: true, fromParser: false, label: 'bar' }]; + logContextProvider.cachedContextFilters = [{ value: 'baz', enabled: true, nonIndexed: false, label: 'bar' }]; const contextQuery = await logContextProvider.prepareLogRowContextQueryTarget( defaultLogRow, 10, @@ -281,7 +281,7 @@ describe('LogContextProvider', () => { it('should not apply line filters if flag is set', async () => { window.localStorage.setItem(SHOULD_INCLUDE_PIPELINE_OPERATIONS, 'true'); - logContextProvider.cachedContextFilters = [{ value: 'baz', enabled: true, fromParser: false, label: 'bar' }]; + logContextProvider.cachedContextFilters = [{ value: 'baz', enabled: true, nonIndexed: false, label: 'bar' }]; let contextQuery = await logContextProvider.prepareLogRowContextQueryTarget( defaultLogRow, 10, @@ -333,7 +333,7 @@ describe('LogContextProvider', () => { it('should not apply line filters if nested between two operations', async () => { window.localStorage.setItem(SHOULD_INCLUDE_PIPELINE_OPERATIONS, 'true'); - logContextProvider.cachedContextFilters = [{ value: 'baz', enabled: true, fromParser: false, label: 'bar' }]; + logContextProvider.cachedContextFilters = [{ value: 'baz', enabled: true, nonIndexed: false, label: 'bar' }]; const contextQuery = await logContextProvider.prepareLogRowContextQueryTarget( defaultLogRow, 10, @@ -349,7 +349,7 @@ describe('LogContextProvider', () => { it('should not apply label filters', async () => { window.localStorage.setItem(SHOULD_INCLUDE_PIPELINE_OPERATIONS, 'true'); - logContextProvider.cachedContextFilters = [{ value: 'baz', enabled: true, fromParser: false, label: 'bar' }]; + logContextProvider.cachedContextFilters = [{ value: 'baz', enabled: true, nonIndexed: false, label: 'bar' }]; const contextQuery = await logContextProvider.prepareLogRowContextQueryTarget( defaultLogRow, 10, @@ -365,7 +365,7 @@ describe('LogContextProvider', () => { it('should not apply additional parsers', async () => { window.localStorage.setItem(SHOULD_INCLUDE_PIPELINE_OPERATIONS, 'true'); - logContextProvider.cachedContextFilters = [{ value: 'baz', enabled: true, fromParser: false, label: 'bar' }]; + logContextProvider.cachedContextFilters = [{ value: 'baz', enabled: true, nonIndexed: false, label: 'bar' }]; const contextQuery = await logContextProvider.prepareLogRowContextQueryTarget( defaultLogRow, 10, @@ -401,9 +401,9 @@ describe('LogContextProvider', () => { it('should correctly create contextFilters', async () => { const result = await logContextProvider.getInitContextFilters(defaultLogRow.labels, queryWithoutParser); expect(result.contextFilters).toEqual([ - { enabled: true, fromParser: false, label: 'bar', value: 'baz' }, - { enabled: false, fromParser: true, label: 'foo', value: 'uniqueParsedLabel' }, - { enabled: true, fromParser: false, label: 'xyz', value: 'abc' }, + { enabled: true, nonIndexed: false, label: 'bar', value: 'baz' }, + { enabled: false, nonIndexed: true, label: 'foo', value: 'uniqueParsedLabel' }, + { enabled: true, nonIndexed: false, label: 'xyz', value: 'abc' }, ]); expect(result.preservedFiltersApplied).toBe(false); }); @@ -444,9 +444,9 @@ describe('LogContextProvider', () => { it('should correctly create contextFilters', async () => { const result = await logContextProvider.getInitContextFilters(defaultLogRow.labels, queryWithParser); expect(result.contextFilters).toEqual([ - { enabled: true, fromParser: false, label: 'bar', value: 'baz' }, - { enabled: false, fromParser: true, label: 'foo', value: 'uniqueParsedLabel' }, - { enabled: true, fromParser: false, label: 'xyz', value: 'abc' }, + { enabled: true, nonIndexed: false, label: 'bar', value: 'baz' }, + { enabled: false, nonIndexed: true, label: 'foo', value: 'uniqueParsedLabel' }, + { enabled: true, nonIndexed: false, label: 'xyz', value: 'abc' }, ]); expect(result.preservedFiltersApplied).toBe(false); }); @@ -479,9 +479,9 @@ describe('LogContextProvider', () => { ); const result = await logContextProvider.getInitContextFilters(defaultLogRow.labels, queryWithParser); expect(result.contextFilters).toEqual([ - { enabled: false, fromParser: false, label: 'bar', value: 'baz' }, // disabled real label - { enabled: true, fromParser: true, label: 'foo', value: 'uniqueParsedLabel' }, // enabled parsed label - { enabled: true, fromParser: false, label: 'xyz', value: 'abc' }, + { enabled: false, nonIndexed: false, label: 'bar', value: 'baz' }, // disabled real label + { enabled: true, nonIndexed: true, label: 'foo', value: 'uniqueParsedLabel' }, // enabled parsed label + { enabled: true, nonIndexed: false, label: 'xyz', value: 'abc' }, ]); expect(result.preservedFiltersApplied).toBe(true); }); @@ -496,9 +496,9 @@ describe('LogContextProvider', () => { ); const result = await logContextProvider.getInitContextFilters(defaultLogRow.labels, queryWithParser); expect(result.contextFilters).toEqual([ - { enabled: true, fromParser: false, label: 'bar', value: 'baz' }, // enabled real label - { enabled: false, fromParser: true, label: 'foo', value: 'uniqueParsedLabel' }, - { enabled: true, fromParser: false, label: 'xyz', value: 'abc' }, // enabled real label + { enabled: true, nonIndexed: false, label: 'bar', value: 'baz' }, // enabled real label + { enabled: false, nonIndexed: true, label: 'foo', value: 'uniqueParsedLabel' }, + { enabled: true, nonIndexed: false, label: 'xyz', value: 'abc' }, // enabled real label ]); expect(result.preservedFiltersApplied).toBe(false); }); @@ -513,9 +513,9 @@ describe('LogContextProvider', () => { ); const result = await logContextProvider.getInitContextFilters(defaultLogRow.labels, queryWithParser); expect(result.contextFilters).toEqual([ - { enabled: false, fromParser: false, label: 'bar', value: 'baz' }, - { enabled: true, fromParser: true, label: 'foo', value: 'uniqueParsedLabel' }, - { enabled: true, fromParser: false, label: 'xyz', value: 'abc' }, + { enabled: false, nonIndexed: false, label: 'bar', value: 'baz' }, + { enabled: true, nonIndexed: true, label: 'foo', value: 'uniqueParsedLabel' }, + { enabled: true, nonIndexed: false, label: 'xyz', value: 'abc' }, ]); expect(result.preservedFiltersApplied).toBe(true); }); diff --git a/public/app/plugins/datasource/loki/LogContextProvider.ts b/public/app/plugins/datasource/loki/LogContextProvider.ts index fdbcf65b646..e7e45999ddd 100644 --- a/public/app/plugins/datasource/loki/LogContextProvider.ts +++ b/public/app/plugins/datasource/loki/LogContextProvider.ts @@ -29,7 +29,7 @@ import { isQueryWithParser, } from './queryUtils'; import { sortDataFrameByTime, SortDirection } from './sortDataFrame'; -import { ContextFilter, LokiQuery, LokiQueryDirection, LokiQueryType } from './types'; +import { ContextFilter, LabelType, LokiQuery, LokiQueryDirection, LokiQueryType } from './types'; export const LOKI_LOG_CONTEXT_PRESERVED_LABELS = 'lokiLogContextPreservedLabels'; export const SHOULD_INCLUDE_PIPELINE_OPERATIONS = 'lokiLogContextShouldIncludePipelineOperations'; @@ -223,7 +223,7 @@ export class LogContextProvider { processContextFiltersToExpr = (contextFilters: ContextFilter[], query: LokiQuery | undefined): string => { const labelFilters = contextFilters .map((filter) => { - if (!filter.fromParser && filter.enabled) { + if (!filter.nonIndexed && filter.enabled) { // escape backslashes in label as users can't escape them by themselves return `${filter.label}="${escapeLabelValueInExactSelector(filter.value)}"`; } @@ -237,15 +237,26 @@ export class LogContextProvider { // We need to have original query to get parser and include parsed labels // We only add parser and parsed labels if there is only one parser in query - if (query && isQueryWithParser(query.expr).parserCount === 1) { - const parser = getParserFromQuery(query.expr); - if (parser) { - expr = addParserToQuery(expr, parser); - const parsedLabels = contextFilters.filter((filter) => filter.fromParser && filter.enabled); - for (const parsedLabel of parsedLabels) { - if (parsedLabel.enabled) { - expr = addLabelToQuery(expr, parsedLabel.label, '=', parsedLabel.value); - } + if (query) { + let hasParser = false; + if (isQueryWithParser(query.expr).parserCount === 1) { + hasParser = true; + const parser = getParserFromQuery(query.expr); + if (parser) { + expr = addParserToQuery(expr, parser); + } + } + + const nonIndexedLabels = contextFilters.filter((filter) => filter.nonIndexed && filter.enabled); + for (const parsedLabel of nonIndexedLabels) { + if (parsedLabel.enabled) { + expr = addLabelToQuery( + expr, + parsedLabel.label, + '=', + parsedLabel.value, + hasParser ? LabelType.Parsed : LabelType.StructuredMetadata + ); } } } @@ -332,7 +343,7 @@ export class LogContextProvider { label, value: value, enabled: allLabels.includes(label), - fromParser: !allLabels.includes(label), + nonIndexed: !allLabels.includes(label), }; contextFilters.push(filter); @@ -368,7 +379,7 @@ export class LogContextProvider { return { ...contextFilter }; }); - const isAtLeastOneRealLabelEnabled = newContextFilters.some(({ enabled, fromParser }) => enabled && !fromParser); + const isAtLeastOneRealLabelEnabled = newContextFilters.some(({ enabled, nonIndexed }) => enabled && !nonIndexed); if (!isAtLeastOneRealLabelEnabled) { // If we end up with no real labels enabled, we need to reset the init filters return { contextFilters, preservedFiltersApplied }; diff --git a/public/app/plugins/datasource/loki/components/LokiContextUi.test.tsx b/public/app/plugins/datasource/loki/components/LokiContextUi.test.tsx index a18f456b672..0ba89331811 100644 --- a/public/app/plugins/datasource/loki/components/LokiContextUi.test.tsx +++ b/public/app/plugins/datasource/loki/components/LokiContextUi.test.tsx @@ -43,8 +43,8 @@ const mockLogContextProvider = { getInitContextFilters: jest.fn().mockImplementation(() => Promise.resolve({ contextFilters: [ - { value: 'value1', enabled: true, fromParser: false, label: 'label1' }, - { value: 'value3', enabled: false, fromParser: true, label: 'label3' }, + { value: 'value1', enabled: true, nonIndexed: false, label: 'label1' }, + { value: 'value3', enabled: false, nonIndexed: true, label: 'label3' }, ], preservedFiltersApplied: false, }) @@ -339,8 +339,8 @@ describe('LokiContextUi', () => { getInitContextFilters: jest.fn().mockImplementation(() => Promise.resolve({ contextFilters: [ - { value: 'value1', enabled: true, fromParser: false, label: 'label1' }, - { value: 'value3', enabled: false, fromParser: true, label: 'label3' }, + { value: 'value1', enabled: true, nonIndexed: false, label: 'label1' }, + { value: 'value3', enabled: false, nonIndexed: true, label: 'label3' }, ], preservedFiltersApplied: true, }) diff --git a/public/app/plugins/datasource/loki/components/LokiContextUi.tsx b/public/app/plugins/datasource/loki/components/LokiContextUi.tsx index 0b03fd32316..abea3b584c2 100644 --- a/public/app/plugins/datasource/loki/components/LokiContextUi.tsx +++ b/public/app/plugins/datasource/loki/components/LokiContextUi.tsx @@ -28,7 +28,6 @@ import { SHOULD_INCLUDE_PIPELINE_OPERATIONS, } from '../LogContextProvider'; import { escapeLabelValueInSelector } from '../languageUtils'; -import { isQueryWithParser } from '../queryUtils'; import { lokiGrammar } from '../syntax'; import { ContextFilter, LokiQuery } from '../types'; @@ -133,7 +132,7 @@ export function LokiContextUi(props: LokiContextUiProps) { const isInitialState = useMemo(() => { // Initial query has all regular labels enabled and all parsed labels disabled - if (initialized && contextFilters.some((filter) => filter.fromParser === filter.enabled)) { + if (initialized && contextFilters.some((filter) => filter.nonIndexed === filter.enabled)) { return false; } @@ -156,7 +155,7 @@ export function LokiContextUi(props: LokiContextUiProps) { return; } - if (contextFilters.filter(({ enabled, fromParser }) => enabled && !fromParser).length === 0) { + if (contextFilters.filter(({ enabled, nonIndexed }) => enabled && !nonIndexed).length === 0) { setContextFilters(previousContextFilters.current); return; } @@ -176,13 +175,13 @@ export function LokiContextUi(props: LokiContextUiProps) { selectedExtractedLabels: [], }; - contextFilters.forEach(({ enabled, fromParser, label }) => { + contextFilters.forEach(({ enabled, nonIndexed, label }) => { // We only want to store real labels that were removed from the initial query - if (!enabled && !fromParser) { + if (!enabled && !nonIndexed) { preservedLabels.removedLabels.push(label); } // Or extracted labels that were added to the initial query - if (enabled && fromParser) { + if (enabled && nonIndexed) { preservedLabels.selectedExtractedLabels.push(label); } }); @@ -240,10 +239,10 @@ export function LokiContextUi(props: LokiContextUiProps) { }; }, [row.uid]); - const realLabels = contextFilters.filter(({ fromParser }) => !fromParser); + const realLabels = contextFilters.filter(({ nonIndexed }) => !nonIndexed); const realLabelsEnabled = realLabels.filter(({ enabled }) => enabled); - const parsedLabels = contextFilters.filter(({ fromParser }) => fromParser); + const parsedLabels = contextFilters.filter(({ nonIndexed }) => nonIndexed); const parsedLabelsEnabled = parsedLabels.filter(({ enabled }) => enabled); const contextFilterToSelectFilter = useCallback((contextFilter: ContextFilter): SelectableValue => { @@ -253,8 +252,8 @@ export function LokiContextUi(props: LokiContextUiProps) { }; }, []); - // Currently we support adding of parser and showing parsed labels only if there is 1 parser - const showParsedLabels = origQuery && isQueryWithParser(origQuery.expr).parserCount === 1 && parsedLabels.length > 0; + // If there's any nonIndexed labels, that includes structured metadata and parsed labels, we show the nonIndexed labels input + const showNonIndexedLabels = parsedLabels.length > 0; let queryExpr = logContextProvider.prepareExpression( contextFilters.filter(({ enabled }) => enabled), @@ -285,7 +284,7 @@ export function LokiContextUi(props: LokiContextUiProps) { return contextFilters.map((contextFilter) => ({ ...contextFilter, // For revert to initial query we need to enable all labels and disable all parsed labels - enabled: !contextFilter.fromParser, + enabled: !contextFilter.nonIndexed, })); }); // We are removing the preserved labels from local storage so we can preselect the labels in the UI @@ -358,7 +357,7 @@ export function LokiContextUi(props: LokiContextUiProps) { } return setContextFilters( contextFilters.map((filter) => { - if (filter.fromParser) { + if (filter.nonIndexed) { return filter; } filter.enabled = keys.some((key) => key.value === filter.label); @@ -367,7 +366,7 @@ export function LokiContextUi(props: LokiContextUiProps) { ); }} /> - {showParsedLabels && ( + {showNonIndexedLabels && ( <>