From fced789daf50c863c58da5eb421c107e1ce022b9 Mon Sep 17 00:00:00 2001 From: Ben Bardin Date: Tue, 28 Feb 2023 08:54:05 -0500 Subject: [PATCH] ui/tracez: display aggregated data on children spans Spans record aggregated metadata on their children span. This PR returns that data in the debug UI snapshot response, and renders it on the span details page. Release note: None Informs: None --- docs/generated/http/full.md | 15 ++ pkg/server/admin.go | 15 +- pkg/server/serverpb/admin.proto | 7 + .../cluster-ui/src/api/tracezApi.ts | 2 + .../src/tracez/snapshot.module.scss | 21 +++ .../src/tracez/snapshot/spanComponent.tsx | 161 +++++++++++------- .../src/tracez/snapshot/spanMetadataTable.tsx | 93 ++++++++++ .../src/tracez/snapshot/spanTable.tsx | 1 - .../tracing/tracingui/span_registry_ui.go | 14 +- 9 files changed, 257 insertions(+), 72 deletions(-) create mode 100644 pkg/ui/workspaces/cluster-ui/src/tracez/snapshot/spanMetadataTable.tsx diff --git a/docs/generated/http/full.md b/docs/generated/http/full.md index 3d7fef53f7e0..b822e3f7bd69 100644 --- a/docs/generated/http/full.md +++ b/docs/generated/http/full.md @@ -7212,6 +7212,7 @@ the tracing UI. | processed_tags | [SpanTag](#cockroach.server.serverpb.GetTracingSnapshotResponse-cockroach.server.serverpb.SpanTag) | repeated | | [reserved](#support-status) | | current | [bool](#cockroach.server.serverpb.GetTracingSnapshotResponse-bool) | | current is set if the span is still alive (i.e. still present in the active spans registry). | [reserved](#support-status) | | current_recording_mode | [cockroach.util.tracing.tracingpb.RecordingMode](#cockroach.server.serverpb.GetTracingSnapshotResponse-cockroach.util.tracing.tracingpb.RecordingMode) | | current_recording_mode represents the span's current recording mode. This is not set if current == false. | [reserved](#support-status) | +| children_metadata | [NamedOperationMetadata](#cockroach.server.serverpb.GetTracingSnapshotResponse-cockroach.server.serverpb.NamedOperationMetadata) | repeated | | [reserved](#support-status) | @@ -7255,6 +7256,20 @@ of the tracing UI. + +#### NamedOperationMetadata + + + +| Field | Type | Label | Description | Support status | +| ----- | ---- | ----- | ----------- | -------------- | +| name | [string](#cockroach.server.serverpb.GetTracingSnapshotResponse-string) | | | [reserved](#support-status) | +| metadata | [cockroach.util.tracing.tracingpb.OperationMetadata](#cockroach.server.serverpb.GetTracingSnapshotResponse-cockroach.util.tracing.tracingpb.OperationMetadata) | | | [reserved](#support-status) | + + + + + #### TracingSnapshot.StacksEntry diff --git a/pkg/server/admin.go b/pkg/server/admin.go index a0ebe8f15e45..4e1443511d4a 100644 --- a/pkg/server/admin.go +++ b/pkg/server/admin.go @@ -4175,8 +4175,18 @@ func (s *adminServer) GetTracingSnapshot( for i, s := range spansList.Spans { tags := make([]*serverpb.SpanTag, len(s.Tags)) - for j, t := range s.Tags { - tags[j] = getSpanTag(t) + for j, tag := range s.Tags { + tags[j] = getSpanTag(tag) + } + childrenMetadata := make([]*serverpb.NamedOperationMetadata, len(s.ChildrenMetadata)) + + j := 0 + for name, cm := range s.ChildrenMetadata { + childrenMetadata[j] = &serverpb.NamedOperationMetadata{ + Name: name, + Metadata: cm, + } + j++ } spans[i] = &serverpb.TracingSpan{ @@ -4189,6 +4199,7 @@ func (s *adminServer) GetTracingSnapshot( ProcessedTags: tags, Current: s.Current, CurrentRecordingMode: s.CurrentRecordingMode.ToProto(), + ChildrenMetadata: childrenMetadata, } } diff --git a/pkg/server/serverpb/admin.proto b/pkg/server/serverpb/admin.proto index 635b4c194497..29ed3cf05a48 100644 --- a/pkg/server/serverpb/admin.proto +++ b/pkg/server/serverpb/admin.proto @@ -25,6 +25,7 @@ import "roachpb/metadata.proto"; import "roachpb/data.proto"; import "ts/catalog/chart_catalog.proto"; import "util/metric/metric.proto"; +import "util/tracing/tracingpb/recorded_span.proto"; import "gogoproto/gogo.proto"; import "google/api/annotations.proto"; import "google/protobuf/timestamp.proto"; @@ -1396,6 +1397,11 @@ message TracingSnapshot { map stacks = 4; } +message NamedOperationMetadata { + string name = 1; + util.tracing.tracingpb.OperationMetadata metadata = 2 [(gogoproto.nullable) = false]; +} + // TracingSpan represents a span, in a form slightly processed for the use of // the tracing UI. message TracingSpan { @@ -1412,6 +1418,7 @@ message TracingSpan { // current_recording_mode represents the span's current recording mode. This is // not set if current == false. util.tracing.tracingpb.RecordingMode current_recording_mode = 9; + repeated NamedOperationMetadata children_metadata = 10; } // SpanTag represents a tag on a tracing span, in a form processed for the use diff --git a/pkg/ui/workspaces/cluster-ui/src/api/tracezApi.ts b/pkg/ui/workspaces/cluster-ui/src/api/tracezApi.ts index ba202ee61843..7d748aafad94 100644 --- a/pkg/ui/workspaces/cluster-ui/src/api/tracezApi.ts +++ b/pkg/ui/workspaces/cluster-ui/src/api/tracezApi.ts @@ -26,6 +26,8 @@ export type GetTracingSnapshotRequest = export type GetTracingSnapshotResponse = cockroach.server.serverpb.GetTracingSnapshotResponse; +export type NamedOperationMetadata = + cockroach.server.serverpb.INamedOperationMetadata; export type Span = cockroach.server.serverpb.ITracingSpan; export type Snapshot = cockroach.server.serverpb.ITracingSnapshot; diff --git a/pkg/ui/workspaces/cluster-ui/src/tracez/snapshot.module.scss b/pkg/ui/workspaces/cluster-ui/src/tracez/snapshot.module.scss index dd0c17af434d..7687f11b2bc3 100644 --- a/pkg/ui/workspaces/cluster-ui/src/tracez/snapshot.module.scss +++ b/pkg/ui/workspaces/cluster-ui/src/tracez/snapshot.module.scss @@ -59,6 +59,17 @@ justify-content: right; } +.metadata-icon-cell { + padding: 8px 4px 4px 0px; + margin: 0px; + display: flex; + justify-content: right; +} + +.metadata-name-cell { + padding: 4px 0px 4px 0px; // For vertical, override default of 16px. +} + .table-cell-time { padding: 4px 16px; // For vertical, override default of 16px. display: flex; @@ -106,6 +117,16 @@ padding-top: 1px; } +.icon-hollow-green { + fill: none; + height: 10px; + width: 10px; + padding-top: 2px; + stroke: green; + stroke-width: 4px; + clip-path: circle(4px at 5px 6px); +} + .icon-gray { fill: lightgray; height: 10px; diff --git a/pkg/ui/workspaces/cluster-ui/src/tracez/snapshot/spanComponent.tsx b/pkg/ui/workspaces/cluster-ui/src/tracez/snapshot/spanComponent.tsx index cff7946868b1..b4c4fa71c833 100644 --- a/pkg/ui/workspaces/cluster-ui/src/tracez/snapshot/spanComponent.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/tracez/snapshot/spanComponent.tsx @@ -31,6 +31,7 @@ import "antd/lib/switch/style"; import Long from "long"; import { Button } from "src/button"; import { useHistory } from "react-router-dom"; +import { SpanMetadataTable } from "./spanMetadataTable"; const cx = classNames.bind(styles); @@ -60,6 +61,10 @@ const SpanStatus: React.FC<{ .finally(() => { setRecordingInFlight(false); }); + // Objects in hook dependencies use referential equality, not value + // equality. To force a hook refresh on span, explicitly mark spanID. But, + // having done that, there's no need to include the span, as it's purely a + // function of that input. // eslint-disable-next-line react-hooks/exhaustive-deps }, [ nodeID, @@ -108,8 +113,6 @@ const SpanStatus: React.FC<{ export const SpanComponent: React.FC<{ snapshot: GetTracingSnapshotResponse; - sort: SortSetting; - changeSortSetting: (_: SortSetting) => void; spanDetailsURL: (_: Long) => string; span: Span; @@ -126,8 +129,6 @@ export const SpanComponent: React.FC<{ }> = props => { const { snapshot, - sort, - changeSortSetting, span, spanDetailsURL, snapshotError, @@ -139,11 +140,16 @@ export const SpanComponent: React.FC<{ const snapshotID = snapshot?.snapshot.snapshot_id; const spans = snapshot?.snapshot.spans; const spanID = span?.span_id; + const childFilteredSnapshot = useMemo(() => { return { ...snapshot?.snapshot, spans: spans?.filter(s => s.parent_span_id.equals(spanID)), }; + // Objects in hook dependencies use referential equality, not value + // equality. To force a hook refresh, explicitly mark nodeID, snapshotID, + // and spanID. But, having done that, there's no need to include the + // snapshot and spans, as they're purely a function of those inputs. // eslint-disable-next-line react-hooks/exhaustive-deps }, [nodeID, snapshotID, spanID]); @@ -152,11 +158,19 @@ export const SpanComponent: React.FC<{ ...snapshot?.snapshot, spans: spans?.filter(s => s.span_id.equals(span.parent_span_id)), }; + // Objects in hook dependencies use referential equality, not value + // equality. To force a hook refresh, explicitly mark nodeID, snapshotID, + // and spanID. But, having done that, there's no need to include the + // snapshot and spans, as they're purely a function of those inputs. // eslint-disable-next-line react-hooks/exhaustive-deps }, [nodeID, snapshotID, spanID]); const snapshotTime = useMemo(() => { return TimestampToMoment(snapshot?.snapshot.captured_at); + // Objects in hook dependencies use referential equality, not value + // equality. To force a hook refresh, explicitly mark nodeID and + // snapshotID. But, having done that, there's no need to include the + // snapshot, as it's purely a function of those inputs. // eslint-disable-next-line react-hooks/exhaustive-deps }, [nodeID, snapshotID]); @@ -164,9 +178,17 @@ export const SpanComponent: React.FC<{ () => { return TimestampToMoment(span?.start); }, + // Objects in hook dependencies use referential equality, not value + // equality. To force a hook refresh, explicitly mark nodeID, snapshotID + // and spanID. But, having done that, there's no need to include the + // span, as it's purely a function of those inputs. // eslint-disable-next-line react-hooks/exhaustive-deps [nodeID, snapshotID, spanID], ); + const [childSpanSortSetting, setChildSpanSortSetting] = + useState(); + + const childrenMetadata = span?.children_metadata; const history = useHistory(); return ( @@ -187,67 +209,80 @@ export const SpanComponent: React.FC<{ View Raw Trace -
-
-
-
Snapshot Time (UTC)
- {snapshotTime.format("YYYY-MM-DD HH:mm:ss.SSS")} -
-
-
Start Time (UTC)
- {startTime.format("YYYY-MM-DD HH:mm:ss.SSS")} -
-
-
Duration
- {formatDurationHours(moment.duration(snapshotTime.diff(startTime)))} -
- -
-
-
- Tags -
- {span && } -
-
- {parentFilteredSnapshot.spans?.length > 0 && ( -
-

Parent Span

- ( - + + ( +
+
+
+
+
+ Snapshot Time (UTC) +
+ {snapshotTime.format("YYYY-MM-DD HH:mm:ss.SSS")} +
+
+
+ Start Time (UTC) +
+ {startTime.format("YYYY-MM-DD HH:mm:ss.SSS")} +
+
+
Duration
+ {formatDurationHours( + moment.duration(snapshotTime.diff(startTime)), + )} +
+ +
+
+
+ Tags +
+ {span && } +
+
+ {parentFilteredSnapshot.spans?.length > 0 && ( +
+

Parent Span

+ +
)} - /> -
- )} - {childFilteredSnapshot.spans?.length > 0 && ( -
-

Child Spans

- ( - + {childrenMetadata?.length > 0 && ( +
+

+ Aggregated Child Span Metadata +

+ + +
)} - /> -
- )} + {childFilteredSnapshot.spans?.length > 0 && ( +
+

Child Spans

+ +
+ )} + + )} + />
); diff --git a/pkg/ui/workspaces/cluster-ui/src/tracez/snapshot/spanMetadataTable.tsx b/pkg/ui/workspaces/cluster-ui/src/tracez/snapshot/spanMetadataTable.tsx new file mode 100644 index 000000000000..118b23b2040b --- /dev/null +++ b/pkg/ui/workspaces/cluster-ui/src/tracez/snapshot/spanMetadataTable.tsx @@ -0,0 +1,93 @@ +// Copyright 2022 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. +import moment from "moment"; +import React, { useState } from "react"; +import { Nodes } from "@cockroachlabs/icons"; +import { NamedOperationMetadata } from "src/api/tracezApi"; +import { EmptyTable } from "src/empty"; +import { ColumnDescriptor, SortSetting, SortedTable } from "src/sortedtable"; + +import styles from "../snapshot.module.scss"; +import classNames from "classnames/bind"; +import { CircleFilled } from "src/icon"; +import { formatDurationHours } from "./spanTable"; +import { Tooltip } from "antd"; +import "antd/lib/tooltip/style"; +const cx = classNames.bind(styles); + +class SpanMetadataSortedTable extends SortedTable {} + +const columns: ColumnDescriptor[] = [ + { + name: "icons", + title: "", + cell: row => { + return row.metadata.contains_unfinished ? ( + + + + ) : null; + }, + className: cx("metadata-icon-cell"), + }, + { + name: "name", + title: "Name", + cell: row => row.name, + sort: row => row.name, + hideTitleUnderline: true, + className: cx("metadata-name-cell"), + }, + { + name: "count", + title: "Count", + cell: row => row.metadata.count.toNumber(), + sort: row => row.metadata.count, + hideTitleUnderline: true, + className: cx("table-cell"), + }, + { + name: "duration", + title: "Duration", + cell: row => + formatDurationHours( + moment.duration(row.metadata.duration.toNumber() * 1e-6), + ), + sort: row => row.metadata.duration, + hideTitleUnderline: true, + className: cx("table-cell"), + }, +]; + +export interface SpanMetadataTableProps { + childrenMetadata: NamedOperationMetadata[]; +} + +export const SpanMetadataTable: React.FC = props => { + const { childrenMetadata } = props; + const [sortSetting, setSortSetting] = useState(); + + if (!childrenMetadata) { + return } />; + } + + return ( + cx("table-row")} + /> + ); +}; diff --git a/pkg/ui/workspaces/cluster-ui/src/tracez/snapshot/spanTable.tsx b/pkg/ui/workspaces/cluster-ui/src/tracez/snapshot/spanTable.tsx index 1b97a8b5c308..0c809da2572d 100644 --- a/pkg/ui/workspaces/cluster-ui/src/tracez/snapshot/spanTable.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/tracez/snapshot/spanTable.tsx @@ -22,7 +22,6 @@ import ISpanTag = cockroach.server.serverpb.ISpanTag; import RecordingMode = cockroach.util.tracing.tracingpb.RecordingMode; import { CircleFilled } from "src/icon"; import { Dropdown } from "src/dropdown"; -import "antd/lib/switch/style"; import { Link } from "react-router-dom"; import Long from "long"; const cx = classNames.bind(styles); diff --git a/pkg/util/tracing/tracingui/span_registry_ui.go b/pkg/util/tracing/tracingui/span_registry_ui.go index 3c8fd476c5e1..d083762b1239 100644 --- a/pkg/util/tracing/tracingui/span_registry_ui.go +++ b/pkg/util/tracing/tracingui/span_registry_ui.go @@ -124,6 +124,7 @@ type processedSpan struct { // CurrentRecordingMode indicates the spans's current recording mode. The // field is not set if Current == false. CurrentRecordingMode tracingpb.RecordingType + ChildrenMetadata map[string]tracingpb.OperationMetadata } // ProcessedTag is a span tag that was processed and expanded by processTag. @@ -194,12 +195,13 @@ func propagateInheritTagDownwards( // expanded. func processSpan(s tracingpb.RecordedSpan, snap tracing.SpansSnapshot) processedSpan { p := processedSpan{ - Operation: s.Operation, - TraceID: uint64(s.TraceID), - SpanID: uint64(s.SpanID), - ParentSpanID: uint64(s.ParentSpanID), - Start: s.StartTime, - GoroutineID: s.GoroutineID, + Operation: s.Operation, + TraceID: uint64(s.TraceID), + SpanID: uint64(s.SpanID), + ParentSpanID: uint64(s.ParentSpanID), + Start: s.StartTime, + GoroutineID: s.GoroutineID, + ChildrenMetadata: s.ChildrenMetadata, } p.Tags = make([]ProcessedTag, 0)