Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@sentry/nextjs bundle size and tree shaking #7680

Closed
3 tasks done
carbonrobot opened this issue Mar 30, 2023 · 23 comments · Fixed by #7710
Closed
3 tasks done

@sentry/nextjs bundle size and tree shaking #7680

carbonrobot opened this issue Mar 30, 2023 · 23 comments · Fixed by #7710

Comments

@carbonrobot
Copy link

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using? If you use the CDN bundles, please specify the exact bundle (e.g. bundle.tracing.min.js) in your SDK setup.

@sentry/nextjs

SDK Version

7.46.0

Framework Version

7.46.0

Link to Sentry event

No response

SDK Setup

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

const SENTRY_DSN = process.env.SENTRY_DSN || process.env.NEXT_PUBLIC_SENTRY_DSN;

Sentry.init({
  dsn: SENTRY_DSN,
  environment: process.env.STAGE
});

Steps to Reproduce

Using @sentry/nextjs": "^7.15.0, the bundle analyzer reports

Stat: 84.93KB
Parsed Size: 20.52KB
Gzipped: 6.41KB

Using @sentry/nextjs": "^7.46.0, the bundle analyzer reports

Stat: 305.32KB
Parsed Size: 50.19KB
Gzipped: 15.56KB

Attempting to follow the tree shaking guide and the following setup produces roughly the same result

import {
  defaultStackParser,
  getCurrentHub,
  Breadcrumbs,
  BrowserClient,
  makeFetchTransport,
  Dedupe,
  FunctionToString,
  GlobalHandlers,
  HttpContext,
  InboundFilters,
  LinkedErrors,
  TryCatch,
} from '@sentry/nextjs';

const SENTRY_DSN = process.env.SENTRY_DSN || process.env.NEXT_PUBLIC_SENTRY_DSN;

getCurrentHub().bindClient(
  new BrowserClient({
    dsn: SENTRY_DSN,
    transport: makeFetchTransport,
    stackParser: defaultStackParser,
    environment: process.env.STAGE,
    integrations: [
      new Breadcrumbs(),
      new Dedupe(),
      new FunctionToString(),
      new GlobalHandlers(),
      new HttpContext(),
      new InboundFilters(),
      new LinkedErrors(),
      new TryCatch(),
    ],
  })
);

Expected Result

The bundle size should not constitute a significant portion of the overall application bundle, or at least be tree shakeable.

Actual Result

The bundle size has grown significantly over past releases

The following related issue does not seem to have a resolution

#2707

@lforst
Copy link
Member

lforst commented Mar 31, 2023

We do have a continuous effort to keep bundle size low. Unfortunately for the amount of features we provide (and users want to see and use) there is a trade-off to be had. Trust me when I say, that the term "bundle size" is a term that is mentioned almost daily in the team ^^

I am gonna close this issue because it is not actionable at the moment. Feel free to reopen if you have a suspicion that this is a) a regression or b) if you have an idea of how to resolve this.

In case of a) please also provide a link to a website where we can check if unwanted code is included.

@davidfurlong
Copy link

davidfurlong commented Apr 1, 2023

Screenshot 2023-04-01 at 13 40 13

this is kinda unusable for a bunch of kinds of apps; production build - standard @sentry/next-js setup on "@sentry/nextjs": "^7.46.0",

@rchl
Copy link
Contributor

rchl commented Apr 2, 2023

Inclusion of @sentry/replay is probably the biggest offender as it's probably not used by most people anyway.

I would think that NOT doing import * as Sentry should tree shake it out but I don't have experience with Next SDK, only with Vue SDK, where it does indeed help to not import *.

@lforst
Copy link
Member

lforst commented Apr 3, 2023

I couldn't reproduce this exact issue in a test app but something caught my eye and I have a feeling it is gonna fix this: #7710

@philwolstenholme
Copy link

Do you know when that change might make it into a release @lforst ? I'm evaluating Sentry with Next at the moment at work, but adding it has taken us over our performance budget.

@lforst
Copy link
Member

lforst commented Apr 3, 2023

@philwolstenholme probably sometime in the next few days. Just to confirm what you mean by performance budget: It's bundle size right and not Sentry Transaction quota, right?

@philwolstenholme
Copy link

@lforst ah yes sorry, it's nothing to do with Sentry, it's Lighthouse in CI running against a deployment preview URL.

@luukdv
Copy link

luukdv commented Apr 3, 2023

Another option is not bundling Sentry in your main bundle (if you can accept the tradeoff of not having Sentry available from the very first millisecond):

// sentry.js

export default function sentry() {
  Sentry.init();
}
// main.js

import("./sentry").then((sentry) => sentry.default());

@carbonrobot
Copy link
Author

@luukdv absolutely. I've been looking at ways of removing our dependency on the sentry/nextjs package and just pulling everything in manually to avoid the bundling. Nextjs packages are already fairly large and as much as we love sentry we can't take the size it imposes for many apps

@lforst
Copy link
Member

lforst commented Apr 3, 2023

Another option is not bundling Sentry in your main bundle (if you can accept the tradeoff of not having Sentry available from the very first millisecond):

// sentry.js

export default function sentry() {
  Sentry.init();
}
// main.js

import("./sentry").then((sentry) => sentry.default());

I have a feeling this won't work due to how we bundle Sentry in Next.js app but I will happily be proven otherwise! Also I think this will mess up performance measurements and request instrumentation as these usually require Sentry to be initialized asap.

@carbonrobot
Copy link
Author

The 2 issues I see are (305KB total bundle size for @sentry)

  • Express/Nodejs code is bundled into the CLIENT browser bundle (36KB)
  • @sentry/internal/tracing gets bundled, regardless of settings, even when disabling in webpack (76KB)
  • core/esm/tracing (11KB)

The Nodejs integrations definitely seem like a bug

image

@lforst
Copy link
Member

lforst commented Apr 4, 2023

@carbonrobot Hi, thanks for letting us know about this. The node stuff is something that slipped through. Fix: #7720

@luukdv
Copy link

luukdv commented Apr 4, 2023

I have a feeling this won't work due to how we bundle Sentry in Next.js app

You're probably right! Haven't tested this setup in a Next.js app.

Also I think this will mess up performance measurements and request instrumentation as these usually require Sentry to be initialized asap.

Yup, this is mostly for simple/non-critical use-cases, where for example only runtime error reporting is necessary (which is indeed what I meant with the "accepting tradeoffs" comment) 🙂

@sanex3339
Copy link

sanex3339 commented Apr 5, 2023

Another option is not bundling Sentry in your main bundle (if you can accept the tradeoff of not having Sentry available from the very first millisecond):

// sentry.js

export default function sentry() {
  Sentry.init();
}
// main.js

import("./sentry").then((sentry) => sentry.default());

The problem with this approach is that Sentry is extracted to the separate chunk, but this chunk is loaded at the same time as the next.js chunk.
image
You can see that the chunk contains also some next.js modules

We're trying to conditionally disable sentry (including loading its code) on the client side based on a query parameter (for debug purposes), so we want to load it lazily only if the query parameter is missing.
But right now because this behavior with lazy chunks we can't properly disable sentry in some cases.

For example, just for client bundle i tried to replace @sentry/nextjs by @sentry/react and it's extracted to the lazy chunk properly.

@okonet
Copy link

okonet commented Apr 5, 2023

Similar issue here with the latest @sentry/next taking ~50% of my _app bundle.

CleanShot 2023-04-05 at 19 12 10@2x

Is there any way of reducing it?

Update:

Updated to 7.47.0 and the bundle size went from 56KB to 53KB which is a bit better but still huge compared to the rest of my app:

CleanShot 2023-04-05 at 19 18 48@2x

@lforst
Copy link
Member

lforst commented Apr 6, 2023

@okonet you can follow our tree shaking guide if you aren't already: https://docs.sentry.io/platforms/javascript/guides/nextjs/configuration/tree-shaking/

Note that you will miss out on features though!

@okonet
Copy link

okonet commented Apr 6, 2023

I actually already did this so I was expecting a bigger savings. Right now I see that the replay is about 50% of the bundle but it still leaves me with about 30KB if I disable it completely. If this the minimum bundle size possible?

@lforst
Copy link
Member

lforst commented Apr 6, 2023

@okonet I am not sure where we're at with the minimum size right now but it seems about right. I am sorry that the bundle is quite large. If you have suggestions on how to improve it, let us know.

@okonet
Copy link

okonet commented Apr 6, 2023

Okay thanks for the info. I'll play around with the custom client setup to shrink it a bit further.

@stevez86
Copy link

stevez86 commented Jun 5, 2023

Due to the attitude of the team around bundle size, we are removing sentry from our business.

@lforst
Copy link
Member

lforst commented Jun 6, 2023

Due to the attitude of the team around bundle size, we are removing sentry from our business.

Respectable decision

@mtariqsajid
Copy link

mtariqsajid commented Aug 12, 2023

can we use sentry in party town so it should load lazy ?

@lforst
Copy link
Member

lforst commented Aug 14, 2023

can we use sentry in party town so it should load lazy ?

We have a loader script that will load the SDK lazily but comes with a few drawbacks - like for instance no special Next.js performance instrumentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants