-
Notifications
You must be signed in to change notification settings - Fork 4
Esm import support #540
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
base: main
Are you sure you want to change the base?
Esm import support #540
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| import * as module from 'module'; | ||
| import * as path from 'path'; | ||
| import * as fs from 'fs'; | ||
|
|
||
| function _hasFolderPackageJsonTypeModule(folder) { | ||
| if (folder.endsWith('/node_modules')) { | ||
| return false; | ||
| } | ||
| const pj = path.join(folder, '/package.json'); | ||
| if (fs.existsSync(pj)) { | ||
| try { | ||
| const pkg = JSON.parse(fs.readFileSync(pj).toString()); | ||
| if (pkg) { | ||
| if (pkg.type === 'module') { | ||
| return true; | ||
| } else { | ||
| return false; | ||
| } | ||
| } | ||
| } catch (e) { | ||
| console.warn(`${pj} cannot be read, it will be ignored for ES module detection purposes.`, e); | ||
| return false; | ||
| } | ||
| } | ||
| if (folder === '/') { | ||
| return false; | ||
| } | ||
| return _hasFolderPackageJsonTypeModule(path.resolve(folder, '..')); | ||
| } | ||
|
|
||
| function _hasPackageJsonTypeModule(file) { | ||
| const jsPath = file + '.js'; | ||
| if (fs.existsSync(jsPath)) { | ||
| return _hasFolderPackageJsonTypeModule(path.resolve(path.dirname(jsPath))); | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| function _resolveHandlerFileName() { | ||
| const taskRoot = process.env.LAMBDA_TASK_ROOT; | ||
| const handlerDef = process.env._HANDLER; | ||
| if (!taskRoot || !handlerDef) { | ||
| return null; | ||
| } | ||
| const handler = path.basename(handlerDef); | ||
| const moduleRoot = handlerDef.substr(0, handlerDef.length - handler.length); | ||
| const [module] = handler.split('.', 2); | ||
| return path.resolve(taskRoot, moduleRoot, module); | ||
| } | ||
|
|
||
| function _isHandlerAnESModule() { | ||
| try { | ||
| const handlerFileName = _resolveHandlerFileName(); | ||
| if (!handlerFileName) { | ||
| return false; | ||
| } | ||
| if (fs.existsSync(handlerFileName + '.mjs')) { | ||
| return true; | ||
| } else if (fs.existsSync(handlerFileName + '.cjs')) { | ||
| return false; | ||
| } else { | ||
| return _hasPackageJsonTypeModule(handlerFileName); | ||
| } | ||
| } catch (e) { | ||
| console.error('Unknown error occurred while checking whether handler is an ES module', e); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| function _isSupportedNodeForEsmLoaderHook() { | ||
| try { | ||
| const nodeVersion = process.versions && process.versions.node; | ||
| if (!nodeVersion) { | ||
| return false; | ||
| } | ||
| const [majorStr, minorStr] = nodeVersion.split('.', 3); | ||
| const major = Number.parseInt(majorStr, 10); | ||
| const minor = Number.parseInt(minorStr, 10); | ||
| if (!Number.isFinite(major) || !Number.isFinite(minor)) { | ||
| return false; | ||
| } | ||
| return major > 20 || (major === 20 && minor >= 6); | ||
| } catch { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| let registered = false; | ||
|
|
||
| export function registerLoader() { | ||
| if (!registered) { | ||
| if (_isSupportedNodeForEsmLoaderHook() && typeof module.register === 'function') { | ||
| module.register('import-in-the-middle/hook.mjs', import.meta.url); | ||
| } | ||
| registered = true; | ||
| } | ||
| } | ||
|
|
||
| if (_isHandlerAnESModule()) { | ||
| registerLoader(); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,17 @@ | ||
| #!/bin/bash | ||
| source /opt/splunk-default-config | ||
|
|
||
| export NODE_OPTIONS="${NODE_OPTIONS} --require /opt/wrapper.js" | ||
| node_version="$(node -p "process.versions.node" 2>/dev/null || echo "")" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't really correct, as the actual node executable to run is contained somewhere in
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will mean we will need to drop support for versions < 20. PS: AWS will still allow Lambda functions to be updated for nodejs version 18 till Sep 30, 2026.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Node.js 18 reached end of life last year. According to OTel's policy only Active or Maintenance LTS versions are supported. It would be completely fine to drop support for Node < 20 now (we just have to document it).
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I think that's the conclusion we're reaching. This will still need to wait for a regular update release before we merge it though, and that's gated on open-telemetry/opentelemetry-lambda#2122 (comment) at the moment. |
||
| node_major="${node_version%%.*}" | ||
| node_minor="${node_version#*.}" | ||
| node_minor="${node_minor%%.*}" | ||
|
|
||
| if [[ "$node_major" =~ ^[0-9]+$ ]] && [[ "$node_minor" =~ ^[0-9]+$ ]] && { [ "$node_major" -gt 20 ] || { [ "$node_major" -eq 20 ] && [ "$node_minor" -ge 6 ]; }; } | ||
| then | ||
| export NODE_OPTIONS="${NODE_OPTIONS} --import /opt/loader.mjs --require /opt/wrapper.js" | ||
| else | ||
| export NODE_OPTIONS="${NODE_OPTIONS} --require /opt/wrapper.js" | ||
| fi | ||
|
|
||
| if [[ $OTEL_RESOURCE_ATTRIBUTES != *"service.name="* ]]; then | ||
| export OTEL_RESOURCE_ATTRIBUTES="service.name=${AWS_LAMBDA_FUNCTION_NAME},${OTEL_RESOURCE_ATTRIBUTES}" | ||
|
|
||
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.
Are there any changes in this from upstream? If not, then we should copy this into our build by simply pulling it from the git submodule. If there are, then there should be a comment stating where it came from and what the tweaks are so that future maintainers can have a chance to reconcile diffs.
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.
Agree, they seem identical, so the submodule would work great. At the very least we should provide attribution where it's coming from.