Browse Source

fix(js-sdk): Move tracePropagationTargets to top level config (#51900)

In https://github.com/getsentry/sentry/pull/51797 we bumped the JS SDK
to `7.57.0`. Changelog here:
https://github.com/getsentry/sentry-javascript/releases/tag/7.57.0

Notable in this release was adding the ability to set
`tracePropagationTargets` as a top level option to `Sentry.init`.
Previously `tracePropagationTargets` was a nested option inside the
`BrowserTracing` integration.

```js
// old way
Sentry.init({
  integrations: [
   Sentry.BrowserTracing({
      tracePropagationTargets: ['foo', 'bar'],
    }),
  ],
});

// new with `7.57.0`
Sentry.init({
  integrations: [new Sentry.BrowserTracing()],
  // controls what requests trace headers attached - required because cors 😭
  tracePropagationTargets: ['foo', 'bar']
});
```

This change is done in a backwards compat manner though, so that if you
defined `tracePropagationTargets` in `BrowserTracing`, it is considered
valid. What's notable is that we made the decision to say that defining
a top level `tracePropagationTargets` (in `Sentry.init`) would override
the `tracePropagationTargets` set in `BrowserTracing` options - this
minimized the bundle size of the backwards compat change.

Unfortunately, our sdk init code in the sentry frontend was
unintentionally setting `tracePropagationTargets` in the top level, via
the spread operator.

```ts
sentryConfig as {
  allowUrls: string[];
  dsn: string;
  release: string;
  tracePropagationTargets: string[];
  profilesSampleRate?: number;
};

Sentry.init({
    // This defines tracePropagationTargets on the init options object
    ...sentryConfig,
// ...
```

Previously this did nothing, but with the bump to `7.57.0`, we started
to use this top level `tracePropagationTargets` instead of the one in
`BrowserTracing`. This means we removed the `/^\//` target in
`['localhost', /^\//, ...extraTracePropagationTargets]`, which meant we
stopped attaching headers to our outgoing api requests.

This PR fixes this predicament by removing the usage of `BrowserTracing`
`tracePropagationTargets` in favour of using the top level option.
Abhijeet Prasad 1 year ago
parent
commit
8baef7cf5d
1 changed files with 6 additions and 7 deletions
  1. 6 7
      static/app/bootstrap/initializeSdk.tsx

+ 6 - 7
static/app/bootstrap/initializeSdk.tsx

@@ -45,11 +45,7 @@ const shouldEnableBrowserProfiling = window?.__initialData?.user?.isSuperuser;
  * having routing instrumentation in order to have a smaller bundle size.
  * (e.g.  `static/views/integrationPipeline`)
  */
-function getSentryIntegrations(sentryConfig: Config['sentryConfig'], routes?: Function) {
-  const extraTracePropagationTargets = SPA_DSN
-    ? SPA_MODE_TRACE_PROPAGATION_TARGETS
-    : [...sentryConfig?.tracePropagationTargets];
-
+function getSentryIntegrations(routes?: Function) {
   const integrations = [
     new ExtraErrorData({
       // 6 is arbitrary, seems like a nice number
@@ -69,7 +65,6 @@ function getSentryIntegrations(sentryConfig: Config['sentryConfig'], routes?: Fu
         enableInteractions: true,
         onStartRouteTransaction: Sentry.onProfilingStartRouteTransaction,
       },
-      tracePropagationTargets: ['localhost', /^\//, ...extraTracePropagationTargets],
     }),
     new Sentry.BrowserProfilingIntegration(),
     new HTTPTimingIntegration(),
@@ -87,6 +82,9 @@ function getSentryIntegrations(sentryConfig: Config['sentryConfig'], routes?: Fu
 export function initializeSdk(config: Config, {routes}: {routes?: Function} = {}) {
   const {apmSampling, sentryConfig, userIdentity} = config;
   const tracesSampleRate = apmSampling ?? 0;
+  const extraTracePropagationTargets = SPA_DSN
+    ? SPA_MODE_TRACE_PROPAGATION_TARGETS
+    : [...sentryConfig?.tracePropagationTargets];
 
   Sentry.init({
     ...sentryConfig,
@@ -102,10 +100,11 @@ export function initializeSdk(config: Config, {routes}: {routes?: Function} = {}
      */
     release: SENTRY_RELEASE_VERSION ?? sentryConfig?.release,
     allowUrls: SPA_DSN ? SPA_MODE_ALLOW_URLS : sentryConfig?.allowUrls,
-    integrations: getSentryIntegrations(sentryConfig, routes),
+    integrations: getSentryIntegrations(routes),
     tracesSampleRate,
     // @ts-expect-error not part of browser SDK types yet
     profilesSampleRate: shouldEnableBrowserProfiling ? 1 : 0,
+    tracePropagationTargets: ['localhost', /^\//, ...extraTracePropagationTargets],
     tracesSampler: context => {
       if (context.transactionContext.op?.startsWith('ui.action')) {
         return tracesSampleRate / 100;