From 330172c1d8a499357451c4acc57c557f627a5089 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Wed, 23 Oct 2024 11:15:16 +0200 Subject: [PATCH] fix(instrumentation-http): add server attributes after they become available (#5081) --- .../src/http.ts | 18 ++--- .../src/utils.ts | 20 ++++- .../test/functionals/http-enable.test.ts | 78 +++++++++++++++++++ .../test/functionals/http-metrics.test.ts | 8 +- 4 files changed, 111 insertions(+), 13 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts index 7c7eb64dada..733106f1623 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts @@ -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, @@ -80,6 +79,7 @@ import { getIncomingRequestAttributesOnResponse, getIncomingRequestMetricAttributes, getIncomingRequestMetricAttributesOnResponse, + getIncomingStableRequestMetricAttributesOnResponse, getOutgoingRequestAttributes, getOutgoingRequestAttributesOnResponse, getOutgoingRequestMetricAttributes, @@ -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 { /** keep track on spans not ended */ @@ -430,7 +430,8 @@ export class HttpInstrumentation extends InstrumentationBase response.getHeader(header) @@ -943,7 +942,6 @@ export class HttpInstrumentation extends InstrumentationBase { + 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(); for (let i = 0, len = headers.length; i < len; i++) { 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 b6470e1f3e6..033317306ee 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 @@ -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, @@ -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', () => { @@ -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) }) @@ -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', () => { 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 de43ae8c521..9f9d7b4a354 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 @@ -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, @@ -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; }); @@ -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); @@ -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; }); @@ -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', });