From 30881c4f9b76f1898dd2721072efea9ec5e98ff9 Mon Sep 17 00:00:00 2001 From: Arriana Blais Date: Thu, 12 Dec 2024 09:48:49 -0500 Subject: [PATCH 1/5] add tests for content length bug --- .../test/fetch.test.ts | 13 +++++++++++++ .../test/xhr.test.ts | 13 +++++++++++++ 2 files changed, 26 insertions(+) diff --git a/experimental/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts b/experimental/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts index 28874a5ba53..7f81d7cd423 100644 --- a/experimental/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts @@ -1211,5 +1211,18 @@ describe('fetch', () => { 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/test/xhr.test.ts b/experimental/packages/opentelemetry-instrumentation-xml-http-request/test/xhr.test.ts index 2f23765f4d9..cd9d18e84b7 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 @@ -859,6 +859,19 @@ describe('xhr', () => { 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` + ); + }); }); }); From 74f2bdabb8c1104e14d5bb2c76c94e5630c70c15 Mon Sep 17 00:00:00 2001 From: Arriana Blais Date: Thu, 12 Dec 2024 09:51:29 -0500 Subject: [PATCH 2/5] add new utility method --- packages/opentelemetry-sdk-trace-web/src/index.ts | 1 + packages/opentelemetry-sdk-trace-web/src/utils.ts | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/packages/opentelemetry-sdk-trace-web/src/index.ts b/packages/opentelemetry-sdk-trace-web/src/index.ts index 017b934a8f5..25101e1f8c0 100644 --- a/packages/opentelemetry-sdk-trace-web/src/index.ts +++ b/packages/opentelemetry-sdk-trace-web/src/index.ts @@ -27,6 +27,7 @@ export { URLLike, addSpanNetworkEvent, addSpanNetworkEvents, + addSpanContentLengthAttributes, getElementXPath, getResource, hasKey, diff --git a/packages/opentelemetry-sdk-trace-web/src/utils.ts b/packages/opentelemetry-sdk-trace-web/src/utils.ts index 5fcd9abf4ee..e8d7bdb513f 100644 --- a/packages/opentelemetry-sdk-trace-web/src/utils.ts +++ b/packages/opentelemetry-sdk-trace-web/src/utils.ts @@ -111,6 +111,17 @@ export function addSpanNetworkEvents( addSpanNetworkEvent(span, PTN.REQUEST_START, resource); addSpanNetworkEvent(span, PTN.RESPONSE_START, resource); addSpanNetworkEvent(span, PTN.RESPONSE_END, resource); +} + +/** + * Helper function for adding content length attributes to a network span + * @param span + * @param resource + */ +export function addSpanContentLengthAttributes( + span: api.Span, + resource: PerformanceEntries +): void { const encodedLength = resource[PTN.ENCODED_BODY_SIZE]; if (encodedLength !== undefined) { span.setAttribute(SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH, encodedLength); From 0584df9039ecab40535f86db6a59fa6629a74aac Mon Sep 17 00:00:00 2001 From: Arriana Blais Date: Thu, 12 Dec 2024 10:01:32 -0500 Subject: [PATCH 3/5] call utility method within modules --- .../packages/opentelemetry-instrumentation-fetch/src/fetch.ts | 2 ++ .../opentelemetry-instrumentation-xml-http-request/src/xhr.ts | 3 +++ 2 files changed, 5 insertions(+) diff --git a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts index e5d9a84bdeb..6f847e78f54 100644 --- a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts +++ b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts @@ -115,6 +115,7 @@ export class FetchInstrumentation extends InstrumentationBase Date: Thu, 12 Dec 2024 10:10:22 -0500 Subject: [PATCH 4/5] Add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 239db0b072f..d9f6e753bb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se ### :bug: (Bug Fix) * 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) From 2d9376326837b8e49857c7070d3bc88d74c59691 Mon Sep 17 00:00:00 2001 From: Arriana Blais Date: Thu, 12 Dec 2024 15:30:14 -0500 Subject: [PATCH 5/5] move to correct changelog --- CHANGELOG.md | 1 - CHANGELOG_NEXT.md | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d9f6e753bb1..239db0b072f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,6 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se ### :bug: (Bug Fix) * 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) diff --git a/CHANGELOG_NEXT.md b/CHANGELOG_NEXT.md index 06717c2ebe2..6b6532c27d9 100644 --- a/CHANGELOG_NEXT.md +++ b/CHANGELOG_NEXT.md @@ -16,6 +16,7 @@ * refactor(sdk-trace-base)!: remove `new Span` constructor in favor of `Tracer.startSpan` API [#5048](https://github.com/open-telemetry/opentelemetry-js/pull/5048) @david-luna * refactor(sdk-trace-base)!: remove `BasicTracerProvider.addSpanProcessor` API in favor of constructor options. [#5134](https://github.com/open-telemetry/opentelemetry-js/pull/5134) @david-luna * refactor(sdk-trace-base)!: make `resource` property private in `BasicTracerProvider` and remove `getActiveSpanProcessor` API. [#5192](https://github.com/open-telemetry/opentelemetry-js/pull/5192) @david-luna +* feat(sdk-trace-web)!: content length attributes are no longer added in `addSpanNetworkEvents`. Logic has been moved to new method `addSpanContentLengthAttributes` [#5257](https://github.com/open-telemetry/opentelemetry-js/pull/5257) @arriIsHere ### :rocket: (Enhancement)