Link extensions: fix false positive introduced by field filters (#1000)

* fix: fix bug where Matcher was grabbing field filters from outside the label selector --------- Co-authored-by: Matias Chomicki <[email protected]>
grafana · Jan 16, 2025 · b96af89 · b96af89
1 parent 218ca99
commit b96af89
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 10 deletions.
diff --git a/src/services/extensions/links.test.ts b/src/services/extensions/links.test.ts
@@ -145,6 +145,29 @@ describe('contextToLink', () => {
     // &var-lineFilters=caseSensitive,0|__gfp__=|\n
     const expectedLineFiltersUrlString = '&var-lineFilters=caseSensitive%2C0%7C__gfp__%3D%7C%5Cn';
 
+    expect(config).toEqual({
+      path: `/a/grafana-lokiexplore-app/explore/cluster/eu-west-1/logs?var-ds=123abc&from=1675828800000&to=1675854000000${expectedLabelFiltersUrlString}${expectedLineFiltersUrlString}`,
+    });
+  });
+  it('should return undefined when no non-regex include filters are present', () => {
+    const target = getTestTarget({
+      expr: `sort_desc(sum by (error) (count_over_time({service_name=~"grafana/.*", cluster=~"prod-eu-west-2"} | logfmt | level="error" | logger=~".*grafana-datasource.*|.*coreplugin" | statusSource!="downstream" | error!="" |~"Partial data response error|Plugin Request Completed" | endpoint="queryData" [$__auto])))`,
+    });
+    const config = getTestConfig(linkConfigs, target);
+    expect(config).toEqual(undefined);
+  });
+  it('should not confuse field filters with indexed label filters', () => {
+    const target = getTestTarget({
+      expr: `sort_desc(sum by (error) (count_over_time({service_name=~"grafana/.*", cluster="eu-west-1"} | logfmt | level="error" | logger=~".*grafana-datasource.*|.*coreplugin" | statusSource!="downstream" | error!="" |~"Partial data response error|Plugin Request Completed" | endpoint="queryData" [$__auto])))`,
+    });
+    const config = getTestConfig(linkConfigs, target);
+
+    const expectedLabelFiltersUrlString = '&var-filters=cluster%7C%3D%7Ceu-west-1';
+
+    // var-lineFilters=caseSensitive,0|__gfp__~|Partial data response error__gfp__Plugin Request Completed
+    const expectedLineFiltersUrlString =
+      '&var-lineFilters=caseSensitive%2C0%7C__gfp__%7E%7CPartial+data+response+error__gfp__Plugin+Request+Completed';
+
     expect(config).toEqual({
       path: `/a/grafana-lokiexplore-app/explore/cluster/eu-west-1/logs?var-ds=123abc&from=1675828800000&to=1675854000000${expectedLabelFiltersUrlString}${expectedLineFiltersUrlString}`,
     });

diff --git a/src/services/logqlMatchers.ts b/src/services/logqlMatchers.ts
@@ -79,28 +79,25 @@ function getAllPositionsInNodeByType(node: SyntaxNode, type: number): NodePositi
   return positions;
 }
 
-function parseLabelFilters(selector: SyntaxNode[], query: string, filter: Filter[]) {
-  const selectorPosition = NodePosition.fromNode(selector[0]);
-
+function parseLabelFilters(query: string, filter: Filter[]) {
+  // `Matcher` will select field filters as well as indexed label filters
   const allMatcher = getNodesFromQuery(query, [Matcher]);
   for (const matcher of allMatcher) {
-    const matcherPosition = NodePosition.fromNode(matcher);
     const identifierPosition = getAllPositionsInNodeByType(matcher, Identifier);
     const valuePosition = getAllPositionsInNodeByType(matcher, String);
-    const operation = query.substring(identifierPosition[0].to, valuePosition[0].from);
-    const op = operation === '=' ? FilterOperator.Equal : FilterOperator.NotEqual;
+    const operator = query.substring(identifierPosition[0].to, valuePosition[0].from);
     const key = identifierPosition[0].getExpression(query);
     const value = valuePosition.map((position) => query.substring(position.from + 1, position.to - 1))[0];
 
-    if (!key || !value) {
+    if (!key || !value || (operator !== FilterOperator.NotEqual && operator !== FilterOperator.Equal)) {
       continue;
     }
 
     filter.push({
       key,
-      operator: op,
+      operator,
       value,
-      type: selectorPosition.contains(matcherPosition) ? LabelType.Indexed : undefined,
+      type: LabelType.Indexed,
     });
   }
 }
@@ -171,11 +168,15 @@ export function getMatcherFromQuery(query: string): { labelFilters: Filter[]; li
   const filter: Filter[] = [];
   const lineFilters: LineFilterType[] = [];
   const selector = getNodesFromQuery(query, [Selector]);
+
   if (selector.length === 0) {
     return { labelFilters: filter };
   }
 
-  parseLabelFilters(selector, query, filter);
+  // Get the stream selector portion of the query
+  const selectorQuery = getAllPositionsInNodeByType(selector[0], Selector)[0].getExpression(query);
+
+  parseLabelFilters(selectorQuery, filter);
   parseLineFilters(query, lineFilters);
 
   return { labelFilters: filter, lineFilters };