diff --git a/e2e/cypress/fixtures/long-trace-response-backend.json b/e2e/cypress/fixtures/long-trace-response-backend.json index 5605ab1dd7c..56163b4b3d5 100644 --- a/e2e/cypress/fixtures/long-trace-response-backend.json +++ b/e2e/cypress/fixtures/long-trace-response-backend.json @@ -167,6 +167,16 @@ "3fa414edcef6ad90", "3fa414edcef6ad90", "3fa414edcef6ad90", + "3fa414edcef6ad90", + "3fa414edcef6ad90", + "3fa414edcef6ad90", + "3fa414edcef6ad90", + "3fa414edcef6ad90", + "3fa414edcef6ad90", + "3fa414edcef6ad90", + "3fa414edcef6ad90", + "3fa414edcef6ad90", + "3fa414edcef6ad90", "3fa414edcef6ad90" ], [ @@ -289,7 +299,17 @@ "0000000000000074", "0000000000000075", "0000000000000076", - "0000000000000077" + "0000000000000077", + "0000000000000078", + "0000000000000079", + "000000000000007a", + "000000000000007b", + "000000000000007c", + "000000000000007d", + "000000000000007e", + "000000000000007f", + "0000000000000080", + "0000000000000081" ], [ "", @@ -411,7 +431,17 @@ "0000000000000039", "000000000000003a", "000000000000003a", - "000000000000003b" + "000000000000003b", + "000000000000003b", + "0000000000000078", + "0000000000000078", + "0000000000000079", + "0000000000000079", + "000000000000007a", + "000000000000007a", + "000000000000007b", + "000000000000007b", + "000000000000007c" ], [ "GET /api/health", @@ -533,7 +563,17 @@ "GET /rollback/prepare", "POST /commit/save", "GET /branch/merge", - "POST /tag/create" + "POST /tag/create", + "GET /release/notes", + "POST /changelog/generate", + "GET /documentation/build", + "POST /test/run", + "GET /coverage/report", + "POST /artifact/publish", + "GET /registry/pull", + "POST /image/push", + "GET /manifest/inspect", + "POST /deployment/verify" ], [ "api-gateway", @@ -655,7 +695,17 @@ "rollback-mgr", "git-api", "merge-api", - "tag-api" + "tag-api", + "release-api", + "changelog-gen", + "doc-builder", + "test-runner", + "coverage-tool", + "artifact-mgr", + "registry-client", + "image-pusher", + "manifest-parser", + "deploy-verifier" ], [ 1579270400000, 1579270401000, 1579270402000, 1579270403000, 1579270404000, 1579270405000, 1579270406000, @@ -675,7 +725,8 @@ 1579270498000, 1579270499000, 1579270500000, 1579270501000, 1579270502000, 1579270503000, 1579270504000, 1579270505000, 1579270506000, 1579270507000, 1579270508000, 1579270509000, 1579270510000, 1579270511000, 1579270512000, 1579270513000, 1579270514000, 1579270515000, 1579270516000, 1579270517000, 1579270518000, - 1579270519000 + 1579270519000, 1579270520000, 1579270521000, 1579270522000, 1579270523000, 1579270524000, 1579270525000, + 1579270526000, 1579270527000, 1579270528000, 1579270529000 ], [ 100000, 50000, 75000, 200000, 25000, 150000, 80000, 300000, 45000, 120000, 60000, 180000, 35000, 250000, @@ -686,7 +737,8 @@ 190000, 58000, 145000, 92000, 220000, 48000, 175000, 85000, 205000, 65000, 180000, 72000, 195000, 55000, 160000, 98000, 225000, 62000, 140000, 78000, 200000, 48000, 165000, 85000, 215000, 58000, 150000, 88000, 230000, 45000, 175000, 72000, 185000, 62000, 155000, 95000, 210000, 52000, 170000, 82000, 195000, 68000, - 145000, 92000, 235000, 58000, 160000, 75000, 180000, 65000 + 145000, 92000, 235000, 58000, 160000, 75000, 180000, 65000, 125000, 78000, 190000, 52000, 165000, 88000, + 215000, 62000, 155000, 95000 ] ] } diff --git a/e2e/old-arch/various-suite/trace-view-scrolling.spec.ts b/e2e/old-arch/various-suite/trace-view-scrolling.spec.ts index f73c3c6f2c1..372c2d11032 100644 --- a/e2e/old-arch/various-suite/trace-view-scrolling.spec.ts +++ b/e2e/old-arch/various-suite/trace-view-scrolling.spec.ts @@ -33,15 +33,15 @@ describe('Trace view', () => { e2e.components.TraceViewer.spanBar().should('be.visible'); + e2e.components.TraceViewer.spanBar().its('length').should('be.equal', 100); + + e2e.pages.Explore.General.scrollView().children().first().scrollTo('bottom'); + + // After scrolling we should see 50 spans e2e.components.TraceViewer.spanBar() .its('length') - .then((oldLength) => { - e2e.pages.Explore.General.scrollView().children().first().scrollTo('center'); - - // After scrolling we should load more spans - e2e.components.TraceViewer.spanBar().should(($span) => { - expect($span.length).to.be.gt(oldLength); - }); + .should(($span) => { + expect($span).to.be.equal(50); }); }); }); diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 6392492ae77..5456b0d38dd 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -2886,31 +2886,11 @@ "count": 1 } }, - "public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanBarRow.tsx": { - "react-prefer-function-component/react-prefer-function-component": { - "count": 1 - } - }, "public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetail/SpanDetailLinkButtons.tsx": { "react-hooks/rules-of-hooks": { "count": 1 } }, - "public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetailRow.tsx": { - "react-prefer-function-component/react-prefer-function-component": { - "count": 1 - } - }, - "public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanLinks.tsx": { - "no-restricted-syntax": { - "count": 1 - } - }, - "public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanTreeOffset.tsx": { - "react-prefer-function-component/react-prefer-function-component": { - "count": 1 - } - }, "public/app/features/explore/TraceView/components/TraceTimelineViewer/TimelineHeaderRow/TimelineColumnResizer.tsx": { "react-prefer-function-component/react-prefer-function-component": { "count": 1 diff --git a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanBar.tsx b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanBar.tsx index 0e447aa0ce0..77981605295 100644 --- a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanBar.tsx +++ b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanBar.tsx @@ -45,11 +45,11 @@ const getStyles = (theme: GrafanaTheme2) => { }), bar: css({ label: 'bar', - borderRadius: theme.shape.radius.default, + borderRadius: theme.shape.radius.sm, minWidth: '2px', position: 'absolute', - height: '36%', - top: '32%', + height: '40%', + top: '30%', }), rpc: css({ label: 'rpc', @@ -93,7 +93,7 @@ const getStyles = (theme: GrafanaTheme2) => { }), criticalPath: css({ position: 'absolute', - top: '45%', + top: '44%', height: '11%', zIndex: 2, overflow: 'hidden', diff --git a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanBarRow.tsx b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanBarRow.tsx index 778743894cb..4a27ce2105e 100644 --- a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanBarRow.tsx +++ b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanBarRow.tsx @@ -41,7 +41,7 @@ const nameWrapperMatchingFilterClassName = 'nameWrapperMatchingFilter'; const viewClassName = 'jaegerView'; const nameColumnClassName = 'nameColumn'; -const getStyles = stylesFactory((theme: GrafanaTheme2, showSpanFilterMatchesOnly: boolean) => { +const getStyles = stylesFactory((theme: GrafanaTheme2, showSpanFilterMatchesOnly: boolean, serviceColor: string) => { const animations = { flash: keyframes` from { @@ -60,10 +60,18 @@ const getStyles = stylesFactory((theme: GrafanaTheme2, showSpanFilterMatchesOnly lineHeight: '27px', overflow: 'hidden', display: 'flex', + + [`& > *`]: { + background: theme.colors.background.secondary, + }, }), nameWrapperMatchingFilter: css({ label: 'nameWrapperMatchingFilter', backgroundColor: backgroundColor, + + [`& > *`]: { + background: backgroundColor, + }, }), nameColumn: css({ label: 'nameColumn', @@ -96,6 +104,7 @@ const getStyles = stylesFactory((theme: GrafanaTheme2, showSpanFilterMatchesOnly row: css({ label: 'row', fontSize: '0.9em', + [`&:hover .${spanBarClassName}`]: { opacity: 1, }, @@ -114,6 +123,11 @@ const getStyles = stylesFactory((theme: GrafanaTheme2, showSpanFilterMatchesOnly backgroundColor: autoColor(theme, '#f5f5f5'), outline: `1px solid ${autoColor(theme, '#ddd')}`, }, + ['& .icon-wrapper']: { + borderBottomColor: `${serviceColor}CF`, + borderBottomWidth: '2px', + borderBottomStyle: 'solid', + }, }), rowClippingLeft: css({ label: 'rowClippingLeft', @@ -168,7 +182,7 @@ const getStyles = stylesFactory((theme: GrafanaTheme2, showSpanFilterMatchesOnly }), rowMatchingFilter: css({ label: 'rowMatchingFilter', - // background-color: ${autoColor(theme, '#fffbde')}; + [`&:hover .${nameWrapperClassName}`]: { background: `linear-gradient( 90deg, @@ -200,9 +214,22 @@ const getStyles = stylesFactory((theme: GrafanaTheme2, showSpanFilterMatchesOnly [`& .${spanBarLabelClassName}`]: { color: autoColor(theme, '#000'), }, - ['&:hover .${nameWrapperClassName}, :hover .${viewClassName}']: { - background: autoColor(theme, '#d5ebff'), - boxShadow: `0 1px 0 ${autoColor(theme, '#ddd')}`, + }), + + rowError: css({ + label: 'rowError', + backgroundColor: theme.colors.error.transparent, + + [`&:hover .${nameWrapperClassName}`]: { + background: theme.colors.error.borderTransparent, + }, + [`&:hover .${viewClassName}`]: { + backgroundColor: theme.colors.error.borderTransparent, + outline: `1px solid ${theme.colors.error.borderTransparent}`, + }, + + [`& .${nameWrapperClassName} > *`]: { + background: theme.colors.error.transparent, }, }), @@ -221,8 +248,7 @@ const getStyles = stylesFactory((theme: GrafanaTheme2, showSpanFilterMatchesOnly outline: 'none', overflowY: 'hidden', overflowX: 'auto', - paddingLeft: '4px', - paddingRight: '0.25em', + padding: '4px', position: 'relative', '-ms-overflow-style': 'none', scrollbarWidth: 'none', @@ -236,9 +262,9 @@ const getStyles = stylesFactory((theme: GrafanaTheme2, showSpanFilterMatchesOnly color: autoColor(theme, '#000'), }, textAlign: 'left', - background: 'transparent', border: 'none', - borderBottomWidth: '1px', + borderBottomColor: `${serviceColor}CF`, + borderBottomWidth: '2px', borderBottomStyle: 'solid', }), nameDetailExpanded: css({ @@ -250,27 +276,25 @@ const getStyles = stylesFactory((theme: GrafanaTheme2, showSpanFilterMatchesOnly svcName: css({ label: 'svcName', fontSize: '0.9em', - fontWeight: 'bold', + fontWeight: '500', marginRight: '0.25rem', }), svcNameChildrenCollapsed: css({ label: 'svcNameChildrenCollapsed', - fontWeight: 'bold', + fontWeight: '500', fontStyle: 'italic', }), errorIcon: css({ label: 'errorIcon', - // eslint-disable-next-line @grafana/no-border-radius-literal - borderRadius: '6.5px', + borderRadius: theme.shape.radius.md, color: autoColor(theme, '#fff'), - fontSize: '0.85em', + fontSize: '0.6em', marginRight: '0.25rem', padding: '1px', }), rpcColorMarker: css({ label: 'rpcColorMarker', - // eslint-disable-next-line @grafana/no-border-radius-literal - borderRadius: '6.5px', + borderRadius: theme.shape.radius.md, display: 'inline-block', fontSize: '0.85em', height: '1em', @@ -335,257 +359,252 @@ export type SpanBarRowProps = { criticalPath: CriticalPathSection[]; }; -/** - * This was originally a stateless function, but changing to a PureComponent - * reduced the render time of expanding a span row detail by ~50%. This is - * even true in the case where the stateless function has the same prop types as - * this class and arrow functions are created in the stateless function as - * handlers to the onClick props. E.g. for now, the PureComponent is more - * performance than the stateless function. - */ -export class UnthemedSpanBarRow extends React.PureComponent { - static displayName = 'UnthemedSpanBarRow'; - static defaultProps: Partial = { - className: '', - rpc: null, - }; +const UnthemedSpanBarRow = React.memo((props) => { + const { + className = '', + color, + spanBarOptions, + columnDivision, + isChildrenExpanded, + isDetailExpanded, + isMatchingFilter, + showSpanFilterMatchesOnly, + isFocused, + numTicks, + rpc = null, + noInstrumentedServer, + showErrorIcon, + getViewedBounds, + traceStartTime, + span, + hoverIndentGuideIds, + addHoverIndentGuideId, + removeHoverIndentGuideId, + clippingLeft, + clippingRight, + theme, + createSpanLink, + datasourceType, + showServiceName, + visibleSpanIds, + criticalPath, + onDetailToggled, + onChildrenToggled, + } = props; - _detailToggle = () => { - this.props.onDetailToggled(this.props.span.spanID); - }; + const { + duration, + hasChildren: isParent, + operationName, + process: { serviceName }, + } = span; + const label = formatDuration(duration); - _childrenToggle = () => { - this.props.onChildrenToggled(this.props.span.spanID); - }; + const viewBounds = getViewedBounds(span.startTime, span.startTime + span.duration); + const viewStart = viewBounds.start; + const viewEnd = viewBounds.end; + const styles = getStyles(theme, showSpanFilterMatchesOnly, color); - render() { - const { - className, - color, - spanBarOptions, - columnDivision, - isChildrenExpanded, - isDetailExpanded, - isMatchingFilter, - showSpanFilterMatchesOnly, - isFocused, - numTicks, - rpc, - noInstrumentedServer, - showErrorIcon, - getViewedBounds, - traceStartTime, - span, - hoverIndentGuideIds, - addHoverIndentGuideId, - removeHoverIndentGuideId, - clippingLeft, - clippingRight, - theme, - createSpanLink, - datasourceType, - showServiceName, - visibleSpanIds, - criticalPath, - } = this.props; - const { - duration, - hasChildren: isParent, - operationName, - process: { serviceName }, - } = span; - const label = formatDuration(duration); - - const viewBounds = getViewedBounds(span.startTime, span.startTime + span.duration); - const viewStart = viewBounds.start; - const viewEnd = viewBounds.end; - const styles = getStyles(theme, showSpanFilterMatchesOnly); - - const labelDetail = `${serviceName}::${operationName}`; - let longLabel; - let hintClassName; - if (viewStart > 1 - viewEnd) { - longLabel = `${labelDetail} | ${label}`; - hintClassName = styles.labelLeft; - } else { - longLabel = `${label} | ${labelDetail}`; - hintClassName = styles.labelRight; - } - - return ( - - -
- - - {createSpanLink && - (() => { - const links = createSpanLink(span); - const count = links?.length || 0; - if (links && count === 1) { - if (!links[0]) { - return null; - } - - return ( - { - if (!(event.ctrlKey || event.metaKey || event.shiftKey) && links[0].onClick) { - event.preventDefault(); - links[0].onClick(event); - } - } - : undefined - } - > - {links[0].content} - - ); - } else if (links && count > 1) { - return ; - } else { - return null; - } - })()} -
- - - - - - - ); + const labelDetail = `${serviceName}::${operationName}`; + let longLabel; + let hintClassName; + if (viewStart > 1 - viewEnd) { + longLabel = `${labelDetail} | ${label}`; + hintClassName = styles.labelLeft; + } else { + longLabel = `${label} | ${labelDetail}`; + hintClassName = styles.labelRight; } - getSpanBarLabel = (span: TraceSpan, spanBarOptions: SpanBarOptions | undefined, duration: string) => { - const type = spanBarOptions?.type ?? ''; + const handleDetailToggle = React.useCallback(() => { + onDetailToggled(span.spanID); + }, [onDetailToggled, span.spanID]); - if (type === NONE) { - return ''; - } else if (type === '' || type === DURATION) { - return `(${duration})`; - } else if (type === TAG) { - const tagKey = spanBarOptions?.tag?.trim() ?? ''; - if (tagKey !== '' && span.tags) { - const tag = span.tags?.find((tag: TraceKeyValuePair) => { - return tag.key === tagKey; - }); - if (tag) { - return `(${tag.value})`; - } + const handleChildrenToggle = React.useCallback(() => { + onChildrenToggled(span.spanID); + }, [onChildrenToggled, span.spanID]); - const process = span.process?.tags?.find((process: TraceKeyValuePair) => { - return process.key === tagKey; - }); - if (process) { - return `(${process.value})`; + const getSpanBarLabel = React.useCallback( + (span: TraceSpan, spanBarOptions: SpanBarOptions | undefined, duration: string) => { + const type = spanBarOptions?.type ?? ''; + + if (type === NONE) { + return ''; + } else if (type === '' || type === DURATION) { + return `(${duration})`; + } else if (type === TAG) { + const tagKey = spanBarOptions?.tag?.trim() ?? ''; + if (tagKey !== '' && span.tags) { + const tag = span.tags?.find((tag: TraceKeyValuePair) => { + return tag.key === tagKey; + }); + if (tag) { + return `(${tag.value})`; + } + + const process = span.process?.tags?.find((process: TraceKeyValuePair) => { + return process.key === tagKey; + }); + if (process) { + return `(${process.value})`; + } } } - } - return ''; - }; -} + return ''; + }, + [] + ); + + return ( + + +
+ + + {createSpanLink && + (() => { + const links = createSpanLink(span); + const count = links?.length || 0; + if (links && count === 1) { + if (!links[0]) { + return null; + } + + return ( + { + if (!(event.ctrlKey || event.metaKey || event.shiftKey) && links[0].onClick) { + event.preventDefault(); + links[0].onClick(event); + } + } + : undefined + } + > + {links[0].content} + + ); + } else if (links && count > 1) { + return ; + } else { + return null; + } + })()} +
+ + + + + + + ); +}); + +UnthemedSpanBarRow.displayName = 'UnthemedSpanBarRow'; export default withTheme2(UnthemedSpanBarRow); diff --git a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetailRow.test.tsx b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetailRow.test.tsx index c4c94b9b102..c4edb95f7d2 100644 --- a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetailRow.test.tsx +++ b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetailRow.test.tsx @@ -18,7 +18,7 @@ import { createTheme, dateTime } from '@grafana/data'; import { setPluginLinksHook } from '@grafana/runtime'; import DetailState from './SpanDetail/DetailState'; -import { UnthemedSpanDetailRow, SpanDetailRowProps } from './SpanDetailRow'; +import SpanDetailRow, { type SpanDetailRowProps } from './SpanDetailRow'; const testSpan = { spanID: 'testSpanID', @@ -57,7 +57,7 @@ const setup = (propOverrides?: SpanDetailRowProps) => { }, ...propOverrides, }; - return render(); + return render(); }; describe('SpanDetailRow tests', () => { diff --git a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetailRow.tsx b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetailRow.tsx index 201f0570b27..7312daaccab 100644 --- a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetailRow.tsx +++ b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetailRow.tsx @@ -13,7 +13,7 @@ // limitations under the License. import { css } from '@emotion/css'; -import { PureComponent } from 'react'; +import React from 'react'; import { CoreApp, GrafanaTheme2, LinkModel, TimeRange, TraceLog } from '@grafana/data'; import { TraceToProfilesOptions } from '@grafana/o11y-ds-frontend'; @@ -117,95 +117,93 @@ export type SpanDetailRowProps = { app: CoreApp; }; -export class UnthemedSpanDetailRow extends PureComponent { - _detailToggle = () => { - this.props.onDetailToggled(this.props.span.spanID); - }; +const UnthemedSpanDetailRow = React.memo((props) => { + const { + color, + detailState, + logItemToggle, + logsToggle, + processToggle, + referenceItemToggle, + referencesToggle, + warningsToggle, + stackTracesToggle, + span, + traceToProfilesOptions, + timeZone, + tagsToggle, + traceStartTime, + traceDuration, + traceName, + theme, + createSpanLink, + focusedSpanId, + createFocusSpanLink, + datasourceType, + datasourceUid, + traceFlameGraphs, + setTraceFlameGraphs, + setRedrawListView, + timeRange, + app, + hoverIndentGuideIds, + addHoverIndentGuideId, + removeHoverIndentGuideId, + visibleSpanIds, + } = props; - render() { - const { - color, - detailState, - logItemToggle, - logsToggle, - processToggle, - referenceItemToggle, - referencesToggle, - warningsToggle, - stackTracesToggle, - span, - traceToProfilesOptions, - timeZone, - tagsToggle, - traceStartTime, - traceDuration, - traceName, - theme, - createSpanLink, - focusedSpanId, - createFocusSpanLink, - datasourceType, - datasourceUid, - traceFlameGraphs, - setTraceFlameGraphs, - setRedrawListView, - timeRange, - app, - hoverIndentGuideIds, - addHoverIndentGuideId, - removeHoverIndentGuideId, - visibleSpanIds, - } = this.props; - const styles = getStyles(theme); - return ( - - -
- + +
+ +
+
+
+
-
-
- -
-
- - - ); - } -} +
+ + + ); +}); + +UnthemedSpanDetailRow.displayName = 'UnthemedSpanDetailRow'; export default withTheme2(UnthemedSpanDetailRow); diff --git a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanLinks.tsx b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanLinks.tsx index 4f2f68f9c5b..4d653f60360 100644 --- a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanLinks.tsx +++ b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanLinks.tsx @@ -18,7 +18,8 @@ const renderMenuItems = ( closeMenu: () => void, datasourceType: string ) => { - links.sort(function (linkA, linkB) { + links.sort((linkA, linkB) => { + // eslint-disable-next-line no-restricted-syntax return (linkA.title || 'link').toLowerCase().localeCompare((linkB.title || 'link').toLowerCase()); }); @@ -88,7 +89,7 @@ const getStyles = (color: string) => ({ border: 'none', background: `${color}10`, borderBottom: `1px solid ${color}CF`, - paddingRight: '4px', + paddingInline: '4px', }), button: css({ background: 'transparent', diff --git a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanTreeOffset.test.tsx b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanTreeOffset.test.tsx index 463b2035841..e9c0ad9f9ad 100644 --- a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanTreeOffset.test.tsx +++ b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanTreeOffset.test.tsx @@ -47,15 +47,32 @@ describe('SpanTreeOffset', () => { }); describe('.SpanTreeOffset--indentGuide', () => { - it('renders only one SpanTreeOffset--indentGuide for entire trace if span has no ancestors', () => { + it('renders no indentGuide if span has no ancestors and no children', () => { jest.mocked(spanAncestorIdsSpy).mockReturnValue([]); render(); + const indentGuides = screen.queryAllByTestId('SpanTreeOffset--indentGuide'); + expect(indentGuides.length).toBe(0); + }); + + it('renders only one SpanTreeOffset--indentGuide for entire trace if span has no ancestors but has children', () => { + jest.mocked(spanAncestorIdsSpy).mockReturnValue([]); + props.span.hasChildren = true; + render(); const indentGuide = screen.getByTestId('SpanTreeOffset--indentGuide'); expect(indentGuide).toBeInTheDocument(); expect(indentGuide).toHaveAttribute('data-ancestor-id', specialRootID); }); - it('renders one SpanTreeOffset--indentGuide per ancestor span, plus one for entire trace', () => { + it('renders one SpanTreeOffset--indentGuide per ancestor span when span has no children', () => { + render(); + const indentGuides = screen.getAllByTestId('SpanTreeOffset--indentGuide'); + expect(indentGuides.length).toBe(2); + expect(indentGuides[0]).toHaveAttribute('data-ancestor-id', specialRootID); + expect(indentGuides[1]).toHaveAttribute('data-ancestor-id', rootSpanID); + }); + + it('renders one SpanTreeOffset--indentGuide per ancestor span, plus one for entire trace when span has children', () => { + props.span.hasChildren = true; render(); const indentGuides = screen.getAllByTestId('SpanTreeOffset--indentGuide'); expect(indentGuides.length).toBe(3); @@ -65,28 +82,28 @@ describe('SpanTreeOffset', () => { }); it('adds .is-active to correct indentGuide', () => { - props.hoverIndentGuideIds = new Set([parentSpanID]); + props.hoverIndentGuideIds = new Set([rootSpanID]); render(); const styles = getStyles(createTheme()); const activeIndentGuide = document.querySelector(`.${styles.indentGuideActive}`); expect(activeIndentGuide).toBeInTheDocument(); - expect(activeIndentGuide).toHaveAttribute('data-ancestor-id', parentSpanID); + expect(activeIndentGuide).toHaveAttribute('data-ancestor-id', rootSpanID); }); it('calls props.addHoverIndentGuideId on mouse enter', async () => { render(); - const span = document.querySelector(`[data-ancestor-id=${parentSpanID}]`); + const span = document.querySelector(`[data-ancestor-id=${rootSpanID}]`); await userEvent.hover(span!); expect(props.addHoverIndentGuideId).toHaveBeenCalledTimes(1); - expect(props.addHoverIndentGuideId).toHaveBeenCalledWith(parentSpanID); + expect(props.addHoverIndentGuideId).toHaveBeenCalledWith(rootSpanID); }); it('calls props.removeHoverIndentGuideId on mouse leave', async () => { render(); - const span = document.querySelector(`[data-ancestor-id=${parentSpanID}]`); + const span = document.querySelector(`[data-ancestor-id=${rootSpanID}]`); await userEvent.unhover(span!); expect(props.removeHoverIndentGuideId).toHaveBeenCalledTimes(1); - expect(props.removeHoverIndentGuideId).toHaveBeenCalledWith(parentSpanID); + expect(props.removeHoverIndentGuideId).toHaveBeenCalledWith(rootSpanID); }); }); diff --git a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanTreeOffset.tsx b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanTreeOffset.tsx index 72a985faad4..1d2bea3105e 100644 --- a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanTreeOffset.tsx +++ b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanTreeOffset.tsx @@ -64,6 +64,8 @@ export const getStyles = stylesFactory((theme: GrafanaTheme2) => ({ label: 'iconWrapper', position: 'absolute', right: 0, + height: '100%', + paddingTop: '1px', }), })); @@ -80,26 +82,27 @@ export type TProps = { visibleSpanIds: string[]; }; -export class UnthemedSpanTreeOffset extends React.PureComponent { - static displayName = 'UnthemedSpanTreeOffset'; +const UnthemedSpanTreeOffset = React.memo((props) => { + const { + childrenVisible = false, + showChildrenIcon = true, + onClick, + span, + theme, + visibleSpanIds, + hoverIndentGuideIds, + addHoverIndentGuideId, + removeHoverIndentGuideId, + } = props; - ancestorIds: string[]; - - static defaultProps = { - childrenVisible: false, - showChildrenIcon: true, - }; - - constructor(props: TProps) { - super(props); - - this.ancestorIds = spanAncestorIds(props.span); + const ancestorIds = React.useMemo(() => { + const ids = spanAncestorIds(span); // Some traces have multiple root-level spans, this connects them all under one guideline and adds the // necessary padding for the collapse icon on root-level spans. - this.ancestorIds.push('root'); - - this.ancestorIds.reverse(); - } + ids.push('root'); + ids.reverse(); + return ids; + }, [span]); /** * If the mouse leaves to anywhere except another span with the same ancestor id, this span's ancestor id is @@ -109,14 +112,17 @@ export class UnthemedSpanTreeOffset extends React.PureComponent { * the element the user is now hovering. * @param {string} ancestorId - The span id that the user was hovering over. */ - handleMouseLeave = (event: React.MouseEvent, ancestorId: string) => { - if ( - !(event.relatedTarget instanceof HTMLSpanElement) || - _get(event, 'relatedTarget.dataset.ancestorId') !== ancestorId - ) { - this.props.removeHoverIndentGuideId(ancestorId); - } - }; + const handleMouseLeave = React.useCallback( + (event: React.MouseEvent, ancestorId: string) => { + if ( + !(event.relatedTarget instanceof HTMLSpanElement) || + _get(event, 'relatedTarget.dataset.ancestorId') !== ancestorId + ) { + removeHoverIndentGuideId(ancestorId); + } + }, + [removeHoverIndentGuideId] + ); /** * If the mouse entered this span from anywhere except another span with the same ancestor id, this span's @@ -126,58 +132,65 @@ export class UnthemedSpanTreeOffset extends React.PureComponent { * the last element the user was hovering. * @param {string} ancestorId - The span id that the user is now hovering over. */ - handleMouseEnter = (event: React.MouseEvent, ancestorId: string) => { - if ( - !(event.relatedTarget instanceof HTMLSpanElement) || - _get(event, 'relatedTarget.dataset.ancestorId') !== ancestorId - ) { - this.props.addHoverIndentGuideId(ancestorId); - } - }; + const handleMouseEnter = React.useCallback( + (event: React.MouseEvent, ancestorId: string) => { + if ( + !(event.relatedTarget instanceof HTMLSpanElement) || + _get(event, 'relatedTarget.dataset.ancestorId') !== ancestorId + ) { + addHoverIndentGuideId(ancestorId); + } + }, + [addHoverIndentGuideId] + ); - render() { - 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 ? ( - - ) : ( - - )); - const styles = getStyles(theme); + const { hasChildren, spanID } = span; + const wrapperProps = hasChildren ? { onClick, role: 'switch', 'aria-checked': childrenVisible } : null; + const icon = + showChildrenIcon && + hasChildren && + (childrenVisible ? ( + + ) : ( + + )); + const styles = getStyles(theme); - return ( - - {this.ancestorIds.map((ancestorId, index) => ( - this.handleMouseEnter(event, ancestorId)} - onMouseLeave={(event) => this.handleMouseLeave(event, ancestorId)} - /> - ))} - {icon && ( - this.handleMouseEnter(event, spanID)} - onMouseLeave={(event) => this.handleMouseLeave(event, spanID)} - data-testid="icon-wrapper" - > - {icon} - - )} - - ); - } -} + // If span has no children, don't show the last indent guide + const displayedAncestorIds = hasChildren ? ancestorIds : ancestorIds.slice(0, -1); + + return ( + + {displayedAncestorIds.map((ancestorId, index) => ( + handleMouseEnter(event, ancestorId)} + onMouseLeave={(event) => handleMouseLeave(event, ancestorId)} + /> + ))} + {icon && ( + handleMouseEnter(event, spanID)} + onMouseLeave={(event) => handleMouseLeave(event, spanID)} + data-testid="icon-wrapper" + > + {icon} + + )} + + ); +}); + +UnthemedSpanTreeOffset.displayName = 'UnthemedSpanTreeOffset'; export default withTheme2(UnthemedSpanTreeOffset);