diff --git a/.size-limit.js b/.size-limit.js index 44e701b3466b..3a4689d59faa 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -241,7 +241,7 @@ module.exports = [ path: createCDNPath('bundle.tracing.min.js'), gzip: false, brotli: false, - limit: '129 KB', + limit: '130 KB', }, { name: 'CDN Bundle (incl. Logs, Metrics) - uncompressed', diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/http-timings-streamed/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/http-timings-streamed/test.ts index ffa63937bf32..25d4ac497992 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/http-timings-streamed/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/http-timings-streamed/test.ts @@ -3,16 +3,8 @@ import { sentryTest } from '../../../../utils/fixtures'; import { shouldSkipTracingTest, testingCdnBundle } from '../../../../utils/helpers'; import { getSpanOp, waitForStreamedSpans } from '../../../../utils/spanUtils'; -/** - * This test details a limitation of span streaming in comparison to transaction-based tracing: - * We can no longer attach http PerformanceResourceTiming attributes to http.client spans in - * span streaming mode. The reason is that we track `http.client` spans in real time but only - * get the detailed timing information after the span already ended. - * We can probably fix this (somehat at least) but will do so in a follow-up PR. - * @see https://github.com/getsentry/sentry-javascript/issues/19613 - */ sentryTest( - "[limitation] doesn't add http timing to http.client spans in span streaming mode", + 'adds http timing to http.client spans in span streaming mode', async ({ browserName, getLocalTestUrl, page }) => { const supportedBrowsers = ['chromium', 'firefox']; @@ -49,7 +41,7 @@ sentryTest( end_timestamp: expect.any(Number), trace_id: pageloadSpan?.trace_id, status: 'ok', - attributes: expect.not.objectContaining({ + attributes: expect.objectContaining({ 'http.request.redirect_start': expect.any(Object), 'http.request.redirect_end': expect.any(Object), 'http.request.worker_start': expect.any(Object), diff --git a/dev-packages/browser-integration-tests/suites/tracing/setSpanActive-streamed/nested-parentAlwaysRoot/test.ts b/dev-packages/browser-integration-tests/suites/tracing/setSpanActive-streamed/nested-parentAlwaysRoot/test.ts index 58728bba07f9..8f5e54e1fba0 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/setSpanActive-streamed/nested-parentAlwaysRoot/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/setSpanActive-streamed/nested-parentAlwaysRoot/test.ts @@ -11,18 +11,14 @@ sentryTest( const checkoutSpansPromise = waitForStreamedSpans(page, spans => spans.some(s => s.name === 'checkout-flow' && s.is_segment), ); - const postCheckoutSpansPromise = waitForStreamedSpans(page, spans => - spans.some(s => s.name === 'post-checkout' && s.is_segment), - ); const url = await getLocalTestUrl({ testDir: __dirname }); await page.goto(url); const checkoutSpans = await checkoutSpansPromise; - const postCheckoutSpans = await postCheckoutSpansPromise; const checkoutSpan = checkoutSpans.find(s => s.name === 'checkout-flow'); - const postCheckoutSpan = postCheckoutSpans.find(s => s.name === 'post-checkout'); + const postCheckoutSpan = checkoutSpans.find(s => s.name === 'post-checkout'); const checkoutSpanId = checkoutSpan?.span_id; const postCheckoutSpanId = postCheckoutSpan?.span_id; @@ -30,8 +26,7 @@ sentryTest( expect(checkoutSpanId).toMatch(/[a-f\d]{16}/); expect(postCheckoutSpanId).toMatch(/[a-f\d]{16}/); - expect(checkoutSpans.filter(s => !s.is_segment)).toHaveLength(4); - expect(postCheckoutSpans.filter(s => !s.is_segment)).toHaveLength(1); + expect(checkoutSpans.filter(s => !s.is_segment)).toHaveLength(5); const checkoutStep1 = checkoutSpans.find(s => s.name === 'checkout-step-1'); const checkoutStep2 = checkoutSpans.find(s => s.name === 'checkout-step-2'); @@ -56,7 +51,7 @@ sentryTest( // post-checkout trace is started as a new trace because ending checkoutSpan removes the active // span on the scope - const postCheckoutStep1 = postCheckoutSpans.find(s => s.name === 'post-checkout-1'); + const postCheckoutStep1 = checkoutSpans.find(s => s.name === 'post-checkout-1'); expect(postCheckoutStep1).toBeDefined(); expect(postCheckoutStep1?.parent_span_id).toBe(postCheckoutSpanId); }, diff --git a/dev-packages/browser-integration-tests/suites/tracing/setSpanActive-streamed/nested/test.ts b/dev-packages/browser-integration-tests/suites/tracing/setSpanActive-streamed/nested/test.ts index 4d11a36982b7..1b04553090bc 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/setSpanActive-streamed/nested/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/setSpanActive-streamed/nested/test.ts @@ -11,18 +11,14 @@ sentryTest( const checkoutSpansPromise = waitForStreamedSpans(page, spans => spans.some(s => s.name === 'checkout-flow' && s.is_segment), ); - const postCheckoutSpansPromise = waitForStreamedSpans(page, spans => - spans.some(s => s.name === 'post-checkout' && s.is_segment), - ); const url = await getLocalTestUrl({ testDir: __dirname }); await page.goto(url); const checkoutSpans = await checkoutSpansPromise; - const postCheckoutSpans = await postCheckoutSpansPromise; const checkoutSpan = checkoutSpans.find(s => s.name === 'checkout-flow'); - const postCheckoutSpan = postCheckoutSpans.find(s => s.name === 'post-checkout'); + const postCheckoutSpan = checkoutSpans.find(s => s.name === 'post-checkout'); const checkoutSpanId = checkoutSpan?.span_id; const postCheckoutSpanId = postCheckoutSpan?.span_id; @@ -30,8 +26,7 @@ sentryTest( expect(checkoutSpanId).toMatch(/[a-f\d]{16}/); expect(postCheckoutSpanId).toMatch(/[a-f\d]{16}/); - expect(checkoutSpans.filter(s => !s.is_segment)).toHaveLength(4); - expect(postCheckoutSpans.filter(s => !s.is_segment)).toHaveLength(1); + expect(checkoutSpans.filter(s => !s.is_segment)).toHaveLength(5); const checkoutStep1 = checkoutSpans.find(s => s.name === 'checkout-step-1'); const checkoutStep2 = checkoutSpans.find(s => s.name === 'checkout-step-2'); @@ -51,7 +46,7 @@ sentryTest( // root span due to this being default behaviour in browser environments expect(checkoutStep21?.parent_span_id).toBe(checkoutSpanId); - const postCheckoutStep1 = postCheckoutSpans.find(s => s.name === 'post-checkout-1'); + const postCheckoutStep1 = checkoutSpans.find(s => s.name === 'post-checkout-1'); expect(postCheckoutStep1).toBeDefined(); expect(postCheckoutStep1?.parent_span_id).toBe(postCheckoutSpanId); }, diff --git a/packages/browser/src/integrations/spanstreaming.ts b/packages/browser/src/integrations/spanstreaming.ts index f741e2746885..ab07f75c2b7d 100644 --- a/packages/browser/src/integrations/spanstreaming.ts +++ b/packages/browser/src/integrations/spanstreaming.ts @@ -58,7 +58,12 @@ export const spanStreamingIntegration = defineIntegration(() => { // In addition to capturing the span, we also flush the trace when the segment // span ends to ensure things are sent timely. We never know when the browser // is closed, users navigate away, etc. - client.on('afterSegmentSpanEnd', segmentSpan => buffer.flush(segmentSpan.spanContext().traceId)); + client.on('afterSegmentSpanEnd', segmentSpan => { + const traceId = segmentSpan.spanContext().traceId; + setTimeout(() => { + buffer.flush(traceId); + }, 500); + }); }, }; }) satisfies IntegrationFn; diff --git a/packages/browser/src/tracing/request.ts b/packages/browser/src/tracing/request.ts index 85faf0871a55..6211cf72947a 100644 --- a/packages/browser/src/tracing/request.ts +++ b/packages/browser/src/tracing/request.ts @@ -1,3 +1,4 @@ +/* eslint-disable max-lines */ import type { Client, HandlerDataXhr, @@ -5,6 +6,7 @@ import type { ResponseHookInfo, SentryWrappedXMLHttpRequest, Span, + SpanTimeInput, } from '@sentry/core'; import { addFetchEndInstrumentationHandler, @@ -14,6 +16,7 @@ import { getLocationHref, getTraceData, hasSpansEnabled, + hasSpanStreamingEnabled, instrumentFetchRequest, parseUrl, SEMANTIC_ATTRIBUTE_SENTRY_OP, @@ -25,6 +28,7 @@ import { stringMatchesSomePattern, stripDataUrlContent, stripUrlQueryAndFragment, + timestampInSeconds, } from '@sentry/core'; import type { XhrHint } from '@sentry-internal/browser-utils'; import { @@ -205,7 +209,7 @@ export function instrumentOutgoingRequests(client: Client, _options?: Partial { + // Clean up the performance observer and other resources + // We have to wait here because otherwise this cleans itself up before it is fully done. + // Default (non-streaming): just deregister the observer. + let onEntryFound = (): void => void setTimeout(unsubscribePerformanceObsever); + + // For streamed spans, we have to artificially delay the ending of the span until we + // either receive the timing data, or HTTP_TIMING_WAIT_MS elapses. + if (hasSpanStreamingEnabled(client)) { + const originalEnd = span.end.bind(span); + + span.end = (endTimestamp?: SpanTimeInput) => { + const capturedEndTimestamp = endTimestamp ?? timestampInSeconds(); + let isEnded = false; + + const endSpanAndCleanup = (): void => { + if (isEnded) { + return; + } + isEnded = true; + setTimeout(unsubscribePerformanceObsever); + originalEnd(capturedEndTimestamp); + clearTimeout(fallbackTimeout); + }; + + onEntryFound = endSpanAndCleanup; + + // Fallback: always end the span after HTTP_TIMING_WAIT_MS even if no + // PerformanceResourceTiming entry arrives (e.g. cross-origin without + // Timing-Allow-Origin, or the browser didn't fire the observer in time). + const fallbackTimeout = setTimeout(endSpanAndCleanup, HTTP_TIMING_WAIT_MS); + }; + } + + const unsubscribePerformanceObsever = addPerformanceInstrumentationHandler('resource', ({ entries }) => { entries.forEach(entry => { if (isPerformanceResourceTiming(entry) && entry.name.endsWith(url)) { span.setAttributes(resourceTimingToSpanAttributes(entry)); - // In the next tick, clean this handler up - // We have to wait here because otherwise this cleans itself up before it is fully done - setTimeout(cleanup); + onEntryFound(); } }); }); diff --git a/packages/browser/test/integrations/spanstreaming.test.ts b/packages/browser/test/integrations/spanstreaming.test.ts index 63e07738570c..5b84c52f8d7e 100644 --- a/packages/browser/test/integrations/spanstreaming.test.ts +++ b/packages/browser/test/integrations/spanstreaming.test.ts @@ -171,7 +171,8 @@ describe('spanStreamingIntegration', () => { expect(mockSpanBufferInstance.flush).not.toHaveBeenCalled(); }); - it('flushes the trace when the segment span ends', () => { + it('flushes the trace when the segment span ends after a delay for close to finished child spans', () => { + vi.useFakeTimers(); const client = new BrowserClient({ ...getDefaultBrowserClientOptions(), dsn: 'https://username@domain/123', @@ -185,6 +186,10 @@ describe('spanStreamingIntegration', () => { const span = new SentryCore.SentrySpan({ name: 'test' }); client.emit('afterSegmentSpanEnd', span); + vi.advanceTimersByTime(500); + expect(mockSpanBufferInstance.flush).toHaveBeenCalledWith(span.spanContext().traceId); + + vi.useRealTimers(); }); });