Allow zero/negative performance timings (#1769)

Co-authored-by: Daniel Dyla <[email protected]>
open-telemetry · Jan 20, 2021 · ee014c9 · ee014c9
1 parent 6b1285c
commit ee014c9
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 28 deletions.
diff --git a/packages/opentelemetry-web/src/enums/PerformanceTimingNames.ts b/packages/opentelemetry-web/src/enums/PerformanceTimingNames.ts
@@ -27,6 +27,7 @@ export enum PerformanceTimingNames {
   FETCH_START = 'fetchStart',
   LOAD_EVENT_END = 'loadEventEnd',
   LOAD_EVENT_START = 'loadEventStart',
+  NAVIGATION_START = 'navigationStart',
   REDIRECT_END = 'redirectEnd',
   REDIRECT_START = 'redirectStart',
   REQUEST_START = 'requestStart',

diff --git a/packages/opentelemetry-web/src/utils.ts b/packages/opentelemetry-web/src/utils.ts
@@ -56,11 +56,6 @@ export function addSpanNetworkEvent(
     hasKey(entries, performanceName) &&
     typeof entries[performanceName] === 'number'
   ) {
-    // some metrics are available but have value 0 which means they are invalid
-    // for example "secureConnectionStart" is 0 which makes the events to be wrongly interpreted
-    if (entries[performanceName] === 0) {
-      return undefined;
-    }
     span.addEvent(performanceName, entries[performanceName]);
     return span;
   }

diff --git a/packages/opentelemetry-web/test/utils.test.ts b/packages/opentelemetry-web/test/utils.test.ts
@@ -164,40 +164,46 @@ describe('utils', () => {
     });
   });
   describe('addSpanNetworkEvent', () => {
-    describe('when entries contain the performance', () => {
-      it('should add event to span', () => {
-        const addEventSpy = sinon.spy();
-        const span = ({
-          addEvent: addEventSpy,
-        } as unknown) as tracing.Span;
-        const entries = {
-          [PTN.FETCH_START]: 123,
-        } as PerformanceEntries;
-
-        assert.strictEqual(addEventSpy.callCount, 0);
-
-        addSpanNetworkEvent(span, PTN.FETCH_START, entries);
-
-        assert.strictEqual(addEventSpy.callCount, 1);
-        const args = addEventSpy.args[0];
-
-        assert.strictEqual(args[0], 'fetchStart');
-        assert.strictEqual(args[1], 123);
+    [0, -2, 123].forEach(value => {
+      describe(`when entry is ${value}`, () => {
+        it('should add event to span', () => {
+          const addEventSpy = sinon.spy();
+          const span = ({
+            addEvent: addEventSpy,
+          } as unknown) as tracing.Span;
+          const entries = {
+            [PTN.FETCH_START]: value,
+          } as PerformanceEntries;
+
+          assert.strictEqual(addEventSpy.callCount, 0);
+
+          addSpanNetworkEvent(span, PTN.FETCH_START, entries);
+
+          assert.strictEqual(addEventSpy.callCount, 1);
+          const args = addEventSpy.args[0];
+
+          assert.strictEqual(args[0], 'fetchStart');
+          assert.strictEqual(args[1], value);
+        });
       });
     });
-    describe('when entry has time equal to 0', () => {
+    describe('when entry is not numeric', () => {
       it('should NOT add event to span', () => {
         const addEventSpy = sinon.spy();
         const span = ({
           addEvent: addEventSpy,
         } as unknown) as tracing.Span;
         const entries = {
-          [PTN.FETCH_START]: 0,
-        } as PerformanceEntries;
+          [PTN.FETCH_START]: 'non-numeric',
+        } as unknown;
 
         assert.strictEqual(addEventSpy.callCount, 0);
 
-        addSpanNetworkEvent(span, PTN.FETCH_START, entries);
+        addSpanNetworkEvent(
+          span,
+          PTN.FETCH_START,
+          entries as PerformanceEntries
+        );
 
         assert.strictEqual(addEventSpy.callCount, 0);
       });