Trace View: UI improvements (#114156)

* Fix unclear child span relationship

* Trace view UI improvements

* Fix tests

public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetail/index.tsx

@@ -157,7 +157,7 @@ const getStyles = (theme: GrafanaTheme2) => {
       label: 'ServiceNameAndLinks',
       display: 'flex',
       width: '100%',
-      marginBottom: '16px',
+      marginBottom: theme.spacing(1),
     }),
     operationName: css({
       label: 'SpanDetailOperationName',

public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetailRow.tsx

@@ -77,6 +77,8 @@ const getStyles = stylesFactory((theme: GrafanaTheme2) => {
       label: 'detailWrapper',
       flex: '1',
       minWidth: 0,
+      backgroundColor: theme.colors.background.canvas,
+      border: `1px solid ${theme.colors.border.weak}`,
     }),
   };
 });
@@ -165,6 +167,7 @@ const UnthemedSpanDetailRow = React.memo<SpanDetailRowProps>((props) => {
             addHoverIndentGuideId={addHoverIndentGuideId}
             removeHoverIndentGuideId={removeHoverIndentGuideId}
             visibleSpanIds={visibleSpanIds}
+            removeLastIndentGuide={true}
           />
         </div>
         <div className={styles.detailWrapper}>

public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanTreeOffset.test.tsx

@@ -47,11 +47,12 @@ describe('SpanTreeOffset', () => {
   });
 
   describe('.SpanTreeOffset--indentGuide', () => {
-    it('renders no indentGuide if span has no ancestors and no children', () => {
+    it('renders only the trace-level indentGuide if span has no ancestors and no children', () => {
       jest.mocked(spanAncestorIdsSpy).mockReturnValue([]);
       render(<SpanTreeOffset {...props} />);
       const indentGuides = screen.queryAllByTestId('SpanTreeOffset--indentGuide');
-      expect(indentGuides.length).toBe(0);
+      expect(indentGuides.length).toBe(1);
+      expect(indentGuides[0]).toHaveAttribute('data-ancestor-id', specialRootID);
     });
 
     it('renders only one SpanTreeOffset--indentGuide for entire trace if span has no ancestors but has children', () => {
@@ -66,9 +67,10 @@ describe('SpanTreeOffset', () => {
     it('renders one SpanTreeOffset--indentGuide per ancestor span when span has no children', () => {
       render(<SpanTreeOffset {...props} />);
       const indentGuides = screen.getAllByTestId('SpanTreeOffset--indentGuide');
-      expect(indentGuides.length).toBe(2);
+      expect(indentGuides.length).toBe(3);
       expect(indentGuides[0]).toHaveAttribute('data-ancestor-id', specialRootID);
       expect(indentGuides[1]).toHaveAttribute('data-ancestor-id', rootSpanID);
+      expect(indentGuides[2]).toHaveAttribute('data-ancestor-id', parentSpanID);
     });
 
     it('renders one SpanTreeOffset--indentGuide per ancestor span, plus one for entire trace when span has children', () => {
@@ -112,16 +114,20 @@ describe('SpanTreeOffset', () => {
       props = { ...props, span: { ...props.span, hasChildren: true } };
     });
 
-    it('does not render icon if props.span.hasChildren is false', () => {
+    it('renders placeholder content when props.span.hasChildren is false', () => {
       props.span.hasChildren = false;
       render(<SpanTreeOffset {...props} />);
-      expect(screen.queryByTestId('icon-wrapper')).not.toBeInTheDocument();
+      expect(screen.queryByTestId('icon-arrow-right')).not.toBeInTheDocument();
+      expect(screen.queryByTestId('icon-arrow-down')).not.toBeInTheDocument();
+      expect(screen.getByTestId('icon-wrapper')).toHaveTextContent('-');
     });
 
-    it('does not render icon if props.span.hasChildren is true and showChildrenIcon is false', () => {
+    it('renders placeholder content when props.span.hasChildren is true but showChildrenIcon is false', () => {
       props.showChildrenIcon = false;
       render(<SpanTreeOffset {...props} />);
-      expect(screen.queryByTestId('icon-wrapper')).not.toBeInTheDocument();
+      expect(screen.queryByTestId('icon-arrow-right')).not.toBeInTheDocument();
+      expect(screen.queryByTestId('icon-arrow-down')).not.toBeInTheDocument();
+      expect(screen.getByTestId('icon-wrapper')).toHaveTextContent('-');
     });
 
     it('renders arrow-right if props.span.hasChildren is true and props.childrenVisible is false', () => {

public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanTreeOffset.tsx

@@ -66,6 +66,8 @@ export const getStyles = stylesFactory((theme: GrafanaTheme2) => ({
     right: 0,
     height: '100%',
     paddingTop: '1px',
+    width: '1rem',
+    textAlign: 'center',
   }),
 }));
 
@@ -80,6 +82,7 @@ export type TProps = {
   removeHoverIndentGuideId: (spanID: string) => void;
   theme: GrafanaTheme2;
   visibleSpanIds: string[];
+  removeLastIndentGuide?: boolean;
 };
 
 const UnthemedSpanTreeOffset = React.memo<TProps>((props) => {
@@ -93,6 +96,7 @@ const UnthemedSpanTreeOffset = React.memo<TProps>((props) => {
     hoverIndentGuideIds,
     addHoverIndentGuideId,
     removeHoverIndentGuideId,
+    removeLastIndentGuide = false,
   } = props;
 
   const ancestorIds = React.useMemo(() => {
@@ -101,8 +105,12 @@ const UnthemedSpanTreeOffset = React.memo<TProps>((props) => {
     // necessary padding for the collapse icon on root-level spans.
     ids.push('root');
     ids.reverse();
+
+    if (removeLastIndentGuide) {
+      ids.pop();
+    }
     return ids;
-  }, [span]);
+  }, [span, removeLastIndentGuide]);
 
   /**
    * If the mouse leaves to anywhere except another span with the same ancestor id, this span's ancestor id is
@@ -156,20 +164,15 @@ const UnthemedSpanTreeOffset = React.memo<TProps>((props) => {
     ));
   const styles = getStyles(theme);
 
-  // If span has no children, don't show the last indent guide
-  const displayedAncestorIds = hasChildren ? ancestorIds : ancestorIds.slice(0, -1);
-
   return (
     <span className={cx(styles.SpanTreeOffset, { [styles.SpanTreeOffsetParent]: hasChildren })} {...wrapperProps}>
-      {displayedAncestorIds.map((ancestorId, index) => (
+      {ancestorIds.map((ancestorId, index) => (
         <span
           key={ancestorId}
           className={cx(styles.indentGuide, {
             [styles.indentGuideActive]: hoverIndentGuideIds.has(ancestorId),
             [styles.indentGuideThin]:
-              index !== displayedAncestorIds.length - 1 &&
-              ancestorId !== 'root' &&
-              !visibleSpanIds.includes(ancestorId),
+              index !== ancestorIds.length - 1 && ancestorId !== 'root' && !visibleSpanIds.includes(ancestorId),
           })}
           data-ancestor-id={ancestorId}
           data-testid="SpanTreeOffset--indentGuide"
@@ -177,16 +180,14 @@ const UnthemedSpanTreeOffset = React.memo<TProps>((props) => {
           onMouseLeave={(event) => handleMouseLeave(event, ancestorId)}
         />
       ))}
-      {icon && (
-        <span
-          className={cx(styles.iconWrapper, 'icon-wrapper')}
-          onMouseEnter={(event) => handleMouseEnter(event, spanID)}
-          onMouseLeave={(event) => handleMouseLeave(event, spanID)}
-          data-testid="icon-wrapper"
-        >
-          {icon}
-        </span>
-      )}
+      <span
+        className={cx(styles.iconWrapper, 'icon-wrapper')}
+        onMouseEnter={(event) => icon && handleMouseEnter(event, spanID)}
+        onMouseLeave={(event) => icon && handleMouseLeave(event, spanID)}
+        data-testid="icon-wrapper"
+      >
+        {icon || (!removeLastIndentGuide && '-')}
+      </span>
     </span>
   );
 });

public/app/features/explore/TraceView/components/common/LabeledList.tsx

@@ -28,6 +28,7 @@ const getStyles = (divider: boolean) => (theme: GrafanaTheme2) => {
       listStyle: 'none',
       margin: 0,
       padding: 0,
+      fontSize: theme.typography.size.sm,
       ...(divider
         ? {
             marginRight: '-8px',
@@ -49,7 +50,7 @@ const getStyles = (divider: boolean) => (theme: GrafanaTheme2) => {
     }),
     LabeledListLabel: css({
       label: 'LabeledListLabel',
-      color: theme.isLight ? '#999' : '#666',
+      color: theme.colors.text.secondary,
       marginRight: '0.25rem',
     }),
     LabeledListValue: css({
@@ -66,7 +67,7 @@ const getStyles = (divider: boolean) => (theme: GrafanaTheme2) => {
     LabeledListServiceLine: css({
       label: 'LabeledListServiceLine',
       display: 'inline-block',
-      width: '1.25rem',
+      width: '1rem',
       height: '0.35rem',
       marginRight: '0.5rem',
       verticalAlign: 'middle',
@@ -95,7 +96,7 @@ export default function LabeledList(props: LabeledListProps) {
             {label === 'Service:' && (
               <div className={styles.LabeledListServiceLine} style={{ backgroundColor: color }} />
             )}
-            {icon && <Icon name={icon} className={styles.LabeledListIcon} />}
+            {icon && <Icon name={icon} className={styles.LabeledListIcon} size="sm" />}
             <span className={styles.LabeledListLabel}>{label}</span>
             <strong className={styles.LabeledListValue}>{value}</strong>
           </li>