From c9faaf760037c818ac024b20b31ad5d33411fa9d Mon Sep 17 00:00:00 2001 From: Joey <90795735+joey-grafana@users.noreply.github.com> Date: Mon, 13 Nov 2023 11:10:19 +0000 Subject: [PATCH] Tempo: Fix missing deep span link (#77936) * Fix span deep link not showing * Update test --- public/app/features/explore/Explore.tsx | 5 +- .../app/features/explore/ExploreToolbar.tsx | 15 ++---- .../explore/TraceView/TraceView.test.tsx | 2 - .../features/explore/TraceView/TraceView.tsx | 12 +---- .../SpanDetail/index.test.tsx | 4 +- .../TraceTimelineViewer/SpanDetail/index.tsx | 51 +++++++++---------- .../TraceTimelineViewer/SpanDetailRow.tsx | 4 -- .../VirtualizedTraceView.tsx | 23 +++------ .../components/TraceTimelineViewer/index.tsx | 3 +- .../app/plugins/panel/traces/TracesPanel.tsx | 2 - 10 files changed, 39 insertions(+), 82 deletions(-) diff --git a/public/app/features/explore/Explore.tsx b/public/app/features/explore/Explore.tsx index 74914b9d3e5..81aad3edc5b 100644 --- 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; export class Explore extends React.PureComponent { scrollElement: HTMLDivElement | undefined; - topOfViewRef = createRef(); graphEventBus: EventBus; logsEventBus: EventBus; memoizedGetNodeGraphDataFrames = memoizeOne(getNodeGraphDataFrames); @@ -580,7 +579,7 @@ export class Explore extends React.PureComponent { scrollRefCallback={(scrollElement) => (this.scrollElement = scrollElement || undefined)} hideHorizontalTrack > -
+
{datasourceInstance ? ( <> diff --git a/public/app/features/explore/ExploreToolbar.tsx b/public/app/features/explore/ExploreToolbar.tsx index 78170df4bd1..6016ebf538b 100644 --- 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; } -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 ( -
+
{refreshInterval && } -
+
); diff --git a/public/app/features/explore/TraceView/TraceView.tsx b/public/app/features/explore/TraceView/TraceView.tsx index 0bac749dd80..db72d6f4beb 100644 --- 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 | undefined; topOfViewRef?: RefObject; - 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} /> diff --git a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetail/index.test.tsx b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetail/index.test.tsx index a048f3a4f9b..0348e2ed939 100644 --- 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('', () => { 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('', () => { it('renders deep link URL', () => { render(); - expect(document.getElementsByTagName('a').length).toBeGreaterThan(1); + expect(screen.getByText('test-spanID')).toBeInTheDocument(); }); }); diff --git a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetail/index.tsx b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetail/index.tsx index c54b1735d14..ab9510322d7 100644 --- 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 && ( - - {/* TODO: fix keyboard a11y */} - {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */} - { - // 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); - } - }} - > - - - {spanID} - - )} + + {/* TODO: fix keyboard a11y */} + {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */} + { + // 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); + } + }} + > + + + {spanID} +
); diff --git a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetailRow.tsx b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetailRow.tsx index 137f77c4ca4..477e0f1837f 100644 --- 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
diff --git a/public/app/features/explore/TraceView/components/TraceTimelineViewer/VirtualizedTraceView.tsx b/public/app/features/explore/TraceView/components/TraceTimelineViewer/VirtualizedTraceView.tsx index 7c45e88f548..cc8d57f38cc 100644 --- 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; - topOfViewRefType?: TopOfViewRefType; datasourceType: string; headerHeight: number; criticalPath: CriticalPathSection[]; @@ -494,7 +485,7 @@ export class UnthemedVirtualizedTraceView extends React.Component @@ -590,7 +580,6 @@ export class UnthemedVirtualizedTraceView extends React.Component @@ -621,7 +610,7 @@ export class UnthemedVirtualizedTraceView extends React.Component - {this.props.topOfViewRef && ( + {this.props.topOfViewRef && ( // only for panel as explore uses content outline to scroll to top { @@ -105,7 +105,6 @@ export type TProps = { showCriticalPathSpansOnly: boolean; createFocusSpanLink: (traceId: string, spanId: string) => LinkModel; topOfViewRef?: RefObject; - topOfViewRefType?: TopOfViewRefType; headerHeight: number; criticalPath: CriticalPathSection[]; }; diff --git a/public/app/plugins/panel/traces/TracesPanel.tsx b/public/app/plugins/panel/traces/TracesPanel.tsx index c09c812515e..ecf8261c1fa 100644 --- 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) = queryResponse={data} datasource={dataSource.value} topOfViewRef={topOfViewRef} - topOfViewRefType={TopOfViewRefType.Panel} createSpanLink={options.createSpanLink} />