Skip to content

feat(instrumentation): Add allowUrls config option to web instrumentation #4938

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
da710c9
Add allowUrls config option to Fetch Instrumentation
jairo-mendoza Aug 21, 2024
8496c18
Clean up code and add comments
jairo-mendoza Aug 21, 2024
6edaa15
Add allowUrls config option to xhr instrumentation
jairo-mendoza Aug 21, 2024
467acda
Add unit tests
jairo-mendoza Aug 21, 2024
43ba3fd
Merge branch 'main' into jairo.allowUrls-config-option
jairo-mendoza Aug 22, 2024
d7a7375
Add entry to experimental changelog
jairo-mendoza Aug 22, 2024
9ea1fe7
Merge branch 'main' into jairo.allowUrls-config-option
jairo-mendoza Sep 3, 2024
f45b61e
Call out dependence on new function
jairo-mendoza Sep 3, 2024
9050429
Merge branch 'main' into jairo.allowUrls-config-option
jairo-mendoza Sep 10, 2024
8321c02
Fix merge conflicts
jairo-mendoza Sep 24, 2024
c3ad925
Fix merge conflicts
jairo-mendoza Nov 5, 2024
e819c8d
Fix up experimental changelog
jairo-mendoza Nov 5, 2024
bbf05f2
Add newline to changelog
jairo-mendoza Nov 6, 2024
1045036
Merge branch 'open-telemetry:main' into jairo.allowUrls-config-option
jairo-mendoza Nov 6, 2024
e9b7e2c
Update main changelog
jairo-mendoza Nov 6, 2024
4962272
Increase test coverage for isUrlAllowed
jairo-mendoza Nov 6, 2024
2630273
Remove isUrlAllowed from core package
jairo-mendoza Dec 2, 2024
618e6f6
Update fetch instr with private isUrlAllowed
jairo-mendoza Dec 3, 2024
fbd03f0
Update xhr instr with private isUrlAllowed
jairo-mendoza Dec 3, 2024
5d396e8
Fix merge conflicts
jairo-mendoza Dec 3, 2024
8ea60cc
Merge branch 'main' into jairo.allowUrls-config-option
pichlermarc Dec 5, 2024
c797fb6
Remove changes from regular changelog
jairo-mendoza Dec 5, 2024
8daed5f
Move change to unreleased
jairo-mendoza Dec 5, 2024
2c3481a
Fix missing args in test
jairo-mendoza Dec 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ 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)

Expand Down
1 change: 1 addition & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ 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<string | RegExp>;
/**
* URLs that partially match any regex in ignoreUrls will not be traced.
* In addition, URLs that are _exact matches_ of strings in ignoreUrls will
Expand Down Expand Up @@ -201,6 +206,10 @@ export class FetchInstrumentation extends InstrumentationBase<FetchInstrumentati
url: string,
options: Partial<Request | RequestInit> = {}
): api.Span | undefined {
if (!core.isUrlAllowed(url, this.getConfig().allowUrls)) {
this._diag.debug('ignoring span 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');
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<string | RegExp>;
/**
* URLs that partially match any regex in ignoreUrls will not be traced.
* In addition, URLs that are _exact matches_ of strings in ignoreUrls will
Expand Down Expand Up @@ -331,6 +341,10 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase<XMLHttpRe
url: string,
method: string
): api.Span | undefined {
if (!isUrlAllowed(url, this.getConfig().allowUrls)) {
this._diag.debug('ignoring span as url does not match an allowed url');
return;
}
if (isUrlIgnored(url, this.getConfig().ignoreUrls)) {
this._diag.debug('ignoring span as url matches ignored url');
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion packages/opentelemetry-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
20 changes: 20 additions & 0 deletions packages/opentelemetry-core/src/utils/url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,29 @@
if (typeof urlToMatch === 'string') {
return url === urlToMatch;
} else {
return !!url.match(urlToMatch);

Check failure

Code scanning / CodeQL

Polynomial regular expression used on uncontrolled data High

This
regular expression
that depends on
library input
may run slow on strings with many repetitions of 'a'.
This
regular expression
that depends on
library input
may run slow on strings with many repetitions of 'a'.
This
regular expression
that depends on
library input
may run slow on strings with many repetitions of 'a'.
This
regular expression
that depends on
library input
may run slow on strings with many repetitions of 'a'.
}
}
/**
* Check if {@param url} should be allowed when comparing against {@param allowedUrls}
* @param url
* @param allowedUrls
*/
export function isUrlAllowed(
url: string,
allowedUrls?: Array<string | RegExp>
): 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
Expand Down
Loading