diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e99ea702f..ab75b14a05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,8 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se * fix(sdk-trace-base): do not load OTEL_ env vars on module load, but when needed [#5224](https://github.com/open-telemetry/opentelemetry-js/pull/5224) +* fix(instrumentation-xhr, instrumentation-fetch): content length attributes no longer get removed with `ignoreNetworkEvents: true` being set [#5229](https://github.com/open-telemetry/opentelemetry-js/issues/5229) + ### :books: (Refine Doc) ### :house: (Internal) diff --git a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts index e5d9a84bde..ca19b31659 100644 --- a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts +++ b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts @@ -112,9 +112,11 @@ export class FetchInstrumentation extends InstrumentationBase { const events = span.events; assert.strictEqual(events.length, 0, 'number of events is wrong'); }); + + it('should still add the CONTENT_LENGTH attribute', () => { + const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const attributes = span.attributes; + const responseContentLength = attributes[ + SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH + ] as number; + assert.strictEqual( + responseContentLength, + 30, + `attributes ${SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH} is <= 0` + ); + }); }); }); diff --git a/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts b/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts index 6d2eafa054..dc0f3ea948 100644 --- a/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts +++ b/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts @@ -149,9 +149,11 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase { const events = span.events; assert.strictEqual(events.length, 3, 'number of events is wrong'); }); + + it('should still add the CONTENT_LENGTH attribute', () => { + const span: tracing.ReadableSpan = exportSpy.args[1][0][0]; + const attributes = span.attributes; + const responseContentLength = attributes[ + SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH + ] as number; + assert.strictEqual( + responseContentLength, + 30, + `attributes ${SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH} is <= 0` + ); + }); }); }); diff --git a/packages/opentelemetry-sdk-trace-web/src/utils.ts b/packages/opentelemetry-sdk-trace-web/src/utils.ts index 5fcd9abf4e..e8fe4ef1d6 100644 --- a/packages/opentelemetry-sdk-trace-web/src/utils.ts +++ b/packages/opentelemetry-sdk-trace-web/src/utils.ts @@ -89,28 +89,32 @@ export function addSpanNetworkEvent( } /** - * Helper function for adding network events + * Helper function for adding network events and content length attributes * @param span * @param resource + * @param ignoreNetworkEvents */ export function addSpanNetworkEvents( span: api.Span, - resource: PerformanceEntries + resource: PerformanceEntries, + ignoreNetworkEvents = false ): void { - 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); + 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.CONNECT_END, resource); - addSpanNetworkEvent(span, PTN.REQUEST_START, resource); - addSpanNetworkEvent(span, PTN.RESPONSE_START, resource); - addSpanNetworkEvent(span, PTN.RESPONSE_END, resource); const encodedLength = resource[PTN.ENCODED_BODY_SIZE]; if (encodedLength !== undefined) { span.setAttribute(SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH, encodedLength); diff --git a/packages/opentelemetry-sdk-trace-web/test/utils.test.ts b/packages/opentelemetry-sdk-trace-web/test/utils.test.ts index b70061e489..0011dde148 100644 --- a/packages/opentelemetry-sdk-trace-web/test/utils.test.ts +++ b/packages/opentelemetry-sdk-trace-web/test/utils.test.ts @@ -127,6 +127,35 @@ describe('utils', () => { } as PerformanceResourceTiming); assert.strictEqual(addEventSpy.callCount, 9); }); + + it('should ignore network events when ignoreNetworkEvents is true', () => { + const addEventSpy = sinon.spy(); + const setAttributeSpy = sinon.spy(); + const span = { + addEvent: addEventSpy, + setAttribute: setAttributeSpy, + } as unknown as tracing.Span; + const entries = { + [PTN.FETCH_START]: 123, + [PTN.DOMAIN_LOOKUP_START]: 123, + [PTN.DOMAIN_LOOKUP_END]: 123, + [PTN.CONNECT_START]: 123, + [PTN.SECURE_CONNECTION_START]: 123, + [PTN.CONNECT_END]: 123, + [PTN.REQUEST_START]: 123, + [PTN.RESPONSE_START]: 123, + [PTN.RESPONSE_END]: 123, + [PTN.DECODED_BODY_SIZE]: 123, + [PTN.ENCODED_BODY_SIZE]: 61, + } as PerformanceEntries; + + 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 only include encoded size when content encoding is being used', () => { const addEventSpy = sinon.spy(); const setAttributeSpy = sinon.spy();