Skip to content

Commit

Permalink
fix(instrumentation-http): add server attributes after they become av…
Browse files Browse the repository at this point in the history 
…ailable (open-telemetry#5081)
  • Loading branch information
@pichlermarc @Annosha
pichlermarc authored and Annosha committed Nov 2, 2024
1 parent af70a67 commit 5f3329c
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 13 deletions.
18 changes: 8 additions & 10 deletions experimental/packages/opentelemetry-instrumentation-http/src/http.ts
View file
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 @@ -430,7 +430,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 @@ -666,12 +667,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 @@ -931,6 +926,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 @@ -943,7 +942,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
20 changes: 19 additions & 1 deletion experimental/packages/opentelemetry-instrumentation-http/src/utils.ts
View file
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
78 changes: 78 additions & 0 deletions ...rimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts
View file
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
8 changes: 6 additions & 2 deletions ...imental/packages/opentelemetry-instrumentation-http/test/functionals/http-metrics.test.ts
View file
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

0 comments on commit 5f3329c

Please sign in to comment.