From 60d040504eb636c223fdbcd301b70259d6716868 Mon Sep 17 00:00:00 2001 From: Prashansa Kulshrestha Date: Mon, 6 May 2024 12:21:07 +0530 Subject: [PATCH 1/3] refactor(instrumentation-xhr): use exported strings for semantic attributes Signed-off-by: Prashansa Kulshrestha --- experimental/CHANGELOG.md | 1 + .../src/xhr.ts | 24 +++--- .../test/unmocked.test.ts | 6 +- .../test/xhr.test.ts | 78 ++++++++++--------- 4 files changed, 60 insertions(+), 49 deletions(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index f336cb0de9..1963e9b2fd 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -22,6 +22,7 @@ All notable changes to experimental packages in this project will be documented * feat: support node 22 [#4666](https://github.com/open-telemetry/opentelemetry-js/pull/4666) @dyladan * feat(propagator-aws-xray-lambda): add AWS Xray Lambda propagator [4554](https://github.com/open-telemetry/opentelemetry-js/pull/4554) +* refactor(instrumentation-xml-http-request): use exported strings for semantic attributes. ### :bug: (Bug Fix) 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 57dbe79744..4c840097f5 100644 --- a/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts +++ b/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts @@ -22,7 +22,14 @@ import { safeExecuteInTheMiddle, } from '@opentelemetry/instrumentation'; import { hrTime, isUrlIgnored, otperformance } from '@opentelemetry/core'; -import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; +import { + SEMATTRS_HTTP_HOST, + SEMATTRS_HTTP_METHOD, + SEMATTRS_HTTP_SCHEME, + SEMATTRS_HTTP_STATUS_CODE, + SEMATTRS_HTTP_URL, + SEMATTRS_HTTP_USER_AGENT, +} from '@opentelemetry/semantic-conventions'; import { addSpanNetworkEvents, getResource, @@ -156,23 +163,20 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase { // content length comes from the PerformanceTiming resource; this ensures that our // matching logic found the right one assert.ok( - (span.attributes[ - SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH - ] as any) > 0 + (span.attributes[SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH] as any) > 0 ); done(); }, 500); 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 7a55b5c9d8..608e309dd4 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 @@ -25,7 +25,15 @@ import { } from '@opentelemetry/propagator-b3'; import { ZoneContextManager } from '@opentelemetry/context-zone'; import * as tracing from '@opentelemetry/sdk-trace-base'; -import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; +import { + SEMATTRS_HTTP_HOST, + SEMATTRS_HTTP_METHOD, + SEMATTRS_HTTP_REQUEST_CONTENT_LENGTH, + SEMATTRS_HTTP_SCHEME, + SEMATTRS_HTTP_STATUS_CODE, + SEMATTRS_HTTP_URL, + SEMATTRS_HTTP_USER_AGENT, +} from '@opentelemetry/semantic-conventions'; import { PerformanceTimingNames as PTN, WebTracerProvider, @@ -356,12 +364,12 @@ describe('xhr', () => { assert.strictEqual( attributes[keys[0]], 'GET', - `attributes ${SemanticAttributes.HTTP_METHOD} is wrong` + `attributes ${SEMATTRS_HTTP_METHOD} is wrong` ); assert.strictEqual( attributes[keys[1]], url, - `attributes ${SemanticAttributes.HTTP_URL} is wrong` + `attributes ${SEMATTRS_HTTP_URL} is wrong` ); assert.ok( (attributes[keys[2]] as number) > 0, @@ -370,7 +378,7 @@ describe('xhr', () => { assert.strictEqual( attributes[keys[3]], 200, - `attributes ${SemanticAttributes.HTTP_STATUS_CODE} is wrong` + `attributes ${SEMATTRS_HTTP_STATUS_CODE} is wrong` ); assert.strictEqual( attributes[keys[4]], @@ -380,15 +388,15 @@ describe('xhr', () => { assert.strictEqual( attributes[keys[5]], parseUrl(url).host, - `attributes ${SemanticAttributes.HTTP_HOST} is wrong` + `attributes ${SEMATTRS_HTTP_HOST} is wrong` ); assert.ok( attributes[keys[6]] === 'http' || attributes[keys[6]] === 'https', - `attributes ${SemanticAttributes.HTTP_SCHEME} is wrong` + `attributes ${SEMATTRS_HTTP_SCHEME} is wrong` ); assert.ok( attributes[keys[7]] !== '', - `attributes ${SemanticAttributes.HTTP_USER_AGENT} is not defined` + `attributes ${SEMATTRS_HTTP_USER_AGENT} is not defined` ); assert.strictEqual(keys.length, 8, 'number of attributes is wrong'); @@ -713,7 +721,7 @@ describe('xhr', () => { assert.strictEqual( attributes[keys[1]], secondUrl, - `attribute ${SemanticAttributes.HTTP_URL} is wrong` + `attribute ${SEMATTRS_HTTP_URL} is wrong` ); }); }); @@ -777,9 +785,9 @@ describe('xhr', () => { const attributes = span.attributes; assert.strictEqual( - attributes[SemanticAttributes.HTTP_URL], + attributes[SEMATTRS_HTTP_URL], location.origin + '/get', - `attributes ${SemanticAttributes.HTTP_URL} is wrong` + `attributes ${SEMATTRS_HTTP_URL} is wrong` ); }); }); @@ -955,22 +963,22 @@ describe('xhr', () => { assert.strictEqual( attributes[keys[0]], 'GET', - `attributes ${SemanticAttributes.HTTP_METHOD} is wrong` + `attributes ${SEMATTRS_HTTP_METHOD} is wrong` ); assert.strictEqual( attributes[keys[1]], url, - `attributes ${SemanticAttributes.HTTP_URL} is wrong` + `attributes ${SEMATTRS_HTTP_URL} is wrong` ); assert.strictEqual( attributes[keys[2]], 0, - `attributes ${SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH} is wrong` + `attributes ${SEMATTRS_HTTP_REQUEST_CONTENT_LENGTH} is wrong` ); assert.strictEqual( attributes[keys[3]], 400, - `attributes ${SemanticAttributes.HTTP_STATUS_CODE} is wrong` + `attributes ${SEMATTRS_HTTP_STATUS_CODE} is wrong` ); assert.strictEqual( attributes[keys[4]], @@ -980,15 +988,15 @@ describe('xhr', () => { assert.strictEqual( attributes[keys[5]], 'raw.githubusercontent.com', - `attributes ${SemanticAttributes.HTTP_HOST} is wrong` + `attributes ${SEMATTRS_HTTP_HOST} is wrong` ); assert.ok( attributes[keys[6]] === 'http' || attributes[keys[6]] === 'https', - `attributes ${SemanticAttributes.HTTP_SCHEME} is wrong` + `attributes ${SEMATTRS_HTTP_SCHEME} is wrong` ); assert.ok( attributes[keys[7]] !== '', - `attributes ${SemanticAttributes.HTTP_USER_AGENT} is not defined` + `attributes ${SEMATTRS_HTTP_USER_AGENT} is not defined` ); assert.strictEqual(keys.length, 8, 'number of attributes is wrong'); @@ -1029,17 +1037,17 @@ describe('xhr', () => { assert.strictEqual( attributes[keys[0]], 'GET', - `attributes ${SemanticAttributes.HTTP_METHOD} is wrong` + `attributes ${SEMATTRS_HTTP_METHOD} is wrong` ); assert.strictEqual( attributes[keys[1]], url, - `attributes ${SemanticAttributes.HTTP_URL} is wrong` + `attributes ${SEMATTRS_HTTP_URL} is wrong` ); assert.strictEqual( attributes[keys[2]], 0, - `attributes ${SemanticAttributes.HTTP_STATUS_CODE} is wrong` + `attributes ${SEMATTRS_HTTP_STATUS_CODE} is wrong` ); assert.strictEqual( attributes[keys[3]], @@ -1049,15 +1057,15 @@ describe('xhr', () => { assert.strictEqual( attributes[keys[4]], 'raw.githubusercontent.com', - `attributes ${SemanticAttributes.HTTP_HOST} is wrong` + `attributes ${SEMATTRS_HTTP_HOST} is wrong` ); assert.ok( attributes[keys[5]] === 'http' || attributes[keys[5]] === 'https', - `attributes ${SemanticAttributes.HTTP_SCHEME} is wrong` + `attributes ${SEMATTRS_HTTP_SCHEME} is wrong` ); assert.ok( attributes[keys[6]] !== '', - `attributes ${SemanticAttributes.HTTP_USER_AGENT} is not defined` + `attributes ${SEMATTRS_HTTP_USER_AGENT} is not defined` ); assert.strictEqual(keys.length, 7, 'number of attributes is wrong'); @@ -1095,17 +1103,17 @@ describe('xhr', () => { assert.strictEqual( attributes[keys[0]], 'GET', - `attributes ${SemanticAttributes.HTTP_METHOD} is wrong` + `attributes ${SEMATTRS_HTTP_METHOD} is wrong` ); assert.strictEqual( attributes[keys[1]], url, - `attributes ${SemanticAttributes.HTTP_URL} is wrong` + `attributes ${SEMATTRS_HTTP_URL} is wrong` ); assert.strictEqual( attributes[keys[2]], 0, - `attributes ${SemanticAttributes.HTTP_STATUS_CODE} is wrong` + `attributes ${SEMATTRS_HTTP_STATUS_CODE} is wrong` ); assert.strictEqual( attributes[keys[3]], @@ -1115,15 +1123,15 @@ describe('xhr', () => { assert.strictEqual( attributes[keys[4]], 'raw.githubusercontent.com', - `attributes ${SemanticAttributes.HTTP_HOST} is wrong` + `attributes ${SEMATTRS_HTTP_HOST} is wrong` ); assert.ok( attributes[keys[5]] === 'http' || attributes[keys[5]] === 'https', - `attributes ${SemanticAttributes.HTTP_SCHEME} is wrong` + `attributes ${SEMATTRS_HTTP_SCHEME} is wrong` ); assert.ok( attributes[keys[6]] !== '', - `attributes ${SemanticAttributes.HTTP_USER_AGENT} is not defined` + `attributes ${SEMATTRS_HTTP_USER_AGENT} is not defined` ); assert.strictEqual(keys.length, 7, 'number of attributes is wrong'); @@ -1161,17 +1169,17 @@ describe('xhr', () => { assert.strictEqual( attributes[keys[0]], 'GET', - `attributes ${SemanticAttributes.HTTP_METHOD} is wrong` + `attributes ${SEMATTRS_HTTP_METHOD} is wrong` ); assert.strictEqual( attributes[keys[1]], url, - `attributes ${SemanticAttributes.HTTP_URL} is wrong` + `attributes ${SEMATTRS_HTTP_URL} is wrong` ); assert.strictEqual( attributes[keys[2]], 0, - `attributes ${SemanticAttributes.HTTP_STATUS_CODE} is wrong` + `attributes ${SEMATTRS_HTTP_STATUS_CODE} is wrong` ); assert.strictEqual( attributes[keys[3]], @@ -1181,15 +1189,15 @@ describe('xhr', () => { assert.strictEqual( attributes[keys[4]], 'raw.githubusercontent.com', - `attributes ${SemanticAttributes.HTTP_HOST} is wrong` + `attributes ${SEMATTRS_HTTP_HOST} is wrong` ); assert.ok( attributes[keys[5]] === 'http' || attributes[keys[5]] === 'https', - `attributes ${SemanticAttributes.HTTP_SCHEME} is wrong` + `attributes ${SEMATTRS_HTTP_SCHEME} is wrong` ); assert.ok( attributes[keys[6]] !== '', - `attributes ${SemanticAttributes.HTTP_USER_AGENT} is not defined` + `attributes ${SEMATTRS_HTTP_USER_AGENT} is not defined` ); assert.strictEqual(keys.length, 7, 'number of attributes is wrong'); From 2c6b31acc5ebe11991d3060c0e9468cf6cafe637 Mon Sep 17 00:00:00 2001 From: Prashansa Kulshrestha Date: Mon, 6 May 2024 12:34:52 +0530 Subject: [PATCH 2/3] Updated changelog entry with PR id and link Signed-off-by: Prashansa Kulshrestha --- experimental/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 1963e9b2fd..1b1dfeee1d 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -22,7 +22,7 @@ All notable changes to experimental packages in this project will be documented * feat: support node 22 [#4666](https://github.com/open-telemetry/opentelemetry-js/pull/4666) @dyladan * feat(propagator-aws-xray-lambda): add AWS Xray Lambda propagator [4554](https://github.com/open-telemetry/opentelemetry-js/pull/4554) -* refactor(instrumentation-xml-http-request): use exported strings for semantic attributes. +* refactor(instrumentation-xml-http-request): use exported strings for semantic attributes. [#4681](https://github.com/open-telemetry/opentelemetry-js/pull/4681/files) ### :bug: (Bug Fix) From f13b18b82a570446a6c034bbb03e876a02beac93 Mon Sep 17 00:00:00 2001 From: Prashansa Kulshrestha Date: Mon, 6 May 2024 19:41:22 +0530 Subject: [PATCH 3/3] Changed normal string to template string and replaced old HTTP_RESPONSE_CONTENT_SIZE to new exported string Signed-off-by: Prashansa Kulshrestha --- .../test/xhr.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 608e309dd4..f3685e06a1 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 @@ -28,6 +28,7 @@ import * as tracing from '@opentelemetry/sdk-trace-base'; import { SEMATTRS_HTTP_HOST, SEMATTRS_HTTP_METHOD, + SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH, SEMATTRS_HTTP_REQUEST_CONTENT_LENGTH, SEMATTRS_HTTP_SCHEME, SEMATTRS_HTTP_STATUS_CODE, @@ -373,7 +374,7 @@ describe('xhr', () => { ); assert.ok( (attributes[keys[2]] as number) > 0, - 'attributes ${SemanticAttributess.HTTP_RESPONSE_CONTENT_SIZE} <= 0' + `attributes ${SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH} <= 0` ); assert.strictEqual( attributes[keys[3]],