From b55b79b72451c65080e01c2ec11655cabd5f65d9 Mon Sep 17 00:00:00 2001
From: Nathan Walters <nwalters512@gmail.com>
Date: Fri, 15 Apr 2022 10:48:29 -0700
Subject: [PATCH] fix: correctly disable Express instrumentation (#972)

Co-authored-by: Rauno Viskus <Rauno56@users.noreply.github.com>
---
 .../src/instrumentation.ts                    |  7 ++--
 .../test/express.test.ts                      | 32 +++++++++----------
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts
index 37a1ea3763..8b7ed5d119 100644
--- 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
index cfbe225db5..04f2826f61 100644
--- 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],