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.

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

Http server semconv span stable #4978

Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
c5f4b41
feat(instr-http): emit stable semconv for client spans
dyladan Aug 21, 2024
3d1b95d
Document OTEL_SEMCONV_STABILITY_OPT_IN for http
dyladan Aug 21, 2024
d978eb1
Lint
dyladan Aug 21, 2024
963e19a
Merge remote-tracking branch 'origin/main' into http-client-semconv-s…
dyladan Aug 22, 2024
dc75e57
Fix build
dyladan Aug 22, 2024
c71cdb0
Changelog
dyladan Aug 22, 2024
eae1d5c
lint
dyladan Aug 22, 2024
f7efbe8
Add tests for 1.27 semconv
dyladan Aug 23, 2024
2d60245
Merge remote-tracking branch 'origin/main' into http-client-semconv-s…
dyladan Aug 23, 2024
5031276
Use actual remote address for tests
dyladan Aug 23, 2024
10e49f1
Apply fix to stable spans
dyladan Aug 23, 2024
e4a36b1
Merge branch 'main' into http-client-semconv-span-stable
dyladan Aug 28, 2024
9b5ae5b
Update experimental/packages/opentelemetry-instrumentation-http/src/u…
dyladan Sep 4, 2024
83b91f0
chore(deps): update dependency chromedriver to v128 (#4954)
renovate-bot Aug 28, 2024
61f344d
chore(deps): update dependency webpack to v5.94.0 (#4961)
pichlermarc Aug 28, 2024
362a1bc
chore(deps): update dependency glob to v11 (#4955)
renovate-bot Aug 28, 2024
9e2485d
chore(deps): update all patch versions (#4942)
renovate-bot Aug 28, 2024
08c5a9a
chore(deps): update dependency chromedriver to v128.0.1 (#4967)
renovate-bot Sep 2, 2024
4920848
chore(deps): lock file maintenance (#4968)
renovate-bot Sep 2, 2024
93d43ce
Finish transition plan in readme
dyladan Sep 4, 2024
e725146
Review comments
dyladan Sep 4, 2024
0635448
Await server.listen in before to be sure it finishes
dyladan Sep 4, 2024
253c840
Lint
dyladan Sep 4, 2024
2533480
Merge remote-tracking branch 'origin/main' into http-client-semconv-s…
dyladan Sep 4, 2024
e4ccec8
Changelog
dyladan Sep 4, 2024
1bdce0f
Merge remote-tracking branch 'origin/main' into http-client-semconv-s…
dyladan Sep 4, 2024
1c9abb5
Update http server semconv to 1.27
dyladan Sep 4, 2024
9a0c2de
Add server to readme
dyladan Sep 4, 2024
3703045
Lint
dyladan Sep 4, 2024
397ebc6
Fix compilation and test
dyladan Sep 5, 2024
98e02f5
Merge remote-tracking branch 'origin/main' into http-server-semconv-s…
dyladan Sep 11, 2024
7addab4
Fix changelog
dyladan Sep 11, 2024
12d0489
Add tests for server spans
dyladan Sep 11, 2024
4c19670
Lint
dyladan Sep 11, 2024
1054a44
Changelog
dyladan Sep 11, 2024
966eaa5
Change address for tests
dyladan Sep 11, 2024
090f394
Fix node 14 tests
dyladan Sep 11, 2024
46a42e0
remove console
dyladan Sep 11, 2024
826e79b
Add a more detailed changelog
dyladan Sep 17, 2024
0df46dd
Normalize request methods
dyladan Sep 17, 2024
01b90c5
Only set attributes if they are collected
dyladan Sep 17, 2024
bc4d955
More robust host header parsing
dyladan Sep 17, 2024
52c10d4
Lint
dyladan Sep 17, 2024
8a02c47
Merge remote-tracking branch 'origin/main' into http-server-semconv-s…
dyladan Sep 17, 2024
6354a8f
Lint eqeqeq
dyladan Sep 17, 2024
c9df445
Merge branch 'main' into http-server-semconv-span-stable
dyladan Sep 20, 2024
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
2 changes: 2 additions & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ All notable changes to experimental packages in this project will be documented

### :rocket: (Enhancement)

* feat(instrumentation-http): Add support for client span semantic conventions 1.27 [#4940](https://github.com/open-telemetry/opentelemetry-js/pull/4940) @dyladan

### :bug: (Bug Fix)

* fix(sampler-jaeger-remote): fixes an issue where package could emit unhandled promise rejections @Just-Sieb
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,44 @@ The following options are deprecated:

## Semantic Conventions

This package uses `@opentelemetry/semantic-conventions` version `1.22+`, which implements Semantic Convention [Version 1.7.0](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.7.0/semantic_conventions/README.md)
### Client and Server Spans
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how best to update without making this big PR even bigger, but the section with the table of attributes will need to be updated in some way (starting on line 114 that still references Server Spans). It may be worth having two tables - 1 representing old, 1 representing new. I'm happy to help craft that table in a separate issue and we can issue a follow-up PR. I would recommend at least a small change to that section though along the lines of "These are the attributes in the default behavior using only experimental attributes".

Copy link
Member Author

Choose a reason for hiding this comment

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

I left the old table as-is for now. The new semconv is documented already in the semconv repo. When we remove this fallback the old table will be removed entirely. Would that be sufficient or do you think we need a table here as well?

Copy link
Member

Choose a reason for hiding this comment

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

I've gotten a lot of requests for showing what attributes are included in instrumentation, especially as different instrumentations (and languages) are in different versions. Folks also ask about other attributes (non-semconv) in different instrumentations, e.g. express.name, though that's less relevant for http here. The semconv might be in the semconv repo but it doesn't specify what we include, and may not be easily navigable by end users. Ideally we autogenerate a list of attributes vs handcrafting a table though, but with the table already there we may as well use it.


Prior to version `0.54`, this instrumentation created spans targeting an experimental semantic convention [Version 1.7.0](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.7.0/semantic_conventions/README.md).

This package is capable of emitting both Semantic Convention [Version 1.7.0](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.7.0/semantic_conventions/README.md) and [Version 1.27.0](https://github.com/open-telemetry/semantic-conventions/blob/v1.27.0/docs/http/http-spans.md).
It is controlled using the environment variable `OTEL_SEMCONV_STABILITY_OPT_IN`, which is a comma separated list of values.
The values `http` and `http/dup` control this instrumentation.
See details for the behavior of each of these values below.
If neither `http` or `http/dup` is included in `OTEL_SEMCONV_STABILITY_OPT_IN`, the old experimental semantic conventions will be used by default.

#### Stable Semantic Conventions 1.27

Enabled when `OTEL_SEMCONV_STABILITY_OPT_IN` contains `http` OR `http/dup`.
This is the recommended configuration, and will soon become the default behavior.

Follow all requirements and recommendations of HTTP Client and Server Span Semantic Conventions [Version 1.27.0](https://github.com/open-telemetry/semantic-conventions/blob/v1.27.0/docs/http/http-spans.md), including all required and recommended attributes.

#### Legacy Behavior (default)

Enabled when `OTEL_SEMCONV_STABILITY_OPT_IN` contains `http/dup` or DOES NOT CONTAIN `http`.
This is the current default behavior.

Create HTTP client spans which implement Semantic Convention [Version 1.7.0](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.7.0/semantic_conventions/README.md).

#### Upgrading Semantic Conventions

When upgrading to the new semantic conventions, it is recommended to do so in the following order:

1. Upgrade `@opentelemetry/instrumentation-http` to the latest version
2. Set `OTEL_SEMCONV_STABILITY_OPT_IN=http/dup` to emit both old and new semantic conventions
3. Modify alerts, dashboards, metrics, and other processes to expect the new semantic conventions
4. Set `OTEL_SEMCONV_STABILITY_OPT_IN=http` to emit only the new semantic conventions

This will cause both the old and new semantic conventions to be emitted during the transition period.

### Server Spans

This package uses `@opentelemetry/semantic-conventions` version `1.22+`, which implements Semantic Convention [Version 1.7.0](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.7.0/semantic_conventions/README.md).

Attributes collected:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
"@opentelemetry/core": "1.26.0",
"@opentelemetry/instrumentation": "0.53.0",
"@opentelemetry/semantic-conventions": "1.27.0",
"forwarded-parse": "2.1.2",
Copy link
Member Author

Choose a reason for hiding this comment

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

Note

NEW DEPENDENCY

This is needed to parse the somewhat complex forwarded header

Copy link
Member

Choose a reason for hiding this comment

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

thanks for pointing this out - I also had a quick look and it does not seem like there's a easy way to do it 😞
I think using the library is justified.

"semver": "^7.5.2"
},
"homepage": "https://github.com/open-telemetry/opentelemetry-js/tree/main/experimental/packages/opentelemetry-instrumentation-http",
Expand Down
124 changes: 84 additions & 40 deletions experimental/packages/opentelemetry-instrumentation-http/src/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,39 @@ import {
HttpInstrumentationConfig,
HttpRequestArgs,
Https,
SemconvStability,
} from './types';
import * as utils from './utils';
import { VERSION } from './version';
import {
InstrumentationBase,
InstrumentationNodeModuleDefinition,
safeExecuteInTheMiddle,
} from '@opentelemetry/instrumentation';
import { RPCMetadata, RPCType, setRPCMetadata } from '@opentelemetry/core';
import {
RPCMetadata,
RPCType,
setRPCMetadata,
getEnv,
} from '@opentelemetry/core';
import { errorMonitor } from 'events';
import { SEMATTRS_HTTP_ROUTE } from '@opentelemetry/semantic-conventions';
import {
extractHostnameAndPort,
getIncomingRequestAttributes,
getIncomingRequestAttributesOnResponse,
getIncomingRequestMetricAttributes,
getIncomingRequestMetricAttributesOnResponse,
getOutgoingRequestAttributes,
getOutgoingRequestAttributesOnResponse,
getOutgoingRequestMetricAttributes,
getOutgoingRequestMetricAttributesOnResponse,
getRequestInfo,
headerCapture,
isIgnored,
isValidOptionsType,
parseResponseStatus,
setSpanWithError,
} from './utils';

/**
* Http instrumentation instrumentation for Opentelemetry
Expand All @@ -69,9 +91,21 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
private _httpServerDurationHistogram!: Histogram;
private _httpClientDurationHistogram!: Histogram;

private _semconvStability = SemconvStability.OLD;

constructor(config: HttpInstrumentationConfig = {}) {
super('@opentelemetry/instrumentation-http', VERSION, config);
this._headerCapture = this._createHeaderCapture();

for (const entry in getEnv().OTEL_SEMCONV_STABILITY_OPT_IN) {
if (entry.toLowerCase() === 'http/dup') {
// http/dup takes highest precedence. If it is found, there is no need to read the rest of the list
this._semconvStability = SemconvStability.DUPLICATE;
break;
} else if (entry.toLowerCase() === 'http') {
this._semconvStability = SemconvStability.STABLE;
}
}
}

protected override _updateMetricInstruments() {
Expand Down Expand Up @@ -320,12 +354,14 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
if (request.listenerCount('response') <= 1) {
response.resume();
}
const responseAttributes =
utils.getOutgoingRequestAttributesOnResponse(response);
const responseAttributes = getOutgoingRequestAttributesOnResponse(
response,
this._semconvStability
);
span.setAttributes(responseAttributes);
metricAttributes = Object.assign(
metricAttributes,
utils.getOutgoingRequestMetricAttributesOnResponse(responseAttributes)
getOutgoingRequestMetricAttributesOnResponse(responseAttributes)
);

if (this.getConfig().responseHook) {
Expand Down Expand Up @@ -353,11 +389,9 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
if (response.aborted && !response.complete) {
status = { code: SpanStatusCode.ERROR };
} else {
// behaves same for new and old semconv
status = {
code: utils.parseResponseStatus(
SpanKind.CLIENT,
response.statusCode
),
code: parseResponseStatus(SpanKind.CLIENT, response.statusCode),
};
}

Expand Down Expand Up @@ -395,7 +429,7 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
return;
}
responseFinished = true;
utils.setSpanWithError(span, error);
setSpanWithError(span, error, this._semconvStability);
span.setStatus({
code: SpanStatusCode.ERROR,
message: error.message,
Expand Down Expand Up @@ -423,7 +457,7 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
return;
}
responseFinished = true;
utils.setSpanWithError(span, error);
setSpanWithError(span, error, this._semconvStability);
this._closeHttpSpan(span, SpanKind.CLIENT, startTime, metricAttributes);
});

Expand Down Expand Up @@ -458,7 +492,7 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
);

if (
utils.isIgnored(
isIgnored(
pathname,
instrumentation.getConfig().ignoreIncomingPaths,
(e: unknown) =>
Expand Down Expand Up @@ -487,13 +521,14 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation

const headers = request.headers;

const spanAttributes = utils.getIncomingRequestAttributes(request, {
const spanAttributes = getIncomingRequestAttributes(request, {
component: component,
serverName: instrumentation.getConfig().serverName,
hookAttributes: instrumentation._callStartSpanHook(
request,
instrumentation.getConfig().startIncomingSpanHook
),
semconvStability: instrumentation._semconvStability,
});

const spanOptions: SpanOptions = {
Expand All @@ -503,7 +538,7 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation

const startTime = hrTime();
const metricAttributes =
utils.getIncomingRequestMetricAttributes(spanAttributes);
getIncomingRequestMetricAttributes(spanAttributes);

const ctx = propagation.extract(ROOT_CONTEXT, headers);
const span = instrumentation._startHttpSpan(method, spanOptions, ctx);
Expand Down Expand Up @@ -558,7 +593,11 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
() => original.apply(this, [event, ...args]),
error => {
if (error) {
utils.setSpanWithError(span, error);
setSpanWithError(
span,
error,
instrumentation._semconvStability
);
instrumentation._closeHttpSpan(
span,
SpanKind.SERVER,
Expand All @@ -584,15 +623,15 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
options: url.URL | http.RequestOptions | string,
...args: unknown[]
): http.ClientRequest {
if (!utils.isValidOptionsType(options)) {
if (!isValidOptionsType(options)) {
return original.apply(this, [options, ...args]);
}
const extraOptions =
typeof args[0] === 'object' &&
(typeof options === 'string' || options instanceof url.URL)
? (args.shift() as http.RequestOptions)
: undefined;
const { origin, pathname, method, optionsParsed } = utils.getRequestInfo(
const { origin, pathname, method, optionsParsed } = getRequestInfo(
options,
extraOptions
);
Expand All @@ -610,7 +649,7 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
}

if (
utils.isIgnored(
isIgnored(
origin + pathname,
instrumentation.getConfig().ignoreOutgoingUrls,
(e: unknown) =>
Expand All @@ -635,21 +674,25 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
return original.apply(this, [optionsParsed, ...args]);
}

const { hostname, port } = utils.extractHostnameAndPort(optionsParsed);

const attributes = utils.getOutgoingRequestAttributes(optionsParsed, {
component,
port,
hostname,
hookAttributes: instrumentation._callStartSpanHook(
optionsParsed,
instrumentation.getConfig().startOutgoingSpanHook
),
});
const { hostname, port } = extractHostnameAndPort(optionsParsed);

const attributes = getOutgoingRequestAttributes(
optionsParsed,
{
component,
port,
hostname,
hookAttributes: instrumentation._callStartSpanHook(
optionsParsed,
instrumentation.getConfig().startOutgoingSpanHook
),
},
instrumentation._semconvStability
);

const startTime = hrTime();
const metricAttributes: MetricAttributes =
utils.getOutgoingRequestMetricAttributes(attributes);
getOutgoingRequestMetricAttributes(attributes);

const spanOptions: SpanOptions = {
kind: SpanKind.CLIENT,
Expand Down Expand Up @@ -683,7 +726,7 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
() => original.apply(this, [optionsParsed, ...args]),
error => {
if (error) {
utils.setSpanWithError(span, error);
setSpanWithError(span, error, instrumentation._semconvStability);
instrumentation._closeHttpSpan(
span,
SpanKind.CLIENT,
Expand Down Expand Up @@ -716,21 +759,22 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
metricAttributes: MetricAttributes,
startTime: HrTime
) {
const attributes = utils.getIncomingRequestAttributesOnResponse(
const attributes = getIncomingRequestAttributesOnResponse(
request,
response
response,
this._semconvStability
);
metricAttributes = Object.assign(
metricAttributes,
utils.getIncomingRequestMetricAttributesOnResponse(attributes)
getIncomingRequestMetricAttributesOnResponse(attributes)
);

this._headerCapture.server.captureResponseHeaders(span, header =>
response.getHeader(header)
);

span.setAttributes(attributes).setStatus({
code: utils.parseResponseStatus(SpanKind.SERVER, response.statusCode),
code: parseResponseStatus(SpanKind.SERVER, response.statusCode),
});

const route = attributes[SEMATTRS_HTTP_ROUTE];
Expand Down Expand Up @@ -760,7 +804,7 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
startTime: HrTime,
error: Err
) {
utils.setSpanWithError(span, error);
setSpanWithError(span, error, this._semconvStability);
this._closeHttpSpan(span, SpanKind.SERVER, startTime, metricAttributes);
}

Expand Down Expand Up @@ -854,21 +898,21 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation

return {
client: {
captureRequestHeaders: utils.headerCapture(
captureRequestHeaders: headerCapture(
'request',
config.headersToSpanAttributes?.client?.requestHeaders ?? []
),
captureResponseHeaders: utils.headerCapture(
captureResponseHeaders: headerCapture(
'response',
config.headersToSpanAttributes?.client?.responseHeaders ?? []
),
},
server: {
captureRequestHeaders: utils.headerCapture(
captureRequestHeaders: headerCapture(
'request',
config.headersToSpanAttributes?.server?.requestHeaders ?? []
),
captureResponseHeaders: utils.headerCapture(
captureResponseHeaders: headerCapture(
'response',
config.headersToSpanAttributes?.server?.responseHeaders ?? []
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,3 +132,18 @@ export interface Err extends Error {
syscall?: string;
stack?: string;
}

/**
* Tracks whether this instrumentation emits old experimental,
* new stable, or both semantic conventions.
*
* Enum values chosen such that the enum may be used as a bitmask.
*/
export const enum SemconvStability {
/** Emit only stable semantic conventions */
STABLE = 0x1,
/** Emit only old semantic convetions */
OLD = 0x2,
/** Emit both stable and old semantic convetions */
DUPLICATE = 0x1 | 0x2,
}
Loading