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 JavaScript SDK Version 7.0 #4876

Merged
merged 208 commits into from
May 30, 2022
Merged

Sentry JavaScript SDK Version 7.0 #4876

merged 208 commits into from
May 30, 2022

Conversation

AbhiPrasad
Copy link
Member

@AbhiPrasad AbhiPrasad commented Apr 7, 2022

@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2022

size-limit report 📦

Path Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified) 19.36 KB (-3.88% 🔽)
@sentry/browser - ES5 CDN Bundle (minified) 60.05 KB (-7.07% 🔽)
@sentry/browser - ES6 CDN Bundle (gzipped + minified) 18.21 KB (-3.47% 🔽)
@sentry/browser - ES6 CDN Bundle (minified) 53.66 KB (-7.44% 🔽)
@sentry/browser - Webpack (gzipped + minified) 19.92 KB (-14.26% 🔽)
@sentry/browser - Webpack (minified) 63.12 KB (-22.76% 🔽)
@sentry/react - Webpack (gzipped + minified) 19.95 KB (-14.31% 🔽)
@sentry/nextjs Client - Webpack (gzipped + minified) 43.74 KB (-8.99% 🔽)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) 25.38 KB (-2.67% 🔽)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified) 23.91 KB (-2.33% 🔽)

@vladanpaunovic
Copy link
Contributor

@AbhiPrasad, this should close #2707 and other similar issues.

@Lms24 Lms24 force-pushed the 7.x branch 2 times, most recently from 25aa095 to 20c7773 Compare April 7, 2022 11:32
@lobsterkatie lobsterkatie changed the title [v7] Sentry JavaScript SDK Version 7.0 Sentry JavaScript SDK Version 7.0 Apr 7, 2022
@lobsterkatie
Copy link
Member

Heads up @timfish @onurtemizkan - we rebased the 7.x branch to bring it up to the tip-tip of master, so please pull it and rebase your existing branches. Thanks!

@Lms24
Copy link
Member

Lms24 commented Apr 26, 2022

@timfish @onurtemizkan - we just rebased the 7.x branch to include the changes from the 6.19.7 patch. Please pull it and rebase your branches. Thank you!

AbhiPrasad and others added 17 commits April 26, 2022 12:03
Removes all references to `@sentry/apm`, a deprecated package we do not use anymore.
Removes the deprecated `API` class.
Remove deprecated methods `startSpan` and `child`.

These deprecated methods were removed in favour of `span.startChild`.
Removes `getActiveDomain` function and corresponding type.
The `user` dsn component was renamed to `publicKey`. This PR removes that from the dsn field.
Increases the creation timeout of `MongoMemoryServer` to temporarily fix Node integration test flakiness
This PR removes the previously deprecated `frameContextLines` from `NodeOptions`.
These exports were historically used in `@sentry/electron`, but are no longer being used by the Electron SDK or the React Native SDK, so they can be removed.
This updates the SDK to use Typescript 3.8.3, in order to be able to use type-only imports and exports[1]. (These are needed for us to turn on `isolatedModules`, which is in turn is needed for us to switch our build tool away from `tsc`[2], since no other tool understands the relationship between files.)

As a result of this change, a few of the browser integration tests needed to be fixed so that all promises were explicitly awaited, a point about which the linter in 3.8 complains.

This is a breaking change for anyone using TS 3.7.x (there's no one using TS < 3.7.x, since that's our current minimum). That said, though there are plenty of public projects on GH using TS 3.7 and `@sentry/xyz`, if you restrict it to projects using TS 3.7 and `@sentry/xyz` 6.x, all you get are forks of this very repo. Granted, this isn't every project ever, but it's likely decently representative of the fact that if you've upgraded our SDK, you've almost certainly upgraded TS as well. We're going to wait until v7 to release this change in any case, but that's an indication that it won't affect many people when we do. (See this commit's PR for links to searches demonstrating the points.)

Note: Originally there was some thought of going farther, into TS 4.x, but we decided that for the foreseeable future, we're going to stick with 3.8.3. Though moving up to 4.x doesn't seem like it would affect many customers (repeating the same TS/sentry 6.x crossover search with TS 3.8 and 3.9 reveals a total of 5 projects), it does have the known side effect of replacing export statements which, after compilation, currently look like 

    `exports.SdkInfo = types_1.SdkInfo;` 

with ones that look like 

    `Object.defineProperty(exports, "SdkInfo", { enumerable: true, get: function () { return types_1.SdkInfo; } });`. 

(For those of you following along at home, that's a 3x character increase.) Though we might be able to engineer around this in order to avoid the inevitable substantial bundle size hit, attempting to do so is only worth it if there are features from 4.x that we desperately need. Given that we've agreed that right now there aren't, we'll stick with 3.8.3.

[1] https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-8.html#type-only-imports-and-export
[2] https://www.typescriptlang.org/tsconfig#isolatedModules
antonpirker and others added 19 commits May 23, 2022 16:12
…production ready (#5148)

Deactivated deployment of Lambda Layer until new Lambda Extension is production ready.
With this we make sure, that we do not break existing Lambda Layer users.
Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
Currently, when looking at runs of our `Build & Test` GHA workflow, it's often hard to tell exactly what commit is being used. When the workflow is triggered by a push to a PR, the PR title ends up as the label, which can be pretty uninformative. Runs triggered manually are no better, as they're merely labeled "Build & Test." Runs triggered by pushes _are_ better, at least from the Actions tab, because they're labeled for their commit, but once you open one up, it's not obvious from the logs which commit is being used, since "Check out current commit (<some SHA here>)" doesn't actually list the branch's `HEAD` commit but rather a made-up merge commit between `HEAD` and the base branch, making it useless for identifying `HEAD`.

(See the PR description for screenshots of these different cases.)

This fixes that problem, by adding a preliminary GHA job to gather and store the SHA of the actual branch head (along with its commit message). Subsequent jobs can then use that information to label their checkout step, making it much easier to tell where in the commit history the job falls.
remove an unnecessary nested call to dropUndefinedKeys which we don't need because the function is already recursive.
This patch fixes a bug where attachments were previously not added to the envelope that would be sent to Sentry. Additionally, it adds a test that ensures that the attachments actually make it into the raw envelope.
Our current system for building the AWS lambda layer involves a script which manually traces the serverless package's dependencies, including other monorepo packages, and symlinks them into a `node_modules` folder in a directory set up to mimic an installation from npm. There are a few disadvantages to this system:
- The signals we rely on to figure out what is and isn't a dependency aren't exhaustive, and we're not experts at this, both of which have made getting the right files and only the right files into the layer a brittle process, which has broken down more than once and caused issue for our users.
- The script is complicated, which makes it harder to figure out exactly what's causing it when something does go wrong.
- We symlink in entire packages at a time, regardless of whether or not we're using most of the code. This is true even of the serverless package itself, where we include all of the GCP code along with the AWS code.

This refactors our lambda layer build process to use the new bundling config functions we use for CDN bundles, to create a bundle which can take the place of the the index file, but which contains everything it needs internally. This has the following advantages:

- This puts Rollup in charge of figuring out dependencies instead of us. 
- The config is in line with existing configs and is therefore much easier to reason about and maintain.
- It lets us easily exclude anything GCP-related in the SDK. Between that, Rollup's treeshaking, and terser's minifying, the layer will now take up much less of the finite size allotted to each lambda function.
- Removing extraneous files means less to cache and retrieve from the cache in each GHA job.

Key changes:

- The layer now builds in `packages/serverless/build/aws` rather than at the top level of the repo. (It is now moved to the top level only in our GHA workflow creating the lambda zip.)
- In that workflow, the process to determine the SDK version has been simplified.
- The bundle builds based not on the main index file but on a new bundle-specific index file, which only includes AWS code.
- There is new rollup config just for the layer, which uses the bundle-building functions to make the main bundle and the npm-package-building functions to create a separate module for the optional script which automatically starts up the SDK in AWS.
- The old build script has been replaced by one which runs rollup with that config, and then symlinks the built auto-startup script into its legacy location, so as to be backwards compatible with folks who run it using an environment variable pointing to the old path.
- The building of the layer has temporarily been shifted from `build:awslambda-layer` to `build:bundle`, so that it will run during the first phase of the repo-level `yarn build`. (The goal is to eventually do everything in one phase, for greater parallelization and shorter overall build time.)

h/t to @antonpirker for all his help testing and thinking through this with me.
Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
@AbhiPrasad AbhiPrasad marked this pull request as ready for review May 30, 2022 11:06
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

Copy link
Contributor

@vladanpaunovic vladanpaunovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job team! 🚀 LGTM.

@AbhiPrasad AbhiPrasad merged commit ca31aa3 into master May 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Announcement: We're on the v7 Road! Sentry JavaScript v7 Major Release Plan