Skip to content

Commit 2477239

Browse files
Lms24onurtemizkan
authored andcommitted
ref(browser): Temporarily add sentry.previous_trace span attribute (#15957)
Temporarily add a span attribute for the previous trace to get trace links working in the EAP-based trace view. This needs to be removed once EAP properly supports span links.
1 parent 0da8a6d commit 2477239

File tree

6 files changed

+53
-8
lines changed

6 files changed

+53
-8
lines changed

.size-limit.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ module.exports = [
210210
import: createImport('init'),
211211
ignore: ['next/router', 'next/constants'],
212212
gzip: true,
213-
limit: '41 KB',
213+
limit: '42 KB',
214214
},
215215
// SvelteKit SDK (ESM)
216216
{

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/default/test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ sentryTest("navigation spans link back to previous trace's root span", async ({
4949
},
5050
]);
5151

52+
expect(navigation1TraceContext?.data).toMatchObject({
53+
'sentry.previous_trace': `${pageloadTraceId}-${pageloadTraceContext?.span_id}-1`,
54+
});
55+
5256
expect(navigation2TraceContext?.links).toEqual([
5357
{
5458
trace_id: navigation1TraceId,
@@ -60,6 +64,10 @@ sentryTest("navigation spans link back to previous trace's root span", async ({
6064
},
6165
]);
6266

67+
expect(navigation2TraceContext?.data).toMatchObject({
68+
'sentry.previous_trace': `${navigation1TraceId}-${navigation1TraceContext?.span_id}-1`,
69+
});
70+
6371
expect(pageloadTraceId).not.toEqual(navigation1TraceId);
6472
expect(navigation1TraceId).not.toEqual(navigation2TraceId);
6573
expect(pageloadTraceId).not.toEqual(navigation2TraceId);

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/previous-trace-links/negatively-sampled/test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ sentryTest('includes a span link to a previously negatively sampled span', async
3434
},
3535
]);
3636

37+
expect(navigationTraceContext?.data).toMatchObject({
38+
'sentry.previous_trace': expect.stringMatching(/[a-f0-9]{32}-[a-f0-9]{16}-0/),
39+
});
40+
3741
expect(navigationTraceContext?.trace_id).not.toEqual(navigationTraceContext?.links![0].trace_id);
3842
});
3943
});

packages/browser/src/tracing/previousTrace.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ export const PREVIOUS_TRACE_MAX_DURATION = 3600;
2121
// session storage key
2222
export const PREVIOUS_TRACE_KEY = 'sentry_previous_trace';
2323

24+
export const PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE = 'sentry.previous_trace';
25+
2426
/**
2527
* Adds a previous_trace span link to the passed span if the passed
2628
* previousTraceInfo is still valid.
@@ -41,7 +43,8 @@ export function addPreviousTraceSpanLink(
4143
};
4244
}
4345

44-
if (previousTraceInfo.spanContext.traceId === spanJson.trace_id) {
46+
const previousTraceSpanCtx = previousTraceInfo.spanContext;
47+
if (previousTraceSpanCtx.traceId === spanJson.trace_id) {
4548
// This means, we're still in the same trace so let's not update the previous trace info
4649
// or add a link to the current span.
4750
// Once we move away from the long-lived, route-based trace model, we can remove this cases
@@ -56,19 +59,30 @@ export function addPreviousTraceSpanLink(
5659
if (Date.now() / 1000 - previousTraceInfo.startTimestamp <= PREVIOUS_TRACE_MAX_DURATION) {
5760
if (DEBUG_BUILD) {
5861
logger.info(
59-
`Adding previous_trace ${previousTraceInfo.spanContext} link to span ${{
62+
`Adding previous_trace ${previousTraceSpanCtx} link to span ${{
6063
op: spanJson.op,
6164
...span.spanContext(),
6265
}}`,
6366
);
6467
}
6568

6669
span.addLink({
67-
context: previousTraceInfo.spanContext,
70+
context: previousTraceSpanCtx,
6871
attributes: {
6972
[SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace',
7073
},
7174
});
75+
76+
// TODO: Remove this once EAP can store span links. We currently only set this attribute so that we
77+
// can obtain the previous trace information from the EAP store. Long-term, EAP will handle
78+
// span links and then we should remove this again. Also throwing in a TODO(v10), to remind us
79+
// to check this at v10 time :)
80+
span.setAttribute(
81+
PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE,
82+
`${previousTraceSpanCtx.traceId}-${previousTraceSpanCtx.spanId}-${
83+
previousTraceSpanCtx.traceFlags === 0x1 ? 1 : 0
84+
}`,
85+
);
7286
}
7387

7488
return {

packages/browser/test/tracing/browserTracingIntegration.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import {
4040
startBrowserTracingPageLoadSpan,
4141
} from '../../src/tracing/browserTracingIntegration';
4242
import { getDefaultBrowserClientOptions } from '../helper/browser-client-options';
43+
import { PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE } from '../../src/tracing/previousTrace';
4344

4445
// We're setting up JSDom here because the Next.js routing instrumentations requires a few things to be present on pageload:
4546
// 1. Access to window.document API for `window.document.getElementById`
@@ -202,6 +203,7 @@ describe('browserTracingIntegration', () => {
202203
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser',
203204
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
204205
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
206+
[PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE]: `${span?.spanContext().traceId}-${span?.spanContext().spanId}-1`,
205207
},
206208
links: [
207209
{
@@ -239,6 +241,7 @@ describe('browserTracingIntegration', () => {
239241
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser',
240242
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
241243
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
244+
[PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE]: `${span2?.spanContext().traceId}-${span2?.spanContext().spanId}-1`,
242245
},
243246
links: [
244247
{
@@ -502,6 +505,7 @@ describe('browserTracingIntegration', () => {
502505
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'manual',
503506
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
504507
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom',
508+
[PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE]: expect.stringMatching(/[a-f0-9]{32}-[a-f0-9]{16}-1/),
505509
},
506510
links: [
507511
{

packages/browser/test/tracing/previousTrace.test.ts

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
getPreviousTraceFromSessionStorage,
66
PREVIOUS_TRACE_KEY,
77
PREVIOUS_TRACE_MAX_DURATION,
8+
PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE,
89
} from '../../src/tracing/previousTrace';
910
import { SentrySpan, spanToJSON, timestampInSeconds } from '@sentry/core';
1011
import { storePreviousTraceInSessionStorage } from '../../src/tracing/previousTrace';
@@ -34,7 +35,9 @@ describe('addPreviousTraceSpanLink', () => {
3435

3536
const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, currentSpan);
3637

37-
expect(spanToJSON(currentSpan).links).toEqual([
38+
const spanJson = spanToJSON(currentSpan);
39+
40+
expect(spanJson.links).toEqual([
3841
{
3942
attributes: {
4043
'sentry.link.type': 'previous_trace',
@@ -45,6 +48,10 @@ describe('addPreviousTraceSpanLink', () => {
4548
},
4649
]);
4750

51+
expect(spanJson.data).toMatchObject({
52+
[PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE]: '123-456-1',
53+
});
54+
4855
expect(updatedPreviousTraceInfo).toEqual({
4956
spanContext: currentSpan.spanContext(),
5057
startTimestamp: currentSpanStart,
@@ -70,7 +77,11 @@ describe('addPreviousTraceSpanLink', () => {
7077

7178
const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, currentSpan);
7279

73-
expect(spanToJSON(currentSpan).links).toBeUndefined();
80+
const spanJson = spanToJSON(currentSpan);
81+
82+
expect(spanJson.links).toBeUndefined();
83+
84+
expect(Object.keys(spanJson.data)).not.toContain(PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE);
7485

7586
// but still updates the previousTraceInfo to the current span
7687
expect(updatedPreviousTraceInfo).toEqual({
@@ -141,7 +152,9 @@ describe('addPreviousTraceSpanLink', () => {
141152

142153
const updatedPreviousTraceInfo = addPreviousTraceSpanLink(undefined, currentSpan);
143154

144-
expect(spanToJSON(currentSpan).links).toBeUndefined();
155+
const spanJson = spanToJSON(currentSpan);
156+
expect(spanJson.links).toBeUndefined();
157+
expect(Object.keys(spanJson.data)).not.toContain(PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE);
145158

146159
expect(updatedPreviousTraceInfo).toEqual({
147160
spanContext: currentSpan.spanContext(),
@@ -169,7 +182,9 @@ describe('addPreviousTraceSpanLink', () => {
169182

170183
const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, currentSpan);
171184

172-
expect(spanToJSON(currentSpan).links).toBeUndefined();
185+
const spanJson = spanToJSON(currentSpan);
186+
expect(spanJson.links).toBeUndefined();
187+
expect(Object.keys(spanJson.data)).not.toContain(PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE);
173188

174189
expect(updatedPreviousTraceInfo).toBe(previousTraceInfo);
175190
});

0 commit comments

Comments
 (0)