Loki: Fix adding of multiple label filters when parser (#52335)

* Loki: Fix adding multiple filters

* Update logic in sorting

public/app/plugins/datasource/loki/addToQuery.ts

@@ -27,15 +27,18 @@ export function addLabelToQuery(query: string, key: string, operator: string, va
 
   const streamSelectorPositions = getStreamSelectorPositions(query);
   const parserPositions = getParserPositions(query);
+  const labelFilterPositions = getLabelFilterPositions(query);
   if (!streamSelectorPositions.length) {
     return query;
   }
 
   const filter = toLabelFilter(key, value, operator);
-  if (!parserPositions.length) {
+  // If we have label filters or parser, we want to add new label filter after the last one
-    return addFilterToStreamSelector(query, streamSelectorPositions, filter);
+  if (labelFilterPositions.length || parserPositions.length) {
+    const positionToAdd = findLastPosition([...labelFilterPositions, ...parserPositions]);
+    return addFilterAsLabelFilter(query, [positionToAdd], filter);
   } else {
-    return addFilterAsLabelFilter(query, parserPositions, filter);
+    return addFilterToStreamSelector(query, streamSelectorPositions, filter);
   }
 }
 
@@ -110,6 +113,24 @@ export function getParserPositions(query: string): Position[] {
   return positions;
 }
 
+/**
+ * Parse the string and get all LabelFilter positions in the query.
+ * @param query
+ */
+export function getLabelFilterPositions(query: string): Position[] {
+  const tree = parser.parse(query);
+  const positions: Position[] = [];
+  tree.iterate({
+    enter: (type, from, to, get): false | void => {
+      if (type.name === 'LabelFilter') {
+        positions.push({ from, to });
+        return false;
+      }
+    },
+  });
+  return positions;
+}
+
 /**
  * Parse the string and get all Line filter positions in the query.
  * @param query
@@ -172,17 +193,21 @@ function addFilterToStreamSelector(
 /**
  * Add filter as label filter after the parsers
  * @param query
- * @param parserPositions
+ * @param positionsToAddAfter
  * @param filter
  */
-function addFilterAsLabelFilter(query: string, parserPositions: Position[], filter: QueryBuilderLabelFilter): string {
+function addFilterAsLabelFilter(
+  query: string,
+  positionsToAddAfter: Position[],
+  filter: QueryBuilderLabelFilter
+): string {
   let newQuery = '';
   let prev = 0;
 
-  for (let i = 0; i < parserPositions.length; i++) {
+  for (let i = 0; i < positionsToAddAfter.length; i++) {
     // This is basically just doing splice on a string for each matched vector selector.
-    const match = parserPositions[i];
+    const match = positionsToAddAfter[i];
-    const isLast = i === parserPositions.length - 1;
+    const isLast = i === positionsToAddAfter.length - 1;
 
     const start = query.substring(prev, match.to);
     const end = isLast ? query.substring(match.to) : '';
@@ -227,3 +252,11 @@ function addParser(query: string, queryPartPositions: Position[], parser: string
 function labelExists(labels: QueryBuilderLabelFilter[], filter: QueryBuilderLabelFilter) {
   return labels.find((label) => label.label === filter.label && label.value === filter.value);
 }
+
+/**
+ * Return the last position based on "to" property
+ * @param positions
+ */
+function findLastPosition(positions: Position[]): Position {
+  return positions.reduce((prev, current) => (prev.to > current.to ? prev : current));
+}

public/app/plugins/datasource/loki/addtoQuery.test.ts

@@ -115,14 +115,14 @@ describe('addLabelToQuery()', () => {
     it('should add label filter after parser', () => {
       expect(addLabelToQuery('{foo="bar"} | logfmt', 'bar', '=', 'baz')).toBe('{foo="bar"} | logfmt | bar=`baz`');
     });
-    it('should add label filter after multiple parsers', () => {
+    it('should add label filter after last parser when multiple parsers', () => {
       expect(addLabelToQuery('{foo="bar"} | logfmt | json', 'bar', '=', 'baz')).toBe(
-        '{foo="bar"} | logfmt | bar=`baz` | json | bar=`baz`'
+        '{foo="bar"} | logfmt | json | bar=`baz`'
       );
     });
-    it('should add label filter after parser when multiple label filters', () => {
+    it('should add label filter after last label filter when multiple label filters', () => {
       expect(addLabelToQuery('{foo="bar"} | logfmt | x="y"', 'bar', '=', 'baz')).toBe(
-        '{foo="bar"} | logfmt | bar=`baz` | x="y"'
+        '{foo="bar"} | logfmt | x="y" | bar=`baz`'
       );
     });
     it('should add label filter in metric query', () => {
@@ -138,7 +138,7 @@ describe('addLabelToQuery()', () => {
           '=',
           'baz'
         )
-      ).toBe('sum by(host) (rate({foo="bar"} | logfmt | bar=`baz` | x="y" | line_format "{{.status}}" [5m]))');
+      ).toBe('sum by(host) (rate({foo="bar"} | logfmt | x="y" | bar=`baz` | line_format "{{.status}}" [5m]))');
     });
     it('should not add adhoc filter to line_format expressions', () => {
       expect(addLabelToQuery('{foo="bar"} | logfmt | line_format "{{.status}}"', 'bar', '=', 'baz')).toBe(

public/app/plugins/datasource/prometheus/querybuilder/shared/OperationList.tsx

@@ -92,7 +92,7 @@ export function OperationList<T extends QueryWithOperations>({
                 <div className={styles.operationList} ref={provided.innerRef} {...provided.droppableProps}>
                   {operations.map((op, index) => (
                     <OperationEditor
-                      key={op.id + index}
+                      key={op.id + JSON.stringify(op.params) + index}
                       queryModeller={queryModeller}
                       index={index}
                       operation={op}