From 8efceed28cfe6d2fe2928d4698ae6b788a12fd55 Mon Sep 17 00:00:00 2001 From: Svetlana Brennan <50715937+svetlanabrennan@users.noreply.github.com> Date: Tue, 22 Apr 2025 12:01:41 -0500 Subject: [PATCH] feat(instrumentation-http)!: Remove legacy http span attributes and metrics (#5552) Co-authored-by: Trent Mick --- experimental/CHANGELOG.md | 2 + .../README.md | 65 +--- .../src/enums/AttributeNames.ts | 24 -- .../src/http.ts | 184 ++-------- .../src/internal-types.ts | 15 - .../src/types.ts | 2 - .../src/utils.ts | 327 ++---------------- .../test/functionals/http-enable.test.ts | 228 +----------- .../test/functionals/http-metrics.test.ts | 239 +------------ .../test/functionals/https-enable.test.ts | 38 +- .../test/functionals/utils.test.ts | 211 ++--------- .../test/integrations/http-enable.test.ts | 22 +- .../test/integrations/https-enable.test.ts | 25 +- .../test/utils/assertSpan.ts | 124 ++----- 14 files changed, 162 insertions(+), 1344 deletions(-) delete mode 100644 experimental/packages/opentelemetry-instrumentation-http/src/enums/AttributeNames.ts diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index f4c1bf24d..43bc7dbdc 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -8,6 +8,8 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2 ### :boom: Breaking Changes +* feat(instrumentation-http)!: Remove legacy http span attributes and metrics [#5552](https://github.com/open-telemetry/opentelemetry-js/pull/5552) @svetlanabrennan + ### :rocket: Features ### :bug: Bug Fixes diff --git a/experimental/packages/opentelemetry-instrumentation-http/README.md b/experimental/packages/opentelemetry-instrumentation-http/README.md index 5d00dcaec..de73b2be1 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/README.md +++ b/experimental/packages/opentelemetry-instrumentation-http/README.md @@ -64,25 +64,13 @@ Http instrumentation has few options available to choose from. You can set the f | `ignoreOutgoingRequestHook` | `IgnoreOutgoingRequestFunction` | Http instrumentation will not trace all outgoing requests that matched with custom function | | `disableOutgoingRequestInstrumentation` | `boolean` | Set to true to avoid instrumenting outgoing requests at all. This can be helpful when another instrumentation handles outgoing requests. | | `disableIncomingRequestInstrumentation` | `boolean` | Set to true to avoid instrumenting incoming requests at all. This can be helpful when another instrumentation handles incoming requests. | -| [`serverName`](https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-instrumentation-http/src/types.ts#L101) | `string` | The primary server name of the matched virtual host. | | [`requireParentforOutgoingSpans`](https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-instrumentation-http/src/types.ts#L103) | Boolean | Require that is a parent span to create new span for outgoing requests. | | [`requireParentforIncomingSpans`](https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-instrumentation-http/src/types.ts#L105) | Boolean | Require that is a parent span to create new span for incoming requests. | | [`headersToSpanAttributes`](https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-instrumentation-http/src/types.ts#L107) | `object` | List of case insensitive HTTP headers to convert to span attributes. Client (outgoing requests, incoming responses) and server (incoming requests, outgoing responses) headers will be converted to span attributes in the form of `http.{request\|response}.header.header_name`, e.g. `http.response.header.content_length` | ## Semantic Conventions -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. +This package emits Semantic Convention [Version 1.27.0](https://github.com/open-telemetry/semantic-conventions/blob/v1.27.0/docs/http/http-spans.md). Follow all requirements and recommendations of HTTP Client and Server Semantic Conventions Version 1.27.0 for [spans](https://github.com/open-telemetry/semantic-conventions/blob/v1.27.0/docs/http/http-spans.md) and [metrics](https://github.com/open-telemetry/semantic-conventions/blob/v1.27.0/docs/http/http-metrics.md), including all required and recommended attributes. @@ -91,57 +79,6 @@ Metrics Exported: - [`http.server.request.duration`](https://github.com/open-telemetry/semantic-conventions/blob/v1.27.0/docs/http/http-metrics.md#metric-httpserverrequestduration) - [`http.client.request.duration`](https://github.com/open-telemetry/semantic-conventions/blob/v1.27.0/docs/http/http-metrics.md#metric-httpclientrequestduration) -### 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. - -### 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). - -#### Server Spans (legacy) - -When `OTEL_SEMCONV_STABILITY_OPT_IN` is not set or includes `http/dup`, this module implements Semantic Convention [Version 1.7.0](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.7.0/semantic_conventions/README.md). - -Attributes collected: - -| Attribute | Short Description | -| ------------------------------------------- | ------------------------------------------------------------------------------ | -| `ip_tcp` | Transport protocol used | -| `ip_udp` | Transport protocol used | -| `http.client_ip` | The IP address of the original client behind all proxies, if known | -| `http.flavor` | Kind of HTTP protocol used | -| `http.host` | The value of the HTTP host header | -| `http.method` | HTTP request method | -| `http.request_content_length` | The size of the request payload body in bytes | -| `http.request_content_length_uncompressed` | The size of the uncompressed request payload body after transport decoding | -| `http.response_content_length` | The size of the response payload body in bytes | -| `http.response_content_length_uncompressed` | The size of the uncompressed response payload body after transport decoding | -| `http.route` | The matched route (path template). | -| `http.scheme` | The URI scheme identifying the used protocol | -| `http.server_name` | The primary server name of the matched virtual host | -| `http.status_code` | HTTP response status code | -| `http.target` | The full request target as passed in a HTTP request line or equivalent | -| `http.url` | Full HTTP request URL in the form `scheme://host[:port]/path?query[#fragment]` | -| `http.user_agent` | Value of the HTTP User-Agent header sent by the client | -| `net.host.ip` | Like net.peer.ip but for the host IP. Useful in case of a multi-IP host | -| `net.host.name` | Local hostname or similar | -| `net.host.port` | Like net.peer.port but for the host port | -| `net.peer.ip.` | Remote address of the peer (dotted decimal for IPv4 or RFC5952 for IPv6) | -| `net.peer.name` | Remote hostname or similar | -| `net.peer.port` | Remote port number | -| `net.transport` | Transport protocol used | - ## Useful links - For more information on OpenTelemetry, visit: diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/enums/AttributeNames.ts b/experimental/packages/opentelemetry-instrumentation-http/src/enums/AttributeNames.ts deleted file mode 100644 index f9b8be3c8..000000000 --- a/experimental/packages/opentelemetry-instrumentation-http/src/enums/AttributeNames.ts +++ /dev/null @@ -1,24 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/** - * https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/http.md - */ -export enum AttributeNames { - HTTP_ERROR_NAME = 'http.error_name', - HTTP_ERROR_MESSAGE = 'http.error_message', - HTTP_STATUS_TEXT = 'http.status_text', -} diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts index 678e579e8..a563a60c1 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts @@ -30,7 +30,6 @@ import { ValueType, } from '@opentelemetry/api'; import { - getStringListFromEnv, hrTime, hrTimeDuration, hrTimeToMilliseconds, @@ -54,39 +53,28 @@ import { errorMonitor } from 'events'; import { ATTR_HTTP_REQUEST_METHOD, ATTR_HTTP_RESPONSE_STATUS_CODE, + ATTR_HTTP_ROUTE, ATTR_NETWORK_PROTOCOL_VERSION, ATTR_SERVER_ADDRESS, ATTR_SERVER_PORT, ATTR_URL_SCHEME, METRIC_HTTP_CLIENT_REQUEST_DURATION, METRIC_HTTP_SERVER_REQUEST_DURATION, - SEMATTRS_HTTP_ROUTE, } from '@opentelemetry/semantic-conventions'; import { extractHostnameAndPort, getIncomingRequestAttributes, getIncomingRequestAttributesOnResponse, - getIncomingRequestMetricAttributes, - getIncomingRequestMetricAttributesOnResponse, getIncomingStableRequestMetricAttributesOnResponse, getOutgoingRequestAttributes, getOutgoingRequestAttributesOnResponse, - getOutgoingRequestMetricAttributes, - getOutgoingRequestMetricAttributesOnResponse, getRequestInfo, headerCapture, isValidOptionsType, parseResponseStatus, setSpanWithError, } from './utils'; -import { - Err, - Func, - Http, - HttpRequestArgs, - Https, - SemconvStability, -} from './internal-types'; +import { Err, Func, Http, HttpRequestArgs, Https } from './internal-types'; /** * `node:http` and `node:https` instrumentation for OpenTelemetry @@ -95,46 +83,15 @@ export class HttpInstrumentation extends InstrumentationBase = new WeakSet(); private _headerCapture; - declare private _oldHttpServerDurationHistogram: Histogram; declare private _stableHttpServerDurationHistogram: Histogram; - declare private _oldHttpClientDurationHistogram: Histogram; declare private _stableHttpClientDurationHistogram: Histogram; - private _semconvStability = SemconvStability.OLD; - constructor(config: HttpInstrumentationConfig = {}) { super('@opentelemetry/instrumentation-http', VERSION, config); this._headerCapture = this._createHeaderCapture(); - - for (const entry of getStringListFromEnv('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() { - this._oldHttpServerDurationHistogram = this.meter.createHistogram( - 'http.server.duration', - { - description: 'Measures the duration of inbound HTTP requests.', - unit: 'ms', - valueType: ValueType.DOUBLE, - } - ); - this._oldHttpClientDurationHistogram = this.meter.createHistogram( - 'http.client.duration', - { - description: 'Measures the duration of outbound HTTP requests.', - unit: 'ms', - valueType: ValueType.DOUBLE, - } - ); this._stableHttpServerDurationHistogram = this.meter.createHistogram( METRIC_HTTP_SERVER_REQUEST_DURATION, { @@ -167,52 +124,22 @@ export class HttpInstrumentation extends InstrumentationBase original.apply(this, [event, ...args]), error => { if (error) { - setSpanWithError( - span, - error, - instrumentation._semconvStability - ); + setSpanWithError(span, error); instrumentation._closeHttpSpan( span, SpanKind.SERVER, startTime, - oldMetricAttributes, stableMetricAttributes ); throw error; @@ -783,23 +688,17 @@ export class HttpInstrumentation extends InstrumentationBase { if (error) { - setSpanWithError(span, error, instrumentation._semconvStability); + setSpanWithError(span, error); + instrumentation._closeHttpSpan( span, SpanKind.CLIENT, startTime, - oldMetricAttributes, stableMetricAttributes ); throw error; @@ -882,7 +781,6 @@ export class HttpInstrumentation extends InstrumentationBase { +export const setSpanWithError = (span: Span, error: Err): void => { const message = error.message; - - if ((semconvStability & SemconvStability.OLD) === SemconvStability.OLD) { - span.setAttribute(AttributeNames.HTTP_ERROR_NAME, error.name); - span.setAttribute(AttributeNames.HTTP_ERROR_MESSAGE, message); - } - - if ( - (semconvStability & SemconvStability.STABLE) === - SemconvStability.STABLE - ) { - span.setAttribute(ATTR_ERROR_TYPE, error.name); - } + span.setAttribute(ATTR_ERROR_TYPE, error.name); span.setStatus({ code: SpanStatusCode.ERROR, message }); span.recordException(error); }; -/** - * Adds attributes for request content-length and content-encoding HTTP headers - * @param { IncomingMessage } Request object whose headers will be analyzed - * @param { Attributes } Attributes object to be modified - */ -export const setRequestContentLengthAttribute = ( - request: IncomingMessage, - attributes: Attributes -): void => { - const length = getContentLength(request.headers); - if (length === null) return; - - if (isCompressed(request.headers)) { - attributes[SEMATTRS_HTTP_REQUEST_CONTENT_LENGTH] = length; - } else { - attributes[SEMATTRS_HTTP_REQUEST_CONTENT_LENGTH_UNCOMPRESSED] = length; - } -}; - -/** - * Adds attributes for response content-length and content-encoding HTTP headers - * @param { IncomingMessage } Response object whose headers will be analyzed - * @param { Attributes } Attributes object to be modified - * - * @deprecated this is for an older version of semconv. It is retained for compatibility using OTEL_SEMCONV_STABILITY_OPT_IN - */ -export const setResponseContentLengthAttribute = ( - response: IncomingMessage, - attributes: Attributes -): void => { - const length = getContentLength(response.headers); - if (length === null) return; - - if (isCompressed(response.headers)) { - attributes[SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH] = length; - } else { - attributes[SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH_UNCOMPRESSED] = length; - } -}; - -function getContentLength( - headers: OutgoingHttpHeaders | IncomingHttpHeaders -): number | null { - const contentLengthHeader = headers['content-length']; - if (contentLengthHeader === undefined) return null; - - const contentLength = parseInt(contentLengthHeader as string, 10); - if (isNaN(contentLength)) return null; - - return contentLength; -} - export const isCompressed = ( headers: OutgoingHttpHeaders | IncomingHttpHeaders ): boolean => { @@ -432,7 +335,6 @@ export const extractHostnameAndPort = ( * Returns outgoing request attributes scoped to the options passed to the request * @param {ParsedRequestOptions} requestOptions the same options used to make the request * @param {{ component: string, hostname: string, hookAttributes?: Attributes }} options used to pass data needed to create attributes - * @param {SemconvStability} semconvStability determines which semconv version to use */ export const getOutgoingRequestAttributes = ( requestOptions: ParsedRequestOptions, @@ -441,28 +343,18 @@ export const getOutgoingRequestAttributes = ( hostname: string; port: string | number; hookAttributes?: Attributes; - }, - semconvStability: SemconvStability + } ): Attributes => { const hostname = options.hostname; const port = options.port; const method = requestOptions.method ?? 'GET'; const normalizedMethod = normalizeMethod(method); const headers = requestOptions.headers || {}; - const userAgent = headers['user-agent']; const urlFull = getAbsoluteUrl( requestOptions, headers, `${options.component}:` ); - const oldAttributes: Attributes = { - [SEMATTRS_HTTP_URL]: urlFull, - [SEMATTRS_HTTP_METHOD]: method, - [SEMATTRS_HTTP_TARGET]: requestOptions.path || '/', - [SEMATTRS_NET_PEER_NAME]: hostname, - [SEMATTRS_HTTP_HOST]: headers.host ?? `${hostname}:${port}`, - }; - const newAttributes: Attributes = { // Required attributes [ATTR_HTTP_REQUEST_METHOD]: normalizedMethod, @@ -481,64 +373,17 @@ export const getOutgoingRequestAttributes = ( newAttributes[ATTR_HTTP_REQUEST_METHOD_ORIGINAL] = method; } - if (userAgent !== undefined) { - oldAttributes[SEMATTRS_HTTP_USER_AGENT] = userAgent; - } - - switch (semconvStability) { - case SemconvStability.STABLE: - return Object.assign(newAttributes, options.hookAttributes); - case SemconvStability.OLD: - return Object.assign(oldAttributes, options.hookAttributes); - } - - return Object.assign(oldAttributes, newAttributes, options.hookAttributes); -}; - -/** - * Returns outgoing request Metric attributes scoped to the request data - * @param {Attributes} spanAttributes the span attributes - */ -export const getOutgoingRequestMetricAttributes = ( - spanAttributes: Attributes -): Attributes => { - const metricAttributes: Attributes = {}; - metricAttributes[SEMATTRS_HTTP_METHOD] = spanAttributes[SEMATTRS_HTTP_METHOD]; - metricAttributes[SEMATTRS_NET_PEER_NAME] = - spanAttributes[SEMATTRS_NET_PEER_NAME]; - //TODO: http.url attribute, it should substitute any parameters to avoid high cardinality. - return metricAttributes; -}; - -/** - * Returns attributes related to the kind of HTTP protocol used - * @param {string} [kind] Kind of HTTP protocol used: "1.0", "1.1", "2", "SPDY" or "QUIC". - */ -export const setAttributesFromHttpKind = ( - kind: string | undefined, - attributes: Attributes -): void => { - if (kind) { - attributes[SEMATTRS_HTTP_FLAVOR] = kind; - if (kind.toUpperCase() !== 'QUIC') { - attributes[SEMATTRS_NET_TRANSPORT] = NETTRANSPORTVALUES_IP_TCP; - } else { - attributes[SEMATTRS_NET_TRANSPORT] = NETTRANSPORTVALUES_IP_UDP; - } - } + return Object.assign(newAttributes, options.hookAttributes); }; /** * Returns outgoing request attributes scoped to the response data * @param {IncomingMessage} response the response object - * @param {SemconvStability} semconvStability determines which semconv version to use */ export const getOutgoingRequestAttributesOnResponse = ( - response: IncomingMessage, - semconvStability: SemconvStability + response: IncomingMessage ): Attributes => { - const { statusCode, statusMessage, httpVersion, socket } = response; - const oldAttributes: Attributes = {}; + const { statusCode, socket } = response; const stableAttributes: Attributes = {}; if (statusCode != null) { @@ -547,49 +392,14 @@ export const getOutgoingRequestAttributesOnResponse = ( if (socket) { const { remoteAddress, remotePort } = socket; - oldAttributes[SEMATTRS_NET_PEER_IP] = remoteAddress; - oldAttributes[SEMATTRS_NET_PEER_PORT] = remotePort; // Recommended stableAttributes[ATTR_NETWORK_PEER_ADDRESS] = remoteAddress; stableAttributes[ATTR_NETWORK_PEER_PORT] = remotePort; stableAttributes[ATTR_NETWORK_PROTOCOL_VERSION] = response.httpVersion; } - setResponseContentLengthAttribute(response, oldAttributes); - if (statusCode) { - oldAttributes[SEMATTRS_HTTP_STATUS_CODE] = statusCode; - oldAttributes[AttributeNames.HTTP_STATUS_TEXT] = ( - statusMessage || '' - ).toUpperCase(); - } - - setAttributesFromHttpKind(httpVersion, oldAttributes); - - switch (semconvStability) { - case SemconvStability.STABLE: - return stableAttributes; - case SemconvStability.OLD: - return oldAttributes; - } - - return Object.assign(oldAttributes, stableAttributes); -}; - -/** - * Returns outgoing request Metric attributes scoped to the response data - * @param {Attributes} spanAttributes the span attributes - */ -export const getOutgoingRequestMetricAttributesOnResponse = ( - spanAttributes: Attributes -): Attributes => { - const metricAttributes: Attributes = {}; - metricAttributes[SEMATTRS_NET_PEER_PORT] = - spanAttributes[SEMATTRS_NET_PEER_PORT]; - metricAttributes[SEMATTRS_HTTP_STATUS_CODE] = - spanAttributes[SEMATTRS_HTTP_STATUS_CODE]; - metricAttributes[SEMATTRS_HTTP_FLAVOR] = spanAttributes[SEMATTRS_HTTP_FLAVOR]; - return metricAttributes; + return stableAttributes; }; function parseHostHeader( @@ -781,32 +591,22 @@ function getInfoFromIncomingMessage( /** * Returns incoming request attributes scoped to the request data * @param {IncomingMessage} request the request object - * @param {{ component: string, serverName?: string, hookAttributes?: Attributes }} options used to pass data needed to create attributes - * @param {SemconvStability} semconvStability determines which semconv version to use + * @param {{ component: string, hookAttributes?: Attributes }} options used to pass data needed to create attributes */ export const getIncomingRequestAttributes = ( request: IncomingMessage, options: { component: 'http' | 'https'; - serverName?: string; hookAttributes?: Attributes; - semconvStability: SemconvStability; }, logger: DiagLogger ): Attributes => { const headers = request.headers; const userAgent = headers['user-agent']; - const ips = headers['x-forwarded-for']; - const httpVersion = request.httpVersion; - const host = headers.host; - const hostname = host?.replace(/^(.*)(:[0-9]{1,5})/, '$1') || 'localhost'; - const method = request.method; const normalizedMethod = normalizeMethod(method); const serverAddress = getServerAddress(request, options.component); - const serverName = options.serverName; - const remoteClientAddress = getRemoteClientAddress(request); const newAttributes: Attributes = { @@ -829,8 +629,13 @@ export const getIncomingRequestAttributes = ( newAttributes[ATTR_URL_PATH] = parsedUrl.pathname; } + if (parsedUrl.search) { + // Remove leading '?' from URL search (https://developer.mozilla.org/en-US/docs/Web/API/URL/search). + newAttributes[ATTR_URL_QUERY] = parsedUrl.search.slice(1); + } + if (remoteClientAddress != null) { - newAttributes[ATTR_CLIENT_ADDRESS] = remoteClientAddress; + newAttributes[ATTR_CLIENT_ADDRESS] = remoteClientAddress.split(',')[0]; } if (serverAddress?.port != null) { @@ -842,59 +647,7 @@ export const getIncomingRequestAttributes = ( newAttributes[ATTR_HTTP_REQUEST_METHOD_ORIGINAL] = method; } - const oldAttributes: Attributes = { - [SEMATTRS_HTTP_URL]: parsedUrl.toString(), - [SEMATTRS_HTTP_HOST]: host, - [SEMATTRS_NET_HOST_NAME]: hostname, - [SEMATTRS_HTTP_METHOD]: method, - [SEMATTRS_HTTP_SCHEME]: options.component, - }; - - if (typeof ips === 'string') { - oldAttributes[SEMATTRS_HTTP_CLIENT_IP] = ips.split(',')[0]; - } - - if (typeof serverName === 'string') { - oldAttributes[SEMATTRS_HTTP_SERVER_NAME] = serverName; - } - - if (parsedUrl?.pathname) { - oldAttributes[SEMATTRS_HTTP_TARGET] = - parsedUrl?.pathname + parsedUrl?.search || '/'; - } - - if (userAgent !== undefined) { - oldAttributes[SEMATTRS_HTTP_USER_AGENT] = userAgent; - } - setRequestContentLengthAttribute(request, oldAttributes); - setAttributesFromHttpKind(httpVersion, oldAttributes); - - switch (options.semconvStability) { - case SemconvStability.STABLE: - return Object.assign(newAttributes, options.hookAttributes); - case SemconvStability.OLD: - return Object.assign(oldAttributes, options.hookAttributes); - } - - return Object.assign(oldAttributes, newAttributes, options.hookAttributes); -}; - -/** - * Returns incoming request Metric attributes scoped to the request data - * @param {Attributes} spanAttributes the span attributes - * @param {{ component: string }} options used to pass data needed to create attributes - */ -export const getIncomingRequestMetricAttributes = ( - spanAttributes: Attributes -): Attributes => { - const metricAttributes: Attributes = {}; - metricAttributes[SEMATTRS_HTTP_SCHEME] = spanAttributes[SEMATTRS_HTTP_SCHEME]; - metricAttributes[SEMATTRS_HTTP_METHOD] = spanAttributes[SEMATTRS_HTTP_METHOD]; - metricAttributes[SEMATTRS_NET_HOST_NAME] = - spanAttributes[SEMATTRS_NET_HOST_NAME]; - metricAttributes[SEMATTRS_HTTP_FLAVOR] = spanAttributes[SEMATTRS_HTTP_FLAVOR]; - //TODO: http.target attribute, it should substitute any parameters to avoid high cardinality. - return metricAttributes; + return Object.assign(newAttributes, options.hookAttributes); }; /** @@ -903,71 +656,33 @@ export const getIncomingRequestMetricAttributes = ( */ export const getIncomingRequestAttributesOnResponse = ( request: IncomingMessage, - response: ServerResponse, - semconvStability: SemconvStability + response: ServerResponse ): Attributes => { - // take socket from the request, - // since it may be detached from the response object in keep-alive mode - const { socket } = request; - const { statusCode, statusMessage } = response; + const { statusCode } = response; const newAttributes: Attributes = { [ATTR_HTTP_RESPONSE_STATUS_CODE]: statusCode, }; const rpcMetadata = getRPCMetadata(context.active()); - const oldAttributes: Attributes = {}; - if (socket) { - const { localAddress, localPort, remoteAddress, remotePort } = socket; - oldAttributes[SEMATTRS_NET_HOST_IP] = localAddress; - oldAttributes[SEMATTRS_NET_HOST_PORT] = localPort; - oldAttributes[SEMATTRS_NET_PEER_IP] = remoteAddress; - oldAttributes[SEMATTRS_NET_PEER_PORT] = remotePort; - } - oldAttributes[SEMATTRS_HTTP_STATUS_CODE] = statusCode; - oldAttributes[AttributeNames.HTTP_STATUS_TEXT] = ( - statusMessage || '' - ).toUpperCase(); if (rpcMetadata?.type === RPCType.HTTP && rpcMetadata.route !== undefined) { - oldAttributes[SEMATTRS_HTTP_ROUTE] = rpcMetadata.route; newAttributes[ATTR_HTTP_ROUTE] = rpcMetadata.route; } - switch (semconvStability) { - case SemconvStability.STABLE: - return newAttributes; - case SemconvStability.OLD: - return oldAttributes; - } - - return Object.assign(oldAttributes, newAttributes); + return newAttributes; }; /** * Returns incoming request Metric attributes scoped to the request data * @param {Attributes} spanAttributes the span attributes */ -export const getIncomingRequestMetricAttributesOnResponse = ( - spanAttributes: Attributes -): Attributes => { - const metricAttributes: Attributes = {}; - metricAttributes[SEMATTRS_HTTP_STATUS_CODE] = - spanAttributes[SEMATTRS_HTTP_STATUS_CODE]; - metricAttributes[SEMATTRS_NET_HOST_PORT] = - spanAttributes[SEMATTRS_NET_HOST_PORT]; - if (spanAttributes[SEMATTRS_HTTP_ROUTE] !== undefined) { - metricAttributes[SEMATTRS_HTTP_ROUTE] = spanAttributes[SEMATTRS_HTTP_ROUTE]; - } - return metricAttributes; -}; - export const getIncomingStableRequestMetricAttributesOnResponse = ( spanAttributes: Attributes ): Attributes => { const metricAttributes: Attributes = {}; if (spanAttributes[ATTR_HTTP_ROUTE] !== undefined) { - metricAttributes[ATTR_HTTP_ROUTE] = spanAttributes[SEMATTRS_HTTP_ROUTE]; + metricAttributes[ATTR_HTTP_ROUTE] = spanAttributes[ATTR_HTTP_ROUTE]; } // required if and only if one was sent, same as span requirement diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts index 344035491..ef5c5ee55 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts @@ -36,6 +36,7 @@ import { ATTR_HTTP_ROUTE, ATTR_NETWORK_PEER_ADDRESS, ATTR_NETWORK_PEER_PORT, + ATTR_NETWORK_PROTOCOL_NAME, ATTR_NETWORK_PROTOCOL_VERSION, ATTR_SERVER_ADDRESS, ATTR_SERVER_PORT, @@ -43,24 +44,6 @@ import { ATTR_URL_PATH, ATTR_URL_SCHEME, HTTP_REQUEST_METHOD_VALUE_GET, - NETTRANSPORTVALUES_IP_TCP, - SEMATTRS_HTTP_CLIENT_IP, - SEMATTRS_HTTP_FLAVOR, - SEMATTRS_HTTP_HOST, - SEMATTRS_HTTP_METHOD, - SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH_UNCOMPRESSED, - SEMATTRS_HTTP_ROUTE, - SEMATTRS_HTTP_SCHEME, - SEMATTRS_HTTP_STATUS_CODE, - SEMATTRS_HTTP_TARGET, - SEMATTRS_HTTP_URL, - SEMATTRS_NET_HOST_IP, - SEMATTRS_NET_HOST_NAME, - SEMATTRS_NET_HOST_PORT, - SEMATTRS_NET_PEER_IP, - SEMATTRS_NET_PEER_NAME, - SEMATTRS_NET_PEER_PORT, - SEMATTRS_NET_TRANSPORT, } from '@opentelemetry/semantic-conventions'; import * as assert from 'assert'; import * as nock from 'nock'; @@ -86,9 +69,7 @@ instrumentation.enable(); instrumentation.disable(); import * as http from 'http'; -import { AttributeNames } from '../../src/enums/AttributeNames'; import { getRemoteClientAddress } from '../../src/utils'; -import { SemconvStability } from '../../src/internal-types'; const applyCustomAttributesOnSpanErrorMessage = 'bad applyCustomAttributesOnSpan function'; @@ -98,7 +79,6 @@ const serverPort = 22346; const protocol = 'http'; const hostname = 'localhost'; const pathname = '/test'; -const serverName = 'my.server.name'; const memoryExporter = new InMemorySpanExporter(); const provider = new NodeTracerProvider({ spanProcessors: [new SimpleSpanProcessor(memoryExporter)], @@ -234,33 +214,14 @@ describe('HttpInstrumentation', () => { assertSpan(incomingSpan, SpanKind.SERVER, validations); assertSpan(outgoingSpan, SpanKind.CLIENT, validations); assert.strictEqual( - incomingSpan.attributes[SEMATTRS_NET_HOST_PORT], + incomingSpan.attributes[ATTR_SERVER_PORT], serverPort ); assert.strictEqual( - outgoingSpan.attributes[SEMATTRS_NET_PEER_PORT], + outgoingSpan.attributes[ATTR_SERVER_PORT], serverPort ); }); - - it('should remove auth from the `http.url` attribute (client side and server side)', async () => { - await httpRequest.get( - `${protocol}://user:pass@${hostname}:${serverPort}${pathname}` - ); - const spans = memoryExporter.getFinishedSpans(); - const [incomingSpan, outgoingSpan] = spans; - assert.strictEqual(spans.length, 2); - assert.strictEqual(incomingSpan.kind, SpanKind.SERVER); - assert.strictEqual(outgoingSpan.kind, SpanKind.CLIENT); - assert.strictEqual( - incomingSpan.attributes[SEMATTRS_HTTP_URL], - `${protocol}://${hostname}:${serverPort}${pathname}` - ); - assert.strictEqual( - outgoingSpan.attributes[SEMATTRS_HTTP_URL], - `${protocol}://${hostname}:${serverPort}${pathname}` - ); - }); }); describe('partially disable instrumentation', () => { @@ -338,7 +299,6 @@ describe('HttpInstrumentation', () => { responseHook: responseHookFunction, startIncomingSpanHook: startIncomingSpanHookFunction, startOutgoingSpanHook: startOutgoingSpanHookFunction, - serverName, }); instrumentation.enable(); server = http.createServer((request, response) => { @@ -414,30 +374,28 @@ describe('HttpInstrumentation', () => { resHeaders: result.resHeaders, reqHeaders: result.reqHeaders, component: 'http', - serverName, }; assert.strictEqual(spans.length, 2); assert.strictEqual( - incomingSpan.attributes[SEMATTRS_HTTP_CLIENT_IP], + incomingSpan.attributes[ATTR_CLIENT_ADDRESS], '' ); assert.strictEqual( - incomingSpan.attributes[SEMATTRS_NET_HOST_PORT], + incomingSpan.attributes[ATTR_SERVER_PORT], serverPort ); assert.strictEqual( - outgoingSpan.attributes[SEMATTRS_NET_PEER_PORT], + outgoingSpan.attributes[ATTR_SERVER_PORT], serverPort ); [ { span: incomingSpan, kind: SpanKind.SERVER }, { span: outgoingSpan, kind: SpanKind.CLIENT }, ].forEach(({ span, kind }) => { - assert.strictEqual(span.attributes[SEMATTRS_HTTP_FLAVOR], '1.1'); - assert.strictEqual( - span.attributes[SEMATTRS_NET_TRANSPORT], - NETTRANSPORTVALUES_IP_TCP + assert.ok( + !span.attributes[ATTR_NETWORK_PROTOCOL_NAME], + 'should not be added for HTTP kind' ); assertSpan(span, kind, validations); }); @@ -450,7 +408,7 @@ describe('HttpInstrumentation', () => { const span = memoryExporter.getFinishedSpans()[0]; assert.strictEqual(span.kind, SpanKind.SERVER); - assert.strictEqual(span.attributes[SEMATTRS_HTTP_ROUTE], 'TheRoute'); + assert.strictEqual(span.attributes[ATTR_HTTP_ROUTE], 'TheRoute'); assert.strictEqual(span.name, 'GET TheRoute'); }); @@ -868,7 +826,10 @@ describe('HttpInstrumentation', () => { const [span] = spans; assert.strictEqual(spans.length, 1); assert.ok(Object.keys(span.attributes).length > 6); - assert.strictEqual(span.attributes[SEMATTRS_HTTP_STATUS_CODE], 404); + assert.strictEqual( + span.attributes[ATTR_HTTP_RESPONSE_STATUS_CODE], + 404 + ); assert.strictEqual(span.status.code, SpanStatusCode.ERROR); done(); }); @@ -1064,14 +1025,13 @@ describe('HttpInstrumentation', () => { }); }); - describe('with semconv stability set to http', () => { + describe('with semconv stability', () => { beforeEach(() => { memoryExporter.reset(); }); before(async () => { instrumentation.setConfig({}); - instrumentation['_semconvStability'] = SemconvStability.STABLE; instrumentation.enable(); server = http.createServer((request, response) => { if (request.url?.includes('/premature-close')) { @@ -1203,164 +1163,6 @@ describe('HttpInstrumentation', () => { }); }); - describe('with semconv stability set to http/dup', () => { - beforeEach(() => { - memoryExporter.reset(); - instrumentation.setConfig({}); - }); - - before(async () => { - instrumentation['_semconvStability'] = SemconvStability.DUPLICATE; - instrumentation.enable(); - server = http.createServer((request, response) => { - 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.setHeader('Content-Type', 'application/json'); - response.end( - JSON.stringify({ address: getRemoteClientAddress(request) }) - ); - }); - - await new Promise(resolve => server.listen(serverPort, resolve)); - }); - - after(() => { - server.close(); - instrumentation.disable(); - }); - - it('should create client spans with semconv 1.27 and old 1.7', async () => { - const response = await httpRequest.get( - `${protocol}://${hostname}:${serverPort}${pathname}` - ); - const spans = memoryExporter.getFinishedSpans(); - assert.strictEqual(spans.length, 2); - const outgoingSpan = spans[1]; - - // should have only required and recommended attributes for semconv 1.27 - assert.deepStrictEqual(outgoingSpan.attributes, { - // 1.27 attributes - [ATTR_HTTP_REQUEST_METHOD]: HTTP_REQUEST_METHOD_VALUE_GET, - [ATTR_SERVER_ADDRESS]: hostname, - [ATTR_SERVER_PORT]: serverPort, - [ATTR_URL_FULL]: `http://${hostname}:${serverPort}${pathname}`, - [ATTR_HTTP_RESPONSE_STATUS_CODE]: 200, - [ATTR_NETWORK_PEER_ADDRESS]: response.address, - [ATTR_NETWORK_PEER_PORT]: serverPort, - [ATTR_NETWORK_PROTOCOL_VERSION]: '1.1', - - // 1.7 attributes - [SEMATTRS_HTTP_FLAVOR]: '1.1', - [SEMATTRS_HTTP_HOST]: `${hostname}:${serverPort}`, - [SEMATTRS_HTTP_METHOD]: 'GET', - [SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH_UNCOMPRESSED]: - response.data.length, - [SEMATTRS_HTTP_STATUS_CODE]: 200, - [SEMATTRS_HTTP_TARGET]: '/test', - [SEMATTRS_HTTP_URL]: `http://${hostname}:${serverPort}${pathname}`, - [SEMATTRS_NET_PEER_IP]: response.address, - [SEMATTRS_NET_PEER_NAME]: hostname, - [SEMATTRS_NET_PEER_PORT]: serverPort, - [SEMATTRS_NET_TRANSPORT]: 'ip_tcp', - - // unspecified old names - [AttributeNames.HTTP_STATUS_TEXT]: 'OK', - }); - }); - - it('should create server spans with semconv 1.27 and old 1.7', async () => { - const response = await httpRequest.get( - `${protocol}://${hostname}:${serverPort}${pathname}` - ); - const spans = memoryExporter.getFinishedSpans(); - assert.strictEqual(spans.length, 2); - const incomingSpan = spans[0]; - const body = JSON.parse(response.data); - - // should have only required and recommended attributes for semconv 1.27 - assert.deepStrictEqual(incomingSpan.attributes, { - // 1.27 attributes - [ATTR_CLIENT_ADDRESS]: body.address, - [ATTR_HTTP_REQUEST_METHOD]: HTTP_REQUEST_METHOD_VALUE_GET, - [ATTR_SERVER_ADDRESS]: hostname, - [ATTR_SERVER_PORT]: serverPort, - [ATTR_HTTP_RESPONSE_STATUS_CODE]: 200, - [ATTR_NETWORK_PEER_ADDRESS]: body.address, - [ATTR_NETWORK_PEER_PORT]: response.clientRemotePort, - [ATTR_NETWORK_PROTOCOL_VERSION]: '1.1', - [ATTR_URL_PATH]: pathname, - [ATTR_URL_SCHEME]: protocol, - - // 1.7 attributes - [SEMATTRS_HTTP_FLAVOR]: '1.1', - [SEMATTRS_HTTP_HOST]: `${hostname}:${serverPort}`, - [SEMATTRS_HTTP_METHOD]: 'GET', - [SEMATTRS_HTTP_SCHEME]: protocol, - [SEMATTRS_HTTP_STATUS_CODE]: 200, - [SEMATTRS_HTTP_TARGET]: '/test', - [SEMATTRS_HTTP_URL]: `http://${hostname}:${serverPort}${pathname}`, - [SEMATTRS_NET_TRANSPORT]: 'ip_tcp', - [SEMATTRS_NET_HOST_IP]: body.address, - [SEMATTRS_NET_HOST_NAME]: hostname, - [SEMATTRS_NET_HOST_PORT]: serverPort, - [SEMATTRS_NET_PEER_IP]: body.address, - [SEMATTRS_NET_PEER_PORT]: response.clientRemotePort, - - // unspecified old names - [AttributeNames.HTTP_STATUS_TEXT]: 'OK', - }); - }); - - it('should create server spans with semconv 1.27 and old 1.7 including http.route if RPC metadata is available', async () => { - const response = await httpRequest.get( - `${protocol}://${hostname}:${serverPort}${pathname}/setroute` - ); - const spans = memoryExporter.getFinishedSpans(); - assert.strictEqual(spans.length, 2); - const incomingSpan = spans[0]; - const body = JSON.parse(response.data); - - // should have only required and recommended attributes for semconv 1.27 - assert.deepStrictEqual(incomingSpan.attributes, { - // 1.27 attributes - [ATTR_CLIENT_ADDRESS]: body.address, - [ATTR_HTTP_REQUEST_METHOD]: HTTP_REQUEST_METHOD_VALUE_GET, - [ATTR_SERVER_ADDRESS]: hostname, - [ATTR_SERVER_PORT]: serverPort, - [ATTR_HTTP_RESPONSE_STATUS_CODE]: 200, - [ATTR_NETWORK_PEER_ADDRESS]: body.address, - [ATTR_NETWORK_PEER_PORT]: response.clientRemotePort, - [ATTR_NETWORK_PROTOCOL_VERSION]: '1.1', - [ATTR_URL_PATH]: `${pathname}/setroute`, - [ATTR_URL_SCHEME]: protocol, - [ATTR_HTTP_ROUTE]: 'TheRoute', - - // 1.7 attributes - [SEMATTRS_HTTP_FLAVOR]: '1.1', - [SEMATTRS_HTTP_HOST]: `${hostname}:${serverPort}`, - [SEMATTRS_HTTP_METHOD]: 'GET', - [SEMATTRS_HTTP_SCHEME]: protocol, - [SEMATTRS_HTTP_STATUS_CODE]: 200, - [SEMATTRS_HTTP_TARGET]: `${pathname}/setroute`, - [SEMATTRS_HTTP_URL]: `http://${hostname}:${serverPort}${pathname}/setroute`, - [SEMATTRS_NET_TRANSPORT]: 'ip_tcp', - [SEMATTRS_NET_HOST_IP]: body.address, - [SEMATTRS_NET_HOST_NAME]: hostname, - [SEMATTRS_NET_HOST_PORT]: serverPort, - [SEMATTRS_NET_PEER_IP]: body.address, - [SEMATTRS_NET_PEER_PORT]: response.clientRemotePort, - - // unspecified old names - [AttributeNames.HTTP_STATUS_TEXT]: 'OK', - }); - }); - }); - describe('with require parent span', () => { beforeEach(done => { memoryExporter.reset(); diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-metrics.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-metrics.test.ts index 907a67b10..ddd92fa35 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-metrics.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-metrics.test.ts @@ -28,14 +28,6 @@ import { ATTR_SERVER_ADDRESS, ATTR_SERVER_PORT, ATTR_URL_SCHEME, - SEMATTRS_HTTP_FLAVOR, - SEMATTRS_HTTP_METHOD, - SEMATTRS_HTTP_SCHEME, - SEMATTRS_HTTP_STATUS_CODE, - SEMATTRS_NET_HOST_NAME, - SEMATTRS_NET_HOST_PORT, - SEMATTRS_NET_PEER_NAME, - SEMATTRS_NET_PEER_PORT, } from '@opentelemetry/semantic-conventions'; import * as assert from 'assert'; import { HttpInstrumentation } from '../../src/http'; @@ -48,7 +40,6 @@ instrumentation.enable(); instrumentation.disable(); import * as http from 'http'; -import { SemconvStability } from '../../src/internal-types'; import { getRPCMetadata, RPCType } from '@opentelemetry/core'; import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks'; @@ -96,97 +87,7 @@ describe('metrics', () => { instrumentation.disable(); }); - describe('with no stability set', () => { - it('should add server/client duration metrics', async () => { - const requestCount = 3; - for (let i = 0; i < requestCount; i++) { - await httpRequest.get( - `${protocol}://${hostname}:${serverPort}${pathname}` - ); - } - await metricReader.collectAndExport(); - const resourceMetrics = metricsMemoryExporter.getMetrics(); - const scopeMetrics = resourceMetrics[0].scopeMetrics; - assert.strictEqual(scopeMetrics.length, 1, 'scopeMetrics count'); - const metrics = scopeMetrics[0].metrics; - assert.strictEqual(metrics.length, 2, 'metrics count'); - assert.strictEqual(metrics[0].dataPointType, DataPointType.HISTOGRAM); - assert.strictEqual( - metrics[0].descriptor.description, - 'Measures the duration of inbound HTTP requests.' - ); - assert.strictEqual(metrics[0].descriptor.name, 'http.server.duration'); - assert.strictEqual(metrics[0].descriptor.unit, 'ms'); - assert.strictEqual(metrics[0].dataPoints.length, 1); - assert.strictEqual( - (metrics[0].dataPoints[0].value as any).count, - requestCount - ); - assert.strictEqual( - metrics[0].dataPoints[0].attributes[SEMATTRS_HTTP_SCHEME], - 'http' - ); - assert.strictEqual( - metrics[0].dataPoints[0].attributes[SEMATTRS_HTTP_METHOD], - 'GET' - ); - assert.strictEqual( - metrics[0].dataPoints[0].attributes[SEMATTRS_HTTP_FLAVOR], - '1.1' - ); - assert.strictEqual( - metrics[0].dataPoints[0].attributes[SEMATTRS_NET_HOST_NAME], - 'localhost' - ); - assert.strictEqual( - metrics[0].dataPoints[0].attributes[SEMATTRS_HTTP_STATUS_CODE], - 200 - ); - assert.strictEqual( - metrics[0].dataPoints[0].attributes[SEMATTRS_NET_HOST_PORT], - 22346 - ); - - assert.strictEqual(metrics[1].dataPointType, DataPointType.HISTOGRAM); - assert.strictEqual( - metrics[1].descriptor.description, - 'Measures the duration of outbound HTTP requests.' - ); - assert.strictEqual(metrics[1].descriptor.name, 'http.client.duration'); - assert.strictEqual(metrics[1].descriptor.unit, 'ms'); - assert.strictEqual(metrics[1].dataPoints.length, 1); - assert.strictEqual( - (metrics[1].dataPoints[0].value as any).count, - requestCount - ); - assert.strictEqual( - metrics[1].dataPoints[0].attributes[SEMATTRS_HTTP_METHOD], - 'GET' - ); - assert.strictEqual( - metrics[1].dataPoints[0].attributes[SEMATTRS_NET_PEER_NAME], - 'localhost' - ); - assert.strictEqual( - metrics[1].dataPoints[0].attributes[SEMATTRS_NET_PEER_PORT], - 22346 - ); - assert.strictEqual( - metrics[1].dataPoints[0].attributes[SEMATTRS_HTTP_STATUS_CODE], - 200 - ); - assert.strictEqual( - metrics[1].dataPoints[0].attributes[SEMATTRS_HTTP_FLAVOR], - '1.1' - ); - }); - }); - - describe('with semconv stability set to stable', () => { - before(() => { - instrumentation['_semconvStability'] = SemconvStability.STABLE; - }); - + describe('using semconv stability attributes', () => { it('should add server/client duration metrics', async () => { const requestCount = 3; for (let i = 0; i < requestCount; i++) { @@ -246,142 +147,4 @@ describe('metrics', () => { }); }); }); - - describe('with semconv stability set to duplicate', () => { - before(() => { - instrumentation['_semconvStability'] = SemconvStability.DUPLICATE; - }); - - it('should add server/client duration metrics', async () => { - const requestCount = 3; - for (let i = 0; i < requestCount; i++) { - await httpRequest.get( - `${protocol}://${hostname}:${serverPort}${pathname}` - ); - } - await metricReader.collectAndExport(); - const resourceMetrics = metricsMemoryExporter.getMetrics(); - const scopeMetrics = resourceMetrics[0].scopeMetrics; - assert.strictEqual(scopeMetrics.length, 1, 'scopeMetrics count'); - const metrics = scopeMetrics[0].metrics; - assert.strictEqual(metrics.length, 4, 'metrics count'); - - // old metrics - assert.strictEqual(metrics[0].dataPointType, DataPointType.HISTOGRAM); - assert.strictEqual( - metrics[0].descriptor.description, - 'Measures the duration of inbound HTTP requests.' - ); - assert.strictEqual(metrics[0].descriptor.name, 'http.server.duration'); - assert.strictEqual(metrics[0].descriptor.unit, 'ms'); - assert.strictEqual(metrics[0].dataPoints.length, 1); - assert.strictEqual( - (metrics[0].dataPoints[0].value as any).count, - requestCount - ); - assert.strictEqual( - metrics[0].dataPoints[0].attributes[SEMATTRS_HTTP_SCHEME], - 'http' - ); - assert.strictEqual( - metrics[0].dataPoints[0].attributes[SEMATTRS_HTTP_METHOD], - 'GET' - ); - assert.strictEqual( - metrics[0].dataPoints[0].attributes[SEMATTRS_HTTP_FLAVOR], - '1.1' - ); - assert.strictEqual( - metrics[0].dataPoints[0].attributes[SEMATTRS_NET_HOST_NAME], - 'localhost' - ); - assert.strictEqual( - metrics[0].dataPoints[0].attributes[SEMATTRS_HTTP_STATUS_CODE], - 200 - ); - assert.strictEqual( - metrics[0].dataPoints[0].attributes[SEMATTRS_NET_HOST_PORT], - 22346 - ); - - assert.strictEqual(metrics[1].dataPointType, DataPointType.HISTOGRAM); - assert.strictEqual( - metrics[1].descriptor.description, - 'Measures the duration of outbound HTTP requests.' - ); - assert.strictEqual(metrics[1].descriptor.name, 'http.client.duration'); - assert.strictEqual(metrics[1].descriptor.unit, 'ms'); - assert.strictEqual(metrics[1].dataPoints.length, 1); - assert.strictEqual( - (metrics[1].dataPoints[0].value as any).count, - requestCount - ); - assert.strictEqual( - metrics[1].dataPoints[0].attributes[SEMATTRS_HTTP_METHOD], - 'GET' - ); - assert.strictEqual( - metrics[1].dataPoints[0].attributes[SEMATTRS_NET_PEER_NAME], - 'localhost' - ); - assert.strictEqual( - metrics[1].dataPoints[0].attributes[SEMATTRS_NET_PEER_PORT], - 22346 - ); - assert.strictEqual( - metrics[1].dataPoints[0].attributes[SEMATTRS_HTTP_STATUS_CODE], - 200 - ); - assert.strictEqual( - metrics[1].dataPoints[0].attributes[SEMATTRS_HTTP_FLAVOR], - '1.1' - ); - - // Stable metrics - assert.strictEqual(metrics[2].dataPointType, DataPointType.HISTOGRAM); - assert.strictEqual( - metrics[2].descriptor.description, - 'Duration of HTTP server requests.' - ); - assert.strictEqual( - metrics[2].descriptor.name, - 'http.server.request.duration' - ); - assert.strictEqual(metrics[2].descriptor.unit, 's'); - assert.strictEqual(metrics[2].dataPoints.length, 1); - assert.strictEqual( - (metrics[2].dataPoints[0].value as any).count, - requestCount - ); - assert.deepStrictEqual(metrics[2].dataPoints[0].attributes, { - [ATTR_HTTP_REQUEST_METHOD]: 'GET', - [ATTR_URL_SCHEME]: 'http', - [ATTR_HTTP_RESPONSE_STATUS_CODE]: 200, - [ATTR_NETWORK_PROTOCOL_VERSION]: '1.1', - [ATTR_HTTP_ROUTE]: 'TheRoute', - }); - - assert.strictEqual(metrics[3].dataPointType, DataPointType.HISTOGRAM); - assert.strictEqual( - metrics[3].descriptor.description, - 'Duration of HTTP client requests.' - ); - assert.strictEqual( - metrics[3].descriptor.name, - 'http.client.request.duration' - ); - assert.strictEqual(metrics[3].descriptor.unit, 's'); - assert.strictEqual(metrics[3].dataPoints.length, 1); - assert.strictEqual( - (metrics[3].dataPoints[0].value as any).count, - requestCount - ); - - assert.deepStrictEqual(metrics[3].dataPoints[0].attributes, { - [ATTR_HTTP_REQUEST_METHOD]: 'GET', - [ATTR_SERVER_ADDRESS]: 'localhost', - [ATTR_SERVER_PORT]: 22346, - }); - }); - }); }); diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/https-enable.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/https-enable.test.ts index faced1030..1f5e5c72e 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/https-enable.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/https-enable.test.ts @@ -30,13 +30,10 @@ import { SimpleSpanProcessor, } from '@opentelemetry/sdk-trace-base'; import { - NETTRANSPORTVALUES_IP_TCP, - SEMATTRS_HTTP_CLIENT_IP, - SEMATTRS_HTTP_FLAVOR, - SEMATTRS_HTTP_STATUS_CODE, - SEMATTRS_NET_HOST_PORT, - SEMATTRS_NET_PEER_PORT, - SEMATTRS_NET_TRANSPORT, + ATTR_CLIENT_ADDRESS, + ATTR_HTTP_RESPONSE_STATUS_CODE, + ATTR_SERVER_PORT, + ATTR_NETWORK_PROTOCOL_NAME, } from '@opentelemetry/semantic-conventions'; import * as assert from 'assert'; import * as fs from 'fs'; @@ -62,7 +59,6 @@ let server: https.Server; const serverPort = 32345; const protocol = 'https'; const hostname = 'localhost'; -const serverName = 'my.server.name'; const pathname = '/test'; const memoryExporter = new InMemorySpanExporter(); const provider = new BasicTracerProvider({ @@ -166,11 +162,11 @@ describe('HttpsInstrumentation', () => { assertSpan(incomingSpan, SpanKind.SERVER, validations); assertSpan(outgoingSpan, SpanKind.CLIENT, validations); assert.strictEqual( - incomingSpan.attributes[SEMATTRS_NET_HOST_PORT], + incomingSpan.attributes[ATTR_SERVER_PORT], serverPort ); assert.strictEqual( - outgoingSpan.attributes[SEMATTRS_NET_PEER_PORT], + outgoingSpan.attributes[ATTR_SERVER_PORT], serverPort ); }); @@ -198,7 +194,6 @@ describe('HttpsInstrumentation', () => { return false; }, applyCustomAttributesOnSpan: customAttributeFunction, - serverName, }); instrumentation.enable(); server = https.createServer( @@ -246,20 +241,19 @@ describe('HttpsInstrumentation', () => { resHeaders: result.resHeaders, reqHeaders: result.reqHeaders, component: 'https', - serverName, }; assert.strictEqual(spans.length, 2); assert.strictEqual( - incomingSpan.attributes[SEMATTRS_HTTP_CLIENT_IP], + incomingSpan.attributes[ATTR_CLIENT_ADDRESS], '' ); assert.strictEqual( - incomingSpan.attributes[SEMATTRS_NET_HOST_PORT], + incomingSpan.attributes[ATTR_SERVER_PORT], serverPort ); assert.strictEqual( - outgoingSpan.attributes[SEMATTRS_NET_PEER_PORT], + outgoingSpan.attributes[ATTR_SERVER_PORT], serverPort ); @@ -267,10 +261,9 @@ describe('HttpsInstrumentation', () => { { span: incomingSpan, kind: SpanKind.SERVER }, { span: outgoingSpan, kind: SpanKind.CLIENT }, ].forEach(({ span, kind }) => { - assert.strictEqual(span.attributes[SEMATTRS_HTTP_FLAVOR], '1.1'); - assert.strictEqual( - span.attributes[SEMATTRS_NET_TRANSPORT], - NETTRANSPORTVALUES_IP_TCP + assert.ok( + !span.attributes[ATTR_NETWORK_PROTOCOL_NAME], + 'should not be added for HTTP kind' ); assertSpan(span, kind, validations); }); @@ -613,7 +606,7 @@ describe('HttpsInstrumentation', () => { const [span] = spans; assert.strictEqual(spans.length, 1); assert.strictEqual(span.status.code, SpanStatusCode.ERROR); - assert.ok(Object.keys(span.attributes).length >= 6); + assert.ok(Object.keys(span.attributes).length >= 5); }); it('should have 1 ended span when request is aborted after receiving response', async () => { @@ -666,7 +659,10 @@ describe('HttpsInstrumentation', () => { const [span] = spans; assert.strictEqual(spans.length, 1); assert.ok(Object.keys(span.attributes).length > 6); - assert.strictEqual(span.attributes[SEMATTRS_HTTP_STATUS_CODE], 404); + assert.strictEqual( + span.attributes[ATTR_HTTP_RESPONSE_STATUS_CODE], + 404 + ); assert.strictEqual(span.status.code, SpanStatusCode.ERROR); done(); }); diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts index 4f41548c8..e705b44c1 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts @@ -22,25 +22,18 @@ import { diag, } from '@opentelemetry/api'; import { - SEMATTRS_HTTP_REQUEST_CONTENT_LENGTH, - SEMATTRS_HTTP_REQUEST_CONTENT_LENGTH_UNCOMPRESSED, - SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH, - SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH_UNCOMPRESSED, - SEMATTRS_HTTP_ROUTE, - SEMATTRS_HTTP_TARGET, + ATTR_ERROR_TYPE, + ATTR_HTTP_ROUTE, + ATTR_URL_PATH, + ATTR_URL_QUERY, } from '@opentelemetry/semantic-conventions'; import * as assert from 'assert'; import { IncomingMessage, ServerResponse } from 'http'; import { Socket } from 'net'; import * as sinon from 'sinon'; import * as url from 'url'; -import { - IgnoreMatcher, - ParsedRequestOptions, - SemconvStability, -} from '../../src/internal-types'; +import { IgnoreMatcher, ParsedRequestOptions } from '../../src/internal-types'; import * as utils from '../../src/utils'; -import { AttributeNames } from '../../src/enums/AttributeNames'; import { RPCType, setRPCMetadata } from '@opentelemetry/core'; import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks'; import { extractHostnameAndPort } from '../../src/utils'; @@ -185,19 +178,14 @@ describe('Utility', () => { } as unknown as Span; const mock = sinon.mock(span); - mock - .expects('setAttribute') - .calledWithExactly(AttributeNames.HTTP_ERROR_NAME, 'error'); - mock - .expects('setAttribute') - .calledWithExactly(AttributeNames.HTTP_ERROR_MESSAGE, errorMessage); + mock.expects('setAttribute').calledWithExactly(ATTR_ERROR_TYPE, 'Error'); mock.expects('setStatus').calledWithExactly({ code: SpanStatusCode.ERROR, message: errorMessage, }); mock.expects('recordException').calledWithExactly(error); - utils.setSpanWithError(span, error, SemconvStability.OLD); + utils.setSpanWithError(span, error); mock.verify(); }); }); @@ -234,10 +222,9 @@ describe('Utility', () => { () => { const attributes = utils.getIncomingRequestAttributesOnResponse( request, - {} as ServerResponse, - SemconvStability.OLD + {} as ServerResponse ); - assert.deepStrictEqual(attributes[SEMATTRS_HTTP_ROUTE], '/user/:id'); + assert.deepStrictEqual(attributes[ATTR_HTTP_ROUTE], '/user/:id'); context.disable(); return done(); } @@ -248,177 +235,33 @@ describe('Utility', () => { const request = { socket: {}, } as IncomingMessage; - const attributes = utils.getIncomingRequestAttributesOnResponse( - request, - { - socket: {}, - } as ServerResponse & { socket: Socket }, - SemconvStability.OLD - ); - assert.deepEqual(attributes[SEMATTRS_HTTP_ROUTE], undefined); + const attributes = utils.getIncomingRequestAttributesOnResponse(request, { + socket: {}, + } as ServerResponse & { socket: Socket }); + assert.deepEqual(attributes[ATTR_HTTP_ROUTE], undefined); }); }); - describe('getIncomingRequestMetricAttributesOnResponse()', () => { + describe('getIncomingStableRequestMetricAttributesOnResponse()', () => { it('should correctly add http_route if span has it', () => { const spanAttributes: Attributes = { - [SEMATTRS_HTTP_ROUTE]: '/user/:id', + [ATTR_HTTP_ROUTE]: '/user/:id', }; const metricAttributes = - utils.getIncomingRequestMetricAttributesOnResponse(spanAttributes); + utils.getIncomingStableRequestMetricAttributesOnResponse( + spanAttributes + ); - assert.deepStrictEqual( - metricAttributes[SEMATTRS_HTTP_ROUTE], - '/user/:id' - ); + assert.deepStrictEqual(metricAttributes[ATTR_HTTP_ROUTE], '/user/:id'); }); it('should skip http_route if span does not have it', () => { const spanAttributes: Attributes = {}; const metricAttributes = - utils.getIncomingRequestMetricAttributesOnResponse(spanAttributes); - assert.deepEqual(metricAttributes[SEMATTRS_HTTP_ROUTE], undefined); - }); - }); - // Verify the key in the given attributes is set to the given value, - // and that no other HTTP Content Length attributes are set. - function verifyValueInAttributes( - attributes: Attributes, - key: string | undefined, - value: number - ) { - const SemanticAttributess = [ - SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH_UNCOMPRESSED, - SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH, - SEMATTRS_HTTP_REQUEST_CONTENT_LENGTH_UNCOMPRESSED, - SEMATTRS_HTTP_REQUEST_CONTENT_LENGTH, - ]; - - for (const attr of SemanticAttributess) { - if (attr === key) { - assert.strictEqual(attributes[attr], value); - } else { - assert.strictEqual(attributes[attr], undefined); - } - } - } - - describe('setRequestContentLengthAttributes()', () => { - it('should set request content-length uncompressed attribute with no content-encoding header', () => { - const attributes: Attributes = {}; - const request = {} as IncomingMessage; - - request.headers = { - 'content-length': '1200', - }; - utils.setRequestContentLengthAttribute(request, attributes); - - verifyValueInAttributes( - attributes, - SEMATTRS_HTTP_REQUEST_CONTENT_LENGTH_UNCOMPRESSED, - 1200 - ); - }); - - it('should set request content-length uncompressed attribute with "identity" content-encoding header', () => { - const attributes: Attributes = {}; - const request = {} as IncomingMessage; - request.headers = { - 'content-length': '1200', - 'content-encoding': 'identity', - }; - utils.setRequestContentLengthAttribute(request, attributes); - - verifyValueInAttributes( - attributes, - SEMATTRS_HTTP_REQUEST_CONTENT_LENGTH_UNCOMPRESSED, - 1200 - ); - }); - - it('should set request content-length compressed attribute with "gzip" content-encoding header', () => { - const attributes: Attributes = {}; - const request = {} as IncomingMessage; - request.headers = { - 'content-length': '1200', - 'content-encoding': 'gzip', - }; - utils.setRequestContentLengthAttribute(request, attributes); - - verifyValueInAttributes( - attributes, - SEMATTRS_HTTP_REQUEST_CONTENT_LENGTH, - 1200 - ); - }); - }); - - describe('setResponseContentLengthAttributes()', () => { - it('should set response content-length uncompressed attribute with no content-encoding header', () => { - const attributes: Attributes = {}; - - const response = {} as IncomingMessage; - - response.headers = { - 'content-length': '1200', - }; - utils.setResponseContentLengthAttribute(response, attributes); - - verifyValueInAttributes( - attributes, - SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH_UNCOMPRESSED, - 1200 - ); - }); - - it('should set response content-length uncompressed attribute with "identity" content-encoding header', () => { - const attributes: Attributes = {}; - - const response = {} as IncomingMessage; - - response.headers = { - 'content-length': '1200', - 'content-encoding': 'identity', - }; - - utils.setResponseContentLengthAttribute(response, attributes); - - verifyValueInAttributes( - attributes, - SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH_UNCOMPRESSED, - 1200 - ); - }); - - it('should set response content-length compressed attribute with "gzip" content-encoding header', () => { - const attributes: Attributes = {}; - - const response = {} as IncomingMessage; - - response.headers = { - 'content-length': '1200', - 'content-encoding': 'gzip', - }; - - utils.setResponseContentLengthAttribute(response, attributes); - - verifyValueInAttributes( - attributes, - SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH, - 1200 - ); - }); - - it('should set no attributes with no content-length header', () => { - const attributes: Attributes = {}; - const message = {} as IncomingMessage; - - message.headers = { - 'content-encoding': 'gzip', - }; - utils.setResponseContentLengthAttribute(message, attributes); - - verifyValueInAttributes(attributes, undefined, 1200); + utils.getIncomingStableRequestMetricAttributesOnResponse( + spanAttributes + ); + assert.deepEqual(metricAttributes[ATTR_HTTP_ROUTE], undefined); }); }); @@ -437,11 +280,10 @@ describe('Utility', () => { request, { component: 'http', - semconvStability: SemconvStability.OLD, }, diag ); - assert.strictEqual(attributes[SEMATTRS_HTTP_ROUTE], undefined); + assert.strictEqual(attributes[ATTR_HTTP_ROUTE], undefined); }); it('should set http.target as path in http span attributes', () => { @@ -457,11 +299,12 @@ describe('Utility', () => { request, { component: 'http', - semconvStability: SemconvStability.OLD, }, diag ); - assert.strictEqual(attributes[SEMATTRS_HTTP_TARGET], '/user/?q=val'); + const path = String(attributes[ATTR_URL_PATH] ?? ''); + const query = String(attributes[ATTR_URL_QUERY] ?? ''); + assert.strictEqual(path + '?' + query, '/user/?q=val'); }); }); diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/integrations/http-enable.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/integrations/http-enable.test.ts index 47fb01989..c322f07ed 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/integrations/http-enable.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/integrations/http-enable.test.ts @@ -15,13 +15,7 @@ */ import { SpanKind, Span, context, propagation } from '@opentelemetry/api'; -import { - HTTPFLAVORVALUES_HTTP_1_1, - NETTRANSPORTVALUES_IP_TCP, - SEMATTRS_HTTP_FLAVOR, - SEMATTRS_HTTP_HOST, - SEMATTRS_NET_TRANSPORT, -} from '@opentelemetry/semantic-conventions'; +import { ATTR_NETWORK_PROTOCOL_NAME } from '@opentelemetry/semantic-conventions'; import * as assert from 'assert'; import * as url from 'url'; import { HttpInstrumentation } from '../../src/http'; @@ -230,13 +224,9 @@ describe('HttpInstrumentation Integration tests', () => { assert.strictEqual(spans.length, 2); assert.strictEqual(span.name, 'GET'); assert.strictEqual(result.reqHeaders['x-foo'], 'foo'); - assert.strictEqual( - span.attributes[SEMATTRS_HTTP_FLAVOR], - HTTPFLAVORVALUES_HTTP_1_1 - ); - assert.strictEqual( - span.attributes[SEMATTRS_NET_TRANSPORT], - NETTRANSPORTVALUES_IP_TCP + assert.ok( + !span.attributes[ATTR_NETWORK_PROTOCOL_NAME], + 'should not be added for HTTP kind' ); assertSpan(span, SpanKind.CLIENT, validations); }); @@ -405,10 +395,6 @@ describe('HttpInstrumentation Integration tests', () => { const span = spans.find(s => s.kind === SpanKind.CLIENT); assert.ok(span); assert.strictEqual(span.name, 'GET'); - assert.strictEqual( - span.attributes[SEMATTRS_HTTP_HOST], - `localhost:${mockServerPort}` - ); }); }); }); diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/integrations/https-enable.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/integrations/https-enable.test.ts index 309ad0afe..eec9e8563 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/integrations/https-enable.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/integrations/https-enable.test.ts @@ -15,12 +15,7 @@ */ import { SpanKind, Span, context, propagation } from '@opentelemetry/api'; -import { - HTTPFLAVORVALUES_HTTP_1_1, - NETTRANSPORTVALUES_IP_TCP, - SEMATTRS_HTTP_FLAVOR, - SEMATTRS_NET_TRANSPORT, -} from '@opentelemetry/semantic-conventions'; +import { ATTR_NETWORK_PROTOCOL_NAME } from '@opentelemetry/semantic-conventions'; import * as assert from 'assert'; import * as http from 'http'; import * as fs from 'fs'; @@ -165,8 +160,6 @@ describe('HttpsInstrumentation Integration tests', () => { hostname: 'localhost', httpStatusCode: result.statusCode!, httpMethod: 'GET', - pathname: '/', - path: '/?query=test', resHeaders: result.resHeaders, reqHeaders: result.reqHeaders, component: 'https', @@ -192,8 +185,6 @@ describe('HttpsInstrumentation Integration tests', () => { hostname: 'localhost', httpStatusCode: result.statusCode!, httpMethod: 'GET', - pathname: '/', - path: '/?query=test', resHeaders: result.resHeaders, reqHeaders: result.reqHeaders, component: 'https', @@ -222,8 +213,6 @@ describe('HttpsInstrumentation Integration tests', () => { hostname: 'localhost', httpStatusCode: result.statusCode!, httpMethod: 'GET', - pathname: '/', - path: '/?query=test', resHeaders: result.resHeaders, reqHeaders: result.reqHeaders, component: 'https', @@ -232,13 +221,9 @@ describe('HttpsInstrumentation Integration tests', () => { assert.strictEqual(spans.length, 2); assert.strictEqual(span.name, 'GET'); assert.strictEqual(result.reqHeaders['x-foo'], 'foo'); - assert.strictEqual( - span.attributes[SEMATTRS_HTTP_FLAVOR], - HTTPFLAVORVALUES_HTTP_1_1 - ); - assert.strictEqual( - span.attributes[SEMATTRS_NET_TRANSPORT], - NETTRANSPORTVALUES_IP_TCP + assert.ok( + !span.attributes[ATTR_NETWORK_PROTOCOL_NAME], + 'should not be added for HTTP kind' ); assertSpan(span, SpanKind.CLIENT, validations); }); @@ -254,7 +239,6 @@ describe('HttpsInstrumentation Integration tests', () => { hostname: 'localhost', httpStatusCode: result.statusCode!, httpMethod: 'GET', - pathname: '/', resHeaders: result.resHeaders, reqHeaders: result.reqHeaders, component: 'https', @@ -282,7 +266,6 @@ describe('HttpsInstrumentation Integration tests', () => { hostname: 'localhost', httpStatusCode: 200, httpMethod: 'GET', - pathname: '/', resHeaders: result.resHeaders, reqHeaders: result.reqHeaders, component: 'https', diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/utils/assertSpan.ts b/experimental/packages/opentelemetry-instrumentation-http/test/utils/assertSpan.ts index 25ab861fb..0f27f0bc9 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/utils/assertSpan.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/utils/assertSpan.ts @@ -22,28 +22,20 @@ import { import { hrTimeToNanoseconds } from '@opentelemetry/core'; import { ReadableSpan } from '@opentelemetry/sdk-trace-base'; import { - SEMATTRS_HTTP_METHOD, - SEMATTRS_HTTP_REQUEST_CONTENT_LENGTH, - SEMATTRS_HTTP_REQUEST_CONTENT_LENGTH_UNCOMPRESSED, - SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH, - SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH_UNCOMPRESSED, - SEMATTRS_HTTP_SCHEME, - SEMATTRS_HTTP_SERVER_NAME, - SEMATTRS_HTTP_STATUS_CODE, - SEMATTRS_HTTP_TARGET, - SEMATTRS_HTTP_URL, - SEMATTRS_HTTP_USER_AGENT, - SEMATTRS_NET_HOST_IP, - SEMATTRS_NET_HOST_PORT, - SEMATTRS_NET_PEER_IP, - SEMATTRS_NET_PEER_NAME, - SEMATTRS_NET_PEER_PORT, + ATTR_HTTP_REQUEST_METHOD, + ATTR_HTTP_RESPONSE_STATUS_CODE, + ATTR_NETWORK_PEER_ADDRESS, + ATTR_SERVER_ADDRESS, + ATTR_SERVER_PORT, + ATTR_URL_FULL, + ATTR_URL_SCHEME, + ATTR_USER_AGENT_ORIGINAL, + ATTR_URL_PATH, } from '@opentelemetry/semantic-conventions'; import * as assert from 'assert'; import * as http from 'http'; import * as utils from '../../src/utils'; import { DummyPropagation } from './DummyPropagation'; -import { AttributeNames } from '../../src/enums/AttributeNames'; export const assertSpan = ( span: ReadableSpan, @@ -53,11 +45,10 @@ export const assertSpan = ( httpMethod: string; resHeaders: http.IncomingHttpHeaders; hostname: string; - pathname: string; + pathname?: string; reqHeaders?: http.OutgoingHttpHeaders; path?: string | null; forceStatus?: SpanStatus; - serverName?: string; component: string; noNetPeer?: boolean; // we don't expect net peer info when request throw before being sent error?: Exception; @@ -67,20 +58,14 @@ export const assertSpan = ( assert.strictEqual(span.spanContext().spanId.length, 16); assert.strictEqual(span.kind, kind); assert.strictEqual(span.name, validations.httpMethod); + assert.strictEqual( - span.attributes[AttributeNames.HTTP_ERROR_MESSAGE], - span.status.message - ); - assert.strictEqual( - span.attributes[SEMATTRS_HTTP_METHOD], + span.attributes[ATTR_HTTP_REQUEST_METHOD], validations.httpMethod ); + assert.strictEqual( - span.attributes[SEMATTRS_HTTP_TARGET], - validations.path || validations.pathname - ); - assert.strictEqual( - span.attributes[SEMATTRS_HTTP_STATUS_CODE], + span.attributes[ATTR_HTTP_RESPONSE_STATUS_CODE], validations.httpStatusCode ); @@ -111,82 +96,49 @@ export const assertSpan = ( assert.ok(span.endTime, 'must be finished'); assert.ok(hrTimeToNanoseconds(span.duration), 'must have positive duration'); - if (validations.reqHeaders) { - const userAgent = validations.reqHeaders['user-agent']; - if (userAgent) { - assert.strictEqual(span.attributes[SEMATTRS_HTTP_USER_AGENT], userAgent); - } - } if (span.kind === SpanKind.CLIENT) { - if (validations.resHeaders['content-length']) { - const contentLength = Number(validations.resHeaders['content-length']); - - if ( - validations.resHeaders['content-encoding'] && - validations.resHeaders['content-encoding'] !== 'identity' - ) { - assert.strictEqual( - span.attributes[SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH], - contentLength - ); - } else { - assert.strictEqual( - span.attributes[SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH_UNCOMPRESSED], - contentLength - ); - } - } assert.strictEqual( - span.attributes[SEMATTRS_NET_PEER_NAME], + span.attributes[ATTR_SERVER_ADDRESS], validations.hostname, 'must be consistent (PEER_NAME and hostname)' ); if (!validations.noNetPeer) { - assert.ok(span.attributes[SEMATTRS_NET_PEER_IP], 'must have PEER_IP'); - assert.ok(span.attributes[SEMATTRS_NET_PEER_PORT], 'must have PEER_PORT'); + assert.ok( + span.attributes[ATTR_NETWORK_PEER_ADDRESS], + 'must have PEER_IP' + ); + assert.ok(span.attributes[ATTR_SERVER_PORT], 'must have PEER_PORT'); } assert.ok( - (span.attributes[SEMATTRS_HTTP_URL] as string).indexOf( - span.attributes[SEMATTRS_NET_PEER_NAME] as string + (span.attributes[ATTR_URL_FULL] as string).indexOf( + span.attributes[ATTR_SERVER_ADDRESS] as string ) > -1, 'must be consistent' ); } if (span.kind === SpanKind.SERVER) { - if (validations.reqHeaders && validations.reqHeaders['content-length']) { - const contentLength = validations.reqHeaders['content-length']; - - if ( - validations.reqHeaders['content-encoding'] && - validations.reqHeaders['content-encoding'] !== 'identity' - ) { - assert.strictEqual( - span.attributes[SEMATTRS_HTTP_REQUEST_CONTENT_LENGTH], - contentLength - ); - } else { - assert.strictEqual( - span.attributes[SEMATTRS_HTTP_REQUEST_CONTENT_LENGTH_UNCOMPRESSED], - contentLength - ); - } - } - if (validations.serverName) { - assert.strictEqual( - span.attributes[SEMATTRS_HTTP_SERVER_NAME], - validations.serverName, - ' must have serverName attribute' - ); - assert.ok(span.attributes[SEMATTRS_NET_HOST_PORT], 'must have HOST_PORT'); - assert.ok(span.attributes[SEMATTRS_NET_HOST_IP], 'must have HOST_IP'); - } assert.strictEqual( - span.attributes[SEMATTRS_HTTP_SCHEME], + span.attributes[ATTR_URL_SCHEME], validations.component, ' must have http.scheme attribute' ); assert.ok(typeof span.parentSpanContext?.spanId === 'string'); assert.ok(isValidSpanId(span.parentSpanContext.spanId)); + + assert.strictEqual( + span.attributes[ATTR_URL_PATH], + validations.path || validations.pathname + ); + + if (validations.reqHeaders) { + const userAgent = validations.reqHeaders['user-agent']; + if (userAgent) { + assert.strictEqual( + span.attributes[ATTR_USER_AGENT_ORIGINAL], + userAgent + ); + } + } } else if (validations.reqHeaders) { assert.ok(validations.reqHeaders[DummyPropagation.TRACE_CONTEXT_KEY]); assert.ok(validations.reqHeaders[DummyPropagation.SPAN_CONTEXT_KEY]);