Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,16 @@ export class AwsLambdaInstrumentation extends InstrumentationBase<AwsLambdaInstr
if (isWrapped(moduleExports[functionName])) {
this._unwrap(moduleExports, functionName);
}
// try {
this._wrap(
moduleExports,
functionName,
this._getHandler(lambdaStartTime)
);
// } catch (error) {
// // _wrap is not error safe. We do not want the instrumented lambda to fail if wrapping fails.
// diag.error('patching handler function failed', error);
// }
Comment on lines +160 to +169
Copy link
Contributor

@jj22ee jj22ee Apr 25, 2025

Choose a reason for hiding this comment

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

Instead of letting an error get thrown and potentially relying on catching it, one idea is preemptively determining if TypeError: Cannot redefine property: handler will occur, and in that case we just return [] and don't attempt to use InstrumentationNodeModuleDefinition or InstrumentationNodeModuleFile.

I haven't looked into yet, but is it possible to check if the "lambda handler is compiled with esbuild and using an immutable ESM export export const handler = function() {} transpiled to CJS."?

Copy link
Author

Choose a reason for hiding this comment

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

I think so, but trying to guess if the wrap-function would throw an error seems not a good approach to me. If the implementation of shimmer changes, we would need to update this as well. Besides, there could be other reasons for wrapping to fail and in my opinion this should never lead to an error in the instrumented code.

return moduleExports;
},
(moduleExports?: LambdaModule) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/*
* Copyright The OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

// We access through node_modules to allow it to be patched.
/* eslint-disable node/no-extraneous-require */

import * as path from 'path';

import {
AwsLambdaInstrumentation,
AwsLambdaInstrumentationConfig,
} from '../../src';
import {
BatchSpanProcessor,
InMemorySpanExporter,
} from '@opentelemetry/sdk-trace-base';
import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node';
import { Context } from 'aws-lambda';
import * as assert from 'assert';

const memoryExporter = new InMemorySpanExporter();

describe('lambda handler', () => {
let instrumentation: AwsLambdaInstrumentation;

let oldEnv: NodeJS.ProcessEnv;

const ctx = {
functionName: 'my_function',
invokedFunctionArn: 'my_arn',
awsRequestId: 'aws_request_id',
} as Context;

const initializeHandler = (
handler: string,
config: AwsLambdaInstrumentationConfig = {}
) => {
process.env._HANDLER = handler;

const provider = new NodeTracerProvider({
spanProcessors: [new BatchSpanProcessor(memoryExporter)],
});
provider.register();

instrumentation = new AwsLambdaInstrumentation(config);
instrumentation.setTracerProvider(provider);

return provider;
};

const lambdaRequire = (module: string) =>
require(path.resolve(__dirname, '..', module));

beforeEach(() => {
oldEnv = { ...process.env };
process.env.LAMBDA_TASK_ROOT = path.resolve(__dirname, '..');
});

afterEach(() => {
process.env = oldEnv;
instrumentation.disable();

memoryExporter.reset();
});

describe('handler with module.exports syntax', () => {
it('should not fail', async () => {
initializeHandler('lambda-test/instrumentationError.handler');

const result = await new Promise((resolve, reject) => {
lambdaRequire('lambda-test/instrumentationError').handler(
'arg',
ctx,
(err: Error, res: any) => {
if (err) {
reject(err);
} else {
resolve(res);
}
}
);
});
assert.strictEqual(result, 'ok');
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
const obj = {};
Object.defineProperty(obj, 'handler', {
get: () => handler,
enumerable: true,
configurable: false, // ❗️Nicht konfigurierbar = Problem für Shimmer
});

module.exports = obj;

async function handler(event) {
return 'ok';
}