Skip to content

Commit 11b95bc

Browse files
gggritsopriscilawebdev
authored andcommitted
feat(otlp): Collapse Trace Waterfall spans based on span name (#97706)
If the user has OTel-friendly UI enabled, update the logic that figures out which sibling spans should be collapsed. Since the _labels_ for the spans use the op and name, the collapsing logic also needs to use op and name, rather than op and description. Unfortunately, in order to check the feature flag, I had to pass `organization` through _everywhere_ in the `TraceTree` class. There are so many call sites, and passing an `organization` in the options is pattern that already exists, so I followed it here, too. One alternative is to do a shenanigan, and to fill the `description` field of a span with the name, and let everything else work the same, but that feels like it'll just cause more confusion.
1 parent 295e8b1 commit 11b95bc

16 files changed

+351
-143
lines changed

static/app/views/insights/agents/hooks/useAITrace.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ export function useAITrace(traceSlug: string): UseAITraceResult {
6565
meta: meta.data,
6666
replay: null,
6767
preferences: DEFAULT_TRACE_VIEW_PREFERENCES,
68+
organization,
6869
});
6970

7071
tree.build();

static/app/views/performance/newTraceDetails/traceApi/useIssuesTraceTree.tsx

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,13 @@ export function useIssuesTraceTree({
4949
setTree(t =>
5050
t.type === 'error'
5151
? t
52-
: IssuesTraceTree.Error({
53-
project_slug: projects?.[0]?.slug ?? '',
54-
event_id: traceSlug,
55-
})
52+
: IssuesTraceTree.Error(
53+
{
54+
project_slug: projects?.[0]?.slug ?? '',
55+
event_id: traceSlug,
56+
},
57+
organization
58+
)
5659
);
5760
traceAnalytics.trackTraceErrorState(organization, 'issue_details');
5861
return;
@@ -68,10 +71,13 @@ export function useIssuesTraceTree({
6871
setTree(t =>
6972
t.type === 'loading'
7073
? t
71-
: IssuesTraceTree.Loading({
72-
project_slug: projects?.[0]?.slug ?? '',
73-
event_id: traceSlug,
74-
})
74+
: IssuesTraceTree.Loading(
75+
{
76+
project_slug: projects?.[0]?.slug ?? '',
77+
event_id: traceSlug,
78+
},
79+
organization
80+
)
7581
);
7682
return;
7783
}
@@ -81,6 +87,7 @@ export function useIssuesTraceTree({
8187
meta: null,
8288
replay,
8389
preferences: traceState.preferences,
90+
organization,
8491
});
8592

8693
setTree(newTree);

static/app/views/performance/newTraceDetails/traceApi/useTraceTree.tsx

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,13 @@ export function useTraceTree({trace, replay, traceSlug}: UseTraceTreeParams): Tr
3232
setTree(t =>
3333
t.type === 'error'
3434
? t
35-
: TraceTree.Error({
36-
project_slug: projects?.[0]?.slug ?? '',
37-
event_id: traceSlug,
38-
})
35+
: TraceTree.Error(
36+
{
37+
project_slug: projects?.[0]?.slug ?? '',
38+
event_id: traceSlug,
39+
},
40+
organization
41+
)
3942
);
4043
traceAnalytics.trackTraceErrorState(organization, traceWaterfallSource);
4144
return;
@@ -51,10 +54,13 @@ export function useTraceTree({trace, replay, traceSlug}: UseTraceTreeParams): Tr
5154
setTree(t =>
5255
t.type === 'loading'
5356
? t
54-
: TraceTree.Loading({
55-
project_slug: projects?.[0]?.slug ?? '',
56-
event_id: traceSlug,
57-
})
57+
: TraceTree.Loading(
58+
{
59+
project_slug: projects?.[0]?.slug ?? '',
60+
event_id: traceSlug,
61+
},
62+
organization
63+
)
5864
);
5965
return;
6066
}
@@ -64,6 +70,7 @@ export function useTraceTree({trace, replay, traceSlug}: UseTraceTreeParams): Tr
6470
meta: null,
6571
replay,
6672
preferences: traceState.preferences,
73+
organization,
6774
});
6875

6976
setTree(newTree);

static/app/views/performance/newTraceDetails/traceModels/__snapshots__/traceTree.autogrouping.spec.tsx.snap

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Jest Snapshot v1, https://goo.gl/fbAQLP
1+
// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing
22

33
exports[`autogrouping parent autogrouping assigns children to tail node 1`] = `
44
"
@@ -245,6 +245,14 @@ eap trace root
245245
"
246246
`;
247247

248+
exports[`autogrouping sibling autogrouping eap traces groups spans with the same op and name if OTel UI is enabled 1`] = `
249+
"
250+
eap trace root
251+
http.server - redis
252+
sibling autogroup (db: 5)
253+
"
254+
`;
255+
248256
exports[`autogrouping sibling autogrouping eap traces removes sibling autogroup 1`] = `
249257
"
250258
eap trace root

static/app/views/performance/newTraceDetails/traceModels/issuesTraceTree.spec.tsx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ const errorsOnlyTrace = makeTrace({
4646
orphan_errors: new Array(20).fill(null).map(() => makeTraceError({})),
4747
});
4848

49+
const organization = OrganizationFixture();
50+
4951
function mockSpansResponse(
5052
spans: TraceTree.Span[],
5153
project_slug: string,
@@ -72,6 +74,7 @@ describe('IssuesTraceTree', () => {
7274
const tree = IssuesTraceTree.FromTrace(traceWithErrorInMiddle, {
7375
meta: null,
7476
replay: null,
77+
organization,
7578
});
7679

7780
const issues = IssuesTraceTree.FindAll(tree.root, hasErrors);
@@ -82,6 +85,7 @@ describe('IssuesTraceTree', () => {
8285
const tree = IssuesTraceTree.FromTrace(traceWithChildError, {
8386
meta: null,
8487
replay: null,
88+
organization,
8589
});
8690

8791
const error = IssuesTraceTree.Find(tree.root, hasErrors);
@@ -101,6 +105,7 @@ describe('IssuesTraceTree', () => {
101105
const tree = IssuesTraceTree.FromTrace(errorsOnlyTrace, {
102106
meta: null,
103107
replay: null,
108+
organization,
104109
});
105110

106111
const errors = IssuesTraceTree.FindAll(tree.root, hasErrors).slice(0, 10);
@@ -111,6 +116,7 @@ describe('IssuesTraceTree', () => {
111116
const tree = IssuesTraceTree.FromTrace(traceWithErrorInMiddle, {
112117
meta: null,
113118
replay: null,
119+
organization,
114120
});
115121

116122
const issues = IssuesTraceTree.FindAll(tree.root, hasErrors);
@@ -131,6 +137,7 @@ describe('IssuesTraceTree', () => {
131137
const tree = IssuesTraceTree.FromTrace(traceWithErrorInMiddle, {
132138
meta: null,
133139
replay: null,
140+
organization,
134141
});
135142

136143
const issues = IssuesTraceTree.FindAll(tree.root, hasErrors);
@@ -172,6 +179,7 @@ describe('IssuesTraceTree', () => {
172179
const tree = IssuesTraceTree.FromTrace(traceWithSpans, {
173180
meta: null,
174181
replay: null,
182+
organization,
175183
});
176184

177185
mockSpansResponse(

static/app/views/performance/newTraceDetails/traceModels/issuesTraceTree.tsx

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,23 @@ export class IssuesTraceTree extends TraceTree {
2323
return tree;
2424
}
2525

26-
static Loading(metadata: TraceTree.Metadata): IssuesTraceTree {
27-
const t = makeExampleTrace(metadata);
26+
static Loading(
27+
metadata: TraceTree.Metadata,
28+
organization: Organization
29+
): IssuesTraceTree {
30+
const t = makeExampleTrace(metadata, organization);
2831
const tree = new IssuesTraceTree();
2932
tree.root = t.root;
3033
tree.type = 'loading';
3134
tree.build();
3235
return tree;
3336
}
3437

35-
static Error(metadata: TraceTree.Metadata): IssuesTraceTree {
36-
const t = makeExampleTrace(metadata);
38+
static Error(
39+
metadata: TraceTree.Metadata,
40+
organization: Organization
41+
): IssuesTraceTree {
42+
const t = makeExampleTrace(metadata, organization);
3743
const tree = new IssuesTraceTree();
3844
tree.root = t.root;
3945
tree.type = 'error';
@@ -45,6 +51,7 @@ export class IssuesTraceTree extends TraceTree {
4551
trace: TraceTree.Trace,
4652
options: {
4753
meta: TraceMetaQueryResults['data'] | null;
54+
organization: Organization;
4855
replay: HydratedReplayRecord | null;
4956
preferences?: Pick<TracePreferencesState, 'autogroup' | 'missing_instrumentation'>;
5057
}

static/app/views/performance/newTraceDetails/traceModels/makeExampleTrace.tsx

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import type {Organization} from 'sentry/types/organization';
2+
13
import {TraceTree} from './traceTree';
24

35
// Creates an example trace response that we use to render the loading placeholder
@@ -27,7 +29,10 @@ function partialTransaction(
2729
};
2830
}
2931

30-
export function makeExampleTrace(metadata: TraceTree.Metadata): TraceTree {
32+
export function makeExampleTrace(
33+
metadata: TraceTree.Metadata,
34+
organization: Organization
35+
): TraceTree {
3136
const trace: TraceTree.Trace = {
3237
transactions: [],
3338
orphan_errors: [],
@@ -81,5 +86,5 @@ export function makeExampleTrace(metadata: TraceTree.Metadata): TraceTree {
8186
start = end;
8287
}
8388

84-
return TraceTree.FromTrace(trace, {meta: null, replay: null});
89+
return TraceTree.FromTrace(trace, {meta: null, replay: null, organization});
8590
}

0 commit comments

Comments
 (0)