Repository: getsentry/sentry
Pull request: feat: Add base config for JS loader with performance/replay
This PR adjusts the loader to include default config when enabling debug/performance/replay.

We should probably do a bit more testing etc. for this, as this is not really covered well as of now.
I did some tests locally with different scenarios, where stuff worked well, as far as I can tell.

## Lazy mode

Lazy mode is only available for errors-only config. Users can manually opt-out of this by adding `data-lazy="no"` to their script tag.

In lazy mode, nothing happens until an error occurs. Once an error occurs, we download the CDN bundle, and then...:

* If `onLoad` is configured, trigger it
* If `SDK.init()` has not been called, call it with defaults

## Eager mode

If either performance or replay is enabled, we are always in eager (non-lazy) mode. In this mode, we'll automatically load the CDN bundle, and then...:

* If `onLoad` is configured, trigger it
* If `SDK.init()` has not been called, call it with defaults

## Defaults

The following default config is defined:

* `dsn`: Always set
* `debug: true` is set when debug bundles are enabled
* `tracesSampleRate: 0.1` is set when performance is enabled
* `replaysSessionSampleRate: 0.1` & `replaysOnErrorSampleRate: 1` are set when replay is enabled
* `integrations`: Will add `Sentry.BrowserTracing` and/or `Sentry.Replay` if performance/replay is enabled

This config can be overwritten by the user, and is merged. So e.g. if they do:

```js
Sentry.onLoad(function() {
  Sentry.init({
    tracesSampleRate: 0.5
  });
});
```

For a full loader build with every option enabled, the actual config will be:

```js
{
  dsn: 'your-dsn',
  debug: true,
  tracesSampleRate: 0.5,
  replaysSessionSampleRate: 0.1,
  replaysOnErrorSampleRate: 1,
  integrations: [new Sentry.BrowserTracing(), new Sentry.Replay()]
}
```

Closes https://github.com/getsentry/sentry/issues/46368

Problem statement:
JS Loader: Sentry.init should be called - with defaults from docs for Performance/Replay
As the title says, the goal should be that someone should be able to just paste the `<script/>` tag and without any manual calls to `Sentry.init` get the experience using it.
That means, for example, pageload transactions and Replays are automatically captured when Performance and/or Replay is enabled.

It should be possible to still call `Sentry.init` yourself and with that overwrite our defaults.

Hints from issue discussion:
Routing to @getsentry/team-web-sdk-frontend for [triage](https://develop.sentry.dev/processing-tickets/#3-triage), due by **<time datetime=2023-03-29T00:00:00.000Z>Tuesday, March 28th at 5:00 pm</time> (sfo)**. ⏲️
So, I think I have the loader stuff working (locally) now. However, IMHO it would be great to have some kind of E2E or integration test in sentry that actually checks the behavior, because this is not really tested as of now and I think very hard to test with the current setup. With the changes, there are basically the following scenarios that have slightly different behavior:

## Lazy mode

Lazy mode is only available for errors-only config. Users can manually opt-out of this by adding `data-lazy="no"` to their script tag.

In lazy mode, nothing happens until an error occurs. Once an error occurs, we download the CDN bundle, and then...:

* If `onLoad` is configured, trigger it
* If `SDK.init()` has not been called, call it with defaults

## Eager mode

If either performance or replay is enabled, we are always in eager (non-lazy) mode. In this mode, we'll automatically load the CDN bundle, and then...:

* If `onLoad` is configured, trigger it
* If `SDK.init()` has not been called, call it with defaults

## Defaults

The following default config is defined:

* `dsn`: Always set
* `debug: true` is set when debug bundles are enabled
* `tracesSampleRate: 0.1` is set when performance is enabled
* `replaysSessionSampleRate: 0.1` & `replaysOnErrorSampleRate: 1` are set when replay is enabled
* `integrations`: Will add `Sentry.BrowserTracing` and/or `Sentry.Replay` if performance/replay is enabled

This config can be overwritten by the user, and is merged. So e.g. if they do:

```js
Sentry.onLoad(function() {
  Sentry.init({
    tracesSampleRate: 0.5
  });
});
```

For a full loader build with every option enabled, the actual config will be:

```js
{
  dsn: 'your-dsn',
  debug: true,
  tracesSampleRate: 0.5,
  replaysSessionSampleRate: 0.1,
  replaysOnErrorSampleRate: 1
}
```
We also need to add `Sentry.BrowserTracing` and `Sentry.Replay` as well for auto-instrumentation.
> We also need to add `Sentry.BrowserTracing` and `Sentry.Replay` as well for auto-instrumentation.

Right, this too (actually I did this but forgot to write it down xD) will add it to the list above for completeness!

Commit message: fix loader for performance to v7

Patch to review:
diff --git a/src/sentry/templates/sentry/js-sdk-loader.js.tmpl b/src/sentry/templates/sentry/js-sdk-loader.js.tmpl
index aee842cba6b899..8e933c5387b44b 100644
--- a/src/sentry/templates/sentry/js-sdk-loader.js.tmpl
+++ b/src/sentry/templates/sentry/js-sdk-loader.js.tmpl
@@ -67,11 +67,15 @@
     _newScriptTag.crossOrigin = 'anonymous';
 
     // Once our SDK is loaded
-    _newScriptTag.addEventListener('load', function() {
+    _newScriptTag.addEventListener('load', function () {
       try {
-        // Restore onerror/onunhandledrejection handlers
-        _window[_onerror] = _oldOnerror;
-        _window[_onunhandledrejection] = _oldOnunhandledrejection;
+        // Restore onerror/onunhandledrejection handlers - only if not mutated in the meanwhile
+        if (_window[_onerror] && _window[_onerror].__SENTRY_LOADER__) {
+          _window[_onerror] = _oldOnerror;
+        }
+        if (_window[_onunhandledrejection] && _window[_onunhandledrejection].__SENTRY_LOADER__) {
+          _window[_onunhandledrejection] = _oldOnunhandledrejection;
+        }
 
         // Add loader as SDK source
         _window.SENTRY_SDK_SOURCE = 'loader';
@@ -80,6 +84,19 @@
 
         var oldInit = SDK.init;
 
+        var integrations = [];
+        if (_config.tracesSampleRate) {
+          integrations.push(new Sentry.BrowserTracing());
+        }
+
+        if (_config.replaysSessionSampleRate || _config.replaysOnErrorSampleRate) {
+          integrations.push(new Sentry.Replay());
+        }
+
+        if (integrations.length) {
+          _config.integrations = integrations;
+        }
+
         // Configure it using provided DSN and config object
         SDK.init = function(options) {
           var target = _config;
@@ -100,10 +117,14 @@
     _currentScriptTag.parentNode.insertBefore(_newScriptTag, _currentScriptTag);
   }
 
+  function sdkIsLoaded() {
+    var __sentry = _window['__SENTRY__'];
+    // If there is a global __SENTRY__ that means that in any of the callbacks init() was already invoked
+    return !!(!(typeof __sentry === 'undefined') && __sentry.hub && __sentry.hub.getClient());
+  }
+
   function sdkLoaded(callbacks, SDK) {
     try {
-      var data = queue.data;
-
       // We have to make sure to call all callbacks first
       for (var i = 0; i < callbacks.length; i++) {
         if (typeof callbacks[i] === 'function') {
@@ -111,12 +132,12 @@
         }
       }
 
-      var initAlreadyCalled = false;
-      var __sentry = _window['__SENTRY__'];
-      // If there is a global __SENTRY__ that means that in any of the callbacks init() was already invoked
-      if (!(typeof __sentry === 'undefined') && __sentry.hub && __sentry.hub.getClient()) {
-        initAlreadyCalled = true;
-      }
+      var data = queue.data;
+
+      var initAlreadyCalled = sdkIsLoaded();
+
+      // Call init first, if provided
+      data.sort((a, b) => a.f === 'init' ? -1 : 0);
 
       // We want to replay all calls to Sentry and also make sure that `init` is called if it wasn't already
       // We replay all calls to `Sentry.*` now
@@ -176,7 +197,6 @@
     }
   };
 
-
   [
     'init',
     'addBreadcrumb',
@@ -195,7 +215,7 @@
   // Store reference to the old `onerror` handler and override it with our own function
   // that will just push exceptions to the queue and call through old handler if we found one
   var _oldOnerror = _window[_onerror];
-  _window[_onerror] = function(message, source, lineno, colno, exception) {
+  _window[_onerror] = function() {
     // Use keys as "data type" to save some characters"
     queue({
       e: [].slice.call(arguments)
@@ -203,6 +223,7 @@
 
     if (_oldOnerror) _oldOnerror.apply(_window, arguments);
   };
+  _window[_onerror].__SENTRY_LOADER__ = true;
 
   // Do the same store/queue/call operations for `onunhandledrejection` event
   var _oldOnunhandledrejection = _window[_onunhandledrejection];
@@ -212,6 +233,7 @@
     });
     if (_oldOnunhandledrejection) _oldOnunhandledrejection.apply(_window, arguments);
   };
+  _window[_onunhandledrejection].__SENTRY_LOADER__ = true;
 
   if (!lazy) {
     setTimeout(function () {
diff --git a/src/sentry/templates/sentry/js-sdk-loader.min.js.tmpl b/src/sentry/templates/sentry/js-sdk-loader.min.js.tmpl
index 39b0f855f22bea..d45f71e760f234 100644
--- a/src/sentry/templates/sentry/js-sdk-loader.min.js.tmpl
+++ b/src/sentry/templates/sentry/js-sdk-loader.min.js.tmpl
@@ -1,4 +1,4 @@
-{% load sentry_helpers %}(function(c,w,C,q,r,k,D,E,x,l){function t(a){if(!y){y=!0;var f=w.scripts[0],d=w.createElement(C);d.src=E;d.crossOrigin="anonymous";d.addEventListener("load",function(){try{c[q]=u;c[r]=v;c.SENTRY_SDK_SOURCE="loader";var b=c[k],g=b.init;b.init=function(e){for(var h in e)Object.prototype.hasOwnProperty.call(e,h)&&(x[h]=e[h]);g(x)};F(a,b)}catch(e){console.error(e)}});f.parentNode.insertBefore(d,f)}}function F(a,f){try{for(var d=m.data,b=0;b<a.length;b++)if("function"===typeof a[b])a[b]();
-var g=!1,e=c.__SENTRY__;"undefined"!==typeof e&&e.hub&&e.hub.getClient()&&(g=!0);e=!1;for(b=0;b<d.length;b++)if(d[b].f){e=!0;var h=d[b];!1===g&&"init"!==h.f&&f.init();g=!0;f[h.f].apply(f,h.a)}!1===g&&!1===e&&f.init();var z=c[q],A=c[r];for(b=0;b<d.length;b++)"e"in d[b]&&z?z.apply(c,d[b].e):"p"in d[b]&&A&&A.apply(c,[d[b].p])}catch(G){console.error(G)}}var n=l,B=!1;for(l=0;l<document.scripts.length;l++)if(-1<document.scripts[l].src.indexOf(D)){n&&"no"===document.scripts[l].getAttribute("data-lazy")&&
-(n=!1);break}var y=!1,p=[],m=function(a){("e"in a||"p"in a||a.f&&-1<a.f.indexOf("capture")||a.f&&-1<a.f.indexOf("showReportDialog"))&&n&&t(p);m.data.push(a)};m.data=[];c[k]=c[k]||{};c[k].onLoad=function(a){p.push(a);n&&!B||t(p)};c[k].forceLoad=function(){B=!0;n&&setTimeout(function(){t(p)})};"init addBreadcrumb captureMessage captureException captureEvent configureScope withScope showReportDialog".split(" ").forEach(function(a){c[k][a]=function(){m({f:a,a:arguments})}});var u=c[q];c[q]=function(a,
-f,d,b,g){m({e:[].slice.call(arguments)});u&&u.apply(c,arguments)};var v=c[r];c[r]=function(a){m({p:"reason"in a?a.reason:"detail"in a&&"reason"in a.detail?a.detail.reason:a});v&&v.apply(c,arguments)};n||setTimeout(function(){t(p)})})(window,document,"script","onerror","onunhandledrejection","Sentry","{{ publicKey|safe }}","{{ jsSdkUrl|safe }}",{{ config|to_json|safe }},{{ isLazy|safe|lower }});
+{% load sentry_helpers %}(function(b,z,D,f,g,l,E,F,m,n){function p(a){("e"in a||"p"in a||a.f&&-1<a.f.indexOf("capture")||a.f&&-1<a.f.indexOf("showReportDialog"))&&q&&v(t);p.data.push(a)}function v(a){if(!A){A=!0;var h=z.scripts[0],c=z.createElement(D);c.src=F;c.crossOrigin="anonymous";c.addEventListener("load",function(){try{b[f]&&b[f].g&&(b[f]=w);b[g]&&b[g].g&&(b[g]=x);b.m="loader";var d=b[l],u=d.h,k=[];m.C&&k.push(new Sentry.j);(m.B||m.A)&&k.push(new Sentry.l);k.length&&(m.u=k);d.h=function(e){for(var r in e)Object.prototype.hasOwnProperty.call(e,
+r)&&(m[r]=e[r]);u(m)};G(a,d)}catch(e){console.error(e)}});h.parentNode.insertBefore(c,h)}}function H(){var a=b.__SENTRY__;return!("undefined"===typeof a||!a.i||!a.i.s())}function G(a,h){try{for(var c=0;c<a.length;c++)if("function"===typeof a[c])a[c]();var d=p.data,u=H();d.sort(function(y){return"init"===y.f?-1:0});var k=!1;for(c=0;c<d.length;c++)if(d[c].f){k=!0;var e=d[c];!1===u&&"init"!==e.f&&h.h();u=!0;h[e.f].apply(h,e.a)}!1===u&&!1===k&&h.h();var r=b[f],B=b[g];for(c=0;c<d.length;c++)"e"in d[c]&&
+r?r.apply(b,d[c].e):"p"in d[c]&&B&&B.apply(b,[d[c].p])}catch(y){console.error(y)}}var q=n,C=!1;for(n=0;n<document.scripts.length;n++)if(-1<document.scripts[n].src.indexOf(E)){q&&"no"===document.scripts[n].getAttribute("data-lazy")&&(q=!1);break}var A=!1,t=[];p.data=[];b[l]=b[l]||{};b[l].v=function(a){t.push(a);q&&!C||v(t)};b[l].o=function(){C=!0;q&&setTimeout(function(){v(t)})};"init addBreadcrumb captureMessage captureException captureEvent configureScope withScope showReportDialog".split(" ").forEach(function(a){b[l][a]=
+function(){p({f:a,a:arguments})}});var w=b[f];b[f]=function(){p({e:[].slice.call(arguments)});w&&w.apply(b,arguments)};b[f].g=!0;var x=b[g];b[g]=function(a){p({p:"reason"in a?a.reason:"detail"in a&&"reason"in a.detail?a.detail.reason:a});x&&x.apply(b,arguments)};b[g].g=!0;q||setTimeout(function(){v(t)})})(window,document,"script","onerror","onunhandledrejection","Sentry","{{ publicKey|safe }}","{{ jsSdkUrl|safe }}",{{ config|to_json|safe }},{{ isLazy|safe|lower }});
diff --git a/src/sentry/web/frontend/js_sdk_loader.py b/src/sentry/web/frontend/js_sdk_loader.py
index c5eb74c4e4dec0..9c814a6181bf6a 100644
--- a/src/sentry/web/frontend/js_sdk_loader.py
+++ b/src/sentry/web/frontend/js_sdk_loader.py
@@ -20,6 +20,10 @@
 
 class SdkConfig(TypedDict):
     dsn: str
+    tracesSampleRate: Optional[float]
+    replaysSessionSampleRate: Optional[float]
+    replaysOnErrorSampleRate: Optional[float]
+    debug: Optional[bool]
 
 
 class LoaderContext(TypedDict):
@@ -38,24 +42,28 @@ class JavaScriptSdkLoader(BaseView):
     def determine_active_organization(self, request: Request, organization_slug=None) -> None:
         pass
 
-    def _get_bundle_kind_modifier(self, key: ProjectKey, sdk_version: str) -> Tuple[str, bool]:
+    def _get_bundle_kind_modifier(
+        self, key: ProjectKey, sdk_version: str
+    ) -> Tuple[str, bool, bool, bool, bool]:
         """Returns a string that is used to modify the bundle name"""
 
         is_v7_sdk = sdk_version >= Version("7.0.0")
 
         is_lazy = True
         bundle_kind_modifier = ""
+        has_replay = get_dynamic_sdk_loader_option(key, DynamicSdkLoaderOption.HAS_REPLAY)
+        has_performance = get_dynamic_sdk_loader_option(key, DynamicSdkLoaderOption.HAS_PERFORMANCE)
+        has_debug = get_dynamic_sdk_loader_option(key, DynamicSdkLoaderOption.HAS_DEBUG)
 
         # The order in which these modifiers are added is important, as the
         # bundle name is built up from left to right.
         # https://docs.sentry.io/platforms/javascript/install/cdn/
 
-        if get_dynamic_sdk_loader_option(key, DynamicSdkLoaderOption.HAS_PERFORMANCE):
+        # We depend on fixes in the tracing bundle that are only available in v7
+        if is_v7_sdk and has_performance:
             bundle_kind_modifier += ".tracing"
             is_lazy = False
 
-        has_replay = get_dynamic_sdk_loader_option(key, DynamicSdkLoaderOption.HAS_REPLAY)
-
         # If the project does not have a v7 sdk set, we cannot load the replay bundle.
         if is_v7_sdk and has_replay:
             bundle_kind_modifier += ".replay"
@@ -69,10 +77,10 @@ def _get_bundle_kind_modifier(self, key: ProjectKey, sdk_version: str) -> Tuple[
         if is_v7_sdk and not has_replay:
             bundle_kind_modifier += ".es5"
 
-        if get_dynamic_sdk_loader_option(key, DynamicSdkLoaderOption.HAS_DEBUG):
+        if has_debug:
             bundle_kind_modifier += ".debug"
 
-        return bundle_kind_modifier, is_lazy
+        return bundle_kind_modifier, is_lazy, has_performance, has_replay, has_debug
 
     def _get_context(
         self, key: Optional[ProjectKey]
@@ -89,7 +97,13 @@ def _get_context(
 
         sdk_version = get_browser_sdk_version(key)
 
-        bundle_kind_modifier, is_lazy = self._get_bundle_kind_modifier(key, sdk_version)
+        (
+            bundle_kind_modifier,
+            is_lazy,
+            has_performance,
+            has_replay,
+            has_debug,
+        ) = self._get_bundle_kind_modifier(key, sdk_version)
 
         js_sdk_loader_default_sdk_url_template_slot_count = (
             settings.JS_SDK_LOADER_DEFAULT_SDK_URL.count("%s")
@@ -108,9 +122,21 @@ def _get_context(
         except TypeError:
             sdk_url = ""  # It fails if it cannot inject the version in the string
 
+        config: SdkConfig = {"dsn": key.dsn_public}
+
+        if has_debug:
+            config["debug"] = True
+
+        if has_performance:
+            config["tracesSampleRate"] = 0.1
+
+        if has_replay:
+            config["replaysSessionSampleRate"] = 0.1
+            config["replaysOnErrorSampleRate"] = 1
+
         return (
             {
-                "config": {"dsn": key.dsn_public},
+                "config": config,
                 "jsSdkUrl": sdk_url,
                 "publicKey": key.public_key,
                 "isLazy": is_lazy,
diff --git a/tests/sentry/web/frontend/test_js_sdk_loader.py b/tests/sentry/web/frontend/test_js_sdk_loader.py
index 39b3b66fafd161..7e2d35cf6d602b 100644
--- a/tests/sentry/web/frontend/test_js_sdk_loader.py
+++ b/tests/sentry/web/frontend/test_js_sdk_loader.py
@@ -8,6 +8,7 @@
 
 from sentry.loader.dynamic_sdk_options import DynamicSdkLoaderOption
 from sentry.testutils import TestCase
+from sentry.utils import json
 
 
 class JavaScriptSdkLoaderTest(TestCase):
@@ -113,7 +114,9 @@ def test_bundle_kind_modifiers(self, load_version_from_file, get_selected_browse
         settings.JS_SDK_LOADER_DEFAULT_SDK_URL = "https://browser.sentry-cdn.com/%s/bundle%s.min.js"
         settings.JS_SDK_LOADER_SDK_VERSION = "7.32.0"
 
-        for data, expected in [
+        dsn = self.projectkey.get_dsn(public=True)
+
+        for data, expected_bundle, expected_options in [
             (
                 {
                     "dynamicSdkLoaderOptions": {
@@ -121,6 +124,7 @@ def test_bundle_kind_modifiers(self, load_version_from_file, get_selected_browse
                     }
                 },
                 b"/7.37.0/bundle.tracing.es5.min.js",
+                {"dsn": dsn, "tracesSampleRate": 0.1},
             ),
             (
                 {
@@ -129,6 +133,7 @@ def test_bundle_kind_modifiers(self, load_version_from_file, get_selected_browse
                     }
                 },
                 b"/7.37.0/bundle.es5.debug.min.js",
+                {"dsn": dsn, "debug": True},
             ),
             (
                 {
@@ -137,6 +142,7 @@ def test_bundle_kind_modifiers(self, load_version_from_file, get_selected_browse
                     }
                 },
                 b"/7.37.0/bundle.replay.min.js",
+                {"dsn": dsn, "replaysSessionSampleRate": 0.1, "replaysOnErrorSampleRate": 1},
             ),
             (
                 {
@@ -146,6 +152,12 @@ def test_bundle_kind_modifiers(self, load_version_from_file, get_selected_browse
                     }
                 },
                 b"/7.37.0/bundle.tracing.replay.min.js",
+                {
+                    "dsn": dsn,
+                    "tracesSampleRate": 0.1,
+                    "replaysSessionSampleRate": 0.1,
+                    "replaysOnErrorSampleRate": 1,
+                },
             ),
             (
                 {
@@ -155,6 +167,12 @@ def test_bundle_kind_modifiers(self, load_version_from_file, get_selected_browse
                     }
                 },
                 b"/7.37.0/bundle.replay.debug.min.js",
+                {
+                    "dsn": dsn,
+                    "replaysSessionSampleRate": 0.1,
+                    "replaysOnErrorSampleRate": 1,
+                    "debug": True,
+                },
             ),
             (
                 {
@@ -164,6 +182,7 @@ def test_bundle_kind_modifiers(self, load_version_from_file, get_selected_browse
                     }
                 },
                 b"/7.37.0/bundle.tracing.es5.debug.min.js",
+                {"dsn": dsn, "tracesSampleRate": 0.1, "debug": True},
             ),
             (
                 {
@@ -174,6 +193,13 @@ def test_bundle_kind_modifiers(self, load_version_from_file, get_selected_browse
                     }
                 },
                 b"/7.37.0/bundle.tracing.replay.debug.min.js",
+                {
+                    "dsn": dsn,
+                    "tracesSampleRate": 0.1,
+                    "replaysSessionSampleRate": 0.1,
+                    "replaysOnErrorSampleRate": 1,
+                    "debug": True,
+                },
             ),
         ]:
             self.projectkey.data = data
@@ -181,7 +207,13 @@ def test_bundle_kind_modifiers(self, load_version_from_file, get_selected_browse
             resp = self.client.get(self.path)
             assert resp.status_code == 200
             self.assertTemplateUsed(resp, "sentry/js-sdk-loader.js.tmpl")
-            assert expected in resp.content
+            assert expected_bundle in resp.content
+
+            for key in expected_options:
+                # Convert to e.g. "option_name": 0.1
+                single_option = {key: expected_options[key]}
+                assert bytes(json.dumps(single_option)[1:-1], "utf-8") in resp.content
+
             self.projectkey.data = {}
             self.projectkey.save()
 


Code review comments:
diff_hunk: @@ -108,9 +122,21 @@ def _get_context(
         except TypeError:
             sdk_url = ""  # It fails if it cannot inject the version in the string
 
+        config: SdkConfig = {"dsn": key.dsn_public}
+
+        if has_debug:
+            config["debug"] = True
+
+        if has_performance:
+            config["tracesSampleRate"] = 0.1
path: src/sentry/web/frontend/js_sdk_loader.py
text: @user1:
```suggestion
            config["tracesSampleRate"] = 1
```
Let's change this to `1`