From 584febf2fd5f2aa48d3f88baee2d8dad2fb32ced Mon Sep 17 00:00:00 2001 From: Arriana Blais Date: Tue, 3 Dec 2024 10:39:39 -0500 Subject: [PATCH 01/10] Make network events optional --- .../opentelemetry-sdk-trace-web/src/utils.ts | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/packages/opentelemetry-sdk-trace-web/src/utils.ts b/packages/opentelemetry-sdk-trace-web/src/utils.ts index 5fcd9abf4e..c5d89319b2 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); From 9fbf81486113be023f72ebda933482c88ef7c79d Mon Sep 17 00:00:00 2001 From: Arriana Blais Date: Tue, 3 Dec 2024 10:39:49 -0500 Subject: [PATCH 02/10] utilize new property --- .../src/xhr.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) 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..c6376d177a 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,7 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase Date: Tue, 3 Dec 2024 10:43:10 -0500 Subject: [PATCH 03/10] use new param for fetch --- .../opentelemetry-instrumentation-fetch/src/fetch.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts index e5d9a84bde..dfa8a55a22 100644 --- a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts +++ b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts @@ -112,9 +112,7 @@ export class FetchInstrumentation extends InstrumentationBase Date: Tue, 3 Dec 2024 10:57:50 -0500 Subject: [PATCH 04/10] fix linting --- .../opentelemetry-instrumentation-fetch/src/fetch.ts | 12 ++++++++++-- .../src/xhr.ts | 12 ++++++++++-- packages/opentelemetry-sdk-trace-web/src/utils.ts | 2 +- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts index dfa8a55a22..ca19b31659 100644 --- a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts +++ b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts @@ -112,7 +112,11 @@ export class FetchInstrumentation extends InstrumentationBase Date: Tue, 3 Dec 2024 10:58:02 -0500 Subject: [PATCH 05/10] add test for fetch --- .../test/fetch.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/experimental/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts b/experimental/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts index e76d984324..955c734fad 100644 --- a/experimental/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts @@ -1212,5 +1212,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` + ); + }); }); }); From e1d56d39f670440afc1a3025838a7d3a94604c20 Mon Sep 17 00:00:00 2001 From: Arriana Blais Date: Tue, 3 Dec 2024 11:06:30 -0500 Subject: [PATCH 06/10] test for content length --- .../test/xhr.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) 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 481e3f1538..ede395200a 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 @@ -857,6 +857,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 bd5e6e37de8363eb48cbf08f560673aa66e11bdf Mon Sep 17 00:00:00 2001 From: Arriana Blais Date: Tue, 3 Dec 2024 11:16:19 -0500 Subject: [PATCH 07/10] test for utility function --- .../test/utils.test.ts | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/packages/opentelemetry-sdk-trace-web/test/utils.test.ts b/packages/opentelemetry-sdk-trace-web/test/utils.test.ts index b70061e489..212eebceba 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, false); + 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(); From d645e6ce22d9448a4f180d922236dd8343179ab2 Mon Sep 17 00:00:00 2001 From: Arriana Blais Date: Thu, 5 Dec 2024 12:26:28 -0500 Subject: [PATCH 08/10] update changelog --- semantic-conventions/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/semantic-conventions/CHANGELOG.md b/semantic-conventions/CHANGELOG.md index 4bf95ff047..2bbc382b34 100644 --- a/semantic-conventions/CHANGELOG.md +++ b/semantic-conventions/CHANGELOG.md @@ -11,6 +11,8 @@ All notable changes to the semantic-conventions package will be documented in th ### :bug: (Bug Fix) +* bug: 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) From 679a84e8f93bcd5bd95a6e63a99cfafc7c303fd0 Mon Sep 17 00:00:00 2001 From: Arriana Blais Date: Fri, 6 Dec 2024 09:32:00 -0500 Subject: [PATCH 09/10] remove wip code --- packages/opentelemetry-sdk-trace-web/test/utils.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry-sdk-trace-web/test/utils.test.ts b/packages/opentelemetry-sdk-trace-web/test/utils.test.ts index 212eebceba..0011dde148 100644 --- a/packages/opentelemetry-sdk-trace-web/test/utils.test.ts +++ b/packages/opentelemetry-sdk-trace-web/test/utils.test.ts @@ -151,7 +151,7 @@ describe('utils', () => { assert.strictEqual(addEventSpy.callCount, 0); - addSpanNetworkEvents(span, entries, false); + 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); From 43f5627b701dbef28a5a9d6412f5c60084201f08 Mon Sep 17 00:00:00 2001 From: Arriana Blais Date: Thu, 12 Dec 2024 09:14:32 -0500 Subject: [PATCH 10/10] move to correct changelog --- CHANGELOG.md | 2 ++ semantic-conventions/CHANGELOG.md | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f378c59c1..f7b5745a6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,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/semantic-conventions/CHANGELOG.md b/semantic-conventions/CHANGELOG.md index 2bbc382b34..4bf95ff047 100644 --- a/semantic-conventions/CHANGELOG.md +++ b/semantic-conventions/CHANGELOG.md @@ -11,8 +11,6 @@ All notable changes to the semantic-conventions package will be documented in th ### :bug: (Bug Fix) -* bug: 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)