Trace view: Improved span details view (#108101)

* Details view improvents * Update tests * Label update * Prettier * Improve span tree offset
2025-07-17 11:33:27 +01:00
parent 4b94009a10
commit 47ce658154
9 changed files with 167 additions and 138 deletions
@@ -96,9 +96,7 @@ describe('AccordianKeyValues test', () => {
  it('renders the summary instead of the table when it is not expanded', () => {
    setupAccordian({ isOpen: false } as AccordianKeyValuesProps);

-    expect(
-      screen.getByRole('switch', { name: 'test accordian: span.kind = client omg = mos-def' })
-    ).toBeInTheDocument();
+    expect(screen.getByRole('switch', { name: 'test accordian: span.kind client omg mos-def' })).toBeInTheDocument();
    expect(screen.queryByRole('table')).not.toBeInTheDocument();
    expect(screen.queryAllByRole('cell')).toHaveLength(0);
  });
@@ -68,7 +68,6 @@ export const getStyles = (theme: GrafanaTheme2) => {
      label: 'summaryItem',
      display: 'inline',
      paddingRight: '0.5rem',
-      borderRight: `1px solid ${autoColor(theme, '#ddd')}`,
      '&:last-child': {
        paddingRight: 0,
        borderRight: 'none',
@@ -77,11 +76,7 @@ export const getStyles = (theme: GrafanaTheme2) => {
    summaryLabel: css({
      label: 'summaryLabel',
      color: autoColor(theme, '#777'),
-    }),
-    summaryDelim: css({
-      label: 'summaryDelim',
-      color: autoColor(theme, '#bbb'),
-      padding: '0 0.2em',
+      paddingRight: '0.5rem',
    }),
  };
 };
@@ -116,7 +111,6 @@ export function KeyValuesSummary({ data = null }: KeyValuesSummaryProps) {
        // `i` is necessary in the key because item.key can repeat
        <li className={styles.summaryItem} key={`${item.key}-${i}`}>
          <span className={styles.summaryLabel}>{item.key}</span>
-          <span className={styles.summaryDelim}>=</span>
          {String(item.value)}
        </li>
      ))}
@@ -56,7 +56,7 @@ describe('AccordianLogs tests', () => {
  it('shows the number of log entries', () => {
    setup();

-    expect(screen.getByRole('switch', { name: 'Events (2)' })).toBeInTheDocument();
+    expect(screen.getByRole('switch', { name: 'Events 2' })).toBeInTheDocument();
  });

  it('hides log entries when not expanded', () => {
@@ -103,12 +103,10 @@ describe('AccordianLogs tests', () => {
    setup({ isOpen: true, openedItems: new Set() } as AccordianLogsProps);
    expect(
      screen.getByRole('switch', {
-        name: '15μs (foo event name) : message = oh the next log message more = stuff',
+        name: '15μs (foo event name) : message oh the next log message more stuff',
      })
    ).toBeInTheDocument();
-    expect(
-      screen.getByRole('switch', { name: '5μs: message = oh the log message something = else' })
-    ).toBeInTheDocument();
+    expect(screen.getByRole('switch', { name: '5μs: message oh the log message something else' })).toBeInTheDocument();
  });

  it('renders event name and duration when events list is open', () => {
@@ -18,7 +18,7 @@ import * as React from 'react';

 import { GrafanaTheme2, TraceLog } from '@grafana/data';
 import { Trans } from '@grafana/i18n';
-import { Icon, useStyles2 } from '@grafana/ui';
+import { Counter, Icon, useStyles2 } from '@grafana/ui';

 import { autoColor } from '../../Theme';
 import { formatDuration } from '../utils';
@@ -31,24 +31,22 @@ const getStyles = (theme: GrafanaTheme2) => {
  return {
    AccordianLogs: css({
      label: 'AccordianLogs',
-      border: `1px solid ${autoColor(theme, '#d8d8d8')}`,
      position: 'relative',
      marginBottom: '0.25rem',
    }),
    AccordianLogsHeader: css({
      label: 'AccordianLogsHeader',
-      background: autoColor(theme, '#e4e4e4'),
      color: 'inherit',
-      display: 'block',
-      padding: '0.25rem 0.5rem',
+      display: 'flex',
+      alignItems: 'center',
+      padding: '0.25rem 0.1em',
      '&:hover': {
-        background: autoColor(theme, '#dadada'),
+        background: autoColor(theme, '#e8e8e8'),
      },
    }),
    AccordianLogsContent: css({
      label: 'AccordianLogsContent',
      background: autoColor(theme, '#f0f0f0'),
-      borderTop: `1px solid ${autoColor(theme, '#d8d8d8')}`,
      padding: '0.5rem 0.5rem 0.25rem 0.5rem',
    }),
    AccordianLogsFooter: css({
@@ -90,7 +88,7 @@ export default function AccordianLogs({
    arrow = isOpen ? (
      <Icon name={'angle-down'} className={alignIcon} />
    ) : (
-      <Icon name={'angle-right'} className="u-align-icon" />
+      <Icon name={'angle-right'} className="u-align-icon" style={{ margin: '0 0.25rem 0 0' }} />
    );
    HeaderComponent = 'a';
    headerProps = {
@@ -108,7 +106,7 @@ export default function AccordianLogs({
        <strong>
          <Trans i18nKey="explore.accordian-logs.events">Events</Trans>
        </strong>{' '}
-        ({logs.length})
+        <Counter value={logs.length} variant="secondary" />
      </HeaderComponent>
      {isOpen && (
        <div className={styles.AccordianLogsContent}>
@@ -28,12 +28,13 @@ import {
  TraceLog,
  PluginExtensionResourceAttributesContext,
  PluginExtensionPoints,
+  IconName,
 } from '@grafana/data';
 import { Trans, t } from '@grafana/i18n';
 import { TraceToProfilesOptions } from '@grafana/o11y-ds-frontend';
 import { usePluginLinks } from '@grafana/runtime';
 import { TimeZone } from '@grafana/schema';
-import { Divider, Icon, TextArea, useStyles2 } from '@grafana/ui';
+import { Icon, TextArea, useStyles2 } from '@grafana/ui';

 import { pyroscopeProfileIdTagKey } from '../../../createSpanLink';
 import { autoColor } from '../../Theme';
@@ -159,9 +160,6 @@ const getStyles = (theme: GrafanaTheme2) => {
      label: 'AccordianWarningsLabel',
      color: autoColor(theme, '#d36c08'),
    }),
-    AccordianKeyValuesItem: css({
-      marginBottom: theme.spacing(0.5),
-    }),
    Textarea: css({
      wordBreak: 'break-all',
      whiteSpace: 'pre',
@@ -173,6 +171,7 @@ const getStyles = (theme: GrafanaTheme2) => {
      display: 'flex',
      flexWrap: 'wrap',
      gap: '10px',
+      marginBottom: theme.spacing(2),
    }),
  };
 };
@@ -186,6 +185,7 @@ export type TraceFlameGraphs = {
 };

 export type SpanDetailProps = {
+  color: string;
  detailState: DetailState;
  logItemToggle: (spanID: string, log: TraceLog) => void;
  logsToggle: (spanID: string) => void;
@@ -215,6 +215,7 @@ export type SpanDetailProps = {

 export default function SpanDetail(props: SpanDetailProps) {
  const {
+    color,
    detailState,
    logItemToggle,
    logsToggle,
@@ -263,6 +264,9 @@ export default function SpanDetail(props: SpanDetailProps) {
  } = span;

  const { timeZone } = props;
+  const durationIcon: IconName = 'hourglass';
+  const startIcon: IconName = 'clock-nine';
+
  let overviewItems = [
    {
      key: 'svc',
@@ -273,11 +277,13 @@ export default function SpanDetail(props: SpanDetailProps) {
      key: 'duration',
      label: t('explore.span-detail.overview-items.label.duration', 'Duration:'),
      value: formatDuration(duration),
+      icon: durationIcon,
    },
    {
      key: 'start',
      label: t('explore.span-detail.overview-items.label.start-time', 'Start Time:'),
      value: formatDuration(relativeStartTime) + getAbsoluteTime(startTime, timeZone),
+      icon: startIcon,
    },
    ...(span.childSpanCount > 0
      ? [
@@ -353,11 +359,10 @@ export default function SpanDetail(props: SpanDetailProps) {
          {operationName}
        </h2>
        <div className={styles.listWrapper}>
-          <LabeledList className={styles.list} divider={true} items={overviewItems} />
+          <LabeledList className={styles.list} divider={false} items={overviewItems} color={color} />
        </div>
      </div>
      <div className={styles.linkList}>{linksComponent}</div>
-      <Divider spacing={1} />
      <div>
        <div>
          <AccordianKeyValues
@@ -368,7 +373,6 @@ export default function SpanDetail(props: SpanDetailProps) {
          />
          {process.tags && (
            <AccordianKeyValues
-              className={styles.AccordianKeyValuesItem}
              data={process.tags}
              label={t('explore.span-detail.label-resource-attributes', 'Resource attributes')}
              linksGetter={resourceLinksGetter}
@@ -13,7 +13,6 @@
 // limitations under the License.

 import { render, screen } from '@testing-library/react';
-import userEvent from '@testing-library/user-event';

 import { createTheme } from '@grafana/data';
 import { setPluginLinksHook } from '@grafana/runtime';
@@ -71,22 +70,6 @@ describe('SpanDetailRow tests', () => {
    expect(() => setup()).not.toThrow();
  });

-  it('calls toggle on click', async () => {
-    const mockToggle = jest.fn();
-    setup({ onDetailToggled: mockToggle } as unknown as SpanDetailRowProps);
-    expect(mockToggle).not.toHaveBeenCalled();
-
-    const detailRow = screen.getByTestId('detail-row-expanded-accent');
-    await userEvent.click(detailRow);
-    expect(mockToggle).toHaveBeenCalled();
-  });
-
-  it('renders the span tree offset', () => {
-    setup();
-
-    expect(screen.getByTestId('SpanTreeOffset--indentGuide')).toBeInTheDocument();
-  });
-
  it('renders the SpanDetail', () => {
    setup();

@@ -13,15 +13,13 @@
 // limitations under the License.

 import { css } from '@emotion/css';
-import classNames from 'classnames';
 import { PureComponent } from 'react';

 import { CoreApp, GrafanaTheme2, LinkModel, TimeRange, TraceLog } from '@grafana/data';
 import { TraceToProfilesOptions } from '@grafana/o11y-ds-frontend';
 import { TimeZone } from '@grafana/schema';
-import { Button, clearButtonStyles, stylesFactory, withTheme2 } from '@grafana/ui';
+import { stylesFactory, withTheme2 } from '@grafana/ui';

-import { autoColor } from '../Theme';
 import { SpanLinkFunc } from '../types/links';
 import { TraceSpan, TraceSpanReference } from '../types/trace';

@@ -64,10 +62,22 @@ const getStyles = stylesFactory((theme: GrafanaTheme2) => {
    }),
    infoWrapper: css({
      label: 'infoWrapper',
-      border: `1px solid ${autoColor(theme, '#d3d3d3')}`,
-      borderTop: '3px solid',
      padding: '0.75rem',
    }),
+    cell: css({
+      label: 'cell',
+      display: 'flex !important',
+      width: '100% !important',
+    }),
+    indentSpacer: css({
+      label: 'indentSpacer',
+      flex: 'none',
+    }),
+    detailWrapper: css({
+      label: 'detailWrapper',
+      flex: '1',
+      minWidth: 0,
+    }),
  };
 });

@@ -115,7 +125,6 @@ export class UnthemedSpanDetailRow extends PureComponent<SpanDetailRowProps> {
  render() {
    const {
      color,
-      columnDivision,
      detailState,
      logItemToggle,
      logsToggle,
@@ -131,72 +140,68 @@ export class UnthemedSpanDetailRow extends PureComponent<SpanDetailRowProps> {
      traceStartTime,
      traceDuration,
      traceName,
-      hoverIndentGuideIds,
-      addHoverIndentGuideId,
-      removeHoverIndentGuideId,
      theme,
      createSpanLink,
      focusedSpanId,
      createFocusSpanLink,
      datasourceType,
      datasourceUid,
-      visibleSpanIds,
      traceFlameGraphs,
      setTraceFlameGraphs,
      setRedrawListView,
      timeRange,
      app,
+      hoverIndentGuideIds,
+      addHoverIndentGuideId,
+      removeHoverIndentGuideId,
+      visibleSpanIds,
    } = this.props;
    const styles = getStyles(theme);
    return (
      <TimelineRow>
-        <TimelineRow.Cell width={columnDivision} style={{ overflow: 'hidden' }}>
-          <SpanTreeOffset
-            span={span}
-            showChildrenIcon={false}
-            hoverIndentGuideIds={hoverIndentGuideIds}
-            addHoverIndentGuideId={addHoverIndentGuideId}
-            removeHoverIndentGuideId={removeHoverIndentGuideId}
-            visibleSpanIds={visibleSpanIds}
-          />
-          <Button
-            fill="text"
-            onClick={this._detailToggle}
-            className={classNames(styles.expandedAccent, clearButtonStyles(theme))}
-            style={{ borderColor: color }}
-            data-testid="detail-row-expanded-accent"
-          ></Button>
-        </TimelineRow.Cell>
-        <TimelineRow.Cell width={1 - columnDivision}>
-          <div className={styles.infoWrapper} style={{ borderTopColor: color }}>
-            <SpanDetail
-              detailState={detailState}
-              logItemToggle={logItemToggle}
-              logsToggle={logsToggle}
-              processToggle={processToggle}
-              referenceItemToggle={referenceItemToggle}
-              referencesToggle={referencesToggle}
-              warningsToggle={warningsToggle}
-              stackTracesToggle={stackTracesToggle}
+        <TimelineRow.Cell width={1} className={styles.cell}>
+          <div className={styles.indentSpacer}>
+            <SpanTreeOffset
              span={span}
-              traceToProfilesOptions={traceToProfilesOptions}
-              timeZone={timeZone}
-              tagsToggle={tagsToggle}
-              traceStartTime={traceStartTime}
-              traceDuration={traceDuration}
-              traceName={traceName}
-              createSpanLink={createSpanLink}
-              focusedSpanId={focusedSpanId}
-              createFocusSpanLink={createFocusSpanLink}
-              datasourceType={datasourceType}
-              datasourceUid={datasourceUid}
-              traceFlameGraphs={traceFlameGraphs}
-              setTraceFlameGraphs={setTraceFlameGraphs}
-              setRedrawListView={setRedrawListView}
-              timeRange={timeRange}
-              app={app}
+              showChildrenIcon={false}
+              hoverIndentGuideIds={hoverIndentGuideIds}
+              addHoverIndentGuideId={addHoverIndentGuideId}
+              removeHoverIndentGuideId={removeHoverIndentGuideId}
+              visibleSpanIds={visibleSpanIds}
            />
          </div>
+          <div className={styles.detailWrapper}>
+            <div className={styles.infoWrapper} style={{ borderTopColor: color }}>
+              <SpanDetail
+                color={color}
+                detailState={detailState}
+                logItemToggle={logItemToggle}
+                logsToggle={logsToggle}
+                processToggle={processToggle}
+                referenceItemToggle={referenceItemToggle}
+                referencesToggle={referencesToggle}
+                warningsToggle={warningsToggle}
+                stackTracesToggle={stackTracesToggle}
+                span={span}
+                traceToProfilesOptions={traceToProfilesOptions}
+                timeZone={timeZone}
+                tagsToggle={tagsToggle}
+                traceStartTime={traceStartTime}
+                traceDuration={traceDuration}
+                traceName={traceName}
+                createSpanLink={createSpanLink}
+                focusedSpanId={focusedSpanId}
+                createFocusSpanLink={createFocusSpanLink}
+                datasourceType={datasourceType}
+                datasourceUid={datasourceUid}
+                traceFlameGraphs={traceFlameGraphs}
+                setTraceFlameGraphs={setTraceFlameGraphs}
+                setRedrawListView={setRedrawListView}
+                timeRange={timeRange}
+                app={app}
+              />
+            </div>
+          </div>
        </TimelineRow.Cell>
      </TimelineRow>
    );
@@ -14,10 +14,11 @@

 import { css } from '@emotion/css';

+import { GrafanaTheme2 } from '@grafana/data';
 import { t } from '@grafana/i18n';
-import { IconButton, useStyles2 } from '@grafana/ui';
+import { Button, useStyles2 } from '@grafana/ui';

-const getStyles = () => ({
+const getStyles = (theme: GrafanaTheme2) => ({
  TimelineCollapser: css({
    alignItems: 'center',
    display: 'flex',
@@ -25,6 +26,16 @@ const getStyles = () => ({
    justifyContent: 'center',
    marginRight: '0.5rem',
  }),
+  buttonsContainer: css({
+    display: 'flex',
+    flexDirection: 'row',
+    gap: '0.5rem',
+    paddingRight: theme.spacing(1),
+  }),
+  buttonContainer: css({
+    display: 'flex',
+    alignItems: 'center',
+  }),
 });

 type CollapserProps = {
@@ -40,34 +51,52 @@ export function TimelineCollapser(props: CollapserProps) {

  return (
    <div className={styles.TimelineCollapser} data-testid="TimelineCollapser">
-      <IconButton
-        tooltip={t('explore.timeline-collapser.tooltip-expand', 'Expand +1')}
-        size="xl"
-        tooltipPlacement="top"
-        name="angle-down"
-        onClick={onExpandOne}
-      />
-      <IconButton
-        tooltip={t('explore.timeline-collapser.tooltip-collapse', 'Collapse +1')}
-        size="xl"
-        tooltipPlacement="top"
-        name="angle-right"
-        onClick={onCollapseOne}
-      />
-      <IconButton
-        tooltip={t('explore.timeline-collapser.tooltip-expand-all', 'Expand all')}
-        size="xl"
-        tooltipPlacement="top"
-        name="angle-double-down"
-        onClick={onExpandAll}
-      />
-      <IconButton
-        tooltip={t('explore.timeline-collapser.tooltip-collapse-all', 'Collapse all')}
-        size="xl"
-        tooltipPlacement="top"
-        name="angle-double-right"
-        onClick={onCollapseAll}
-      />
+      <div className={styles.buttonsContainer}>
+        <div className={styles.buttonContainer}>
+          <Button
+            aria-label={t('explore.timeline-collapser.tooltip-expand', 'Expand +1')}
+            tooltip={t('explore.timeline-collapser.tooltip-expand', 'Expand +1')}
+            size="sm"
+            tooltipPlacement="top"
+            icon="angle-down"
+            onClick={onExpandOne}
+            fill="solid"
+            variant="secondary"
+          />
+          <Button
+            aria-label={t('explore.timeline-collapser.tooltip-collapse', 'Collapse +1')}
+            tooltip={t('explore.timeline-collapser.tooltip-collapse', 'Collapse +1')}
+            size="sm"
+            tooltipPlacement="top"
+            icon="angle-up"
+            onClick={onCollapseOne}
+            fill="solid"
+            variant="secondary"
+          />
+        </div>
+        <div className={styles.buttonContainer}>
+          <Button
+            aria-label={t('explore.timeline-collapser.tooltip-expand-all', 'Expand all')}
+            tooltip={t('explore.timeline-collapser.tooltip-expand-all', 'Expand all')}
+            size="sm"
+            tooltipPlacement="top"
+            icon="angle-double-down"
+            onClick={onExpandAll}
+            fill="solid"
+            variant="secondary"
+          />
+          <Button
+            aria-label={t('explore.timeline-collapser.tooltip-collapse-all', 'Collapse all')}
+            tooltip={t('explore.timeline-collapser.tooltip-collapse-all', 'Collapse all')}
+            size="sm"
+            tooltipPlacement="top"
+            icon="angle-double-up"
+            onClick={onCollapseAll}
+            fill="solid"
+            variant="secondary"
+          />
+        </div>
+      </div>
    </div>
  );
 }
@@ -16,8 +16,8 @@ import { css } from '@emotion/css';
 import cx from 'classnames';
 import * as React from 'react';

-import { GrafanaTheme2 } from '@grafana/data';
-import { useStyles2 } from '@grafana/ui';
+import { GrafanaTheme2, IconName } from '@grafana/data';
+import { Icon, useStyles2 } from '@grafana/ui';

 import { autoColor } from '../Theme';

@@ -45,7 +45,7 @@ const getStyles = (divider: boolean) => (theme: GrafanaTheme2) => {
            borderRight: `1px solid ${autoColor(theme, '#ddd')}`,
            padding: '0 8px',
          }
-        : {}),
+        : { padding: '0 4px' }),
    }),
    LabeledListLabel: css({
      label: 'LabeledListLabel',
@@ -56,24 +56,44 @@ const getStyles = (divider: boolean) => (theme: GrafanaTheme2) => {
      label: 'LabeledListValue',
      marginRight: divider ? undefined : '0.55rem',
    }),
+    LabeledListIcon: css({
+      label: 'LabeledListIcon',
+      marginRight: '0.25rem',
+      marginTop: '-0.1rem',
+    }),
+    LabeledListServiceLine: css({
+      label: 'LabeledListServiceLine',
+      display: 'inline-block',
+      width: '1.25rem',
+      height: '0.35rem',
+      marginRight: '0.5rem',
+      verticalAlign: 'middle',
+      borderRadius: theme.shape.radius.default,
+    }),
  };
 };

 type LabeledListProps = {
  className?: string;
  divider?: boolean;
-  items: Array<{ key: string; label: React.ReactNode; value: React.ReactNode }>;
+  items: Array<{ key: string; label: React.ReactNode; value: React.ReactNode; icon?: IconName }>;
+  color?: string;
 };

 export default function LabeledList(props: LabeledListProps) {
-  const { className, divider = false, items } = props;
+  const { className, divider = false, items, color } = props;
  const styles = useStyles2(getStyles(divider));

  return (
    <ul className={cx(styles.LabeledList, className)}>
-      {items.map(({ key, label, value }) => {
+      {items.map(({ key, label, value, icon }) => {
        return (
+          // If label is service, create small line on left with color
          <li className={styles.LabeledListItem} key={`${key}`}>
+            {label === 'Service:' && (
+              <div className={styles.LabeledListServiceLine} style={{ backgroundColor: color }} />
+            )}
+            {icon && <Icon name={icon} className={styles.LabeledListIcon} />}
            <span className={styles.LabeledListLabel}>{label}</span>
            <strong className={styles.LabeledListValue}>{value}</strong>
          </li>