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

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

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

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

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

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

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

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 = <T extends PromLokiVisualQuery>({
               <Tooltip content={`${hint.label} ${hint.fix?.label}`} key={hint.type}>
                 <Button
                   onClick={() => {
+                    reportInteraction('grafana_query_builder_hints_clicked', {
+                      hint: hint.type,
+                      datasourceType: datasource.type,
+                    });
+
                     if (hint?.fix?.action) {
                       const query = { expr: queryModeller.renderQuery(visualQuery), refId: '' };
                       const newQuery = datasource.modifyQuery(query, hint.fix.action);