fix(xhr): make performance observer work with relative urls (#2226)

Co-authored-by: Daniel Dyla <[email protected]> Co-authored-by: Bartlomiej Obecny <[email protected]> Co-authored-by: Valentin Marchaud <[email protected]>
open-telemetry · Jun 23, 2021 · 6fb4fd1 · 6fb4fd1
1 parent bd2a005
commit 6fb4fd1
Show file tree

Hide file tree

Showing 5 changed files with 77 additions and 28 deletions.
diff --git 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
@@ -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
@@ -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<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);

diff --git 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
@@ -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) {