From ee014c94c973e64541d79aa2197e169dd6c7aa2c Mon Sep 17 00:00:00 2001 From: John Bley Date: Wed, 20 Jan 2021 11:35:39 -0500 Subject: [PATCH] Allow zero/negative performance timings (#1769) Co-authored-by: Daniel Dyla --- .../src/enums/PerformanceTimingNames.ts | 1 + packages/opentelemetry-web/src/utils.ts | 5 -- packages/opentelemetry-web/test/utils.test.ts | 52 +++++++++++-------- 3 files changed, 30 insertions(+), 28 deletions(-) diff --git a/packages/opentelemetry-web/src/enums/PerformanceTimingNames.ts b/packages/opentelemetry-web/src/enums/PerformanceTimingNames.ts index cbcfc4fe36..03e8fe2330 100644 --- 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 index 6b94959f96..2cd5750e5b 100644 --- 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 index 046589793b..5189d397ee 100644 --- 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); });