From 2b02ada7ad50325d4ce151702d23e317da3f4c79 Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Fri, 5 Jan 2024 11:01:47 +0100 Subject: [PATCH] Loki: Remove dependency on appNotification (#80035) * Loki: Remove dependency on appNotification * Fix and add tests * Fix lint --- .../loki/LogContextProvider.test.ts | 63 +++++++++++-------- .../datasource/loki/LogContextProvider.ts | 43 ++++++------- .../loki/components/LokiContextUi.test.tsx | 42 +++++++++++-- .../loki/components/LokiContextUi.tsx | 29 ++++++++- 4 files changed, 120 insertions(+), 57 deletions(-) diff --git a/public/app/plugins/datasource/loki/LogContextProvider.test.ts b/public/app/plugins/datasource/loki/LogContextProvider.test.ts index c35901aee7b..381aa11fbb0 100644 --- 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); }); }); }); diff --git a/public/app/plugins/datasource/loki/LogContextProvider.ts b/public/app/plugins/datasource/loki/LogContextProvider.ts index 2035829d9f7..fdbcf65b646 100644 --- 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 }; } } }; diff --git a/public/app/plugins/datasource/loki/components/LokiContextUi.test.tsx b/public/app/plugins/datasource/loki/components/LokiContextUi.test.tsx index 7e6c71a8ce8..788c423992f 100644 --- 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(); + 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(); + await waitFor(() => { + expect(screen.queryByText('Previously used filters have been applied.')).not.toBeInTheDocument(); + }); + }); }); diff --git a/public/app/plugins/datasource/loki/components/LokiContextUi.tsx b/public/app/plugins/datasource/loki/components/LokiContextUi.tsx index 53041c119a7..6b7dda28026 100644 --- 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([]); + 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 (
+ {showPreservedFiltersAppliedNotification && ( + + )}