fix: use absolute path require

open-telemetry · Apr 6, 2021 · d66cc93 · d66cc93
1 parent e892c90
commit d66cc93
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 31 deletions.
diff --git a/plugins/node/opentelemetry-instrumentation-awslambda/package.json b/plugins/node/opentelemetry-instrumentation-awslambda/package.json
@@ -14,7 +14,6 @@
     "codecov": "nyc report --reporter=json && codecov -f coverage/*.json -p ../../",
     "precompile": "tsc --version",
     "prepare": "npm run compile",
-    "postinstall": "copyfiles -f 'test/lambdatest/*' node_modules/lambdatest",
     "version:update": "node ../../../scripts/version-update.js",
     "compile": "npm run version:update && tsc -p ."
   },

diff --git a/plugins/node/opentelemetry-instrumentation-awslambda/src/awslambda.ts b/plugins/node/opentelemetry-instrumentation-awslambda/src/awslambda.ts
@@ -14,47 +14,72 @@
  * limitations under the License.
  */
 
+import * as path from 'path';
+
 import {
   InstrumentationBase,
   InstrumentationNodeModuleDefinition,
+  InstrumentationNodeModuleFile,
   isWrapped,
   safeExecuteInTheMiddle,
 } from '@opentelemetry/instrumentation';
-import { VERSION } from './version';
-import { Callback, Context, Handler } from 'aws-lambda';
 import { diag, Span, SpanKind, SpanStatusCode } from '@opentelemetry/api';
 import { FaasAttribute } from '@opentelemetry/semantic-conventions';
+
+import { Callback, Context, Handler } from 'aws-lambda';
+
 import { LambdaModule } from './types';
+import { VERSION } from './version';
 
 export class AwsLambdaInstrumentation extends InstrumentationBase {
   constructor() {
     super('@opentelemetry/instrumentation-awslambda', VERSION);
   }
 
   init() {
-    // _HANDLER is always defined in Lambda.
-    const handler = process.env._HANDLER!;
-    const dotIndex = handler.lastIndexOf('.');
-    const module = handler.substring(0, dotIndex);
-    const functionName = handler.substring(dotIndex + 1);
+    // _HANDLER and LAMBDA_TASK_ROOT are always defined in Lambda.
+    const taskRoot = process.env.LAMBDA_TASK_ROOT!;
+    const handlerDef = process.env._HANDLER!;
+
+    const handler = path.basename(handlerDef);
+    const moduleRoot = handlerDef.substr(0, handlerDef.length - handler.length);
+
+    const [module, functionName] = handler.split('.', 2);
+
+    // Lambda loads user function using an absolute path.
+    let filename = path.resolve(taskRoot, moduleRoot, module);
+    if (!filename.endsWith('.js')) {
+      // Patching infrastructure currently requires a filename when requiring with an absolute path.
+      filename += '.js';
+    }
 
     return [
       new InstrumentationNodeModuleDefinition(
-        module,
+        // NB: The patching infrastructure seems to match names backwards, this must be the filename, while
+        // InstrumentationNodeModuleFile must be the module name.
+        filename,
         ['*'],
-        (moduleExports: LambdaModule) => {
-          diag.debug('Applying patch for lambdatest handler');
-          if (isWrapped(moduleExports[functionName])) {
-            this._unwrap(moduleExports, functionName);
-          }
-          this._wrap(moduleExports, functionName, this._getHandler());
-          return moduleExports;
-        },
-        (moduleExports: LambdaModule) => {
-          if (moduleExports == undefined) return;
-          diag.debug('Removing patch for lambdatest handler');
-          this._unwrap(moduleExports, functionName);
-        }
+        undefined,
+        undefined,
+        [
+          new InstrumentationNodeModuleFile(
+            module,
+            ['*'],
+            (moduleExports: LambdaModule) => {
+              diag.debug('Applying patch for lambdatest handler');
+              if (isWrapped(moduleExports[functionName])) {
+                this._unwrap(moduleExports, functionName);
+              }
+              this._wrap(moduleExports, functionName, this._getHandler());
+              return moduleExports;
+            },
+            (moduleExports?: LambdaModule) => {
+              if (moduleExports == undefined) return;
+              diag.debug('Removing patch for lambdatest handler');
+              this._unwrap(moduleExports, functionName);
+            }
+          ),
+        ]
       ),
     ];
   }

diff --git a/...ins/node/opentelemetry-instrumentation-awslambda/test/integrations/lambda-handler.test.ts b/...ins/node/opentelemetry-instrumentation-awslambda/test/integrations/lambda-handler.test.ts
@@ -17,6 +17,8 @@
 // We access through node_modules to allow it to be patched.
 /* eslint-disable node/no-extraneous-require */
 
+import * as path from 'path';
+
 import { AwsLambdaInstrumentation } from '../../src/awslambda';
 import {
   InMemorySpanExporter,
@@ -69,13 +71,20 @@ describe('lambdatest handler', () => {
   } as Context;
 
   const initializeHandler = (handler: string) => {
-    oldEnv = process.env;
     process.env._HANDLER = handler;
 
     instrumentation = new AwsLambdaInstrumentation();
     instrumentation.setTracerProvider(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();
@@ -87,7 +96,10 @@ describe('lambdatest handler', () => {
     it('should export a valid span', async () => {
       initializeHandler('lambdatest/async.handler');
 
-      const result = await require('lambdatest/async').handler('arg', ctx);
+      const result = await lambdaRequire('lambdatest/async').handler(
+        'arg',
+        ctx
+      );
       assert.strictEqual(result, 'ok');
       const spans = memoryExporter.getFinishedSpans();
       const [span] = spans;
@@ -100,7 +112,7 @@ describe('lambdatest handler', () => {
 
       let err: Error;
       try {
-        await require('lambdatest/async').error('arg', ctx);
+        await lambdaRequire('lambdatest/async').error('arg', ctx);
       } catch (e) {
         err = e;
       }
@@ -116,7 +128,7 @@ describe('lambdatest handler', () => {
 
       let err: string;
       try {
-        await require('lambdatest/async').stringerror('arg', ctx);
+        await lambdaRequire('lambdatest/async').stringerror('arg', ctx);
       } catch (e) {
         err = e;
       }
@@ -132,7 +144,7 @@ describe('lambdatest handler', () => {
       initializeHandler('lambdatest/sync.handler');
 
       const result = await new Promise((resolve, reject) => {
-        require('lambdatest/sync').handler(
+        lambdaRequire('lambdatest/sync').handler(
           'arg',
           ctx,
           (err: Error, res: any) => {
@@ -156,7 +168,7 @@ describe('lambdatest handler', () => {
 
       let err: Error;
       try {
-        require('lambdatest/sync').error(
+        lambdaRequire('lambdatest/sync').error(
           'arg',
           ctx,
           (err: Error, res: any) => {}
@@ -177,7 +189,7 @@ describe('lambdatest handler', () => {
       let err: Error;
       try {
         await new Promise((resolve, reject) => {
-          require('lambdatest/sync').callbackerror(
+          lambdaRequire('lambdatest/sync').callbackerror(
             'arg',
             ctx,
             (err: Error, res: any) => {
@@ -204,7 +216,7 @@ describe('lambdatest handler', () => {
 
       let err: string;
       try {
-        require('lambdatest/sync').stringerror(
+        lambdaRequire('lambdatest/sync').stringerror(
           'arg',
           ctx,
           (err: Error, res: any) => {}
@@ -226,7 +238,7 @@ describe('lambdatest handler', () => {
     let err: string;
     try {
       await new Promise((resolve, reject) => {
-        require('lambdatest/sync').callbackstringerror(
+        lambdaRequire('lambdatest/sync').callbackstringerror(
           'arg',
           ctx,
           (err: Error, res: any) => {