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

fix(instrumentation-http): add server attributes after they become available #5081

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ 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,
Expand All @@ -80,6 +79,7 @@ import {
getIncomingRequestAttributesOnResponse,
getIncomingRequestMetricAttributes,
getIncomingRequestMetricAttributesOnResponse,
getIncomingStableRequestMetricAttributesOnResponse,
getOutgoingRequestAttributes,
getOutgoingRequestAttributesOnResponse,
getOutgoingRequestMetricAttributes,
Expand All @@ -93,7 +93,7 @@ import {
} from './utils';

/**
* Http instrumentation instrumentation for Opentelemetry
* `node:http` and `node:https` instrumentation for OpenTelemetry
*/
export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentationConfig> {
/** keep track on spans not ended */
Expand Down Expand Up @@ -416,7 +416,8 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
* @param request The original request object.
* @param span representing the current operation
* @param startTime representing the start time of the request to calculate duration in Metric
* @param oldMetricAttributes metric attributes
* @param oldMetricAttributes metric attributes for old semantic conventions
* @param stableMetricAttributes metric attributes for new semantic conventions
*/
private _traceClientRequest(
request: http.ClientRequest,
Expand Down Expand Up @@ -652,12 +653,6 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
[ATTR_URL_SCHEME]: spanAttributes[ATTR_URL_SCHEME],
};

// required if and only if one was sent, same as span requirement
if (spanAttributes[ATTR_HTTP_RESPONSE_STATUS_CODE]) {
stableMetricAttributes[ATTR_HTTP_RESPONSE_STATUS_CODE] =
spanAttributes[ATTR_HTTP_RESPONSE_STATUS_CODE];
}

// recommended if and only if one was sent, same as span recommendation
if (spanAttributes[ATTR_NETWORK_PROTOCOL_VERSION]) {
stableMetricAttributes[ATTR_NETWORK_PROTOCOL_VERSION] =
Expand Down Expand Up @@ -917,6 +912,10 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
oldMetricAttributes,
getIncomingRequestMetricAttributesOnResponse(attributes)
);
stableMetricAttributes = Object.assign(
stableMetricAttributes,
getIncomingStableRequestMetricAttributesOnResponse(attributes)
);

this._headerCapture.server.captureResponseHeaders(span, header =>
response.getHeader(header)
Expand All @@ -929,7 +928,6 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
const route = attributes[SEMATTRS_HTTP_ROUTE];
if (route) {
span.updateName(`${request.method || 'GET'} ${route}`);
stableMetricAttributes[ATTR_HTTP_ROUTE] = route;
}

if (this.getConfig().applyCustomAttributesOnSpan) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
ATTR_HTTP_REQUEST_METHOD,
ATTR_HTTP_REQUEST_METHOD_ORIGINAL,
ATTR_HTTP_RESPONSE_STATUS_CODE,
ATTR_HTTP_ROUTE,
ATTR_NETWORK_PEER_ADDRESS,
ATTR_NETWORK_PEER_PORT,
ATTR_NETWORK_PROTOCOL_VERSION,
Expand Down Expand Up @@ -822,7 +823,7 @@ export const getIncomingRequestAttributesOnResponse = (
const { socket } = request;
const { statusCode, statusMessage } = response;

const newAttributes = {
const newAttributes: Attributes = {
[ATTR_HTTP_RESPONSE_STATUS_CODE]: statusCode,
};

Expand All @@ -842,6 +843,7 @@ export const getIncomingRequestAttributesOnResponse = (

if (rpcMetadata?.type === RPCType.HTTP && rpcMetadata.route !== undefined) {
oldAttributes[SEMATTRS_HTTP_ROUTE] = rpcMetadata.route;
newAttributes[ATTR_HTTP_ROUTE] = rpcMetadata.route;
}

switch (semconvStability) {
Expand Down Expand Up @@ -872,6 +874,22 @@ export const getIncomingRequestMetricAttributesOnResponse = (
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];
}

// required if and only if one was sent, same as span requirement
if (spanAttributes[ATTR_HTTP_RESPONSE_STATUS_CODE]) {
metricAttributes[ATTR_HTTP_RESPONSE_STATUS_CODE] =
spanAttributes[ATTR_HTTP_RESPONSE_STATUS_CODE];
}
return metricAttributes;
};

export function headerCapture(type: 'request' | 'response', headers: string[]) {
const normalizedHeaders = new Map<string, string>();
for (let i = 0, len = headers.length; i < len; i++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
ATTR_CLIENT_ADDRESS,
ATTR_HTTP_REQUEST_METHOD,
ATTR_HTTP_RESPONSE_STATUS_CODE,
ATTR_HTTP_ROUTE,
ATTR_NETWORK_PEER_ADDRESS,
ATTR_NETWORK_PEER_PORT,
ATTR_NETWORK_PROTOCOL_VERSION,
Expand Down Expand Up @@ -1134,6 +1135,32 @@ describe('HttpInstrumentation', () => {
[ATTR_URL_SCHEME]: protocol,
});
});

it('should generate semconv 1.27 server spans with route when RPC metadata is available', async () => {
const response = await httpRequest.get(
`${protocol}://${hostname}:${serverPort}${pathname}/setroute`
);
const spans = memoryExporter.getFinishedSpans();
const [incomingSpan, _] = spans;
assert.strictEqual(spans.length, 2);

const body = JSON.parse(response.data);

// should have only required and recommended attributes for semconv 1.27
assert.deepStrictEqual(incomingSpan.attributes, {
[ATTR_CLIENT_ADDRESS]: body.address,
[ATTR_HTTP_REQUEST_METHOD]: HTTP_REQUEST_METHOD_VALUE_GET,
[ATTR_SERVER_ADDRESS]: hostname,
[ATTR_HTTP_ROUTE]: 'TheRoute',
[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,
});
});
});

describe('with semconv stability set to http/dup', () => {
Expand All @@ -1146,6 +1173,13 @@ describe('HttpInstrumentation', () => {
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) })
Expand Down Expand Up @@ -1241,6 +1275,50 @@ describe('HttpInstrumentation', () => {
[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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node';
import {
ATTR_HTTP_REQUEST_METHOD,
ATTR_HTTP_RESPONSE_STATUS_CODE,
ATTR_HTTP_ROUTE,
ATTR_NETWORK_PROTOCOL_VERSION,
ATTR_SERVER_ADDRESS,
Expand Down Expand Up @@ -181,7 +182,7 @@ describe('metrics', () => {
});
});

describe('with no semconv stability set to stable', () => {
describe('with semconv stability set to stable', () => {
before(() => {
instrumentation['_semconvStability'] = SemconvStability.STABLE;
});
Expand Down Expand Up @@ -217,7 +218,9 @@ describe('metrics', () => {
assert.deepStrictEqual(metrics[0].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[1].dataPointType, DataPointType.HISTOGRAM);
Expand All @@ -244,7 +247,7 @@ describe('metrics', () => {
});
});

describe('with no semconv stability set to duplicate', () => {
describe('with semconv stability set to duplicate', () => {
before(() => {
instrumentation['_semconvStability'] = SemconvStability.DUPLICATE;
});
Expand Down Expand Up @@ -353,6 +356,7 @@ describe('metrics', () => {
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',
});
Expand Down
Loading