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

@@ -667,7 +667,6 @@ class UnthemedLogs extends PureComponent<Props, State> {
               <div className={styles.logRows} data-testid="logRowsTable">
                 {/* Width should be full width minus logsnavigation and padding */}
                 <LogsTable
-                  rows={logRows}
                   logsSortOrder={this.state.logsSortOrder}
                   range={this.props.range}
                   splitOpen={this.props.splitOpen}

public/app/features/explore/Logs/LogsTable.test.tsx

@@ -1,15 +1,7 @@
 import { render, screen, waitFor } from '@testing-library/react';
 import React, { ComponentProps } from 'react';
 
-import {
-  FieldType,
-  LogLevel,
-  LogRowModel,
-  LogsSortOrder,
-  MutableDataFrame,
-  standardTransformersRegistry,
-  toUtc,
-} from '@grafana/data';
+import { FieldType, LogRowModel, LogsSortOrder, standardTransformersRegistry, toUtc } from '@grafana/data';
 import { organizeFieldsTransformer } from '@grafana/data/src/transformations/transformers/organize';
 import { config } from '@grafana/runtime';
 import { extractFieldsTransformer } from 'app/features/transformers/extractFields/extractFields';
@@ -68,7 +60,6 @@ describe('LogsTable', () => {
     };
     return (
       <LogsTable
-        rows={[makeLog({})]}
         logsSortOrder={LogsSortOrder.Descending}
         splitOpen={() => undefined}
         timeZone={'utc'}
@@ -140,26 +131,3 @@ describe('LogsTable', () => {
     });
   });
 });
-
-const makeLog = (overrides: Partial<LogRowModel>): 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,
-  };
-};

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> = (props) => {
-  const { timeZone, splitOpen, range, logsSortOrder, width, logsFrames, rows } = props;
+  const { timeZone, splitOpen, range, logsSortOrder, width, logsFrames } = props;
 
   const [tableFrame, setTableFrame] = useState<DataFrame | undefined>(undefined);
 
@@ -129,18 +127,17 @@ export const LogsTable: React.FunctionComponent<Props> = (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> = (props) => {
       }
     };
     prepare();
-  }, [prepareTableFrame, logsFrames, logsSortOrder, rows]);
+  }, [prepareTableFrame, logsFrames, logsSortOrder]);
 
   if (!tableFrame) {
     return null;

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', () => {

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<LinkModel<Field>>
   ): 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<number> {
+  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 };
 }