Skip to content

Commit

Permalink
fix(lambda): fix lambda instr to work with a '.' in the handler modul…
Browse files Browse the repository at this point in the history
…e path (#4294)

Before this if the _HANDLER string had a '.' in the module path (the
part before the 'moduleName.functionExport'), then the parsing of
that handler string would silently produce bogus 'lambdaHandlerInfo'
that would result in a RITM path that would never actually get loaded,
hence no Lambda instrumentation.

Fixes: #4293
  • Loading branch information
trentm authored Nov 4, 2024
1 parent d321a91 commit 41add4d
Show file tree
Hide file tree
Showing 6 changed files with 128 additions and 24 deletions.
18 changes: 18 additions & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,24 @@ Notes:
See the <<upgrade-to-v4>> guide.
==== Unreleased
[float]
===== Breaking changes
[float]
===== Features
[float]
===== Bug fixes
* Fix AWS Lambda instrumentation to work with a "handler" string that includes
a period (`.`) in the module path. E.g. the leading `.` in `Handler: ./src/functions/myfunc/handler.main`. ({issues}4293[#4293]).
[float]
===== Chores
[[release-notes-4.8.0]]
==== 4.8.0 - 2024/10/08
Expand Down
2 changes: 1 addition & 1 deletion lib/instrumentation/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ Instrumentation.prototype.clearPatches = function (modules) {

// If in a Lambda environment, find its handler and add a patcher for it.
Instrumentation.prototype._maybeLoadLambdaPatcher = function () {
let lambdaHandlerInfo = getLambdaHandlerInfo(process.env);
let lambdaHandlerInfo = getLambdaHandlerInfo(process.env, this._log);

if (lambdaHandlerInfo && this._patcherReg.has(lambdaHandlerInfo.modName)) {
this._log.warn(
Expand Down
82 changes: 62 additions & 20 deletions lib/lambda.js
Original file line number Diff line number Diff line change
Expand Up @@ -817,18 +817,34 @@ function isLambdaExecutionEnvironment() {
// .mjs file extension (which indicates an ECMAScript/import module, which the
// agent does not support.
//
// @param string taskRoot
// @param string handlerModule
// @return string
function getFilePath(taskRoot, handlerModule) {
let filePath = path.resolve(taskRoot, `${handlerModule}.js`);
if (!fs.existsSync(filePath)) {
filePath = path.resolve(taskRoot, `${handlerModule}.cjs`);
// TODO: support "extensionless"? per https://github.com/aws/aws-lambda-nodejs-runtime-interface-client/blob/v3.2.1/src/UserFunction.js#L149 Is this for a dir/index.js?
// TODO: support ESM and .mjs
//
// @param {string} taskRoot
// @param {string} moduleRoot - The subdir under `taskRoot` holding the module.
// @param {string} module - The module name.
// @return {string | null}
function getFilePath(taskRoot, moduleRoot, module) {
const lambdaStylePath = path.resolve(taskRoot, moduleRoot, module);
if (fs.existsSync(lambdaStylePath + '.js')) {
return lambdaStylePath + '.js';
} else if (fs.existsSync(lambdaStylePath + '.cjs')) {
return lambdaStylePath + '.cjs';
} else {
return null;
}
return filePath;
}

function getLambdaHandlerInfo(env) {
/**
* Gather module and export info for the Lambda "handler" string.
*
* Compare to the Node.js Lambda runtime's equivalent processing here:
* https://github.com/aws/aws-lambda-nodejs-runtime-interface-client/blob/v3.2.1/src/UserFunction.js#L288
*
* @param {object} env - The process environment.
* @param {any} [logger] - Optional logger for trace/warn log output.
*/
function getLambdaHandlerInfo(env, logger) {
if (
!isLambdaExecutionEnvironment() ||
!env._HANDLER ||
Expand All @@ -837,22 +853,48 @@ function getLambdaHandlerInfo(env) {
return null;
}

// extract module name and "path" from handler using the same regex as the runtime
// from https://github.com/aws/aws-lambda-nodejs-runtime-interface-client/blob/c31c41ffe5f2f03ae9e8589b96f3b005e2bb8a4a/src/utils/UserFunction.ts#L21
const functionExpression = /^([^.]*)\.(.*)$/;
const match = env._HANDLER.match(functionExpression);
// Dev Note: This intentionally uses some of the same var names at
// https://github.com/aws/aws-lambda-nodejs-runtime-interface-client/blob/v3.2.1/src/UserFunction.js#L288
const fullHandlerString = env._HANDLER;
const moduleAndHandler = path.basename(fullHandlerString);
const moduleRoot = fullHandlerString.substring(
0,
fullHandlerString.indexOf(moduleAndHandler),
);
const FUNCTION_EXPR = /^([^.]*)\.(.*)$/;
const match = moduleAndHandler.match(FUNCTION_EXPR);
if (!match || match.length !== 3) {
if (logger) {
logger.warn(
{ fullHandlerString, moduleAndHandler },
'Lambda handler string did not match FUNCTION_EXPR',
);
}
return null;
}
const module = match[1];
const handlerPath = match[2];

const moduleAbsPath = getFilePath(env.LAMBDA_TASK_ROOT, moduleRoot, module);
if (!moduleAbsPath) {
if (logger) {
logger.warn(
{ fullHandlerString, moduleRoot, module },
'could not find Lambda handler module file (ESM not yet supported)',
);
}
return null;
}
const handlerModule = match[1].split('/').pop();
const handlerFunctionPath = match[2];
const handlerFilePath = getFilePath(env.LAMBDA_TASK_ROOT, match[1]);

return {
filePath: handlerFilePath,
modName: handlerModule,
propPath: handlerFunctionPath,
const lambdaHandlerInfo = {
filePath: moduleAbsPath,
modName: module,
propPath: handlerPath,
};
if (logger) {
logger.trace({ fullHandlerString, lambdaHandlerInfo }, 'lambdaHandlerInfo');
}
return lambdaHandlerInfo;
}

function lowerCaseObjectKeys(obj) {
Expand Down
13 changes: 13 additions & 0 deletions test/lambda/fixtures/foo.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright Elasticsearch B.V. and other contributors where applicable.
* Licensed under the BSD 2-Clause License; you may not use this file except in
* compliance with the BSD 2-Clause License.
*/

'use strict';

module.exports = {
bar: function (event, context) {
return 'fake handler';
},
};
13 changes: 13 additions & 0 deletions test/lambda/fixtures/handlermodule.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright Elasticsearch B.V. and other contributors where applicable.
* Licensed under the BSD 2-Clause License; you may not use this file except in
* compliance with the BSD 2-Clause License.
*/

module.exports = {
lambda: {
foo: function myHandler(event, context) {
return 'hi';
},
},
};
24 changes: 21 additions & 3 deletions test/lambda/wrapper.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,24 @@ tape.test('getLambdaHandlerInfo', function (suite) {
t.end();
});

suite.test('extracts info with leading "./" on _HANDLER path', function (t) {
process.env.AWS_LAMBDA_FUNCTION_NAME = 'foo';

const info = getLambdaHandlerInfo({
_HANDLER: './lambda.bar',
LAMBDA_TASK_ROOT: path.resolve(__dirname, 'fixtures'),
});

t.equals(
info.filePath,
path.resolve(__dirname, 'fixtures', 'lambda.js'),
'extracted handler file path',
);
t.equals(info.modName, 'lambda', 'extracted handler module');
t.equals(info.propPath, 'bar', 'extracted handler propPath');
t.end();
});

suite.test('extracts info with extended path, cjs extension', function (t) {
process.env.AWS_LAMBDA_FUNCTION_NAME = 'foo';

Expand Down Expand Up @@ -93,7 +111,7 @@ tape.test('getLambdaHandlerInfo', function (suite) {
suite.test('malformed handler: too few', function (t) {
process.env.AWS_LAMBDA_FUNCTION_NAME = 'foo';
const handler = getLambdaHandlerInfo({
LAMBDA_TASK_ROOT: '/var/task',
LAMBDA_TASK_ROOT: path.resolve(__dirname, 'fixtures'),
_HANDLER: 'foo',
});

Expand All @@ -104,13 +122,13 @@ tape.test('getLambdaHandlerInfo', function (suite) {
suite.test('longer handler', function (t) {
process.env.AWS_LAMBDA_FUNCTION_NAME = 'foo';
const handler = getLambdaHandlerInfo({
LAMBDA_TASK_ROOT: '/var/task',
LAMBDA_TASK_ROOT: path.resolve(__dirname, 'fixtures'),
_HANDLER: 'foo.baz.bar',
});

t.equals(
handler.filePath,
path.resolve('/var', 'task', 'foo.cjs'),
path.resolve(__dirname, 'fixtures', 'foo.js'),
'extracted handler file path',
);
t.equals(handler.modName, 'foo', 'extracted handler module name');
Expand Down

0 comments on commit 41add4d

Please sign in to comment.