- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 1.7k
feat(aws): Add support for automatic wrapping in ESM #17407
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
Conversation
| responseHook(_span, { err }) { | ||
| if (err) { | ||
| captureException(err, scope => markEventUnhandled(scope, 'auto.function.aws-serverless.otel')); | ||
| } | ||
| }, | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
capturing errors here ensures that they're connected to the span correctly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved init code to separate file to avoid circular dependency
| size-limit report 📦
 | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good stuff! Excited about this.
| const PACKAGE_VERSION = '0.54.0'; | ||
| const PACKAGE_NAME = '@opentelemetry/instrumentation-aws-lambda'; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
l/m: Since this is now our own instrumentation, let's use @sentry/instrumenation-aws-lambda and for version we can import und use SDK_VERSION from @sentry/core, but let's leave a comment that at the time of vendoring we used 0.54.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not strictly necessary but probably makes it easier to understand/debug.
        
          
                packages/aws-serverless/src/integration/instrumentation-aws-lambda/instrumentation.ts
          
            Show resolved
            Hide resolved
        
      | } | ||
|  | ||
| /** */ | ||
| export function tryPatchHandler(taskRoot: string, handlerPath: string): void { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mhm this is technically a breaking change 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep it around but no-op + deprecation warning.
ba931a7    to
    3ee810f      
    Compare
  
    3ee810f    to
    dee487a      
    Compare
  
    
This allows code-less setup for Lambda functions running in ESM (and thus the aws-serverless SDK in general) by vendoring the OpenTelemetry AwsLambda instrumentation and wrapping the patched handler with Sentry's
wrapHandler.