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

refactor(instrumentation-http): replace SpanAttributes and MetricAttributes with Attributes #5023

Merged
merged 2 commits into from
Sep 27, 2024
Merged

refactor(instrumentation-http): replace SpanAttributes and MetricAttributes with Attributes #5023

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
782fd84
refactor(instrumentation-http): replace SpanAttributes and MetricAttr…
david-luna Sep 27, 2024
ea174b5
chore: update CHANGELOG
david-luna Sep 27, 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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se
* deps: set `@opentelemetry/api` dependency min version to 1.3.0 in `examples`, `experimental/packages`, `integration-tests` and `selenium-tests`
[#4992](https://github.com/open-telemetry/opentelemetry-js/pull/4992)
* refactor(sdk-metrics): replace `MetricsAttributes` with `Attributes` [#5021](https://github.com/open-telemetry/opentelemetry-js/pull/5021) @david-luna
* refactor(instrumentation-http): replace `SpanAttributes` and `MetricsAttributes` with `Attributes` [#5023](https://github.com/open-telemetry/opentelemetry-js/pull/5023) @david-luna

## 1.26.0

Expand Down
12 changes: 6 additions & 6 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 @@ -26,7 +26,7 @@ import {
SpanStatusCode,
trace,
Histogram,
MetricAttributes,
Attributes,
ValueType,
} from '@opentelemetry/api';
import {
Expand Down Expand Up @@ -331,7 +331,7 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
request: http.ClientRequest,
span: Span,
startTime: HrTime,
metricAttributes: MetricAttributes
metricAttributes: Attributes
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder if we also need to change param names. But I think that would make it a breaking change.

Copy link
Member

Choose a reason for hiding this comment

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

ah, I think that's fine here. The name here is actually helping us understand that the attributes here are intended for the metric data point. 🙂

): http.ClientRequest {
if (this.getConfig().requestHook) {
this._callRequestHook(span, request);
Expand Down Expand Up @@ -691,7 +691,7 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
);

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

const spanOptions: SpanOptions = {
Expand Down Expand Up @@ -756,7 +756,7 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
request: http.IncomingMessage,
response: http.ServerResponse,
span: Span,
metricAttributes: MetricAttributes,
metricAttributes: Attributes,
startTime: HrTime
) {
const attributes = getIncomingRequestAttributesOnResponse(
Expand Down Expand Up @@ -800,7 +800,7 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation

private _onServerResponseError(
span: Span,
metricAttributes: MetricAttributes,
metricAttributes: Attributes,
startTime: HrTime,
error: Err
) {
Expand Down Expand Up @@ -840,7 +840,7 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
span: Span,
spanKind: SpanKind,
startTime: HrTime,
metricAttributes: MetricAttributes
metricAttributes: Attributes
) {
if (!this._spanNotEnded.has(span)) {
return;
Expand Down
6 changes: 3 additions & 3 deletions experimental/packages/opentelemetry-instrumentation-http/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { Span, SpanAttributes } from '@opentelemetry/api';
import { Span, Attributes } from '@opentelemetry/api';
import type * as http from 'http';
import type * as https from 'https';
import {
Expand Down Expand Up @@ -73,11 +73,11 @@ export interface HttpResponseCustomAttributeFunction {
}

export interface StartIncomingSpanCustomAttributeFunction {
(request: IncomingMessage): SpanAttributes;
(request: IncomingMessage): Attributes;
}

export interface StartOutgoingSpanCustomAttributeFunction {
(request: RequestOptions): SpanAttributes;
(request: RequestOptions): Attributes;
}

/**
Expand Down
68 changes: 33 additions & 35 deletions experimental/packages/opentelemetry-instrumentation-http/src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,11 @@
* limitations under the License.
*/
import {
MetricAttributes,
SpanAttributes,
Attributes,
SpanStatusCode,
Span,
context,
SpanKind,
Attributes,
} from '@opentelemetry/api';
import {
ATTR_CLIENT_ADDRESS,
Expand Down Expand Up @@ -212,11 +210,11 @@ export const setSpanWithError = (
/**
* Adds attributes for request content-length and content-encoding HTTP headers
* @param { IncomingMessage } Request object whose headers will be analyzed
* @param { SpanAttributes } SpanAttributes object to be modified
* @param { Attributes } Attributes object to be modified
*/
export const setRequestContentLengthAttribute = (
request: IncomingMessage,
attributes: SpanAttributes
attributes: Attributes
): void => {
const length = getContentLength(request.headers);
if (length === null) return;
Expand All @@ -231,13 +229,13 @@ export const setRequestContentLengthAttribute = (
/**
* Adds attributes for response content-length and content-encoding HTTP headers
* @param { IncomingMessage } Response object whose headers will be analyzed
* @param { SpanAttributes } SpanAttributes object to be modified
* @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: SpanAttributes
attributes: Attributes
): void => {
const length = getContentLength(response.headers);
if (length === null) return;
Expand Down Expand Up @@ -380,7 +378,7 @@ 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?: SpanAttributes }} options used to pass data needed to create attributes
* @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 = (
Expand All @@ -389,10 +387,10 @@ export const getOutgoingRequestAttributes = (
component: string;
hostname: string;
port: string | number;
hookAttributes?: SpanAttributes;
hookAttributes?: Attributes;
},
semconvStability: SemconvStability
): SpanAttributes => {
): Attributes => {
const hostname = options.hostname;
const port = options.port;
const method = requestOptions.method ?? 'GET';
Expand All @@ -404,7 +402,7 @@ export const getOutgoingRequestAttributes = (
headers,
`${options.component}:`
);
const oldAttributes: SpanAttributes = {
const oldAttributes: Attributes = {
[SEMATTRS_HTTP_URL]: urlFull,
[SEMATTRS_HTTP_METHOD]: method,
[SEMATTRS_HTTP_TARGET]: requestOptions.path || '/',
Expand Down Expand Up @@ -446,12 +444,12 @@ export const getOutgoingRequestAttributes = (

/**
* Returns outgoing request Metric attributes scoped to the request data
* @param {SpanAttributes} spanAttributes the span attributes
* @param {Attributes} spanAttributes the span attributes
*/
export const getOutgoingRequestMetricAttributes = (
spanAttributes: SpanAttributes
): MetricAttributes => {
const metricAttributes: MetricAttributes = {};
spanAttributes: Attributes
): Attributes => {
const metricAttributes: Attributes = {};
metricAttributes[SEMATTRS_HTTP_METHOD] = spanAttributes[SEMATTRS_HTTP_METHOD];
metricAttributes[SEMATTRS_NET_PEER_NAME] =
spanAttributes[SEMATTRS_NET_PEER_NAME];
Expand All @@ -465,7 +463,7 @@ export const getOutgoingRequestMetricAttributes = (
*/
export const setAttributesFromHttpKind = (
kind: string | undefined,
attributes: SpanAttributes
attributes: Attributes
): void => {
if (kind) {
attributes[SEMATTRS_HTTP_FLAVOR] = kind;
Expand All @@ -485,9 +483,9 @@ export const setAttributesFromHttpKind = (
export const getOutgoingRequestAttributesOnResponse = (
response: IncomingMessage,
semconvStability: SemconvStability
): SpanAttributes => {
): Attributes => {
const { statusCode, statusMessage, httpVersion, socket } = response;
const oldAttributes: SpanAttributes = {};
const oldAttributes: Attributes = {};
const stableAttributes: Attributes = {};

if (statusCode != null) {
Expand Down Expand Up @@ -527,12 +525,12 @@ export const getOutgoingRequestAttributesOnResponse = (

/**
* Returns outgoing request Metric attributes scoped to the response data
* @param {SpanAttributes} spanAttributes the span attributes
* @param {Attributes} spanAttributes the span attributes
*/
export const getOutgoingRequestMetricAttributesOnResponse = (
spanAttributes: SpanAttributes
): MetricAttributes => {
const metricAttributes: MetricAttributes = {};
spanAttributes: Attributes
): Attributes => {
const metricAttributes: Attributes = {};
metricAttributes[SEMATTRS_NET_PEER_PORT] =
spanAttributes[SEMATTRS_NET_PEER_PORT];
metricAttributes[SEMATTRS_HTTP_STATUS_CODE] =
Expand Down Expand Up @@ -694,18 +692,18 @@ export function getRemoteClientAddress(
/**
* Returns incoming request attributes scoped to the request data
* @param {IncomingMessage} request the request object
* @param {{ component: string, serverName?: string, hookAttributes?: SpanAttributes }} options used to pass data needed to create attributes
* @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
*/
export const getIncomingRequestAttributes = (
request: IncomingMessage,
options: {
component: 'http' | 'https';
serverName?: string;
hookAttributes?: SpanAttributes;
hookAttributes?: Attributes;
semconvStability: SemconvStability;
}
): SpanAttributes => {
): Attributes => {
const headers = request.headers;
const userAgent = headers['user-agent'];
const ips = headers['x-forwarded-for'];
Expand Down Expand Up @@ -794,13 +792,13 @@ export const getIncomingRequestAttributes = (

/**
* Returns incoming request Metric attributes scoped to the request data
* @param {SpanAttributes} spanAttributes the span attributes
* @param {Attributes} spanAttributes the span attributes
* @param {{ component: string }} options used to pass data needed to create attributes
*/
export const getIncomingRequestMetricAttributes = (
spanAttributes: SpanAttributes
): MetricAttributes => {
const metricAttributes: MetricAttributes = {};
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] =
Expand All @@ -818,7 +816,7 @@ export const getIncomingRequestAttributesOnResponse = (
request: IncomingMessage,
response: ServerResponse,
semconvStability: SemconvStability
): SpanAttributes => {
): Attributes => {
// take socket from the request,
// since it may be detached from the response object in keep-alive mode
const { socket } = request;
Expand All @@ -829,7 +827,7 @@ export const getIncomingRequestAttributesOnResponse = (
};

const rpcMetadata = getRPCMetadata(context.active());
const oldAttributes: SpanAttributes = {};
const oldAttributes: Attributes = {};
if (socket) {
const { localAddress, localPort, remoteAddress, remotePort } = socket;
oldAttributes[SEMATTRS_NET_HOST_IP] = localAddress;
Expand Down Expand Up @@ -858,12 +856,12 @@ export const getIncomingRequestAttributesOnResponse = (

/**
* Returns incoming request Metric attributes scoped to the request data
* @param {SpanAttributes} spanAttributes the span attributes
* @param {Attributes} spanAttributes the span attributes
*/
export const getIncomingRequestMetricAttributesOnResponse = (
spanAttributes: SpanAttributes
): MetricAttributes => {
const metricAttributes: MetricAttributes = {};
spanAttributes: Attributes
): Attributes => {
const metricAttributes: Attributes = {};
metricAttributes[SEMATTRS_HTTP_STATUS_CODE] =
spanAttributes[SEMATTRS_HTTP_STATUS_CODE];
metricAttributes[SEMATTRS_NET_HOST_PORT] =
Expand Down
6 changes: 3 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 @@ -21,7 +21,7 @@ import {
Span as ISpan,
SpanKind,
trace,
SpanAttributes,
Attributes,
DiagConsoleLogger,
} from '@opentelemetry/api';
import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node';
Expand Down Expand Up @@ -144,13 +144,13 @@ export const responseHookFunction = (

export const startIncomingSpanHookFunction = (
request: IncomingMessage
): SpanAttributes => {
): Attributes => {
return { guid: request.headers?.guid };
};

export const startOutgoingSpanHookFunction = (
request: RequestOptions
): SpanAttributes => {
): Attributes => {
return { guid: request.headers?.guid };
};

Expand Down
Loading
Loading