Tempo: Fix missing deep span link (#77936)

* Fix span deep link not showing * Update test
2025-02-25 18:55:37 -06:00 · 2023-11-13 11:10:19 +00:00 · 2023-11-13 11:10:19 +00:00 · c9faaf7600
commit c9faaf7600
parent bd85d3e25e
10 changed files with 39 additions and 82 deletions
--- a/public/app/features/explore/Explore.tsx
+++ b/public/app/features/explore/Explore.tsx
@ -1,7 +1,7 @@
 import { css, cx } from '@emotion/css';
 import { get, groupBy } from 'lodash';
 import memoizeOne from 'memoize-one';
-import React, { createRef } from 'react';
+import React from 'react';
 import { connect, ConnectedProps } from 'react-redux';
 import AutoSizer from 'react-virtualized-auto-sizer';

@ -151,7 +151,6 @@ export type Props = ExploreProps & ConnectedProps<typeof connector>;

 export class Explore extends React.PureComponent<Props, ExploreState> {
  scrollElement: HTMLDivElement | undefined;
-  topOfViewRef = createRef<HTMLDivElement>();
  graphEventBus: EventBus;
  logsEventBus: EventBus;
  memoizedGetNodeGraphDataFrames = memoizeOne(getNodeGraphDataFrames);
@ -580,7 +579,7 @@ export class Explore extends React.PureComponent<Props, ExploreState> {
              scrollRefCallback={(scrollElement) => (this.scrollElement = scrollElement || undefined)}
              hideHorizontalTrack
            >
-              <div className={styles.exploreContainer} ref={this.topOfViewRef}>
+              <div className={styles.exploreContainer}>
                {datasourceInstance ? (
                  <>
                    <ContentOutlineItem title="Queries" icon="arrow">
--- a/public/app/features/explore/ExploreToolbar.tsx
+++ b/public/app/features/explore/ExploreToolbar.tsx
@ -1,6 +1,6 @@
 import { css, cx } from '@emotion/css';
 import { pick } from 'lodash';
-import React, { RefObject, useMemo } from 'react';
+import React, { useMemo } from 'react';
 import { shallowEqual } from 'react-redux';

 import { DataSourceInstanceSettings, RawTimeRange, GrafanaTheme2 } from '@grafana/data';
@ -62,16 +62,9 @@ interface Props {
  onChangeTime: (range: RawTimeRange, changedByScanner?: boolean) => void;
  onContentOutlineToogle: () => void;
  isContentOutlineOpen: boolean;
-  topOfViewRef?: RefObject<HTMLDivElement>;
 }

-export function ExploreToolbar({
-  exploreId,
-  topOfViewRef,
-  onChangeTime,
-  onContentOutlineToogle,
-  isContentOutlineOpen,
-}: Props) {
+export function ExploreToolbar({ exploreId, onChangeTime, onContentOutlineToogle, isContentOutlineOpen }: Props) {
  const dispatch = useDispatch();
  const splitted = useSelector(isSplit);
  const styles = useStyles2(getStyles, splitted);
@ -225,9 +218,9 @@ export function ExploreToolbar({
  ];

  return (
-    <div ref={topOfViewRef}>
+    <div>
      {refreshInterval && <SetInterval func={onRunQuery} interval={refreshInterval} loading={loading} />}
-      <div ref={topOfViewRef}>
+      <div>
        <AppChromeUpdate actions={navBarActions} />
      </div>
      <PageToolbar
--- a/public/app/features/explore/TraceView/TraceView.test.tsx
+++ b/public/app/features/explore/TraceView/TraceView.test.tsx
@ -9,7 +9,6 @@ import { DataSourceSrv, setDataSourceSrv } from '@grafana/runtime';
 import { configureStore } from '../../../store/configureStore';

 import { TraceView } from './TraceView';
-import { TopOfViewRefType } from './components/TraceTimelineViewer/VirtualizedTraceView';
 import { TraceData, TraceSpanData } from './components/types/trace';
 import { transformDataFrames } from './utils/transform';

@ -32,7 +31,6 @@ function getTraceView(frames: DataFrame[]) {
        queryResponse={mockPanelData}
        datasource={undefined}
        topOfViewRef={topOfViewRef}
-        topOfViewRefType={TopOfViewRefType.Explore}
      />
    </Provider>
  );
--- a/public/app/features/explore/TraceView/TraceView.tsx
+++ b/public/app/features/explore/TraceView/TraceView.tsx
@ -38,7 +38,6 @@ import {
 } from './components';
 import memoizedTraceCriticalPath from './components/CriticalPath';
 import SpanGraph from './components/TracePageHeader/SpanGraph';
-import { TopOfViewRefType } from './components/TraceTimelineViewer/VirtualizedTraceView';
 import { createSpanLinkFactory } from './createSpanLink';
 import { useChildrenState } from './useChildrenState';
 import { useDetailState } from './useDetailState';
@ -67,19 +66,11 @@ type Props = {
  queryResponse: PanelData;
  datasource: DataSourceApi<DataQuery, DataSourceJsonData, {}> | undefined;
  topOfViewRef?: RefObject<HTMLDivElement>;
-  topOfViewRefType?: TopOfViewRefType;
  createSpanLink?: SpanLinkFunc;
 };

 export function TraceView(props: Props) {
-  const {
-    traceProp,
-    datasource,
-    topOfViewRef,
-    topOfViewRefType,
-    exploreId,
-    createSpanLink: createSpanLinkFromProps,
-  } = props;
+  const { traceProp, datasource, topOfViewRef, exploreId, createSpanLink: createSpanLinkFromProps } = props;

  const {
    detailStates,
@ -232,7 +223,6 @@ export function TraceView(props: Props) {
            showCriticalPathSpansOnly={showCriticalPathSpansOnly}
            createFocusSpanLink={createFocusSpanLink}
            topOfViewRef={topOfViewRef}
-            topOfViewRefType={topOfViewRefType}
            headerHeight={headerHeight}
            criticalPath={criticalPath}
          />
--- a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetail/index.test.tsx
+++ b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetail/index.test.tsx
@ -45,9 +45,9 @@ describe('<SpanDetail>', () => {
    warningsToggle: jest.fn(),
    referencesToggle: jest.fn(),
    createFocusSpanLink: jest.fn().mockReturnValue({}),
-    topOfViewRefType: 'Explore',
  };

+  span.spanID = 'test-spanID';
  span.kind = 'test-kind';
  span.statusCode = 2;
  span.statusMessage = 'test-message';
@ -195,6 +195,6 @@ describe('<SpanDetail>', () => {

  it('renders deep link URL', () => {
    render(<SpanDetail {...(props as unknown as SpanDetailProps)} />);
-    expect(document.getElementsByTagName('a').length).toBeGreaterThan(1);
+    expect(screen.getByText('test-spanID')).toBeInTheDocument();
  });
 });
--- a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetail/index.tsx
+++ b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetail/index.tsx
@ -30,7 +30,6 @@ import { SpanLinkFunc, TNil } from '../../types';
 import { SpanLinkDef, SpanLinkType } from '../../types/links';
 import { TraceKeyValuePair, TraceLink, TraceLog, TraceSpan, TraceSpanReference } from '../../types/trace';
 import { uAlignIcon, ubM0, ubMb1, ubMy1, ubTxRightAlign } from '../../uberUtilityStyles';
-import { TopOfViewRefType } from '../VirtualizedTraceView';
 import { formatDuration } from '../utils';

 import AccordianKeyValues from './AccordianKeyValues';
@ -124,7 +123,6 @@ export type SpanDetailProps = {
  createSpanLink?: SpanLinkFunc;
  focusedSpanId?: string;
  createFocusSpanLink: (traceId: string, spanId: string) => LinkModel;
-  topOfViewRefType?: TopOfViewRefType;
  datasourceType: string;
 };

@ -144,7 +142,6 @@ export default function SpanDetail(props: SpanDetailProps) {
    referenceItemToggle,
    createSpanLink,
    createFocusSpanLink,
-    topOfViewRefType,
    datasourceType,
  } = props;
  const {
@ -380,31 +377,29 @@ export default function SpanDetail(props: SpanDetailProps) {
            createFocusSpanLink={createFocusSpanLink}
          />
        )}
-        {topOfViewRefType === TopOfViewRefType.Explore && (
-          <small className={styles.debugInfo}>
-            {/* TODO: fix keyboard a11y */}
-            {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */}
-            <a
-              {...focusSpanLink}
-              onClick={(e) => {
-                // click handling logic copied from react router:
-                // https://github.com/remix-run/react-router/blob/997b4d67e506d39ac6571cb369d6d2d6b3dda557/packages/react-router-dom/index.tsx#L392-L394s
-                if (
-                  focusSpanLink.onClick &&
-                  e.button === 0 && // Ignore everything but left clicks
-                  (!e.currentTarget.target || e.currentTarget.target === '_self') && // Let browser handle "target=_blank" etc.
-                  !(e.metaKey || e.altKey || e.ctrlKey || e.shiftKey) // Ignore clicks with modifier keys
-                ) {
-                  e.preventDefault();
-                  focusSpanLink.onClick(e);
-                }
-              }}
-            >
-              <Icon name={'link'} className={cx(uAlignIcon, styles.LinkIcon)}></Icon>
-            </a>
-            <span className={styles.debugLabel} data-label="SpanID:" /> {spanID}
-          </small>
-        )}
+        <small className={styles.debugInfo}>
+          {/* TODO: fix keyboard a11y */}
+          {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */}
+          <a
+            {...focusSpanLink}
+            onClick={(e) => {
+              // click handling logic copied from react router:
+              // https://github.com/remix-run/react-router/blob/997b4d67e506d39ac6571cb369d6d2d6b3dda557/packages/react-router-dom/index.tsx#L392-L394s
+              if (
+                focusSpanLink.onClick &&
+                e.button === 0 && // Ignore everything but left clicks
+                (!e.currentTarget.target || e.currentTarget.target === '_self') && // Let browser handle "target=_blank" etc.
+                !(e.metaKey || e.altKey || e.ctrlKey || e.shiftKey) // Ignore clicks with modifier keys
+              ) {
+                e.preventDefault();
+                focusSpanLink.onClick(e);
+              }
+            }}
+          >
+            <Icon name={'link'} className={cx(uAlignIcon, styles.LinkIcon)}></Icon>
+          </a>
+          <span className={styles.debugLabel} data-label="SpanID:" /> {spanID}
+        </small>
      </div>
    </div>
  );
--- a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetailRow.tsx
+++ b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetailRow.tsx
@ -27,7 +27,6 @@ import SpanDetail from './SpanDetail';
 import DetailState from './SpanDetail/DetailState';
 import SpanTreeOffset from './SpanTreeOffset';
 import TimelineRow from './TimelineRow';
-import { TopOfViewRefType } from './VirtualizedTraceView';

 const getStyles = stylesFactory((theme: GrafanaTheme2) => {
  return {
@ -95,7 +94,6 @@ export type SpanDetailRowProps = {
  createSpanLink?: SpanLinkFunc;
  focusedSpanId?: string;
  createFocusSpanLink: (traceId: string, spanId: string) => LinkModel;
-  topOfViewRefType?: TopOfViewRefType;
  datasourceType: string;
  visibleSpanIds: string[];
 };
@ -133,7 +131,6 @@ export class UnthemedSpanDetailRow extends React.PureComponent<SpanDetailRowProp
      createSpanLink,
      focusedSpanId,
      createFocusSpanLink,
-      topOfViewRefType,
      datasourceType,
      visibleSpanIds,
    } = this.props;
@ -176,7 +173,6 @@ export class UnthemedSpanDetailRow extends React.PureComponent<SpanDetailRowProp
              createSpanLink={createSpanLink}
              focusedSpanId={focusedSpanId}
              createFocusSpanLink={createFocusSpanLink}
-              topOfViewRefType={topOfViewRefType}
              datasourceType={datasourceType}
            />
          </div>
--- a/public/app/features/explore/TraceView/components/TraceTimelineViewer/VirtualizedTraceView.tsx
+++ b/public/app/features/explore/TraceView/components/TraceTimelineViewer/VirtualizedTraceView.tsx
@ -41,10 +41,7 @@ import {
  ViewedBoundsFunctionType,
 } from './utils';

-const getStyles = stylesFactory((props: TVirtualizedTraceViewOwnProps) => {
-  const { topOfViewRefType } = props;
-  const position = topOfViewRefType === TopOfViewRefType.Explore ? 'fixed' : 'absolute';
-
+const getStyles = stylesFactory(() => {
  return {
    rowsWrapper: css`
      width: 100%;
@ -59,7 +56,7 @@ const getStyles = stylesFactory((props: TVirtualizedTraceViewOwnProps) => {
      align-items: center;
      width: 40px;
      height: 40px;
-      position: ${position};
+      position: absolute;
      bottom: 30px;
      right: 30px;
      z-index: 1;
@ -73,11 +70,6 @@ type RowState = {
  spanIndex: number;
 };

-export enum TopOfViewRefType {
-  Explore = 'Explore',
-  Panel = 'Panel',
-}
-
 type TVirtualizedTraceViewOwnProps = {
  currentViewRangeTime: [number, number];
  timeZone: TimeZone;
@ -108,7 +100,6 @@ type TVirtualizedTraceViewOwnProps = {
  showCriticalPathSpansOnly: boolean;
  createFocusSpanLink: (traceId: string, spanId: string) => LinkModel;
  topOfViewRef?: RefObject<HTMLDivElement>;
-  topOfViewRefType?: TopOfViewRefType;
  datasourceType: string;
  headerHeight: number;
  criticalPath: CriticalPathSection[];
@ -494,7 +485,7 @@ export class UnthemedVirtualizedTraceView extends React.Component<VirtualizedTra
      return each.spanId === spanID;
    });

-    const styles = getStyles(this.props);
+    const styles = getStyles();
    return (
      <div className={styles.row} key={key} style={style} {...attrs}>
        <SpanBarRow
@ -554,7 +545,6 @@ export class UnthemedVirtualizedTraceView extends React.Component<VirtualizedTra
      createSpanLink,
      focusedSpanId,
      createFocusSpanLink,
-      topOfViewRefType,
      theme,
      datasourceType,
    } = this.props;
@ -563,7 +553,7 @@ export class UnthemedVirtualizedTraceView extends React.Component<VirtualizedTra
      return null;
    }
    const color = getColorByKey(serviceName, theme);
-    const styles = getStyles(this.props);
+    const styles = getStyles();

    return (
      <div className={styles.row} key={key} style={{ ...style, zIndex: 1 }} {...attrs}>
@ -590,7 +580,6 @@ export class UnthemedVirtualizedTraceView extends React.Component<VirtualizedTra
          createSpanLink={createSpanLink}
          focusedSpanId={focusedSpanId}
          createFocusSpanLink={createFocusSpanLink}
-          topOfViewRefType={topOfViewRefType}
          datasourceType={datasourceType}
          visibleSpanIds={visibleSpanIds}
        />
@ -621,7 +610,7 @@ export class UnthemedVirtualizedTraceView extends React.Component<VirtualizedTra
  });

  render() {
-    const styles = getStyles(this.props);
+    const styles = getStyles();
    const { scrollElement } = this.props;

    return (
@ -639,7 +628,7 @@ export class UnthemedVirtualizedTraceView extends React.Component<VirtualizedTra
          windowScroller={false}
          scrollElement={scrollElement}
        />
-        {this.props.topOfViewRef && (
+        {this.props.topOfViewRef && ( // only for panel as explore uses content outline to scroll to top
          <ToolbarButton
            className={styles.scrollToTopButton}
            onClick={this.scrollToTop}
--- a/public/app/features/explore/TraceView/components/TraceTimelineViewer/index.tsx
+++ b/public/app/features/explore/TraceView/components/TraceTimelineViewer/index.tsx
@ -27,7 +27,7 @@ import TTraceTimeline from '../types/TTraceTimeline';
 import { TraceSpan, Trace, TraceLog, TraceKeyValuePair, TraceLink, TraceSpanReference } from '../types/trace';

 import TimelineHeaderRow from './TimelineHeaderRow';
-import VirtualizedTraceView, { TopOfViewRefType } from './VirtualizedTraceView';
+import VirtualizedTraceView from './VirtualizedTraceView';
 import { TUpdateViewRangeTimeFunction, ViewRange, ViewRangeTimeUpdate } from './types';

 const getStyles = stylesFactory((theme: GrafanaTheme2) => {
@ -105,7 +105,6 @@ export type TProps = {
  showCriticalPathSpansOnly: boolean;
  createFocusSpanLink: (traceId: string, spanId: string) => LinkModel;
  topOfViewRef?: RefObject<HTMLDivElement>;
-  topOfViewRefType?: TopOfViewRefType;
  headerHeight: number;
  criticalPath: CriticalPathSection[];
 };
--- a/public/app/plugins/panel/traces/TracesPanel.tsx
+++ b/public/app/plugins/panel/traces/TracesPanel.tsx
@ -6,7 +6,6 @@ import { PanelProps } from '@grafana/data';
 import { getDataSourceSrv } from '@grafana/runtime';
 import { TraceView } from 'app/features/explore/TraceView/TraceView';
 import { SpanLinkFunc } from 'app/features/explore/TraceView/components';
-import { TopOfViewRefType } from 'app/features/explore/TraceView/components/TraceTimelineViewer/VirtualizedTraceView';
 import { transformDataFrames } from 'app/features/explore/TraceView/utils/transform';

 const styles = {
@ -45,7 +44,6 @@ export const TracesPanel = ({ data, options }: PanelProps<TracesPanelOptions>) =
        queryResponse={data}
        datasource={dataSource.value}
        topOfViewRef={topOfViewRef}
-        topOfViewRefType={TopOfViewRefType.Panel}
        createSpanLink={options.createSpanLink}
      />
    </div>