From 4a056ef957a6469d143b2deb64dc6853a2e6fe98 Mon Sep 17 00:00:00 2001 From: Bartlomiej Obecny Date: Mon, 18 Nov 2019 14:13:35 +0100 Subject: [PATCH] chore: fixing issue when metric time is 0 in document-load plugin --- .../src/documentLoad.ts | 26 ++++--- .../test/documentLoad.test.ts | 68 ++++++++++++++++++- 2 files changed, 84 insertions(+), 10 deletions(-) diff --git a/packages/opentelemetry-plugin-document-load/src/documentLoad.ts b/packages/opentelemetry-plugin-document-load/src/documentLoad.ts index 9770decd46..dd3c9c6454 100644 --- a/packages/opentelemetry-plugin-document-load/src/documentLoad.ts +++ b/packages/opentelemetry-plugin-document-load/src/documentLoad.ts @@ -61,7 +61,9 @@ export class DocumentLoad extends BasePlugin { ) as PerformanceResourceTiming[]; if (resources) { resources.forEach(resource => { - this._initResourceSpan(rootSpan, resource); + this._initResourceSpan(resource, { + parent: rootSpan, + }); }); } } @@ -81,7 +83,12 @@ export class DocumentLoad extends BasePlugin { 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; @@ -205,16 +212,19 @@ export class DocumentLoad extends BasePlugin { /** * 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 index 3c317286c4..c07f75bdbf 100644 --- 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', @@ -346,6 +372,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;