From 53d58964fe51fa7fae30cc654bf7c6413074259c Mon Sep 17 00:00:00 2001 From: Bryan English Date: Tue, 3 Nov 2026 13:52:02 -0500 Subject: [PATCH] feat: set the remaining semconv server span attributes Previously, the only address attribute HTTP/1 and HTTP/2 server spans set was server.address, from the Host header or the :authority pseudo-header. Now they also set server.port, client.address, network.peer.address, network.peer.port, and user_agent.original, per the HTTP semantic conventions. server.address and server.port come from the first of the Forwarded header's host parameter, the first X-Forwarded-Host entry, and the Host header or :authority, each parsed for both the host and the port, including bracketed IPv6. When the chosen value omits the port, the default comes from the Forwarded proto parameter, the first X-Forwarded-Proto entry, or, for the authority, the request scheme, in any letter case. client.address comes from the first Forwarded for parameter or the first X-Forwarded-For entry, and otherwise from the immediate peer. The network.peer attributes come from the socket. The proxy headers are followed the way OpenTelemetry's own HTTP instrumentation follows them, although a client can spoof them. The package has no runtime dependencies, so the Forwarded header is parsed by a small RFC 7239 parser in lib/forwarded.js. A port that is not a decimal number from 0 to 65535 is now omitted on client spans too, rather than exported as NaN, Infinity, or a rounded number. Signed-off-by: Bryan English --- README.md | 36 +++++++- lib/forwarded.js | 168 +++++++++++++++++++++++++++++++++++++ lib/instrumentations.js | 176 +++++++++++++++++++++++++++++++++++---- lib/span.js | 4 + test/test-forwarded.js | 60 +++++++++++++ test/test-http-client.js | 49 ++++++----- test/test-http-server.js | 176 +++++++++++++++++++++++++++++++++++++++ test/test-http2.js | 92 ++++++++++++++++++++ 8 files changed, 717 insertions(+), 44 deletions(-) create mode 100644 lib/forwarded.js create mode 100644 test/test-forwarded.js diff --git a/README.md b/README.md index f7457b9..c1deec4 100644 --- a/README.md +++ b/README.md @@ -89,11 +89,30 @@ 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` | Requested host, without the port | When `Host` header is present | +| `server.address` | Requested host, without the port | When a host source is present | +| `server.port` | Requested port, or its default | When the port is known | +| `client.address` | Client address, behind proxies when known, else the immediate peer | When the client or peer is known | +| `network.peer.address` | Immediate peer address | When the socket exposes it | +| `network.peer.port` | Immediate peer port | When the peer address is set | | `network.protocol.version` | HTTP version (e.g. `1.1`) | Always | +| `user_agent.original` | Client `User-Agent` header | When the header is present | | `http.response.status_code` | Response status code | When response finishes | | `error.type` | HTTP status code as string | On 5xx responses | +The address attributes follow the proxy headers the way OpenTelemetry's +own HTTP instrumentation does, although a client can spoof them. +`server.address` and `server.port` come from the first of the `Forwarded` +header's `host` parameter, the first `X-Forwarded-Host` entry, and the +`Host` header, each parsed for both the host and the port, including +bracketed IPv6. When the chosen value omits the port, the default comes +from the `Forwarded` `proto` parameter, the first `X-Forwarded-Proto` +entry, or, for the `Host` header, the request scheme, in any letter case. +A port that is not a decimal number from 0 to 65535 is omitted. `client.address` comes from the first `Forwarded` `for` +parameter or the first `X-Forwarded-For` entry, and otherwise from the +immediate peer. `network.peer.address` and `network.peer.port` come +from the socket, so behind a proxy they name the proxy. A malformed +`Forwarded` header is treated as absent. + ### HTTP/2 server A span with kind `SERVER` is created for each incoming HTTP/2 request @@ -115,11 +134,20 @@ Attributes set on HTTP/2 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` | Requested host, without the port | When `:authority` is present | +| `server.address` | Requested host, without the port | When a host source is present | +| `server.port` | Requested port, or its default | When the port is known | +| `client.address` | Client address, behind proxies when known, else the immediate peer | When the client or peer is known | +| `network.peer.address` | Immediate peer address | When the socket exposes it | +| `network.peer.port` | Immediate peer port | When the peer address is set | | `network.protocol.version` | `2.0` | Always | +| `user_agent.original` | Client `User-Agent` header | When the header is present | | `http.response.status_code` | Response status code | When the response is sent | | `error.type` | HTTP status code as string | On 5xx responses | +The address attributes follow the same rules as for HTTP/1 server +spans. The `:authority` pseudo-header stands in for the `Host` header, +and the peer attributes come from the session's socket. + ### HTTP client A span with kind `CLIENT` is created for each outgoing HTTP request made via @@ -143,8 +171,8 @@ Attributes set on client spans: | `error.type` | Status code or error name | On 4xx/5xx or errors | The `server.port` value is the decimal port from the Host header, or the scheme -default (`80` for `http:`, `443` for `https:`) when the header omits it; a -port that is not a decimal number is omitted. +default (`80` for `http:`, `443` for `https:`) when the header omits it. A +port that is not a decimal number from 0 to 65535 is omitted. ### Undici/Fetch client diff --git a/lib/forwarded.js b/lib/forwarded.js new file mode 100644 index 0000000..e070836 --- /dev/null +++ b/lib/forwarded.js @@ -0,0 +1,168 @@ +'use strict'; + +// This parses the Forwarded header (RFC 7239). The header is a +// comma-separated list of elements, each a semicolon-separated list of +// name=value parameters, where a value is a token or a quoted string with +// backslash escapes. Parameter names are case-insensitive, so they are +// lowercased, and values keep their case. A malformed header yields null, +// so a caller treats garbage as absent rather than guessing at it. + +// RFC 7230 section 3.2.6 token characters, the alphabet parameter names +// and unquoted values are built from. +function isTokenChar(code) { + return ( + code === 0x21 || + (code >= 0x23 && code <= 0x27) || + code === 0x2a || + code === 0x2b || + code === 0x2d || + code === 0x2e || + (code >= 0x30 && code <= 0x39) || + (code >= 0x41 && code <= 0x5a) || + (code >= 0x5e && code <= 0x7a) || + code === 0x7c || + code === 0x7e + ); +} + +// These are the delimiters of the same section. An unquoted value ends at +// them, and a quoted one may contain them. +function isDelimiter(code) { + return ( + code === 0x22 || + code === 0x28 || + code === 0x29 || + code === 0x2c || + code === 0x2f || + (code >= 0x3a && code <= 0x40) || + (code >= 0x5b && code <= 0x5d) || + code === 0x7b || + code === 0x7d + ); +} + +function isPrint(code) { + return code >= 0x20 && code <= 0x7e; +} + +function isExtended(code) { + return code >= 0x80 && code <= 0xff; +} + +// Only a quoted value can carry escapes, and only printable ones, so the +// backslashes are dropped once the value is complete. +function unescapeValue(value) { + return value.replace(/\\(.)/g, '$1'); +} + +function parseForwarded(header) { + const elements = []; + let element = {}; + let parameter; + let start = -1; + let end = -1; + let inQuotes = false; + let escaping = false; + let unescape = false; + let code; + + for (let i = 0; i < header.length; i++) { + code = header.charCodeAt(i); + + if (parameter === undefined) { + // Whitespace between elements is not part of any parameter. + if (i !== 0 && start === -1 && (code === 0x20 || code === 0x09)) continue; + if (isTokenChar(code)) { + if (start === -1) start = i; + } else if (code === 0x3d && start !== -1) { + parameter = header.slice(start, i).toLowerCase(); + start = -1; + } else { + return null; + } + continue; + } + + if (escaping && (code === 0x09 || isPrint(code) || isExtended(code))) { + escaping = false; + } else if (isTokenChar(code)) { + // A token cannot resume after whitespace ended the value. + if (end !== -1) return null; + if (start === -1) start = i; + } else if (isDelimiter(code) || isExtended(code)) { + if (inQuotes) { + if (code === 0x22) { + inQuotes = false; + end = i; + } else if (code === 0x5c) { + if (start === -1) start = i; + escaping = unescape = true; + } else if (start === -1) { + start = i; + } + } else if (code === 0x22 && header.charCodeAt(i - 1) === 0x3d) { + // A quote opens a value only directly after the equals sign. + inQuotes = true; + } else if ( + (code === 0x2c || code === 0x3b) && + (start !== -1 || end !== -1) + ) { + if (start !== -1) { + if (end === -1) end = i; + element[parameter] = unescape + ? unescapeValue(header.slice(start, end)) + : header.slice(start, end); + } else { + // A closed quoted pair can be empty, but an unquoted value cannot. + element[parameter] = ''; + } + if (code === 0x2c) { + elements.push(element); + element = {}; + } + parameter = undefined; + start = end = -1; + } else { + return null; + } + } else if (code === 0x20 || code === 0x09) { + if (end !== -1) continue; + if (inQuotes) { + if (start === -1) start = i; + } else if (start !== -1) { + // Trailing whitespace is not part of an unquoted value. + end = i; + } else { + return null; + } + } else { + return null; + } + } + + // The header must end inside a complete pair, not right after a + // separator, inside quotes, or on trailing whitespace. + if ( + parameter === undefined || + inQuotes || + (start === -1 && end === -1) || + code === 0x20 || + code === 0x09 + ) + return null; + + if (start !== -1) { + if (end === -1) end = header.length; + element[parameter] = unescape + ? unescapeValue(header.slice(start, end)) + : header.slice(start, end); + } else { + element[parameter] = ''; + } + elements.push(element); + return elements; +} + +module.exports = { + parseForwarded, +}; diff --git a/lib/instrumentations.js b/lib/instrumentations.js index 5cb80ff..e247cc5 100644 --- a/lib/instrumentations.js +++ b/lib/instrumentations.js @@ -9,6 +9,7 @@ const { STATUS_ERROR, kSpan, } = require('./span'); +const { parseForwarded } = require('./forwarded'); const { isModuleEnabled, getSpanStorage, @@ -42,6 +43,138 @@ function failClientSpan(span, error) { span.end(); } +const kDefaultPorts = { __proto__: null, http: 80, https: 443 }; + +// A port is recorded only when it is a decimal number in the TCP range. +// Anything else came from a malformed header, and as a number it could +// export as NaN, Infinity, or a rounded value. +function parsePort(port) { + if (!/^\d+$/.test(port)) return undefined; + const number = Number(port); + return number <= 65535 ? number : undefined; +} + +// Proxies append to the X-Forwarded headers, and Node joins repeated ones +// with commas, so the first entry is the original. +function firstEntry(value) { + const comma = value.indexOf(','); + return (comma === -1 ? value : value.slice(0, comma)).trim(); +} + +// Strips the port from a Forwarded for parameter or an X-Forwarded-For +// entry, keeping IPv6 without its brackets, the way the official +// instrumentation does. +function removeAddressPort(value) { + try { + const { hostname } = new URL(`http://${value}`); + if (hostname.startsWith('[') && hostname.endsWith(']')) + return hostname.slice(1, -1); + return hostname; + } catch { + return value; + } +} + +// Server span address attributes shared by the HTTP and HTTP/2 +// instrumentations, per the HTTP semantic conventions. The proxy headers +// are client controlled, but they are followed anyway: a reverse proxy +// rewrites them, and ignoring them would report the proxy as both the +// server and the client. Node joins repeated header fields with commas, +// which the Forwarded element grammar and the X-Forwarded-For split below +// already cover. +function applyServerRequestAttributes( + span, + headers, + authority, + scheme, + socket, +) { + const peerAddress = socket?.remoteAddress; + + const forwarded = headers.forwarded; + let elements; + if (forwarded) { + const parsed = parseForwarded(forwarded); + if (parsed) elements = parsed; + } + + // server.address and server.port come from the first of the Forwarded + // host parameter, X-Forwarded-Host, and the authority. The proto fills + // the default port when the chosen value omits one: the Forwarded proto + // parameter or X-Forwarded-Proto for the proxy headers, the request + // scheme for the authority. + let host; + let proto; + if (elements) { + for (const element of elements) { + if (element.host) { + host = element.host; + proto = element.proto; + break; + } + } + } + + const xForwardedHost = headers['x-forwarded-host']; + if (host === undefined && xForwardedHost) { + const first = firstEntry(xForwardedHost); + if (first) { + host = first; + const xForwardedProto = headers['x-forwarded-proto']; + if (xForwardedProto) proto = firstEntry(xForwardedProto); + } + } + + if (host === undefined && authority) { + host = authority; + proto = scheme; + } + + if (host !== undefined) { + const [address, port] = splitAuthority(host); + span.setAttribute('server.address', address); + // A missing port means the proto's default. URI schemes are + // case-insensitive, so the proto is compared in lowercase. + if (port == null) { + const defaultPort = kDefaultPorts[proto?.toLowerCase()]; + if (defaultPort !== undefined) + span.setAttribute('server.port', defaultPort); + } else { + const number = parsePort(port); + if (number !== undefined) span.setAttribute('server.port', number); + } + } + + // client.address is the original client behind the proxies, from the + // first Forwarded for parameter or the first X-Forwarded-For entry, and + // otherwise the immediate peer. + let clientAddress; + if (elements) { + for (const element of elements) { + if (element.for) { + clientAddress = removeAddressPort(element.for); + break; + } + } + } + if (clientAddress === undefined) { + const xForwardedFor = headers['x-forwarded-for']; + const first = xForwardedFor && firstEntry(xForwardedFor); + if (first) clientAddress = removeAddressPort(first); + } + if (clientAddress === undefined) clientAddress = peerAddress; + if (clientAddress != null) span.setAttribute('client.address', clientAddress); + + if (peerAddress != null) { + span.setAttribute('network.peer.address', peerAddress); + const peerPort = socket.remotePort; + if (peerPort != null) span.setAttribute('network.peer.port', peerPort); + } + + const userAgent = headers['user-agent']; + if (userAgent) span.setAttribute('user_agent.original', userAgent); +} + function onHttpServerRequestStart({ request, response, socket }) { // Skip the export request itself: tracing an in-process collector would // feed itself indefinitely. Both the authority and the resolved export @@ -85,11 +218,17 @@ function onHttpServerRequestStart({ request, response, socket }) { span.setAttribute('url.path', path); if (qIdx !== -1) span.setAttribute('url.query', rawUrl.slice(qIdx + 1)); - span.setAttribute('url.scheme', socket?.encrypted ? 'https' : 'http'); + const scheme = socket?.encrypted ? 'https' : 'http'; + span.setAttribute('url.scheme', scheme); span.setAttribute('network.protocol.version', request.httpVersion || '1.1'); - const host = request.headers.host; - if (host) span.setAttribute('server.address', splitAuthority(host)[0]); + applyServerRequestAttributes( + span, + request.headers, + request.headers.host, + scheme, + socket, + ); // The span is stashed on the response so the response handlers do not // need a second AsyncLocalStorage lookup. @@ -133,8 +272,9 @@ function onHttpServerResponseFinish({ response }) { span.end(); } -// Splits an authority (a Host header value) into host and port. The port -// is null when the authority omits it, meaning the scheme default. +// Splits an authority (a Host or :authority value) into host and port. +// The port is null when the authority omits it, meaning the scheme +// default. function splitAuthority(authority) { if (authority.startsWith('[')) { // IPv6 literal: the port, if any, follows the closing bracket. @@ -177,13 +317,12 @@ function onHttpClientRequestCreated({ request }) { const [hostPart, portPart] = splitAuthority(authority); span.setAttribute('server.address', host || hostPart); - // A decimal port from the authority is recorded, a missing one means the - // scheme default, and anything else is omitted: NaN would encode as an - // invalid OTLP value. + // A missing port means the scheme's default. if (portPart == null) { span.setAttribute('server.port', protocol === 'https:' ? 443 : 80); - } else if (/^\d+$/.test(portPart)) { - span.setAttribute('server.port', Number(portPart)); + } else { + const port = parsePort(portPart); + if (port !== undefined) span.setAttribute('server.port', port); } span.setAttribute('url.full', `${protocol}//${authority}${path}`); @@ -273,15 +412,18 @@ function onHttp2ServerStreamStart({ stream, headers }) { // The :scheme pseudo-header carries the request scheme; requests that // omit it fall back to the session's encryption state, like the // socket check for HTTP/1.1. - span.setAttribute( - 'url.scheme', - headers?.[':scheme'] || (stream?.session?.encrypted ? 'https' : 'http'), - ); + const scheme = + headers?.[':scheme'] || (stream?.session?.encrypted ? 'https' : 'http'); + span.setAttribute('url.scheme', scheme); span.setAttribute('network.protocol.version', '2.0'); - const authority = headers?.[':authority']; - if (authority) - span.setAttribute('server.address', splitAuthority(authority)[0]); + applyServerRequestAttributes( + span, + headers, + headers?.[':authority'], + scheme, + stream.session.socket, + ); // The span is stashed on the stream so the finish and close handlers do // not need a second AsyncLocalStorage lookup. diff --git a/lib/span.js b/lib/span.js index fdf76c8..0bfd77a 100644 --- a/lib/span.js +++ b/lib/span.js @@ -44,9 +44,12 @@ const kTraceFlagsHex = Array.from({ length: 256 }, (_, i) => // properties and no prototype of its own, so the object inherits nothing, // and a key named __proto__ is stored as an ordinary own property. class SpanAttributes { + 'client.address' = undefined; 'error.type' = undefined; 'http.request.method' = undefined; 'http.response.status_code' = undefined; + 'network.peer.address' = undefined; + 'network.peer.port' = undefined; 'network.protocol.version' = undefined; 'server.address' = undefined; 'server.port' = undefined; @@ -54,6 +57,7 @@ class SpanAttributes { 'url.path' = undefined; 'url.query' = undefined; 'url.scheme' = undefined; + 'user_agent.original' = undefined; } delete SpanAttributes.prototype.constructor; Object.setPrototypeOf(SpanAttributes.prototype, null); diff --git a/test/test-forwarded.js b/test/test-forwarded.js new file mode 100644 index 0000000..2a6257d --- /dev/null +++ b/test/test-forwarded.js @@ -0,0 +1,60 @@ +'use strict'; + +const assert = require('node:assert'); +const { describe, it } = require('node:test'); +const { parseForwarded } = require('../lib/forwarded'); + +describe('Forwarded header parser', () => { + // Each case is a header value and the elements it must yield, or null + // when the value is malformed. + const cases = [ + ['for=192.0.2.60', [{ for: '192.0.2.60' }]], + // An element carries any number of parameters in any order. + [ + 'for=192.0.2.60;proto=http;by=203.0.113.43', + [{ for: '192.0.2.60', proto: 'http', by: '203.0.113.43' }], + ], + [ + 'for=192.0.2.60,for=198.51.100.17', + [{ for: '192.0.2.60' }, { for: '198.51.100.17' }], + ], + // Parameter names are case-insensitive, values keep their case. + [ + 'FOR=192.0.2.60;Host=Example.COM', + [{ for: '192.0.2.60', host: 'Example.COM' }], + ], + // Values with delimiters in them, IPv6 among them, must be quoted. + ['for="[2001:db8::1]:4711"', [{ for: '[2001:db8::1]:4711' }]], + [ + 'host="example.com:8080";proto=http', + [{ host: 'example.com:8080', proto: 'http' }], + ], + ['host="[2001:db8::1]:8080"', [{ host: '[2001:db8::1]:8080' }]], + // A quoted value keeps its spaces, may be empty, and may escape a + // character by dropping its backslash. + ['for=" spaced "', [{ for: ' spaced ' }]], + ['host=""', [{ host: '' }]], + ['for="say \\"hi\\""', [{ for: 'say "hi"' }]], + ['for="a;b,c"', [{ for: 'a;b,c' }]], + // An unquoted value ends at whitespace, which is not part of it. + ['for=1.2.3.4 ; host=a', [{ for: '1.2.3.4', host: 'a' }]], + // Malformed values yield null: a missing equals sign or value, an + // unterminated quote, a trailing separator, a token after a closed + // value, or a delimiter an unquoted value cannot carry, like the + // colon of a port. + ['for', null], + ['for=', null], + ['for="unterminated', null], + ['for=1.2.3.4,', null], + ['for=1.2.3.4;host', null], + ['for=1.2.3.4 host=a', null], + ['host=example.com:8080', null], + ['', null], + ]; + + for (const [header, expected] of cases) { + it(`parses ${JSON.stringify(header)}`, () => { + assert.deepStrictEqual(parseForwarded(header), expected); + }); + } +}); diff --git a/test/test-http-client.js b/test/test-http-client.js index dfc4485..19bd949 100644 --- a/test/test-http-client.js +++ b/test/test-http-client.js @@ -185,38 +185,41 @@ describe('otel HTTP client spans', () => { }); } - it('omits server.port when the Host port is not decimal', async () => { + it('omits server.port when the Host port is not a valid port', async () => { const target = await startHTTPServer((req, res) => { res.writeHead(200); res.end('ok'); }); - await new Promise((resolve, reject) => { - const req = http.get( - { - host: '127.0.0.1', - port: target.port, - path: '/odd-host', - headers: { Host: 'example.test:not-a-port' }, - }, - (res) => { - res.resume(); - res.on('end', resolve); - }, - ); - req.on('error', reject); - }); + // A port that is not decimal or is outside the TCP range must be + // omitted, not encoded as NaN, Infinity, or a rounded number. + const ports = ['not-a-port', '65536', '9'.repeat(400)]; + for (const [i, port] of ports.entries()) { + await new Promise((resolve, reject) => { + const req = http.get( + { + host: '127.0.0.1', + port: target.port, + path: `/odd-host-${i}`, + headers: { Host: `example.test:${port}` }, + }, + (res) => { + res.resume(); + res.on('end', resolve); + }, + ); + req.on('error', reject); + }); + } const spans = await flushAndCollect(collector); await target.close(); - const clientSpan = getClientSpan(spans, '/odd-host'); - assert.ok(clientSpan, 'Expected a client span'); - assert.strictEqual( - spanAttrs(clientSpan)['server.port'], - undefined, - 'a non-decimal port must be omitted, not encoded', - ); + for (const [i, port] of ports.entries()) { + const clientSpan = getClientSpan(spans, `/odd-host-${i}`); + assert.ok(clientSpan, `Expected a client span for ${port}`); + assert.strictEqual(spanAttrs(clientSpan)['server.port'], undefined, port); + } }); it('parses IPv6 authorities and default ports from the Host header', async () => { diff --git a/test/test-http-server.js b/test/test-http-server.js index 69e2661..30186a1 100644 --- a/test/test-http-server.js +++ b/test/test-http-server.js @@ -9,6 +9,7 @@ const { flushAndCollect, getServerSpan, httpGet, + rawRequest, spanAttrs, } = require('./common/otel'); @@ -54,6 +55,181 @@ describe('otel HTTP server spans', () => { }); }); + it('sets the peer, client, and user agent attributes without proxy headers', async () => { + const app = await startHTTPServer(); + + await rawRequest(app.port, '/direct', { + 'User-Agent': 'microtel-test/1', + }); + + const spans = await flushAndCollect(collector); + + await app.close(); + + const serverSpan = getServerSpan(spans, '/direct'); + assert.ok(serverSpan, 'Expected a server span'); + + // With no proxy headers the client is the immediate peer, and the + // Host header carries the port. + assert.partialDeepStrictEqual(spanAttrs(serverSpan), { + 'client.address': '127.0.0.1', + 'network.peer.address': '127.0.0.1', + 'server.address': '127.0.0.1', + 'server.port': String(app.port), + 'user_agent.original': 'microtel-test/1', + }); + + // network.peer.port is an int, not a string. + const peerPort = serverSpan.attributes.find( + (a) => a.key === 'network.peer.port', + ); + assert.match(peerPort.value.intValue, /^\d+$/); + }); + + it('reads the Forwarded header', async () => { + const app = await startHTTPServer(); + + // Each request exercises one shape of the header: quoted values + // carry IPv6 and ports, the first element with a for or host + // parameter wins, and the proto parameter fills the default port. + const cases = [ + [ + '/forwarded', + { Forwarded: 'for="[2001:db8::1]:4711";host="example.com:8080"' }, + { + 'client.address': '2001:db8::1', + 'server.address': 'example.com', + 'server.port': '8080', + }, + ], + [ + '/forwarded-list', + { + Forwarded: 'for=192.0.2.60, for=198.51.100.17;host=example.org', + }, + { + 'client.address': '192.0.2.60', + 'server.address': 'example.org', + }, + ], + [ + '/forwarded-proto', + { Forwarded: 'for=192.0.2.60;host=example.com;proto=https' }, + { + 'client.address': '192.0.2.60', + 'server.address': 'example.com', + 'server.port': '443', + }, + ], + // A port requires quotes, so an unquoted host makes the whole + // header malformed and the headers below it apply. + [ + '/forwarded-invalid', + { + Forwarded: 'host=example.com:8080', + 'X-Forwarded-Host': 'fallback.example', + }, + { 'server.address': 'fallback.example' }, + ], + ]; + + for (const [path, headers] of cases) + await rawRequest(app.port, path, headers); + + const spans = await flushAndCollect(collector); + + await app.close(); + + for (const [path, , expected] of cases) { + const serverSpan = getServerSpan(spans, path); + assert.ok(serverSpan, `Expected a server span for ${path}`); + assert.partialDeepStrictEqual(spanAttrs(serverSpan), expected); + } + + // The host parameter carries no port and no proto applies, so + // server.port is not set. + const listAttrs = spanAttrs(getServerSpan(spans, '/forwarded-list')); + assert.strictEqual(listAttrs['server.port'], undefined); + }); + + it('reads X-Forwarded-For and X-Forwarded-Host', async () => { + const app = await startHTTPServer(); + + await rawRequest(app.port, '/x-forwarded', { + 'X-Forwarded-For': '[2001:db8::1]:4711, 198.51.100.17', + 'X-Forwarded-Host': 'proxy.example.com:9090', + }); + // X-Forwarded-Proto fills the default port when the host has none. + await rawRequest(app.port, '/x-forwarded-proto', { + 'X-Forwarded-Host': 'proxy.example.com', + 'X-Forwarded-Proto': 'https', + }); + await rawRequest(app.port, '/x-forwarded-host', { + 'X-Forwarded-Host': 'proxy.example.com', + }); + // Proxies append to these headers, so the first entry is the original, + // and a scheme matches in any letter case. + await rawRequest(app.port, '/x-forwarded-lists', { + 'X-Forwarded-Host': 'public.example, proxy.example', + 'X-Forwarded-Proto': 'HTTPS, http', + 'X-Forwarded-For': ', 198.51.100.17', + }); + await rawRequest(app.port, '/forwarded-proto-case', { + Forwarded: 'host=public.example;proto=HTTPS', + }); + // A port outside the TCP range came from a broken header. + for (const port of ['65536', '9007199254740993', '9'.repeat(400)]) + await rawRequest(app.port, `/bad-port-${port.length}`, { + 'X-Forwarded-Host': `example.com:${port}`, + }); + + const spans = await flushAndCollect(collector); + + await app.close(); + + const forwarded = spanAttrs(getServerSpan(spans, '/x-forwarded')); + assert.ok(forwarded, 'Expected a server span for /x-forwarded'); + assert.partialDeepStrictEqual(forwarded, { + 'client.address': '2001:db8::1', + 'server.address': 'proxy.example.com', + 'server.port': '9090', + }); + + const proto = spanAttrs(getServerSpan(spans, '/x-forwarded-proto')); + assert.ok(proto, 'Expected a server span for /x-forwarded-proto'); + assert.partialDeepStrictEqual(proto, { + 'server.address': 'proxy.example.com', + 'server.port': '443', + }); + + // Without X-Forwarded-Proto there is no default port, not even from + // the request scheme. + const host = spanAttrs(getServerSpan(spans, '/x-forwarded-host')); + assert.ok(host, 'Expected a server span for /x-forwarded-host'); + assert.strictEqual(host['server.address'], 'proxy.example.com'); + assert.strictEqual(host['server.port'], undefined); + + // An empty first X-Forwarded-For entry is absent, so client.address + // falls back to the peer. + assert.partialDeepStrictEqual( + spanAttrs(getServerSpan(spans, '/x-forwarded-lists')), + { + 'server.address': 'public.example', + 'server.port': '443', + 'client.address': '127.0.0.1', + }, + ); + assert.partialDeepStrictEqual( + spanAttrs(getServerSpan(spans, '/forwarded-proto-case')), + { 'server.address': 'public.example', 'server.port': '443' }, + ); + for (const length of [5, 16, 400]) { + const attrs = spanAttrs(getServerSpan(spans, `/bad-port-${length}`)); + assert.strictEqual(attrs['server.address'], 'example.com', `${length}`); + assert.strictEqual(attrs['server.port'], undefined, `${length}`); + } + }); + it('does not end the span while a delayed response is pending', async () => { // A request with a body completes its input stream well before the // response is sent; the span must wait for the response. diff --git a/test/test-http2.js b/test/test-http2.js index 1b07529..732a0ba 100644 --- a/test/test-http2.js +++ b/test/test-http2.js @@ -113,6 +113,98 @@ describe('otel HTTP/2 server spans', () => { }); }); + it('sets the peer, client, and user agent attributes without proxy headers', async () => { + const target = await startHTTP2Server(); + + await h2Get(target.port, '/direct', { 'user-agent': 'microtel-test/1' }); + + const spans = await flushAndCollect(collector); + + await target.close(); + + const serverSpan = getServerSpan(spans, '/direct'); + assert.ok(serverSpan, 'Expected a server span'); + + // With no proxy headers the client is the immediate peer, and + // :authority carries the port. A portless :authority falls back to + // the scheme default, like the Host header on HTTP/1.1. + assert.partialDeepStrictEqual(spanAttrs(serverSpan), { + 'client.address': '127.0.0.1', + 'network.peer.address': '127.0.0.1', + 'server.address': '127.0.0.1', + 'server.port': String(target.port), + 'user_agent.original': 'microtel-test/1', + }); + + const peerPort = serverSpan.attributes.find( + (a) => a.key === 'network.peer.port', + ); + assert.match(peerPort.value.intValue, /^\d+$/); + }); + + it('derives the server port from :authority', async () => { + const target = await startHTTP2Server(); + + await h2Get(target.port, '/authority-port', { + ':authority': 'example.com:8080', + }); + await h2Get(target.port, '/authority-default', { + ':authority': 'example.com', + }); + + const spans = await flushAndCollect(collector); + + await target.close(); + + const port = spanAttrs(getServerSpan(spans, '/authority-port')); + assert.ok(port, 'Expected a server span for /authority-port'); + assert.partialDeepStrictEqual(port, { + 'server.address': 'example.com', + 'server.port': '8080', + }); + + // The portless authority yields the scheme default. + const def = spanAttrs(getServerSpan(spans, '/authority-default')); + assert.ok(def, 'Expected a server span for /authority-default'); + assert.partialDeepStrictEqual(def, { + 'server.address': 'example.com', + 'server.port': '80', + }); + }); + + it('reads the proxy headers', async () => { + const target = await startHTTP2Server(); + + await h2Get(target.port, '/forwarded', { + forwarded: 'for="[2001:db8::1]:4711";host="example.com:8080";proto=http', + }); + await h2Get(target.port, '/x-forwarded', { + 'x-forwarded-for': '192.0.2.60, 198.51.100.17', + 'x-forwarded-host': 'proxy.example.com', + 'x-forwarded-proto': 'https', + }); + + const spans = await flushAndCollect(collector); + + await target.close(); + + const forwarded = spanAttrs(getServerSpan(spans, '/forwarded')); + assert.ok(forwarded, 'Expected a server span for /forwarded'); + assert.partialDeepStrictEqual(forwarded, { + 'client.address': '2001:db8::1', + 'server.address': 'example.com', + 'server.port': '8080', + }); + + const xForwarded = spanAttrs(getServerSpan(spans, '/x-forwarded')); + assert.ok(xForwarded, 'Expected a server span for /x-forwarded'); + assert.partialDeepStrictEqual(xForwarded, { + 'client.address': '192.0.2.60', + 'server.address': 'proxy.example.com', + 'server.port': '443', + }); + }); + it('reads the status code when respond() takes an array of headers', async () => { const target = await startHTTP2Server((stream, headers) => { const status = headers[':path'] === '/boom' ? 500 : 404;