From bd9cce2866ff507e95b62dd32b73c0a25394f342 Mon Sep 17 00:00:00 2001 From: Gareth Dawson Date: Thu, 5 Jan 2023 13:56:31 +0000 Subject: [PATCH] Loki: Add hints for query filters (#60293) * add hint for label filter * add hint for line filter * add feature tracking for hints click * fix failing tests * add tests for new query builder hints * update hint labels * fix: hint continues to render after adding operation * add missing title property to hints * make use of isQueryWithParser and remove isQueryWithLogFmt * refactor isQueryWithLabelFilter * always render label operation, even if incomplete * suggestion * fix oversight in feature tracking name on hint click * update query hint label * improvements * add missing test for ADD_NO_PIPELINE_ERROR --- .../app/plugins/datasource/loki/datasource.ts | 18 +++ .../plugins/datasource/loki/modifyQuery.ts | 14 +- .../datasource/loki/queryHints.test.ts | 132 +++++++++++++++--- .../app/plugins/datasource/loki/queryHints.ts | 42 +++++- .../app/plugins/datasource/loki/queryUtils.ts | 32 +++++ .../loki/querybuilder/operationUtils.ts | 4 - .../querybuilder/shared/QueryBuilderHints.tsx | 6 + 7 files changed, 221 insertions(+), 27 deletions(-) diff --git a/public/app/plugins/datasource/loki/datasource.ts b/public/app/plugins/datasource/loki/datasource.ts index 890e4eba390..b403051b693 100644 --- a/public/app/plugins/datasource/loki/datasource.ts +++ b/public/app/plugins/datasource/loki/datasource.ts @@ -57,6 +57,12 @@ import { addNoPipelineErrorToQuery, addParserToQuery, removeCommentsFromQuery, + addFilterAsLabelFilter, + getParserPositions, + toLabelFilter, + addLineFilter, + findLastPosition, + getLabelFilterPositions, } from './modifyQuery'; import { getQueryHints } from './queryHints'; import { getNormalizedLokiQuery, isLogsQuery, isValidQuery } from './queryUtils'; @@ -496,6 +502,18 @@ export class LokiDatasource } break; } + case 'ADD_LABEL_FILTER': { + const parserPositions = getParserPositions(query.expr); + const labelFilterPositions = getLabelFilterPositions(query.expr); + const lastPosition = findLastPosition([...parserPositions, ...labelFilterPositions]); + const filter = toLabelFilter('', '', '='); + expression = addFilterAsLabelFilter(expression, [lastPosition], filter); + break; + } + case 'ADD_LINE_FILTER': { + expression = addLineFilter(expression); + break; + } default: break; } diff --git a/public/app/plugins/datasource/loki/modifyQuery.ts b/public/app/plugins/datasource/loki/modifyQuery.ts index 4329eacb142..ffbd6960761 100644 --- a/public/app/plugins/datasource/loki/modifyQuery.ts +++ b/public/app/plugins/datasource/loki/modifyQuery.ts @@ -267,7 +267,7 @@ function getLogQueryPositions(query: string): Position[] { return positions; } -function toLabelFilter(key: string, value: string, operator: string): QueryBuilderLabelFilter { +export function toLabelFilter(key: string, value: string, operator: string): QueryBuilderLabelFilter { // We need to make sure that we convert the value back to string because it may be a number return { label: key, op: operator, value }; } @@ -314,7 +314,7 @@ function addFilterToStreamSelector( * @param positionsToAddAfter * @param filter */ -function addFilterAsLabelFilter( +export function addFilterAsLabelFilter( query: string, positionsToAddAfter: Position[], filter: QueryBuilderLabelFilter @@ -393,6 +393,14 @@ function addLabelFormat( return newQuery; } +export function addLineFilter(query: string): string { + const streamSelectorPositions = getStreamSelectorPositions(query); + const streamSelectorEnd = streamSelectorPositions[0].to; + + const newQueryExpr = query.slice(0, streamSelectorEnd) + ' |= ``' + query.slice(streamSelectorEnd); + return newQueryExpr; +} + function getLineCommentPositions(query: string): Position[] { const tree = parser.parse(query); const positions: Position[] = []; @@ -420,7 +428,7 @@ function labelExists(labels: QueryBuilderLabelFilter[], filter: QueryBuilderLabe * Return the last position based on "to" property * @param positions */ -function findLastPosition(positions: Position[]): Position { +export function findLastPosition(positions: Position[]): Position { return positions.reduce((prev, current) => (prev.to > current.to ? prev : current)); } diff --git a/public/app/plugins/datasource/loki/queryHints.test.ts b/public/app/plugins/datasource/loki/queryHints.test.ts index 9f01591a537..28be4701f77 100644 --- a/public/app/plugins/datasource/loki/queryHints.test.ts +++ b/public/app/plugins/datasource/loki/queryHints.test.ts @@ -16,11 +16,17 @@ describe('getQueryHints', () => { }, ], }; + it('suggest json parser when no parser in query', () => { - expect(getQueryHints('{job="grafana"', [jsonSeries])).toMatchObject([{ type: 'ADD_JSON_PARSER' }]); + expect(getQueryHints('{job="grafana"', [jsonSeries])).toEqual( + expect.arrayContaining([expect.objectContaining({ type: 'ADD_JSON_PARSER' })]) + ); }); + it('does not suggest parser when parser in query', () => { - expect(getQueryHints('{job="grafana" | json', [jsonSeries])).toEqual([]); + expect(getQueryHints('{job="grafana" | json', [jsonSeries])).not.toEqual( + expect.arrayContaining([expect.objectContaining({ type: 'ADD_JSON_PARSER' })]) + ); }); }); @@ -39,10 +45,15 @@ describe('getQueryHints', () => { }; it('suggest logfmt parser when no parser in query', () => { - expect(getQueryHints('{job="grafana"', [logfmtSeries])).toMatchObject([{ type: 'ADD_LOGFMT_PARSER' }]); + expect(getQueryHints('{job="grafana"', [logfmtSeries])).toEqual( + expect.arrayContaining([expect.objectContaining({ type: 'ADD_LOGFMT_PARSER' })]) + ); }); + it('does not suggest parser when parser in query', () => { - expect(getQueryHints('{job="grafana" | json', [logfmtSeries])).toEqual([]); + expect(getQueryHints('{job="grafana" | logfmt', [logfmtSeries])).not.toEqual( + expect.arrayContaining([expect.objectContaining({ type: 'ADD_LOGFMT_PARSER' })]) + ); }); }); @@ -60,23 +71,33 @@ describe('getQueryHints', () => { ], }; - it('suggest logfmt parser when no parser in query', () => { - expect(getQueryHints('{job="grafana"', [jsonAndLogfmtSeries])).toMatchObject([ - { type: 'ADD_JSON_PARSER' }, - { type: 'ADD_LOGFMT_PARSER' }, - ]); + it('suggest logfmt and json parser when no parser in query', () => { + expect(getQueryHints('{job="grafana"', [jsonAndLogfmtSeries])).toEqual( + expect.arrayContaining([ + expect.objectContaining({ type: 'ADD_JSON_PARSER' }), + expect.objectContaining({ type: 'ADD_LOGFMT_PARSER' }), + ]) + ); }); + it('does not suggest parser when parser in query', () => { - expect(getQueryHints('{job="grafana" | json', [jsonAndLogfmtSeries])).toEqual([]); + expect(getQueryHints('{job="grafana" | json', [jsonAndLogfmtSeries])).not.toEqual( + expect.arrayContaining([ + expect.objectContaining({ type: 'ADD_JSON_PARSER' }), + expect.objectContaining({ type: 'ADD_LOGFMT_PARSER' }), + ]) + ); }); }); describe('when series with level-like label', () => { const createSeriesWithLabel = (labelName?: string): DataFrame => { const labelVariable: { [key: string]: string } = { job: 'a' }; + if (labelName) { labelVariable[labelName] = 'error'; } + return { name: 'logs', length: 2, @@ -98,17 +119,90 @@ describe('getQueryHints', () => { }; it('suggest level renaming when no level label', () => { - expect(getQueryHints('{job="grafana"', [createSeriesWithLabel('lvl')])).toMatchObject([ - { type: 'ADD_JSON_PARSER' }, - { type: 'ADD_LOGFMT_PARSER' }, - { type: 'ADD_LEVEL_LABEL_FORMAT' }, - ]); + expect(getQueryHints('{job="grafana"', [createSeriesWithLabel('lvl')])).toEqual( + expect.arrayContaining([expect.objectContaining({ type: 'ADD_LEVEL_LABEL_FORMAT' })]) + ); }); + it('does not suggest level renaming if level label', () => { - expect(getQueryHints('{job="grafana"', [createSeriesWithLabel('level')])).toMatchObject([ - { type: 'ADD_JSON_PARSER' }, - { type: 'ADD_LOGFMT_PARSER' }, - ]); + expect(getQueryHints('{job="grafana"', [createSeriesWithLabel('level')])).not.toEqual( + expect.arrayContaining([expect.objectContaining({ type: 'ADD_LEVEL_LABEL_FORMAT' })]) + ); + }); + }); + + describe('when series with line filter', () => { + const jsonAndLogfmtSeries: DataFrame = { + name: 'logs', + length: 2, + fields: [ + { + name: 'Line', + type: FieldType.string, + config: {}, + values: new ArrayVector(['{"foo": "bar", "bar": "baz"}', 'foo="bar" bar="baz"']), + }, + ], + }; + + it('suggest line filter when no line filter in query', () => { + expect(getQueryHints('{job="grafana"', [jsonAndLogfmtSeries])).toEqual( + expect.arrayContaining([expect.objectContaining({ type: 'ADD_LINE_FILTER' })]) + ); + }); + + it('does not suggest line filter when line filter in query', () => { + expect(getQueryHints('{job="grafana" |= `bar`', [jsonAndLogfmtSeries])).not.toEqual( + expect.arrayContaining([expect.objectContaining({ type: 'ADD_LINE_FILTER' })]) + ); + }); + }); + + describe('when series with label filter', () => { + const jsonAndLogfmtSeries: DataFrame = { + name: 'logs', + length: 2, + fields: [ + { + name: 'Line', + type: FieldType.string, + config: {}, + values: new ArrayVector(['{"foo": "bar", "bar": "baz"}', 'foo="bar" bar="baz"']), + }, + ], + }; + + it('suggest label filter when no label filter in query', () => { + expect(getQueryHints('{job="grafana" | logfmt', [jsonAndLogfmtSeries])).toEqual( + expect.arrayContaining([expect.objectContaining({ type: 'ADD_LABEL_FILTER' })]) + ); + }); + + it('does not suggest label filter when label filter in query', () => { + expect(getQueryHints('{job="grafana" | logfmt | foo = `bar`', [jsonAndLogfmtSeries])).not.toEqual( + expect.arrayContaining([expect.objectContaining({ type: 'ADD_LABEL_FILTER' })]) + ); + }); + }); + + describe('suggest remove pipeline error', () => { + const logfmtSeries: DataFrame = { + name: 'logs', + length: 1, + fields: [ + { + name: 'labels', + type: FieldType.other, + config: {}, + values: new ArrayVector([{ __error__: 'some error', job: 'a' }]), + }, + ], + }; + + it('suggest remove pipeline error', () => { + expect(getQueryHints('{job="grafana" | json', [logfmtSeries])).toEqual( + expect.arrayContaining([expect.objectContaining({ type: 'ADD_NO_PIPELINE_ERROR' })]) + ); }); }); }); diff --git a/public/app/plugins/datasource/loki/queryHints.ts b/public/app/plugins/datasource/loki/queryHints.ts index e48c8b2cb75..87d3275d8bb 100644 --- a/public/app/plugins/datasource/loki/queryHints.ts +++ b/public/app/plugins/datasource/loki/queryHints.ts @@ -1,6 +1,12 @@ import { DataFrame, QueryHint } from '@grafana/data'; -import { isQueryPipelineErrorFiltering, isQueryWithLabelFormat, isQueryWithParser } from './queryUtils'; +import { + isQueryWithLabelFilter, + isQueryPipelineErrorFiltering, + isQueryWithLabelFormat, + isQueryWithParser, + isQueryWithLineFilter, +} from './queryUtils'; import { dataFrameHasLevelLabel, extractHasErrorLabelFromDataFrame, @@ -69,6 +75,23 @@ export function getQueryHints(query: string, series: DataFrame[]): QueryHint[] { }); } } + + const hasLabelFilter = isQueryWithLabelFilter(query); + + if (!hasLabelFilter) { + hints.push({ + type: 'ADD_LABEL_FILTER', + label: 'Consider filtering logs by their label and value.', + fix: { + title: 'add label filter', + label: '', + action: { + type: 'ADD_LABEL_FILTER', + query, + }, + }, + }); + } } const queryWithLabelFormat = isQueryWithLabelFormat(query); @@ -97,5 +120,22 @@ export function getQueryHints(query: string, series: DataFrame[]): QueryHint[] { } } + const hasLineFilter = isQueryWithLineFilter(query); + + if (!hasLineFilter) { + hints.push({ + type: 'ADD_LINE_FILTER', + label: 'Consider filtering logs for specific string.', + fix: { + title: 'add line filter', + label: '', + action: { + type: 'ADD_LINE_FILTER', + query, + }, + }, + }); + } + return hints; } diff --git a/public/app/plugins/datasource/loki/queryUtils.ts b/public/app/plugins/datasource/loki/queryUtils.ts index b0997d9cc74..7922998f2a4 100644 --- a/public/app/plugins/datasource/loki/queryUtils.ts +++ b/public/app/plugins/datasource/loki/queryUtils.ts @@ -236,3 +236,35 @@ export function getLogQueryFromMetricsQuery(query: string): string { return selector + pipelineExpr; } + +export function isQueryWithLabelFilter(query: string): boolean { + const tree = parser.parse(query); + let hasLabelFilter = false; + + tree.iterate({ + enter: ({ type, node }): false | void => { + if (type.id === LabelFilter) { + hasLabelFilter = true; + return; + } + }, + }); + + return hasLabelFilter; +} + +export function isQueryWithLineFilter(query: string): boolean { + const tree = parser.parse(query); + let queryWithLineFilter = false; + + tree.iterate({ + enter: ({ type }): false | void => { + if (type.id === LineFilter) { + queryWithLineFilter = true; + return; + } + }, + }); + + return queryWithLineFilter; +} diff --git a/public/app/plugins/datasource/loki/querybuilder/operationUtils.ts b/public/app/plugins/datasource/loki/querybuilder/operationUtils.ts index b9808dbb41a..7ee156715af 100644 --- a/public/app/plugins/datasource/loki/querybuilder/operationUtils.ts +++ b/public/app/plugins/datasource/loki/querybuilder/operationUtils.ts @@ -148,10 +148,6 @@ function operationWithRangeVectorRenderer( } export function labelFilterRenderer(model: QueryBuilderOperation, def: QueryBuilderOperationDef, innerExpr: string) { - if (model.params[0] === '') { - return innerExpr; - } - if (model.params[1] === '<' || model.params[1] === '>') { return `${innerExpr} | ${model.params[0]} ${model.params[1]} ${model.params[2]}`; } diff --git a/public/app/plugins/datasource/prometheus/querybuilder/shared/QueryBuilderHints.tsx b/public/app/plugins/datasource/prometheus/querybuilder/shared/QueryBuilderHints.tsx index 7f7f30a01c6..00ff96d2780 100644 --- a/public/app/plugins/datasource/prometheus/querybuilder/shared/QueryBuilderHints.tsx +++ b/public/app/plugins/datasource/prometheus/querybuilder/shared/QueryBuilderHints.tsx @@ -2,6 +2,7 @@ import { css } from '@emotion/css'; import React, { useState, useEffect } from 'react'; import { GrafanaTheme2, PanelData, QueryHint } from '@grafana/data'; +import { reportInteraction } from '@grafana/runtime'; import { Button, Tooltip, useStyles2 } from '@grafana/ui'; import { LokiDatasource } from 'app/plugins/datasource/loki/datasource'; @@ -45,6 +46,11 @@ export const QueryBuilderHints = ({