Merge pull request #330 from mhennoch/normalize-timings

open-telemetry · Feb 9, 2021 · 24b346d · 24b346d
2 parents dbbf02f + 4045ea6
commit 24b346d
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 28 deletions.
diff --git a/plugins/web/opentelemetry-plugin-document-load/src/documentLoad.ts b/plugins/web/opentelemetry-plugin-document-load/src/documentLoad.ts
@@ -173,7 +173,7 @@ export class DocumentLoad extends BasePlugin<unknown> {
       keys.forEach((key: string) => {
         if (hasKey(performanceNavigationTiming, key)) {
           const value = performanceNavigationTiming[key];
-          if (typeof value === 'number' && value >= 0) {
+          if (typeof value === 'number') {
             entries[key] = value;
           }
         }
@@ -187,7 +187,7 @@ export class DocumentLoad extends BasePlugin<unknown> {
         keys.forEach((key: string) => {
           if (hasKey(performanceTiming, key)) {
             const value = performanceTiming[key];
-            if (typeof value === 'number' && value >= 0) {
+            if (typeof value === 'number') {
               entries[key] = value;
             }
           }

diff --git a/plugins/web/opentelemetry-plugin-document-load/test/documentLoad.test.ts b/plugins/web/opentelemetry-plugin-document-load/test/documentLoad.test.ts
@@ -512,6 +512,12 @@ describe('DocumentLoad Plugin', () => {
       const spyOnEnd = sinon.spy(dummyExporter, 'export');
       plugin.enable(moduleExports, provider, logger, config);
       setTimeout(() => {
+        assert.strictEqual(
+          spyOnEnd.callCount,
+          2,
+          `Unexpected number of spans: ${spyOnEnd.callCount}`
+        );
+
         const rootSpan = spyOnEnd.args[0][0][0] as ReadableSpan;
         const fetchSpan = spyOnEnd.args[1][0][0] as ReadableSpan;
         const rsEvents = rootSpan.events;
@@ -537,60 +543,51 @@ describe('DocumentLoad Plugin', () => {
 
         assert.strictEqual(rsEvents.length, 9);
         assert.strictEqual(fsEvents.length, 9);
-        assert.strictEqual(spyOnEnd.callCount, 2);
         done();
       });
     });
   }
 
   describe('when navigation entries types are NOT available then fallback to "performance.timing"', () => {
-    let spyEntries: any;
+    const sandbox = sinon.createSandbox();
     beforeEach(() => {
-      spyEntries = sinon.stub(window.performance, 'getEntriesByType');
-      spyEntries.withArgs('navigation').returns([]);
-      spyEntries.withArgs('resource').returns([]);
-
-      Object.defineProperty(window.performance, 'timing', {
-        writable: true,
-        value: entriesFallback,
+      sandbox.stub(window.performance, 'getEntriesByType').value(undefined);
+      sandbox.stub(window.performance, 'timing').get(() => {
+        return entriesFallback;
       });
     });
     afterEach(() => {
-      spyEntries.restore();
+      sandbox.restore();
     });
 
     shouldExportCorrectSpan();
   });
 
   describe('when getEntriesByType is not defined then fallback to "performance.timing"', () => {
+    const sandbox = sinon.createSandbox();
     beforeEach(() => {
-      Object.defineProperty(window.performance, 'getEntriesByType', {
-        writable: true,
-        value: undefined,
-      });
-
-      Object.defineProperty(window.performance, 'timing', {
-        writable: true,
-        value: entriesFallback,
+      sandbox.stub(window.performance, 'getEntriesByType').value(undefined);
+      sandbox.stub(window.performance, 'timing').get(() => {
+        return entriesFallback;
       });
     });
+    afterEach(() => {
+      sandbox.restore();
+    });
 
     shouldExportCorrectSpan();
   });
 
   describe('when navigation entries types and "performance.timing" are NOT available', () => {
-    let spyEntries: any;
+    const sandbox = sinon.createSandbox();
     beforeEach(() => {
-      Object.defineProperty(window.performance, 'timing', {
-        writable: true,
-        value: undefined,
+      sandbox.stub(window.performance, 'getEntriesByType').value(undefined);
+      sandbox.stub(window.performance, 'timing').get(() => {
+        return undefined;
       });
-      spyEntries = sinon.stub(window.performance, 'getEntriesByType');
-      spyEntries.withArgs('navigation').returns([]);
-      spyEntries.withArgs('resource').returns([]);
     });
     afterEach(() => {
-      spyEntries.restore();
+      sandbox.restore();
     });
 
     it('should not create any span', done => {
@@ -602,4 +599,27 @@ describe('DocumentLoad Plugin', () => {
       });
     });
   });
+
+  describe('when fetchStart is negative still create spans', () => {
+    const sandbox = sinon.createSandbox();
+    beforeEach(() => {
+      const navEntriesWithNegativeFetch = Object.assign({}, entries, {
+        fetchStart: -1,
+      });
+      sandbox
+        .stub(window.performance, 'getEntriesByType')
+        .withArgs('navigation')
+        .returns([navEntriesWithNegativeFetch])
+        .withArgs('resource')
+        .returns([]);
+
+      sandbox.stub(window.performance, 'timing').get(() => {
+        return undefined;
+      });
+    });
+    afterEach(() => {
+      sandbox.restore();
+    });
+    shouldExportCorrectSpan();
+  });
 });