From da710c9620ac8b57944fd514a129ef4c70000c8f Mon Sep 17 00:00:00 2001 From: Jairo Date: Wed, 21 Aug 2024 10:08:08 -0700 Subject: [PATCH 01/16] Add allowUrls config option to Fetch Instrumentation --- .../src/fetch.ts | 7 +++++++ packages/opentelemetry-core/src/index.ts | 2 +- packages/opentelemetry-core/src/utils/url.ts | 15 +++++++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts index fedb495d685..9ba1ee7e373 100644 --- a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts +++ b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts @@ -64,6 +64,7 @@ export interface FetchInstrumentationConfig extends InstrumentationConfig { clearTimingResources?: boolean; // urls which should include trace headers when origin doesn't match propagateTraceHeaderCorsUrls?: web.PropagateTraceHeaderCorsUrls; + allowUrls?: Array; /** * URLs that partially match any regex in ignoreUrls will not be traced. * In addition, URLs that are _exact matches_ of strings in ignoreUrls will @@ -201,8 +202,14 @@ export class FetchInstrumentation extends InstrumentationBase = {} ): api.Span | undefined { + if (!core.isUrlAllowed(url, this.getConfig().allowUrls)) { + this._diag.debug('ignoring span as url does not match an allowed url'); + console.log(`ignoring ${url} as url does not match an allowed url`); + return; + } if (core.isUrlIgnored(url, this.getConfig().ignoreUrls)) { this._diag.debug('ignoring span as url matches ignored url'); + console.log(`ignoring ${url} as url does matches an ignored url`); return; } const method = (options.method || 'GET').toUpperCase(); diff --git a/packages/opentelemetry-core/src/index.ts b/packages/opentelemetry-core/src/index.ts index 0c2dc7c1a8d..8e246cc6d9c 100644 --- a/packages/opentelemetry-core/src/index.ts +++ b/packages/opentelemetry-core/src/index.ts @@ -112,7 +112,7 @@ export { export { merge } from './utils/merge'; export { TracesSamplerValues } from './utils/sampling'; export { TimeoutError, callWithTimeout } from './utils/timeout'; -export { isUrlIgnored, urlMatches } from './utils/url'; +export { isUrlAllowed, isUrlIgnored, urlMatches } from './utils/url'; export { isWrapped } from './utils/wrap'; export { BindOnceFuture } from './utils/callback'; export { VERSION } from './version'; diff --git a/packages/opentelemetry-core/src/utils/url.ts b/packages/opentelemetry-core/src/utils/url.ts index a6122ae7842..4a2c7d03650 100644 --- a/packages/opentelemetry-core/src/utils/url.ts +++ b/packages/opentelemetry-core/src/utils/url.ts @@ -20,6 +20,21 @@ export function urlMatches(url: string, urlToMatch: string | RegExp): boolean { return !!url.match(urlToMatch); } } +export function isUrlAllowed( + url: string, + allowedUrls?: Array +): boolean { + if (!allowedUrls) { + return true; + } + + for (const allowedUrl of allowedUrls) { + if (urlMatches(url, allowedUrl)) { + return true; + } + } + return false; +} /** * Check if {@param url} should be ignored when comparing against {@param ignoredUrls} * @param url From 8496c189215fec2027e202d7615cff3d543d98c0 Mon Sep 17 00:00:00 2001 From: Jairo Date: Wed, 21 Aug 2024 10:46:29 -0700 Subject: [PATCH 02/16] Clean up code and add comments --- .../opentelemetry-instrumentation-fetch/src/fetch.ts | 6 ++++-- packages/opentelemetry-core/src/utils/url.ts | 5 +++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts index 9ba1ee7e373..96bd28ca484 100644 --- a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts +++ b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts @@ -64,6 +64,10 @@ export interface FetchInstrumentationConfig extends InstrumentationConfig { clearTimingResources?: boolean; // urls which should include trace headers when origin doesn't match propagateTraceHeaderCorsUrls?: web.PropagateTraceHeaderCorsUrls; + /** + * URLs that partially match any regex or exactly match strings in allowUrls + * will be traced. + */ allowUrls?: Array; /** * URLs that partially match any regex in ignoreUrls will not be traced. @@ -204,12 +208,10 @@ export class FetchInstrumentation extends InstrumentationBase From 6edaa151e4b195e87bc21b6a8c711d4ea3fc4bb3 Mon Sep 17 00:00:00 2001 From: Jairo Date: Wed, 21 Aug 2024 13:38:04 -0700 Subject: [PATCH 03/16] Add allowUrls config option to xhr instrumentation --- .../src/xhr.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts b/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts index 5a8ba3012cb..1bfe1639b04 100644 --- a/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts +++ b/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts @@ -21,7 +21,12 @@ import { InstrumentationConfig, safeExecuteInTheMiddle, } from '@opentelemetry/instrumentation'; -import { hrTime, isUrlIgnored, otperformance } from '@opentelemetry/core'; +import { + hrTime, + isUrlAllowed, + isUrlIgnored, + otperformance, +} from '@opentelemetry/core'; import { SEMATTRS_HTTP_HOST, SEMATTRS_HTTP_METHOD, @@ -73,6 +78,11 @@ export interface XMLHttpRequestInstrumentationConfig clearTimingResources?: boolean; /** URLs which should include trace headers when origin doesn't match */ propagateTraceHeaderCorsUrls?: PropagateTraceHeaderCorsUrls; + /** + * URLs that partially match any regex or exactly match strings in allowUrls + * will be traced. + */ + allowUrls?: Array; /** * URLs that partially match any regex in ignoreUrls will not be traced. * In addition, URLs that are _exact matches_ of strings in ignoreUrls will @@ -331,6 +341,10 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase Date: Wed, 21 Aug 2024 15:28:11 -0700 Subject: [PATCH 04/16] Add unit tests --- .../test/fetch.test.ts | 40 ++++++++++++++++++- .../test/xhr.test.ts | 31 ++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/experimental/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts b/experimental/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts index 7e14cc35883..d8f21f52233 100644 --- a/experimental/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts @@ -717,6 +717,40 @@ describe('fetch', () => { }); }); + describe('when url is allowed', () => { + beforeEach(async () => { + const propagateTraceHeaderCorsUrls = url; + await prepareData(url, { + propagateTraceHeaderCorsUrls, + allowUrls: [propagateTraceHeaderCorsUrls], + }); + }); + afterEach(() => { + clearData(); + }); + it('should create a span', () => { + assert.ok(exportSpy.args.length > 0, 'span should be exported'); + }); + }); + + describe('when url is NOT allowed', () => { + const otherUrl = 'http://localhost:8099/get'; + + beforeEach(async () => { + const propagateTraceHeaderCorsUrls = url; + await prepareData(url, { + propagateTraceHeaderCorsUrls, + allowUrls: [otherUrl], + }); + }); + afterEach(() => { + clearData(); + }); + it('should NOT create any span', () => { + assert.ok(exportSpy.args.length === 0, "span shouldn't be exported"); + }); + }); + describe('when url is ignored', () => { beforeEach(async () => { const propagateTraceHeaderCorsUrls = url; @@ -729,7 +763,11 @@ describe('fetch', () => { clearData(); }); it('should NOT create any span', () => { - assert.strictEqual(exportSpy.args.length, 0, "span shouldn't b exported"); + assert.strictEqual( + exportSpy.args.length, + 0, + "span shouldn't be exported" + ); }); it('should pass request object as the first parameter to the original function (#2411)', () => { const r = new Request(url); diff --git a/experimental/packages/opentelemetry-instrumentation-xml-http-request/test/xhr.test.ts b/experimental/packages/opentelemetry-instrumentation-xml-http-request/test/xhr.test.ts index f3685e06a13..32e30de3116 100644 --- a/experimental/packages/opentelemetry-instrumentation-xml-http-request/test/xhr.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-xml-http-request/test/xhr.test.ts @@ -626,6 +626,37 @@ describe('xhr', () => { } ); + describe('when url is allowed', () => { + beforeEach(done => { + clearData(); + const propagateTraceHeaderCorsUrls = url; + prepareData(done, url, { + propagateTraceHeaderCorsUrls, + allowUrls: [propagateTraceHeaderCorsUrls], + }); + }); + + it('should create a span', () => { + assert.ok(exportSpy.called, 'span should be exported'); + }); + }); + + describe('when another url is allowed', () => { + const otherUrl = 'http://localhost:8099/get'; + beforeEach(done => { + clearData(); + const propagateTraceHeaderCorsUrls = url; + prepareData(done, url, { + propagateTraceHeaderCorsUrls, + allowUrls: [otherUrl], + }); + }); + + it('should NOT create a span', () => { + assert.ok(exportSpy.notCalled, "span shouldn't be exported"); + }); + }); + describe('when url is ignored', () => { beforeEach(done => { clearData(); From d7a73755b310b961b8b30a3d76adc4859a94cf5b Mon Sep 17 00:00:00 2001 From: Jairo Date: Thu, 22 Aug 2024 08:32:54 -0700 Subject: [PATCH 05/16] Add entry to experimental changelog --- experimental/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 796d00e393c..ef49bf10af3 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -24,6 +24,7 @@ All notable changes to experimental packages in this project will be documented * feat(otlp-transformer): Do not limit @opentelemetry/api upper range peerDependency [#4816](https://github.com/open-telemetry/opentelemetry-js/pull/4816) @mydea * feat(instrumentation-http): Allow to opt-out of instrumenting incoming/outgoing requests [#4643](https://github.com/open-telemetry/opentelemetry-js/pull/4643) @mydea +* feat(instrumentation): Add allowUrls config option to web instrumentation [#4938](https://github.com/open-telemetry/opentelemetry-js/pull/4938) @jairo-mendoza ### :bug: (Bug Fix) From f45b61ef4379ee4361202d2a19de3286e761b9a6 Mon Sep 17 00:00:00 2001 From: Jairo Date: Tue, 3 Sep 2024 14:33:48 -0700 Subject: [PATCH 06/16] Call out dependence on new function --- CHANGELOG.md | 4 ++++ experimental/CHANGELOG.md | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 613c2a18067..c349bf1459e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se ### :rocket: (Enhancement) +* feat(instrumentation): Add allowUrls config option to web instrumentation [#4938](https://github.com/open-telemetry/opentelemetry-js/pull/4938) @jairo-mendoza + * `XMLHttpRequestInstrumentation` and `FetchInstrumentation` now accept an `allowUrls` config option. + * `isUrlAllowed` function added to core. Both web instrumentation classes will depend on this function when using the new config option. + ### :bug: (Bug Fix) ### :books: (Refine Doc) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 99e81ad48ba..0bc41e92f5d 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -9,6 +9,8 @@ All notable changes to experimental packages in this project will be documented ### :rocket: (Enhancement) +* feat(instrumentation): Add allowUrls config option to web instrumentation [#4938](https://github.com/open-telemetry/opentelemetry-js/pull/4938) @jairo-mendoza + ### :bug: (Bug Fix) ### :books: (Refine Doc) @@ -37,7 +39,6 @@ All notable changes to experimental packages in this project will be documented * feat(otlp-transformer): Do not limit @opentelemetry/api upper range peerDependency [#4816](https://github.com/open-telemetry/opentelemetry-js/pull/4816) @mydea * feat(instrumentation-http): Allow to opt-out of instrumenting incoming/outgoing requests [#4643](https://github.com/open-telemetry/opentelemetry-js/pull/4643) @mydea * feat(sampler-jaeger-remote): added support of jaeger-remote-sampler according to this [spec](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk.md#jaegerremotesampler) [#4534](https://github.com/open-telemetry/opentelemetry-js/pull/4589) @legalimpurity -* feat(instrumentation): Add allowUrls config option to web instrumentation [#4938](https://github.com/open-telemetry/opentelemetry-js/pull/4938) @jairo-mendoza ### :bug: (Bug Fix) From e819c8d27385cfd1bc6caa76564453399d93dbe8 Mon Sep 17 00:00:00 2001 From: Jairo Date: Tue, 5 Nov 2024 08:19:28 -0800 Subject: [PATCH 07/16] Fix up experimental changelog --- experimental/CHANGELOG.md | 7 ------- 1 file changed, 7 deletions(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index c8255a1caa0..4735811f50b 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -48,17 +48,10 @@ All notable changes to experimental packages in this project will be documented ### :rocket: (Enhancement) * feat(api-logs): Add delegating no-op logger provider [#4861](https://github.com/open-telemetry/opentelemetry-js/pull/4861) @hectorhdzg -<<<<<<< HEAD -* feat(instrumentation-http): Add support for [Semantic Conventions 1.27+](https://github.com/open-telemetry/semantic-conventions/releases/tag/v1.27.0) [#4940](https://github.com/open-telemetry/opentelemetry-js/pull/4940) [#4978](https://github.com/open-telemetry/opentelemetry-js/pull/4978) @dyladan - * Applies to both client and server spans - * Generate spans compliant with Semantic Conventions 1.27+ when `OTEL_SEMCONV_STABILITY_OPT_IN` contains `http` or `http/dup` - * Generate spans backwards compatible with previous attributes when `OTEL_SEMCONV_STABILITY_OPT_IN` contains `http/dup` or DOES NOT contain `http` -======= * feat(instrumentation-http): Add support for [Semantic Conventions 1.27+](https://github.com/open-telemetry/semantic-conventions/releases/tag/v1.27.0) [#4940](https://github.com/open-telemetry/opentelemetry-js/pull/4940) [#4978](https://github.com/open-telemetry/opentelemetry-js/pull/4978) [#5026](https://github.com/open-telemetry/opentelemetry-js/pull/5026) @dyladan * Applies to client and server spans and metrics * Generate spans and metrics compliant with Semantic Conventions 1.27+ when `OTEL_SEMCONV_STABILITY_OPT_IN` contains `http` or `http/dup` * Generate spans and metrics backwards compatible with previous attributes when `OTEL_SEMCONV_STABILITY_OPT_IN` contains `http/dup` or DOES NOT contain `http` ->>>>>>> main ### :bug: (Bug Fix) From bbf05f260e9cd5af17f8c1623c585c526d45a9e0 Mon Sep 17 00:00:00 2001 From: Jairo Date: Wed, 6 Nov 2024 09:34:18 -0800 Subject: [PATCH 08/16] Add newline to changelog --- experimental/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 4735811f50b..cfbc192ab8b 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -8,6 +8,7 @@ All notable changes to experimental packages in this project will be documented ### :boom: Breaking Change ### :rocket: (Enhancement) + * feat(instrumentation): Add allowUrls config option to web instrumentation [#4938](https://github.com/open-telemetry/opentelemetry-js/pull/4938) @jairo-mendoza ### :bug: (Bug Fix) From e9b7e2c9f700d0ed60fe416af68d0e128c7591b9 Mon Sep 17 00:00:00 2001 From: Jairo Date: Wed, 6 Nov 2024 11:18:42 -0800 Subject: [PATCH 09/16] Update main changelog --- CHANGELOG.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d3b4b05954..048e4b54bef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se ### :rocket: (Enhancement) +* feat(instrumentation): Add allowUrls config option to web instrumentation [#4938](https://github.com/open-telemetry/opentelemetry-js/pull/4938) @jairo-mendoza + * `XMLHttpRequestInstrumentation` and `FetchInstrumentation` now accept an `allowUrls` config option. + * `isUrlAllowed` function added to core. Both web instrumentation classes will depend on this function when using the new config option. + ### :bug: (Bug Fix) ### :books: (Refine Doc) @@ -24,9 +28,6 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se ### :rocket: (Enhancement) * feat: add processors for adding session.id attribute to spans and logs [#4972](https://github.com/open-telemetry/opentelemetry-js/pull/4972) -* feat(instrumentation): Add allowUrls config option to web instrumentation [#4938](https://github.com/open-telemetry/opentelemetry-js/pull/4938) @jairo-mendoza - * `XMLHttpRequestInstrumentation` and `FetchInstrumentation` now accept an `allowUrls` config option. - * `isUrlAllowed` function added to core. Both web instrumentation classes will depend on this function when using the new config option. ### :bug: (Bug Fix) From 4962272e88028e500aeb35e1aecc6e9f8e750766 Mon Sep 17 00:00:00 2001 From: Jairo Date: Wed, 6 Nov 2024 12:35:23 -0800 Subject: [PATCH 10/16] Increase test coverage for isUrlAllowed --- .../opentelemetry-core/test/utils/url.test.ts | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/packages/opentelemetry-core/test/utils/url.test.ts b/packages/opentelemetry-core/test/utils/url.test.ts index 8855cff23c3..49193533a2f 100644 --- a/packages/opentelemetry-core/test/utils/url.test.ts +++ b/packages/opentelemetry-core/test/utils/url.test.ts @@ -16,10 +16,12 @@ import * as assert from 'assert'; -import { isUrlIgnored } from '../../src'; +import { isUrlAllowed, isUrlIgnored } from '../../src'; const urlIgnored = 'url should be ignored'; const urlNotIgnored = 'url should NOT be ignored'; +const urlAllowed = 'url should be allowed'; +const urlNotAllowed = 'url should NOT be allowed'; const urlToTest = 'http://myaddress.com/somepath'; @@ -85,4 +87,35 @@ describe('Core - Utils - url', () => { }); }); }); + + describe('isUrlAllowed', () => { + describe('when allowed urls are undefined', () => { + it('should return true', () => { + assert.strictEqual(isUrlAllowed(urlToTest), true, urlAllowed); + }); + }); + describe('when allowed urls are empty', () => { + it('should return false', () => { + assert.strictEqual(isUrlAllowed(urlToTest, []), false, urlAllowed); + }); + }); + describe('when allowed urls contains test url', () => { + it('should return true', () => { + assert.strictEqual( + isUrlAllowed(urlToTest, ['http://myaddress.com/somepath']), + true, + urlAllowed + ); + }); + }); + describe('when allowed urls does NOT contain test url', () => { + it('should return false', () => { + assert.strictEqual( + isUrlAllowed(urlToTest, ['http://myaddress.com/other']), + false, + urlNotAllowed + ); + }); + }); + }); }); From 26302734647fc5fbcc0f83348921750440c1b61c Mon Sep 17 00:00:00 2001 From: Jairo Date: Mon, 2 Dec 2024 15:59:08 -0800 Subject: [PATCH 11/16] Remove isUrlAllowed from core package --- packages/opentelemetry-core/src/index.ts | 2 +- packages/opentelemetry-core/src/utils/url.ts | 20 ----------- .../opentelemetry-core/test/utils/url.test.ts | 35 +------------------ 3 files changed, 2 insertions(+), 55 deletions(-) diff --git a/packages/opentelemetry-core/src/index.ts b/packages/opentelemetry-core/src/index.ts index 8e246cc6d9c..0c2dc7c1a8d 100644 --- a/packages/opentelemetry-core/src/index.ts +++ b/packages/opentelemetry-core/src/index.ts @@ -112,7 +112,7 @@ export { export { merge } from './utils/merge'; export { TracesSamplerValues } from './utils/sampling'; export { TimeoutError, callWithTimeout } from './utils/timeout'; -export { isUrlAllowed, isUrlIgnored, urlMatches } from './utils/url'; +export { isUrlIgnored, urlMatches } from './utils/url'; export { isWrapped } from './utils/wrap'; export { BindOnceFuture } from './utils/callback'; export { VERSION } from './version'; diff --git a/packages/opentelemetry-core/src/utils/url.ts b/packages/opentelemetry-core/src/utils/url.ts index 35214cf6488..a6122ae7842 100644 --- a/packages/opentelemetry-core/src/utils/url.ts +++ b/packages/opentelemetry-core/src/utils/url.ts @@ -20,26 +20,6 @@ export function urlMatches(url: string, urlToMatch: string | RegExp): boolean { return !!url.match(urlToMatch); } } -/** - * Check if {@param url} should be allowed when comparing against {@param allowedUrls} - * @param url - * @param allowedUrls - */ -export function isUrlAllowed( - url: string, - allowedUrls?: Array -): boolean { - if (!allowedUrls) { - return true; - } - - for (const allowedUrl of allowedUrls) { - if (urlMatches(url, allowedUrl)) { - return true; - } - } - return false; -} /** * Check if {@param url} should be ignored when comparing against {@param ignoredUrls} * @param url diff --git a/packages/opentelemetry-core/test/utils/url.test.ts b/packages/opentelemetry-core/test/utils/url.test.ts index 49193533a2f..8855cff23c3 100644 --- a/packages/opentelemetry-core/test/utils/url.test.ts +++ b/packages/opentelemetry-core/test/utils/url.test.ts @@ -16,12 +16,10 @@ import * as assert from 'assert'; -import { isUrlAllowed, isUrlIgnored } from '../../src'; +import { isUrlIgnored } from '../../src'; const urlIgnored = 'url should be ignored'; const urlNotIgnored = 'url should NOT be ignored'; -const urlAllowed = 'url should be allowed'; -const urlNotAllowed = 'url should NOT be allowed'; const urlToTest = 'http://myaddress.com/somepath'; @@ -87,35 +85,4 @@ describe('Core - Utils - url', () => { }); }); }); - - describe('isUrlAllowed', () => { - describe('when allowed urls are undefined', () => { - it('should return true', () => { - assert.strictEqual(isUrlAllowed(urlToTest), true, urlAllowed); - }); - }); - describe('when allowed urls are empty', () => { - it('should return false', () => { - assert.strictEqual(isUrlAllowed(urlToTest, []), false, urlAllowed); - }); - }); - describe('when allowed urls contains test url', () => { - it('should return true', () => { - assert.strictEqual( - isUrlAllowed(urlToTest, ['http://myaddress.com/somepath']), - true, - urlAllowed - ); - }); - }); - describe('when allowed urls does NOT contain test url', () => { - it('should return false', () => { - assert.strictEqual( - isUrlAllowed(urlToTest, ['http://myaddress.com/other']), - false, - urlNotAllowed - ); - }); - }); - }); }); From 618e6f688ac998e03c4b5d39058036976a68700a Mon Sep 17 00:00:00 2001 From: Jairo Date: Mon, 2 Dec 2024 16:12:30 -0800 Subject: [PATCH 12/16] Update fetch instr with private isUrlAllowed --- .../src/fetch.ts | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts index 96bd28ca484..10fe8fed6ea 100644 --- a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts +++ b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts @@ -197,6 +197,24 @@ export class FetchInstrumentation extends InstrumentationBase | undefined): boolean { + if (!allowedUrls) { + return true; + } + + for (const allowedUrl of allowedUrls) { + if (core.urlMatches(url, allowedUrl)) { + return true; + } + } + return false; + } + /** * Creates a new span * @param url @@ -206,7 +224,7 @@ export class FetchInstrumentation extends InstrumentationBase = {} ): api.Span | undefined { - if (!core.isUrlAllowed(url, this.getConfig().allowUrls)) { + if (!this._isUrlAllowed(url, this.getConfig().allowUrls)) { this._diag.debug('ignoring span as url does not match an allowed url'); return; } From fbd03f07f3f19a93cebbcc01497e9b6b131c460f Mon Sep 17 00:00:00 2001 From: Jairo Date: Mon, 2 Dec 2024 16:27:37 -0800 Subject: [PATCH 13/16] Update xhr instr with private isUrlAllowed --- .../src/fetch.ts | 7 ++++-- .../src/xhr.ts | 25 +++++++++++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts index 10fe8fed6ea..a82be375ef7 100644 --- a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts +++ b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts @@ -202,11 +202,14 @@ export class FetchInstrumentation extends InstrumentationBase | undefined): boolean { + private _isUrlAllowed( + url: string, + allowedUrls: Array | undefined + ): boolean { if (!allowedUrls) { return true; } - + for (const allowedUrl of allowedUrls) { if (core.urlMatches(url, allowedUrl)) { return true; diff --git a/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts b/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts index 1bfe1639b04..a0facb489f1 100644 --- a/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts +++ b/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts @@ -23,9 +23,9 @@ import { } from '@opentelemetry/instrumentation'; import { hrTime, - isUrlAllowed, isUrlIgnored, otperformance, + urlMatches, } from '@opentelemetry/core'; import { SEMATTRS_HTTP_HOST, @@ -329,6 +329,27 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase | undefined + ): boolean { + if (!allowedUrls) { + return true; + } + + for (const allowedUrl of allowedUrls) { + if (urlMatches(url, allowedUrl)) { + return true; + } + } + return false; + } + /** * Creates a new span when method "open" is called * @param xhr @@ -341,7 +362,7 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase Date: Thu, 5 Dec 2024 07:22:03 -0800 Subject: [PATCH 14/16] Remove changes from regular changelog --- CHANGELOG.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f37f72c03d..2f378c59c12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,10 +13,6 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se ### :rocket: (Enhancement) -* feat(instrumentation): Add allowUrls config option to web instrumentation [#4938](https://github.com/open-telemetry/opentelemetry-js/pull/4938) @jairo-mendoza - * `XMLHttpRequestInstrumentation` and `FetchInstrumentation` now accept an `allowUrls` config option. - * `isUrlAllowed` function added to core. Both web instrumentation classes will depend on this function when using the new config option. - ### :bug: (Bug Fix) * fix(sdk-trace-base): do not load OTEL_ env vars on module load, but when needed [#5224](https://github.com/open-telemetry/opentelemetry-js/pull/5224) From 8daed5f28df0d21429c9ed156b071ac17ebdd385 Mon Sep 17 00:00:00 2001 From: Jairo Date: Thu, 5 Dec 2024 07:23:38 -0800 Subject: [PATCH 15/16] Move change to unreleased --- experimental/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 9a14396e18d..5803b1fb728 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -9,6 +9,8 @@ All notable changes to experimental packages in this project will be documented ### :rocket: (Enhancement) +* feat(instrumentation): Add allowUrls config option to web instrumentation [#4938](https://github.com/open-telemetry/opentelemetry-js/pull/4938) @jairo-mendoza + ### :bug: (Bug Fix) ### :books: (Refine Doc) @@ -88,7 +90,6 @@ All notable changes to experimental packages in this project will be documented ### :rocket: (Enhancement) -* feat(instrumentation): Add allowUrls config option to web instrumentation [#4938](https://github.com/open-telemetry/opentelemetry-js/pull/4938) @jairo-mendoza * feat(otlp-exporter-base): handle OTLP partial success [#5183](https://github.com/open-telemetry/opentelemetry-js/pull/5183) @pichlermarc * feat(otlp-exporter-base): internally accept a http header provider function only [#5179](https://github.com/open-telemetry/opentelemetry-js/pull/5179) @pichlermarc * refactor(otlp-exporter-base): don't create blob before sending xhr [#5193](https://github.com/open-telemetry/opentelemetry-js/pull/5193) @pichlermarc From 2c3481a533256a7185b359c2666bb389698f817c Mon Sep 17 00:00:00 2001 From: Jairo Date: Thu, 5 Dec 2024 07:34:17 -0800 Subject: [PATCH 16/16] Fix missing args in test --- .../opentelemetry-instrumentation-fetch/test/fetch.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts b/experimental/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts index 2bf22336719..794f173c42d 100644 --- a/experimental/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts @@ -967,7 +967,7 @@ describe('fetch', () => { describe('when url is allowed', () => { beforeEach(async () => { const propagateTraceHeaderCorsUrls = url; - await prepareData(url, { + await prepareData(url, () => getData(url), { propagateTraceHeaderCorsUrls, allowUrls: [propagateTraceHeaderCorsUrls], }); @@ -985,7 +985,7 @@ describe('fetch', () => { beforeEach(async () => { const propagateTraceHeaderCorsUrls = url; - await prepareData(url, { + await prepareData(url, () => getData(url), { propagateTraceHeaderCorsUrls, allowUrls: [otherUrl], });