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

feat(instrumentation-aws-lambda): support ESM handlers and other less common handler patterns #2000

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

raphael-theriault-swi
Copy link
Contributor

@raphael-theriault-swi raphael-theriault-swi commented Mar 6, 2024

Which problem is this PR solving?

The Lambda instrumentation doesn't currently support instrumenting ESM handlers, which are now the default, and other less common handler patterns supported by Lambda such as extensionless handler modules and nested handler functions.

Short description of the changes

The previous logic to determine the handler to patch is replaced with an implementation based on the one present in the Lambda runtime. I tried to keep the function names and structure as close to possible to the original to make it easier to cross-reference.

The tests also now use code copied from the Lambda runtime to load handlers, as opposed to just calling require which doesn't properly reflect a real-world scenario and doesn't work at all for ESM.

I also updated the notes about absolute path support in the patching infrastructure. The instrumentation keeps using the same hack but it would probably be a good idea to fix the issue upstream, which is that while absolute paths are taken into account while registering hooks, they are not when matching against the module name within the hook function, which ignores the baseDir.

Copy link

codecov bot commented Mar 6, 2024

Codecov Report

Attention: Patch coverage is 97.50000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 90.35%. Comparing base (dfb2dff) to head (6452504).
Report is 171 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2000      +/-   ##
==========================================
- Coverage   90.97%   90.35%   -0.62%     
==========================================
  Files         146      148       +2     
  Lines        7492     7323     -169     
  Branches     1502     1528      +26     
==========================================
- Hits         6816     6617     -199     
- Misses        676      706      +30     
Files Coverage Δ
...-instrumentation-aws-lambda/src/instrumentation.ts 93.61% <97.22%> (-0.07%) ⬇️
...ry-instrumentation-aws-lambda/src/user-function.ts 97.72% <97.72%> (ø)

... and 58 files with indirect coverage changes

@raphael-theriault-swi

This comment was marked as outdated.

@raphael-theriault-swi raphael-theriault-swi changed the title feat(instrumentation-aws-lambda): support .mjs handlers feat(instrumentation-aws-lambda): support ESM handlers and other less common handler patterns Jun 21, 2024
@raphael-theriault-swi
Copy link
Contributor Author

@JamieDanielson I think you've been pushing a lot for ESM support as a whole, if I rebased this do you think you'd be able to take a look ?

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.

1 participant