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

Improve sourcemap upload duration for multi-stage builds #626

Open
lforst opened this issue Oct 30, 2024 · 8 comments
Open

Improve sourcemap upload duration for multi-stage builds #626

lforst opened this issue Oct 30, 2024 · 8 comments

Comments

@lforst
Copy link
Member

lforst commented Oct 30, 2024

I'll limit this issue to Next.js because it is a super-set of everything else.

In high-level frameworks there are often multiple build-passes the framework does. This can cause the Sentry plugin to pick up and upload the same files multiple times, potentially leading to unnecessarily long uploads.

Brainstorming a solution:

  • Allow to pass in a build-id into the plugin that will associate multiple bundler runs with the same build, allowing us to store and deduplicate files to avoid double uploading.
  • Telemetry-wise, it would probably also good to share a trace id between these runs.

Related getsentry/sentry-javascript#14132

@joaocanaverde-blue
Copy link

@lforst Very interested in this 🙂

We have an SST repo that takes around 40 minutes to build when Sentry's ESBuild plugin is enabled. It more than doubles our build time (which wasn't fast to begin with).

The benefits of having the source code available in Sentry issues still outweigh the added build time, but it is properly painful.

@andreiborza
Copy link
Member

@joaocanaverde-blue thanks for chiming in, definitely appreciate the feedback and hearing of actual pain points. We have this in our backlog and hopefully get to it soon.

@Lms24
Copy link
Member

Lms24 commented Dec 17, 2024

One thing you could do today to limit the impact is to remove the plugin in favour of using Sentry CLI directly. You could inject debugIds and upload source maps once at the end of all your builds and point the CLI to all files. Just as an alternative suggestion if you don't want to wait for the change in the plugin. This comes with the disadvantage that release wouldn't be injected and you'd manually need to set it in Sentry.init calls.

@joaocanaverde-blue
Copy link

Thanks @andreiborza and @Lms24!

I had previously tried to use this approach (which uses Sentry's CLI) without much success. I then tried Sentry's ESBuild plugin. That worked without much fuss, so I left it despite the slowness.

@Lms24 really appreciate your suggestion - it gave me the push to give the CLI approach another go. Really thought it would work this time, but unfortunately the only way I can see the source code in Sentry is still by using the ESBuild plugin.

This is very likely to just be an SST quirk, but for the record this is what I'm doing when using the CLI approach:

  • I pass --enable-source-maps when running sst deploy
  • I use SST's afterBuild hook to copy the .map & .mjs files to a separate folder .build/sourcemaps. (Also experimented with copying the source .ts files.)
  • I've got NODE_OPTIONS: '--enable-source-maps' in each function's environment.
  • I've got sourcemap: true in SST's NodeJSProps for each function.
  • And I run this after deploying: sentry-cli sourcemaps inject --org my-org--project my-project .build/sourcemaps && sentry-cli sourcemaps upload --org my-org --project my-project .build/sourcemaps

I've also experimented with creating a release instead of injecting debug ids. That got me as far as the .mjs file showing up on Sentry, but not the actual TypeScript source code that appears when I use the ESBuild plugin.

I spent most of yesterday on this so I think I'm going to revert to the ESBuild plugin again and live with slow builds for now.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Dec 18, 2024
@Lms24
Copy link
Member

Lms24 commented Dec 18, 2024

I use SST's afterBuild hook to copy the .map & .mjs files to a separate folder

Just to confirm: This means that the .map file that corresponds to the .mjs file are still in the same directory? When you run the sentry-cli inject command, it needs to find both the script file as well as the corresponding source map. Did you verify that debug ids were injected correctly?

@joaocanaverde-blue
Copy link

@Lms24 yup, I can see sentryDebugIds in the .mjs and .map files when I search for it. They're in the same directory.

The only thing missing at that point is the code mapping to the original .ts file.

I've also tried to include the .ts file in the same directory with no luck.

And if I try to click the "Set up Code Mapping" button on Sentry and paste the relevant GitHub link, Sentry tells me it's not the same file (presumably because it's expecting an .mjs file.)

Guessing the ESBuild plugin must be doing something extra that I'm not.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Dec 18, 2024
@Lms24
Copy link
Member

Lms24 commented Dec 18, 2024

The only thing missing at that point is the code mapping to the original .ts file.

Oh does this mean that the source map itself is maybe incorrect? You could try inspecting the source map with for example this tool.

@joaocanaverde-blue
Copy link

The only thing missing at that point is the code mapping to the original .ts file.

Oh does this mean that the source map itself is maybe incorrect? You could try inspecting the source map with for example this tool.

@Lms24 Possibly?

Troubleshooting the CLI approach has proved to not be trivial.

At this stage I think it's better for me to hold off until this ESBuild issue gets resolved. It does at least work, as long as I'm patient 🙂

Thanks for all your help so far!

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

No branches or pull requests

4 participants