@sentry/aws-serverless instrumentation ineffective

[jd-carroll]

Is there an existing issue for this?

• [X] I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
• [X] I have reviewed the documentation https://docs.sentry.io/
• [X] I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/aws-serverless

SDK Version

8.0.0

Framework Version

No response

Link to Sentry event

No response

SDK Setup

No response

Steps to Reproduce

The "0 bytes" indicates esbuild has dropped them from the bundle.

Expected Result

NA

Actual Result

NA

[jd-carroll]

@mydea I missed this in my other comment, the aws instrumentation is now completely dropped.

[Lms24]

Hey @jd-carroll just to confirm, are you using @sentry/aws-serverless directly or the Sentry AWS Lambda Layer?

I wonder what makes esbuild shake out the two AWS instrumentations. They're directly referenced in the SDK init function: https://github.com/getsentry/sentry-javascript/blob/d651f1319c95867be03c55f95c5878771c1e4c26/packages/aws-serverless/src/sdk.ts#L63-L97

Regardless, we'll investigate this soon. Thanks for raising!

[Lms24]

You probably shared this already in another issue but would you mind posting your Sentry.init function call again? Thanks!

[jd-carroll]

I appreciate the support on this, you guys have been real quick with turn around this issue.

I am not using the lambda layer. If I were using a lambda layer, I'd be marking all @sentry/* as external to esbuild. (so nothing would be included at all)

For completeness, here is what the esbuild output looks like if I do that:

(So that's 800kb from Sentry alone --> Honestly, that sentry is 2x the size of the lambda otherwise makes this more of a critical issue for me)

This is my Sentry.init, it is injected into the top of the generated files using inject from esbuild

import * as Sentry from '@sentry/aws-serverless';

function SentryInit() {
  const dsn = buildSentryDsn();
  if (!dsn) {
    return;
  }

  Sentry.init({
    dsn,
    tracesSampleRate: 1
  });
}

SentryInit();

And to further clarify, I am running esbuild with minify: true because that is the only way dead code elmination actually runs.

(Continuation from #11991)

While the aws instrumentation is an issue, I'd actually suggest focusing on why the other instrumentation elements are there.

I'll also point to my comment on the last issue: https://github.com/getsentry/sentry-javascript/issues/11991#issuecomment-2108201858

TLDR: I think @sentry/node should be broken up into "node-core" and "node". "node-core" representing the composable pieces and "node" as the instantiation of those pieces. That way "aws-serverless" can just depend on "node-core". (I also do not think this would result in any breaking changes.)

[timfish]

Are you bundling to ESM or CommonJs and are the Sentry dependencies being loaded via the ESM paths?

The esbuild docs say:

The main fields setting is set to main,module. This means tree shaking will likely not happen for packages that provide both module and main since tree shaking works with ECMAScript modules but not with CommonJS modules.

Unfortunately some packages incorrectly treat module as meaning "browser code" instead of "ECMAScript module code" so this default behavior is required for compatibility. You can manually configure the main fields setting to module,main if you want to enable tree shaking and know it is safe to do so.

And both main and module set:

https://github.com/getsentry/sentry-javascript/blob/13904020da1e0a988a44ff83ef3dfebab922227b/packages/node/package.json#L20-L21

The docs also suggest that esbuild just doesn't support tree shaking with CommonJs.

[jd-carroll]

Hey @timfish, thanks for the info, but just to clarify: Are you saying you have a working ESM build that successfully treeshakes the unnecessary @sentry code? Because I don't know that your comments are super helpful otherwise.

I'm truly not trying to be (nor want to be) an ass here, but there is so much nuance around ESM and treeshaking that it's important to be precise.

To answer one of your questions: Of course I'm bundling to ESM, it's not possible to having a meaningful conversation otherwise.

And yes you need to have the mainFields set to module, main. Further it also makes some difference whether you have platform: node or not. Additionally you must have minimize: true in esbuild because that is the only way DCE will be performed against the bundled code.

Now all that said, I may very well be missing something (I've been embarrassingly wrong before so I'm sure it'll happen again). If you do have a solution for treeshaking the sentry code base I would be eager to understand where I've gone awry 🙈

[mydea]

FWIW in regard to the tree-shaking part, this PR: https://github.com/getsentry/sentry-javascript/pull/12017 should hopefully fix this for good.

[jd-carroll]

This is resolved, thank you! ❤️