Trace View: Span list visual update (#75238)

* Show color of row as a border under the row. Hide service name for sequential spans * Increase default span name column width. Smaller font for service and span names in span list * New background color on spans. Fixed hover of indent markers * Service name and span name style tweaks * Collapse hidden levels * Fixed test * Small tweak to Buffer size to make sure tests pass * Trigger runs * Update betterer results * Address comment * Style tweaks * Remove duplicated code * Rollback change to join <span> since they are needed for the tests
2025-02-25 18:55:37 -06:00 · 2023-10-04 14:00:40 +01:00 · 2023-10-04 14:00:40 +01:00 · 607a664ef4
commit 607a664ef4
parent 65fa94b16b
8 changed files with 134 additions and 76 deletions
--- a/.betterer.results
+++ b/.betterer.results
@ -4163,14 +4163,17 @@ exports[`better eslint`] = {
    ],
    "public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanLinks.tsx:5381": [
      [0, 0, 0, "Styles should be written using objects.", "0"],
-      [0, 0, 0, "Styles should be written using objects.", "1"]
+      [0, 0, 0, "Styles should be written using objects.", "1"],
+      [0, 0, 0, "Styles should be written using objects.", "2"],
+      [0, 0, 0, "Styles should be written using objects.", "3"]
    ],
    "public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanTreeOffset.tsx:5381": [
      [0, 0, 0, "Styles should be written using objects.", "0"],
      [0, 0, 0, "Styles should be written using objects.", "1"],
      [0, 0, 0, "Styles should be written using objects.", "2"],
      [0, 0, 0, "Styles should be written using objects.", "3"],
-      [0, 0, 0, "Styles should be written using objects.", "4"]
+      [0, 0, 0, "Styles should be written using objects.", "4"],
+      [0, 0, 0, "Styles should be written using objects.", "5"]
    ],
    "public/app/features/explore/TraceView/components/TraceTimelineViewer/Ticks.tsx:5381": [
      [0, 0, 0, "Styles should be written using objects.", "0"],
--- a/public/app/features/explore/TraceView/TraceView.tsx
+++ b/public/app/features/explore/TraceView/TraceView.tsx
@ -91,7 +91,7 @@ export function TraceView(props: Props) {
  /**
   * Keeps state of resizable name column width
   */
-  const [spanNameColumnWidth, setSpanNameColumnWidth] = useState(0.25);
+  const [spanNameColumnWidth, setSpanNameColumnWidth] = useState(0.4);

  const [focusedSpanId, createFocusSpanLink] = useFocusSpanLink({
    refId: props.dataFrames[0]?.refId,
--- a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanBarRow.tsx
+++ b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanBarRow.tsx
@ -73,7 +73,8 @@ const getStyles = stylesFactory((theme: GrafanaTheme2, showSpanFilterMatchesOnly
    `,
    endpointName: css`
      label: endpointName;
-      color: ${autoColor(theme, '#808080')};
+      color: ${autoColor(theme, '#484848')};
+      font-size: 0.9em;
    `,
    view: css`
      label: view;
@ -91,6 +92,7 @@ const getStyles = stylesFactory((theme: GrafanaTheme2, showSpanFilterMatchesOnly
    `,
    row: css`
      label: row;
+      font-size: 0.9em;
      &:hover .${spanBarClassName} {
        opacity: 1;
      }
@ -213,7 +215,6 @@ const getStyles = stylesFactory((theme: GrafanaTheme2, showSpanFilterMatchesOnly
      outline: none;
      overflow-y: hidden;
      overflow-x: auto;
-      margin-right: 8px;
      padding-left: 4px;
      padding-right: 0.25em;
      position: relative;
@ -222,24 +223,17 @@ const getStyles = stylesFactory((theme: GrafanaTheme2, showSpanFilterMatchesOnly
      &::-webkit-scrollbar {
        display: none;
      }
-      &::before {
-        content: ' ';
-        position: absolute;
-        top: 4px;
-        bottom: 4px;
-        left: 0;
-        border-left: 4px solid;
-        border-left-color: inherit;
-      }
      &:focus {
        text-decoration: none;
      }
-      &:hover > small {
+      &:hover > span {
        color: ${autoColor(theme, '#000')};
      }
      text-align: left;
      background: transparent;
      border: none;
+      border-bottom-width: 1px;
+      border-bottom-style: solid;
    `,
    nameDetailExpanded: css`
      label: nameDetailExpanded;
@ -249,8 +243,9 @@ const getStyles = stylesFactory((theme: GrafanaTheme2, showSpanFilterMatchesOnly
    `,
    svcName: css`
      label: svcName;
-      padding: 0 0.25rem 0 0.5rem;
-      font-size: 1.05em;
+      font-size: 0.9em;
+      font-weight: bold;
+      margin-right: 0.25rem;
    `,
    svcNameChildrenCollapsed: css`
      label: svcNameChildrenCollapsed;
@ -301,6 +296,7 @@ export type SpanBarRowProps = {
  onDetailToggled: (spanID: string) => void;
  onChildrenToggled: (spanID: string) => void;
  numTicks: number;
+  showServiceName: boolean;
  rpc?:
    | {
        viewStart: number;
@ -327,6 +323,7 @@ export type SpanBarRowProps = {
  clippingRight?: boolean;
  createSpanLink?: SpanLinkFunc;
  datasourceType: string;
+  visibleSpanIds: string[];
 };

 /**
@ -378,6 +375,8 @@ export class UnthemedSpanBarRow extends React.PureComponent<SpanBarRowProps> {
      theme,
      createSpanLink,
      datasourceType,
+      showServiceName,
+      visibleSpanIds,
    } = this.props;
    const {
      duration,
@ -432,6 +431,7 @@ export class UnthemedSpanBarRow extends React.PureComponent<SpanBarRowProps> {
              hoverIndentGuideIds={hoverIndentGuideIds}
              addHoverIndentGuideId={addHoverIndentGuideId}
              removeHoverIndentGuideId={removeHoverIndentGuideId}
+              visibleSpanIds={visibleSpanIds}
            />
            <button
              type="button"
@ -440,43 +440,45 @@ export class UnthemedSpanBarRow extends React.PureComponent<SpanBarRowProps> {
              title={labelDetail}
              onClick={this._detailToggle}
              role="switch"
-              style={{ borderColor: color }}
+              style={{ background: `${color}10`, borderBottomColor: `${color}CF` }}
              tabIndex={0}
            >
-              <span
-                className={cx(styles.svcName, {
-                  [styles.svcNameChildrenCollapsed]: isParent && !isChildrenExpanded,
-                })}
-              >
-                {showErrorIcon && (
-                  <Icon
-                    name={'exclamation-circle'}
-                    style={{
-                      backgroundColor: span.errorIconColor
-                        ? autoColor(theme, span.errorIconColor)
-                        : autoColor(theme, '#db2828'),
-                    }}
-                    className={styles.errorIcon}
-                  />
-                )}
-                {serviceName}{' '}
-                {rpc && (
-                  <span>
-                    <Icon name={'arrow-right'} />{' '}
-                    <i className={styles.rpcColorMarker} style={{ background: rpc.color }} />
-                    {rpc.serviceName}
-                  </span>
-                )}
-                {noInstrumentedServer && (
-                  <span>
-                    <Icon name={'arrow-right'} />{' '}
-                    <i className={styles.rpcColorMarker} style={{ background: noInstrumentedServer.color }} />
-                    {noInstrumentedServer.serviceName}
-                  </span>
-                )}
-              </span>
-              <small className={styles.endpointName}>{rpc ? rpc.operationName : operationName}</small>
-              <small className={styles.endpointName}> {this.getSpanBarLabel(span, spanBarOptions, label)}</small>
+              {showErrorIcon && (
+                <Icon
+                  name={'exclamation-circle'}
+                  style={{
+                    backgroundColor: span.errorIconColor
+                      ? autoColor(theme, span.errorIconColor)
+                      : autoColor(theme, '#db2828'),
+                  }}
+                  className={styles.errorIcon}
+                />
+              )}
+              {showServiceName && (
+                <span
+                  className={cx(styles.svcName, {
+                    [styles.svcNameChildrenCollapsed]: isParent && !isChildrenExpanded,
+                  })}
+                >
+                  {`${serviceName} `}
+                </span>
+              )}
+              {rpc && (
+                <span>
+                  <Icon name={'arrow-right'} />{' '}
+                  <i className={styles.rpcColorMarker} style={{ background: rpc.color }} />
+                  {rpc.serviceName}
+                </span>
+              )}
+              {noInstrumentedServer && (
+                <span>
+                  <Icon name={'arrow-right'} />{' '}
+                  <i className={styles.rpcColorMarker} style={{ background: noInstrumentedServer.color }} />
+                  {noInstrumentedServer.serviceName}
+                </span>
+              )}
+              <span className={styles.endpointName}>{rpc ? rpc.operationName : operationName}</span>
+              <span className={styles.endpointName}> {this.getSpanBarLabel(span, spanBarOptions, label)}</span>
            </button>
            {createSpanLink &&
              (() => {
@ -492,7 +494,7 @@ export class UnthemedSpanBarRow extends React.PureComponent<SpanBarRowProps> {
                      href={links[0].href}
                      // Needs to have target otherwise preventDefault would not work due to angularRouter.
                      target={'_blank'}
-                      style={{ marginRight: '5px' }}
+                      style={{ background: `${color}10`, borderBottom: `1px solid ${color}CF`, paddingRight: '4px' }}
                      rel="noopener noreferrer"
                      onClick={
                        links[0].onClick
@ -509,7 +511,7 @@ export class UnthemedSpanBarRow extends React.PureComponent<SpanBarRowProps> {
                    </a>
                  );
                } else if (links && count > 1) {
-                  return <SpanLinksMenu links={links} datasourceType={datasourceType} />;
+                  return <SpanLinksMenu links={links} datasourceType={datasourceType} color={color} />;
                } else {
                  return null;
                }
--- a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetailRow.tsx
+++ b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetailRow.tsx
@ -38,7 +38,7 @@ const getStyles = stylesFactory((theme: GrafanaTheme2) => {
      position: absolute;
      width: 100%;
      &::before {
-        border-left: 4px solid;
+        border-left: 1px solid;
        pointer-events: none;
        width: 1000px;
      }
@ -97,6 +97,7 @@ export type SpanDetailRowProps = {
  createFocusSpanLink: (traceId: string, spanId: string) => LinkModel;
  topOfViewRefType?: TopOfViewRefType;
  datasourceType: string;
+  visibleSpanIds: string[];
 };

 export class UnthemedSpanDetailRow extends React.PureComponent<SpanDetailRowProps> {
@ -134,6 +135,7 @@ export class UnthemedSpanDetailRow extends React.PureComponent<SpanDetailRowProp
      createFocusSpanLink,
      topOfViewRefType,
      datasourceType,
+      visibleSpanIds,
    } = this.props;
    const styles = getStyles(theme);
    return (
@ -145,6 +147,7 @@ export class UnthemedSpanDetailRow extends React.PureComponent<SpanDetailRowProp
            hoverIndentGuideIds={hoverIndentGuideIds}
            addHoverIndentGuideId={addHoverIndentGuideId}
            removeHoverIndentGuideId={removeHoverIndentGuideId}
+            visibleSpanIds={visibleSpanIds}
          />
          <Button
            fill="text"
--- a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanLinks.tsx
+++ b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanLinks.tsx
@ -9,6 +9,7 @@ import { SpanLinkDef } from '../types/links';
 interface SpanLinksProps {
  links: SpanLinkDef[];
  datasourceType: string;
+  color: string;
 }

 const renderMenuItems = (
@ -46,15 +47,15 @@ const renderMenuItems = (
  ));
 };

-export const SpanLinksMenu = ({ links, datasourceType }: SpanLinksProps) => {
-  const styles = useStyles2(getStyles);
+export const SpanLinksMenu = ({ links, datasourceType, color }: SpanLinksProps) => {
+  const styles = useStyles2(() => getStyles(color));
  const [isMenuOpen, setIsMenuOpen] = useState(false);
  const [menuPosition, setMenuPosition] = useState({ x: 0, y: 0 });

  const closeMenu = () => setIsMenuOpen(false);

  return (
-    <div data-testid="SpanLinksMenu">
+    <div data-testid="SpanLinksMenu" className={styles.wrapper}>
      <button
        onClick={(e) => {
          setIsMenuOpen(true);
@ -65,7 +66,7 @@ export const SpanLinksMenu = ({ links, datasourceType }: SpanLinksProps) => {
        }}
        className={styles.button}
      >
-        <Icon name="link" className={styles.button} />
+        <Icon name="link" className={styles.icon} />
      </button>

      {isMenuOpen ? (
@ -81,13 +82,23 @@ export const SpanLinksMenu = ({ links, datasourceType }: SpanLinksProps) => {
  );
 };

-const getStyles = () => {
+const getStyles = (color: string) => {
  return {
+    wrapper: css`
+      border: none;
+      background: ${color}10;
+      border-bottom: 1px solid ${color}CF;
+      padding-right: 4px;
+    `,
    button: css`
      background: transparent;
      border: none;
      padding: 0;
-      margin: 0 3px 0 0;
+    `,
+    icon: css`
+      background: transparent;
+      border: none;
+      padding: 0;
    `,
    menuItem: css`
      max-width: 60ch;
--- a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanTreeOffset.test.tsx
+++ b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanTreeOffset.test.tsx
@ -39,6 +39,7 @@ describe('SpanTreeOffset', () => {
      addHoverIndentGuideId: jest.fn(),
      hoverIndentGuideIds: new Set(),
      removeHoverIndentGuideId: jest.fn(),
+      visibleSpanIds: [],
      span: {
        hasChildren: false,
        spanID: ownSpanID,
--- a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanTreeOffset.tsx
+++ b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanTreeOffset.tsx
@ -40,10 +40,10 @@ export const getStyles = stylesFactory((theme: GrafanaTheme2) => {
    indentGuide: css`
      label: indentGuide;
      /* The size of the indentGuide is based off of the iconWrapper */
-      padding-right: calc(0.5rem + 12px);
+      padding-right: 1rem;
      height: 100%;
-      border-left: 3px solid transparent;
      display: inline-flex;
+      transition: padding 300ms ease-out;
      &::before {
        content: '';
        padding-left: 1px;
@ -52,15 +52,17 @@ export const getStyles = stylesFactory((theme: GrafanaTheme2) => {
    `,
    indentGuideActive: css`
      label: indentGuideActive;
-      border-color: ${autoColor(theme, 'darkgrey')};
      &::before {
-        background-color: transparent;
+        background-color: ${autoColor(theme, '#777')};
      }
    `,
+    indentGuideThin: css`
+      padding-right: 0.3rem;
+    `,
    iconWrapper: css`
      label: iconWrapper;
      position: absolute;
-      right: 0.25rem;
+      right: 0;
    `,
  };
 });
@ -75,6 +77,7 @@ export type TProps = {
  addHoverIndentGuideId: (spanID: string) => void;
  removeHoverIndentGuideId: (spanID: string) => void;
  theme: GrafanaTheme2;
+  visibleSpanIds: string[];
 };

 export class UnthemedSpanTreeOffset extends React.PureComponent<TProps> {
@ -133,25 +136,28 @@ export class UnthemedSpanTreeOffset extends React.PureComponent<TProps> {
  };

  render() {
-    const { childrenVisible, onClick, showChildrenIcon, span, theme } = this.props;
+    const { childrenVisible, onClick, showChildrenIcon, span, theme, visibleSpanIds } = this.props;
    const { hasChildren, spanID } = span;
    const wrapperProps = hasChildren ? { onClick, role: 'switch', 'aria-checked': childrenVisible } : null;
    const icon =
      showChildrenIcon &&
      hasChildren &&
      (childrenVisible ? (
-        <Icon name={'angle-down'} data-testid="icon-arrow-down" />
+        <Icon name={'angle-down'} data-testid="icon-arrow-down" size={'sm'} />
      ) : (
-        <Icon name={'angle-right'} data-testid="icon-arrow-right" />
+        <Icon name={'angle-right'} data-testid="icon-arrow-right" size={'sm'} />
      ));
    const styles = getStyles(theme);
+
    return (
      <span className={cx(styles.SpanTreeOffset, { [styles.SpanTreeOffsetParent]: hasChildren })} {...wrapperProps}>
-        {this.ancestorIds.map((ancestorId) => (
+        {this.ancestorIds.map((ancestorId, index) => (
          <span
            key={ancestorId}
            className={cx(styles.indentGuide, {
              [styles.indentGuideActive]: this.props.hoverIndentGuideIds.has(ancestorId),
+              [styles.indentGuideThin]:
+                index !== this.ancestorIds.length - 1 && ancestorId !== 'root' && !visibleSpanIds.includes(ancestorId),
            })}
            data-ancestor-id={ancestorId}
            data-testid="SpanTreeOffset--indentGuide"
--- a/public/app/features/explore/TraceView/components/TraceTimelineViewer/VirtualizedTraceView.tsx
+++ b/public/app/features/explore/TraceView/components/TraceTimelineViewer/VirtualizedTraceView.tsx
@ -122,6 +122,7 @@ export const DEFAULT_HEIGHTS = {
 };

 const NUM_TICKS = 5;
+const BUFFER_SIZE = 33;

 function generateRowStates(
  spans: TraceSpan[] | TNil,
@ -331,9 +332,15 @@ export class UnthemedVirtualizedTraceView extends React.Component<VirtualizedTra

  renderRow = (key: string, style: React.CSSProperties, index: number, attrs: {}) => {
    const { isDetail, span, spanIndex } = this.getRowStates()[index];
+
+    // Compute the list of currently visible span IDs to pass to the row renderers.
+    const start = Math.max((this.listView?.getTopVisibleIndex() || 0) - BUFFER_SIZE, 0);
+    const end = (this.listView?.getBottomVisibleIndex() || 0) + BUFFER_SIZE;
+    const visibleSpanIds = this.getVisibleSpanIds(start, end);
+
    return isDetail
-      ? this.renderSpanDetailRow(span, key, style, attrs)
-      : this.renderSpanBarRow(span, spanIndex, key, style, attrs);
+      ? this.renderSpanDetailRow(span, key, style, attrs, visibleSpanIds)
+      : this.renderSpanBarRow(span, spanIndex, key, style, attrs, visibleSpanIds);
  };

  scrollToSpan = (headerHeight: number, spanID?: string) => {
@ -346,7 +353,14 @@ export class UnthemedVirtualizedTraceView extends React.Component<VirtualizedTra
    }
  };

-  renderSpanBarRow(span: TraceSpan, spanIndex: number, key: string, style: React.CSSProperties, attrs: {}) {
+  renderSpanBarRow(
+    span: TraceSpan,
+    spanIndex: number,
+    key: string,
+    style: React.CSSProperties,
+    attrs: {},
+    visibleSpanIds: string[]
+  ) {
    const { spanID } = span;
    const { serviceName } = span.process;
    const {
@ -406,6 +420,8 @@ export class UnthemedVirtualizedTraceView extends React.Component<VirtualizedTra
      };
    }

+    const prevSpan = spanIndex > 0 ? trace.spans[spanIndex - 1] : null;
+
    const styles = getStyles(this.props);
    return (
      <div className={styles.row} key={key} style={style} {...attrs}>
@ -434,12 +450,14 @@ export class UnthemedVirtualizedTraceView extends React.Component<VirtualizedTra
          removeHoverIndentGuideId={removeHoverIndentGuideId}
          createSpanLink={createSpanLink}
          datasourceType={datasourceType}
+          showServiceName={prevSpan === null || prevSpan.process.serviceName !== span.process.serviceName}
+          visibleSpanIds={visibleSpanIds}
        />
      </div>
    );
  }

-  renderSpanDetailRow(span: TraceSpan, key: string, style: React.CSSProperties, attrs: {}) {
+  renderSpanDetailRow(span: TraceSpan, key: string, style: React.CSSProperties, attrs: {}, visibleSpanIds: string[]) {
    const { spanID } = span;
    const { serviceName } = span.process;
    const {
@ -473,6 +491,7 @@ export class UnthemedVirtualizedTraceView extends React.Component<VirtualizedTra
    }
    const color = getColorByKey(serviceName, theme);
    const styles = getStyles(this.props);
+
    return (
      <div className={styles.row} key={key} style={{ ...style, zIndex: 1 }} {...attrs}>
        <SpanDetailRow
@ -500,6 +519,7 @@ export class UnthemedVirtualizedTraceView extends React.Component<VirtualizedTra
          createFocusSpanLink={createFocusSpanLink}
          topOfViewRefType={topOfViewRefType}
          datasourceType={datasourceType}
+          visibleSpanIds={visibleSpanIds}
        />
      </div>
    );
@ -516,9 +536,21 @@ export class UnthemedVirtualizedTraceView extends React.Component<VirtualizedTra
    });
  };

+  getVisibleSpanIds = memoizeOne((start: number, end: number) => {
+    const spanIds = [];
+    for (let i = start; i < end; i++) {
+      const rowState = this.getRowStates()[i];
+      if (rowState?.span) {
+        spanIds.push(rowState.span.spanID);
+      }
+    }
+    return spanIds;
+  });
+
  render() {
    const styles = getStyles(this.props);
    const { scrollElement } = this.props;
+
    return (
      <>
        <ListView
@ -526,8 +558,8 @@ export class UnthemedVirtualizedTraceView extends React.Component<VirtualizedTra
          dataLength={this.getRowStates().length}
          itemHeightGetter={this.getRowHeight}
          itemRenderer={this.renderRow}
-          viewBuffer={50}
-          viewBufferMin={50}
+          viewBuffer={BUFFER_SIZE}
+          viewBufferMin={BUFFER_SIZE}
          itemsWrapperClassName={styles.rowsWrapper}
          getKeyFromIndex={this.getKeyFromIndex}
          getIndexFromKey={this.getIndexFromKey}