diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 724a0c32882..adb6b0ec854 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -21,6 +21,10 @@ All notable changes to experimental packages in this project will be documented * `appendResourcePathToUrlIfNeeded` * `configureExporterTimeout` * `invalidTimeout` +* feat(instrumentation-http)!: remove long deprecated options [#5085](https://github.com/open-telemetry/opentelemetry-js/pull/5085) @pichlermarc + * `ignoreIncomingPaths` has been removed, use the more versatile `ignoreIncomingRequestHook` instead. + * `ignoreOutgoingUrls` has been removed, use the more versatile `ignoreOutgoingRequestHook` instead. + * `isIgnored` utility function was intended for internal use and has been removed without replacement. ### :rocket: (Enhancement) diff --git a/experimental/packages/opentelemetry-instrumentation-http/README.md b/experimental/packages/opentelemetry-instrumentation-http/README.md index 030e8f327b6..a0c19c8e33c 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/README.md +++ b/experimental/packages/opentelemetry-instrumentation-http/README.md @@ -68,12 +68,6 @@ Http instrumentation has few options available to choose from. You can set the f | [`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 | - ## Semantic Conventions Prior to version `0.54`, this instrumentation created spans targeting an experimental semantic convention [Version 1.7.0](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.7.0/semantic_conventions/README.md). diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts index 733106f1623..98a7ea2e04a 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts @@ -86,7 +86,6 @@ import { getOutgoingRequestMetricAttributesOnResponse, getRequestInfo, headerCapture, - isIgnored, isValidOptionsType, parseResponseStatus, setSpanWithError, @@ -603,9 +602,6 @@ export class HttpInstrumentation extends InstrumentationBase - instrumentation._diag.error('caught ignoreIncomingPaths error: ', e) - ) || safeExecuteInTheMiddle( () => instrumentation.getConfig().ignoreIncomingRequestHook?.(request), @@ -767,10 +757,7 @@ export class HttpInstrumentation extends InstrumentationBase - instrumentation._diag.error('caught ignoreOutgoingUrls error: ', e) - ) || safeExecuteInTheMiddle( () => instrumentation diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/index.ts b/experimental/packages/opentelemetry-instrumentation-http/src/index.ts index 62d2d699100..436951fc3aa 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/index.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/index.ts @@ -51,7 +51,6 @@ export { getRequestInfo, headerCapture, isCompressed, - isIgnored, isValidOptionsType, parseResponseStatus, satisfiesPattern, diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/types.ts b/experimental/packages/opentelemetry-instrumentation-http/src/types.ts index 19647fcb49a..fc24995d3da 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/types.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/types.ts @@ -84,18 +84,8 @@ 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 - * @deprecated use `ignoreIncomingRequestHook` instead - */ - ignoreIncomingPaths?: IgnoreMatcher[]; /** 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; /** If set to true, incoming requests will not be instrumented at all. */ diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts b/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts index 6acb5df66e0..a03600dfd42 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts @@ -146,39 +146,6 @@ export const satisfiesPattern = ( } }; -/** - * Check whether the given request is ignored by configuration - * It will not re-throw exceptions from `list` provided by the client - * @param constant e.g URL of request - * @param [list] List of ignore patterns - * @param [onException] callback for doing something when an exception has - * occurred - */ -export const isIgnored = ( - constant: string, - list?: IgnoreMatcher[], - onException?: (error: unknown) => void -): boolean => { - if (!list) { - // No ignored urls - trace everything - return false; - } - // Try/catch outside the loop for failing fast - try { - for (const pattern of list) { - if (satisfiesPattern(constant, pattern)) { - return true; - } - } - } catch (e) { - if (onException) { - onException(e); - } - } - - return false; -}; - /** * Sets the span with the error passed in params * @param {Span} span the span that need to be set diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts index 033317306ee..f38d2018342 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts @@ -183,19 +183,9 @@ describe('HttpInstrumentation', () => { before(async () => { const config: HttpInstrumentationConfig = { - ignoreIncomingPaths: [ - (url: string) => { - 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'); }, @@ -308,21 +298,11 @@ describe('HttpInstrumentation', () => { before(async () => { instrumentation.setConfig({ - ignoreIncomingPaths: [ - '/ignored/string', - /\/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 ( @@ -592,19 +572,7 @@ describe('HttpInstrumentation', () => { }); }); - for (const ignored of ['string', 'function', 'regexp']) { - it(`should not trace ignored requests with paths (client and server side) with type ${ignored}`, async () => { - const testPath = `/ignored/${ignored}`; - - await httpRequest.get( - `${protocol}://${hostname}:${serverPort}${testPath}` - ); - const spans = memoryExporter.getFinishedSpans(); - assert.strictEqual(spans.length, 0); - }); - } - - it('should not trace ignored requests with headers (client and server side)', async () => { + it('should not trace ignored requests when ignore hook returns true', async () => { const testValue = 'ignored-string'; await Promise.all([ @@ -618,7 +586,7 @@ describe('HttpInstrumentation', () => { assert.strictEqual(spans.length, 0); }); - it('should trace not ignored requests with headers (client and server side)', async () => { + it('should trace requests when ignore hook returns false', async () => { await httpRequest.get(`${protocol}://${hostname}:${serverPort}`, { headers: { 'user-agent': 'test-bot', diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/https-enable.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/https-enable.test.ts index 0f48a30ae72..7c2e68bb0f7 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/https-enable.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/https-enable.test.ts @@ -111,19 +111,9 @@ describe('HttpsInstrumentation', () => { before(() => { instrumentation.setConfig({ - ignoreIncomingPaths: [ - (url: string) => { - 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'); }, @@ -192,21 +182,11 @@ describe('HttpsInstrumentation', () => { before(() => { instrumentation.setConfig({ - ignoreIncomingPaths: [ - '/ignored/string', - /\/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 ( @@ -441,19 +421,7 @@ describe('HttpsInstrumentation', () => { }); }); - for (const ignored of ['string', 'function', 'regexp']) { - it(`should not trace ignored requests with paths (client and server side) with type ${ignored}`, async () => { - const testPath = `/ignored/${ignored}`; - - await httpsRequest.get( - `${protocol}://${hostname}:${serverPort}${testPath}` - ); - const spans = memoryExporter.getFinishedSpans(); - assert.strictEqual(spans.length, 0); - }); - } - - it('should not trace ignored requests with headers (client and server side)', async () => { + it('should trace requests when ignore hook returns false', async () => { const testValue = 'ignored-string'; await Promise.all([ diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts index c091529c9fc..2d9911c7d37 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts @@ -149,85 +149,6 @@ describe('Utility', () => { }); }); - describe('isIgnored()', () => { - beforeEach(() => { - sinon.spy(utils, 'satisfiesPattern'); - }); - - afterEach(() => { - sinon.restore(); - }); - - it('should call isSatisfyPattern, n match', () => { - const answer1 = utils.isIgnored('/test/1', ['/test/11']); - assert.strictEqual(answer1, false); - assert.strictEqual( - (utils.satisfiesPattern as sinon.SinonSpy).callCount, - 1 - ); - }); - - it('should call isSatisfyPattern, match for function', () => { - const answer1 = utils.isIgnored('/test/1', [ - url => url.endsWith('/test/1'), - ]); - assert.strictEqual(answer1, true); - }); - - it('should not re-throw when function throws an exception', () => { - const onException = (e: unknown) => { - // Do nothing - }; - for (const callback of [undefined, onException]) { - assert.doesNotThrow(() => - utils.isIgnored( - '/test/1', - [ - () => { - throw new Error('test'); - }, - ], - callback - ) - ); - } - }); - - it('should call onException when function throws an exception', () => { - const onException = sinon.spy(); - assert.doesNotThrow(() => - utils.isIgnored( - '/test/1', - [ - () => { - throw new Error('test'); - }, - ], - onException - ) - ); - assert.strictEqual((onException as sinon.SinonSpy).callCount, 1); - }); - - it('should not call isSatisfyPattern', () => { - utils.isIgnored('/test/1', []); - assert.strictEqual( - (utils.satisfiesPattern as sinon.SinonSpy).callCount, - 0 - ); - }); - - it('should return false on empty list', () => { - const answer1 = utils.isIgnored('/test/1', []); - assert.strictEqual(answer1, false); - }); - - it('should not throw and return false when list is undefined', () => { - const answer2 = utils.isIgnored('/test/1', undefined); - assert.strictEqual(answer2, false); - }); - }); - describe('getAbsoluteUrl()', () => { it('should return absolute url with localhost', () => { const path = '/test/1'; diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/integrations/http-enable.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/integrations/http-enable.test.ts index 1f7829c4d22..ef42ced2e97 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/integrations/http-enable.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/integrations/http-enable.test.ts @@ -45,7 +45,6 @@ import { Socket } from 'net'; import { sendRequestTwice } from '../utils/rawRequest'; const protocol = 'http'; -const serverPort = 32345; const hostname = 'localhost'; const memoryExporter = new InMemorySpanExporter(); @@ -138,14 +137,7 @@ describe('HttpInstrumentation Integration tests', () => { }); before(() => { - const ignoreConfig = [ - `${protocol}://${hostname}:${serverPort}/ignored/string`, - /\/ignored\/regexp$/i, - (url: string) => url.endsWith('/ignored/function'), - ]; instrumentation.setConfig({ - ignoreIncomingPaths: ignoreConfig, - ignoreOutgoingUrls: ignoreConfig, applyCustomAttributesOnSpan: customAttributeFunction, }); instrumentation.enable(); diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/integrations/https-enable.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/integrations/https-enable.test.ts index aa19a8629f9..ec0c35f6303 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/integrations/https-enable.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/integrations/https-enable.test.ts @@ -46,8 +46,6 @@ import { httpsRequest } from '../utils/httpsRequest'; import { DummyPropagation } from '../utils/DummyPropagation'; const protocol = 'https'; -const serverPort = 42345; -const hostname = 'localhost'; const memoryExporter = new InMemorySpanExporter(); export const customAttributeFunction = (span: Span): void => { @@ -139,15 +137,8 @@ describe('HttpsInstrumentation Integration tests', () => { }); before(() => { - const ignoreConfig = [ - `${protocol}://${hostname}:${serverPort}/ignored/string`, - /\/ignored\/regexp$/i, - (url: string) => url.endsWith('/ignored/function'), - ]; propagation.setGlobalPropagator(new DummyPropagation()); instrumentation.setConfig({ - ignoreIncomingPaths: ignoreConfig, - ignoreOutgoingUrls: ignoreConfig, applyCustomAttributesOnSpan: customAttributeFunction, }); instrumentation.enable();