Fix invalid timings in span events

CHANGELOG.md

@@ -30,6 +30,7 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/
 
 ### :bug: (Bug Fix)
 
+* fix(sdk-trace-web): fix invalid timings in span events [#4486](https://github.com/open-telemetry/opentelemetry-js/pull/4486) @Abinet18
 * fix(sdk-metrics): handle zero bucket counts in exponential histogram merge [#4459](https://github.com/open-telemetry/opentelemetry-js/pull/4459) @mwear
 * fix(sdk-metrics): ignore `NaN` value recordings in Histograms [#4455](https://github.com/open-telemetry/opentelemetry-js/pull/4455) @pichlermarc
   * fixes a bug where recording `NaN` on a histogram would result in the sum of bucket count values not matching the overall count

packages/opentelemetry-sdk-trace-web/src/utils.ts

@@ -60,13 +60,25 @@ export function hasKey<O extends object>(
 export function addSpanNetworkEvent(
   span: api.Span,
   performanceName: string,
-  entries: PerformanceEntries
+  entries: PerformanceEntries,
+  refPerfName?: string
 ): api.Span | undefined {
+  let perfTime = undefined;
+  let refTime = undefined;
   if (
     hasKey(entries, performanceName) &&
     typeof entries[performanceName] === 'number'
   ) {
-    span.addEvent(performanceName, entries[performanceName]);
+    perfTime = entries[performanceName];
+  }
+  const refName = refPerfName || PTN.FETCH_START;
+  // Use a reference time which is the earliest possible value so that the performance timings that are earlier should not be added
+  // using FETCH START time in case no reference is provided
+  if (hasKey(entries, refName) && typeof entries[refName] === 'number') {
+    refTime = entries[refName];
+  }
+  if (perfTime !== undefined && refTime !== undefined && perfTime >= refTime) {
+    span.addEvent(performanceName, perfTime);
     return span;
   }
   return undefined;

packages/opentelemetry-sdk-trace-web/test/utils.test.ts

@@ -213,7 +213,84 @@ describe('utils', () => {
         );
       });
     });
+    describe('when entries contain invalid performance timing', () => {
+      it('should only add events with time greater that or equal to reference value to span', () => {
+        const addEventSpy = sinon.spy();
+        const span = {
+          addEvent: addEventSpy,
+        } as unknown as tracing.Span;
+        const entries = {
+          [PTN.FETCH_START]: 123, // default reference time
+          [PTN.CONNECT_START]: 0,
+          [PTN.REQUEST_START]: 140,
+        } as PerformanceEntries;
+
+        assert.strictEqual(addEventSpy.callCount, 0);
+
+        addSpanNetworkEvent(span, PTN.CONNECT_START, entries);
+
+        assert.strictEqual(
+          addEventSpy.callCount,
+          0,
+          'should not call addEvent'
+        );
+
+        addSpanNetworkEvent(span, PTN.REQUEST_START, entries);
+
+        assert.strictEqual(
+          addEventSpy.callCount,
+          1,
+          'should call addEvent for valid value'
+        );
+      });
+    });
+
+    describe('when entries contain invalid performance timing and a reference event', () => {
+      it('should only add events with time greater that or equal to reference value to span', () => {
+        const addEventSpy = sinon.spy();
+        const span = {
+          addEvent: addEventSpy,
+        } as unknown as tracing.Span;
+        const entries = {
+          [PTN.FETCH_START]: 120,
+          [PTN.CONNECT_START]: 120, // this is used as reference time here
+          [PTN.REQUEST_START]: 10,
+        } as PerformanceEntries;
+
+        assert.strictEqual(addEventSpy.callCount, 0);
+
+        addSpanNetworkEvent(
+          span,
+          PTN.REQUEST_START,
+          entries,
+          PTN.CONNECT_START
+        );
+
+        assert.strictEqual(
+          addEventSpy.callCount,
+          0,
+          'should not call addEvent'
+        );
+
+        addSpanNetworkEvent(span, PTN.FETCH_START, entries, PTN.CONNECT_START);
+
+        assert.strictEqual(
+          addEventSpy.callCount,
+          1,
+          'should call addEvent for valid value'
+        );
+
+        addEventSpy.resetHistory();
+        addSpanNetworkEvent(span, PTN.CONNECT_START, entries, 'foo'); // invalid reference , not adding event to span
+        assert.strictEqual(
+          addEventSpy.callCount,
+          0,
+          'should not call addEvent for invalid reference(non-existent)'
+        );
+      });
+    });
   });
+
   describe('getResource', () => {
     const startTime = [0, 123123123] as HrTime;
     beforeEach(() => {