Loki: Fix trailing comma in renderers for json, logfmt, keep and drop operations (#78594)

* Loki: Fix renderers for json, logfmt, keep and drop pipe operations

* Make spacing and trimming more consistent

* Fix

public/app/plugins/datasource/loki/querybuilder/operationUtils.test.ts

@@ -214,7 +214,7 @@ describe('pipelineRenderer', () => {
     definitions = getOperationDefinitions();
   });
 
-  it('Correctly renders unpack expressions', () => {
+  it('correctly renders unpack expressions', () => {
     const model: QueryBuilderOperation = {
       id: LokiOperationId.Unpack,
       params: [],
@@ -222,4 +222,103 @@ describe('pipelineRenderer', () => {
     const definition = definitions.find((def) => def.id === LokiOperationId.Unpack);
     expect(pipelineRenderer(model, definition!, '{}')).toBe('{} | unpack');
   });
+
+  it('correctly renders unpack expressions', () => {
+    const model: QueryBuilderOperation = {
+      id: LokiOperationId.Unpack,
+      params: [],
+    };
+    const definition = definitions.find((def) => def.id === LokiOperationId.Unpack);
+    expect(pipelineRenderer(model, definition!, '{}')).toBe('{} | unpack');
+  });
+
+  it('correctly renders empty logfmt expression', () => {
+    const model: QueryBuilderOperation = {
+      id: LokiOperationId.Logfmt,
+      params: [],
+    };
+    const definition = definitions.find((def) => def.id === LokiOperationId.Logfmt);
+    expect(pipelineRenderer(model, definition!, '{}')).toBe('{} | logfmt');
+  });
+
+  it('correctly renders logfmt expression', () => {
+    const model: QueryBuilderOperation = {
+      id: LokiOperationId.Logfmt,
+      params: [true, false, 'foo', ''],
+    };
+    const definition = definitions.find((def) => def.id === LokiOperationId.Logfmt);
+    expect(pipelineRenderer(model, definition!, '{}')).toBe('{} | logfmt --strict foo');
+  });
+
+  it('correctly renders logfmt expression with multiple params', () => {
+    const model: QueryBuilderOperation = {
+      id: LokiOperationId.Logfmt,
+      params: [true, false, 'foo', 'bar', 'baz'],
+    };
+    const definition = definitions.find((def) => def.id === LokiOperationId.Logfmt);
+    expect(pipelineRenderer(model, definition!, '{}')).toBe('{} | logfmt --strict foo, bar, baz');
+  });
+
+  it('correctly renders empty json expression', () => {
+    const model: QueryBuilderOperation = {
+      id: LokiOperationId.Json,
+      params: [],
+    };
+    const definition = definitions.find((def) => def.id === LokiOperationId.Json);
+    expect(pipelineRenderer(model, definition!, '{}')).toBe('{} | json');
+  });
+
+  it('correctly renders json expression', () => {
+    const model: QueryBuilderOperation = {
+      id: LokiOperationId.Json,
+      params: ['foo', ''],
+    };
+    const definition = definitions.find((def) => def.id === LokiOperationId.Json);
+    expect(pipelineRenderer(model, definition!, '{}')).toBe('{} | json foo');
+  });
+
+  it('correctly renders json expression with multiple params', () => {
+    const model: QueryBuilderOperation = {
+      id: LokiOperationId.Json,
+      params: ['foo', 'bar', 'baz'],
+    };
+    const definition = definitions.find((def) => def.id === LokiOperationId.Json);
+    expect(pipelineRenderer(model, definition!, '{}')).toBe('{} | json foo, bar, baz');
+  });
+
+  it('correctly renders keep expression', () => {
+    const model: QueryBuilderOperation = {
+      id: LokiOperationId.Keep,
+      params: ['foo', ''],
+    };
+    const definition = definitions.find((def) => def.id === LokiOperationId.Keep);
+    expect(pipelineRenderer(model, definition!, '{}')).toBe('{} | keep foo');
+  });
+
+  it('correctly renders keep expression with multiple params', () => {
+    const model: QueryBuilderOperation = {
+      id: LokiOperationId.Keep,
+      params: ['foo', 'bar', 'baz'],
+    };
+    const definition = definitions.find((def) => def.id === LokiOperationId.Keep);
+    expect(pipelineRenderer(model, definition!, '{}')).toBe('{} | keep foo, bar, baz');
+  });
+
+  it('correctly renders drop expression', () => {
+    const model: QueryBuilderOperation = {
+      id: LokiOperationId.Drop,
+      params: ['foo', ''],
+    };
+    const definition = definitions.find((def) => def.id === LokiOperationId.Drop);
+    expect(pipelineRenderer(model, definition!, '{}')).toBe('{} | drop foo');
+  });
+
+  it('correctly renders drop expression with multiple params', () => {
+    const model: QueryBuilderOperation = {
+      id: LokiOperationId.Drop,
+      params: ['foo', 'bar', 'baz'],
+    };
+    const definition = definitions.find((def) => def.id === LokiOperationId.Drop);
+    expect(pipelineRenderer(model, definition!, '{}')).toBe('{} | drop foo, bar, baz');
+  });
 });

public/app/plugins/datasource/loki/querybuilder/operationUtils.ts

@@ -187,9 +187,15 @@ export function pipelineRenderer(model: QueryBuilderOperation, def: QueryBuilder
   switch (model.id) {
     case LokiOperationId.Logfmt:
       const [strict = false, keepEmpty = false, ...labels] = model.params;
-      return `${innerExpr} | logfmt${strict ? ' --strict' : ''}${keepEmpty ? ' --keep-empty' : ''} ${labels.join(
-        ', '
-      )}`.trim();
+      return `${innerExpr} | logfmt${strict ? ' --strict' : ''}${keepEmpty ? ' --keep-empty' : ''} ${labels
+        .filter((label) => label)
+        .join(', ')}`.trim();
+    case LokiOperationId.Json:
+      return `${innerExpr} | json ${model.params.filter((param) => param).join(', ')}`.trim();
+    case LokiOperationId.Drop:
+      return `${innerExpr} | drop ${model.params.filter((param) => param).join(', ')}`.trim();
+    case LokiOperationId.Keep:
+      return `${innerExpr} | keep ${model.params.filter((param) => param).join(', ')}`.trim();
     default:
       return `${innerExpr} | ${model.id}`;
   }

public/app/plugins/datasource/loki/querybuilder/operations.ts

@@ -92,7 +92,7 @@ export function getOperationDefinitions(): QueryBuilderOperationDef[] {
       alternativesKey: 'format',
       category: LokiVisualQueryOperationCategory.Formats,
       orderRank: LokiOperationOrder.Parsers,
-      renderer: (model, def, innerExpr) => `${innerExpr} | json ${model.params.join(', ')}`.trim(),
+      renderer: pipelineRenderer,
       addOperationHandler: addLokiOperation,
       explainHandler: () =>
         `This will extract keys and values from a [json](https://grafana.com/docs/loki/latest/logql/log_queries/#json) formatted log line as labels. The extracted labels can be used in label filter expressions and used as values for a range aggregation via the unwrap operation.`,
@@ -531,7 +531,7 @@ Example: \`\`error_level=\`level\` \`\`
       alternativesKey: 'format',
       category: LokiVisualQueryOperationCategory.Formats,
       orderRank: LokiOperationOrder.PipeOperations,
-      renderer: (op, def, innerExpr) => `${innerExpr} | drop ${op.params.join(',')}`,
+      renderer: pipelineRenderer,
       addOperationHandler: addLokiOperation,
       explainHandler: () => 'The drop expression will drop the given labels in the pipeline.',
     },
@@ -555,7 +555,7 @@ Example: \`\`error_level=\`level\` \`\`
       alternativesKey: 'format',
       category: LokiVisualQueryOperationCategory.Formats,
       orderRank: LokiOperationOrder.PipeOperations,
-      renderer: (op, def, innerExpr) => `${innerExpr} | keep ${op.params.join(',')}`,
+      renderer: pipelineRenderer,
       addOperationHandler: addLokiOperation,
       explainHandler: () =>
         'The keep expression will keep only the specified labels in the pipeline and drop all the other labels.',