Skip to content

Commit 330172c

Browse files
pichlermarcpichlermarc
authored
fix(instrumentation-http): add server attributes after they become available (#5081)
1 parent 55a1fc8 commit 330172c

File tree

4 files changed

+111
-13
lines changed

4 files changed

+111
-13
lines changed

‎experimental/packages/opentelemetry-instrumentation-http/src/http.ts‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ import { errorMonitor } from 'events';
6565
import {
6666
ATTR_HTTP_REQUEST_METHOD,
6767
ATTR_HTTP_RESPONSE_STATUS_CODE,
68-
ATTR_HTTP_ROUTE,
6968
ATTR_NETWORK_PROTOCOL_VERSION,
7069
ATTR_SERVER_ADDRESS,
7170
ATTR_SERVER_PORT,
@@ -80,6 +79,7 @@ import {
8079
getIncomingRequestAttributesOnResponse,
8180
getIncomingRequestMetricAttributes,
8281
getIncomingRequestMetricAttributesOnResponse,
82+
getIncomingStableRequestMetricAttributesOnResponse,
8383
getOutgoingRequestAttributes,
8484
getOutgoingRequestAttributesOnResponse,
8585
getOutgoingRequestMetricAttributes,
@@ -93,7 +93,7 @@ import {
9393
} from './utils';
9494

9595
/**
96-
* Http instrumentation instrumentation for Opentelemetry
96+
* `node:http` and `node:https` instrumentation for OpenTelemetry
9797
*/
9898
export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentationConfig> {
9999
/** keep track on spans not ended */
@@ -430,7 +430,8 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
430430
* @param request The original request object.
431431
* @param span representing the current operation
432432
* @param startTime representing the start time of the request to calculate duration in Metric
433-
* @param oldMetricAttributes metric attributes
433+
* @param oldMetricAttributes metric attributes for old semantic conventions
434+
* @param stableMetricAttributes metric attributes for new semantic conventions
434435
*/
435436
private _traceClientRequest(
436437
request: http.ClientRequest,
@@ -666,12 +667,6 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
666667
[ATTR_URL_SCHEME]: spanAttributes[ATTR_URL_SCHEME],
667668
};
668669

669-
// required if and only if one was sent, same as span requirement
670-
if (spanAttributes[ATTR_HTTP_RESPONSE_STATUS_CODE]) {
671-
stableMetricAttributes[ATTR_HTTP_RESPONSE_STATUS_CODE] =
672-
spanAttributes[ATTR_HTTP_RESPONSE_STATUS_CODE];
673-
}
674-
675670
// recommended if and only if one was sent, same as span recommendation
676671
if (spanAttributes[ATTR_NETWORK_PROTOCOL_VERSION]) {
677672
stableMetricAttributes[ATTR_NETWORK_PROTOCOL_VERSION] =
@@ -931,6 +926,10 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
931926
oldMetricAttributes,
932927
getIncomingRequestMetricAttributesOnResponse(attributes)
933928
);
929+
stableMetricAttributes = Object.assign(
930+
stableMetricAttributes,
931+
getIncomingStableRequestMetricAttributesOnResponse(attributes)
932+
);
934933

935934
this._headerCapture.server.captureResponseHeaders(span, header =>
936935
response.getHeader(header)
@@ -943,7 +942,6 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
943942
const route = attributes[SEMATTRS_HTTP_ROUTE];
944943
if (route) {
945944
span.updateName(`${request.method || 'GET'} ${route}`);
946-
stableMetricAttributes[ATTR_HTTP_ROUTE] = route;
947945
}
948946

949947
if (this.getConfig().applyCustomAttributesOnSpan) {

‎experimental/packages/opentelemetry-instrumentation-http/src/utils.ts‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
ATTR_HTTP_REQUEST_METHOD,
2727
ATTR_HTTP_REQUEST_METHOD_ORIGINAL,
2828
ATTR_HTTP_RESPONSE_STATUS_CODE,
29+
ATTR_HTTP_ROUTE,
2930
ATTR_NETWORK_PEER_ADDRESS,
3031
ATTR_NETWORK_PEER_PORT,
3132
ATTR_NETWORK_PROTOCOL_VERSION,
@@ -822,7 +823,7 @@ export const getIncomingRequestAttributesOnResponse = (
822823
const { socket } = request;
823824
const { statusCode, statusMessage } = response;
824825

825-
const newAttributes = {
826+
const newAttributes: Attributes = {
826827
[ATTR_HTTP_RESPONSE_STATUS_CODE]: statusCode,
827828
};
828829

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

843844
if (rpcMetadata?.type === RPCType.HTTP && rpcMetadata.route !== undefined) {
844845
oldAttributes[SEMATTRS_HTTP_ROUTE] = rpcMetadata.route;
846+
newAttributes[ATTR_HTTP_ROUTE] = rpcMetadata.route;
845847
}
846848

847849
switch (semconvStability) {
@@ -872,6 +874,22 @@ export const getIncomingRequestMetricAttributesOnResponse = (
872874
return metricAttributes;
873875
};
874876

877+
export const getIncomingStableRequestMetricAttributesOnResponse = (
878+
spanAttributes: Attributes
879+
): Attributes => {
880+
const metricAttributes: Attributes = {};
881+
if (spanAttributes[ATTR_HTTP_ROUTE] !== undefined) {
882+
metricAttributes[ATTR_HTTP_ROUTE] = spanAttributes[SEMATTRS_HTTP_ROUTE];
883+
}
884+
885+
// required if and only if one was sent, same as span requirement
886+
if (spanAttributes[ATTR_HTTP_RESPONSE_STATUS_CODE]) {
887+
metricAttributes[ATTR_HTTP_RESPONSE_STATUS_CODE] =
888+
spanAttributes[ATTR_HTTP_RESPONSE_STATUS_CODE];
889+
}
890+
return metricAttributes;
891+
};
892+
875893
export function headerCapture(type: 'request' | 'response', headers: string[]) {
876894
const normalizedHeaders = new Map<string, string>();
877895
for (let i = 0, len = headers.length; i < len; i++) {

‎experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {
3333
ATTR_CLIENT_ADDRESS,
3434
ATTR_HTTP_REQUEST_METHOD,
3535
ATTR_HTTP_RESPONSE_STATUS_CODE,
36+
ATTR_HTTP_ROUTE,
3637
ATTR_NETWORK_PEER_ADDRESS,
3738
ATTR_NETWORK_PEER_PORT,
3839
ATTR_NETWORK_PROTOCOL_VERSION,
@@ -1134,6 +1135,32 @@ describe('HttpInstrumentation', () => {
11341135
[ATTR_URL_SCHEME]: protocol,
11351136
});
11361137
});
1138+
1139+
it('should generate semconv 1.27 server spans with route when RPC metadata is available', async () => {
1140+
const response = await httpRequest.get(
1141+
`${protocol}://${hostname}:${serverPort}${pathname}/setroute`
1142+
);
1143+
const spans = memoryExporter.getFinishedSpans();
1144+
const [incomingSpan, _] = spans;
1145+
assert.strictEqual(spans.length, 2);
1146+
1147+
const body = JSON.parse(response.data);
1148+
1149+
// should have only required and recommended attributes for semconv 1.27
1150+
assert.deepStrictEqual(incomingSpan.attributes, {
1151+
[ATTR_CLIENT_ADDRESS]: body.address,
1152+
[ATTR_HTTP_REQUEST_METHOD]: HTTP_REQUEST_METHOD_VALUE_GET,
1153+
[ATTR_SERVER_ADDRESS]: hostname,
1154+
[ATTR_HTTP_ROUTE]: 'TheRoute',
1155+
[ATTR_SERVER_PORT]: serverPort,
1156+
[ATTR_HTTP_RESPONSE_STATUS_CODE]: 200,
1157+
[ATTR_NETWORK_PEER_ADDRESS]: body.address,
1158+
[ATTR_NETWORK_PEER_PORT]: response.clientRemotePort,
1159+
[ATTR_NETWORK_PROTOCOL_VERSION]: '1.1',
1160+
[ATTR_URL_PATH]: `${pathname}/setroute`,
1161+
[ATTR_URL_SCHEME]: protocol,
1162+
});
1163+
});
11371164
});
11381165

11391166
describe('with semconv stability set to http/dup', () => {
@@ -1146,6 +1173,13 @@ describe('HttpInstrumentation', () => {
11461173
instrumentation['_semconvStability'] = SemconvStability.DUPLICATE;
11471174
instrumentation.enable();
11481175
server = http.createServer((request, response) => {
1176+
if (request.url?.includes('/setroute')) {
1177+
const rpcData = getRPCMetadata(context.active());
1178+
assert.ok(rpcData != null);
1179+
assert.strictEqual(rpcData.type, RPCType.HTTP);
1180+
assert.strictEqual(rpcData.route, undefined);
1181+
rpcData.route = 'TheRoute';
1182+
}
11491183
response.setHeader('Content-Type', 'application/json');
11501184
response.end(
11511185
JSON.stringify({ address: getRemoteClientAddress(request) })
@@ -1241,6 +1275,50 @@ describe('HttpInstrumentation', () => {
12411275
[AttributeNames.HTTP_STATUS_TEXT]: 'OK',
12421276
});
12431277
});
1278+
1279+
it('should create server spans with semconv 1.27 and old 1.7 including http.route if RPC metadata is available', async () => {
1280+
const response = await httpRequest.get(
1281+
`${protocol}://${hostname}:${serverPort}${pathname}/setroute`
1282+
);
1283+
const spans = memoryExporter.getFinishedSpans();
1284+
assert.strictEqual(spans.length, 2);
1285+
const incomingSpan = spans[0];
1286+
const body = JSON.parse(response.data);
1287+
1288+
// should have only required and recommended attributes for semconv 1.27
1289+
assert.deepStrictEqual(incomingSpan.attributes, {
1290+
// 1.27 attributes
1291+
[ATTR_CLIENT_ADDRESS]: body.address,
1292+
[ATTR_HTTP_REQUEST_METHOD]: HTTP_REQUEST_METHOD_VALUE_GET,
1293+
[ATTR_SERVER_ADDRESS]: hostname,
1294+
[ATTR_SERVER_PORT]: serverPort,
1295+
[ATTR_HTTP_RESPONSE_STATUS_CODE]: 200,
1296+
[ATTR_NETWORK_PEER_ADDRESS]: body.address,
1297+
[ATTR_NETWORK_PEER_PORT]: response.clientRemotePort,
1298+
[ATTR_NETWORK_PROTOCOL_VERSION]: '1.1',
1299+
[ATTR_URL_PATH]: `${pathname}/setroute`,
1300+
[ATTR_URL_SCHEME]: protocol,
1301+
[ATTR_HTTP_ROUTE]: 'TheRoute',
1302+
1303+
// 1.7 attributes
1304+
[SEMATTRS_HTTP_FLAVOR]: '1.1',
1305+
[SEMATTRS_HTTP_HOST]: `${hostname}:${serverPort}`,
1306+
[SEMATTRS_HTTP_METHOD]: 'GET',
1307+
[SEMATTRS_HTTP_SCHEME]: protocol,
1308+
[SEMATTRS_HTTP_STATUS_CODE]: 200,
1309+
[SEMATTRS_HTTP_TARGET]: `${pathname}/setroute`,
1310+
[SEMATTRS_HTTP_URL]: `http://${hostname}:${serverPort}${pathname}/setroute`,
1311+
[SEMATTRS_NET_TRANSPORT]: 'ip_tcp',
1312+
[SEMATTRS_NET_HOST_IP]: body.address,
1313+
[SEMATTRS_NET_HOST_NAME]: hostname,
1314+
[SEMATTRS_NET_HOST_PORT]: serverPort,
1315+
[SEMATTRS_NET_PEER_IP]: body.address,
1316+
[SEMATTRS_NET_PEER_PORT]: response.clientRemotePort,
1317+
1318+
// unspecified old names
1319+
[AttributeNames.HTTP_STATUS_TEXT]: 'OK',
1320+
});
1321+
});
12441322
});
12451323

12461324
describe('with require parent span', () => {

‎experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-metrics.test.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node';
2323
import {
2424
ATTR_HTTP_REQUEST_METHOD,
25+
ATTR_HTTP_RESPONSE_STATUS_CODE,
2526
ATTR_HTTP_ROUTE,
2627
ATTR_NETWORK_PROTOCOL_VERSION,
2728
ATTR_SERVER_ADDRESS,
@@ -181,7 +182,7 @@ describe('metrics', () => {
181182
});
182183
});
183184

184-
describe('with no semconv stability set to stable', () => {
185+
describe('with semconv stability set to stable', () => {
185186
before(() => {
186187
instrumentation['_semconvStability'] = SemconvStability.STABLE;
187188
});
@@ -217,7 +218,9 @@ describe('metrics', () => {
217218
assert.deepStrictEqual(metrics[0].dataPoints[0].attributes, {
218219
[ATTR_HTTP_REQUEST_METHOD]: 'GET',
219220
[ATTR_URL_SCHEME]: 'http',
221+
[ATTR_HTTP_RESPONSE_STATUS_CODE]: 200,
220222
[ATTR_NETWORK_PROTOCOL_VERSION]: '1.1',
223+
[ATTR_HTTP_ROUTE]: 'TheRoute',
221224
});
222225

223226
assert.strictEqual(metrics[1].dataPointType, DataPointType.HISTOGRAM);
@@ -244,7 +247,7 @@ describe('metrics', () => {
244247
});
245248
});
246249

247-
describe('with no semconv stability set to duplicate', () => {
250+
describe('with semconv stability set to duplicate', () => {
248251
before(() => {
249252
instrumentation['_semconvStability'] = SemconvStability.DUPLICATE;
250253
});
@@ -353,6 +356,7 @@ describe('metrics', () => {
353356
assert.deepStrictEqual(metrics[2].dataPoints[0].attributes, {
354357
[ATTR_HTTP_REQUEST_METHOD]: 'GET',
355358
[ATTR_URL_SCHEME]: 'http',
359+
[ATTR_HTTP_RESPONSE_STATUS_CODE]: 200,
356360
[ATTR_NETWORK_PROTOCOL_VERSION]: '1.1',
357361
[ATTR_HTTP_ROUTE]: 'TheRoute',
358362
});

0 commit comments

Comments
 (0)