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

@@ -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"],

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;
       }

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"');
+  });
+});

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;
     }

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;
 }

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,
+    });
   });
 });

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;
 }

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__']);
+}