From 9a6b012ea400678eed014b64459285d062d0fd0b Mon Sep 17 00:00:00 2001 From: Matias Chomicki Date: Mon, 18 Aug 2025 21:06:31 +0200 Subject: [PATCH] New Logs Panel: Performance and display updates and fixes (#109672) * LogLine: don't run overflow check with unwrapped logs * InfiniteScroll: don't listen to scroll events without scroll * InfiniteScroll: minimize use of element.scrollHeight * LogLineDetails: constrain to 95vw * LogList: process logs while streaming * Remove unused prop * InfiniteScroll: update test * Update test * Inline Log Details: read max width from context * Remove console --- .../logs/components/InfiniteScroll.test.tsx | 15 ++++---- .../logs/components/InfiniteScroll.tsx | 7 ++-- .../components/panel/InfiniteScroll.test.tsx | 13 +++---- .../logs/components/panel/InfiniteScroll.tsx | 8 +++-- .../logs/components/panel/LogLine.tsx | 2 +- .../logs/components/panel/LogLineDetails.tsx | 4 +-- .../logs/components/panel/LogList.tsx | 8 ++--- .../logs/components/panel/LogListContext.tsx | 35 +++++++++++++------ .../logs/components/panel/virtualization.ts | 2 ++ 9 files changed, 56 insertions(+), 38 deletions(-) diff --git a/public/app/features/logs/components/InfiniteScroll.test.tsx b/public/app/features/logs/components/InfiniteScroll.test.tsx index 249ed391185..cf62687b36d 100644 --- a/public/app/features/logs/components/InfiniteScroll.test.tsx +++ b/public/app/features/logs/components/InfiniteScroll.test.tsx @@ -57,10 +57,9 @@ function setup( startPosition: number, rows: LogRowModel[], order: LogsSortOrder, - app?: CoreApp + app?: CoreApp, + { element, events } = getMockElement(startPosition) ) { - const { element, events } = getMockElement(startPosition); - function scrollTo(position: number, timeStamp?: number) { element.scrollTop = position; @@ -69,7 +68,7 @@ function setup( if (timeStamp) { jest.spyOn(event, 'timeStamp', 'get').mockReturnValue(timeStamp); } - events['scroll'](event); + events['scroll']?.(event); }); // When scrolling top, we wait for the user to reach the top, and then for a new scrolling event @@ -192,12 +191,14 @@ describe('InfiniteScroll', () => { test('Does not request more logs when there is no scroll', async () => { const loadMoreMock = jest.fn(); - const { scrollTo, element } = setup(loadMoreMock, 0, rows, order); - - expect(await screen.findByTestId('contents')).toBeInTheDocument(); + const { element, events } = getMockElement(0); element.clientHeight = 40; element.scrollHeight = element.clientHeight; + const { scrollTo } = setup(loadMoreMock, 0, rows, order, undefined, { element, events }); + + expect(await screen.findByTestId('contents')).toBeInTheDocument(); + scrollTo(39, 1); scrollTo(40, 600); diff --git a/public/app/features/logs/components/InfiniteScroll.tsx b/public/app/features/logs/components/InfiniteScroll.tsx index 90f0919ca7d..e21ca4a4447 100644 --- a/public/app/features/logs/components/InfiniteScroll.tsx +++ b/public/app/features/logs/components/InfiniteScroll.tsx @@ -81,6 +81,9 @@ export const InfiniteScroll = ({ if (!scrollElement || !loadMoreLogs) { return; } + if (scrollElement.scrollHeight <= scrollElement.clientHeight) { + return; + } function handleScroll(event: Event | WheelEvent) { if (!scrollElement || !loadMoreLogs || !rows.length || loading || !config.featureToggles.logsInfiniteScrolling) { @@ -227,10 +230,6 @@ export function shouldLoadMore( element: HTMLDivElement, lastScroll: number ): ScrollDirection { - // Disable behavior if there is no scroll - if (element.scrollHeight <= element.clientHeight) { - return ScrollDirection.NoScroll; - } const delta = event instanceof WheelEvent ? event.deltaY : element.scrollTop - lastScroll; if (delta === 0) { return ScrollDirection.NoScroll; diff --git a/public/app/features/logs/components/panel/InfiniteScroll.test.tsx b/public/app/features/logs/components/panel/InfiniteScroll.test.tsx index d981cfa5027..631c2831e9c 100644 --- a/public/app/features/logs/components/panel/InfiniteScroll.test.tsx +++ b/public/app/features/logs/components/panel/InfiniteScroll.test.tsx @@ -46,10 +46,9 @@ function setup( startPosition: number, logs: LogListModel[], order: LogsSortOrder, - infiniteScrollMode: InfiniteScrollMode = 'interval' + infiniteScrollMode: InfiniteScrollMode = 'interval', + { element, events } = getMockElement(startPosition) ) { - const { element, events } = getMockElement(startPosition); - function scrollTo(position: number, timeStamp?: number) { element.scrollTop = position; @@ -168,12 +167,14 @@ describe('InfiniteScroll', () => { test('Does not request more logs when there is no scroll', async () => { const loadMoreMock = jest.fn(); - const { scrollTo, element } = setup(loadMoreMock, 0, logs, order); - - expect(await screen.findByText('log line 1')).toBeInTheDocument(); + const { element, events } = getMockElement(0); element.clientHeight = 40; element.scrollHeight = element.clientHeight; + const { scrollTo } = setup(loadMoreMock, 0, logs, order, undefined, { element, events }); + + expect(await screen.findByText('log line 1')).toBeInTheDocument(); + scrollTo(39, 1); scrollTo(40, 600); diff --git a/public/app/features/logs/components/panel/InfiniteScroll.tsx b/public/app/features/logs/components/panel/InfiniteScroll.tsx index 6b64bc7e209..6dff87f65e9 100644 --- a/public/app/features/logs/components/panel/InfiniteScroll.tsx +++ b/public/app/features/logs/components/panel/InfiniteScroll.tsx @@ -73,6 +73,7 @@ export const InfiniteScroll = ({ const styles = useStyles2(getStyles, virtualization); const resetStateTimeout = useRef | null>(null); const scrollToLogLineRef = useRef(undefined); + const noScrollRef = useRef(false); useEffect(() => { // Logs have not changed, ignore effect @@ -237,10 +238,13 @@ export const InfiniteScroll = ({ const onItemsRendered = useCallback( (props: ListOnItemsRenderedProps) => { - if (!scrollElement || infiniteLoaderState === 'loading' || infiniteLoaderState === 'out-of-bounds') { + if (!scrollElement) { return; } - if (scrollElement.scrollHeight <= scrollElement.clientHeight) { + if (props.visibleStartIndex === 0) { + noScrollRef.current = scrollElement.scrollHeight <= scrollElement.clientHeight; + } + if (noScrollRef.current || infiniteLoaderState === 'loading' || infiniteLoaderState === 'out-of-bounds') { return; } const lastLogIndex = logs.length - 1; diff --git a/public/app/features/logs/components/panel/LogLine.tsx b/public/app/features/logs/components/panel/LogLine.tsx index 72934c8cffd..0fe2865e6c1 100644 --- a/public/app/features/logs/components/panel/LogLine.tsx +++ b/public/app/features/logs/components/panel/LogLine.tsx @@ -110,7 +110,7 @@ const LogLineComponent = memo( const permalinked = useLogIsPermalinked(log); useEffect(() => { - if (!onOverflow || !logLineRef.current || !virtualization || !height) { + if (!onOverflow || !logLineRef.current || !virtualization || !height || !wrapLogMessage) { return; } const calculatedHeight = typeof height === 'number' ? height : undefined; diff --git a/public/app/features/logs/components/panel/LogLineDetails.tsx b/public/app/features/logs/components/panel/LogLineDetails.tsx index 3887c9dda4b..b2121c64d55 100644 --- a/public/app/features/logs/components/panel/LogLineDetails.tsx +++ b/public/app/features/logs/components/panel/LogLineDetails.tsx @@ -134,7 +134,7 @@ export interface InlineLogLineDetailsProps { } export const InlineLogLineDetails = memo(({ logs, log }: InlineLogLineDetailsProps) => { - const { app, noInteractions } = useLogListContext(); + const { app, detailsWidth, noInteractions } = useLogListContext(); const styles = useStyles2(getStyles, 'inline'); const scrollRef = useRef(null); @@ -159,7 +159,7 @@ export const InlineLogLineDetails = memo(({ logs, log }: InlineLogLineDetailsPro }, [log]); return ( -
+
diff --git a/public/app/features/logs/components/panel/LogList.tsx b/public/app/features/logs/components/panel/LogList.tsx index f3388cdf82f..1fbc0b1fd6e 100644 --- a/public/app/features/logs/components/panel/LogList.tsx +++ b/public/app/features/logs/components/panel/LogList.tsx @@ -96,6 +96,7 @@ type LogListComponentProps = Omit< | 'dedupStrategy' | 'displayedFields' | 'enableLogDetails' + | 'loading' | 'logOptionsStorageKey' | 'permalinkedLogId' | 'showTime' @@ -202,7 +203,6 @@ export const LogList = ({ grammar={grammar} initialScrollPosition={initialScrollPosition} infiniteScrollMode={infiniteScrollMode} - loading={loading} loadMore={loadMore} logs={logs} showControls={showControls} @@ -221,7 +221,6 @@ const LogListComponent = ({ grammar, initialScrollPosition = 'top', infiniteScrollMode = 'interval', - loading, loadMore, logs, showControls, @@ -312,9 +311,6 @@ const LogListComponent = ({ }, [eventBus, filteredLogs]); useEffect(() => { - if (loading) { - return; - } setProcessedLogs( preProcessLogs( logs, @@ -324,7 +320,7 @@ const LogListComponent = ({ ); virtualization.resetLogLineSizes(); listRef.current?.resetAfterIndex(0); - }, [forceEscape, getFieldLinks, grammar, loading, logs, sortOrder, timeZone, virtualization, wrapLogMessage]); + }, [forceEscape, getFieldLinks, grammar, logs, sortOrder, timeZone, virtualization, wrapLogMessage]); useEffect(() => { listRef.current?.resetAfterIndex(0); diff --git a/public/app/features/logs/components/panel/LogListContext.tsx b/public/app/features/logs/components/panel/LogListContext.tsx index c302a510761..711cda9dbe2 100644 --- a/public/app/features/logs/components/panel/LogListContext.tsx +++ b/public/app/features/logs/components/panel/LogListContext.tsx @@ -40,7 +40,7 @@ import { LogLineDetailsMode } from './LogLineDetails'; import { GetRowContextQueryFn, LogLineMenuCustomItem } from './LogLineMenu'; import { LogListFontSize } from './LogList'; import { LogListModel } from './processing'; -import { LOG_LIST_MIN_WIDTH } from './virtualization'; +import { getScrollbarWidth, LOG_LIST_CONTROLS_WIDTH, LOG_LIST_MIN_WIDTH } from './virtualization'; export interface LogListContextData extends Omit { closeDetails: () => void; @@ -259,7 +259,9 @@ export const LogListContextProvider = ({ wrapLogMessage, }); const [showDetails, setShowDetails] = useState([]); - const [detailsWidth, setDetailsWidthState] = useState(getDetailsWidth(containerElement, logOptionsStorageKey)); + const [detailsWidth, setDetailsWidthState] = useState( + getDetailsWidth(containerElement, logOptionsStorageKey, undefined, detailsModeProp, showControls) + ); const [detailsMode, setDetailsMode] = useState(detailsModeProp ?? 'sidebar'); const [isAssistantAvailable, openAssistant] = useAssistant(); @@ -361,17 +363,25 @@ export const LogListContextProvider = ({ } }, [logs, showDetails]); + // Sync log details inline and sidebar width + useEffect(() => { + setDetailsWidthState(getDetailsWidth(containerElement, logOptionsStorageKey, undefined, detailsMode, showControls)); + }, [containerElement, detailsMode, logOptionsStorageKey, showControls]); + // Sync log details width useEffect(() => { + if (!containerElement) { + return; + } const handleResize = debounce(() => { - setDetailsWidthState((detailsWidth) => getDetailsWidth(containerElement, logOptionsStorageKey, detailsWidth)); + setDetailsWidthState((detailsWidth) => + getDetailsWidth(containerElement, logOptionsStorageKey, detailsWidth, detailsMode, showControls) + ); }, 50); - handleResize(); - window.addEventListener('resize', handleResize); - return () => { - window.removeEventListener('resize', handleResize); - }; - }, [containerElement, logOptionsStorageKey]); + const observer = new ResizeObserver(() => handleResize()); + observer.observe(containerElement); + return () => observer.disconnect(); + }, [containerElement, detailsMode, logOptionsStorageKey, showControls]); // Sync timestamp resolution useEffect(() => { @@ -665,11 +675,16 @@ export function isDedupStrategy(value: unknown): value is LogsDedupStrategy { function getDetailsWidth( containerElement: HTMLDivElement | undefined, logOptionsStorageKey?: string, - currentWidth?: number + currentWidth?: number, + detailsMode: LogLineDetailsMode = 'sidebar', + showControls?: boolean ) { if (!containerElement) { return 0; } + if (detailsMode === 'inline') { + return containerElement.clientWidth - getScrollbarWidth() - (showControls ? LOG_LIST_CONTROLS_WIDTH : 0); + } const defaultWidth = containerElement.clientWidth * 0.4; const detailsWidth = currentWidth || diff --git a/public/app/features/logs/components/panel/virtualization.ts b/public/app/features/logs/components/panel/virtualization.ts index 15889c1c2af..52835fbb8c4 100644 --- a/public/app/features/logs/components/panel/virtualization.ts +++ b/public/app/features/logs/components/panel/virtualization.ts @@ -13,6 +13,8 @@ export const FIELD_GAP_MULTIPLIER = 1.5; export const DEFAULT_LINE_HEIGHT = 22; +export const LOG_LIST_CONTROLS_WIDTH = 32; + export class LogLineVirtualization { private ctx: CanvasRenderingContext2D | null = null; private gridSize;