From 09d8a80cf250a721054329765e62eb437226a0f6 Mon Sep 17 00:00:00 2001
From: Charles Robertson <crobertson@conga.com>
Date: Fri, 1 Jul 2022 12:57:24 -0400
Subject: [PATCH] fix(opentelemetry-instrumentation-document-load): updated
 instrumentation of document load to disallow end time being set to value
 before start time

---
 .../src/instrumentation.ts                    | 15 ++++--
 .../test/documentLoad.test.ts                 | 49 +++++++++++++++++++
 2 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/plugins/web/opentelemetry-instrumentation-document-load/src/instrumentation.ts b/plugins/web/opentelemetry-instrumentation-document-load/src/instrumentation.ts
index 9a537ad0a6..d23a1f4341 100644
--- a/plugins/web/opentelemetry-instrumentation-document-load/src/instrumentation.ts
+++ b/plugins/web/opentelemetry-instrumentation-document-load/src/instrumentation.ts
@@ -112,7 +112,7 @@ export class DocumentLoadInstrumentation extends InstrumentationBase<unknown> {
         if (fetchSpan) {
           context.with(trace.setSpan(context.active(), fetchSpan), () => {
             addSpanNetworkEvents(fetchSpan, entries);
-            this._endSpan(fetchSpan, PTN.RESPONSE_END, entries);
+            this._endSpan(fetchSpan, PTN.RESPONSE_END, PTN.FETCH_START, entries);
           });
         }
       });
@@ -141,7 +141,7 @@ export class DocumentLoadInstrumentation extends InstrumentationBase<unknown> {
 
       addSpanPerformancePaintEvents(rootSpan);
 
-      this._endSpan(rootSpan, PTN.LOAD_EVENT_END, entries);
+      this._endSpan(rootSpan, PTN.LOAD_EVENT_END, PTN.FETCH_START, entries);
     });
   }
 
@@ -149,17 +149,24 @@ export class DocumentLoadInstrumentation extends InstrumentationBase<unknown> {
    * Helper function for ending span
    * @param span
    * @param performanceName name of performance entry for time end
+   * @param startPerformanceName name of the performance entry that has the time start.
    * @param entries
    */
   private _endSpan(
     span: Span | undefined,
     performanceName: string,
+    startPerformanceName: string,
     entries: PerformanceEntries
   ) {
     // span can be undefined when entries are missing the certain performance - the span will not be created
     if (span) {
       if (hasKey(entries, performanceName)) {
-        span.end(entries[performanceName]);
+        // @ts-ignore: Object is possibly 'null'.
+        if(hasKey(entries, startPerformanceName) && entries[startPerformanceName] >  entries[performanceName]) {
+          span.end(entries[startPerformanceName]);
+        } else {
+          span.end(entries[performanceName]);
+        }
       } else {
         // just end span
         span.end();
@@ -185,7 +192,7 @@ export class DocumentLoadInstrumentation extends InstrumentationBase<unknown> {
     if (span) {
       span.setAttribute(SemanticAttributes.HTTP_URL, resource.name);
       addSpanNetworkEvents(span, resource);
-      this._endSpan(span, PTN.RESPONSE_END, resource);
+      this._endSpan(span, PTN.RESPONSE_END, PTN.FETCH_START, resource);
     }
   }
 
diff --git a/plugins/web/opentelemetry-instrumentation-document-load/test/documentLoad.test.ts b/plugins/web/opentelemetry-instrumentation-document-load/test/documentLoad.test.ts
index 54eef0045f..b8acccf914 100644
--- a/plugins/web/opentelemetry-instrumentation-document-load/test/documentLoad.test.ts
+++ b/plugins/web/opentelemetry-instrumentation-document-load/test/documentLoad.test.ts
@@ -124,6 +124,32 @@ const resourcesNoSecureConnectionStart = [
     serverTiming: [],
   },
 ];
+const resourcesFetchAfterEnd = [
+  {
+    name: 'http://localhost:8090/bundle.js',
+    entryType: 'resource',
+    startTime: 20.985000010114163,
+    duration: 90.94999998342246,
+    initiatorType: 'script',
+    nextHopProtocol: 'http/1.1',
+    workerStart: 0,
+    redirectStart: 0,
+    redirectEnd: 0,
+    fetchStart: 120.985000010114163,
+    domainLookupStart: 20.985000010114163,
+    domainLookupEnd: 20.985000010114163,
+    connectStart: 20.985000010114163,
+    connectEnd: 20.985000010114163,
+    secureConnectionStart: 0,
+    requestStart: 29.28999997675419,
+    responseStart: 31.88999998383224,
+    responseEnd: 32.93499999353662,
+    transferSize: 1446645,
+    encodedBodySize: 1446396,
+    decodedBodySize: 1446396,
+    serverTiming: [],
+  },
+];
 const entries = {
   name: 'http://localhost:8090/',
   entryType: 'navigation',
@@ -321,6 +347,29 @@ describe('DocumentLoad Instrumentation', () => {
     });
   });
 
+  describe('when navigation entries have bad timings', () => {
+    let spyEntries: sinon.SinonStub;
+    beforeEach(() => {
+      spyEntries = sandbox.stub(window.performance, 'getEntriesByType');
+      spyEntries.withArgs('navigation').returns([entries]);
+      spyEntries.withArgs('resource').returns(resourcesFetchAfterEnd);
+      spyEntries.withArgs('paint').returns(paintEntries);
+    });
+    afterEach(() => {
+      spyEntries.restore();
+    });
+
+    it('should never have negative duration', done => {
+      plugin.enable();
+
+      setTimeout(() => {
+        const fetchSpan = exporter.getFinishedSpans()[1] as ReadableSpan;
+        assert.deepEqual(fetchSpan.duration, [0,0]);
+        done();
+      });
+    });
+  });
+
   describe('when navigation entries types are available', () => {
     let spyEntries: sinon.SinonStub;
     beforeEach(() => {