diff --git a/experimental/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts b/experimental/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts index f81336ae4f6..5ea37a3586e 100644 --- a/experimental/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts @@ -90,27 +90,27 @@ const textToReadableStream = (msg: string): ReadableStream => { const CUSTOM_ATTRIBUTE_KEY = 'span kind'; const defaultResource = { - connectEnd: 15, - connectStart: 13, - decodedBodySize: 0, - domainLookupEnd: 12, - domainLookupStart: 11, - encodedBodySize: 0, - fetchStart: 10.1, + entryType: 'resource', + name: '', initiatorType: 'fetch', - nextHopProtocol: '', - redirectEnd: 0, + startTime: 10.1, redirectStart: 0, + redirectEnd: 0, + workerStart: 0, + fetchStart: 10.1, + domainLookupStart: 11, + domainLookupEnd: 12, + connectStart: 13, + secureConnectionStart: 0, + connectEnd: 15, requestStart: 16, - responseEnd: 20.5, responseStart: 17, - secureConnectionStart: 14, + responseEnd: 20.5, + duration: 10.4, + decodedBodySize: 30, + encodedBodySize: 30, transferSize: 0, - workerStart: 0, - duration: 0, - entryType: '', - name: '', - startTime: 0, + nextHopProtocol: '', }; function createResource(resource = {}): PerformanceResourceTiming { @@ -124,7 +124,7 @@ function createResource(resource = {}): PerformanceResourceTiming { function createMainResource(resource = {}): PerformanceResourceTiming { const mainResource: any = createResource(resource); Object.keys(mainResource).forEach((key: string) => { - if (typeof mainResource[key] === 'number') { + if (typeof mainResource[key] === 'number' && mainResource[key] !== 0) { mainResource[key] = mainResource[key] + 30; } }); @@ -139,8 +139,14 @@ function createFakePerformanceObs(url: string) { const resources: PerformanceObserverEntryList = { getEntries(): PerformanceEntryList { return [ - createResource({ name: absoluteUrl }) as any, - createMainResource({ name: absoluteUrl }) as any, + createResource({ + name: absoluteUrl, + [PTN.SECURE_CONNECTION_START]: url.startsWith('https:') ? 14 : 0, + }) as any, + createMainResource({ + name: absoluteUrl, + [PTN.SECURE_CONNECTION_START]: url.startsWith('https:') ? 14 : 0, + }) as any, ]; }, getEntriesByName(): PerformanceEntryList { @@ -458,7 +464,7 @@ describe('fetch', () => { ] as number; assert.strictEqual( responseContentLength, - 30, + 60, `attributes ${SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH} is <= 0` ); @@ -468,6 +474,8 @@ describe('fetch', () => { it('span should have correct events', () => { const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; const events = span.events; + console.log(events.length, JSON.stringify(events, null, 2)); + assert.strictEqual(events.length, 8, 'number of events is wrong'); testForCorrectEvents(events, [ PTN.FETCH_START, @@ -1217,7 +1225,7 @@ describe('fetch', () => { ] as number; assert.strictEqual( responseContentLength, - 30, + 60, `attributes ${SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH} is <= 0` ); }); diff --git a/experimental/packages/opentelemetry-instrumentation-xml-http-request/test/xhr.test.ts b/experimental/packages/opentelemetry-instrumentation-xml-http-request/test/xhr.test.ts index cd9d18e84b7..823d6f942e9 100644 --- a/experimental/packages/opentelemetry-instrumentation-xml-http-request/test/xhr.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-xml-http-request/test/xhr.test.ts @@ -134,28 +134,37 @@ const postData = ( function createResource(resource = {}): PerformanceResourceTiming { const defaultResource = { - connectEnd: 15, - connectStart: 13, - decodedBodySize: 0, - domainLookupEnd: 12, - domainLookupStart: 11, - encodedBodySize: 0, - fetchStart: 10.1, + entryType: 'resource', + name: '', initiatorType: 'xmlhttprequest', - nextHopProtocol: '', - redirectEnd: 0, + startTime: 10.1, redirectStart: 0, + redirectEnd: 0, + workerStart: 0, + fetchStart: 10.1, + domainLookupStart: 11, + domainLookupEnd: 12, + connectStart: 13, + secureConnectionStart: 0, + connectEnd: 15, requestStart: 16, - responseEnd: 20.5, responseStart: 17, - secureConnectionStart: 14, + responseEnd: 20.5, + duration: 10.4, + decodedBodySize: 30, + encodedBodySize: 30, transferSize: 0, - workerStart: 0, - duration: 0, - entryType: '', - name: '', - startTime: 0, + nextHopProtocol: '', }; + + if ( + 'name' in resource && + typeof resource.name === 'string' && + resource.name.startsWith('https:') + ) { + defaultResource.secureConnectionStart = 14; + } + return Object.assign( {}, defaultResource, @@ -166,7 +175,7 @@ function createResource(resource = {}): PerformanceResourceTiming { function createMainResource(resource = {}): PerformanceResourceTiming { const mainResource: any = createResource(resource); Object.keys(mainResource).forEach((key: string) => { - if (typeof mainResource[key] === 'number') { + if (typeof mainResource[key] === 'number' && mainResource[key] !== 0) { mainResource[key] = mainResource[key] + 30; } }); @@ -424,7 +433,7 @@ describe('xhr', () => { ] as number; assert.strictEqual( responseContentLength, - 30, + 60, `attributes ${SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH} <= 0` ); assert.strictEqual( @@ -868,7 +877,7 @@ describe('xhr', () => { ] as number; assert.strictEqual( responseContentLength, - 30, + 60, `attributes ${SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH} is <= 0` ); }); @@ -1052,7 +1061,7 @@ describe('xhr', () => { ] as number; assert.strictEqual( responseContentLength, - 0, + 30, `attributes ${SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH} <= 0` ); assert.strictEqual( @@ -1622,7 +1631,7 @@ describe('xhr', () => { ] as number; assert.strictEqual( responseContentLength, - 30, + 60, `attributes ${SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH} <= 0` ); assert.strictEqual( @@ -2236,7 +2245,7 @@ describe('xhr', () => { ] as number; assert.strictEqual( responseContentLength, - 0, + 30, `attributes ${SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH} <= 0` ); assert.strictEqual( diff --git a/packages/opentelemetry-sdk-trace-web/src/enums/PerformanceTimingNames.ts b/packages/opentelemetry-sdk-trace-web/src/enums/PerformanceTimingNames.ts index 6730e5e2c11..252562af981 100644 --- a/packages/opentelemetry-sdk-trace-web/src/enums/PerformanceTimingNames.ts +++ b/packages/opentelemetry-sdk-trace-web/src/enums/PerformanceTimingNames.ts @@ -35,6 +35,7 @@ export enum PerformanceTimingNames { RESPONSE_END = 'responseEnd', RESPONSE_START = 'responseStart', SECURE_CONNECTION_START = 'secureConnectionStart', + START_TIME = 'startTime', UNLOAD_EVENT_END = 'unloadEventEnd', UNLOAD_EVENT_START = 'unloadEventStart', } diff --git a/packages/opentelemetry-sdk-trace-web/src/types.ts b/packages/opentelemetry-sdk-trace-web/src/types.ts index 076665cc3f6..00664234d08 100644 --- a/packages/opentelemetry-sdk-trace-web/src/types.ts +++ b/packages/opentelemetry-sdk-trace-web/src/types.ts @@ -35,6 +35,7 @@ export type PerformanceEntries = { [PerformanceTimingNames.RESPONSE_END]?: number; [PerformanceTimingNames.RESPONSE_START]?: number; [PerformanceTimingNames.SECURE_CONNECTION_START]?: number; + [PerformanceTimingNames.START_TIME]?: number; [PerformanceTimingNames.UNLOAD_EVENT_END]?: number; [PerformanceTimingNames.UNLOAD_EVENT_START]?: number; }; diff --git a/packages/opentelemetry-sdk-trace-web/src/utils.ts b/packages/opentelemetry-sdk-trace-web/src/utils.ts index e8fe4ef1d69..a1f7fcccc2f 100644 --- a/packages/opentelemetry-sdk-trace-web/src/utils.ts +++ b/packages/opentelemetry-sdk-trace-web/src/utils.ts @@ -59,32 +59,22 @@ export function hasKey( * @param span * @param performanceName name of performance entry for time start * @param entries - * @param refPerfName name of performance entry to use for reference + * @param ignoreZeros */ export function addSpanNetworkEvent( span: api.Span, performanceName: string, entries: PerformanceEntries, - refPerfName?: string + ignoreZeros = true ): api.Span | undefined { - let perfTime = undefined; - let refTime = undefined; if ( hasKey(entries, performanceName) && - typeof entries[performanceName] === 'number' + typeof entries[performanceName] === 'number' && + !(ignoreZeros && entries[performanceName] === 0) ) { - perfTime = entries[performanceName]; - } - const refName = refPerfName || PTN.FETCH_START; - // Use a reference time which is the earliest possible value so that the performance timings that are earlier should not be added - // using FETCH START time in case no reference is provided - if (hasKey(entries, refName) && typeof entries[refName] === 'number') { - refTime = entries[refName]; - } - if (perfTime !== undefined && refTime !== undefined && perfTime >= refTime) { - span.addEvent(performanceName, perfTime); - return span; + return span.addEvent(performanceName, entries[performanceName]); } + return undefined; } @@ -93,32 +83,40 @@ export function addSpanNetworkEvent( * @param span * @param resource * @param ignoreNetworkEvents + * @param ignoreZeros */ export function addSpanNetworkEvents( span: api.Span, resource: PerformanceEntries, - ignoreNetworkEvents = false + ignoreNetworkEvents = false, + ignoreZeros?: boolean ): void { + if (ignoreZeros === undefined) { + ignoreZeros = resource[PTN.START_TIME] !== 0; + } + if (!ignoreNetworkEvents) { - addSpanNetworkEvent(span, PTN.FETCH_START, resource); - addSpanNetworkEvent(span, PTN.DOMAIN_LOOKUP_START, resource); - addSpanNetworkEvent(span, PTN.DOMAIN_LOOKUP_END, resource); - addSpanNetworkEvent(span, PTN.CONNECT_START, resource); - if ( - hasKey(resource as PerformanceResourceTiming, 'name') && - (resource as PerformanceResourceTiming)['name'].startsWith('https:') - ) { - addSpanNetworkEvent(span, PTN.SECURE_CONNECTION_START, resource); - } - addSpanNetworkEvent(span, PTN.CONNECT_END, resource); - addSpanNetworkEvent(span, PTN.REQUEST_START, resource); - addSpanNetworkEvent(span, PTN.RESPONSE_START, resource); - addSpanNetworkEvent(span, PTN.RESPONSE_END, resource); + addSpanNetworkEvent(span, PTN.FETCH_START, resource, ignoreZeros); + addSpanNetworkEvent(span, PTN.DOMAIN_LOOKUP_START, resource, ignoreZeros); + addSpanNetworkEvent(span, PTN.DOMAIN_LOOKUP_END, resource, ignoreZeros); + addSpanNetworkEvent(span, PTN.CONNECT_START, resource, ignoreZeros); + addSpanNetworkEvent( + span, + PTN.SECURE_CONNECTION_START, + resource, + ignoreZeros + ); + addSpanNetworkEvent(span, PTN.CONNECT_END, resource, ignoreZeros); + addSpanNetworkEvent(span, PTN.REQUEST_START, resource, ignoreZeros); + addSpanNetworkEvent(span, PTN.RESPONSE_START, resource, ignoreZeros); + addSpanNetworkEvent(span, PTN.RESPONSE_END, resource, ignoreZeros); } + const encodedLength = resource[PTN.ENCODED_BODY_SIZE]; if (encodedLength !== undefined) { span.setAttribute(SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH, encodedLength); } + const decodedLength = resource[PTN.DECODED_BODY_SIZE]; // Spec: Not set if transport encoding not used (in which case encoded and decoded sizes match) if (decodedLength !== undefined && encodedLength !== decodedLength) { diff --git a/packages/opentelemetry-sdk-trace-web/test/utils.test.ts b/packages/opentelemetry-sdk-trace-web/test/utils.test.ts index 0011dde1485..46e9ed33c26 100644 --- a/packages/opentelemetry-sdk-trace-web/test/utils.test.ts +++ b/packages/opentelemetry-sdk-trace-web/test/utils.test.ts @@ -100,6 +100,7 @@ describe('utils', () => { setAttribute: setAttributeSpy, } as unknown as tracing.Span; const entries = { + [PTN.START_TIME]: 123, [PTN.FETCH_START]: 123, [PTN.DOMAIN_LOOKUP_START]: 123, [PTN.DOMAIN_LOOKUP_END]: 123, @@ -113,21 +114,13 @@ describe('utils', () => { [PTN.ENCODED_BODY_SIZE]: 61, } as PerformanceEntries; + assert.strictEqual(setAttributeSpy.callCount, 0); assert.strictEqual(addEventSpy.callCount, 0); addSpanNetworkEvents(span, entries); assert.strictEqual(setAttributeSpy.callCount, 2); - //secure connect start should not be added to non-https resource - assert.strictEqual(addEventSpy.callCount, 8); - //secure connect start should be added to an https resource - addEventSpy.resetHistory(); - addSpanNetworkEvents(span, { - ...entries, - name: 'https://foo', - } as PerformanceResourceTiming); assert.strictEqual(addEventSpy.callCount, 9); }); - it('should ignore network events when ignoreNetworkEvents is true', () => { const addEventSpy = sinon.spy(); const setAttributeSpy = sinon.spy(); @@ -136,6 +129,7 @@ describe('utils', () => { setAttribute: setAttributeSpy, } as unknown as tracing.Span; const entries = { + [PTN.START_TIME]: 123, [PTN.FETCH_START]: 123, [PTN.DOMAIN_LOOKUP_START]: 123, [PTN.DOMAIN_LOOKUP_END]: 123, @@ -149,13 +143,100 @@ describe('utils', () => { [PTN.ENCODED_BODY_SIZE]: 61, } as PerformanceEntries; + assert.strictEqual(setAttributeSpy.callCount, 0); assert.strictEqual(addEventSpy.callCount, 0); addSpanNetworkEvents(span, entries, true); assert.strictEqual(setAttributeSpy.callCount, 2); - //secure connect start should not be added to non-https resource assert.strictEqual(addEventSpy.callCount, 0); }); + it('should ignore zero timings by default', () => { + const addEventSpy = sinon.spy(); + const setAttributeSpy = sinon.spy(); + const span = { + addEvent: addEventSpy, + setAttribute: setAttributeSpy, + } as unknown as tracing.Span; + const entries = { + [PTN.START_TIME]: 123, + [PTN.FETCH_START]: 123, + [PTN.DOMAIN_LOOKUP_START]: 0, + [PTN.DOMAIN_LOOKUP_END]: 0, + [PTN.CONNECT_START]: 0, + [PTN.SECURE_CONNECTION_START]: 0, + [PTN.CONNECT_END]: 0, + [PTN.REQUEST_START]: 0, + [PTN.RESPONSE_START]: 0, + [PTN.RESPONSE_END]: 130, + [PTN.DECODED_BODY_SIZE]: 0, + [PTN.ENCODED_BODY_SIZE]: 0, + } as PerformanceEntries; + + assert.strictEqual(setAttributeSpy.callCount, 0); + assert.strictEqual(addEventSpy.callCount, 0); + + addSpanNetworkEvents(span, entries); + assert.strictEqual(setAttributeSpy.callCount, 1); + assert.strictEqual(addEventSpy.callCount, 2); + }); + it('should not ignore zero timings by default if startTime = 0', () => { + const addEventSpy = sinon.spy(); + const setAttributeSpy = sinon.spy(); + const span = { + addEvent: addEventSpy, + setAttribute: setAttributeSpy, + } as unknown as tracing.Span; + const entries = { + [PTN.START_TIME]: 0, + [PTN.FETCH_START]: 0, + [PTN.DOMAIN_LOOKUP_START]: 0, + [PTN.DOMAIN_LOOKUP_END]: 0, + [PTN.CONNECT_START]: 0, + [PTN.SECURE_CONNECTION_START]: 0, + [PTN.CONNECT_END]: 1, + [PTN.REQUEST_START]: 2, + [PTN.RESPONSE_START]: 3, + [PTN.RESPONSE_END]: 4, + [PTN.DECODED_BODY_SIZE]: 123, + [PTN.ENCODED_BODY_SIZE]: 61, + } as PerformanceEntries; + + assert.strictEqual(setAttributeSpy.callCount, 0); + assert.strictEqual(addEventSpy.callCount, 0); + + addSpanNetworkEvents(span, entries); + assert.strictEqual(setAttributeSpy.callCount, 2); + assert.strictEqual(addEventSpy.callCount, 9); + }); + it('should not ignore zero timings if ignoreZeros = false', () => { + const addEventSpy = sinon.spy(); + const setAttributeSpy = sinon.spy(); + const span = { + addEvent: addEventSpy, + setAttribute: setAttributeSpy, + } as unknown as tracing.Span; + const entries = { + [PTN.START_TIME]: 123, + [PTN.FETCH_START]: 123, + [PTN.DOMAIN_LOOKUP_START]: 0, + [PTN.DOMAIN_LOOKUP_END]: 0, + [PTN.CONNECT_START]: 0, + [PTN.SECURE_CONNECTION_START]: 0, + [PTN.CONNECT_END]: 0, + [PTN.REQUEST_START]: 0, + [PTN.RESPONSE_START]: 0, + [PTN.RESPONSE_END]: 130, + [PTN.DECODED_BODY_SIZE]: 0, + [PTN.ENCODED_BODY_SIZE]: 0, + } as PerformanceEntries; + + assert.strictEqual(setAttributeSpy.callCount, 0); + assert.strictEqual(addEventSpy.callCount, 0); + + addSpanNetworkEvents(span, entries, false, false); + assert.strictEqual(setAttributeSpy.callCount, 1); + assert.strictEqual(addEventSpy.callCount, 9); + }); it('should only include encoded size when content encoding is being used', () => { const addEventSpy = sinon.spy(); const setAttributeSpy = sinon.spy(); @@ -177,7 +258,7 @@ describe('utils', () => { }); }); describe('addSpanNetworkEvent', () => { - [0, -2, 123].forEach(value => { + [-2, 123].forEach(value => { describe(`when entry is ${value}`, () => { it('should add event to span', () => { const addEventSpy = sinon.spy(); @@ -200,122 +281,82 @@ describe('utils', () => { }); }); }); - describe('when entry is not numeric', () => { - it('should NOT add event to span', () => { + describe(`when entry is zero`, () => { + it('should not add event to span by default', () => { const addEventSpy = sinon.spy(); const span = { addEvent: addEventSpy, } as unknown as tracing.Span; const entries = { - [PTN.FETCH_START]: 'non-numeric', - } as unknown; + [PTN.SECURE_CONNECTION_START]: 0, + } as PerformanceEntries; assert.strictEqual(addEventSpy.callCount, 0); - addSpanNetworkEvent( - span, - PTN.FETCH_START, - entries as PerformanceEntries - ); + addSpanNetworkEvent(span, PTN.SECURE_CONNECTION_START, entries); assert.strictEqual(addEventSpy.callCount, 0); }); - }); - describe('when entries does NOT contain the performance', () => { - it('should NOT add event to span', () => { + it('should add event to span if ignoreZeros = false', () => { const addEventSpy = sinon.spy(); const span = { addEvent: addEventSpy, } as unknown as tracing.Span; const entries = { - [PTN.FETCH_START]: 123, + [PTN.SECURE_CONNECTION_START]: 0, } as PerformanceEntries; assert.strictEqual(addEventSpy.callCount, 0); - addSpanNetworkEvent(span, 'foo', entries); + addSpanNetworkEvent(span, PTN.SECURE_CONNECTION_START, entries, false); - assert.strictEqual( - addEventSpy.callCount, - 0, - 'should not call addEvent' - ); + assert.strictEqual(addEventSpy.callCount, 1); + const args = addEventSpy.args[0]; + + assert.strictEqual(args[0], 'secureConnectionStart'); + assert.strictEqual(args[1], 0); }); }); - describe('when entries contain invalid performance timing', () => { - it('should only add events with time greater that or equal to reference value to span', () => { + 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]: 123, // default reference time - [PTN.CONNECT_START]: 0, - [PTN.REQUEST_START]: 140, - } as PerformanceEntries; + [PTN.FETCH_START]: 'non-numeric', + } as unknown; assert.strictEqual(addEventSpy.callCount, 0); - addSpanNetworkEvent(span, PTN.CONNECT_START, entries); - - assert.strictEqual( - addEventSpy.callCount, - 0, - 'should not call addEvent' + addSpanNetworkEvent( + span, + PTN.FETCH_START, + entries as PerformanceEntries ); - addSpanNetworkEvent(span, PTN.REQUEST_START, entries); - - assert.strictEqual( - addEventSpy.callCount, - 1, - 'should call addEvent for valid value' - ); + assert.strictEqual(addEventSpy.callCount, 0); }); }); - - describe('when entries contain invalid performance timing and a reference event', () => { - it('should only add events with time greater that or equal to reference value to span', () => { + describe('when entries does NOT contain the performance', () => { + 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]: 120, - [PTN.CONNECT_START]: 120, // this is used as reference time here - [PTN.REQUEST_START]: 10, + [PTN.FETCH_START]: 123, } as PerformanceEntries; assert.strictEqual(addEventSpy.callCount, 0); - addSpanNetworkEvent( - span, - PTN.REQUEST_START, - entries, - PTN.CONNECT_START - ); + addSpanNetworkEvent(span, 'foo', entries); assert.strictEqual( addEventSpy.callCount, 0, 'should not call addEvent' ); - - addSpanNetworkEvent(span, PTN.FETCH_START, entries, PTN.CONNECT_START); - - assert.strictEqual( - addEventSpy.callCount, - 1, - 'should call addEvent for valid value' - ); - - addEventSpy.resetHistory(); - addSpanNetworkEvent(span, PTN.CONNECT_START, entries, 'foo'); // invalid reference , not adding event to span - assert.strictEqual( - addEventSpy.callCount, - 0, - 'should not call addEvent for invalid reference(non-existent)' - ); }); }); });