-
Couldn't load subscription status.
- Fork 615
feat(instrumentation-aws-lambda): take care of ESM based (.mjs) handlers
#2508
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
feat(instrumentation-aws-lambda): take care of ESM based (.mjs) handlers
#2508
Conversation
fea7f36 to
118eb8e
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2508 +/- ##
==========================================
- Coverage 90.79% 90.77% -0.02%
==========================================
Files 169 169
Lines 8009 8015 +6
Branches 1632 1632
==========================================
+ Hits 7272 7276 +4
- Misses 737 739 +2
|
.mjs) handlers for the AWS Lambda instrumentation.mjs) handlers
|
Looks like a subset of #2000, thanks for addressing the issue in the instrumentation code wrt. absolute paths ! |
118eb8e to
1025833
Compare
plugins/node/opentelemetry-instrumentation-aws-lambda/src/instrumentation.ts
Outdated
Show resolved
Hide resolved
plugins/node/opentelemetry-instrumentation-aws-lambda/src/instrumentation.ts
Outdated
Show resolved
Hide resolved
plugins/node/opentelemetry-instrumentation-aws-lambda/src/instrumentation.ts
Outdated
Show resolved
Hide resolved
|
@pichlermarc I haven't merged this yet, in case OP would like to respond to my comments above. I think it is good to merge if you are doing a release that could include this tomorrow. |
|
@raphael-theriault-swi Ah, sorry I hadn't seen your earlier PR ... and I hadn't really been digging into Lambda-related PRs. I'll try to take a look in the next few days. |
|
@trentm I'd just go with whatever is easier to review and get out of the doors, ultimately the important part is ESM support |
|
@trentm Thanks for the review. I have updated PR according to your comments |
Which problem is this PR solving?
Currently, ESM based handlers with
.mjsfile extension are not taken care of by AWS Lambda instrumentation, but ESM based handlers are supported since long time (https://aws.amazon.com/tr/about-aws/whats-new/2022/01/aws-lambda-es-modules-top-level-await-node-js-14/).Short description of the changes
For the fix, while handler file is being resolved, in addition to
.jsand.cjsfile extensions,.mjsfile extension is also checked.This PR is the contrib counter part of the open-telemetry/opentelemetry-js#5094