Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: improve http span name #3603

Merged
merged 6 commits into from
Feb 28, 2023
Merged

feat: improve http span name #3603

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0b164f4
feat: improve http span name
Flarna Feb 10, 2023
786f392
Update experimental/CHANGELOG.md
Flarna Feb 10, 2023
f325e76
Merge branch 'main' into http-name
Flarna Feb 23, 2023
74f7a25
Merge branch 'main' into http-name
Flarna Feb 27, 2023
cde2c6e
improve changelog
Flarna Feb 27, 2023
ac99ffc
Merge branch 'main' into http-name
Flarna Feb 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions experimental/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@ All notable changes to experimental packages in this project will be documented

### :boom: Breaking Change

* feat: remove HTTP/HTTPS prefix from span name [#3603](https://github.com/open-telemetry/opentelemetry-js/pull/3603) @Flarna

### :rocket: (Enhancement)

* feat: use HTTP_ROUTE in span name [#3603](https://github.com/open-telemetry/opentelemetry-js/pull/3603) @Flarna
* feat: add HTTP_ROUTE attribute to http incoming metrics if present [#3581](https://github.com/open-telemetry/opentelemetry-js/pull/3581) @hermogenes
* feat(sdk-node): install diag logger with OTEL_LOG_LEVEL [#3627](https://github.com/open-telemetry/opentelemetry-js/pull/3627) @legendecas

Expand Down
15 changes: 8 additions & 7 deletions experimental/packages/opentelemetry-instrumentation-http/src/http.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import {
} from '@opentelemetry/instrumentation';
import { RPCMetadata, RPCType, setRPCMetadata } from '@opentelemetry/core';
import { errorMonitor } from 'events';
import { SemanticAttributes } from '@opentelemetry/semantic-conventions';

/**
* Http instrumentation instrumentation for Opentelemetry
Expand Down Expand Up @@ -489,11 +490,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
utils.getIncomingRequestMetricAttributes(spanAttributes);

const ctx = propagation.extract(ROOT_CONTEXT, headers);
const span = instrumentation._startHttpSpan(
`${component.toLocaleUpperCase()} ${method}`,
spanOptions,
ctx
);
const span = instrumentation._startHttpSpan(method, spanOptions, ctx);
const rpcMetadata: RPCMetadata = {
type: RPCType.HTTP,
span,
Expand Down Expand Up @@ -622,7 +619,6 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
return original.apply(this, [optionsParsed, ...args]);
}

const operationName = `${component.toUpperCase()} ${method}`;
const { hostname, port } = utils.extractHostnameAndPort(optionsParsed);

const attributes = utils.getOutgoingRequestAttributes(optionsParsed, {
Expand All @@ -643,7 +639,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
kind: SpanKind.CLIENT,
attributes,
};
const span = instrumentation._startHttpSpan(operationName, spanOptions);
const span = instrumentation._startHttpSpan(method, spanOptions);

const parentContext = context.active();
const requestContext = trace.setSpan(parentContext, span);
Expand Down Expand Up @@ -717,6 +713,11 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
code: utils.parseResponseStatus(SpanKind.SERVER, response.statusCode),
});

const route = attributes[SemanticAttributes.HTTP_ROUTE];
if (route) {
span.updateName(`${request.method || 'GET'} ${route}`);
}
Comment on lines +716 to +719
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe an entry for this new feature in the changelog as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


if (this._getConfig().applyCustomAttributesOnSpan) {
safeExecuteInTheMiddle(
() =>
Expand Down
27 changes: 24 additions & 3 deletions ...rimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,13 @@ describe('HttpInstrumentation', () => {
if (request.url?.includes('/ignored')) {
provider.getTracer('test').startSpan('some-span').end();
}
if (request.url?.includes('/setroute')) {
const rpcData = getRPCMetadata(context.active());
assert.ok(rpcData != null);
assert.strictEqual(rpcData.type, RPCType.HTTP);
assert.strictEqual(rpcData.route, undefined);
rpcData.route = 'TheRoute';
}
response.end('Test Server Response');
});

Expand Down Expand Up @@ -339,6 +346,20 @@ describe('HttpInstrumentation', () => {
});
});

it('should respect HTTP_ROUTE', async () => {
await httpRequest.get(
`${protocol}://${hostname}:${serverPort}/setroute`
);
const span = memoryExporter.getFinishedSpans()[0];

assert.strictEqual(span.kind, SpanKind.SERVER);
assert.strictEqual(
span.attributes[SemanticAttributes.HTTP_ROUTE],
'TheRoute'
);
assert.strictEqual(span.name, 'GET TheRoute');
});

const httpErrorCodes = [
400, 401, 403, 404, 429, 501, 503, 504, 500, 505, 597,
];
Expand Down Expand Up @@ -404,7 +425,7 @@ describe('HttpInstrumentation', () => {

assert.ok(localSpan.name.indexOf('TestRootSpan') >= 0);
assert.strictEqual(spans.length, 2);
assert.strictEqual(reqSpan.name, 'HTTP GET');
assert.strictEqual(reqSpan.name, 'GET');
assert.strictEqual(
localSpan.spanContext().traceId,
reqSpan.spanContext().traceId
Expand Down Expand Up @@ -449,7 +470,7 @@ describe('HttpInstrumentation', () => {

assert.ok(localSpan.name.indexOf('TestRootSpan') >= 0);
assert.strictEqual(spans.length, 2);
assert.strictEqual(reqSpan.name, 'HTTP GET');
assert.strictEqual(reqSpan.name, 'GET');
assert.strictEqual(
localSpan.spanContext().traceId,
reqSpan.spanContext().traceId
Expand All @@ -474,7 +495,7 @@ describe('HttpInstrumentation', () => {
for (let i = 0; i < num; i++) {
await httpRequest.get(`${protocol}://${hostname}${testPath}`);
const spans = memoryExporter.getFinishedSpans();
assert.strictEqual(spans[i].name, 'HTTP GET');
assert.strictEqual(spans[i].name, 'GET');
assert.strictEqual(
span.spanContext().traceId,
spans[i].spanContext().traceId
Expand Down
2 changes: 1 addition & 1 deletion ...imental/packages/opentelemetry-instrumentation-http/test/functionals/http-package.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ describe('Packages', () => {
};

assert.strictEqual(spans.length, 1);
assert.strictEqual(span.name, 'HTTP GET');
assert.strictEqual(span.name, 'GET');

switch (name) {
case 'axios':
Expand Down
6 changes: 3 additions & 3 deletions ...imental/packages/opentelemetry-instrumentation-http/test/functionals/https-enable.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ describe('HttpsInstrumentation', () => {

assert.ok(localSpan.name.indexOf('TestRootSpan') >= 0);
assert.strictEqual(spans.length, 2);
assert.strictEqual(reqSpan.name, 'HTTPS GET');
assert.strictEqual(reqSpan.name, 'GET');
assert.strictEqual(
localSpan.spanContext().traceId,
reqSpan.spanContext().traceId
Expand Down Expand Up @@ -401,7 +401,7 @@ describe('HttpsInstrumentation', () => {

assert.ok(localSpan.name.indexOf('TestRootSpan') >= 0);
assert.strictEqual(spans.length, 2);
assert.strictEqual(reqSpan.name, 'HTTPS GET');
assert.strictEqual(reqSpan.name, 'GET');
assert.strictEqual(
localSpan.spanContext().traceId,
reqSpan.spanContext().traceId
Expand All @@ -426,7 +426,7 @@ describe('HttpsInstrumentation', () => {
for (let i = 0; i < num; i++) {
await httpsRequest.get(`${protocol}://${hostname}${testPath}`);
const spans = memoryExporter.getFinishedSpans();
assert.strictEqual(spans[i].name, 'HTTPS GET');
assert.strictEqual(spans[i].name, 'GET');
assert.strictEqual(
span.spanContext().traceId,
spans[i].spanContext().traceId
Expand Down
2 changes: 1 addition & 1 deletion ...mental/packages/opentelemetry-instrumentation-http/test/functionals/https-package.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ describe('Packages', () => {
};

assert.strictEqual(spans.length, 1);
assert.strictEqual(span.name, 'HTTPS GET');
assert.strictEqual(span.name, 'GET');

switch (name) {
case 'axios':
Expand Down
16 changes: 8 additions & 8 deletions ...imental/packages/opentelemetry-instrumentation-http/test/integrations/http-enable.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ describe('HttpInstrumentation Integration tests', () => {
};

assert.strictEqual(spans.length, 2);
assert.strictEqual(span.name, 'HTTP GET');
assert.strictEqual(span.name, 'GET');
assertSpan(span, SpanKind.CLIENT, validations);
});

Expand All @@ -203,7 +203,7 @@ describe('HttpInstrumentation Integration tests', () => {
};

assert.strictEqual(spans.length, 2);
assert.strictEqual(span.name, 'HTTP GET');
assert.strictEqual(span.name, 'GET');
assertSpan(span, SpanKind.CLIENT, validations);
});

Expand Down Expand Up @@ -233,7 +233,7 @@ describe('HttpInstrumentation Integration tests', () => {
};

assert.strictEqual(spans.length, 2);
assert.strictEqual(span.name, 'HTTP GET');
assert.strictEqual(span.name, 'GET');
assert.strictEqual(result.reqHeaders['x-foo'], 'foo');
assert.strictEqual(
span.attributes[SemanticAttributes.HTTP_FLAVOR],
Expand Down Expand Up @@ -264,7 +264,7 @@ describe('HttpInstrumentation Integration tests', () => {
};

assert.strictEqual(spans.length, 2);
assert.strictEqual(span.name, 'HTTP GET');
assert.strictEqual(span.name, 'GET');
assert.strictEqual(span.attributes['span kind'], SpanKind.CLIENT);
assertSpan(span, SpanKind.CLIENT, validations);
});
Expand Down Expand Up @@ -292,7 +292,7 @@ describe('HttpInstrumentation Integration tests', () => {
};

assert.strictEqual(spans.length, 2);
assert.strictEqual(span.name, 'HTTP GET');
assert.strictEqual(span.name, 'GET');
assertSpan(span, SpanKind.CLIENT, validations);
});
for (const headers of [
Expand Down Expand Up @@ -347,7 +347,7 @@ describe('HttpInstrumentation Integration tests', () => {
const span = spans.find(s => s.kind === SpanKind.CLIENT);
assert.ok(span);
assert.strictEqual(spans.length, 2);
assert.strictEqual(span.name, 'HTTP GET');
assert.strictEqual(span.name, 'GET');
assert.ok(data);
assert.ok(validations.reqHeaders[DummyPropagation.TRACE_CONTEXT_KEY]);
assert.ok(validations.reqHeaders[DummyPropagation.SPAN_CONTEXT_KEY]);
Expand All @@ -362,7 +362,7 @@ describe('HttpInstrumentation Integration tests', () => {
const span = spans.find((s: any) => s.kind === SpanKind.SERVER);
assert.ok(span);
assert.strictEqual(spans.length, 2);
assert.strictEqual(span.name, 'HTTP GET');
assert.strictEqual(span.name, 'GET');
});

it('should have correct spans even when request timeout', async () => {
Expand All @@ -381,7 +381,7 @@ describe('HttpInstrumentation Integration tests', () => {
spans = memoryExporter.getFinishedSpans();
const span = spans.find(s => s.kind === SpanKind.CLIENT);
assert.ok(span);
assert.strictEqual(span.name, 'HTTP GET');
assert.strictEqual(span.name, 'GET');
assert.strictEqual(
span.attributes[SemanticAttributes.HTTP_HOST],
`localhost:${mockServerPort}`
Expand Down
12 changes: 6 additions & 6 deletions ...mental/packages/opentelemetry-instrumentation-http/test/integrations/https-enable.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ describe('HttpsInstrumentation Integration tests', () => {
};

assert.strictEqual(spans.length, 2);
assert.strictEqual(span.name, 'HTTPS GET');
assert.strictEqual(span.name, 'GET');
assertSpan(span, SpanKind.CLIENT, validations);
});

Expand All @@ -207,7 +207,7 @@ describe('HttpsInstrumentation Integration tests', () => {
};

assert.strictEqual(spans.length, 2);
assert.strictEqual(span.name, 'HTTPS GET');
assert.strictEqual(span.name, 'GET');
assertSpan(span, SpanKind.CLIENT, validations);
});

Expand Down Expand Up @@ -237,7 +237,7 @@ describe('HttpsInstrumentation Integration tests', () => {
};

assert.strictEqual(spans.length, 2);
assert.strictEqual(span.name, 'HTTPS GET');
assert.strictEqual(span.name, 'GET');
assert.strictEqual(result.reqHeaders['x-foo'], 'foo');
assert.strictEqual(
span.attributes[SemanticAttributes.HTTP_FLAVOR],
Expand Down Expand Up @@ -268,7 +268,7 @@ describe('HttpsInstrumentation Integration tests', () => {
};

assert.strictEqual(spans.length, 2);
assert.strictEqual(span.name, 'HTTPS GET');
assert.strictEqual(span.name, 'GET');
assert.strictEqual(span.attributes['span kind'], SpanKind.CLIENT);
assertSpan(span, SpanKind.CLIENT, validations);
});
Expand Down Expand Up @@ -296,7 +296,7 @@ describe('HttpsInstrumentation Integration tests', () => {
};

assert.strictEqual(spans.length, 2);
assert.strictEqual(span.name, 'HTTPS GET');
assert.strictEqual(span.name, 'GET');
assertSpan(span, SpanKind.CLIENT, validations);
});
for (const headers of [
Expand Down Expand Up @@ -351,7 +351,7 @@ describe('HttpsInstrumentation Integration tests', () => {
const span = spans.find(s => s.kind === SpanKind.CLIENT);
assert.ok(span);
assert.strictEqual(spans.length, 2);
assert.strictEqual(span.name, 'HTTPS GET');
assert.strictEqual(span.name, 'GET');
assert.ok(data);
assert.ok(validations.reqHeaders[DummyPropagation.TRACE_CONTEXT_KEY]);
assert.ok(validations.reqHeaders[DummyPropagation.SPAN_CONTEXT_KEY]);
Expand Down
5 changes: 1 addition & 4 deletions experimental/packages/opentelemetry-instrumentation-http/test/utils/assertSpan.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,7 @@ export const assertSpan = (
assert.strictEqual(span.spanContext().traceId.length, 32);
assert.strictEqual(span.spanContext().spanId.length, 16);
assert.strictEqual(span.kind, kind);
assert.strictEqual(
span.name,
`${validations.component.toUpperCase()} ${validations.httpMethod}`
);
assert.strictEqual(span.name, validations.httpMethod);
assert.strictEqual(
span.attributes[AttributeNames.HTTP_ERROR_MESSAGE],
span.status.message
Expand Down