New Log Details: Support multiple log details displayed (#108327)

* LogLine: optionally highlight search based on syntax highlighting state * Log Details: support multiple details open * Translations * Add test * LogLine: allow to highlight frontend searches * LogLine: improve hover vs details displayed colors * LogLineDetails: add scroll to log line option and improve toggling behavior * Translations * LogLineDetails: prevent sidebar re-rendering * LogLineDetails: more tests
2025-07-21 13:02:20 +02:00
parent ca63c8015d
commit b70d1b0724
7 changed files with 210 additions and 63 deletions
@@ -241,7 +241,7 @@ const LogLineComponent = memo(
            </Button>
          </div>
        )}
-        {detailsMode === 'inline' && detailsShown && <InlineLogLineDetails logs={logs} />}
+        {detailsMode === 'inline' && detailsShown && <InlineLogLineDetails logs={logs} log={log} />}
      </>
    );
  }
@@ -323,7 +323,7 @@ const LogLineBody = ({ log, styles }: { log: LogListModel; styles: LogLineStyles
  const { matchingUids, search } = useLogListSearchContext();

  const highlight = useMemo(() => {
-    const searchWords = log.searchWords && log.searchWords[0] ? log.searchWords.slice() : [];
+    const searchWords = syntaxHighlighting && log.searchWords && log.searchWords[0] ? log.searchWords.slice() : [];
    if (search && matchingUids?.includes(log.uid)) {
      searchWords.push(search);
    }
@@ -331,7 +331,7 @@ const LogLineBody = ({ log, styles }: { log: LogListModel; styles: LogLineStyles
      return undefined;
    }
    return { searchWords, highlightClassName: styles.matchHighLight };
-  }, [log.searchWords, log.uid, matchingUids, search, styles.matchHighLight]);
+  }, [log.searchWords, log.uid, matchingUids, search, styles.matchHighLight, syntaxHighlighting]);

  if (log.hasAnsi) {
    return (
@@ -376,7 +376,7 @@ export const getStyles = (theme: GrafanaTheme2, virtualization?: LogLineVirtuali
    parsedField: theme.colors.text.primary,
  };

-  const hoverColor = tinycolor(theme.colors.background.canvas).darken(4).toRgbString();
+  const hoverColor = tinycolor(theme.colors.background.canvas).darken(5).toRgbString();

  return {
    logLine: css({
@@ -450,7 +450,7 @@ export const getStyles = (theme: GrafanaTheme2, virtualization?: LogLineVirtuali
      lineHeight: theme.typography.bodySmall.lineHeight,
    }),
    detailsDisplayed: css({
-      background: hoverColor,
+      background: tinycolor(theme.colors.background.canvas).darken(2).toRgbString(),
    }),
    pinnedLogLine: css({
      backgroundColor: tinycolor(theme.colors.info.transparent).setAlpha(0.25).toString(),
@@ -515,4 +515,88 @@ describe('LogLineDetails', () => {
      expect(setDisplayedFields).toHaveBeenCalledWith(['key1', 'key3', 'key2']);
    });
  });
+
+  describe('Multiple log details', () => {
+    test('Does not render tabs when displaying a single log', () => {
+      setup(undefined, { labels: { key1: 'label1', key2: 'label2' } });
+      expect(screen.queryAllByRole('tab')).toHaveLength(0);
+    });
+
+    test('Renders multiple log details', async () => {
+      const logs = [
+        createLogLine({ uid: '1', logLevel: LogLevel.error, timeEpochMs: 1546297200000, entry: 'First log' }),
+        createLogLine({ uid: '2', logLevel: LogLevel.error, timeEpochMs: 1546297200000, entry: 'Second log' }),
+      ];
+      setup({ logs }, undefined, { showDetails: logs });
+
+      expect(screen.queryAllByRole('tab')).toHaveLength(2);
+
+      await userEvent.click(screen.getByText('Log line'));
+
+      expect(screen.getAllByText('First log')).toHaveLength(1);
+      expect(screen.getAllByText('Second log')).toHaveLength(2);
+
+      await userEvent.click(screen.queryAllByRole('tab')[0]);
+
+      expect(screen.getAllByText('First log')).toHaveLength(2);
+      expect(screen.getAllByText('Second log')).toHaveLength(1);
+    });
+
+    test('Changes details focus when logs are added and removed', async () => {
+      const logs = [
+        createLogLine({ uid: '1', logLevel: LogLevel.error, timeEpochMs: 1546297200000, entry: 'First log' }),
+        createLogLine({ uid: '2', logLevel: LogLevel.error, timeEpochMs: 1546297200000, entry: 'Second log' }),
+      ];
+
+      const props: Props = {
+        containerElement: document.createElement('div'),
+        focusLogLine: jest.fn(),
+        logs: [logs[0]],
+        onResize: jest.fn(),
+      };
+
+      const contextData: LogListContextData = {
+        ...defaultValue,
+        showDetails: [logs[0]],
+      };
+
+      const { rerender } = render(
+        <LogListContext.Provider value={contextData}>
+          <LogLineDetails {...props} />
+        </LogListContext.Provider>
+      );
+
+      expect(screen.queryAllByRole('tab')).toHaveLength(0);
+
+      await userEvent.click(screen.getByText('Log line'));
+      // Tab not displayed, only line body
+      expect(screen.getAllByText('First log')).toHaveLength(1);
+
+      contextData.showDetails = logs;
+      props.logs = logs;
+
+      rerender(
+        <LogListContext.Provider value={contextData}>
+          <LogLineDetails {...props} />
+        </LogListContext.Provider>
+      );
+
+      expect(screen.queryAllByRole('tab')).toHaveLength(2);
+      // Tab and log line body
+      expect(screen.getAllByText('Second log')).toHaveLength(2);
+
+      contextData.showDetails = [logs[1]];
+      props.logs = [logs[1]];
+
+      rerender(
+        <LogListContext.Provider value={contextData}>
+          <LogLineDetails {...props} />
+        </LogListContext.Provider>
+      );
+
+      expect(screen.queryAllByRole('tab')).toHaveLength(0);
+      // Tab not displayed, only line body
+      expect(screen.getAllByText('Second log')).toHaveLength(1);
+    });
+  });
 });
@@ -1,10 +1,12 @@
 import { css } from '@emotion/css';
 import { Resizable } from 're-resizable';
-import { memo, useCallback, useEffect, useRef } from 'react';
+import { memo, useCallback, useEffect, useRef, useState } from 'react';
+import { usePrevious } from 'react-use';

 import { GrafanaTheme2 } from '@grafana/data';
+import { t } from '@grafana/i18n';
 import { reportInteraction } from '@grafana/runtime';
-import { getDragStyles, useStyles2 } from '@grafana/ui';
+import { getDragStyles, Icon, Tab, TabsBar, useStyles2 } from '@grafana/ui';

 import { LogLineDetailsComponent } from './LogLineDetailsComponent';
 import { getDetailsScrollPosition, saveDetailsScrollPosition, useLogListContext } from './LogListContext';
@@ -20,23 +22,12 @@ export interface Props {

 export type LogLineDetailsMode = 'inline' | 'sidebar';

-export const LogLineDetails = ({ containerElement, focusLogLine, logs, onResize }: Props) => {
-  const { detailsWidth, noInteractions, setDetailsWidth, showDetails } = useLogListContext();
+export const LogLineDetails = memo(({ containerElement, focusLogLine, logs, onResize }: Props) => {
+  const { detailsWidth, noInteractions, setDetailsWidth } = useLogListContext();
  const styles = useStyles2(getStyles, 'sidebar');
  const dragStyles = useStyles2(getDragStyles);
  const containerRef = useRef<HTMLDivElement | null>(null);

-  useEffect(() => {
-    focusLogLine(showDetails[0]);
-    if (!noInteractions) {
-      reportInteraction('logs_log_line_details_displayed', {
-        mode: 'sidebar',
-      });
-    }
-    // Just once
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, []);
-
  const handleResize = useCallback(() => {
    if (containerRef.current) {
      setDetailsWidth(containerRef.current.clientWidth);
@@ -54,10 +45,6 @@ export const LogLineDetails = ({ containerElement, focusLogLine, logs, onResize

  const maxWidth = containerElement.clientWidth - LOG_LIST_MIN_WIDTH;

-  if (!showDetails.length) {
-    return null;
-  }
-
  return (
    <Resizable
      onResize={handleResize}
@@ -70,20 +57,83 @@ export const LogLineDetails = ({ containerElement, focusLogLine, logs, onResize
      maxWidth={maxWidth}
    >
      <div className={styles.container} ref={containerRef}>
-        <div className={styles.scrollContainer}>
-          <LogLineDetailsComponent log={showDetails[0]} logs={logs} />
-        </div>
+        <LogLineDetailsTabs focusLogLine={focusLogLine} logs={logs} />
      </div>
    </Resizable>
  );
-};
+});
+LogLineDetails.displayName = 'LogLineDetails';
+
+const LogLineDetailsTabs = memo(({ focusLogLine, logs }: Pick<Props, 'focusLogLine' | 'logs'>) => {
+  const { closeDetails, noInteractions, showDetails, toggleDetails } = useLogListContext();
+  const [currentLog, setCurrentLog] = useState(showDetails[0]);
+  const previousShowDetails = usePrevious(showDetails);
+  const styles = useStyles2(getStyles, 'sidebar');
+
+  useEffect(() => {
+    focusLogLine(currentLog);
+    if (!noInteractions) {
+      reportInteraction('logs_log_line_details_displayed', {
+        mode: 'sidebar',
+      });
+    }
+    // Once
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, []);
+
+  useEffect(() => {
+    if (!showDetails.length) {
+      closeDetails();
+      return;
+    }
+    // Focus on the recently open
+    if (!previousShowDetails || showDetails.length > previousShowDetails.length) {
+      setCurrentLog(showDetails[showDetails.length - 1]);
+      return;
+    } else if (!showDetails.find((log) => log.uid === currentLog.uid)) {
+      setCurrentLog(showDetails[showDetails.length - 1]);
+    }
+  }, [closeDetails, currentLog.uid, previousShowDetails, showDetails]);
+
+  return (
+    <>
+      {showDetails.length > 1 && (
+        <TabsBar>
+          {showDetails.map((log) => {
+            return (
+              <Tab
+                key={log.uid}
+                truncate
+                label={log.entry.substring(0, 25)}
+                active={currentLog.uid === log.uid}
+                onChangeTab={() => setCurrentLog(log)}
+                suffix={() => (
+                  <Icon
+                    name="times"
+                    aria-label={t('logs.log-line-details.remove-log', 'Remove log')}
+                    onClick={() => toggleDetails(log)}
+                  />
+                )}
+              />
+            );
+          })}
+        </TabsBar>
+      )}
+      <div className={styles.scrollContainer}>
+        <LogLineDetailsComponent focusLogLine={focusLogLine} log={currentLog} logs={logs} />
+      </div>
+    </>
+  );
+});
+LogLineDetailsTabs.displayName = 'LogLineDetailsTabs';

 export interface InlineLogLineDetailsProps {
+  log: LogListModel;
  logs: LogListModel[];
 }

-export const InlineLogLineDetails = memo(({ logs }: InlineLogLineDetailsProps) => {
-  const { noInteractions, showDetails } = useLogListContext();
+export const InlineLogLineDetails = memo(({ logs, log }: InlineLogLineDetailsProps) => {
+  const { noInteractions } = useLogListContext();
  const styles = useStyles2(getStyles, 'inline');
  const scrollRef = useRef<HTMLDivElement | null>(null);

@@ -96,25 +146,21 @@ export const InlineLogLineDetails = memo(({ logs }: InlineLogLineDetailsProps) =
  }, [noInteractions]);

  const saveScroll = useCallback(() => {
-    saveDetailsScrollPosition(showDetails[0], scrollRef.current?.scrollTop ?? 0);
-  }, [showDetails]);
+    saveDetailsScrollPosition(log, scrollRef.current?.scrollTop ?? 0);
+  }, [log]);

  useEffect(() => {
    if (!scrollRef.current) {
      return;
    }
-    scrollRef.current.scrollTop = getDetailsScrollPosition(showDetails[0]);
-  }, [showDetails]);
-
-  if (!showDetails.length) {
-    return null;
-  }
+    scrollRef.current.scrollTop = getDetailsScrollPosition(log);
+  }, [log]);

  return (
    <div className={`${styles.inlineWrapper} log-line-inline-details`}>
      <div className={styles.container}>
        <div className={styles.scrollContainer} ref={scrollRef} onScroll={saveScroll}>
-          <LogLineDetailsComponent log={showDetails[0]} logs={logs} />
+          <LogLineDetailsComponent log={log} logs={logs} />
        </div>
      </div>
    </div>
@@ -19,11 +19,12 @@ import { useLogListContext } from './LogListContext';
 import { LogListModel } from './processing';

 interface LogLineDetailsComponentProps {
+  focusLogLine?: (log: LogListModel) => void;
  log: LogListModel;
  logs: LogListModel[];
 }

-export const LogLineDetailsComponent = memo(({ log, logs }: LogLineDetailsComponentProps) => {
+export const LogLineDetailsComponent = memo(({ focusLogLine, log, logs }: LogLineDetailsComponentProps) => {
  const { displayedFields, noInteractions, logOptionsStorageKey, setDisplayedFields } = useLogListContext();
  const [search, setSearch] = useState('');
  const inputRef = useRef('');
@@ -101,7 +102,7 @@ export const LogLineDetailsComponent = memo(({ log, logs }: LogLineDetailsCompon

  return (
    <>
-      <LogLineDetailsHeader log={log} search={search} onSearch={handleSearch} />
+      <LogLineDetailsHeader focusLogLine={focusLogLine} log={log} search={search} onSearch={handleSearch} />
      <div className={styles.componentWrapper}>
        <ControlledCollapse
          className={styles.collapsable}
@@ -14,12 +14,13 @@ import { useLogIsPinned, useLogListContext } from './LogListContext';
 import { LogListModel } from './processing';

 interface Props {
+  focusLogLine?: (log: LogListModel) => void;
  log: LogListModel;
  search: string;
  onSearch(newSearch: string): void;
 }

-export const LogLineDetailsHeader = ({ log, search, onSearch }: Props) => {
+export const LogLineDetailsHeader = ({ focusLogLine, log, search, onSearch }: Props) => {
  const {
    closeDetails,
    detailsMode,
@@ -53,6 +54,10 @@ export const LogLineDetailsHeader = ({ log, search, onSearch }: Props) => {
    [noInteractions]
  );

+  const scrollToLogLine = useCallback(() => {
+    focusLogLine?.(log);
+  }, [focusLogLine, log]);
+
  const copyLogLine = useCallback(() => {
    copyText(log.entry, containerRef);
    reportInteractionWrapper('logs_log_line_details_header_copy_clicked');
@@ -143,22 +148,32 @@ export const LogLineDetailsHeader = ({ log, search, onSearch }: Props) => {
        ref={inputRef}
        suffix={search !== '' ? clearSearch : undefined}
      />
-      {showLogLineToggle && (
-        <IconButton
-          tooltip={
-            logLineDisplayed
-              ? t('logs.log-line-details.hide-log-line', 'Hide log line')
-              : t('logs.log-line-details.show-log-line', 'Show log line')
-          }
-          tooltipPlacement="top"
-          size="md"
-          name="eye"
-          onClick={toggleLogLine}
-          tabIndex={0}
-          variant={logLineDisplayed ? 'primary' : undefined}
-        />
-      )}
      <div className={styles.icons}>
+        {focusLogLine && (
+          <IconButton
+            tooltip={t('logs.log-line-details.scroll-to-logline', 'Scroll to log line')}
+            tooltipPlacement="top"
+            size="md"
+            name="arrows-v"
+            onClick={scrollToLogLine}
+            tabIndex={0}
+          />
+        )}
+        {showLogLineToggle && (
+          <IconButton
+            tooltip={
+              logLineDisplayed
+                ? t('logs.log-line-details.hide-log-line', 'Hide log line')
+                : t('logs.log-line-details.show-log-line', 'Show log line')
+            }
+            tooltipPlacement="top"
+            size="md"
+            name="eye"
+            onClick={toggleLogLine}
+            tabIndex={0}
+            variant={logLineDisplayed ? 'primary' : undefined}
+          />
+        )}
        <IconButton
          tooltip={t('logs.log-line-details.copy-to-clipboard', 'Copy to clipboard')}
          tooltipPlacement="top"
@@ -466,9 +466,7 @@ export const LogListContextProvider = ({
  );

  const closeDetails = useCallback(() => {
-    if (showDetails.length) {
-      removeDetailsScrollPosition(showDetails[0]);
-    }
+    showDetails.forEach((log) => removeDetailsScrollPosition(log));
    setShowDetails([]);
  }, [showDetails]);

@@ -477,12 +475,13 @@ export const LogListContextProvider = ({
      if (!enableLogDetails) {
        return;
      }
-      const found = showDetails.findIndex((stateLog) => stateLog === log || stateLog.uid === log.uid);
-      if (found >= 0) {
+      const found = showDetails.find((stateLog) => stateLog === log || stateLog.uid === log.uid);
+      if (found) {
+        removeDetailsScrollPosition(found);
        setShowDetails(showDetails.filter((stateLog) => stateLog !== log && stateLog.uid !== log.uid));
      } else {
        // Supporting one displayed details for now
-        setShowDetails([log]);
+        setShowDetails([...showDetails, log]);
      }
    },
    [enableLogDetails, showDetails]
@@ -8912,6 +8912,8 @@
      "no-details": "No fields to display.",
      "pin-line": "Pin log",
      "remove-displayed-field": "Remove field",
+      "remove-log": "Remove log",
+      "scroll-to-logline": "Scroll to log line",
      "search": {
        "no-results": "No results to display."
      },