fix(ember): Ensure only one client is created & Replay works (#7712)

Author: mydea

packages/ember/addon/instance-initializers/sentry-performance.ts

@@ -3,13 +3,14 @@ import { run, _backburner, scheduleOnce } from '@ember/runloop';
 import { subscribe } from '@ember/instrumentation';
 import * as Sentry from '@sentry/browser';
 import { ExtendedBackburner } from '@sentry/ember/runloop';
-import { Span, Transaction, Integration } from '@sentry/types';
+import { Span, Transaction } from '@sentry/types';
 import { EmberRunQueues } from '@ember/runloop/-private/types';
 import { getActiveTransaction } from '..';
 import { browserPerformanceTimeOrigin, GLOBAL_OBJ, timestampWithMs } from '@sentry/utils';
 import { macroCondition, isTesting, getOwnConfig } from '@embroider/macros';
 import { EmberSentryConfig, GlobalConfig, OwnConfig } from '../types';
 import RouterService from '@ember/routing/router-service';
+import type { BaseClient } from '@sentry/core';
 
 function getSentryConfig() {
   const _global = GLOBAL_OBJ as typeof GLOBAL_OBJ & GlobalConfig;
@@ -390,58 +391,49 @@ export async function instrumentForPerformance(appInstance: ApplicationInstance)
 
   const idleTimeout = config.transitionTimeout || 5000;
 
-  const existingIntegrations = (sentryConfig['integrations'] || []) as Integration[];
-
-  sentryConfig['integrations'] = [
-    ...existingIntegrations,
-    new BrowserTracing({
-      routingInstrumentation: (customStartTransaction, startTransactionOnPageLoad) => {
-        const routerMain = appInstance.lookup('router:main');
-        let routerService = appInstance.lookup('service:router') as
-          | RouterService & { externalRouter?: RouterService; _hasMountedSentryPerformanceRouting?: boolean };
-
-        if (routerService.externalRouter) {
-          // Using ember-engines-router-service in an engine.
-          routerService = routerService.externalRouter;
-        }
-        if (routerService._hasMountedSentryPerformanceRouting) {
-          // Routing listens to route changes on the main router, and should not be initialized multiple times per page.
-          return;
-        }
-        if (!routerService.recognize) {
-          // Router is missing critical functionality to limit cardinality of the transaction names.
-          return;
-        }
-        routerService._hasMountedSentryPerformanceRouting = true;
-        _instrumentEmberRouter(routerService, routerMain, config, customStartTransaction, startTransactionOnPageLoad);
-      },
-      idleTimeout,
-      ...browserTracingOptions,
-    }),
-  ];
-
-  class FakeBrowserTracingClass {
-    static id = 'BrowserTracing';
-    public name = FakeBrowserTracingClass.id;
-    setupOnce() {
-      // noop - We're just faking  this class for a lookup
+  const browserTracing = new BrowserTracing({
+    routingInstrumentation: (customStartTransaction, startTransactionOnPageLoad) => {
+      const routerMain = appInstance.lookup('router:main');
+      let routerService = appInstance.lookup('service:router') as
+        | RouterService & { externalRouter?: RouterService; _hasMountedSentryPerformanceRouting?: boolean };
+
+      if (routerService.externalRouter) {
+        // Using ember-engines-router-service in an engine.
+        routerService = routerService.externalRouter;
+      }
+      if (routerService._hasMountedSentryPerformanceRouting) {
+        // Routing listens to route changes on the main router, and should not be initialized multiple times per page.
+        return;
+      }
+      if (!routerService.recognize) {
+        // Router is missing critical functionality to limit cardinality of the transaction names.
+        return;
+      }
+      routerService._hasMountedSentryPerformanceRouting = true;
+      _instrumentEmberRouter(routerService, routerMain, config, customStartTransaction, startTransactionOnPageLoad);
+    },
+    idleTimeout,
+    ...browserTracingOptions,
+  });
+
+  if (macroCondition(isTesting())) {
+    const client = Sentry.getCurrentHub().getClient();
+
+    if (
+      client &&
+      (client as BaseClient<any>).getIntegrationById &&
+      (client as BaseClient<any>).getIntegrationById('BrowserTracing')
+    ) {
+      // Initializers are called more than once in tests, causing the integrations to not be setup correctly.
+      return;
     }
   }
 
-  if (
-    isTesting() &&
-    Sentry.getCurrentHub()?.getIntegration(
-      // This is a temporary hack because the BrowserTracing integration cannot have a static `id` field for tree
-      // shaking reasons. However, `getIntegration` needs that field.
-      FakeBrowserTracingClass,
-    )
-  ) {
-    // Initializers are called more than once in tests, causing the integrations to not be setup correctly.
-    return;
+  const client = Sentry.getCurrentHub().getClient();
+  if (client && client.addIntegration) {
+    client.addIntegration(browserTracing);
   }
 
-  Sentry.init(sentryConfig); // Call init again to rebind client with new integration list in addition to the defaults
-
   _instrumentEmberRunloop(config);
   _instrumentComponents(config);
   _instrumentInitialLoad(config);

packages/ember/tests/acceptance/sentry-replay-test.js

@@ -0,0 +1,20 @@
+import { test, module } from 'qunit';
+import { setupApplicationTest } from 'ember-qunit';
+import { visit } from '@ember/test-helpers';
+import { setupSentryTest } from '../helpers/setup-sentry';
+import * as Sentry from '@sentry/ember';
+
+module('Acceptance | Sentry Session Replay', function (hooks) {
+  setupApplicationTest(hooks);
+  setupSentryTest(hooks);
+
+  test('Test replay', async function (assert) {
+    await visit('/replay');
+
+    const replay = Sentry.getCurrentHub().getIntegration(Sentry.Replay);
+    assert.ok(replay);
+
+    assert.true(replay._replay.isEnabled());
+    assert.false(replay._replay.isPaused());
+  });
+});

packages/ember/tests/dummy/app/app.js

@@ -5,6 +5,8 @@ import config from './config/environment';
 import * as Sentry from '@sentry/ember';
 
 Sentry.init({
+  replaysSessionSampleRate: 1,
+  replaysOnErrorSampleRate: 1,
   browserTracingOptions: {
     _experiments: {
       // This lead to some flaky tests, as that is sometimes logged

packages/ember/tests/dummy/app/router.js

@@ -8,6 +8,7 @@ export default class Router extends EmberRouter {
 
 Router.map(function () {
   this.route('tracing');
+  this.route('replay');
   this.route('slow-loading-route', function () {
     this.route('index', { path: '/' });
   });

packages/ember/tests/dummy/app/routes/replay.js

@@ -0,0 +1,13 @@
+import Route from '@ember/routing/route';
+import * as Sentry from '@sentry/ember';
+
+export default class ReplayRoute extends Route {
+  async beforeModel() {
+    const { Replay } = Sentry;
+
+    if (!Sentry.getCurrentHub().getIntegration(Replay)) {
+      const client = Sentry.getCurrentHub().getClient();
+      client.addIntegration(new Replay());
+    }
+  }
+}

packages/ember/tests/dummy/app/templates/application.hbs

@@ -11,6 +11,7 @@
         <div class="nav">
           <Link @route='index'>Errors</Link>
           <Link @route='tracing'>Tracing</Link>
+          <Link @route='replay'>Replay</Link>
         </div>
         <div class="content-container">
           {{outlet}}

packages/ember/tests/dummy/app/templates/replay.hbs

@@ -0,0 +1 @@
+<h2>Visiting this page starts Replay!</h2>

packages/ember/tests/dummy/config/environment.js

@@ -22,7 +22,8 @@ module.exports = function (environment) {
   ENV['@sentry/ember'] = {
     sentry: {
       tracesSampleRate: 1,
-      dsn: process.env.SENTRY_DSN,
+      // Include fake dsn so that instrumentation is enabled when running from cli
+      dsn: process.env.SENTRY_DSN || 'https://[email protected]/0',
       browserTracingOptions: {
         tracingOrigins: ['localhost', 'doesntexist.example'],
       },
@@ -50,9 +51,6 @@ module.exports = function (environment) {
 
     ENV.APP.rootElement = '#ember-testing';
     ENV.APP.autoboot = false;
-
-    // Include fake dsn so that instrumentation is enabled when running from cli
-    ENV['@sentry/ember'].sentry.dsn = 'https://[email protected]/0';
   }
 
   if (environment === 'production') {