Skip to content

Commit ad89e85

Browse files
committed
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 <bryan@bryanenglish.com>
1 parent 03556fb commit ad89e85

7 files changed

Lines changed: 98 additions & 5 deletions

File tree

‎README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ Attributes set on server spans:
8888
| `url.path` | Request URL path (without query) | Always |
8989
| `url.query` | Query string (without `?`) | When query string is present |
9090
| `url.scheme` | `http` or `https` | Always |
91-
| `server.address` | Host header value | When `Host` header is present |
91+
| `server.address` | Requested host, without the port | When `Host` header is present |
9292
| `network.protocol.version` | HTTP version (e.g. `1.1`) | Always |
9393
| `http.response.status_code` | Response status code | When response finishes |
9494
| `error.type` | HTTP status code as string | On 5xx responses |
@@ -136,7 +136,7 @@ Attributes set on undici/fetch client spans:
136136
| --------------------------- | ------------------------- | ------------------------- |
137137
| `http.request.method` | HTTP method | Always |
138138
| `url.full` | Full request URL | Always |
139-
| `server.address` | Target origin | Always |
139+
| `server.address` | Target host | Always |
140140
| `server.port` | Target port | Always |
141141
| `http.response.status_code` | Response status code | When response is received |
142142
| `error.type` | Status code or error name | On 4xx/5xx or errors |

‎lib/instrumentations.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ function onHttpServerRequestStart({ request, response, socket }) {
8383
span.setAttribute('network.protocol.version', request.httpVersion || '1.1');
8484

8585
const host = request.headers?.host;
86-
if (host) span.setAttribute('server.address', host);
86+
if (host) span.setAttribute('server.address', splitAuthority(host)[0]);
8787

8888
// The span is stashed on the request so the response handlers do not
8989
// need a second AsyncLocalStorage lookup.
@@ -243,9 +243,9 @@ function onUndiciRequestCreate({ request }) {
243243
const { origin } = request;
244244
const path = request.path;
245245
if (origin) {
246-
span.setAttribute('server.address', origin);
247246
// The origin omits the port when it is the scheme default.
248247
const parsed = new URL(origin);
248+
span.setAttribute('server.address', parsed.hostname);
249249
span.setAttribute(
250250
'server.port',
251251
Number(parsed.port) || (parsed.protocol === 'https:' ? 443 : 80),

‎package-lock.json‎

Lines changed: 11 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
"format:check": "prettier --check ."
3232
},
3333
"devDependencies": {
34+
"@opentelemetry/semantic-conventions": "1.43.0",
3435
"prettier": "3.9.10"
3536
}
3637
}

‎test/test-http-server.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ describe('otel HTTP server spans', () => {
4848

4949
assert.partialDeepStrictEqual(spanAttrs(serverSpan), {
5050
'http.request.method': 'GET',
51+
'server.address': '127.0.0.1',
5152
'url.path': '/test-path',
5253
'http.response.status_code': '200',
5354
});

‎test/test-semconv.js‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
'use strict';
2+
3+
const assert = require('node:assert');
4+
const { after, before, describe, it } = require('node:test');
5+
// The registry of defined attribute names: the package's exports are the
6+
// attribute names the OpenTelemetry semantic conventions define.
7+
const semconv = require('@opentelemetry/semantic-conventions');
8+
const { flush } = require('../lib/flush');
9+
const { startTracing, startHTTPServer, httpGet } = require('./common/otel');
10+
11+
const registry = new Set(
12+
Object.values(semconv).filter((v) => typeof v === 'string'),
13+
);
14+
15+
describe('otel semantic conventions', () => {
16+
let collector;
17+
18+
before(async () => {
19+
collector = await startTracing();
20+
});
21+
22+
after(() => collector.close());
23+
24+
it('emits only attribute names the conventions define', async () => {
25+
// One trace through every instrumentation shape: a server span with
26+
// its query attribute, http and fetch client spans, a failed request
27+
// for the error attributes, and a refused connection for the
28+
// exception event attributes.
29+
const ok = await startHTTPServer();
30+
const failing = await startHTTPServer((req, res) => {
31+
res.writeHead(500);
32+
res.end();
33+
});
34+
35+
await httpGet(ok.port, '/semconv?token=1');
36+
await fetch(`http://127.0.0.1:${ok.port}/fetch`)
37+
.then((res) => res.text())
38+
.catch(() => {});
39+
await httpGet(failing.port, '/fail');
40+
await httpGet(1, '/refused').catch(() => {});
41+
42+
flush();
43+
const payload = await collector.next();
44+
45+
await ok.close();
46+
await failing.close();
47+
48+
const spans = payload.resourceSpans[0].scopeSpans[0].spans;
49+
assert.ok(
50+
spans.some((s) => s.kind === 2),
51+
'Expected a server span',
52+
);
53+
assert.ok(
54+
spans.some((s) => s.kind === 3),
55+
'Expected client spans',
56+
);
57+
58+
const assertDefined = (key, where) =>
59+
assert.ok(
60+
registry.has(key),
61+
`${where} attribute "${key}" is not defined by the ` +
62+
'semantic conventions',
63+
);
64+
65+
for (const span of spans) {
66+
for (const attr of span.attributes ?? []) {
67+
assertDefined(attr.key, 'span');
68+
}
69+
for (const event of span.events ?? []) {
70+
for (const attr of event.attributes ?? []) {
71+
assertDefined(attr.key, 'event');
72+
}
73+
}
74+
}
75+
76+
for (const attr of payload.resourceSpans[0].resource.attributes) {
77+
assertDefined(attr.key, 'resource');
78+
}
79+
});
80+
});

‎test/test-undici.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ describe('otel undici spans', () => {
6464

6565
assert.partialDeepStrictEqual(spanAttrs(clientSpan), {
6666
'http.request.method': 'GET',
67-
'server.address': `http://127.0.0.1:${target.port}`,
67+
'server.address': '127.0.0.1',
6868
'server.port': String(target.port),
6969
'url.full': `http://127.0.0.1:${target.port}/fetch-test`,
7070
'http.response.status_code': '200',

0 commit comments

Comments
 (0)