feat(http): Allow to opt-out of instrumenting incoming/outgoing reque…

…sts (#4643) * feat(http): Allow to opt-out of instrumenting incoming/outgoing requests * fix tests * PR feedback * add a changelog entry --------- Co-authored-by: Trent Mick <[email protected]>
open-telemetry · Jul 25, 2024 · 34003c9 · 34003c9
1 parent d4035eb
commit 34003c9
Show file tree

Hide file tree

Showing 6 changed files with 165 additions and 37 deletions.
diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md
@@ -11,6 +11,8 @@ All notable changes to experimental packages in this project will be documented
 
 ### :rocket: (Enhancement)
 
+* feat(instrumentation-http): Allow to opt-out of instrumenting incoming/outgoing requests [#4643](https://github.com/open-telemetry/opentelemetry-js/pull/4643) @mydea
+
 ### :bug: (Bug Fix)
 
 * fix(instrumentation-http): Ensure instrumentation of `http.get` and `https.get` work when used in ESM code [#4857](https://github.com/open-telemetry/opentelemetry-js/issues/4857) @trentm

diff --git a/experimental/packages/opentelemetry-instrumentation-http/README.md b/experimental/packages/opentelemetry-instrumentation-http/README.md
@@ -61,6 +61,8 @@ Http instrumentation has few options available to choose from. You can set the f
 | [`startOutgoingSpanHook`](https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-instrumentation-http/src/types.ts#L99) | `StartOutgoingSpanCustomAttributeFunction` | Function for adding custom attributes before a span is started in outgoingRequest |
 | `ignoreIncomingRequestHook` | `IgnoreIncomingRequestFunction` | Http instrumentation will not trace all incoming requests that matched with custom function |
 | `ignoreOutgoingRequestHook` | `IgnoreOutgoingRequestFunction` | Http instrumentation will not trace all outgoing requests that matched with custom function |
+| `disableOutgoingRequestInstrumentation` | `boolean` | Set to true to avoid instrumenting outgoing requests at all. This can be helpful when another instrumentation handles outgoing requests. |
+| `disableIncomingRequestInstrumentation` | `boolean` | Set to true to avoid instrumenting incoming requests at all. This can be helpful when another instrumentation handles incoming requests. |
 | [`serverName`](https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-instrumentation-http/src/types.ts#L101) | `string` | The primary server name of the matched virtual host. |
 | [`requireParentforOutgoingSpans`](https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-instrumentation-http/src/types.ts#L103) | Boolean | Require that is a parent span to create new span for outgoing requests. |
 | [`requireParentforIncomingSpans`](https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-instrumentation-http/src/types.ts#L105) | Boolean | Require that is a parent span to create new span for incoming requests. |

diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts
@@ -110,29 +110,37 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
       'http',
       ['*'],
       (moduleExports: Http): Http => {
-        const patchedRequest = this._wrap(
-          moduleExports,
-          'request',
-          this._getPatchOutgoingRequestFunction('http')
-        ) as unknown as Func<http.ClientRequest>;
-        this._wrap(
-          moduleExports,
-          'get',
-          this._getPatchOutgoingGetFunction(patchedRequest)
-        );
-        this._wrap(
-          moduleExports.Server.prototype,
-          'emit',
-          this._getPatchIncomingRequestFunction('http')
-        );
+        if (!this.getConfig().disableOutgoingRequestInstrumentation) {
+          const patchedRequest = this._wrap(
+            moduleExports,
+            'request',
+            this._getPatchOutgoingRequestFunction('http')
+          ) as unknown as Func<http.ClientRequest>;
+          this._wrap(
+            moduleExports,
+            'get',
+            this._getPatchOutgoingGetFunction(patchedRequest)
+          );
+        }
+        if (!this.getConfig().disableIncomingRequestInstrumentation) {
+          this._wrap(
+            moduleExports.Server.prototype,
+            'emit',
+            this._getPatchIncomingRequestFunction('http')
+          );
+        }
         return moduleExports;
       },
       (moduleExports: Http) => {
         if (moduleExports === undefined) return;
 
-        this._unwrap(moduleExports, 'request');
-        this._unwrap(moduleExports, 'get');
-        this._unwrap(moduleExports.Server.prototype, 'emit');
+        if (!this.getConfig().disableOutgoingRequestInstrumentation) {
+          this._unwrap(moduleExports, 'request');
+          this._unwrap(moduleExports, 'get');
+        }
+        if (!this.getConfig().disableIncomingRequestInstrumentation) {
+          this._unwrap(moduleExports.Server.prototype, 'emit');
+        }
       }
     );
   }
@@ -142,30 +150,37 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
       'https',
       ['*'],
       (moduleExports: Https): Https => {
-        const patchedRequest = this._wrap(
-          moduleExports,
-          'request',
-          this._getPatchHttpsOutgoingRequestFunction('https')
-        ) as unknown as Func<http.ClientRequest>;
-        this._wrap(
-          moduleExports,
-          'get',
-          this._getPatchHttpsOutgoingGetFunction(patchedRequest)
-        );
-
-        this._wrap(
-          moduleExports.Server.prototype,
-          'emit',
-          this._getPatchIncomingRequestFunction('https')
-        );
+        if (!this.getConfig().disableOutgoingRequestInstrumentation) {
+          const patchedRequest = this._wrap(
+            moduleExports,
+            'request',
+            this._getPatchHttpsOutgoingRequestFunction('https')
+          ) as unknown as Func<http.ClientRequest>;
+          this._wrap(
+            moduleExports,
+            'get',
+            this._getPatchHttpsOutgoingGetFunction(patchedRequest)
+          );
+        }
+        if (!this.getConfig().disableIncomingRequestInstrumentation) {
+          this._wrap(
+            moduleExports.Server.prototype,
+            'emit',
+            this._getPatchIncomingRequestFunction('https')
+          );
+        }
         return moduleExports;
       },
       (moduleExports: Https) => {
         if (moduleExports === undefined) return;
 
-        this._unwrap(moduleExports, 'request');
-        this._unwrap(moduleExports, 'get');
-        this._unwrap(moduleExports.Server.prototype, 'emit');
+        if (!this.getConfig().disableOutgoingRequestInstrumentation) {
+          this._unwrap(moduleExports, 'request');
+          this._unwrap(moduleExports, 'get');
+        }
+        if (!this.getConfig().disableIncomingRequestInstrumentation) {
+          this._unwrap(moduleExports.Server.prototype, 'emit');
+        }
       }
     );
   }

diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/types.ts b/experimental/packages/opentelemetry-instrumentation-http/src/types.ts
@@ -98,6 +98,10 @@ export interface HttpInstrumentationConfig extends InstrumentationConfig {
   ignoreOutgoingUrls?: IgnoreMatcher[];
   /** Not trace all outgoing requests that matched with custom function */
   ignoreOutgoingRequestHook?: IgnoreOutgoingRequestFunction;
+  /** If set to true, incoming requests will not be instrumented at all. */
+  disableIncomingRequestInstrumentation?: boolean;
+  /** If set to true, outgoing requests will not be instrumented at all. */
+  disableOutgoingRequestInstrumentation?: boolean;
   /** Function for adding custom attributes after response is handled */
   applyCustomAttributesOnSpan?: HttpCustomAttributeFunction;
   /** Function for adding custom attributes before request is handled */

diff --git a/...rimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts b/...rimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts
@@ -227,6 +227,55 @@ describe('HttpInstrumentation', () => {
       });
     });
 
+    describe('partially disable instrumentation', () => {
+      beforeEach(() => {
+        memoryExporter.reset();
+      });
+
+      afterEach(() => {
+        server.close();
+        instrumentation.disable();
+      });
+
+      it('allows to disable outgoing request instrumentation', () => {
+        server.close();
+        instrumentation.disable();
+
+        instrumentation.setConfig({
+          disableOutgoingRequestInstrumentation: true,
+        });
+        instrumentation.enable();
+        server = http.createServer((_request, response) => {
+          response.end('Test Server Response');
+        });
+
+        server.listen(serverPort);
+
+        assert.strictEqual(isWrapped(http.Server.prototype.emit), true);
+        assert.strictEqual(isWrapped(http.get), false);
+        assert.strictEqual(isWrapped(http.request), false);
+      });
+
+      it('allows to disable incoming request instrumentation', () => {
+        server.close();
+        instrumentation.disable();
+
+        instrumentation.setConfig({
+          disableIncomingRequestInstrumentation: true,
+        });
+        instrumentation.enable();
+        server = http.createServer((_request, response) => {
+          response.end('Test Server Response');
+        });
+
+        server.listen(serverPort);
+
+        assert.strictEqual(isWrapped(http.Server.prototype.emit), false);
+        assert.strictEqual(isWrapped(http.get), true);
+        assert.strictEqual(isWrapped(http.request), true);
+      });
+    });
+
     describe('with good instrumentation options', () => {
       beforeEach(() => {
         memoryExporter.reset();

diff --git a/...imental/packages/opentelemetry-instrumentation-http/test/functionals/https-enable.test.ts b/...imental/packages/opentelemetry-instrumentation-http/test/functionals/https-enable.test.ts
@@ -184,6 +184,7 @@ describe('HttpsInstrumentation', () => {
         );
       });
     });
+
     describe('with good instrumentation options', () => {
       beforeEach(() => {
         memoryExporter.reset();
@@ -704,5 +705,60 @@ describe('HttpsInstrumentation', () => {
         req.end();
       });
     });
+
+    describe('partially disable instrumentation', () => {
+      beforeEach(() => {
+        memoryExporter.reset();
+      });
+
+      afterEach(() => {
+        server.close();
+        instrumentation.disable();
+      });
+
+      it('allows to disable outgoing request instrumentation', () => {
+        instrumentation.setConfig({
+          disableOutgoingRequestInstrumentation: true,
+        });
+        instrumentation.enable();
+        server = https.createServer(
+          {
+            key: fs.readFileSync('test/fixtures/server-key.pem'),
+            cert: fs.readFileSync('test/fixtures/server-cert.pem'),
+          },
+          (request, response) => {
+            response.end('Test Server Response');
+          }
+        );
+
+        server.listen(serverPort);
+
+        assert.strictEqual(isWrapped(http.Server.prototype.emit), true);
+        assert.strictEqual(isWrapped(http.get), false);
+        assert.strictEqual(isWrapped(http.request), false);
+      });
+
+      it('allows to disable incoming request instrumentation', () => {
+        instrumentation.setConfig({
+          disableIncomingRequestInstrumentation: true,
+        });
+        instrumentation.enable();
+        server = https.createServer(
+          {
+            key: fs.readFileSync('test/fixtures/server-key.pem'),
+            cert: fs.readFileSync('test/fixtures/server-cert.pem'),
+          },
+          (request, response) => {
+            response.end('Test Server Response');
+          }
+        );
+
+        server.listen(serverPort);
+
+        assert.strictEqual(isWrapped(http.Server.prototype.emit), false);
+        assert.strictEqual(isWrapped(http.get), true);
+        assert.strictEqual(isWrapped(http.request), true);
+      });
+    });
   });
 });