From 9480f1555398e55bbeef9c4835b12731a0da2131 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Mon, 21 Oct 2024 12:03:37 +0200 Subject: [PATCH 1/4] fix(instrumentation-http): add server metric attributes after they become available --- .../src/http.ts | 17 +++++++--------- .../src/utils.ts | 20 ++++++++++++++++++- .../test/functionals/http-metrics.test.ts | 9 ++++++--- 3 files changed, 32 insertions(+), 14 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts index 0d0a63920b2..f88122587ef 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, @@ -79,7 +78,7 @@ import { getIncomingRequestAttributes, getIncomingRequestAttributesOnResponse, getIncomingRequestMetricAttributes, - getIncomingRequestMetricAttributesOnResponse, + getIncomingRequestMetricAttributesOnResponse, getIncomingStableRequestMetricAttributesOnResponse, getOutgoingRequestAttributes, getOutgoingRequestAttributesOnResponse, getOutgoingRequestMetricAttributes, @@ -416,7 +415,8 @@ export class HttpInstrumentation extends InstrumentationBase response.getHeader(header) @@ -929,7 +927,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-metrics.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-metrics.test.ts index de43ae8c521..66d992df289 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 @@ -21,7 +21,7 @@ import { } from '@opentelemetry/sdk-metrics'; import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node'; import { - ATTR_HTTP_REQUEST_METHOD, + ATTR_HTTP_REQUEST_METHOD, ATTR_HTTP_RESPONSE_STATUS_CODE, ATTR_HTTP_ROUTE, ATTR_NETWORK_PROTOCOL_VERSION, ATTR_SERVER_ADDRESS, @@ -181,7 +181,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 +217,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 +246,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 +355,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', }); From 335fbf31330c410ca8dbf04a0116decabd64687f Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Mon, 21 Oct 2024 12:21:04 +0200 Subject: [PATCH 2/4] fixup! fix(instrumentation-http): add server metric attributes after they become available --- .../packages/opentelemetry-instrumentation-http/src/http.ts | 5 +++-- .../test/functionals/http-metrics.test.ts | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts index f88122587ef..3fea86becdf 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts @@ -78,7 +78,8 @@ import { getIncomingRequestAttributes, getIncomingRequestAttributesOnResponse, getIncomingRequestMetricAttributes, - getIncomingRequestMetricAttributesOnResponse, getIncomingStableRequestMetricAttributesOnResponse, + getIncomingRequestMetricAttributesOnResponse, + getIncomingStableRequestMetricAttributesOnResponse, getOutgoingRequestAttributes, getOutgoingRequestAttributesOnResponse, getOutgoingRequestMetricAttributes, @@ -416,7 +417,7 @@ export class HttpInstrumentation extends InstrumentationBase Date: Mon, 21 Oct 2024 12:22:30 +0200 Subject: [PATCH 3/4] fixup! fix(instrumentation-http): add server metric attributes after they become available --- .../packages/opentelemetry-instrumentation-http/src/http.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts index 3fea86becdf..279c97f6c5e 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts @@ -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 */ From ec50dc6d6acf2069d253986f8b45fa77b8542752 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Mon, 21 Oct 2024 13:10:04 +0200 Subject: [PATCH 4/4] test(instrumentation-http): also add route tests for http and http/dup --- .../test/functionals/http-enable.test.ts | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) 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', () => {