ref(tracing): Split @sentry/tracing into runtime specific packages

@lforst

Problem Statement

After some discussions around supporting newer JavaScript runtimes (#5611) @lforst pointed out the first thing that needs tackling is @sentry/tracing!

This package currently contains:

Runtime agnostic code used to add transaction support to the hub
Browser specific instrumentation code
Node.js specific instrumentation code

Goal: Remove the tracing package, move stuff into core/browser/node

Solution Brainstorm

It looks like the runtime agnostic code can safely be moved to @sentry/core without affecting bundle sizes since if it goes unused it gets tree-shaken.

The runtime specific code could be moved into package exports under the specific runtime, ie. @sentry/browser/tracing and @sentry/node/tracing. We've been using this in the Electron SDK for a while to keep bundlers happy with the two different contexts we run in. We can't export the tracing code at the root of the packages since importing tracing has side-effects!

I haven't marked this issue as Breaking yet because I think it would be possible to move this code while keeping @sentry/tracing as a stub that would simply re-export the moved code. @sentry/tracing would have to depend on both @sentry/browser and @sentry/node so I'm not sure how viable this is yet or if it's worth that extra step 🤔

### Tasks
- [ ] https://github.com/getsentry/sentry-javascript/issues/7091
- [ ] https://github.com/getsentry/sentry-javascript/pull/7345
- [ ] https://github.com/getsentry/sentry-javascript/issues/7342
- [ ] https://github.com/getsentry/sentry-javascript/issues/7346
- [ ] https://github.com/getsentry/sentry-javascript/issues/7340

### Cleanup
- [ ] https://github.com/getsentry/sentry-docs/issues/6518
- [ ] https://github.com/getsentry/sentry-javascript/issues/7586

Continuing the thread from #5611 (comment):

Would we want to add all the hub extensions by default too?

I think so. Ideally, we wouldn't need hub extensions anymore since it's all already there.

We wouldn't want to run _autoloadDatabaseIntegrations in the Electron SDK because it hits app startup time significantly!

We had an internal discussion about this some time ago and we would like to turn off auto-discovery by default for the next major because of the startup time.

If we move the existing runtime agnostic code over to @sentry/core without any significant changes and simply export the existing code there is no change to browser bundle size since it's just a load of isolated functions.

I still need to do some experimentation to see how much it adds if the code is fully integrated to the hub without extensions. I would guess this could add 5-10kB since the code ends up in the Hub class where tree shaking can't works its magic.

I did the experimenting and moved tracing into @sentry/core and moved the code into Hub (removing hub extensions) and it adds roughly 7kB to the minified, uncompressed browser bundle (bundle.min.js).

This PR on my fork shows how the runtime agnostic code from @sentry/tracing can be moved to @sentry/core without impacting bundle size. This is possible because it retains the use of hub extensions and the tracing code is not referenced by @sentry/core so it can be tree-shaken.

After doing some experimenting, we cannot use the approach in #7345 because this would break older bundlers.

As per #7372, we must do this change in v8

A possible option we can take on here is to create a new tracing package - and then make @sentry/tracing, @sentry/node and @sentry/browser all import from that new package.

This new package can export everything without side effects so it can be tree-shaken/not slow down the node package.

IMHO that can be fine, we can make the package purely private/internal, so it would really just be an implementation detail!

It's been a while, but I think we worked around the older bundler issues while still having named package exports with the Electron SDK by outputting the cjs build output into correctly named directories in the root of the package.

  "exports": {
    "./main": {
      "require": "./main/index.js",
      "import": "./esm/main/index.js"
    },
    "./renderer": {
      "require": "./renderer/index.js",
      "import": "./esm/renderer/index.js"
    }
  },

We don't actually have any webpack 4 e2e tests, but I just did some quick testing and with the above, both const { init } = require('@sentry/electron/renderer'); and import { init } from '@sentry/electron/renderer'; work with webpack 4.

The major downside here is that cjs gets used and therefore no tree shaking occurs. In the case of the Electron SDK, this means that all the session replay code gets bundled. cjs is no doubt why we've seen issues like this even when Replay isn't used.

The major downside here is that cjs gets used and therefore no tree shaking occurs

This is what I ran into, and unfortunately means we can't move forward with this for the stock browser package.

Once the above two linked PRs are merged, I'm guessing the next steps go something like:

Export BrowserTracing and related consts/types from @sentry/browser
Configure browser tracing bundler config from @sentry/browser
Export Node.js tracing integrations from @sentry/node
Update docs to cater for all the above changes
Deprecate all exports in @sentry/tracing

For v8

Remove @sentry/tracing package

@timfish

@timfish one thing we'll have to consider is hub extensions and auto discovery of database integrations.

We'll want to make sure the flow looks like so:

Browser

import * as Sentry from '@sentry/browser';

Sentry.init({
  dsn: "__DSN__",
  tracesSampleRate: 1.0,
  integrations: [new Sentry.BrowserTracing()]
});

If you don't want to use BrowserTracing, you'll have to add the tracing extensions yourself:

import * as Sentry from '@sentry/browser';

Sentry.addTracingExtensions();

Sentry.init({
  dsn: "__DSN__",
  tracesSampleRate: 1.0,
});

Node

import * as Sentry from '@sentry/node';

Sentry.addTracingExtensions();
Sentry.discoverTracingIntegrations();

Sentry.init({
  dsn: "__DSN__",
  tracesSampleRate: 1.0,
});

We need to think about what types need re-exporting from node/browser from @sentry/core. getActiveTransaction is the obvious one but I'm sure there are more.

Once the migration is complete, but before we add any deprecation jsdocs, we'll try to migrate all the Sentry packages to use node/browser rather than tracing. This should give us a good idea of what exports we need!

Since we've removed @sentry/tracing from all the sdks that use it, and migrated the imports to node/core/browser accordingly, I think we can consider this done.

Next up is docs work in getsentry/sentry-docs#6518 and final deprecation in #7586, but we can use those issues to track this.

Navigation Menu

Repository navigation

ref(tracing): Split `@sentry/tracing` into runtime specific packages #5815

Problem Statement

Goal: Remove the tracing package, move stuff into core/browser/node

Solution Brainstorm

1 remaining item

Browser

Node

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

ref(tracing): Split @sentry/tracing into runtime specific packages #5815

Description

Problem Statement

Goal: Remove the tracing package, move stuff into core/browser/node

Solution Brainstorm

Activity

lforst commented on Sep 26, 2022

timfish commented on Sep 26, 2022

timfish commented on Sep 27, 2022

timfish commented on Oct 5, 2022

1 remaining item

AbhiPrasad commented on Mar 7, 2023

AbhiPrasad commented on Mar 7, 2023

AbhiPrasad commented on Mar 9, 2023

mydea commented on Mar 9, 2023

timfish commented on Mar 10, 2023

AbhiPrasad commented on Mar 10, 2023

timfish commented on Mar 13, 2023

AbhiPrasad commented on Mar 14, 2023

Browser

Node

timfish commented on Mar 17, 2023

AbhiPrasad commented on Mar 23, 2023

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

ref(tracing): Split `@sentry/tracing` into runtime specific packages #5815