From 1d077e84ce469106384072c25982d2dcb2cc2de9 Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Thu, 14 Jul 2022 10:48:27 +0200 Subject: [PATCH] Loki: Add hint for pipeline error to query builder (#52134) * WIP * Update * Add tests * Fix lint/betterer errors * Update public/app/plugins/datasource/loki/addToQuery.ts * Update copy --- .betterer.results | 25 +++++++++++ .../app/plugins/datasource/loki/addToQuery.ts | 18 +++++++- .../datasource/loki/addtoQuery.test.ts | 18 +++++++- .../app/plugins/datasource/loki/datasource.ts | 10 +++-- .../app/plugins/datasource/loki/queryHints.ts | 43 +++++++++++++++---- .../datasource/loki/query_utils.test.ts | 14 ++++-- .../plugins/datasource/loki/query_utils.ts | 30 ++++++++++--- .../plugins/datasource/loki/responseUtils.ts | 10 +++++ 8 files changed, 144 insertions(+), 24 deletions(-) diff --git a/.betterer.results b/.betterer.results index 50522743d2b..bb458de4bdc 100644 --- a/.betterer.results +++ b/.betterer.results @@ -7795,11 +7795,36 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"] ], + "public/app/plugins/datasource/mssql/query_ctrl.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"] + ], "public/app/plugins/datasource/mssql/response_parser.ts:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"], [0, 0, 0, "Do not use any type assertions.", "2"] ], + "public/app/plugins/datasource/mssql/specs/datasource.test.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, "Unexpected any. Specify a different type.", "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"] + ], + "public/app/plugins/datasource/mssql/types.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, "Unexpected any. Specify a different type.", "4"], + [0, 0, 0, "Unexpected any. Specify a different type.", "5"] + ], "public/app/plugins/datasource/mysql/datasource.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"], diff --git a/public/app/plugins/datasource/loki/addToQuery.ts b/public/app/plugins/datasource/loki/addToQuery.ts index a3ce4ff7e77..34b27a6cbe0 100644 --- a/public/app/plugins/datasource/loki/addToQuery.ts +++ b/public/app/plugins/datasource/loki/addToQuery.ts @@ -57,6 +57,22 @@ export function addParserToQuery(query: string, parser: string): string { } } +/** + * Adds filtering for pipeline errors to existing query. Useful for query modification for hints. + * It uses LogQL parser to find parsers and adds pipeline errors filtering after them. + * + * @param query + */ +export function addNoPipelineErrorToQuery(query: string): string { + const parserPositions = getParserPositions(query); + if (!parserPositions.length) { + return query; + } + + const filter = toLabelFilter('__error__', '', '='); + return addFilterAsLabelFilter(query, parserPositions, filter); +} + /** * Parse the string and get all Selector positions in the query together with parsed representation of the * selector. @@ -85,7 +101,7 @@ export function getParserPositions(query: string): Position[] { const positions: Position[] = []; tree.iterate({ enter: (type, from, to, get): false | void => { - if (type.name === 'LabelParser') { + if (type.name === 'LabelParser' || type.name === 'JsonExpressionParser') { positions.push({ from, to }); return false; } diff --git a/public/app/plugins/datasource/loki/addtoQuery.test.ts b/public/app/plugins/datasource/loki/addtoQuery.test.ts index dc3765bc7a7..4d69467d11a 100644 --- a/public/app/plugins/datasource/loki/addtoQuery.test.ts +++ b/public/app/plugins/datasource/loki/addtoQuery.test.ts @@ -1,4 +1,4 @@ -import { addLabelToQuery, addParserToQuery } from './addToQuery'; +import { addLabelToQuery, addNoPipelineErrorToQuery, addParserToQuery } from './addToQuery'; describe('addLabelToQuery()', () => { it('should add label to simple query', () => { @@ -177,3 +177,19 @@ describe('addParserToQuery', () => { }); }); }); + +describe('addNoPipelineErrorToQuery', () => { + it('should add error filtering after logfmt parser', () => { + expect(addNoPipelineErrorToQuery('{job="grafana"} | logfmt')).toBe('{job="grafana"} | logfmt | __error__=``'); + }); + + it('should add error filtering after json parser with expressions', () => { + expect(addNoPipelineErrorToQuery('{job="grafana"} | json foo="bar", bar="baz"')).toBe( + '{job="grafana"} | json foo="bar", bar="baz" | __error__=``' + ); + }); + + it('should not add error filtering if no parser', () => { + expect(addNoPipelineErrorToQuery('{job="grafana"} |="no parser"')).toBe('{job="grafana"} |="no parser"'); + }); +}); diff --git a/public/app/plugins/datasource/loki/datasource.ts b/public/app/plugins/datasource/loki/datasource.ts index 7c90a78b2a7..d1b18db48d1 100644 --- a/public/app/plugins/datasource/loki/datasource.ts +++ b/public/app/plugins/datasource/loki/datasource.ts @@ -1,9 +1,7 @@ -// Libraries import { cloneDeep, map as lodashMap } from 'lodash'; import { lastValueFrom, merge, Observable, of, throwError } from 'rxjs'; import { catchError, map, switchMap } from 'rxjs/operators'; -// Types import { AnnotationEvent, AnnotationQueryRequest, @@ -45,7 +43,7 @@ import { getTemplateSrv, TemplateSrv } from 'app/features/templating/template_sr import { serializeParams } from '../../../core/utils/fetch'; import { renderLegendFormat } from '../prometheus/legend'; -import { addLabelToQuery, addParserToQuery } from './addToQuery'; +import { addLabelToQuery, addNoPipelineErrorToQuery, addParserToQuery } from './addToQuery'; import { transformBackendResult } from './backendResultTransformer'; import { LokiAnnotationsQueryEditor } from './components/AnnotationsQueryEditor'; import LanguageProvider from './language_provider'; @@ -359,7 +357,7 @@ export class LokiDatasource expr: query.expr, queryType: LokiQueryType.Range, refId: 'log-samples', - maxLines: 10, + maxLines: 50, }; // For samples, we use defaultTimeRange (now-6h/now) and limit od 10 lines so queries are small and fast @@ -410,6 +408,10 @@ export class LokiDatasource expression = addParserToQuery(expression, 'json'); break; } + case 'ADD_NO_PIPELINE_ERROR': { + expression = addNoPipelineErrorToQuery(expression); + break; + } default: break; } diff --git a/public/app/plugins/datasource/loki/queryHints.ts b/public/app/plugins/datasource/loki/queryHints.ts index 64e02f365da..a31a5dfbdf4 100644 --- a/public/app/plugins/datasource/loki/queryHints.ts +++ b/public/app/plugins/datasource/loki/queryHints.ts @@ -1,15 +1,19 @@ import { DataFrame, QueryHint } from '@grafana/data'; -import { isQueryWithParser } from './query_utils'; -import { extractLogParserFromDataFrame } from './responseUtils'; +import { isQueryPipelineErrorFiltering, isQueryWithParser } from './query_utils'; +import { extractHasErrorLabelFromDataFrame, extractLogParserFromDataFrame } from './responseUtils'; export function getQueryHints(query: string, series: DataFrame[]): QueryHint[] { - const hints: QueryHint[] = []; - if (series.length > 0) { - const { hasLogfmt, hasJSON } = extractLogParserFromDataFrame(series[0]); - const queryWithParser = isQueryWithParser(query); + if (series.length === 0) { + return []; + } - if (hasJSON && !queryWithParser) { + const hints: QueryHint[] = []; + const { queryWithParser, parserCount } = isQueryWithParser(query); + + if (!queryWithParser) { + const { hasLogfmt, hasJSON } = extractLogParserFromDataFrame(series[0]); + if (hasJSON) { hints.push({ type: 'ADD_JSON_PARSER', label: 'Selected log stream selector has JSON formatted logs.', @@ -23,12 +27,12 @@ export function getQueryHints(query: string, series: DataFrame[]): QueryHint[] { }); } - if (hasLogfmt && !queryWithParser) { + if (hasLogfmt) { hints.push({ type: 'ADD_LOGFMT_PARSER', label: 'Selected log stream selector has logfmt formatted logs.', fix: { - label: 'Consider using logfmt parser.', + label: 'Consider using logfmt parser to turn key-value pairs in your log lines to labels.', action: { type: 'ADD_LOGFMT_PARSER', query, @@ -38,5 +42,26 @@ export function getQueryHints(query: string, series: DataFrame[]): QueryHint[] { } } + if (queryWithParser) { + // To keep this simple, we consider pipeline error filtering hint only is query has up to 1 parser + if (parserCount === 1) { + const hasPipelineErrorFiltering = isQueryPipelineErrorFiltering(query); + const hasError = extractHasErrorLabelFromDataFrame(series[0]); + if (hasError && !hasPipelineErrorFiltering) { + hints.push({ + type: 'ADD_NO_PIPELINE_ERROR', + label: 'Some logs in your selected log streams have parsing error.', + fix: { + label: 'Consider filtering out logs with parsing errors.', + action: { + type: 'ADD_NO_PIPELINE_ERROR', + query, + }, + }, + }); + } + } + } + return hints; } diff --git a/public/app/plugins/datasource/loki/query_utils.test.ts b/public/app/plugins/datasource/loki/query_utils.test.ts index 7d46779fd5e..34c8425b1dd 100644 --- a/public/app/plugins/datasource/loki/query_utils.test.ts +++ b/public/app/plugins/datasource/loki/query_utils.test.ts @@ -137,17 +137,23 @@ describe('isLogsQuery', () => { describe('isQueryWithParser', () => { it('returns false if query without parser', () => { - expect(isQueryWithParser('rate({job="grafana" |= "error" }[5m])')).toBe(false); + expect(isQueryWithParser('rate({job="grafana" |= "error" }[5m])')).toEqual({ + parserCount: 0, + queryWithParser: false, + }); }); it('returns true if log query with parser', () => { - expect(isQueryWithParser('{job="grafana"} | json')).toBe(true); + expect(isQueryWithParser('{job="grafana"} | json')).toEqual({ parserCount: 1, queryWithParser: true }); }); it('returns true if metric query with parser', () => { - expect(isQueryWithParser('rate({job="grafana"} | json [5m])')).toBe(true); + expect(isQueryWithParser('rate({job="grafana"} | json [5m])')).toEqual({ parserCount: 1, queryWithParser: true }); }); it('returns true if query with json parser with expressions', () => { - expect(isQueryWithParser('rate({job="grafana"} | json foo="bar", bar="baz" [5m])')).toBe(true); + expect(isQueryWithParser('rate({job="grafana"} | json foo="bar", bar="baz" [5m])')).toEqual({ + parserCount: 1, + queryWithParser: true, + }); }); }); diff --git a/public/app/plugins/datasource/loki/query_utils.ts b/public/app/plugins/datasource/loki/query_utils.ts index c0972af4cb8..a501b729042 100644 --- a/public/app/plugins/datasource/loki/query_utils.ts +++ b/public/app/plugins/datasource/loki/query_utils.ts @@ -121,15 +121,35 @@ export function isLogsQuery(query: string): boolean { return isLogsQuery; } -export function isQueryWithParser(query: string): boolean { - let hasParser = false; +export function isQueryWithParser(query: string): { queryWithParser: boolean; parserCount: number } { + let parserCount = 0; const tree = parser.parse(query); tree.iterate({ enter: (type): false | void => { - if (type.name === 'LabelParser' || type.name === 'JsonExpression') { - hasParser = true; + if (type.name === 'LabelParser' || type.name === 'JsonExpressionParser') { + parserCount++; } }, }); - return hasParser; + return { queryWithParser: parserCount > 0, parserCount }; +} + +export function isQueryPipelineErrorFiltering(query: string): boolean { + let isQueryPipelineErrorFiltering = false; + const tree = parser.parse(query); + tree.iterate({ + enter: (type, from, to, get): false | void => { + if (type.name === 'LabelFilter') { + const label = get().getChild('Matcher')?.getChild('Identifier'); + if (label) { + const labelName = query.substring(label.from, label.to); + if (labelName === '__error__') { + isQueryPipelineErrorFiltering = true; + } + } + } + }, + }); + + return isQueryPipelineErrorFiltering; } diff --git a/public/app/plugins/datasource/loki/responseUtils.ts b/public/app/plugins/datasource/loki/responseUtils.ts index e0eb15665af..fc2b7b904e0 100644 --- a/public/app/plugins/datasource/loki/responseUtils.ts +++ b/public/app/plugins/datasource/loki/responseUtils.ts @@ -27,3 +27,13 @@ export function extractLogParserFromDataFrame(frame: DataFrame): { hasLogfmt: bo return { hasLogfmt, hasJSON }; } + +export function extractHasErrorLabelFromDataFrame(frame: DataFrame): boolean { + const labelField = frame.fields.find((field) => field.name === 'labels' && field.type === FieldType.other); + if (labelField == null) { + return false; + } + + const labels: Array<{ [key: string]: string }> = labelField.values.toArray(); + return labels.some((label) => label['__error__']); +}