From b77b1c7530d612110869e391d9789062e14c375b Mon Sep 17 00:00:00 2001 From: Marten Hennoch Date: Tue, 25 May 2021 18:02:37 +0300 Subject: [PATCH 1/3] fix(xhr): make performance observer work with relative urls and minor fixes --- packages/opentelemetry-core/src/utils/url.ts | 2 +- .../src/xhr.ts | 15 +++- .../test/xhr.test.ts | 79 ++++++++++++++++--- packages/opentelemetry-web/src/utils.ts | 2 +- 4 files changed, 83 insertions(+), 15 deletions(-) diff --git a/packages/opentelemetry-core/src/utils/url.ts b/packages/opentelemetry-core/src/utils/url.ts index a6122ae7842..9db9725b823 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-xml-http-request/src/xhr.ts b/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts index fb7274cdcea..dacd4a0f51c 100644 --- a/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts +++ b/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts @@ -46,6 +46,16 @@ import { AttributeNames } from './enums/AttributeNames'; // 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 type XHRCustomAttributeFunction = ( span: api.Span, xhr: XMLHttpRequest @@ -216,10 +226,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 fa8bb22469a..2133373d9b6 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.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.setSpan(api.context.active(), rootSpan), () => { - getData( + void getData( reusableReq, firstUrl, () => { @@ -665,7 +691,7 @@ describe('xhr', () => { api.context.with( api.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 af9d15c9d38..4bc2b9204c3 100644 --- a/packages/opentelemetry-web/src/utils.ts +++ b/packages/opentelemetry-web/src/utils.ts @@ -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) { From 4ac2086d56c16f72403b03d7caaf72d04dbbd44f Mon Sep 17 00:00:00 2001 From: Marten Hennoch Date: Wed, 26 May 2021 13:42:40 +0300 Subject: [PATCH 2/3] chore: export getUrlNormalizingAnchor so it can be reused --- packages/opentelemetry-web/src/utils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/opentelemetry-web/src/utils.ts b/packages/opentelemetry-web/src/utils.ts index 4bc2b9204c3..ccfb34d7d8a 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 From 1bce44d9f96665e5ca5bff41565572b716386874 Mon Sep 17 00:00:00 2001 From: Marten Hennoch Date: Wed, 26 May 2021 23:37:19 +0300 Subject: [PATCH 3/3] chore: reuse getUrlNormalizingAnchor --- .../opentelemetry-instrumentation-fetch/src/fetch.ts | 12 +----------- .../src/xhr.ts | 11 +---------- 2 files changed, 2 insertions(+), 21 deletions(-) diff --git a/packages/opentelemetry-instrumentation-fetch/src/fetch.ts b/packages/opentelemetry-instrumentation-fetch/src/fetch.ts index ec6cae7cf48..dde243a92af 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 dacd4a0f51c..e00cf0c21a5 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 { @@ -46,16 +47,6 @@ import { AttributeNames } from './enums/AttributeNames'; // 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 type XHRCustomAttributeFunction = ( span: api.Span, xhr: XMLHttpRequest