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 adds 315KB to bundle - can this be reduced/avoided? #7842

Closed
andyjessop opened this issue Apr 13, 2023 · 19 comments
Closed

Sentry adds 315KB to bundle - can this be reduced/avoided? #7842

andyjessop opened this issue Apr 13, 2023 · 19 comments

Comments

@andyjessop
Copy link

Environment

SaaS (https://sentry.io/)

Version

No response

Link

No response

DSN

No response

Steps to Reproduce

  1. Import the following into your react application:
import {
  Event as SentryEvent,
  init as sentryInit,
  reactRouterV6Instrumentation,
  setContext,
  setTags,
  setUser,
  withSentryReactRouterV6Routing,
} from '@sentry/react';
import { BrowserTracing } from '@sentry/tracing';

Result: bundle increased by 315KB

Expected Result

A much smaller bundle, or a way to avoid the client-side library all-together.

Actual Result

Screenshot 2023-04-13 at 14 41 50

@getsantry
Copy link

getsantry bot commented Apr 13, 2023

Assigning to @getsentry/support for routing, due by Thursday, April 13th at 5:00 pm (sfo). ⏲️

@getsantry
Copy link

getsantry bot commented Apr 13, 2023

Routing to @getsentry/team-web-sdk-frontend for triage, due by Friday, April 14th at 5:00 pm (sfo). ⏲️

@AbhiPrasad AbhiPrasad transferred this issue from getsentry/sentry Apr 13, 2023
@AbhiPrasad
Copy link
Member

Hey @andyjessop, thanks for writing in! Have you tried looking at our tree-shaking docs to help improve this bundle size? You can add some build time logic to tree-shake away debug logic from the sdk.

As per our latest release our ES6 bundle is 57.75 KB minified - so having a bundle size of 315 KB seems really strange. Are you running this bundle analysis on a production build after tree-shaking has happened?

@andyjessop
Copy link
Author

We're using __SENTRY_DEBUG__: false and this is our following init:

import {
  Event as SentryEvent,
  init as sentryInit,
  reactRouterV6Instrumentation,
  setContext,
  setTags,
  setUser,
  withSentryReactRouterV6Routing,
} from '@sentry/react';
import { BrowserTracing } from '@sentry/tracing';

...

 sentryInit({
    beforeSend: (event: SentryEvent) => {
    dsn: SENTRY_URL,
    environment,
    ignoreErrors: [
      'ResizeObserver loop limit exceeded',
      'ResizeObserver loop completed with undelivered notifications',
      'window.webkit.messageHandlers',
      'avast',
      'file://',
      'translate.goog',
      'zaloJSV2',
      'ceCurrentVideo',
      'instantSearchSDKJSBridgeClearHighlight',
    ],
    integrations: [
      new BrowserTracing({
        routingInstrumentation: reactRouterV6Instrumentation(
          React.useEffect,
          useLocation,
          useNavigationType,
          createRoutesFromChildren,
          matchRoutes,
        ),

        tracingOrigins: [window.config.VITE_URL],
      }),
    ],
    release: SENTRY_RELEASE,
    // https://docs.io/platforms/javascript/guides/react/performance/#configure-the-sample-rate
    tracesSampleRate: 0.2,
  });
}

export const setUserData = (plan: string, uuid: string): void => setUser({ plan, uuid });

export const setFeatures = (features: string[]): void => setContext('features', features);

@AbhiPrasad
Copy link
Member

Seeing you are using tracing this number makes a lot of sense. When I ran locally this produced 28 kb gzipped, which is in-line with our expectations when you are using react + react routing instrumentation + performance monitoring. In addition the actual parse time should be minimal and pretty optimized for any browsers older than IE11.

Are you experiencing delays with main thread block time/LCP because of this additional bundle size cost? We try to only tackle bundle size when it makes a noticeable impact to user experience - which we did with the v7 version.

We are always working toward making the bundle size better, but at the end of the day features always come with bundle size cost!

@andyjessop
Copy link
Author

We have done analysis of user behaviour on our app as correlated with page load time, and found strong evidence of how page load time reduces conversions. Our page load time is essentially directly proportional to our bundle size, and currently Sentry takes up 1/10th of our bundle.

If this turns out to be due to tracing, I suspect we'll remove it and look for alternative performance monitoring. If monitoring of performance is hurting our performance, then there's a fundamental issue there.

@lforst
Copy link
Member

lforst commented Apr 14, 2023

dank meme

Jk.. On a more serious note, there are some bundle-size wins we could have but they would all come at the sacrifice of individual features. I would be interested: If you had more choice over what specific features you wanted to reduce bundle size, would this lessen the issue for you?

@andyjessop
Copy link
Author

I would prefer not to have a client-side bundle at all, and instead have a REST API where I can send exceptions.

@lforst
Copy link
Member

lforst commented Apr 14, 2023

I would prefer not to have a client-side bundle at all, and instead have a REST API where I can send exceptions.

I don't think you will get away without some amount of client-side code. From experience, I can say that instrumenting all places where stuff can go wrong is not trivial. Especially when performance comes into play. Think about distributed tracing and all.

Also, Sentry is more or less a REST API that you're sending errors to ^^

@github-actions
Copy link
Contributor

github-actions bot commented May 6, 2023

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@tonisives
Copy link

tonisives commented May 7, 2023

I also get 548kb>855kb for my bundle for a AWS Lambda function.

Lambda is built with terraform cdktf. It uses esbuild inside

build info from terraform

Inspect the code for a fully integrated approach in fully-integrated branch. This approach uses yarn based monorepo setup and introduces a NodejsFunction Construct which bundles the code transparently via esbuild. Visit this PR includes additional context and instructions.

I tried treeshaking with this code

import {
  init as sentryInit,
  captureException
} from "@sentry/node"


sentryInit({
  dsn: process.env.SENTRY_DSN,
  tracesSampleRate: 1.0,
});

export let captureError = (payload: any) => {
  captureException(new Error("my error"))
}

but it's still 300kb.

As a quick solution, do you have a REST API I could use? I checked https://docs.sentry.io/api/, but it doesn't write about capturing exceptions.

@lforst
Copy link
Member

lforst commented May 8, 2023

@tonisives Here's all you need to know about developing an SDK: https://develop.sentry.dev/sdk/

@AbhiPrasad
Copy link
Member

^ Adding to that, you'll need to use envelopes to send information to Sentry. See: https://develop.sentry.dev/sdk/envelopes/

@tonisives
Copy link

tonisives commented May 11, 2023

Thanks for the answers @lforst and @AbhiPrasad.

I ran some more tests with my esbuild process. I can see that there is minimal improvement with defining compiler flags.

Tree shaking implementation

I followed the tree shaking with minimal integrations. The bundle size is still the same. around 300kb

Here is my working code

import {
  NodeClient,
  defaultStackParser,
  makeNodeTransport,
  getCurrentHub,
  Integrations
} from "@sentry/node"

let client = new NodeClient({
  dsn: "https://@.ingest.sentry.io/",
  transport: makeNodeTransport,
  stackParser: defaultStackParser,
  integrations: [new Integrations.Http()]
});

getCurrentHub().bindClient(client);

let captureError = (payload: Error) => {
  client.captureException(payload)
}
Screenshot 2023-05-11 at 08 25 59

Using compiler flags

I tried to add define keys to my esbuild function via the define variable.

import { buildSync } from 'esbuild';

const compilerFlags = {
  "__SENTRY_DEBUG__": "false",
  "__SENTRY_TRACING__": "false",
};

buildSync({
  entryPoints: ['src/index.ts'],
  platform: 'node',
  target: 'es2018',
  bundle: true,
  format: 'cjs',
  sourcemap: 'external',
  outdir: 'dist',
  absWorkingDir: workingDirectory,
  define: compilerFlags,
});

This reduces the bundle size by 4kb. But the total bundle size is still around 300kb, which is too big.

Screenshot 2023-05-11 at 08 41 23

size without sentry

Screenshot 2023-05-11 at 08 45 00

More ideas

I am reading here about gzipping. But I haven't investigated how to do this with esbuild yet or if it does it automatically. As I said this is an AWS Lambda function, so maybe it has specific requirements that don't allow gzipping.

@lforst
Copy link
Member

lforst commented May 11, 2023

@tonisives Based on your eslint setup you're not minifying the output. I recommend you try that first. https://esbuild.github.io/api/#minify

@tonisives
Copy link

I tried and it helped a bit. From 837kb > 793kb
Screenshot 2023-05-11 at 13 32 11

What's weird is that the built lambda function is 183kb zipped. But the final uploaded terraform zip file is 813kb.

I don't want to bother you with my issue much more as I'm not completely aware about how the build process works with terraform cdktf. I also found another solution for my problem.

solution

I created a separate Lambda function with the Sentry library. This way my main project doesn't need the Sentry dependency at all.

here is the building

here is the lambda function

This function's size is 293.4 kb. Without Sentry the size is 1.2kb.

@lforst
Copy link
Member

lforst commented May 11, 2023

When I bundle the Node SDK with the following config it's about 200kb - no tree shaking of logger messages:

build({
  entryPoints: ["./src/test.js"],
  outdir: "./out/esbuild",
  platform: "node",
  minify: true,
  bundle: true,
  format: "cjs",
  sourcemap: true,
});

I guess that's the size for now and I think that is acceptable for a node package.

@tonisives
Copy link

tonisives commented May 18, 2023

Follow up:
The biggest issue was actually the sourcemap: true. When I set it to false, it reduced the bundle size from 288kb to 60kb.

@lforst
Copy link
Member

lforst commented May 19, 2023

@tonisives thanks for sharing the learning. Probably good to note that sourcemaps won't influence bundlesize and serverless startup times unless inlined.

Closing this.

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

No branches or pull requests

4 participants