From f3783146bbabdb3349372c5185709bb9c7b21ddb Mon Sep 17 00:00:00 2001 From: Matias Chomicki Date: Tue, 19 Nov 2024 15:27:26 +0000 Subject: [PATCH] Explore: Sync logs displayed fields with URL state (#96242) * Explore: store displayedFields in URL state * Displayed fields: reset when queries change * Add test * LogRowMessageDisplayedFields: pass logRowMenuIcons props to component * LogsMetaRow: use primary outline for reset button * Logs: clear displayedFields from URL --- packages/grafana-data/src/types/explore.ts | 1 + .../app/features/explore/Logs/Logs.test.tsx | 30 ++++++-- public/app/features/explore/Logs/Logs.tsx | 69 ++++++++++++++++--- .../app/features/explore/Logs/LogsMetaRow.tsx | 2 +- .../app/features/explore/Logs/utils/logs.ts | 15 ++++ .../app/features/logs/components/LogRow.tsx | 2 + .../logs/components/LogRowMessage.test.tsx | 1 - .../LogRowMessageDisplayedFields.test.tsx | 23 +++++++ .../LogRowMessageDisplayedFields.tsx | 19 ++++- 9 files changed, 141 insertions(+), 21 deletions(-) diff --git a/packages/grafana-data/src/types/explore.ts b/packages/grafana-data/src/types/explore.ts index 90f21ae568b..f95d90ac409 100644 --- a/packages/grafana-data/src/types/explore.ts +++ b/packages/grafana-data/src/types/explore.ts @@ -54,6 +54,7 @@ export interface ExploreLogsPanelState { labelFieldName?: string; // Used for logs table visualisation, contains the refId of the dataFrame that is currently visualized refId?: string; + displayedFields?: string[]; } export interface SplitOpenOptions { diff --git a/public/app/features/explore/Logs/Logs.test.tsx b/public/app/features/explore/Logs/Logs.test.tsx index ec8ddda9938..620f576e118 100644 --- a/public/app/features/explore/Logs/Logs.test.tsx +++ b/public/app/features/explore/Logs/Logs.test.tsx @@ -150,12 +150,12 @@ describe('Logs', () => { }, }); - const { rerender } = render( + const rendered = render( {getComponent(partialProps, dataFrame ? dataFrame : getMockLokiFrame(), logs)} ); - return { rerender, store: fakeStore }; + return { ...rendered, store: fakeStore }; }; describe('scrolling behavior', () => { @@ -428,12 +428,14 @@ describe('Logs', () => { }); it('should call createAndCopyShortLink on permalinkClick - logs', async () => { - const panelState: Partial = { logs: { id: 'not-included', visualisationType: 'logs' } }; + const panelState: Partial = { + logs: { id: 'not-included', visualisationType: 'logs', displayedFields: ['field'] }, + }; const rows = [ - makeLog({ uid: '1', rowId: 'id1', timeEpochMs: 1 }), - makeLog({ uid: '2', rowId: 'id2', timeEpochMs: 1 }), - makeLog({ uid: '3', rowId: 'id3', timeEpochMs: 2 }), - makeLog({ uid: '4', rowId: 'id3', timeEpochMs: 2 }), + makeLog({ uid: '1', rowId: 'id1', timeEpochMs: 1, labels: { field: '1' } }), + makeLog({ uid: '2', rowId: 'id2', timeEpochMs: 1, labels: { field: '2' } }), + makeLog({ uid: '3', rowId: 'id3', timeEpochMs: 2, labels: { field: '3' } }), + makeLog({ uid: '4', rowId: 'id3', timeEpochMs: 2, labels: { field: '4' } }), ]; setup({ loading: false, panelState, logRows: rows }); @@ -449,6 +451,7 @@ describe('Logs', () => { ) ); expect(createAndCopyShortLink).toHaveBeenCalledWith(expect.stringMatching('visualisationType%22:%22logs')); + expect(createAndCopyShortLink).toHaveBeenCalledWith(expect.stringMatching('displayedFields%22:%5B%22field')); }); it('should call createAndCopyShortLink on permalinkClick - with infinite scrolling', async () => { @@ -480,6 +483,19 @@ describe('Logs', () => { }); }); + describe('displayed fields', () => { + it('should sync displayed fields from the URL', async () => { + const panelState: Partial = { + logs: { id: 'not-included', visualisationType: 'logs', displayedFields: ['field'] }, + }; + const rows = [makeLog({ uid: '1', rowId: 'id1', timeEpochMs: 1, labels: { field: 'field value' } })]; + setup({ loading: false, panelState, logRows: rows }); + + expect(await screen.findByText('field=field value')).toBeInTheDocument(); + expect(screen.queryByText(/log message/)).not.toBeInTheDocument(); + }); + }); + describe('with table visualisation', () => { let originalVisualisationTypeValue = config.featureToggles.logsExploreTableVisualisation; diff --git a/public/app/features/explore/Logs/Logs.tsx b/public/app/features/explore/Logs/Logs.tsx index d04c45fe2fb..8da155b1142 100644 --- a/public/app/features/explore/Logs/Logs.tsx +++ b/public/app/features/explore/Logs/Logs.tsx @@ -10,7 +10,6 @@ import { LogRowModel, LogsMetaItem, DataFrame, - DataQuery, AbsoluteTimeRange, GrafanaTheme2, LoadingState, @@ -37,6 +36,7 @@ import { urlUtil, } from '@grafana/data'; import { config, reportInteraction } from '@grafana/runtime'; +import { DataQuery } from '@grafana/schema'; import { Button, InlineField, @@ -79,7 +79,7 @@ import { LogsMetaRow } from './LogsMetaRow'; import LogsNavigation from './LogsNavigation'; import { LogsTableWrap, getLogsTableHeight } from './LogsTableWrap'; import { LogsVolumePanelList } from './LogsVolumePanelList'; -import { SETTINGS_KEYS, visualisationTypeKey } from './utils/logs'; +import { canKeepDisplayedFields, SETTINGS_KEYS, visualisationTypeKey } from './utils/logs'; interface Props extends Themeable2 { width: number; @@ -221,6 +221,7 @@ const UnthemedLogs: React.FunctionComponent = (props: Props) => { const cancelFlippingTimer = useRef(undefined); const toggleLegendRef = useRef<(name: string, mode: SeriesVisibilityChangeMode) => void>(() => {}); const topLogsRef = useRef(null); + const prevLogsQueries = usePrevious(logsQueries); const tableHeight = getLogsTableHeight(); const styles = getStyles(theme, wrapLogMessage, tableHeight); @@ -330,6 +331,16 @@ const UnthemedLogs: React.FunctionComponent = (props: Props) => { store.set(visualisationTypeKey, visualisationType); }, [panelState?.logs?.visualisationType]); + useEffect(() => { + let displayedFields: string[] = []; + if (Array.isArray(panelState?.logs?.displayedFields)) { + displayedFields = panelState?.logs?.displayedFields; + } else if (panelState?.logs?.displayedFields && typeof panelState?.logs?.displayedFields === 'object') { + displayedFields = Object.values(panelState?.logs?.displayedFields); + } + setDisplayedFields(displayedFields); + }, [panelState?.logs?.displayedFields]); + useEffect(() => { registerLogLevelsWithContentOutline(); }, [logsVolumeData?.data, hiddenLogLevels, registerLogLevelsWithContentOutline]); @@ -344,8 +355,13 @@ const UnthemedLogs: React.FunctionComponent = (props: Props) => { }); useUnmount(() => { - // If we're unmounting logs (e.g. switching to another datasource), we need to remove the table specific panel state, otherwise it will persist in the explore url - if (panelState?.logs?.columns || panelState?.logs?.refId || panelState?.logs?.labelFieldName) { + // If we're unmounting logs (e.g. switching to another datasource), we need to remove the logs specific panel state, otherwise it will persist in the explore url + if ( + panelState?.logs?.columns || + panelState?.logs?.refId || + panelState?.logs?.labelFieldName || + panelState?.logs?.displayedFields + ) { dispatch( changePanelState(exploreId, 'logs', { ...panelState?.logs, @@ -353,6 +369,7 @@ const UnthemedLogs: React.FunctionComponent = (props: Props) => { visualisationType: visualisationType, labelFieldName: undefined, refId: undefined, + displayedFields: undefined, }) ); } @@ -369,13 +386,35 @@ const UnthemedLogs: React.FunctionComponent = (props: Props) => { visualisationType: logsPanelState.visualisationType ?? visualisationType, labelFieldName: logsPanelState.labelFieldName, refId: logsPanelState.refId ?? panelState?.logs?.refId, + displayedFields: logsPanelState.displayedFields ?? panelState?.logs?.displayedFields, }) ); } }, - [dispatch, exploreId, panelState?.logs?.columns, panelState?.logs?.refId, visualisationType] + [ + dispatch, + exploreId, + panelState?.logs?.columns, + panelState?.logs?.displayedFields, + panelState?.logs?.refId, + visualisationType, + ] ); + useEffect(() => { + if (!prevLogsQueries) { + // Initial load, ignore + return; + } + if (!canKeepDisplayedFields(logsQueries, prevLogsQueries)) { + setDisplayedFields([]); + updatePanelState({ + ...panelState?.logs, + displayedFields: [], + }); + } + }, [logsQueries, panelState?.logs, prevLogsQueries, updatePanelState]); + // actions const onLogRowHover = useCallback( (row?: LogRowModel) => { @@ -559,20 +598,30 @@ const UnthemedLogs: React.FunctionComponent = (props: Props) => { const index = displayedFields.indexOf(key); if (index === -1) { - setDisplayedFields(displayedFields.concat(key)); + const updatedDisplayedFields = displayedFields.concat(key); + setDisplayedFields(updatedDisplayedFields); + updatePanelState({ + ...panelState?.logs, + displayedFields: updatedDisplayedFields, + }); } }, - [displayedFields] + [displayedFields, panelState?.logs, updatePanelState] ); const hideField = useCallback( (key: string) => { const index = displayedFields.indexOf(key); if (index > -1) { - setDisplayedFields(displayedFields.filter((k) => key !== k)); + const updatedDisplayedFields = displayedFields.filter((k) => key !== k); + setDisplayedFields(updatedDisplayedFields); + updatePanelState({ + ...panelState?.logs, + displayedFields: updatedDisplayedFields, + }); } }, - [displayedFields] + [displayedFields, panelState?.logs, updatePanelState] ); const clearDetectedFields = useCallback(() => { @@ -658,7 +707,7 @@ const UnthemedLogs: React.FunctionComponent = (props: Props) => { const urlState = getUrlStateFromPaneState(getState().explore.panes[exploreId]!); urlState.panelsState = { ...panelState, - logs: { id: row.uid, visualisationType: visualisationType ?? getDefaultVisualisationType() }, + logs: { id: row.uid, visualisationType: visualisationType ?? getDefaultVisualisationType(), displayedFields }, }; urlState.range = getPermalinkRange(row); diff --git a/public/app/features/explore/Logs/LogsMetaRow.tsx b/public/app/features/explore/Logs/LogsMetaRow.tsx index 26f59a2689a..abf22494946 100644 --- a/public/app/features/explore/Logs/LogsMetaRow.tsx +++ b/public/app/features/explore/Logs/LogsMetaRow.tsx @@ -139,7 +139,7 @@ export const LogsMetaRow = memo( { label: '', value: ( - ), diff --git a/public/app/features/explore/Logs/utils/logs.ts b/public/app/features/explore/Logs/utils/logs.ts index 2aec91da980..61e4f9ce5d5 100644 --- a/public/app/features/explore/Logs/utils/logs.ts +++ b/public/app/features/explore/Logs/utils/logs.ts @@ -1,3 +1,6 @@ +import { shallowCompare } from '@grafana/data'; +import { DataQuery } from '@grafana/schema'; + export const SETTINGS_KEYS = { showLabels: 'grafana.explore.logs.showLabels', showTime: 'grafana.explore.logs.showTime', @@ -8,3 +11,15 @@ export const SETTINGS_KEYS = { }; export const visualisationTypeKey = 'grafana.explore.logs.visualisationType'; + +export const canKeepDisplayedFields = (logsQueries: DataQuery[] | undefined, prevLogsQueries: DataQuery[]): boolean => { + if (!logsQueries) { + return false; + } + for (let i = 0; i < logsQueries.length; i++) { + if (!shallowCompare(logsQueries[i], prevLogsQueries[i])) { + return false; + } + } + return true; +}; diff --git a/public/app/features/logs/components/LogRow.tsx b/public/app/features/logs/components/LogRow.tsx index 686f49e19a4..6a59f32dbfc 100644 --- a/public/app/features/logs/components/LogRow.tsx +++ b/public/app/features/logs/components/LogRow.tsx @@ -299,6 +299,8 @@ class UnThemedLogRow extends PureComponent { pinned={this.props.pinned} mouseIsOver={this.state.mouseIsOver} onBlur={this.onMouseLeave} + logRowMenuIconsBefore={logRowMenuIconsBefore} + logRowMenuIconsAfter={logRowMenuIconsAfter} /> ) : ( { expect(screen.getByText(/place=Earth/)).toBeInTheDocument(); expect(screen.getByText(/planet=Mars/)).toBeInTheDocument(); }); + + describe('With custom buttons', () => { + it('supports custom buttons before and after the default options', async () => { + const onBefore = jest.fn(); + const logRowMenuIconsBefore = [ + , + ]; + const onAfter = jest.fn(); + const logRowMenuIconsAfter = [ + , + ]; + + const { row } = setup({ logRowMenuIconsBefore, logRowMenuIconsAfter }); + + await userEvent.click(screen.getByLabelText('Addon before')); + await userEvent.click(screen.getByLabelText('Addon after')); + + expect(onBefore).toHaveBeenCalledWith(expect.anything(), row); + expect(onAfter).toHaveBeenCalledWith(expect.anything(), row); + }); + }); }); diff --git a/public/app/features/logs/components/LogRowMessageDisplayedFields.tsx b/public/app/features/logs/components/LogRowMessageDisplayedFields.tsx index 5391dd0cce9..363fb97bb17 100644 --- a/public/app/features/logs/components/LogRowMessageDisplayedFields.tsx +++ b/public/app/features/logs/components/LogRowMessageDisplayedFields.tsx @@ -1,5 +1,5 @@ import { css } from '@emotion/css'; -import { memo, useMemo } from 'react'; +import { memo, ReactNode, useMemo } from 'react'; import { LogRowModel, Field, LinkModel, DataFrame } from '@grafana/data'; @@ -21,10 +21,23 @@ export interface Props { pinned?: boolean; mouseIsOver: boolean; onBlur: () => void; + logRowMenuIconsBefore?: ReactNode[]; + logRowMenuIconsAfter?: ReactNode[]; } export const LogRowMessageDisplayedFields = memo((props: Props) => { - const { row, detectedFields, getFieldLinks, wrapLogMessage, styles, mouseIsOver, pinned, ...rest } = props; + const { + row, + detectedFields, + getFieldLinks, + wrapLogMessage, + styles, + mouseIsOver, + pinned, + logRowMenuIconsBefore, + logRowMenuIconsAfter, + ...rest + } = props; const wrapClassName = wrapLogMessage ? '' : displayedFieldsStyles.noWrap; const fields = useMemo(() => getAllFields(row, getFieldLinks), [getFieldLinks, row]); // only single key/value rows are filterable, so we only need the first field key for filtering @@ -63,6 +76,8 @@ export const LogRowMessageDisplayedFields = memo((props: Props) => { styles={styles} pinned={pinned} mouseIsOver={mouseIsOver} + addonBefore={logRowMenuIconsBefore} + addonAfter={logRowMenuIconsAfter} {...rest} /> )}