logs: json/logfmt-detection, simplify code (#61492)

* logs: json/logfmt: simplify code

* remove obsolete comment

Co-authored-by: Sven Grossmann <sven.grossmann@grafana.com>

Co-authored-by: Sven Grossmann <sven.grossmann@grafana.com>

public/app/features/logs/utils.test.ts

@@ -3,8 +3,6 @@ import { Labels, LogLevel, LogsModel, LogRowModel, LogsSortOrder, MutableDataFra
 import {
   getLogLevel,
   calculateLogsLabelStats,
-  getParser,
-  LogsParsers,
   calculateStats,
   getLogLevelFromKey,
   sortLogsResult,
@@ -106,27 +104,6 @@ describe('calculateLogsLabelStats()', () => {
   });
 });
 
-describe('LogsParsers', () => {
-  describe('logfmt', () => {
-    const parser = LogsParsers.logfmt;
-
-    test('should detect format', () => {
-      expect(parser.test('foo')).toBeFalsy();
-      expect(parser.test('foo=bar')).toBeTruthy();
-    });
-  });
-
-  describe('JSON', () => {
-    const parser = LogsParsers.JSON;
-
-    test('should detect format', () => {
-      expect(parser.test('foo')).toBeFalsy();
-      expect(parser.test('"foo"')).toBeFalsy();
-      expect(parser.test('{"foo":"bar"}')).toBeTruthy();
-    });
-  });
-});
-
 describe('calculateStats()', () => {
   test('should return no stats for empty array', () => {
     expect(calculateStats([])).toEqual([]);
@@ -149,25 +126,6 @@ describe('calculateStats()', () => {
   });
 });
 
-describe('getParser()', () => {
-  test('should return no parser on empty line', () => {
-    expect(getParser('')).toBeUndefined();
-  });
-
-  test('should return no parser on unknown line pattern', () => {
-    expect(getParser('To Be or not to be')).toBeUndefined();
-  });
-
-  test('should return logfmt parser on key value patterns', () => {
-    expect(getParser('foo=bar baz="41 + 1')).toEqual(LogsParsers.logfmt);
-  });
-
-  test('should return JSON parser on JSON log lines', () => {
-    // TODO implement other JSON value types than string
-    expect(getParser('{"foo": "bar", "baz": "41 + 1"}')).toEqual(LogsParsers.JSON);
-  });
-});
-
 describe('sortLogsResult', () => {
   const firstRow: LogRowModel = {
     rowIndex: 0,

public/app/features/logs/utils.ts

@@ -2,12 +2,6 @@ import { countBy, chain } from 'lodash';
 
 import { LogLevel, LogRowModel, LogLabelStatsModel, LogsModel, LogsSortOrder } from '@grafana/data';
 
-// This matches:
-// first a label from start of the string or first white space, then any word chars until "="
-// second either an empty quotes, or anything that starts with quote and ends with unescaped quote,
-// or any non whitespace chars that do not start with quote
-const LOGFMT_REGEXP = /(?:^|\s)([\w\(\)\[\]\{\}]+)=(""|(?:".*?[^\\]"|[^"\s]\S*))/;
-
 /**
  * Returns the log level of a log line.
  * Parse the line for level words. If no level is found, it returns `LogLevel.unknown`.
@@ -44,32 +38,6 @@ export function getLogLevelFromKey(key: string | number): LogLevel {
   return LogLevel.unknown;
 }
 
-interface LogsParser {
-  /**
-   * Function to verify if this is a valid parser for the given line.
-   * The parser accepts the line if it returns true.
-   */
-  test: (line: string) => boolean;
-}
-
-export const LogsParsers: { [name: string]: LogsParser } = {
-  JSON: {
-    test: (line) => {
-      let parsed;
-      try {
-        parsed = JSON.parse(line);
-      } catch (error) {}
-      // The JSON parser should only be used for log lines that are valid serialized JSON objects.
-      // If it would be used for a string, detected fields would include each letter as a separate field.
-      return typeof parsed === 'object';
-    },
-  },
-
-  logfmt: {
-    test: (line) => LOGFMT_REGEXP.test(line),
-  },
-};
-
 export function calculateLogsLabelStats(rows: LogRowModel[], label: string): LogLabelStatsModel[] {
   // Consider only rows that have the given label
   const rowsWithLabel = rows.filter((row) => row.labels[label] !== undefined);
@@ -94,21 +62,6 @@ const getSortedCounts = (countsByValue: { [value: string]: number }, rowCount: n
     .value();
 };
 
-export function getParser(line: string): LogsParser | undefined {
-  let parser;
-  try {
-    if (LogsParsers.JSON.test(line)) {
-      parser = LogsParsers.JSON;
-    }
-  } catch (error) {}
-
-  if (!parser && LogsParsers.logfmt.test(line)) {
-    parser = LogsParsers.logfmt;
-  }
-
-  return parser;
-}
-
 export const sortInAscendingOrder = (a: LogRowModel, b: LogRowModel) => {
   // compare milliseconds
   if (a.timeEpochMs < b.timeEpochMs) {

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

@@ -0,0 +1,37 @@
+import { isLogLineJSON, isLogLineLogfmt } from './lineParser';
+
+describe('isLogLineJSON', () => {
+  test('should return false on empty line', () => {
+    expect(isLogLineJSON('')).toBe(false);
+  });
+
+  test('should return false on unknown line pattern', () => {
+    expect(isLogLineJSON('To Be or not to be')).toBe(false);
+  });
+
+  test('should return false on key value patterns', () => {
+    expect(isLogLineJSON('foo=bar baz="41 + 1')).toBe(false);
+  });
+
+  test('should return true on JSON log lines', () => {
+    expect(isLogLineJSON('{"foo": "bar", "baz": "41 + 1"}')).toBe(true);
+  });
+});
+
+describe('isLogLineLogfmt', () => {
+  test('should return false on empty line', () => {
+    expect(isLogLineLogfmt('')).toBe(false);
+  });
+
+  test('should return false on unknown line pattern', () => {
+    expect(isLogLineLogfmt('To Be or not to be')).toBe(false);
+  });
+
+  test('should return true on key value patterns', () => {
+    expect(isLogLineLogfmt('foo=bar baz="41 + 1')).toBe(true);
+  });
+
+  test('should return false on JSON log lines', () => {
+    expect(isLogLineLogfmt('{"foo": "bar", "baz": "41 + 1"}')).toBe(false);
+  });
+});

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

@@ -0,0 +1,18 @@
+export function isLogLineJSON(line: string): boolean {
+  let parsed;
+  try {
+    parsed = JSON.parse(line);
+  } catch (error) {}
+  // The JSON parser should only be used for log lines that are valid serialized JSON objects.
+  return typeof parsed === 'object';
+}
+
+// This matches:
+// first a label from start of the string or first white space, then any word chars until "="
+// second either an empty quotes, or anything that starts with quote and ends with unescaped quote,
+// or any non whitespace chars that do not start with quote
+const LOGFMT_REGEXP = /(?:^|\s)([\w\(\)\[\]\{\}]+)=(""|(?:".*?[^\\]"|[^"\s]\S*))/;
+
+export function isLogLineLogfmt(line: string): boolean {
+  return LOGFMT_REGEXP.test(line);
+}

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

@@ -1,6 +1,6 @@
 import { DataFrame, FieldType, Labels } from '@grafana/data';
 
-import { getParser, LogsParsers } from '../../../features/logs/utils';
+import { isLogLineJSON, isLogLineLogfmt } from './lineParser';
 
 export function dataFrameHasLokiError(frame: DataFrame): boolean {
   const labelSets: Labels[] = frame.fields.find((f) => f.name === 'labels')?.values.toArray() ?? [];
@@ -24,11 +24,10 @@ export function extractLogParserFromDataFrame(frame: DataFrame): { hasLogfmt: bo
   let hasLogfmt = false;
 
   logLines.forEach((line) => {
-    const parser = getParser(line);
+    if (isLogLineJSON(line)) {
-    if (parser === LogsParsers.JSON) {
       hasJSON = true;
     }
-    if (parser === LogsParsers.logfmt) {
+    if (isLogLineLogfmt(line)) {
       hasLogfmt = true;
     }
   });