feat(instrumentation-http): add options to ignore requests (#2704)

Co-authored-by: Valentin Marchaud <contact@vmarchaud.fr>
open-telemetry · Jan 17, 2022 · 21fc8b5 · 21fc8b5
1 parent d315b0a
commit 21fc8b5
Show file tree

Hide file tree

Showing 7 changed files with 164 additions and 54 deletions.
diff --git a/experimental/packages/opentelemetry-instrumentation-http/README.md b/experimental/packages/opentelemetry-instrumentation-http/README.md
@@ -50,13 +50,20 @@ Http instrumentation has few options available to choose from. You can set the f
 | [`responseHook`](https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-instrumentation-http/src/types.ts#L95) | `HttpResponseCustomAttributeFunction` | Function for adding custom attributes before response is handled |
 | [`startIncomingSpanHook`](https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-instrumentation-http/src/types.ts#L97) | `StartIncomingSpanCustomAttributeFunction` | Function for adding custom attributes before a span is started in incomingRequest |
 | [`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 |
-| [`ignoreIncomingPaths`](https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-instrumentation-http/src/types.ts#L87) | `IgnoreMatcher[]` | Http instrumentation will not trace all incoming requests that match paths |
-| [`ignoreOutgoingUrls`](https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-instrumentation-http/src/types.ts#L89) | `IgnoreMatcher[]` | Http instrumentation will not trace all outgoing requests that match urls |
+| `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 |
 | [`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. |
 | [`headersToSpanAttributes`](https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-instrumentation-http/src/types.ts#L107) | `object` | List of case insensitive HTTP headers to convert to span attributes. Client (outgoing requests, incoming responses) and server (incoming requests, outgoing responses) headers will be converted to span attributes in the form of `http.{request\|response}.header.header_name`, e.g. `http.response.header.content_length` |
 
+The following options are deprecated:
+
+| Options | Type | Description |
+| ------- | ---- | ----------- |
+| `ignoreIncomingPaths` | `IgnoreMatcher[]` | Http instrumentation will not trace all incoming requests that match paths |
+| `ignoreOutgoingUrls` | `IgnoreMatcher[]` | Http instrumentation will not trace all outgoing requests that match urls |
+
 ## Useful links
 
 - For more information on OpenTelemetry, visit: <https://opentelemetry.io/>

diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts
@@ -382,7 +382,16 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
         utils.isIgnored(
           pathname,
           instrumentation._getConfig().ignoreIncomingPaths,
-          (e: Error) => instrumentation._diag.error('caught ignoreIncomingPaths error: ', e)
+          (e: unknown) => instrumentation._diag.error('caught ignoreIncomingPaths error: ', e)
+        ) ||
+        safeExecuteInTheMiddle(
+          () => instrumentation._getConfig().ignoreIncomingRequestHook?.(request),
+          (e: unknown) => {
+            if (e != null) {
+              instrumentation._diag.error('caught ignoreIncomingRequestHook error: ', e);
+            }
+          },
+          true
         )
       ) {
         return context.with(suppressTracing(context.active()), () => {
@@ -534,7 +543,16 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
         utils.isIgnored(
           origin + pathname,
           instrumentation._getConfig().ignoreOutgoingUrls,
-          (e: Error) => instrumentation._diag.error('caught ignoreOutgoingUrls error: ', e)
+          (e: unknown) => instrumentation._diag.error('caught ignoreOutgoingUrls error: ', e)
+        ) ||
+        safeExecuteInTheMiddle(
+          () => instrumentation._getConfig().ignoreOutgoingRequestHook?.(optionsParsed),
+          (e: unknown) => {
+            if (e != null) {
+              instrumentation._diag.error('caught ignoreOutgoingRequestHook error: ', e);
+            }
+          },
+          true
         )
       ) {
         return original.apply(this, [optionsParsed, ...args]);

diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/types.ts b/experimental/packages/opentelemetry-instrumentation-http/src/types.ts
@@ -63,6 +63,14 @@ export interface HttpCustomAttributeFunction {
   ): void;
 }
 
+export interface IgnoreIncomingRequestFunction {
+  (request: IncomingMessage ): boolean;
+}
+
+export interface IgnoreOutgoingRequestFunction {
+  (request: RequestOptions ): boolean;
+}
+
 export interface HttpRequestCustomAttributeFunction {
   (span: Span, request: ClientRequest | IncomingMessage): void;
 }
@@ -83,10 +91,20 @@ export interface StartOutgoingSpanCustomAttributeFunction {
  * Options available for the HTTP instrumentation (see [documentation](https://github.com/open-telemetry/opentelemetry-js/tree/main/packages/opentelemetry-instrumentation-http#http-instrumentation-options))
  */
 export interface HttpInstrumentationConfig extends InstrumentationConfig {
-  /** Not trace all incoming requests that match paths */
+  /**
+   * Not trace all incoming requests that match paths
+   * @deprecated use `ignoreIncomingRequestHook` instead
+   */
   ignoreIncomingPaths?: IgnoreMatcher[];
-  /** Not trace all outgoing requests that match urls */
+  /** Not trace all incoming requests that matched with custom function */
+  ignoreIncomingRequestHook?: IgnoreIncomingRequestFunction;
+  /**
+   * Not trace all outgoing requests that match urls
+   * @deprecated use `ignoreOutgoingRequestHook` instead
+   */
   ignoreOutgoingUrls?: IgnoreMatcher[];
+  /** Not trace all outgoing requests that matched with custom function */
+  ignoreOutgoingRequestHook?: IgnoreOutgoingRequestFunction;
   /** Function for adding custom attributes after response is handled */
   applyCustomAttributesOnSpan?: HttpCustomAttributeFunction;
   /** Function for adding custom attributes before request is handled */

diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts b/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts
@@ -85,19 +85,6 @@ export const parseResponseStatus = (
   return { code: SpanStatusCode.ERROR };
 };
 
-/**
- * Returns whether the Expect header is on the given options object.
- * @param options Options for http.request.
- */
-export const hasExpectHeader = (options: RequestOptions): boolean => {
-  if (!options.headers) {
-    return false;
-  }
-
-  const keys = Object.keys(options.headers);
-  return !!keys.find(key => key.toLowerCase() === 'expect');
-};
-
 /**
  * Check whether the given obj match pattern
  * @param constant e.g URL of request
@@ -129,7 +116,7 @@ export const satisfiesPattern = (
 export const isIgnored = (
   constant: string,
   list?: IgnoreMatcher[],
-  onException?: (error: Error) => void
+  onException?: (error: unknown) => void
 ): boolean => {
   if (!list) {
     // No ignored urls - trace everything
@@ -304,18 +291,18 @@ export const getRequestInfo = (
     }`;
   }
 
-  if (hasExpectHeader(optionsParsed)) {
-    optionsParsed.headers = Object.assign({}, optionsParsed.headers);
-  } else if (!optionsParsed.headers) {
-    optionsParsed.headers = {};
-  }
+  const headers = optionsParsed.headers ?? {};
+  optionsParsed.headers = Object.keys(headers).reduce((normalizedHeader, key) => {
+    normalizedHeader[key.toLowerCase()] = headers[key];
+    return normalizedHeader;
+  }, {} as OutgoingHttpHeaders);
   // some packages return method in lowercase..
   // ensure upperCase for consistency
   const method = optionsParsed.method
     ? optionsParsed.method.toUpperCase()
     : 'GET';
 
-  return { origin, pathname, method, optionsParsed };
+  return { origin, pathname, method, optionsParsed, };
 };
 
 /**

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
@@ -142,11 +142,17 @@ describe('HttpInstrumentation', () => {
               throw new Error('bad ignoreIncomingPaths function');
             },
           ],
+          ignoreIncomingRequestHook: _request => {
+            throw new Error('bad ignoreIncomingRequestHook function');
+          },
           ignoreOutgoingUrls: [
             (url: string) => {
               throw new Error('bad ignoreOutgoingUrls function');
             },
           ],
+          ignoreOutgoingRequestHook: _request => {
+            throw new Error('bad ignoreOutgoingRequestHook function');
+          },
           applyCustomAttributesOnSpan: () => {
             throw new Error(applyCustomAttributesOnSpanErrorMessage);
           },
@@ -167,7 +173,12 @@ describe('HttpInstrumentation', () => {
 
       it('should generate valid spans (client side and server side)', async () => {
         const result = await httpRequest.get(
-          `${protocol}://${hostname}:${serverPort}${pathname}`
+          `${protocol}://${hostname}:${serverPort}${pathname}`,
+          {
+            headers: {
+              'user-agent': 'tester'
+            }
+          }
         );
         const spans = memoryExporter.getFinishedSpans();
         const [incomingSpan, outgoingSpan] = spans;
@@ -207,11 +218,20 @@ describe('HttpInstrumentation', () => {
             /\/ignored\/regexp$/i,
             (url: string) => url.endsWith('/ignored/function'),
           ],
+          ignoreIncomingRequestHook: request => {
+            return request.headers['user-agent']?.match('ignored-string') != null;
+          },
           ignoreOutgoingUrls: [
             `${protocol}://${hostname}:${serverPort}/ignored/string`,
             /\/ignored\/regexp$/i,
             (url: string) => url.endsWith('/ignored/function'),
           ],
+          ignoreOutgoingRequestHook: request => {
+            if (request.headers?.['user-agent'] != null) {
+              return `${request.headers['user-agent']}`.match('ignored-string') != null;
+            }
+            return false;
+          },
           applyCustomAttributesOnSpan: customAttributeFunction,
           requestHook: requestHookFunction,
           responseHook: responseHookFunction,
@@ -447,7 +467,7 @@ describe('HttpInstrumentation', () => {
       });
 
       for (const ignored of ['string', 'function', 'regexp']) {
-        it(`should not trace ignored requests (client and server side) with type ${ignored}`, async () => {
+        it(`should not trace ignored requests with paths (client and server side) with type ${ignored}`, async () => {
           const testPath = `/ignored/${ignored}`;
 
           await httpRequest.get(
@@ -458,6 +478,44 @@ describe('HttpInstrumentation', () => {
         });
       }
 
+      it('should not trace ignored requests with headers (client and server side)', async () => {
+        const testValue = 'ignored-string';
+
+        await Promise.all([
+          httpRequest.get(
+            `${protocol}://${hostname}:${serverPort}`,
+            {
+              headers: {
+                'user-agent': testValue
+              }
+            }
+          ),
+          httpRequest.get(
+            `${protocol}://${hostname}:${serverPort}`,
+            {
+              headers: {
+                'uSeR-aGeNt': testValue
+              }
+            }
+          ),
+        ]);
+        const spans = memoryExporter.getFinishedSpans();
+        assert.strictEqual(spans.length, 0);
+      });
+
+      it('should trace not ignored requests with headers (client and server side)', async () => {
+        await httpRequest.get(
+          `${protocol}://${hostname}:${serverPort}`,
+          {
+            headers: {
+              'user-agent': 'test-bot',
+            }
+          }
+        );
+        const spans = memoryExporter.getFinishedSpans();
+        assert.strictEqual(spans.length, 2);
+      });
+
       for (const arg of ['string', {}, new Date()]) {
         it(`should be tracable and not throw exception in ${protocol} instrumentation when passing the following argument ${JSON.stringify(
           arg

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
@@ -112,11 +112,17 @@ describe('HttpsInstrumentation', () => {
               throw new Error('bad ignoreIncomingPaths function');
             },
           ],
+          ignoreIncomingRequestHook: _request => {
+            throw new Error('bad ignoreIncomingRequestHook function');
+          },
           ignoreOutgoingUrls: [
             (url: string) => {
               throw new Error('bad ignoreOutgoingUrls function');
             },
           ],
+          ignoreOutgoingRequestHook: _request => {
+            throw new Error('bad ignoreOutgoingRequestHook function');
+          },
           applyCustomAttributesOnSpan: () => {
             throw new Error(applyCustomAttributesOnSpanErrorMessage);
           },
@@ -142,7 +148,12 @@ describe('HttpsInstrumentation', () => {
 
       it('should generate valid spans (client side and server side)', async () => {
         const result = await httpsRequest.get(
-          `${protocol}://${hostname}:${serverPort}${pathname}`
+          `${protocol}://${hostname}:${serverPort}${pathname}`,
+          {
+            headers: {
+              'user-agent': 'tester'
+            }
+          }
         );
         const spans = memoryExporter.getFinishedSpans();
         const [incomingSpan, outgoingSpan] = spans;
@@ -181,11 +192,20 @@ describe('HttpsInstrumentation', () => {
             /\/ignored\/regexp$/i,
             (url: string) => url.endsWith('/ignored/function'),
           ],
+          ignoreIncomingRequestHook: request => {
+            return request.headers['user-agent']?.match('ignored-string') != null;
+          },
           ignoreOutgoingUrls: [
             `${protocol}://${hostname}:${serverPort}/ignored/string`,
             /\/ignored\/regexp$/i,
             (url: string) => url.endsWith('/ignored/function'),
           ],
+          ignoreOutgoingRequestHook: request => {
+            if (request.headers?.['user-agent'] != null) {
+              return `${request.headers['user-agent']}`.match('ignored-string') != null;
+            }
+            return false;
+          },
           applyCustomAttributesOnSpan: customAttributeFunction,
           serverName,
         });
@@ -412,7 +432,7 @@ describe('HttpsInstrumentation', () => {
       });
 
       for (const ignored of ['string', 'function', 'regexp']) {
-        it(`should not trace ignored requests (client and server side) with type ${ignored}`, async () => {
+        it(`should not trace ignored requests with paths (client and server side) with type ${ignored}`, async () => {
           const testPath = `/ignored/${ignored}`;
 
           await httpsRequest.get(
@@ -423,6 +443,31 @@ describe('HttpsInstrumentation', () => {
         });
       }
 
+      it('should not trace ignored requests with headers (client and server side)', async () => {
+        const testValue = 'ignored-string';
+
+        await Promise.all([
+          httpsRequest.get(
+            `${protocol}://${hostname}:${serverPort}`,
+            {
+              headers: {
+                'user-agent': testValue
+              }
+            }
+          ),
+          httpsRequest.get(
+            `${protocol}://${hostname}:${serverPort}`,
+            {
+              headers: {
+                'uSeR-aGeNt': testValue
+              }
+            }
+          )
+        ]);
+        const spans = memoryExporter.getFinishedSpans();
+        assert.strictEqual(spans.length, 0);
+      });
+
       for (const arg of ['string', {}, new Date()]) {
         it(`should be tracable and not throw exception in ${protocol} instrumentation when passing the following argument ${JSON.stringify(
           arg

diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts
@@ -24,7 +24,6 @@ import {
 import { BasicTracerProvider, Span } from '@opentelemetry/sdk-trace-base';
 import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
 import * as assert from 'assert';
-import * as http from 'http';
 import { IncomingMessage, ServerResponse } from 'http';
 import { Socket } from 'net';
 import * as sinon from 'sinon';
@@ -58,28 +57,6 @@ describe('Utility', () => {
       }
     });
   });
-  describe('hasExpectHeader()', () => {
-    it('should throw if no option', () => {
-      try {
-        utils.hasExpectHeader('' as http.RequestOptions);
-        assert.fail();
-      } catch (ignore) {}
-    });
-
-    it('should not throw if no headers', () => {
-      const result = utils.hasExpectHeader({} as http.RequestOptions);
-      assert.strictEqual(result, false);
-    });
-
-    it('should return true on Expect (no case sensitive)', () => {
-      for (const headers of [{ Expect: 1 }, { expect: 1 }, { ExPect: 1 }]) {
-        const result = utils.hasExpectHeader({
-          headers,
-        } as http.RequestOptions);
-        assert.strictEqual(result, true);
-      }
-    });
-  });
 
   describe('getRequestInfo()', () => {
     it('should get options object', () => {
@@ -170,7 +147,7 @@ describe('Utility', () => {
     });
 
     it('should not re-throw when function throws an exception', () => {
-      const onException = (e: Error) => {
+      const onException = (e: unknown) => {
         // Do nothing
       };
       for (const callback of [undefined, onException]) {