feat(core): Make stream instrumentation opt-in (#13951)

getsentry · Oct 17, 2024 · 778ba53 · 778ba53
1 parent 64fbe5a
commit 778ba53
Show file tree

Hide file tree

Showing 5 changed files with 55 additions and 9 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,12 @@
 
 - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott
 
+- **feat(core): Make stream instrumentation opt-in
+  ([#13951](https://github.com/getsentry/sentry-javascript/pull/13951))**
+
+This change adds a new option `trackFetchStreamPerformance` to the browser tracing integration. Only when set to `true`,
+Sentry will instrument streams via fetch.
+
 Work in this release was contributed by @ZakrepaShe. Thank you for your contribution!
 
 ## 8.34.0

diff --git a/dev-packages/e2e-tests/test-applications/react-router-6/src/index.tsx b/dev-packages/e2e-tests/test-applications/react-router-6/src/index.tsx
@@ -26,6 +26,7 @@ Sentry.init({
       useNavigationType,
       createRoutesFromChildren,
       matchRoutes,
+      trackFetchStreamPerformance: true,
     }),
     replay,
   ],

diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts
@@ -132,6 +132,14 @@ export interface BrowserTracingOptions {
    */
   traceXHR: boolean;
 
+  /**
+   * Flag to disable tracking of long-lived streams, like server-sent events (SSE) via fetch.
+   * Do not enable this in case you have live streams or very long running streams.
+   *
+   * Default: false
+   */
+  trackFetchStreamPerformance: boolean;
+
   /**
    * If true, Sentry will capture http timings and add them to the corresponding http spans.
    *
@@ -200,6 +208,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
     markBackgroundSpan,
     traceFetch,
     traceXHR,
+    trackFetchStreamPerformance,
     shouldCreateSpanForRequest,
     enableHTTPTimings,
     instrumentPageLoad,
@@ -409,6 +418,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
       instrumentOutgoingRequests(client, {
         traceFetch,
         traceXHR,
+        trackFetchStreamPerformance,
         tracePropagationTargets: client.getOptions().tracePropagationTargets,
         shouldCreateSpanForRequest,
         enableHTTPTimings,

diff --git a/packages/browser/src/tracing/request.ts b/packages/browser/src/tracing/request.ts
@@ -1,3 +1,4 @@
+/* eslint-disable max-lines */
 import {
   SENTRY_XHR_DATA_KEY,
   addPerformanceInstrumentationHandler,
@@ -76,7 +77,16 @@ export interface RequestInstrumentationOptions {
    *
    * Default: true
    */
-  traceXHR: boolean;
+  traceXHR: boolean /**
+   * Flag to disable tracking of long-lived streams, like server-sent events (SSE) via fetch.
+   * Do not enable this in case you have live streams or very long running streams.
+   *
+   * Disabled by default since it can lead to issues with streams using the `cancel()` api
+   * (https://github.com/getsentry/sentry-javascript/issues/13950)
+   *
+   * Default: false
+   */;
+  trackFetchStreamPerformance: boolean;
 
   /**
    * If true, Sentry will capture http timings and add them to the corresponding http spans.
@@ -101,13 +111,22 @@ export const defaultRequestInstrumentationOptions: RequestInstrumentationOptions
   traceFetch: true,
   traceXHR: true,
   enableHTTPTimings: true,
+  trackFetchStreamPerformance: false,
 };
 
 /** Registers span creators for xhr and fetch requests  */
 export function instrumentOutgoingRequests(client: Client, _options?: Partial<RequestInstrumentationOptions>): void {
-  const { traceFetch, traceXHR, shouldCreateSpanForRequest, enableHTTPTimings, tracePropagationTargets } = {
+  const {
+    traceFetch,
+    traceXHR,
+    trackFetchStreamPerformance,
+    shouldCreateSpanForRequest,
+    enableHTTPTimings,
+    tracePropagationTargets,
+  } = {
     traceFetch: defaultRequestInstrumentationOptions.traceFetch,
     traceXHR: defaultRequestInstrumentationOptions.traceXHR,
+    trackFetchStreamPerformance: defaultRequestInstrumentationOptions.trackFetchStreamPerformance,
     ..._options,
   };
 
@@ -136,14 +155,16 @@ export function instrumentOutgoingRequests(client: Client, _options?: Partial<Re
       return event;
     });
 
-    addFetchEndInstrumentationHandler(handlerData => {
-      if (handlerData.response) {
-        const span = responseToSpanId.get(handlerData.response);
-        if (span && handlerData.endTimestamp) {
-          spanIdToEndTimestamp.set(span, handlerData.endTimestamp);
+    if (trackFetchStreamPerformance) {
+      addFetchEndInstrumentationHandler(handlerData => {
+        if (handlerData.response) {
+          const span = responseToSpanId.get(handlerData.response);
+          if (span && handlerData.endTimestamp) {
+            spanIdToEndTimestamp.set(span, handlerData.endTimestamp);
+          }
         }
-      }
-    });
+      });
+    }
 
     addFetchInstrumentationHandler(handlerData => {
       const createdSpan = instrumentFetchRequest(handlerData, shouldCreateSpan, shouldAttachHeadersWithTargets, spans);

diff --git a/packages/browser/test/tracing/request.test.ts b/packages/browser/test/tracing/request.test.ts
@@ -54,6 +54,14 @@ describe('instrumentOutgoingRequests', () => {
 
     expect(addXhrSpy).not.toHaveBeenCalled();
   });
+
+  it('does instrument streaming requests if trackFetchStreamPerformance is true', () => {
+    const addFetchEndSpy = vi.spyOn(utils, 'addFetchEndInstrumentationHandler');
+
+    instrumentOutgoingRequests(client, { trackFetchStreamPerformance: true });
+
+    expect(addFetchEndSpy).toHaveBeenCalledWith(expect.any(Function));
+  });
 });
 
 interface ProtocolInfo {