Skip to content

Re-examine usage of sentry-cli and bundler plugins #5461

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

Closed
AbhiPrasad opened this issue Jul 25, 2022 · 8 comments
Closed

Re-examine usage of sentry-cli and bundler plugins #5461

AbhiPrasad opened this issue Jul 25, 2022 · 8 comments

Comments

@AbhiPrasad
Copy link
Member

AbhiPrasad commented Jul 25, 2022

Problem Statement

#4894 (comment) brings up sentry-cli size issues.

Essentially, due to sentry-cli's binary size, we require users to have some kind of bundling setup to make sure that they tree shake away sentry-cli in application code.

Is there a way we can improve this situation?

There's also the concern here with bundlers. Right now we only have a single 1st party plugin, for webpack: https://github.com/getsentry/sentry-webpack-plugin

There is also a community vite plugin:

The more bundlers we support, the better the out of the box experience is (less finagling with sentry-cli).

Solution Brainstorm

Sentry-cli usage

My musings from slack for the sentry-cli size problem:

  1. Produce a minimized version of sentry-cli that pretty much only exposes the API around sourcemaps/releases
  2. Expand the amount of bundler plugins we have, (go from just webpack -> rollup, webpack, vite, esbuild, we know how to do this)
  3. Make sure the framework SDKs include a bundler plugin, and those bundler plugins only use the minimal sentry-cli.
    Another option here is that we can try making adding build time plugins/steps a Step 2 in the onboarding wizard (after yarn add and Sentry.init). Mobile has something similar where you need to explicitly add the gradle plugin for Android for ex.

Note: We'll need to DACI how exactly we make sentry-cli smaller, basically choose between re-writing the needed functionality in JS vs. stripping the CLI rust binary at compile time.

Bundlers

There is https://github.com/unjs/unplugin, which allows us to have a single codebase for bundlers (perhaps in the SDK monorepo 👀). It also uses the rollup plugin API, which @lobsterkatie does have a lot of experience with.

@asonnleitner did some awesome work with https://github.com/asonnleitner/unplugin-sentry as per #4989. We can def use that as a starting point!

@mitsuhiko
Copy link
Contributor

mitsuhiko commented Jul 25, 2022

For the use of sentry-cli in the plugin a lot of stuff can be easily removed at compile time. Even without removing debug logging, color support one can create a binary of under 7MB on macOS by just compiling out unwanted commands, enabling LTO and disabling symbols. This is compared to the 12MB default build. This still pulls in hundreds of dependencies though like libgit2 for some release handling and the like which is probably not always needed or we could shell out.

Moving the core of sentry-cli out is probably reasonable amount of work, would help compile times and potentially throw away the majority of bloat that the javascript use would not need.

@AbhiPrasad AbhiPrasad changed the title Re-examine usage sentry-cli and bundler plugins Re-examine usage of sentry-cli and bundler plugins Jul 25, 2022
@lobsterkatie
Copy link
Member

Related (would only solve some cases, but would give us an easy fix for anything webpack/vercel-y):
vercel/nft#202

@AbhiPrasad
Copy link
Member Author

@lforst

Can someone remind me what we use the cli for in the webpack plugin? Getting release name, creating release, source map upload, what else? Cause just from a JS ecosystem perspective, having a binary at all isn’t very cool.
Can we translate the core features we use from the CLI into JS?

@lobsterkatie

I was thinking the same thing, but there’s more to even just those things than you’d think, and all of those problems have already been solved
if we can just get a cli binary with only the releases and sourcemaps commands, that would make a big difference
I have no idea how compiling/bundling/whatever works in rust, but it feels like that ought to be possible

@AbhiPrasad
image

@AbhiPrasad
Copy link
Member Author

AbhiPrasad commented Jul 25, 2022

Another thing that came up here. Perhaps we propose to the node folks for a new category of deps. buildTimeDeps. Dependencies that act like dependencies when published, but devDependencies when used.

image

@lforst
Copy link
Contributor

lforst commented Jul 25, 2022

Writing some more thoughts here so I don't forget them when we make a decision on this:

  • I think it's perfectly fine for the CLI to be a normal dependency of any plugins. While I like the creativity of buildTimeDeps I also think it would be unnecessary. The ecosystem has come so far without something like that, we're just causing havoc by having a (huge) binary.
  • I think trying to reduce the size of the existing Sentry CLI is the wrong angle of attack: Unnecessary complexity + will grow again when new features are added. As a quick fix, maybe, but it's not a sustainable solution.
  • I already messed around with the unplugin package for a non-trivial amount of time. I think it's yet too early for us to use it for a set of packed plugins like the Sentry webpack plugin is right now. We can use it for source map upload - I think that should work splendidly. But not for release injection - still too many inconsistencies accross bundlers when using unplugin.

@AbhiPrasad
Copy link
Member Author

If we proceed with https://github.com/unjs/unplugin, we'll go in knowing we have to contribute back upstream - maybe we can fix those inconsistencies?

@lobsterkatie
Copy link
Member

  • I think trying to reduce the size of the existing Sentry CLI is the wrong angle of attack: Unnecessary complexity + will grow again when new features are added. As a quick fix, maybe, but it's not a sustainable solution.

If this were done as a proverbial Pick<sentry-cli, 'releases' | 'sourcemaps'> rather than an Omit<sentry-cli, 'all' | 'of' | 'the' | 'other' | 'many' | 'commands'>, I don't see why it would grow significantly over time.

I'm not philosophically against your translate-everything-to-js idea, but it does seem like it'd be more work (and more maintenance) than just wrapping a stripped-down cli tool.

@HazAT
Copy link
Member

HazAT commented Jan 26, 2023

We have unplugin now :)

@HazAT HazAT closed this as completed Jan 26, 2023
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

6 participants