Loki: Fix showing of unusable labels field in detected fields (#53319)

* Loki: Fix showing of labels field in detected fields

* Create reusable createlogRow mock

.betterer.results

@@ -8,7 +8,7 @@ exports[`no enzyme tests`] = {
     "packages/grafana-ui/src/components/Graph/Graph.test.tsx:1664091255": [
       [0, 17, 13, "RegExp match", "2409514259"]
     ],
-    "packages/grafana-ui/src/components/Logs/LogRowContextProvider.test.tsx:2719724375": [
+    "packages/grafana-ui/src/components/Logs/LogRowContextProvider.test.tsx:943686035": [
       [0, 17, 13, "RegExp match", "2409514259"]
     ],
     "packages/grafana-ui/src/components/QueryField/QueryField.test.tsx:375894800": [
@@ -1560,8 +1560,7 @@ exports[`better eslint`] = {
       [0, 0, 0, "Unexpected any. Specify a different type.", "1"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "2"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "3"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "4"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "5"]
+      [0, 0, 0, "Unexpected any. Specify a different type.", "4"]
     ],
     "packages/grafana-ui/src/components/Logs/LogRowContextProvider.tsx:5381": [
       [0, 0, 0, "Do not use any type assertions.", "0"],

packages/grafana-ui/src/components/Logs/LogDetails.test.tsx

@@ -4,30 +4,14 @@ import React from 'react';
 import { Field, GrafanaTheme2, LogLevel, LogRowModel, MutableDataFrame } from '@grafana/data';
 
 import { LogDetails, Props } from './LogDetails';
+import { createLogRow } from './__mocks__/logRow';
 
 const setup = (propOverrides?: Partial<Props>, rowOverrides?: Partial<LogRowModel>) => {
   const props: Props = {
     theme: {} as GrafanaTheme2,
     showDuplicates: false,
     wrapLogMessage: false,
-    row: {
-      dataFrame: new MutableDataFrame(),
-      entryFieldIndex: 0,
-      rowIndex: 0,
-      logLevel: 'error' as LogLevel,
-      timeFromNow: '',
-      timeEpochMs: 1546297200000,
-      timeEpochNs: '1546297200000000000',
-      timeLocal: '',
-      timeUtc: '',
-      hasAnsi: false,
-      hasUnescapedContent: false,
-      entry: '',
-      raw: '',
-      uid: '0',
-      labels: {},
-      ...(rowOverrides || {}),
-    },
+    row: createLogRow({ logLevel: LogLevel.error, timeEpochMs: 1546297200000, ...rowOverrides }),
     getRows: () => [],
     onClickFilterLabel: () => {},
     onClickFilterOutLabel: () => {},

packages/grafana-ui/src/components/Logs/LogRowContextProvider.test.tsx

@@ -2,9 +2,12 @@ import { mount } from 'enzyme';
 import React from 'react';
 import { act } from 'react-dom/test-utils';
 
-import { FieldType, LogRowModel, MutableDataFrame, Labels, LogLevel, DataQueryResponse } from '@grafana/data';
+import { FieldType, LogRowModel, MutableDataFrame, DataQueryResponse } from '@grafana/data';
 
 import { getRowContexts, LogRowContextProvider } from './LogRowContextProvider';
+import { createLogRow } from './__mocks__/logRow';
+
+const row = createLogRow({ entry: '4', timeEpochMs: 4 });
 
 describe('getRowContexts', () => {
   describe('when called with a DataFrame and results are returned', () => {
@@ -175,21 +178,3 @@ describe('LogRowContextProvider', () => {
     });
   });
 });
-
-const row: LogRowModel = {
-  entryFieldIndex: 0,
-  rowIndex: 0,
-  dataFrame: new MutableDataFrame(),
-  entry: '4',
-  labels: null as any as Labels,
-  hasAnsi: false,
-  hasUnescapedContent: false,
-  raw: '4',
-  logLevel: LogLevel.info,
-  timeEpochMs: 4,
-  timeEpochNs: '4000000',
-  timeFromNow: '',
-  timeLocal: '',
-  timeUtc: '',
-  uid: '1',
-};

packages/grafana-ui/src/components/Logs/LogRows.test.tsx

@@ -2,13 +2,14 @@ import { render, screen } from '@testing-library/react';
 import { range } from 'lodash';
 import React from 'react';
 
-import { LogLevel, LogRowModel, LogsDedupStrategy, MutableDataFrame, LogsSortOrder } from '@grafana/data';
+import { LogRowModel, LogsDedupStrategy, LogsSortOrder } from '@grafana/data';
 
 import { LogRows, PREVIEW_LIMIT } from './LogRows';
+import { createLogRow } from './__mocks__/logRow';
 
 describe('LogRows', () => {
   it('renders rows', () => {
-    const rows: LogRowModel[] = [makeLog({ uid: '1' }), makeLog({ uid: '2' }), makeLog({ uid: '3' })];
+    const rows: LogRowModel[] = [createLogRow({ uid: '1' }), createLogRow({ uid: '2' }), createLogRow({ uid: '3' })];
     render(
       <LogRows
         logRows={rows}
@@ -29,7 +30,7 @@ describe('LogRows', () => {
   });
 
   it('renders rows only limited number of rows first', () => {
-    const rows: LogRowModel[] = [makeLog({ uid: '1' }), makeLog({ uid: '2' }), makeLog({ uid: '3' })];
+    const rows: LogRowModel[] = [createLogRow({ uid: '1' }), createLogRow({ uid: '2' }), createLogRow({ uid: '3' })];
     jest.useFakeTimers();
     const { rerender } = render(
       <LogRows
@@ -73,8 +74,8 @@ describe('LogRows', () => {
   });
 
   it('renders deduped rows if supplied', () => {
-    const rows: LogRowModel[] = [makeLog({ uid: '1' }), makeLog({ uid: '2' }), makeLog({ uid: '3' })];
-    const dedupedRows: LogRowModel[] = [makeLog({ uid: '4' }), makeLog({ uid: '5' })];
+    const rows: LogRowModel[] = [createLogRow({ uid: '1' }), createLogRow({ uid: '2' }), createLogRow({ uid: '3' })];
+    const dedupedRows: LogRowModel[] = [createLogRow({ uid: '4' }), createLogRow({ uid: '5' })];
     render(
       <LogRows
         logRows={rows}
@@ -95,7 +96,7 @@ describe('LogRows', () => {
 
   it('renders with default preview limit', () => {
     // PREVIEW_LIMIT * 2 is there because otherwise we just render all rows
-    const rows: LogRowModel[] = range(PREVIEW_LIMIT * 2 + 1).map((num) => makeLog({ uid: num.toString() }));
+    const rows: LogRowModel[] = range(PREVIEW_LIMIT * 2 + 1).map((num) => createLogRow({ uid: num.toString() }));
     render(
       <LogRows
         logRows={rows}
@@ -115,9 +116,9 @@ describe('LogRows', () => {
 
   it('renders asc ordered rows if order and function supplied', () => {
     const rows: LogRowModel[] = [
-      makeLog({ uid: '1', timeEpochMs: 1 }),
-      makeLog({ uid: '3', timeEpochMs: 3 }),
-      makeLog({ uid: '2', timeEpochMs: 2 }),
+      createLogRow({ uid: '1', timeEpochMs: 1 }),
+      createLogRow({ uid: '3', timeEpochMs: 3 }),
+      createLogRow({ uid: '2', timeEpochMs: 2 }),
     ];
     render(
       <LogRows
@@ -139,9 +140,9 @@ describe('LogRows', () => {
   });
   it('renders desc ordered rows if order and function supplied', () => {
     const rows: LogRowModel[] = [
-      makeLog({ uid: '1', timeEpochMs: 1 }),
-      makeLog({ uid: '3', timeEpochMs: 3 }),
-      makeLog({ uid: '2', timeEpochMs: 2 }),
+      createLogRow({ uid: '1', timeEpochMs: 1 }),
+      createLogRow({ uid: '3', timeEpochMs: 3 }),
+      createLogRow({ uid: '2', timeEpochMs: 2 }),
     ];
     render(
       <LogRows
@@ -162,29 +163,3 @@ describe('LogRows', () => {
     expect(screen.queryAllByRole('row').at(2)).toHaveTextContent('log message 1');
   });
 });
-
-const makeLog = (overrides: Partial<LogRowModel>): LogRowModel => {
-  const uid = overrides.uid || '1';
-  const timeEpochMs = overrides.timeEpochMs || 1;
-  const entry = `log message ${uid}`;
-  return {
-    entryFieldIndex: 0,
-    rowIndex: 0,
-    // Does not need to be filled with current tests
-    dataFrame: new MutableDataFrame(),
-    uid,
-    logLevel: LogLevel.debug,
-    entry,
-    hasAnsi: false,
-    hasUnescapedContent: false,
-    labels: {},
-    raw: entry,
-    timeFromNow: '',
-    timeEpochMs,
-    timeEpochNs: (timeEpochMs * 1000000).toString(),
-    timeLocal: '',
-    timeUtc: '',
-    searchWords: [],
-    ...overrides,
-  };
-};

packages/grafana-ui/src/components/Logs/__mocks__/logRow.ts

@@ -0,0 +1,27 @@
+import { LogLevel, LogRowModel, MutableDataFrame } from '@grafana/data';
+
+export const createLogRow = (overrides?: Partial<LogRowModel>): LogRowModel => {
+  const uid = overrides?.uid || '1';
+  const timeEpochMs = overrides?.timeEpochMs || 1;
+  const entry = overrides?.entry || `log message ${uid}`;
+
+  return {
+    entryFieldIndex: 0,
+    rowIndex: 0,
+    dataFrame: new MutableDataFrame(),
+    uid,
+    logLevel: LogLevel.info,
+    entry,
+    hasAnsi: false,
+    hasUnescapedContent: false,
+    labels: {},
+    raw: entry,
+    timeFromNow: '',
+    timeEpochMs,
+    timeEpochNs: (timeEpochMs * 1000000).toString(),
+    timeLocal: '',
+    timeUtc: '',
+    searchWords: [],
+    ...overrides,
+  };
+};

packages/grafana-ui/src/components/Logs/logParser.test.ts

@@ -0,0 +1,168 @@
+import { ArrayVector, FieldType, MutableDataFrame } from '@grafana/data';
+
+import { createLogRow } from './__mocks__/logRow';
+import { getAllFields } from './logParser';
+
+describe('getAllFields', () => {
+  it('should filter out field with labels name and other type', () => {
+    const logRow = createLogRow({
+      entryFieldIndex: 10,
+      dataFrame: new MutableDataFrame({
+        refId: 'A',
+        fields: [
+          testStringField,
+          {
+            name: 'labels',
+            type: FieldType.other,
+            config: {},
+            values: new ArrayVector([{ place: 'luna', source: 'data' }]),
+          },
+        ],
+      }),
+    });
+
+    const fields = getAllFields(logRow);
+    expect(fields.length).toBe(1);
+    expect(fields.find((field) => field.key === 'labels')).toBe(undefined);
+  });
+
+  it('should not filter out field with labels name and string type', () => {
+    const logRow = createLogRow({
+      entryFieldIndex: 10,
+      dataFrame: new MutableDataFrame({
+        refId: 'A',
+        fields: [
+          testStringField,
+          {
+            name: 'labels',
+            type: FieldType.string,
+            config: {},
+            values: new ArrayVector([{ place: 'luna', source: 'data' }]),
+          },
+        ],
+      }),
+    });
+    const fields = getAllFields(logRow);
+    expect(fields.length).toBe(2);
+    expect(fields.find((field) => field.key === 'labels')).not.toBe(undefined);
+  });
+
+  it('should filter out field with id name', () => {
+    const logRow = createLogRow({
+      entryFieldIndex: 10,
+      dataFrame: new MutableDataFrame({
+        refId: 'A',
+        fields: [
+          testStringField,
+          {
+            name: 'id',
+            type: FieldType.string,
+            config: {},
+            values: new ArrayVector(['1659620138401000000_8b1f7688_']),
+          },
+        ],
+      }),
+    });
+
+    const fields = getAllFields(logRow);
+    expect(fields.length).toBe(1);
+    expect(fields.find((field) => field.key === 'id')).toBe(undefined);
+  });
+
+  it('should filter out entry field which is shown as the log message', () => {
+    const logRow = createLogRow({
+      entryFieldIndex: 3,
+      dataFrame: new MutableDataFrame({
+        refId: 'A',
+        fields: [
+          testStringField,
+          {
+            name: 'labels',
+            type: FieldType.other,
+            config: {},
+            values: new ArrayVector([{ place: 'luna', source: 'data' }]),
+          },
+          {
+            name: 'Time',
+            type: FieldType.time,
+            config: {},
+            values: new ArrayVector([1659620138401]),
+          },
+          {
+            name: 'Line',
+            type: FieldType.string,
+            config: {},
+            values: new ArrayVector([
+              '_entry="log text with ANSI \u001b[31mpart of the text\u001b[0m [616951240]" counter=300 float=NaN label=val3 level=info',
+            ]),
+          },
+        ],
+      }),
+    });
+
+    const fields = getAllFields(logRow);
+    expect(fields.find((field) => field.key === 'Line')).toBe(undefined);
+  });
+
+  it('should filter out field with config hidden field', () => {
+    const testField = { ...testStringField };
+    testField.config = {
+      custom: {
+        hidden: true,
+      },
+    };
+    const logRow = createLogRow({
+      entryFieldIndex: 10,
+      dataFrame: new MutableDataFrame({
+        refId: 'A',
+        fields: [{ ...testField }],
+      }),
+    });
+
+    const fields = getAllFields(logRow);
+    expect(fields.length).toBe(0);
+    expect(fields.find((field) => field.key === testField.name)).toBe(undefined);
+  });
+
+  it('should filter out field with null values', () => {
+    const logRow = createLogRow({
+      entryFieldIndex: 10,
+      dataFrame: new MutableDataFrame({
+        refId: 'A',
+        fields: [{ ...testFieldWithNullValue }],
+      }),
+    });
+
+    const fields = getAllFields(logRow);
+    expect(fields.length).toBe(0);
+    expect(fields.find((field) => field.key === testFieldWithNullValue.name)).toBe(undefined);
+  });
+
+  it('should not filter out field with string values', () => {
+    const logRow = createLogRow({
+      entryFieldIndex: 10,
+      dataFrame: new MutableDataFrame({
+        refId: 'A',
+        fields: [{ ...testStringField }],
+      }),
+    });
+
+    const fields = getAllFields(logRow);
+    expect(fields.length).toBe(1);
+    expect(fields.find((field) => field.key === testStringField.name)).not.toBe(undefined);
+  });
+});
+
+const testStringField = {
+  name: 'test_field_string',
+  type: FieldType.string,
+  config: {},
+  values: new ArrayVector(['abc']),
+};
+
+const testFieldWithNullValue = {
+  name: 'test_field_null',
+  type: FieldType.string,
+  config: {},
+  values: new ArrayVector([null]),
+};

packages/grafana-ui/src/components/Logs/logParser.ts

@@ -1,6 +1,6 @@
 import memoizeOne from 'memoize-one';
 
-import { Field, getParser, LinkModel, LogRowModel } from '@grafana/data';
+import { Field, FieldType, getParser, LinkModel, LogRowModel } from '@grafana/data';
 
 import { MAX_CHARACTERS } from './LogRowMessage';
 
@@ -62,31 +62,18 @@ const parseMessage = memoizeOne((rowEntry): FieldDef[] => {
 
 const getDerivedFields = memoizeOne(
   (row: LogRowModel, getFieldLinks?: (field: Field, rowIndex: number) => Array<LinkModel<Field>>): FieldDef[] => {
-    return (
-      row.dataFrame.fields
-        .map((field, index) => ({ ...field, index }))
-        // Remove Id which we use for react key and entry field which we are showing as the log message. Also remove hidden fields.
-        .filter(
-          (field, index) => !('id' === field.name || row.entryFieldIndex === index || field.config.custom?.hidden)
-        )
-        // Filter out fields without values. For example in elastic the fields are parsed from the document which can
-        // have different structure per row and so the dataframe is pretty sparse.
-        .filter((field) => {
-          const value = field.values.get(row.rowIndex);
-          // Not sure exactly what will be the empty value here. And we want to keep 0 as some values can be non
-          // string.
-          return value !== null && value !== undefined;
-        })
-        .map((field) => {
-          const links = getFieldLinks ? getFieldLinks(field, row.rowIndex) : [];
-          return {
-            key: field.name,
-            value: field.values.get(row.rowIndex).toString(),
-            links: links,
-            fieldIndex: field.index,
-          };
-        })
-    );
+    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) : [];
+        return {
+          key: field.name,
+          value: field.values.get(row.rowIndex).toString(),
+          links: links,
+          fieldIndex: field.index,
+        };
+      });
   }
 );
 
@@ -99,3 +86,28 @@ function sortFieldsLinkFirst(fieldA: FieldDef, fieldB: FieldDef) {
   }
   return fieldA.key > fieldB.key ? 1 : fieldA.key < fieldB.key ? -1 : 0;
 }
+
+function shouldRemoveField(field: Field, index: number, row: LogRowModel) {
+  // 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" field which we use for react key
+  if (field.name === 'id') {
+    return true;
+  }
+  // entry field which we are showing as the log message
+  if (row.entryFieldIndex === index) {
+    return true;
+  }
+  // hidden field
+  if (field.config.custom?.hidden) {
+    return true;
+  }
+  // field that has empty value (we want to keep 0 or empty string)
+  if (field.values.get(row.rowIndex) == null) {
+    return true;
+  }
+  return false;
+}