From 6fb4fd1712babdbb5de216c26e1283a94f7159ea Mon Sep 17 00:00:00 2001 From: MartenH <72463136+mhennoch@users.noreply.github.com> Date: Wed, 23 Jun 2021 19:01:48 +0300 Subject: [PATCH] fix(xhr): make performance observer work with relative urls (#2226) Co-authored-by: Daniel Dyla Co-authored-by: Bartlomiej Obecny Co-authored-by: Valentin Marchaud --- packages/opentelemetry-core/src/utils/url.ts | 2 +- .../src/fetch.ts | 12 +-- .../src/xhr.ts | 6 +- .../test/xhr.test.ts | 79 ++++++++++++++++--- packages/opentelemetry-web/src/utils.ts | 6 +- 5 files changed, 77 insertions(+), 28 deletions(-) diff --git a/packages/opentelemetry-core/src/utils/url.ts b/packages/opentelemetry-core/src/utils/url.ts index a6122ae784..9db9725b82 100644 --- a/packages/opentelemetry-core/src/utils/url.ts +++ b/packages/opentelemetry-core/src/utils/url.ts @@ -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); } } /** diff --git a/packages/opentelemetry-instrumentation-fetch/src/fetch.ts b/packages/opentelemetry-instrumentation-fetch/src/fetch.ts index a8860947ba..5d107027d7 100644 --- a/packages/opentelemetry-instrumentation-fetch/src/fetch.ts +++ b/packages/opentelemetry-instrumentation-fetch/src/fetch.ts @@ -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, @@ -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 ( diff --git a/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts b/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts index 1b5ade2660..eabab83d1c 100644 --- a/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts +++ b/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts @@ -29,6 +29,7 @@ import { parseUrl, PerformanceTimingNames as PTN, shouldPropagateTraceHeaders, + getUrlNormalizingAnchor } from '@opentelemetry/web'; import { EventNames } from './enums/EventNames'; import { @@ -216,10 +217,13 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase { 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); diff --git a/packages/opentelemetry-instrumentation-xml-http-request/test/xhr.test.ts b/packages/opentelemetry-instrumentation-xml-http-request/test/xhr.test.ts index 624ec33671..87896e8437 100644 --- a/packages/opentelemetry-instrumentation-xml-http-request/test/xhr.test.ts +++ b/packages/opentelemetry-instrumentation-xml-http-request/test/xhr.test.ts @@ -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 => { @@ -200,6 +230,11 @@ describe('xhr', () => { 'getEntriesByType' ); spyEntries.withArgs('resource').returns(resources); + + sinon + .stub(window, 'PerformanceObserver') + .value(createFakePerformanceObs(fileUrl)); + xmlHttpRequestInstrumentation = new XMLHttpRequestInstrumentation( config ); @@ -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, () => { @@ -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, () => { @@ -665,7 +691,7 @@ describe('xhr', () => { api.context.with( api.trace.setSpan(api.context.active(), rootSpan), () => { - getData( + void getData( reusableReq, secondUrl, () => { @@ -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', () => { diff --git a/packages/opentelemetry-web/src/utils.ts b/packages/opentelemetry-web/src/utils.ts index af9d15c9d3..ccfb34d7d8 100644 --- a/packages/opentelemetry-web/src/utils.ts +++ b/packages/opentelemetry-web/src/utils.ts @@ -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 @@ -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) {