From 0da199324a67ff8db86fa5633d7b9d9a1f94012b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Farkas?= Date: Wed, 26 Jul 2023 10:56:26 +0200 Subject: [PATCH] logs: log-details: handle dataplane-compliant dataframes (#71935) * logs: log-details: handle dataplane-compliant dataframes * lint fix, removed unused import --- public/app/features/explore/Logs/Logs.tsx | 1 - .../features/explore/Logs/LogsTable.test.tsx | 34 +-- .../app/features/explore/Logs/LogsTable.tsx | 29 ++- .../logs/components/logParser.test.ts | 207 +++++++++++++++++- .../app/features/logs/components/logParser.ts | 134 ++++++------ 5 files changed, 292 insertions(+), 113 deletions(-) diff --git a/public/app/features/explore/Logs/Logs.tsx b/public/app/features/explore/Logs/Logs.tsx index aa461951f34..72190ecebdb 100644 --- a/public/app/features/explore/Logs/Logs.tsx +++ b/public/app/features/explore/Logs/Logs.tsx @@ -667,7 +667,6 @@ class UnthemedLogs extends PureComponent {
{/* Width should be full width minus logsnavigation and padding */} { }; return ( undefined} timeZone={'utc'} @@ -140,26 +131,3 @@ describe('LogsTable', () => { }); }); }); - -const makeLog = (overrides: Partial): LogRowModel => { - const uid = overrides.uid || '1'; - const entry = `log message ${uid}`; - return { - uid, - entryFieldIndex: 0, - rowIndex: 0, - dataFrame: new MutableDataFrame(), - logLevel: LogLevel.debug, - entry, - hasAnsi: false, - hasUnescapedContent: false, - labels: {}, - raw: entry, - timeFromNow: '', - timeEpochMs: 1, - timeEpochNs: '1000000', - timeLocal: '', - timeUtc: '', - ...overrides, - }; -}; diff --git a/public/app/features/explore/Logs/LogsTable.tsx b/public/app/features/explore/Logs/LogsTable.tsx index 96f3f1c36f9..f4a7c75d828 100644 --- a/public/app/features/explore/Logs/LogsTable.tsx +++ b/public/app/features/explore/Logs/LogsTable.tsx @@ -6,7 +6,6 @@ import { applyFieldOverrides, DataFrame, Field, - LogRowModel, LogsSortOrder, sortDataFrame, SplitOpen, @@ -16,7 +15,7 @@ import { } from '@grafana/data'; import { config } from '@grafana/runtime'; import { Table } from '@grafana/ui'; -import { shouldRemoveField } from 'app/features/logs/components/logParser'; +import { separateVisibleFields } from 'app/features/logs/components/logParser'; import { parseLogsFrame } from 'app/features/logs/logsFrame'; import { getFieldLinksForExplore } from '../utils/links'; @@ -28,7 +27,6 @@ interface Props { splitOpen: SplitOpen; range: TimeRange; logsSortOrder: LogsSortOrder; - rows: LogRowModel[]; } const getTableHeight = memoizeOne((dataFrames: DataFrame[] | undefined) => { @@ -40,7 +38,7 @@ const getTableHeight = memoizeOne((dataFrames: DataFrame[] | undefined) => { }); export const LogsTable: React.FunctionComponent = (props) => { - const { timeZone, splitOpen, range, logsSortOrder, width, logsFrames, rows } = props; + const { timeZone, splitOpen, range, logsSortOrder, width, logsFrames } = props; const [tableFrame, setTableFrame] = useState(undefined); @@ -129,18 +127,17 @@ export const LogsTable: React.FunctionComponent = (props) => { }); // remove fields that should not be displayed - dataFrame.fields.forEach((field: Field, index: number) => { - const row = rows[0]; // we just take the first row as the relevant row - if (shouldRemoveField(field, index, row, false, false)) { - transformations.push({ - id: 'organize', - options: { - excludeByName: { - [field.name]: true, - }, + + const hiddenFields = separateVisibleFields(dataFrame, { keepBody: true, keepTimestamp: true }).hidden; + hiddenFields.forEach((field: Field, index: number) => { + transformations.push({ + id: 'organize', + options: { + excludeByName: { + [field.name]: true, }, - }); - } + }, + }); }); if (transformations.length > 0) { const [transformedDataFrame] = await lastValueFrom(transformDataFrame(transformations, [dataFrame])); @@ -150,7 +147,7 @@ export const LogsTable: React.FunctionComponent = (props) => { } }; prepare(); - }, [prepareTableFrame, logsFrames, logsSortOrder, rows]); + }, [prepareTableFrame, logsFrames, logsSortOrder]); if (!tableFrame) { return null; diff --git a/public/app/features/logs/components/logParser.test.ts b/public/app/features/logs/components/logParser.test.ts index 4219c3dc8cb..27fcbe390f9 100644 --- a/public/app/features/logs/components/logParser.test.ts +++ b/public/app/features/logs/components/logParser.test.ts @@ -1,4 +1,4 @@ -import { FieldType, MutableDataFrame } from '@grafana/data'; +import { DataFrameType, Field, FieldType, LogRowModel, MutableDataFrame } from '@grafana/data'; import { ExploreFieldLinkModel } from 'app/features/explore/utils/links'; import { createLogRow } from './__mocks__/logRow'; @@ -199,6 +199,211 @@ describe('logParser', () => { expect(fields.length).toBe(1); expect(fields.find((field) => field.keys[0] === testStringField.name)).not.toBe(undefined); }); + + describe('dataplane frames', () => { + const makeLogRow = (fields: Field[], entryFieldIndex: number): LogRowModel => + createLogRow({ + entryFieldIndex, + rowIndex: 0, + dataFrame: { + refId: 'A', + fields, + length: fields[0]?.values.length, + meta: { + type: DataFrameType.LogLines, + }, + }, + }); + + const expectHasField = (defs: FieldDef[], name: string): void => { + expect(defs.find((field) => field.keys[0] === name)).not.toBe(undefined); + }; + + it('should filter out fields with data links that have a nullish value', () => { + const createScenario = (value: unknown) => + makeLogRow( + [ + testTimeField, + testLineField, + { + name: 'link', + type: FieldType.string, + config: { + links: [ + { + title: 'link1', + url: 'https://example.com', + }, + ], + }, + values: [value], + }, + ], + 1 + ); + + expect(getAllFields(createScenario(null))).toHaveLength(0); + expect(getAllFields(createScenario(undefined))).toHaveLength(0); + expect(getAllFields(createScenario(''))).toHaveLength(1); + expect(getAllFields(createScenario('test'))).toHaveLength(1); + // technically this is a field-type-string, but i will add more + // falsy-values, just to be sure + expect(getAllFields(createScenario(false))).toHaveLength(1); + expect(getAllFields(createScenario(NaN))).toHaveLength(1); + expect(getAllFields(createScenario(0))).toHaveLength(1); + expect(getAllFields(createScenario(-0))).toHaveLength(1); + }); + + it('should filter out system-fields without data-links, but should keep severity', () => { + const row = makeLogRow( + [ + testTimeField, + testLineField, + { + config: {}, + name: 'id', + type: FieldType.string, + values: ['id1'], + }, + { + config: {}, + name: 'attributes', + type: FieldType.other, + values: [{ a: 1, b: 2 }], + }, + { + config: {}, + name: 'severity', + type: FieldType.string, + values: ['info'], + }, + testStringField, + ], + 1 + ); + + const output = getAllFields(row); + + expect(output).toHaveLength(2); + expectHasField(output, 'test_field_string'); + expectHasField(output, 'severity'); + }); + + it('should keep system fields with data-links', () => { + const links = [ + { + title: 'link1', + url: 'https://example.com', + }, + ]; + + const row = makeLogRow( + [ + { + ...testTimeField, + config: { links }, + }, + { + ...testLineField, + config: { links }, + }, + { + config: { links }, + name: 'id', + type: FieldType.string, + values: ['id1'], + }, + { + config: { links }, + name: 'attributes', + type: FieldType.other, + values: [{ a: 1, b: 2 }], + }, + { + config: { links }, + name: 'severity', + type: FieldType.string, + values: ['info'], + }, + ], + 1 + ); + + const output = getAllFields(row); + + expect(output).toHaveLength(5); + expectHasField(output, 'timestamp'); + expectHasField(output, 'body'); + expectHasField(output, 'id'); + expectHasField(output, 'attributes'); + expectHasField(output, 'severity'); + }); + + it('should filter out config-hidden fields', () => { + const row = makeLogRow( + [ + testTimeField, + testLineField, + { + ...testStringField, + config: { + custom: { + hidden: true, + }, + }, + }, + ], + 1 + ); + + const output = getAllFields(row); + + expect(output).toHaveLength(0); + }); + + it('should filter out fields with null values', () => { + const row = makeLogRow( + [ + testTimeField, + testLineField, + { + // null-value + config: {}, + type: FieldType.string, + name: 'test1', + values: [null], + }, + { + // null-value and data-link + config: { + links: [ + { + title: 'link1', + url: 'https://example.com', + }, + ], + }, + type: FieldType.string, + name: 'test2', + values: [null], + }, + { + // normal value + config: {}, + type: FieldType.string, + name: 'test3', + values: ['testvalue'], + }, + ], + 1 + ); + + const output = getAllFields(row); + + expect(output).toHaveLength(1); + expectHasField(output, 'test3'); + }); + }); }); describe('createLogLineLinks', () => { diff --git a/public/app/features/logs/components/logParser.ts b/public/app/features/logs/components/logParser.ts index 63344030c3c..7e541ca3e53 100644 --- a/public/app/features/logs/components/logParser.ts +++ b/public/app/features/logs/components/logParser.ts @@ -1,9 +1,12 @@ +import { partition } from 'lodash'; import memoizeOne from 'memoize-one'; -import { DataFrame, Field, FieldType, LinkModel, LogRowModel } from '@grafana/data'; +import { DataFrame, Field, FieldWithIndex, LinkModel, LogRowModel } from '@grafana/data'; import { safeStringifyValue } from 'app/core/utils/explore'; import { ExploreFieldLinkModel } from 'app/features/explore/utils/links'; +import { parseLogsFrame } from '../logsFrame'; + export type FieldDef = { keys: string[]; values: string[]; @@ -65,76 +68,83 @@ export const getDataframeFields = memoizeOne( row: LogRowModel, getFieldLinks?: (field: Field, rowIndex: number, dataFrame: DataFrame) => Array> ): FieldDef[] => { - return row.dataFrame.fields - .map((field, index) => ({ ...field, index })) - .filter((field, index) => !shouldRemoveField(field, index, row)) - .map((field) => { - const links = getFieldLinks ? getFieldLinks(field, row.rowIndex, row.dataFrame) : []; - const fieldVal = field.values[row.rowIndex]; - const outputVal = - typeof fieldVal === 'string' || typeof fieldVal === 'number' - ? fieldVal.toString() - : safeStringifyValue(fieldVal); - return { - keys: [field.name], - values: [outputVal], - links: links, - fieldIndex: field.index, - }; - }); + const visibleFields = separateVisibleFields(row.dataFrame).visible; + const nonEmptyVisibleFields = visibleFields.filter((f) => f.values[row.rowIndex] != null); + return nonEmptyVisibleFields.map((field) => { + const links = getFieldLinks ? getFieldLinks(field, row.rowIndex, row.dataFrame) : []; + const fieldVal = field.values[row.rowIndex]; + const outputVal = + typeof fieldVal === 'string' || typeof fieldVal === 'number' + ? fieldVal.toString() + : safeStringifyValue(fieldVal); + return { + keys: [field.name], + values: [outputVal], + links: links, + fieldIndex: field.index, + }; + }); } ); -export function shouldRemoveField( - field: Field, - index: number, - row: LogRowModel, - shouldRemoveLine = true, - shouldRemoveTime = true -) { - // field that has empty value (we want to keep 0 or empty string) - if (field.values[row.rowIndex] == null) { - return true; +type VisOptions = { + keepTimestamp?: boolean; + keepBody?: boolean; +}; + +// return the fields (their indices to be exact) that should be visible +// based on the logs dataframe structure +function getVisibleFieldIndices(frame: DataFrame, opts: VisOptions): Set { + const logsFrame = parseLogsFrame(frame); + if (logsFrame === null) { + // should not really happen + return new Set(); } - // hidden field, remove - if (field.config.custom?.hidden) { - return true; + // we want to show every "extra" field + const visibleFieldIndices = new Set(logsFrame.extraFields.map((f) => f.index)); + + // we always show the severity field + if (logsFrame.severityField !== null) { + visibleFieldIndices.add(logsFrame.severityField.index); } - // field with data-links, keep - if ((field.config.links ?? []).length > 0) { - return false; - } - // the remaining checks use knowledge of how we parse logs-dataframes - - // Remove field if it is: - // "labels" field that is in Loki used to store all labels - if (field.name === 'labels' && field.type === FieldType.other) { - return true; - } - // id and tsNs are arbitrary added fields in the backend and should be hidden in the UI - if (field.name === 'id' || field.name === 'tsNs') { - return true; - } - if (shouldRemoveTime) { - const firstTimeField = row.dataFrame.fields.find((f) => f.type === FieldType.time); - if ( - field.name === firstTimeField?.name && - field.type === FieldType.time && - field.values[0] === firstTimeField.values[0] - ) { - return true; - } + if (opts.keepBody) { + visibleFieldIndices.add(logsFrame.bodyField.index); } - if (shouldRemoveLine) { - // first string-field is the log-line - const firstStringFieldIndex = row.dataFrame.fields.findIndex((f) => f.type === FieldType.string); - if (firstStringFieldIndex === index) { - return true; - } + if (opts.keepTimestamp) { + visibleFieldIndices.add(logsFrame.timeField.index); } - return false; + return visibleFieldIndices; +} + +// split the dataframe's fields into visible and hidden arrays. +// note: does not do any row-level checks, +// for example does not check if the field's values are nullish +// or not at a givn row. +export function separateVisibleFields( + frame: DataFrame, + opts?: VisOptions +): { visible: FieldWithIndex[]; hidden: FieldWithIndex[] } { + const fieldsWithIndex: FieldWithIndex[] = frame.fields.map((field, index) => ({ ...field, index })); + + const visibleFieldIndices = getVisibleFieldIndices(frame, opts ?? {}); + + const [visible, hidden] = partition(fieldsWithIndex, (f) => { + // hidden fields are always hidden + if (f.config.custom?.hidden) { + return false; + } + + // fields with data-links are visible + if ((f.config.links ?? []).length > 0) { + return true; + } + + return visibleFieldIndices.has(f.index); + }); + + return { visible, hidden }; }