diff --git a/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-child/init.js b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-child/init.js new file mode 100644 index 000000000000..588769fbea94 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-child/init.js @@ -0,0 +1,12 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration(), Sentry.spanStreamingIntegration()], + ignoreSpans: [/ignored-click-listener/], + parentSpanIsAlwaysRootSpan: false, + tracePropagationTargets: ['sentry-test-external.io'], + tracesSampleRate: 0, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-child/subject.js b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-child/subject.js new file mode 100644 index 000000000000..a1dfe86f2f17 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-child/subject.js @@ -0,0 +1,9 @@ +const fetchButton = document.getElementById('fetch'); + +fetchButton.addEventListener('click', async () => { + await Sentry.startSpan({ name: 'ignored-click-listener', op: 'ui.interaction.click' }, async () => { + await fetch('http://sentry-test-external.io'); + }); +}); + +fetchButton.click(); diff --git a/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-child/template.html b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-child/template.html new file mode 100644 index 000000000000..3e07212de2f5 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-child/template.html @@ -0,0 +1,14 @@ + + + + + + + + + + + diff --git a/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-child/test.ts b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-child/test.ts new file mode 100644 index 000000000000..0db192db4da0 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-child/test.ts @@ -0,0 +1,46 @@ +import { expect } from '@playwright/test'; +import type { ClientReport } from '@sentry/core'; +import { sentryTest } from '../../../../utils/fixtures'; +import { + envelopeRequestParser, + hidePage, + shouldSkipTracingTest, + waitForClientReportRequest, + waitForTracingHeadersOnUrl, +} from '../../../../utils/helpers'; +import { getSpanOp, waitForStreamedSpans } from '../../../../utils/spanUtils'; + +sentryTest( + 'ignoring a child span preserves the positive sampling decision of a continued trace when propagating it', + async ({ getLocalTestUrl, page }) => { + sentryTest.skip(shouldSkipTracingTest()); + + const url = await getLocalTestUrl({ testDir: __dirname }); + + const clientReportPromise = waitForClientReportRequest(page); + const tracingHeadersPromise = waitForTracingHeadersOnUrl(page, 'http://sentry-test-external.io'); + const spansPromise = waitForStreamedSpans( + page, + spans => + spans.some(span => getSpanOp(span) === 'pageload') && spans.some(span => getSpanOp(span) === 'http.client'), + ); + + await page.goto(url); + + const [{ baggage, sentryTrace }, spans] = await Promise.all([tracingHeadersPromise, spansPromise]); + const pageloadSpan = spans.find(span => getSpanOp(span) === 'pageload'); + const httpClientSpan = spans.find(span => getSpanOp(span) === 'http.client'); + + expect(pageloadSpan?.is_segment).toBe(true); + expect(pageloadSpan?.trace_id).toBe('12345678901234567890123456789012'); + expect(httpClientSpan?.parent_span_id).toBe(pageloadSpan?.span_id); + expect(sentryTrace).toBe(`${pageloadSpan?.trace_id}-${httpClientSpan?.span_id}-1`); + expect(baggage).toEqual( + 'sentry-trace_id=12345678901234567890123456789012,sentry-sample_rate=1,sentry-sampled=true,sentry-public_key=public,sentry-sample_rand=0.5', + ); + + await hidePage(page); + const clientReport = envelopeRequestParser(await clientReportPromise); + expect(clientReport.discarded_events).toEqual([{ category: 'span', quantity: 1, reason: 'ignored' }]); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-http-client/init.js b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-http-client/init.js new file mode 100644 index 000000000000..788e05781740 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-http-client/init.js @@ -0,0 +1,11 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration(), Sentry.spanStreamingIntegration()], + ignoreSpans: [{ attributes: { 'sentry.op': 'http.client' } }], + tracePropagationTargets: ['sentry-test-external.io'], + tracesSampleRate: 0, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-http-client/subject.js b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-http-client/subject.js new file mode 100644 index 000000000000..df0e5b971af2 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-http-client/subject.js @@ -0,0 +1 @@ +setTimeout(() => fetch('http://sentry-test-external.io')); diff --git a/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-http-client/template.html b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-http-client/template.html new file mode 100644 index 000000000000..c134ad5cd394 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-http-client/template.html @@ -0,0 +1,12 @@ + + + + + + + + + diff --git a/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-http-client/test.ts b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-http-client/test.ts new file mode 100644 index 000000000000..046398297679 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-http-client/test.ts @@ -0,0 +1,37 @@ +import { expect } from '@playwright/test'; +import type { ClientReport } from '@sentry/core'; +import { sentryTest } from '../../../../utils/fixtures'; +import { + envelopeRequestParser, + hidePage, + shouldSkipTracingTest, + waitForClientReportRequest, + waitForTracingHeadersOnUrl, +} from '../../../../utils/helpers'; +import { getSpanOp, waitForStreamedSpan } from '../../../../utils/spanUtils'; + +sentryTest( + 'ignoring an outgoing HTTP span preserves the positive sampling decision of a continued trace', + async ({ getLocalTestUrl, page }) => { + sentryTest.skip(shouldSkipTracingTest()); + + const url = await getLocalTestUrl({ testDir: __dirname }); + const clientReportPromise = waitForClientReportRequest(page); + const tracingHeadersPromise = waitForTracingHeadersOnUrl(page, 'http://sentry-test-external.io'); + const pageloadSpanPromise = waitForStreamedSpan(page, span => getSpanOp(span) === 'pageload'); + + await page.goto(url); + + const [{ baggage, sentryTrace }, pageloadSpan] = await Promise.all([tracingHeadersPromise, pageloadSpanPromise]); + expect(pageloadSpan.is_segment).toBe(true); + expect(pageloadSpan.trace_id).toBe('12345678901234567890123456789012'); + expect(sentryTrace).toBe(`12345678901234567890123456789012-${pageloadSpan.span_id}-1`); + expect(baggage).toEqual( + 'sentry-trace_id=12345678901234567890123456789012,sentry-sample_rate=1,sentry-sampled=true,sentry-public_key=public,sentry-sample_rand=0.5', + ); + + await hidePage(page); + const clientReport = envelopeRequestParser(await clientReportPromise); + expect(clientReport.discarded_events).toEqual([{ category: 'span', quantity: 1, reason: 'ignored' }]); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-segment/init.js b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-segment/init.js new file mode 100644 index 000000000000..616ae3d942f6 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-segment/init.js @@ -0,0 +1,11 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration(), Sentry.spanStreamingIntegration()], + ignoreSpans: [{ op: 'pageload' }], + tracePropagationTargets: ['sentry-test-external.io'], + tracesSampleRate: 0, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-segment/subject.js b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-segment/subject.js new file mode 100644 index 000000000000..4ea86b5fc4e9 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-segment/subject.js @@ -0,0 +1,3 @@ +document.getElementById('fetch').addEventListener('click', () => { + fetch('http://sentry-test-external.io'); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-segment/template.html b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-segment/template.html new file mode 100644 index 000000000000..3e07212de2f5 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-segment/template.html @@ -0,0 +1,14 @@ + + + + + + + + + + + diff --git a/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-segment/test.ts b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-segment/test.ts new file mode 100644 index 000000000000..efe25b178c8a --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-segment/test.ts @@ -0,0 +1,23 @@ +import { expect } from '@playwright/test'; +import { sentryTest } from '../../../../utils/fixtures'; +import { shouldSkipTracingTest, waitForTracingHeadersOnUrl } from '../../../../utils/helpers'; + +sentryTest( + 'ignoring a trace-continued, positively sampled segment span propagates a negative sampling decision when propagating it', + async ({ getLocalTestUrl, page }) => { + sentryTest.skip(shouldSkipTracingTest()); + + const url = await getLocalTestUrl({ testDir: __dirname }); + + const tracingHeadersPromise = waitForTracingHeadersOnUrl(page, 'http://sentry-test-external.io'); + + await page.goto(url); + await page.locator('#fetch').click(); + + const { baggage, sentryTrace } = await tracingHeadersPromise; + expect(sentryTrace).toMatch(/12345678901234567890123456789012-[\da-f]{16}-0/); + expect(baggage).toEqual( + 'sentry-trace_id=12345678901234567890123456789012,sentry-sample_rate=1,sentry-sampled=false,sentry-public_key=public,sentry-sample_rand=0.5', + ); + }, +); diff --git a/packages/browser/src/tracing/request.ts b/packages/browser/src/tracing/request.ts index f4e6b3d184a9..38cb73d085cc 100644 --- a/packages/browser/src/tracing/request.ts +++ b/packages/browser/src/tracing/request.ts @@ -22,6 +22,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SentryNonRecordingSpan, setHttpStatus, + spanIsIgnored, spanToJSON, startInactiveSpan, stringMatchesSomePattern, @@ -325,6 +326,7 @@ export function shouldAttachHeaders( * * @returns Span if a span was created, otherwise void. */ +// oxlint-disable-next-line complexity function xhrCallback( handlerData: HandlerDataXhr, shouldCreateSpan: (url: string) => boolean, @@ -401,6 +403,10 @@ function xhrCallback( }) : new SentryNonRecordingSpan(); + // If the span is ignored, we don't want to continue the trace from it (NonRecordingSpan) but rather + // from the active span. Passing `undefined` here will make `getTraceData` use the active span instead. + const spanForTraceHeaders = spanIsIgnored(span) && hasParent ? undefined : span; + if (shouldCreateSpanResult && !shouldEmitSpan) { client?.recordDroppedEvent('no_parent_span', 'span'); } @@ -414,7 +420,7 @@ function xhrCallback( // If performance is disabled (TWP) or there's no active root span (pageload/navigation/interaction), // we do not want to use the span as base for the trace headers, // which means that the headers will be generated from the scope and the sampling decision is deferred - hasSpansEnabled() && shouldEmitSpan ? span : undefined, + hasSpansEnabled() && shouldEmitSpan ? spanForTraceHeaders : undefined, propagateTraceparent, ); } diff --git a/packages/browser/test/tracing/request.test.ts b/packages/browser/test/tracing/request.test.ts index 920d1095202a..a57efd97f4b4 100644 --- a/packages/browser/test/tracing/request.test.ts +++ b/packages/browser/test/tracing/request.test.ts @@ -16,6 +16,8 @@ class MockClient implements Partial { public getOptions() { return {}; } + + public emit(): void {} } describe('instrumentOutgoingRequests', () => { @@ -51,6 +53,53 @@ describe('instrumentOutgoingRequests', () => { expect(addXhrSpy).not.toHaveBeenCalled(); }); + + describe('XHR trace header span', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('uses the active propagation context for an ignored child span', () => { + const activeSpan = new utils.SentryNonRecordingSpan(); + const ignoredSpan = new utils.SentryNonRecordingSpan({ dropReason: 'ignored' }); + let xhrHandler: ((data: utils.HandlerDataXhr) => void) | undefined; + + vi.spyOn(browserUtils, 'addXhrInstrumentationHandler').mockImplementation(handler => { + xhrHandler = handler; + }); + vi.spyOn(utils, 'getClient').mockReturnValue(client); + vi.spyOn(utils, 'getActiveSpan').mockReturnValue(activeSpan); + vi.spyOn(utils, 'hasSpansEnabled').mockReturnValue(true); + vi.spyOn(utils, 'hasSpanStreamingEnabled').mockReturnValue(true); + vi.spyOn(utils, 'startInactiveSpan').mockReturnValue(ignoredSpan); + const getTraceDataSpy = vi.spyOn(utils, 'getTraceData').mockReturnValue({ + 'sentry-trace': '12345678901234567890123456789012-1234567890123456-1', + }); + + instrumentOutgoingRequests(client, { + traceFetch: false, + tracePropagationTargets: ['example.com'], + enableHTTPTimings: false, + }); + xhrHandler?.({ + xhr: { + [browserUtils.SENTRY_XHR_DATA_KEY]: { + method: 'GET', + url: 'https://example.com/outgoing', + request_headers: {}, + }, + setRequestHeader: vi.fn(), + }, + startTimestamp: Date.now(), + } as utils.HandlerDataXhr); + + expect(xhrHandler).toBeDefined(); + expect(getTraceDataSpy).toHaveBeenCalledWith({ + span: undefined, + propagateTraceparent: undefined, + }); + }); + }); }); describe('shouldAttachHeaders', () => { diff --git a/packages/core/src/fetch.ts b/packages/core/src/fetch.ts index d28fc670fd8d..bfe78d499260 100644 --- a/packages/core/src/fetch.ts +++ b/packages/core/src/fetch.ts @@ -1,6 +1,6 @@ import { getClient } from './currentScopes'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from './semanticAttributes'; -import { setHttpStatus, SPAN_STATUS_ERROR, startInactiveSpan } from './tracing'; +import { setHttpStatus, SPAN_STATUS_ERROR, spanIsIgnored, startInactiveSpan } from './tracing'; import { SentryNonRecordingSpan } from './tracing/sentryNonRecordingSpan'; import { hasSpanStreamingEnabled } from './tracing/spans/hasSpanStreamingEnabled'; import type { FetchBreadcrumbHint } from './types/breadcrumb'; @@ -118,6 +118,7 @@ export function instrumentFetchRequest( shouldCreateSpanResult && shouldEmitSpan ? startInactiveSpan(getSpanStartOptions(url, method, spanOrigin)) : new SentryNonRecordingSpan(); + const spanForTraceHeaders = spanIsIgnored(span) && hasParent ? undefined : span; if (shouldCreateSpanResult && !shouldEmitSpan) { client?.recordDroppedEvent('no_parent_span', 'span'); @@ -139,7 +140,7 @@ export function instrumentFetchRequest( // If performance is disabled (TWP) or there's no active root span (pageload/navigation/interaction), // we do not want to use the span as base for the trace headers, // which means that the headers will be generated from the scope and the sampling decision is deferred - hasSpansEnabled() && shouldEmitSpan ? span : undefined, + hasSpansEnabled() && shouldEmitSpan ? spanForTraceHeaders : undefined, propagateTraceparent, ); if (headers) { diff --git a/packages/core/test/lib/fetch.test.ts b/packages/core/test/lib/fetch.test.ts index dec7feefec99..6cfdb74e8a19 100644 --- a/packages/core/test/lib/fetch.test.ts +++ b/packages/core/test/lib/fetch.test.ts @@ -1,7 +1,11 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { HandlerDataFetch } from '../../src'; import { _INTERNAL_getTracingHeadersForFetchRequest, instrumentFetchRequest } from '../../src/fetch'; +import { SentryNonRecordingSpan } from '../../src/tracing/sentryNonRecordingSpan'; import type { Span } from '../../src/types/span'; +import * as tracing from '../../src/tracing'; +import * as spanUtils from '../../src/utils/spanUtils'; +import * as traceData from '../../src/utils/traceData'; const { DEFAULT_SENTRY_TRACE, DEFAULT_BAGGAGE, hasSpansEnabled } = vi.hoisted(() => ({ DEFAULT_SENTRY_TRACE: 'defaultTraceId-defaultSpanId-1', @@ -440,6 +444,41 @@ describe('_INTERNAL_getTracingHeadersForFetchRequest', () => { }); describe('instrumentFetchRequest', () => { + describe('trace header span', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('uses the active propagation context for an ignored child span', () => { + const activeSpan = new SentryNonRecordingSpan(); + hasSpansEnabled.mockReturnValue(true); + const ignoredSpan = new SentryNonRecordingSpan({ dropReason: 'ignored' }); + vi.spyOn(spanUtils, 'getActiveSpan').mockReturnValue(activeSpan); + vi.spyOn(tracing, 'startInactiveSpan').mockReturnValue(ignoredSpan); + + instrumentFetchRequest( + { + fetchData: { url: '/api/test', method: 'GET' }, + args: ['/api/test'], + startTimestamp: Date.now(), + }, + () => true, + () => true, + {}, + { spanOrigin: 'auto.http.fetch' }, + ); + + expect(traceData.getTraceData).toHaveBeenCalledWith({ + span: undefined, + propagateTraceparent: false, + }); + }); + }); + describe('span cleanup', () => { it.each([ { name: 'non-recording', hasTracingEnabled: false },