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

Patch aws-lambda instrumentation to support ESM #101

Merged
merged 1 commit into from
Oct 11, 2024

Conversation

mxiamxia
Copy link
Member

@mxiamxia mxiamxia commented Oct 10, 2024

Description of changes:

  1. Add IS_ESM check in otel_instrument wrapper before calling lambda handler. If it is EMS format, add ESM node-options supported by OTel community. Added a new wrapper script otel_instrument_esm if the esm auto detection logic is failed, customer can opt-in to tell tell layer go with ESM instrumentation path.
  2. Set a new env var HANDLER_IS_ESM for lambda function when ESM is detected
  3. Patch aws-lambda instrumentation, when IS_ESM env var is set, apply ESM supported InstrumentationNodeModuleDefinition to patch function handler, otherwise keep using the existing handler patcher.

Note: this change add a new branch for supporting ESM, the existing CommonJS path should not be impacted.

  1. Can not use ESM support in lambda environment  open-telemetry/opentelemetry-js#4842
  2. coordinate first pass at ESM support for all instrumentations open-telemetry/opentelemetry-js-contrib#1942

Test


2024-10-09T20:38:13.411-07:00 | Instrumenting lambda handler {
-- | --
  | 2024-10-09T20:38:13.411-07:00 | taskRoot: '/var/task',
  | 2024-10-09T20:38:13.411-07:00 | handlerDef: 'index.handler',
  | 2024-10-09T20:38:13.411-07:00 | handler: 'index.handler',
  | 2024-10-09T20:38:13.411-07:00 | moduleRoot: '',
  | 2024-10-09T20:38:13.411-07:00 | module: 'index',
  | 2024-10-09T20:38:13.411-07:00 | filename: '/var/task/index.mjs',
  | 2024-10-09T20:38:13.411-07:00 | functionName: 'handler'
  | 2024-10-09T20:38:13.411-07:00 | }


  | 2024-10-09T20:38:15.386-07:00 | 'cloud.account.id': '252610625673',
-- | -- | --
  | 2024-10-09T20:38:15.386-07:00 | 'aws.is.local.root': true,
  | 2024-10-09T20:38:15.386-07:00 | 'aws.local.service': 'TestESM',
  | 2024-10-09T20:38:15.386-07:00 | 'aws.local.operation': 'TestESM/Handler',
  | 2024-10-09T20:38:15.386-07:00 | 'aws.span.kind': 'LOCAL_ROOT'


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@mxiamxia mxiamxia force-pushed the lambda-esm branch 3 times, most recently from adb8ed0 to faaee6b Compare October 10, 2024 04:26
@mxiamxia mxiamxia marked this pull request as ready for review October 10, 2024 21:20
@mxiamxia mxiamxia requested a review from a team as a code owner October 10, 2024 21:20
commit d25c3c3
Author: Min Xia <xiami@amazon.com>
Date:   Tue Oct 8 15:45:36 2024 -0700

    fix the lint error
Copy link
Contributor

@jj22ee jj22ee left a comment

Choose a reason for hiding this comment

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

TY! 🚢
Great to have some ESM support for OTel in Lambda!

@jj22ee jj22ee merged commit 2df6d5d into aws-observability:main Oct 11, 2024
9 checks passed
mxiamxia added a commit that referenced this pull request Oct 11, 2024
*Issue #, if available:*

*Description of changes:*
Follow changes on
#101.

Instead of using a `otel-instrument-esm` wrapper which is redudent, we
will have customer to use `HANDLER_IS_ESM` env var to enforce ESM
instrumentation for the failed to detected cases.

By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants