Skip to content

Commit

Permalink
refactor(instrumentation-xhr): use exported strings for semantic attr…
Browse files Browse the repository at this point in the history
…ibutes

Signed-off-by: Prashansa Kulshrestha <prashkulshrestha@gmail.com>
  • Loading branch information
Prashansa-K committed May 6, 2024
1 parent 1c5de7a commit 60d0405
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 49 deletions.
1 change: 1 addition & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -156,23 +163,20 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase<XMLHttpRe
if (typeof spanUrl === 'string') {
const parsedUrl = parseUrl(spanUrl);
if (xhrMem.status !== undefined) {
span.setAttribute(SemanticAttributes.HTTP_STATUS_CODE, xhrMem.status);
span.setAttribute(SEMATTRS_HTTP_STATUS_CODE, xhrMem.status);
}
if (xhrMem.statusText !== undefined) {
span.setAttribute(AttributeNames.HTTP_STATUS_TEXT, xhrMem.statusText);
}
span.setAttribute(SemanticAttributes.HTTP_HOST, parsedUrl.host);
span.setAttribute(SEMATTRS_HTTP_HOST, parsedUrl.host);
span.setAttribute(
SemanticAttributes.HTTP_SCHEME,
SEMATTRS_HTTP_SCHEME,
parsedUrl.protocol.replace(':', '')
);

// @TODO do we want to collect this or it will be collected earlier once only or
// maybe when parent span is not available ?
span.setAttribute(
SemanticAttributes.HTTP_USER_AGENT,
navigator.userAgent
);
span.setAttribute(SEMATTRS_HTTP_USER_AGENT, navigator.userAgent);
}
}

Expand Down Expand Up @@ -336,8 +340,8 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase<XMLHttpRe
const currentSpan = this.tracer.startSpan(spanName, {
kind: api.SpanKind.CLIENT,
attributes: {
[SemanticAttributes.HTTP_METHOD]: method,
[SemanticAttributes.HTTP_URL]: parseUrl(url).toString(),
[SEMATTRS_HTTP_METHOD]: method,
[SEMATTRS_HTTP_URL]: parseUrl(url).toString(),
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/
import { Span } from '@opentelemetry/api';
import { registerInstrumentations } from '@opentelemetry/instrumentation';
import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
import { SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH } from '@opentelemetry/semantic-conventions';
import { ReadableSpan, SpanProcessor } from '@opentelemetry/sdk-trace-base';
import { WebTracerProvider } from '@opentelemetry/sdk-trace-web';
import { XMLHttpRequestInstrumentation } from '../src';
Expand Down Expand Up @@ -65,9 +65,7 @@ describe('unmocked xhr', () => {
// 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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]],
Expand All @@ -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');
Expand Down Expand Up @@ -713,7 +721,7 @@ describe('xhr', () => {
assert.strictEqual(
attributes[keys[1]],
secondUrl,
`attribute ${SemanticAttributes.HTTP_URL} is wrong`
`attribute ${SEMATTRS_HTTP_URL} is wrong`
);
});
});
Expand Down Expand Up @@ -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`
);
});
});
Expand Down Expand Up @@ -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]],
Expand All @@ -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');
Expand Down Expand Up @@ -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]],
Expand All @@ -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');
Expand Down Expand Up @@ -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]],
Expand All @@ -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');
Expand Down Expand Up @@ -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]],
Expand All @@ -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');
Expand Down

0 comments on commit 60d0405

Please sign in to comment.