-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[v8] Changes to serverless package #10976
Comments
generally sounds good to me - should we still have a (generic?) serverless package? is there anything that could be in there? What about non-aws/gcp serverless environments, could they/would they so far have used the serverless package? |
Agreed, sounds like a good change! Should the AWS/GCP packages inherit from On that note, are we sure that serverless environments work with Otel? I guess as long as we're in a Node-compatible runtime they should work but not sure how well we investigated this yet. |
If we don't have any "generic" serverless code (that could not also just live in core) then yes, let's delete it! |
yeah that's fine. I was just under the impression that there's no direct replacement for the default |
With CJS they should work pretty fine! But we need to investigate esm (also the serverless packages use monkeypatching I'm gonna move forward with Option 2 - splitting the package up! Created a tasklist to reflect the work. |
In #10540 we explored refactoring the
serverless
package for v8, and we found some more structural issues that needed to be solved. This was further expanded with #10561.This is a brief proposal for the changes I think the serverless package should take.
Current State
Right now
@sentry/serverless
:@sentry/node
(aka@sentry/node-experimental
) under main package exportsinit
,getDefaultIntegrations
,wrapHandler
,tryPatchHandler
and re-exports all of@sentry/node
underAWSLambda
namespaceinit
,getDefaultIntegrations
,wrapCloudEventHandler
,wrapEventFunction
,wrapHttpFunction
under theGCPFunction
namespaceThere is some logic in
awslambda-auto.ts
that is used for the lambda layer.The main export also contains
awsServicesIntegration
export. There is agoogleCloudGrpcIntegration
andgoogleCloudHttpIntegration
that exists in the package, but is not exported 😢.Problem
AWSLambda
andGCPFunction
are not treeshakable, and bundle size really matters for serverless. This is not that bad forGCPFunction
, but givenAWSLambda
re-exports all of the node sdk, that means anyone usingAWSLambda
and bundling pulls in the entire node sdk!init
exported from the same package, each with their own behaviours (node init, aws init, gcp init)Solution
I propose we split
@sentry/serverless
into two packages,@sentry/aws
and@sentry/google-cloud
. This means we only export a singleinit
, and for users the onboarding becomes way easier. In the future we can easily add support for@sentry/azure
or similar! This makes sure we can optimize for that specific serverless package.We have two options:
@sentry/serverless
package (easier upgrade path)@sentry/aws
or@sentry/google-cloud
(lambda layer users will not be impacted in anyway.The biggest change is to change package name, but we aren't breaking any of the top level exports so migration shouldn't be too bad. Lambda functions are also small, so I can't see it being too much work to go through people's codebase and change things.
Addendum
In #10561 I explored using subpath exports as an alternative to new packages. I'm happier with the separate packages (more accurate download counts too!), but we can change the proposal if sentiments have changed.
Work
Tasks
@sentry/google-cloud-serverless
#11065@sentry/aws-serverless
and@sentry/google-cloud-serverless
ESM compat #11066After Release
The text was updated successfully, but these errors were encountered: