From 97f219a4abe0a8ad7cb628dac4513c4882ae98eb Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 18 Dec 2020 11:55:40 -0500 Subject: [PATCH 1/3] fix: allow zero/negative performance timings --- .../opentelemetry-web/src/enums/PerformanceTimingNames.ts | 1 + packages/opentelemetry-web/src/utils.ts | 5 ----- packages/opentelemetry-web/test/utils.test.ts | 8 ++++---- 3 files changed, 5 insertions(+), 9 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..603f568c89 100644 --- a/packages/opentelemetry-web/test/utils.test.ts +++ b/packages/opentelemetry-web/test/utils.test.ts @@ -185,19 +185,19 @@ describe('utils', () => { assert.strictEqual(args[1], 123); }); }); - 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); }); From 419bb5a64a70680032eb9abfe0f919b85f6cc8b8 Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 18 Dec 2020 12:08:25 -0500 Subject: [PATCH 2/3] chore: run lint:fix --- packages/opentelemetry-web/test/utils.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/opentelemetry-web/test/utils.test.ts b/packages/opentelemetry-web/test/utils.test.ts index 603f568c89..1f8a3d15b2 100644 --- a/packages/opentelemetry-web/test/utils.test.ts +++ b/packages/opentelemetry-web/test/utils.test.ts @@ -197,7 +197,11 @@ describe('utils', () => { assert.strictEqual(addEventSpy.callCount, 0); - addSpanNetworkEvent(span, PTN.FETCH_START, entries as PerformanceEntries); + addSpanNetworkEvent( + span, + PTN.FETCH_START, + entries as PerformanceEntries + ); assert.strictEqual(addEventSpy.callCount, 0); }); From 70948bcd03b52b556f93cfb6ef1e50659b7d892b Mon Sep 17 00:00:00 2001 From: John Bley Date: Mon, 4 Jan 2021 07:12:36 -0500 Subject: [PATCH 3/3] chore: add tests for 0 and negative values --- packages/opentelemetry-web/test/utils.test.ts | 40 ++++++++++--------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/packages/opentelemetry-web/test/utils.test.ts b/packages/opentelemetry-web/test/utils.test.ts index 1f8a3d15b2..5189d397ee 100644 --- a/packages/opentelemetry-web/test/utils.test.ts +++ b/packages/opentelemetry-web/test/utils.test.ts @@ -164,25 +164,27 @@ 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 is not numeric', () => {