From 4c57fba9b8259df886d8758c0060826ac7595139 Mon Sep 17 00:00:00 2001 From: Bryan English Date: Mon, 26 Oct 2026 15:21:23 -0400 Subject: [PATCH] fix: align server.address with the semantic conventions server.address must be the bare host, with the port in server.port. The server span copied the Host header verbatim, port included, and the undici span set the whole origin. Both now carry only the host. A new test validates every emitted attribute name against the semantic-conventions registry at test time: the @opentelemetry/semantic-conventions package, whose exports are the attribute names the conventions define, becomes a devDependency, and one trace through every instrumentation shape walks span, event, and resource attributes through the registry. Signed-off-by: Bryan English --- .github/workflows/ci.yml | 1 + README.md | 4 +- lib/instrumentations.js | 4 +- package-lock.json | 11 ++++++ package.json | 1 + test/test-http-server.js | 1 + test/test-semconv.js | 80 ++++++++++++++++++++++++++++++++++++++++ test/test-undici.js | 2 +- 8 files changed, 99 insertions(+), 5 deletions(-) create mode 100644 test/test-semconv.js diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 554d479..33fec51 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,6 +21,7 @@ jobs: - uses: actions/setup-node@v7 with: node-version: latest + - run: npm ci - run: npm test benchmark: diff --git a/README.md b/README.md index e852ffd..f9feeb9 100644 --- a/README.md +++ b/README.md @@ -88,7 +88,7 @@ Attributes set on server spans: | `url.path` | Request URL path (without query) | Always | | `url.query` | Query string (without `?`) | When query string is present | | `url.scheme` | `http` or `https` | Always | -| `server.address` | Host header value | When `Host` header is present | +| `server.address` | Requested host, without the port | When `Host` header is present | | `network.protocol.version` | HTTP version (e.g. `1.1`) | Always | | `http.response.status_code` | Response status code | When response finishes | | `error.type` | HTTP status code as string | On 5xx responses | @@ -136,7 +136,7 @@ Attributes set on undici/fetch client spans: | --------------------------- | ------------------------- | ------------------------- | | `http.request.method` | HTTP method | Always | | `url.full` | Full request URL | Always | -| `server.address` | Target origin | Always | +| `server.address` | Target host | Always | | `server.port` | Target port | Always | | `http.response.status_code` | Response status code | When response is received | | `error.type` | Status code or error name | On 4xx/5xx or errors | diff --git a/lib/instrumentations.js b/lib/instrumentations.js index f10dab1..f670aec 100644 --- a/lib/instrumentations.js +++ b/lib/instrumentations.js @@ -83,7 +83,7 @@ function onHttpServerRequestStart({ request, response, socket }) { span.setAttribute('network.protocol.version', request.httpVersion || '1.1'); const host = request.headers?.host; - if (host) span.setAttribute('server.address', host); + if (host) span.setAttribute('server.address', splitAuthority(host)[0]); // The span is stashed on the request so the response handlers do not // need a second AsyncLocalStorage lookup. @@ -243,9 +243,9 @@ function onUndiciRequestCreate({ request }) { const { origin } = request; const path = request.path; if (origin) { - span.setAttribute('server.address', origin); // The origin omits the port when it is the scheme default. const parsed = new URL(origin); + span.setAttribute('server.address', parsed.hostname); span.setAttribute( 'server.port', Number(parsed.port) || (parsed.protocol === 'https:' ? 443 : 80), diff --git a/package-lock.json b/package-lock.json index fa26e29..71bb80e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,12 +9,23 @@ "version": "0.0.0", "license": "MIT", "devDependencies": { + "@opentelemetry/semantic-conventions": "1.43.0", "prettier": "3.9.10" }, "engines": { "node": ">=26.9.0" } }, + "node_modules/@opentelemetry/semantic-conventions": { + "version": "1.43.0", + "resolved": "https://registry.npmjs.org/@opentelemetry/semantic-conventions/-/semantic-conventions-1.43.0.tgz", + "integrity": "sha512-eSYWTm620tTk45EKSedaUL8MFYI8hW164hIXsgIHyxu3VobUB3fFCu5t0hQby6OoWRPsG1KkKUG2M5UadiLiVg==", + "dev": true, + "license": "Apache-2.0", + "engines": { + "node": ">=14" + } + }, "node_modules/prettier": { "version": "3.9.10", "resolved": "https://registry.npmjs.org/prettier/-/prettier-3.9.10.tgz", diff --git a/package.json b/package.json index d15c0c0..6873f9b 100644 --- a/package.json +++ b/package.json @@ -31,6 +31,7 @@ "format:check": "prettier --check ." }, "devDependencies": { + "@opentelemetry/semantic-conventions": "1.43.0", "prettier": "3.9.10" } } diff --git a/test/test-http-server.js b/test/test-http-server.js index 486802d..69e2661 100644 --- a/test/test-http-server.js +++ b/test/test-http-server.js @@ -48,6 +48,7 @@ describe('otel HTTP server spans', () => { assert.partialDeepStrictEqual(spanAttrs(serverSpan), { 'http.request.method': 'GET', + 'server.address': '127.0.0.1', 'url.path': '/test-path', 'http.response.status_code': '200', }); diff --git a/test/test-semconv.js b/test/test-semconv.js new file mode 100644 index 0000000..de09ee6 --- /dev/null +++ b/test/test-semconv.js @@ -0,0 +1,80 @@ +'use strict'; + +const assert = require('node:assert'); +const { after, before, describe, it } = require('node:test'); +// The registry of defined attribute names: the package's exports are the +// attribute names the OpenTelemetry semantic conventions define. +const semconv = require('@opentelemetry/semantic-conventions'); +const { flush } = require('../lib/flush'); +const { startTracing, startHTTPServer, httpGet } = require('./common/otel'); + +const registry = new Set( + Object.values(semconv).filter((v) => typeof v === 'string'), +); + +describe('otel semantic conventions', () => { + let collector; + + before(async () => { + collector = await startTracing(); + }); + + after(() => collector.close()); + + it('emits only attribute names the conventions define', async () => { + // One trace through every instrumentation shape: a server span with + // its query attribute, http and fetch client spans, a failed request + // for the error attributes, and a refused connection for the + // exception event attributes. + const ok = await startHTTPServer(); + const failing = await startHTTPServer((req, res) => { + res.writeHead(500); + res.end(); + }); + + await httpGet(ok.port, '/semconv?token=1'); + await fetch(`http://127.0.0.1:${ok.port}/fetch`) + .then((res) => res.text()) + .catch(() => {}); + await httpGet(failing.port, '/fail'); + await httpGet(1, '/refused').catch(() => {}); + + flush(); + const payload = await collector.next(); + + await ok.close(); + await failing.close(); + + const spans = payload.resourceSpans[0].scopeSpans[0].spans; + assert.ok( + spans.some((s) => s.kind === 2), + 'Expected a server span', + ); + assert.ok( + spans.some((s) => s.kind === 3), + 'Expected client spans', + ); + + const assertDefined = (key, where) => + assert.ok( + registry.has(key), + `${where} attribute "${key}" is not defined by the ` + + 'semantic conventions', + ); + + for (const span of spans) { + for (const attr of span.attributes ?? []) { + assertDefined(attr.key, 'span'); + } + for (const event of span.events ?? []) { + for (const attr of event.attributes ?? []) { + assertDefined(attr.key, 'event'); + } + } + } + + for (const attr of payload.resourceSpans[0].resource.attributes) { + assertDefined(attr.key, 'resource'); + } + }); +}); diff --git a/test/test-undici.js b/test/test-undici.js index 2655464..0b5c73b 100644 --- a/test/test-undici.js +++ b/test/test-undici.js @@ -64,7 +64,7 @@ describe('otel undici spans', () => { assert.partialDeepStrictEqual(spanAttrs(clientSpan), { 'http.request.method': 'GET', - 'server.address': `http://127.0.0.1:${target.port}`, + 'server.address': '127.0.0.1', 'server.port': String(target.port), 'url.full': `http://127.0.0.1:${target.port}/fetch-test`, 'http.response.status_code': '200',