fix(browser): Apply Http timing attributes to streamed http.client spans

Author: Lms24

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),

dev-packages/browser-integration-tests/suites/tracing/setSpanActive-streamed/nested-parentAlwaysRoot/test.ts

@@ -11,27 +11,22 @@ 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;
 
     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);
   },

dev-packages/browser-integration-tests/suites/tracing/setSpanActive-streamed/nested/test.ts

@@ -11,27 +11,22 @@ 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;
 
     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);
   },

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;

packages/browser/src/tracing/request.ts

@@ -5,6 +5,7 @@ import type {
   ResponseHookInfo,
   SentryWrappedXMLHttpRequest,
   Span,
+  SpanTimeInput,
 } from '@sentry/core';
 import {
   addFetchEndInstrumentationHandler,
@@ -238,8 +239,17 @@ export function instrumentOutgoingRequests(client: Client, _options?: Partial<Re
 }
 
 /**
- * Creates a temporary observer to listen to the next fetch/xhr resourcing timings,
- * so that when timings hit their per-browser limit they don't need to be removed.
+ * The maximum time (ms) to wait for PerformanceResourceTiming data before ending the span.
+ * Same approach is used by OTel's browser fetch instrumentation:
+ * See {@link https://github.com/open-telemetry/opentelemetry-js/blob/30f94fe99339287b1e4d3c8bb90172c2523f06f4/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts#L352-L372}
+ */
+const HTTP_TIMING_WAIT_MS = 300;
+
+/**
+ * Intercepts `span.end()` to delay the actual span ending until PerformanceResourceTiming
+ * data is available (or until the timeout elapses), so that timing attributes are always
+ * present on the span even in span-streaming mode where spans are serialised immediately
+ * on end.
  *
  * @param span A span that has yet to be finished, must contain `url` on data.
  */
@@ -250,16 +260,44 @@ function addHTTPTimings(span: Span): void {
     return;
   }
 
+  // Capture a reference to the real end implementation before shadowing it.
+  const originalEnd = span.end.bind(span);
+  let capturedEndTimestamp: SpanTimeInput | undefined;
+  let isEnded = false;
+
+  // Shadow the prototype method with an instance property so that when
+  // instrumentFetchRequest / xhrCallback calls span.end(), we intercept it.
+  span.end = (endTimestamp?: SpanTimeInput) => {
+    capturedEndTimestamp = endTimestamp;
+    // Do not call originalEnd here; finishSpan() handles that once timing data
+    // has arrived (or the timeout fires).
+  };
+
+  const endSpanAndCleanup = (): void => {
+    if (isEnded) {
+      return;
+    }
+    isEnded = true;
+    // In the next tick, clean up the performance observer
+    // We have to wait here because otherwise we clean it up before it is fully done
+    setTimeout(cleanup);
+    originalEnd(capturedEndTimestamp);
+    clearTimeout(fallbackTimeout);
+  };
+
   const cleanup = 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);
+        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);
 }
 
 /**