Loki: Remove dependency on appNotification (#80035)

* Loki: Remove dependency on appNotification * Fix and add tests * Fix lint
2025-02-25 18:55:37 -06:00 · 2024-01-05 11:01:47 +01:00 · 2024-01-05 11:01:47 +01:00 · 2b02ada7ad
commit 2b02ada7ad
parent b6bee6f72b
4 changed files with 120 additions and 57 deletions
--- a/public/app/plugins/datasource/loki/LogContextProvider.test.ts
+++ b/public/app/plugins/datasource/loki/LogContextProvider.test.ts
@ -56,9 +56,10 @@ describe('LogContextProvider', () => {

  describe('getLogRowContext', () => {
    it('should call getInitContextFilters if no cachedContextFilters', async () => {
-      logContextProvider.getInitContextFilters = jest
-        .fn()
-        .mockResolvedValue([{ value: 'baz', enabled: true, fromParser: false, label: 'bar' }]);
+      logContextProvider.getInitContextFilters = jest.fn().mockResolvedValue({
+        contextFilters: [{ value: 'baz', enabled: true, fromParser: false, label: 'bar' }],
+        preservedFiltersApplied: false,
+      });

      expect(logContextProvider.cachedContextFilters).toHaveLength(0);
      await logContextProvider.getLogRowContext(
@ -80,8 +81,7 @@ describe('LogContextProvider', () => {
          from: dateTime(defaultLogRow.timeEpochMs),
          to: dateTime(defaultLogRow.timeEpochMs),
          raw: { from: dateTime(defaultLogRow.timeEpochMs), to: dateTime(defaultLogRow.timeEpochMs) },
-        },
-        true
+        }
      );
      expect(logContextProvider.cachedContextFilters).toHaveLength(1);
    });
@ -106,9 +106,10 @@ describe('LogContextProvider', () => {

  describe('getLogRowContextQuery', () => {
    it('should call getInitContextFilters if no cachedContextFilters', async () => {
-      logContextProvider.getInitContextFilters = jest
-        .fn()
-        .mockResolvedValue([{ value: 'baz', enabled: true, fromParser: false, label: 'bar' }]);
+      logContextProvider.getInitContextFilters = jest.fn().mockResolvedValue({
+        contextFilters: [{ value: 'baz', enabled: true, fromParser: false, label: 'bar' }],
+        preservedFiltersApplied: false,
+      });

      const query = await logContextProvider.getLogRowContextQuery(defaultLogRow, {
        limit: 10,
@ -119,9 +120,10 @@ describe('LogContextProvider', () => {
    });

    it('should also call getInitContextFilters if cacheFilters is not set', async () => {
-      logContextProvider.getInitContextFilters = jest
-        .fn()
-        .mockResolvedValue([{ value: 'baz', enabled: true, fromParser: false, label: 'bar' }]);
+      logContextProvider.getInitContextFilters = jest.fn().mockResolvedValue({
+        contextFilters: [{ value: 'baz', enabled: true, fromParser: false, label: 'bar' }],
+        preservedFiltersApplied: false,
+      });
      logContextProvider.cachedContextFilters = [
        { value: 'baz', enabled: true, fromParser: false, label: 'bar' },
        { value: 'abc', enabled: true, fromParser: false, label: 'xyz' },
@ -397,21 +399,23 @@ describe('LogContextProvider', () => {
      };

      it('should correctly create contextFilters', async () => {
-        const filters = await logContextProvider.getInitContextFilters(defaultLogRow.labels, queryWithoutParser);
-        expect(filters).toEqual([
+        const result = await logContextProvider.getInitContextFilters(defaultLogRow.labels, queryWithoutParser);
+        expect(result.contextFilters).toEqual([
          { enabled: true, fromParser: false, label: 'bar', value: 'baz' },
          { enabled: false, fromParser: true, label: 'foo', value: 'uniqueParsedLabel' },
          { enabled: true, fromParser: false, label: 'xyz', value: 'abc' },
        ]);
+        expect(result.preservedFiltersApplied).toBe(false);
      });

      it('should return empty contextFilters if no query', async () => {
-        const filters = await logContextProvider.getInitContextFilters(defaultLogRow.labels, undefined);
+        const filters = (await logContextProvider.getInitContextFilters(defaultLogRow.labels, undefined))
+          .contextFilters;
        expect(filters).toEqual([]);
      });

      it('should return empty contextFilters if no labels', async () => {
-        const filters = await logContextProvider.getInitContextFilters({}, queryWithoutParser);
+        const filters = (await logContextProvider.getInitContextFilters({}, queryWithoutParser)).contextFilters;
        expect(filters).toEqual([]);
      });

@ -420,12 +424,12 @@ describe('LogContextProvider', () => {
        expect(defaultLanguageProviderMock.fetchSeriesLabels).toBeCalled();
      });

-      it('should call fetchSeriesLabels with given timerange', async () => {
+      it('should call fetchSeriesLabels with given time range', async () => {
        await logContextProvider.getInitContextFilters(defaultLogRow.labels, queryWithParser, timeRange);
        expect(defaultLanguageProviderMock.fetchSeriesLabels).toBeCalledWith(`{bar="baz"}`, { timeRange });
      });

-      it('should call `languageProvider.start` if no parser  with given timerange', async () => {
+      it('should call `languageProvider.start` if no parser with given time range', async () => {
        await logContextProvider.getInitContextFilters(defaultLogRow.labels, queryWithoutParser, timeRange);
        expect(defaultLanguageProviderMock.start).toBeCalledWith(timeRange);
      });
@ -438,21 +442,23 @@ describe('LogContextProvider', () => {
      };

      it('should correctly create contextFilters', async () => {
-        const filters = await logContextProvider.getInitContextFilters(defaultLogRow.labels, queryWithParser);
-        expect(filters).toEqual([
+        const result = await logContextProvider.getInitContextFilters(defaultLogRow.labels, queryWithParser);
+        expect(result.contextFilters).toEqual([
          { enabled: true, fromParser: false, label: 'bar', value: 'baz' },
          { enabled: false, fromParser: true, label: 'foo', value: 'uniqueParsedLabel' },
          { enabled: true, fromParser: false, label: 'xyz', value: 'abc' },
        ]);
+        expect(result.preservedFiltersApplied).toBe(false);
      });

      it('should return empty contextFilters if no query', async () => {
-        const filters = await logContextProvider.getInitContextFilters(defaultLogRow.labels, undefined);
+        const filters = (await logContextProvider.getInitContextFilters(defaultLogRow.labels, undefined))
+          .contextFilters;
        expect(filters).toEqual([]);
      });

      it('should return empty contextFilters if no labels', async () => {
-        const filters = await logContextProvider.getInitContextFilters({}, queryWithParser);
+        const filters = (await logContextProvider.getInitContextFilters({}, queryWithParser)).contextFilters;
        expect(filters).toEqual([]);
      });
    });
@ -471,12 +477,13 @@ describe('LogContextProvider', () => {
            selectedExtractedLabels: ['foo'],
          })
        );
-        const filters = await logContextProvider.getInitContextFilters(defaultLogRow.labels, queryWithParser);
-        expect(filters).toEqual([
+        const result = await logContextProvider.getInitContextFilters(defaultLogRow.labels, queryWithParser);
+        expect(result.contextFilters).toEqual([
          { enabled: false, fromParser: false, label: 'bar', value: 'baz' }, // disabled real label
          { enabled: true, fromParser: true, label: 'foo', value: 'uniqueParsedLabel' }, // enabled parsed label
          { enabled: true, fromParser: false, label: 'xyz', value: 'abc' },
        ]);
+        expect(result.preservedFiltersApplied).toBe(true);
      });

      it('should use contextFilters from row labels if all real labels are disabled', async () => {
@ -487,12 +494,13 @@ describe('LogContextProvider', () => {
            selectedExtractedLabels: ['foo'],
          })
        );
-        const filters = await logContextProvider.getInitContextFilters(defaultLogRow.labels, queryWithParser);
-        expect(filters).toEqual([
+        const result = await logContextProvider.getInitContextFilters(defaultLogRow.labels, queryWithParser);
+        expect(result.contextFilters).toEqual([
          { enabled: true, fromParser: false, label: 'bar', value: 'baz' }, // enabled real label
          { enabled: false, fromParser: true, label: 'foo', value: 'uniqueParsedLabel' },
          { enabled: true, fromParser: false, label: 'xyz', value: 'abc' }, // enabled real label
        ]);
+        expect(result.preservedFiltersApplied).toBe(false);
      });

      it('should not introduce new labels as context filters', async () => {
@ -503,12 +511,13 @@ describe('LogContextProvider', () => {
            selectedExtractedLabels: ['foo', 'new'],
          })
        );
-        const filters = await logContextProvider.getInitContextFilters(defaultLogRow.labels, queryWithParser);
-        expect(filters).toEqual([
+        const result = await logContextProvider.getInitContextFilters(defaultLogRow.labels, queryWithParser);
+        expect(result.contextFilters).toEqual([
          { enabled: false, fromParser: false, label: 'bar', value: 'baz' },
          { enabled: true, fromParser: true, label: 'foo', value: 'uniqueParsedLabel' },
          { enabled: true, fromParser: false, label: 'xyz', value: 'abc' },
        ]);
+        expect(result.preservedFiltersApplied).toBe(true);
      });
    });
  });
--- a/public/app/plugins/datasource/loki/LogContextProvider.ts
+++ b/public/app/plugins/datasource/loki/LogContextProvider.ts
@ -17,9 +17,6 @@ import {
 } from '@grafana/data';
 import { LabelParser, LabelFilter, LineFilters, PipelineStage, Logfmt, Json } from '@grafana/lezer-logql';
 import { Labels } from '@grafana/schema';
-import { notifyApp } from 'app/core/actions';
-import { createSuccessNotification } from 'app/core/copy/appNotification';
-import { dispatch } from 'app/store/store';

 import { LokiContextUi } from './components/LokiContextUi';
 import { LokiDatasource, makeRequest, REF_ID_STARTER_LOG_ROW_CONTEXT } from './datasource';
@ -64,17 +61,13 @@ export class LogContextProvider {
    // to use the cached filters, we need to reinitialize them.
    if (this.cachedContextFilters.length === 0 || !cacheFilters) {
      const filters = (
-        await this.getInitContextFilters(
-          row.labels,
-          origQuery,
-          {
-            from: dateTime(row.timeEpochMs),
-            to: dateTime(row.timeEpochMs),
-            raw: { from: dateTime(row.timeEpochMs), to: dateTime(row.timeEpochMs) },
-          },
-          cacheFilters
-        )
-      ).filter((filter) => filter.enabled);
+        await this.getInitContextFilters(row.labels, origQuery, {
+          from: dateTime(row.timeEpochMs),
+          to: dateTime(row.timeEpochMs),
+          raw: { from: dateTime(row.timeEpochMs), to: dateTime(row.timeEpochMs) },
+        })
+      ).contextFilters.filter((filter) => filter.enabled);
+
      this.cachedContextFilters = filters;
    }

@ -307,9 +300,14 @@ export class LogContextProvider {
    );
  };

-  getInitContextFilters = async (labels: Labels, query?: LokiQuery, timeRange?: TimeRange, cacheFilters?: boolean) => {
+  getInitContextFilters = async (
+    labels: Labels,
+    query?: LokiQuery,
+    timeRange?: TimeRange
+  ): Promise<{ contextFilters: ContextFilter[]; preservedFiltersApplied: boolean }> => {
+    let preservedFiltersApplied = false;
    if (!query || isEmpty(labels)) {
-      return [];
+      return { contextFilters: [], preservedFiltersApplied };
    }

    // 1. First we need to get all labels from the log row's label
@ -352,7 +350,7 @@ export class LogContextProvider {

    if (!preservedLabels) {
      // If we don't have preservedLabels, we return contextFilters as they are
-      return contextFilters;
+      return { contextFilters, preservedFiltersApplied };
    } else {
      // Otherwise, we need to update filters based on preserved labels
      let arePreservedLabelsUsed = false;
@ -373,15 +371,12 @@ export class LogContextProvider {
      const isAtLeastOneRealLabelEnabled = newContextFilters.some(({ enabled, fromParser }) => enabled && !fromParser);
      if (!isAtLeastOneRealLabelEnabled) {
        // If we end up with no real labels enabled, we need to reset the init filters
-        return contextFilters;
+        return { contextFilters, preservedFiltersApplied };
      } else {
-        // Otherwise use new filters; also only show the notification if filters
-        // are supposed to be cached, which is currently used in the UI, not
-        // when tab-opened
-        if (arePreservedLabelsUsed && cacheFilters) {
-          dispatch(notifyApp(createSuccessNotification('Previously used log context filters have been applied.')));
+        if (arePreservedLabelsUsed) {
+          preservedFiltersApplied = true;
        }
-        return newContextFilters;
+        return { contextFilters: newContextFilters, preservedFiltersApplied };
      }
    }
  };
--- a/public/app/plugins/datasource/loki/components/LokiContextUi.test.tsx
+++ b/public/app/plugins/datasource/loki/components/LokiContextUi.test.tsx
@ -41,10 +41,13 @@ const setupProps = (): LokiContextUiProps => {

 const mockLogContextProvider = {
  getInitContextFilters: jest.fn().mockImplementation(() =>
-    Promise.resolve([
-      { value: 'value1', enabled: true, fromParser: false, label: 'label1' },
-      { value: 'value3', enabled: false, fromParser: true, label: 'label3' },
-    ])
+    Promise.resolve({
+      contextFilters: [
+        { value: 'value1', enabled: true, fromParser: false, label: 'label1' },
+        { value: 'value3', enabled: false, fromParser: true, label: 'label3' },
+      ],
+      preservedFiltersApplied: false,
+    })
  ),
  processContextFiltersToExpr: jest.fn().mockImplementation(
    (contextFilters: ContextFilter[], query: LokiQuery | undefined) =>
@ -327,4 +330,35 @@ describe('LokiContextUi', () => {
      expect(screen.queryByText('label3="value3"')).not.toBeInTheDocument();
    });
  });
+  it('shows if preserved filters are applied', async () => {
+    const props = setupProps();
+    const newProps = {
+      ...props,
+      logContextProvider: {
+        ...props.logContextProvider,
+        getInitContextFilters: jest.fn().mockImplementation(() =>
+          Promise.resolve({
+            contextFilters: [
+              { value: 'value1', enabled: true, fromParser: false, label: 'label1' },
+              { value: 'value3', enabled: false, fromParser: true, label: 'label3' },
+            ],
+            preservedFiltersApplied: true,
+          })
+        ),
+      },
+    } as unknown as LokiContextUiProps;
+
+    render(<LokiContextUi {...newProps} />);
+    expect(await screen.findByText('Previously used filters have been applied.')).toBeInTheDocument();
+  });
+
+  it('does not shows if preserved filters are not applied', async () => {
+    // setupProps() already has preservedFiltersApplied: false
+    const props = setupProps();
+
+    render(<LokiContextUi {...props} />);
+    await waitFor(() => {
+      expect(screen.queryByText('Previously used filters have been applied.')).not.toBeInTheDocument();
+    });
+  });
 });
--- a/public/app/plugins/datasource/loki/components/LokiContextUi.tsx
+++ b/public/app/plugins/datasource/loki/components/LokiContextUi.tsx
@ -5,6 +5,7 @@ import { useAsync } from 'react-use';
 import { dateTime, GrafanaTheme2, LogRowModel, renderMarkdown, SelectableValue } from '@grafana/data';
 import { reportInteraction } from '@grafana/runtime';
 import {
+  Alert,
  Button,
  Collapse,
  Icon,
@ -80,6 +81,12 @@ function getStyles(theme: GrafanaTheme2) {
      background-color: ${theme.colors.background.secondary};
      padding: ${theme.spacing(2)};
    `,
+    notification: css({
+      position: 'absolute',
+      zIndex: theme.zIndex.portal,
+      top: 0,
+      right: 0,
+    }),
    rawQuery: css`
      display: inline;
    `,
@ -111,6 +118,7 @@ export function LokiContextUi(props: LokiContextUiProps) {
  const styles = useStyles2(getStyles);

  const [contextFilters, setContextFilters] = useState<ContextFilter[]>([]);
+  const [showPreservedFiltersAppliedNotification, setShowPreservedFiltersAppliedNotification] = useState(false);

  const [initialized, setInitialized] = useState(false);
  const [loading, setLoading] = useState(false);
@ -203,12 +211,21 @@ export function LokiContextUi(props: LokiContextUiProps) {
      to: dateTime(row.timeEpochMs),
      raw: { from: dateTime(row.timeEpochMs), to: dateTime(row.timeEpochMs) },
    });
-    setContextFilters(initContextFilters);
-
+    setContextFilters(initContextFilters.contextFilters);
+    setShowPreservedFiltersAppliedNotification(initContextFilters.preservedFiltersApplied);
    setInitialized(true);
    setLoading(false);
  });

+  // To hide previousContextFiltersApplied notification after 2 seconds
+  useEffect(() => {
+    if (showPreservedFiltersAppliedNotification) {
+      setTimeout(() => {
+        setShowPreservedFiltersAppliedNotification(false);
+      }, 2000);
+    }
+  }, [showPreservedFiltersAppliedNotification]);
+
  useEffect(() => {
    reportInteraction('grafana_explore_logs_loki_log_context_loaded', {
      logRowUid: row.uid,
@ -245,6 +262,14 @@ export function LokiContextUi(props: LokiContextUiProps) {
  );
  return (
    <div className={styles.wrapper}>
+      {showPreservedFiltersAppliedNotification && (
+        <Alert
+          className={styles.notification}
+          title="Previously used filters have been applied."
+          severity="info"
+          elevated={true}
+        ></Alert>
+      )}
      <Tooltip content={'Revert to initial log context query.'}>
        <div className={styles.iconButton}>
          <Button