From ed88401a5891ea1830794c16c1bb45d27b9833ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Farkas?= Date: Fri, 13 Jan 2023 15:33:18 +0100 Subject: [PATCH] logs: json/logfmt-detection, simplify code (#61492) * logs: json/logfmt: simplify code * remove obsolete comment Co-authored-by: Sven Grossmann Co-authored-by: Sven Grossmann --- public/app/features/logs/utils.test.ts | 42 ----------------- public/app/features/logs/utils.ts | 47 ------------------- .../datasource/loki/lineParser.test.ts | 37 +++++++++++++++ .../app/plugins/datasource/loki/lineParser.ts | 18 +++++++ .../plugins/datasource/loki/responseUtils.ts | 7 ++- 5 files changed, 58 insertions(+), 93 deletions(-) create mode 100644 public/app/plugins/datasource/loki/lineParser.test.ts create mode 100644 public/app/plugins/datasource/loki/lineParser.ts diff --git a/public/app/features/logs/utils.test.ts b/public/app/features/logs/utils.test.ts index 655d8a01154..b6e3e9f290f 100644 --- a/public/app/features/logs/utils.test.ts +++ b/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, diff --git a/public/app/features/logs/utils.ts b/public/app/features/logs/utils.ts index 538a69c4d6f..298003d419c 100644 --- a/public/app/features/logs/utils.ts +++ b/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) { diff --git a/public/app/plugins/datasource/loki/lineParser.test.ts b/public/app/plugins/datasource/loki/lineParser.test.ts new file mode 100644 index 00000000000..e47b9ab48a8 --- /dev/null +++ b/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); + }); +}); diff --git a/public/app/plugins/datasource/loki/lineParser.ts b/public/app/plugins/datasource/loki/lineParser.ts new file mode 100644 index 00000000000..15510e95354 --- /dev/null +++ b/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); +} diff --git a/public/app/plugins/datasource/loki/responseUtils.ts b/public/app/plugins/datasource/loki/responseUtils.ts index ef9c5902644..3a8c26281ec 100644 --- a/public/app/plugins/datasource/loki/responseUtils.ts +++ b/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 (parser === LogsParsers.JSON) { + if (isLogLineJSON(line)) { hasJSON = true; } - if (parser === LogsParsers.logfmt) { + if (isLogLineLogfmt(line)) { hasLogfmt = true; } });