fix: link net and http instrumentations correctly (#2046)

open-telemetry · Mar 29, 2021 · a707588 · a707588
1 parent ed92829
commit a707588
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 24 deletions.
diff --git a/packages/opentelemetry-instrumentation-http/src/http.ts b/packages/opentelemetry-instrumentation-http/src/http.ts
@@ -291,7 +291,12 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
       this._callRequestHook(span, request);
     }
 
-    request.on(
+    /*
+     * User 'response' event listeners can be added before our listener,
+     * force our listener to be the first, so response emitter is bound
+     * before any user listeners are added to it.
+     */
+    request.prependListener(
       'response',
       (response: http.IncomingMessage & { aborted?: boolean }) => {
         const responseAttributes = utils.getOutgoingRequestAttributesOnResponse(
@@ -427,6 +432,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
             response.end = originalEnd;
             // Cannot pass args of type ResponseEndArgs,
             const returned = safeExecuteInTheMiddle(
+              // eslint-disable-next-line @typescript-eslint/no-explicit-any
               () => response.end.apply(this, arguments as any),
               error => {
                 if (error) {
@@ -528,33 +534,45 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
         kind: SpanKind.CLIENT,
       };
       const span = instrumentation._startHttpSpan(operationName, spanOptions);
+
+      const parentContext = context.active();
+      const requestContext = setSpan(parentContext, span);
+
       if (!optionsParsed.headers) {
         optionsParsed.headers = {};
       }
-      propagation.inject(
-        setSpan(context.active(), span),
-        optionsParsed.headers
-      );
+      propagation.inject(requestContext, optionsParsed.headers);
+
+      return context.with(requestContext, () => {
+        /*
+         * The response callback is registered before ClientRequest is bound,
+         * thus it is needed to bind it before the function call.
+         */
+        const cb = args[args.length - 1];
+        if (typeof cb === 'function') {
+          args[args.length - 1] = context.bind(cb, parentContext);
+        }
 
-      const request: http.ClientRequest = safeExecuteInTheMiddle(
-        () => original.apply(this, [optionsParsed, ...args]),
-        error => {
-          if (error) {
-            utils.setSpanWithError(span, error);
-            instrumentation._closeHttpSpan(span);
-            throw error;
+        const request: http.ClientRequest = safeExecuteInTheMiddle(
+          () => original.apply(this, [optionsParsed, ...args]),
+          error => {
+            if (error) {
+              utils.setSpanWithError(span, error);
+              instrumentation._closeHttpSpan(span);
+              throw error;
+            }
           }
-        }
-      );
+        );
 
-      diag.debug('%s instrumentation outgoingRequest', component);
-      context.bind(request);
-      return instrumentation._traceClientRequest(
-        component,
-        request,
-        optionsParsed,
-        span
-      );
+        diag.debug('%s instrumentation outgoingRequest', component);
+        context.bind(request, parentContext);
+        return instrumentation._traceClientRequest(
+          component,
+          request,
+          optionsParsed,
+          span
+        );
+      });
     };
   }
 

diff --git a/packages/opentelemetry-instrumentation-http/src/types.ts b/packages/opentelemetry-instrumentation-http/src/types.ts
@@ -44,7 +44,7 @@ export type ParsedRequestOptions =
   | http.RequestOptions;
 export type Http = typeof http;
 export type Https = typeof https;
-/* tslint:disable-next-line:no-any */
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
 export type Func<T> = (...args: any[]) => T;
 export type ResponseEndArgs =
   | [((() => void) | undefined)?]

diff --git a/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts b/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts
@@ -181,6 +181,7 @@ describe('HttpInstrumentation', () => {
         );
       });
     });
+
     describe('with good instrumentation options', () => {
       beforeEach(() => {
         memoryExporter.reset();
@@ -705,7 +706,15 @@ describe('HttpInstrumentation', () => {
         assert.deepStrictEqual(getSpan(context.active()), undefined);
         http.get(`${protocol}://${hostname}:${serverPort}/test`, res => {
           assert.deepStrictEqual(getSpan(context.active()), undefined);
-          done();
+
+          res.on('data', () => {
+            assert.deepStrictEqual(getSpan(context.active()), undefined);
+          });
+
+          res.on('end', () => {
+            assert.deepStrictEqual(getSpan(context.active()), undefined);
+            done();
+          });
         });
       });
     });