fix: Correctly disable Express instrumentation

open-telemetry · Apr 12, 2022 · 43e8a7b · 43e8a7b
1 parent e99d682
commit 43e8a7b
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 19 deletions.
diff --git a/plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts
@@ -101,9 +101,10 @@ export class ExpressInstrumentation extends InstrumentationBase<
         (moduleExports, moduleVersion) => {
           if (moduleExports === undefined) return;
           diag.debug(`Removing patch for express@${moduleVersion}`);
-          this._unwrap(moduleExports.Router.prototype, 'route');
-          this._unwrap(moduleExports.Router.prototype, 'use');
-          this._unwrap(moduleExports.application.prototype, 'use');
+          const routerProto = moduleExports.Router as unknown as express.Router;
+          this._unwrap(routerProto, 'route');
+          this._unwrap(routerProto, 'use');
+          this._unwrap(moduleExports.application, 'use');
         }
       ),
     ];

diff --git a/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts b/plugins/node/opentelemetry-instrumentation-express/test/express.test.ts
@@ -56,20 +56,12 @@ describe('ExpressInstrumentation', () => {
   describe('Instrumenting normal get operations', () => {
     it('should create a child span for middlewares', async () => {
       const rootSpan = tracer.startSpan('rootSpan');
-      const app = express();
-      app.use((req, res, next) =>
-        context.with(trace.setSpan(context.active(), rootSpan), next)
-      );
-      app.use(express.json());
       const customMiddleware: express.RequestHandler = (req, res, next) => {
         for (let i = 0; i < 1000000; i++) {
           continue;
         }
         return next();
       };
-      app.use(customMiddleware);
-      const router = express.Router();
-      app.use('/toto', router);
       let finishListenerCount: number | undefined;
       const { server, port } = await serverWithMiddleware(
         tracer,
@@ -289,22 +281,30 @@ describe('ExpressInstrumentation', () => {
 
   describe('Disabling plugin', () => {
     it('should not create new spans', async () => {
+      instrumentation.disable();
       const rootSpan = tracer.startSpan('rootSpan');
-      const app = express();
-      app.use(express.json());
-      app.use((req, res, next) => {
-        for (let i = 0; i < 1000; i++) {
-          continue;
+      const { server, port } = await serverWithMiddleware(
+        tracer,
+        rootSpan,
+        app => {
+          app.use(express.json());
+          const customMiddleware: express.RequestHandler = (req, res, next) => {
+            for (let i = 0; i < 1000; i++) {
+              continue;
+            }
+            return next();
+          };
+          app.use(customMiddleware);
         }
-        return next();
-      });
-      const { server, port } = await createServer(app);
+      );
       assert.strictEqual(memoryExporter.getFinishedSpans().length, 0);
       await context.with(
         trace.setSpan(context.active(), rootSpan),
         async () => {
           await httpRequest.get(`http://localhost:${port}/toto/tata`);
           rootSpan.end();
+          // There should be exactly one span, and it should be the root span.
+          // There should not be any spans from the Express instrumentation.
           assert.deepEqual(memoryExporter.getFinishedSpans().length, 1);
           assert.notStrictEqual(
             memoryExporter.getFinishedSpans()[0],