From 607a664ef450b18f729b32a25b9cd6988a13575a Mon Sep 17 00:00:00 2001 From: Andre Pereira Date: Wed, 4 Oct 2023 14:00:40 +0100 Subject: [PATCH] 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 since they are needed for the tests --- .betterer.results | 7 +- .../features/explore/TraceView/TraceView.tsx | 2 +- .../TraceTimelineViewer/SpanBarRow.tsx | 104 +++++++++--------- .../TraceTimelineViewer/SpanDetailRow.tsx | 5 +- .../TraceTimelineViewer/SpanLinks.tsx | 23 +++- .../SpanTreeOffset.test.tsx | 1 + .../TraceTimelineViewer/SpanTreeOffset.tsx | 24 ++-- .../VirtualizedTraceView.tsx | 44 +++++++- 8 files changed, 134 insertions(+), 76 deletions(-) diff --git a/.betterer.results b/.betterer.results index 3501bb129a1..b16051014c5 100644 --- 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"], diff --git a/public/app/features/explore/TraceView/TraceView.tsx b/public/app/features/explore/TraceView/TraceView.tsx index 22bc6555ffd..80166258735 100644 --- 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, diff --git a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanBarRow.tsx b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanBarRow.tsx index 3035e3ce64d..f74e8258d98 100644 --- 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 { theme, createSpanLink, datasourceType, + showServiceName, + visibleSpanIds, } = this.props; const { duration, @@ -432,6 +431,7 @@ export class UnthemedSpanBarRow extends React.PureComponent { hoverIndentGuideIds={hoverIndentGuideIds} addHoverIndentGuideId={addHoverIndentGuideId} removeHoverIndentGuideId={removeHoverIndentGuideId} + visibleSpanIds={visibleSpanIds} /> {createSpanLink && (() => { @@ -492,7 +494,7 @@ export class UnthemedSpanBarRow extends React.PureComponent { 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 { ); } else if (links && count > 1) { - return ; + return ; } else { return null; } diff --git a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetailRow.tsx b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanDetailRow.tsx index 03af2de9140..137f77c4ca4 100644 --- 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 { @@ -134,6 +135,7 @@ export class UnthemedSpanDetailRow extends React.PureComponent {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; 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 a20f35688c6..76ca9291da6 100644 --- 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, diff --git a/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanTreeOffset.tsx b/public/app/features/explore/TraceView/components/TraceTimelineViewer/SpanTreeOffset.tsx index 452389a5572..1e4119c2850 100644 --- 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 { @@ -133,25 +136,28 @@ export class UnthemedSpanTreeOffset extends React.PureComponent { }; 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 ? ( - + ) : ( - + )); const styles = getStyles(theme); + return ( - {this.ancestorIds.map((ancestorId) => ( + {this.ancestorIds.map((ancestorId, index) => ( { 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 0 ? trace.spans[spanIndex - 1] : null; + const styles = getStyles(this.props); return (
@@ -434,12 +450,14 @@ export class UnthemedVirtualizedTraceView extends React.Component
); } - 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 ); @@ -516,9 +536,21 @@ export class UnthemedVirtualizedTraceView extends React.Component { + 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 ( <>