Skip to content

ref(browser): Temporarily add sentry.previous_trace span attribute #15957

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .size-limit.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ module.exports = [
import: createImport('init'),
ignore: ['next/router', 'next/constants'],
gzip: true,
limit: '41 KB',
limit: '42 KB',
},
// SvelteKit SDK (ESM)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ sentryTest("navigation spans link back to previous trace's root span", async ({
},
]);

expect(navigation1TraceContext?.data).toMatchObject({
'sentry.previous_trace': `${pageloadTraceId}-${pageloadTraceContext?.span_id}-1`,
});

expect(navigation2TraceContext?.links).toEqual([
{
trace_id: navigation1TraceId,
Expand All @@ -60,6 +64,10 @@ sentryTest("navigation spans link back to previous trace's root span", async ({
},
]);

expect(navigation2TraceContext?.data).toMatchObject({
'sentry.previous_trace': `${navigation1TraceId}-${navigation1TraceContext?.span_id}-1`,
});

expect(pageloadTraceId).not.toEqual(navigation1TraceId);
expect(navigation1TraceId).not.toEqual(navigation2TraceId);
expect(pageloadTraceId).not.toEqual(navigation2TraceId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ sentryTest('includes a span link to a previously negatively sampled span', async
},
]);

expect(navigationTraceContext?.data).toMatchObject({
'sentry.previous_trace': expect.stringMatching(/[a-f0-9]{32}-[a-f0-9]{16}-0/),
});

expect(navigationTraceContext?.trace_id).not.toEqual(navigationTraceContext?.links![0].trace_id);
});
});
20 changes: 17 additions & 3 deletions packages/browser/src/tracing/previousTrace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ export const PREVIOUS_TRACE_MAX_DURATION = 3600;
// session storage key
export const PREVIOUS_TRACE_KEY = 'sentry_previous_trace';

export const PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE = 'sentry.previous_trace';

/**
* Adds a previous_trace span link to the passed span if the passed
* previousTraceInfo is still valid.
Expand All @@ -41,7 +43,8 @@ export function addPreviousTraceSpanLink(
};
}

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

span.addLink({
context: previousTraceInfo.spanContext,
context: previousTraceSpanCtx,
attributes: {
[SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE]: 'previous_trace',
},
});

// TODO: Remove this once EAP can store span links. We currently only set this attribute so that we
// can obtain the previous trace information from the EAP store. Long-term, EAP will handle
// span links and then we should remove this again. Also throwing in a TODO(v10), to remind us
// to check this at v10 time :)
span.setAttribute(
PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE,
`${previousTraceSpanCtx.traceId}-${previousTraceSpanCtx.spanId}-${
previousTraceSpanCtx.traceFlags === 0x1 ? 1 : 0
}`,
);
}

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import {
startBrowserTracingPageLoadSpan,
} from '../../src/tracing/browserTracingIntegration';
import { getDefaultBrowserClientOptions } from '../helper/browser-client-options';
import { PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE } from '../../src/tracing/previousTrace';

// We're setting up JSDom here because the Next.js routing instrumentations requires a few things to be present on pageload:
// 1. Access to window.document API for `window.document.getElementById`
Expand Down Expand Up @@ -202,6 +203,7 @@ describe('browserTracingIntegration', () => {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser',
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
[PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE]: `${span?.spanContext().traceId}-${span?.spanContext().spanId}-1`,
},
links: [
{
Expand Down Expand Up @@ -239,6 +241,7 @@ describe('browserTracingIntegration', () => {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser',
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
[PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE]: `${span2?.spanContext().traceId}-${span2?.spanContext().spanId}-1`,
},
links: [
{
Expand Down Expand Up @@ -502,6 +505,7 @@ describe('browserTracingIntegration', () => {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'manual',
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom',
[PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE]: expect.stringMatching(/[a-f0-9]{32}-[a-f0-9]{16}-1/),
},
links: [
{
Expand Down
23 changes: 19 additions & 4 deletions packages/browser/test/tracing/previousTrace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
getPreviousTraceFromSessionStorage,
PREVIOUS_TRACE_KEY,
PREVIOUS_TRACE_MAX_DURATION,
PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE,
} from '../../src/tracing/previousTrace';
import { SentrySpan, spanToJSON, timestampInSeconds } from '@sentry/core';
import { storePreviousTraceInSessionStorage } from '../../src/tracing/previousTrace';
Expand Down Expand Up @@ -34,7 +35,9 @@ describe('addPreviousTraceSpanLink', () => {

const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, currentSpan);

expect(spanToJSON(currentSpan).links).toEqual([
const spanJson = spanToJSON(currentSpan);

expect(spanJson.links).toEqual([
{
attributes: {
'sentry.link.type': 'previous_trace',
Expand All @@ -45,6 +48,10 @@ describe('addPreviousTraceSpanLink', () => {
},
]);

expect(spanJson.data).toMatchObject({
[PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE]: '123-456-1',
});

expect(updatedPreviousTraceInfo).toEqual({
spanContext: currentSpan.spanContext(),
startTimestamp: currentSpanStart,
Expand All @@ -70,7 +77,11 @@ describe('addPreviousTraceSpanLink', () => {

const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, currentSpan);

expect(spanToJSON(currentSpan).links).toBeUndefined();
const spanJson = spanToJSON(currentSpan);

expect(spanJson.links).toBeUndefined();

expect(Object.keys(spanJson.data)).not.toContain(PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE);

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

const updatedPreviousTraceInfo = addPreviousTraceSpanLink(undefined, currentSpan);

expect(spanToJSON(currentSpan).links).toBeUndefined();
const spanJson = spanToJSON(currentSpan);
expect(spanJson.links).toBeUndefined();
expect(Object.keys(spanJson.data)).not.toContain(PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE);

expect(updatedPreviousTraceInfo).toEqual({
spanContext: currentSpan.spanContext(),
Expand Down Expand Up @@ -169,7 +182,9 @@ describe('addPreviousTraceSpanLink', () => {

const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, currentSpan);

expect(spanToJSON(currentSpan).links).toBeUndefined();
const spanJson = spanToJSON(currentSpan);
expect(spanJson.links).toBeUndefined();
expect(Object.keys(spanJson.data)).not.toContain(PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE);

expect(updatedPreviousTraceInfo).toBe(previousTraceInfo);
});
Expand Down
Loading