Skip to content

Commit

Permalink
fix(xhr): make performance observer work with relative urls (#2226)
Browse files Browse the repository at this point in the history
Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com>
Co-authored-by: Bartlomiej Obecny <bobecny@gmail.com>
Co-authored-by: Valentin Marchaud <contact@vmarchaud.fr>
  • Loading branch information
4 people committed Jun 23, 2021
1 parent bd2a005 commit 6fb4fd1
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 28 deletions.
2 changes: 1 addition & 1 deletion packages/opentelemetry-core/src/utils/url.ts
Expand Up @@ -17,7 +17,7 @@ export function urlMatches(url: string, urlToMatch: string | RegExp): boolean {
if (typeof urlToMatch === 'string') {
return url === urlToMatch;
} else {
return !!url.match(urlToMatch);
return urlToMatch.test(url);
}
}
/**
Expand Down
12 changes: 1 addition & 11 deletions packages/opentelemetry-instrumentation-fetch/src/fetch.ts
Expand Up @@ -34,16 +34,6 @@ import { VERSION } from './version';
// safe enough
const OBSERVER_WAIT_TIME_MS = 300;

// Used to normalize relative URLs
let a: HTMLAnchorElement | undefined;
const getUrlNormalizingAnchor = () => {
if (!a) {
a = document.createElement('a');
}

return a;
};

export interface FetchCustomAttributeFunction {
(
span: api.Span,
Expand Down Expand Up @@ -438,7 +428,7 @@ export class FetchInstrumentation extends InstrumentationBase<

const observer: PerformanceObserver = new PerformanceObserver(list => {
const perfObsEntries = list.getEntries() as PerformanceResourceTiming[];
const urlNormalizingAnchor = getUrlNormalizingAnchor();
const urlNormalizingAnchor = web.getUrlNormalizingAnchor();
urlNormalizingAnchor.href = spanUrl;
perfObsEntries.forEach(entry => {
if (
Expand Down
Expand Up @@ -29,6 +29,7 @@ import {
parseUrl,
PerformanceTimingNames as PTN,
shouldPropagateTraceHeaders,
getUrlNormalizingAnchor
} from '@opentelemetry/web';
import { EventNames } from './enums/EventNames';
import {
Expand Down Expand Up @@ -216,10 +217,13 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase<XMLHttpRe
xhrMem.createdResources = {
observer: new PerformanceObserver(list => {
const entries = list.getEntries() as PerformanceResourceTiming[];
const urlNormalizingAnchor = getUrlNormalizingAnchor();
urlNormalizingAnchor.href = spanUrl;

entries.forEach(entry => {
if (
entry.initiatorType === 'xmlhttprequest' &&
entry.name === spanUrl
entry.name === urlNormalizingAnchor.href
) {
if (xhrMem.createdResources) {
xhrMem.createdResources.entries.push(entry);
Expand Down
Expand Up @@ -125,6 +125,36 @@ function createMainResource(resource = {}): PerformanceResourceTiming {
return mainResource;
}

function createFakePerformanceObs(url: string) {
class FakePerfObs implements PerformanceObserver {
constructor(private readonly cb: PerformanceObserverCallback) {}
observe() {
const absoluteUrl = url.startsWith('http') ? url : location.origin + url;
const resources: PerformanceObserverEntryList = {
getEntries(): PerformanceEntryList {
return [
createResource({ name: absoluteUrl }) as any,
createMainResource({ name: absoluteUrl }) as any,
];
},
getEntriesByName(): PerformanceEntryList {
return [];
},
getEntriesByType(): PerformanceEntryList {
return [];
},
};
this.cb(resources, this);
}
disconnect() {}
takeRecords(): PerformanceEntryList {
return [];
}
}

return FakePerfObs;
}

describe('xhr', () => {
const asyncTests = [{ async: true }, { async: false }];
asyncTests.forEach(test => {
Expand Down Expand Up @@ -200,6 +230,11 @@ describe('xhr', () => {
'getEntriesByType'
);
spyEntries.withArgs('resource').returns(resources);

sinon
.stub(window, 'PerformanceObserver')
.value(createFakePerformanceObs(fileUrl));

xmlHttpRequestInstrumentation = new XMLHttpRequestInstrumentation(
config
);
Expand All @@ -221,7 +256,7 @@ describe('xhr', () => {

rootSpan = webTracerWithZone.startSpan('root');
api.context.with(api.trace.setSpan(api.context.active(), rootSpan), () => {
getData(
void getData(
new XMLHttpRequest(),
fileUrl,
() => {
Expand Down Expand Up @@ -635,20 +670,11 @@ describe('xhr', () => {

beforeEach(done => {
requests = [];
const resources: PerformanceResourceTiming[] = [];
resources.push(
createResource({
name: firstUrl,
}),
createResource({
name: secondUrl,
})
);
const reusableReq = new XMLHttpRequest();
api.context.with(
api.trace.setSpan(api.context.active(), rootSpan),
() => {
getData(
void getData(
reusableReq,
firstUrl,
() => {
Expand All @@ -665,7 +691,7 @@ describe('xhr', () => {
api.context.with(
api.trace.setSpan(api.context.active(), rootSpan),
() => {
getData(
void getData(
reusableReq,
secondUrl,
() => {
Expand Down Expand Up @@ -728,6 +754,35 @@ describe('xhr', () => {
assert.ok(attributes['xhr-custom-attribute'] === 'bar');
});
});

describe('when using relative url', () => {
beforeEach(done => {
clearData();
const propagateTraceHeaderCorsUrls = [window.location.origin];
prepareData(done, '/get', { propagateTraceHeaderCorsUrls });
});

it('should create correct span with events', () => {
// no prefetch span because mock observer uses location.origin as url when relative
// and prefetch span finding compares url origins
const span: tracing.ReadableSpan = exportSpy.args[0][0][0];
const events = span.events;

assert.strictEqual(
exportSpy.args.length,
1,
`Wrong number of spans: ${exportSpy.args.length}`
);

assert.strictEqual(events.length, 12, `number of events is wrong: ${events.length}`);
assert.strictEqual(
events[8].name,
PTN.REQUEST_START,
`event ${PTN.REQUEST_START} is not defined`
);
});
});

});

describe('when request is NOT successful', () => {
Expand Down
6 changes: 3 additions & 3 deletions packages/opentelemetry-web/src/utils.ts
Expand Up @@ -30,13 +30,13 @@ import { SemanticAttributes } from '@opentelemetry/semantic-conventions';

// Used to normalize relative URLs
let a: HTMLAnchorElement | undefined;
const getUrlNormalizingAnchor = () => {
export function getUrlNormalizingAnchor(): HTMLAnchorElement {
if (!a) {
a = document.createElement('a');
}

return a;
};
}

/**
* Helper function to be able to use enum as typed key in type and in interface when using forEach
Expand Down Expand Up @@ -155,7 +155,7 @@ export function getResource(
mainRequest: filteredResources[0],
};
}
const sorted = sortResources(filteredResources.slice());
const sorted = sortResources(filteredResources);

const parsedSpanUrl = parseUrl(spanUrl);
if (parsedSpanUrl.origin !== window.location.origin && sorted.length > 1) {
Expand Down

0 comments on commit 6fb4fd1

Please sign in to comment.