chore: fixing issue when metric time is 0 in document-load plugin (#546)

open-telemetry · Nov 20, 2019 · 478f4cd · 478f4cd
1 parent 290eec0
commit 478f4cd
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 10 deletions.
diff --git a/packages/opentelemetry-plugin-document-load/src/documentLoad.ts b/packages/opentelemetry-plugin-document-load/src/documentLoad.ts
@@ -66,7 +66,9 @@ export class DocumentLoad extends BasePlugin<unknown> {
     ) as PerformanceResourceTiming[];
     if (resources) {
       resources.forEach(resource => {
-        this._initResourceSpan(rootSpan, resource);
+        this._initResourceSpan(resource, {
+          parent: rootSpan,
+        });
       });
     }
   }
@@ -86,7 +88,12 @@ export class DocumentLoad extends BasePlugin<unknown> {
       hasKey(entries, performanceName) &&
       typeof entries[performanceName] === 'number'
     ) {
-      span.addEvent(performanceName, undefined, entries[performanceName]);
+      // 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;
     }
     return undefined;
@@ -217,16 +224,19 @@ export class DocumentLoad extends BasePlugin<unknown> {
 
   /**
    * Creates and ends a span with network information about resource added as timed events
-   * @param rootSpan
    * @param resource
+   * @param spanOptions
    */
   private _initResourceSpan(
-    rootSpan: Span,
-    resource: PerformanceResourceTiming
+    resource: PerformanceResourceTiming,
+    spanOptions: SpanOptions = {}
   ) {
-    const span = this._startSpan(resource.name, PTN.FETCH_START, resource, {
-      parent: rootSpan,
-    });
+    const span = this._startSpan(
+      resource.name,
+      PTN.FETCH_START,
+      resource,
+      spanOptions
+    );
     if (span) {
       this._addSpanNetworkEvents(span, resource);
       this._endSpan(span, PTN.RESPONSE_END, resource);

diff --git a/packages/opentelemetry-plugin-document-load/test/documentLoad.test.ts b/packages/opentelemetry-plugin-document-load/test/documentLoad.test.ts
@@ -59,7 +59,7 @@ const resources = [
     domainLookupEnd: 20.985000010114163,
     connectStart: 20.985000010114163,
     connectEnd: 20.985000010114163,
-    secureConnectionStart: 0,
+    secureConnectionStart: 20.985000010114163,
     requestStart: 29.28999997675419,
     responseStart: 31.88999998383224,
     responseEnd: 111.93499999353662,
@@ -83,7 +83,7 @@ const resources = [
     domainLookupEnd: 1998.5950000118464,
     connectStart: 1998.5950000118464,
     connectEnd: 1998.5950000118464,
-    secureConnectionStart: 0,
+    secureConnectionStart: 1998.5950000118464,
     requestStart: 2001.7900000093505,
     responseStart: 2002.3700000019744,
     responseEnd: 2002.8049999964423,
@@ -93,6 +93,32 @@ const resources = [
     serverTiming: [],
   },
 ];
+const resourcesNoSecureConnectionStart = [
+  {
+    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: 20.985000010114163,
+    domainLookupStart: 20.985000010114163,
+    domainLookupEnd: 20.985000010114163,
+    connectStart: 20.985000010114163,
+    connectEnd: 20.985000010114163,
+    secureConnectionStart: 0,
+    requestStart: 29.28999997675419,
+    responseStart: 31.88999998383224,
+    responseEnd: 111.93499999353662,
+    transferSize: 1446645,
+    encodedBodySize: 1446396,
+    decodedBodySize: 1446396,
+    serverTiming: [],
+  },
+];
 const entries = {
   name: 'http://localhost:8090/',
   entryType: 'navigation',
@@ -393,6 +419,44 @@ describe('DocumentLoad Plugin', () => {
       });
     });
   });
+  describe('when resource entries are available AND secureConnectionStart is 0', () => {
+    let spyEntries: any;
+    beforeEach(() => {
+      spyEntries = sinon.stub(window.performance, 'getEntriesByType');
+      spyEntries.withArgs('navigation').returns([entries]);
+      spyEntries.withArgs('resource').returns(resourcesNoSecureConnectionStart);
+    });
+    afterEach(() => {
+      spyEntries.restore();
+    });
+
+    it('should create span for each of the resource', done => {
+      const spyOnEnd = sinon.spy(dummyExporter, 'export');
+      plugin.enable(moduleExports, tracer, logger, config);
+      setTimeout(() => {
+        const spanResource1 = spyOnEnd.args[1][0][0] as ReadableSpan;
+
+        const srEvents1 = spanResource1.events;
+
+        assert.strictEqual(
+          spanResource1.name,
+          'http://localhost:8090/bundle.js'
+        );
+
+        assert.strictEqual(srEvents1[0].name, PTN.FETCH_START);
+        assert.strictEqual(srEvents1[1].name, PTN.DOMAIN_LOOKUP_START);
+        assert.strictEqual(srEvents1[2].name, PTN.DOMAIN_LOOKUP_END);
+        assert.strictEqual(srEvents1[3].name, PTN.CONNECT_START);
+        assert.strictEqual(srEvents1[4].name, PTN.CONNECT_END);
+        assert.strictEqual(srEvents1[5].name, PTN.REQUEST_START);
+        assert.strictEqual(srEvents1[6].name, PTN.RESPONSE_START);
+        assert.strictEqual(srEvents1[7].name, PTN.RESPONSE_END);
+
+        assert.strictEqual(spyOnEnd.callCount, 3);
+        done();
+      });
+    });
+  });
 
   describe('when navigation entries types are available and property "loadEventEnd" is missing', () => {
     let spyEntries: any;