Skip to content

Commit 23e1d14

Browse files
committed
feat: add undici and fetch instrumentation
One CLIENT span per fetch() or request() call, with the same shape as the node:http client spans: http.request.method and url.full (origin + path) attributes, server.address set to the origin, server.port derived from it, and the span ended on trailers, which fire once the response body completes. Trace context injection replaces rather than appends headers, since undici's addHeader appends, and a stale caller-supplied tracestate is removed when the span has none to inherit. The filter values node:undici and node:fetch enable it, as an alias pair. Signed-off-by: Bryan English <bryan@bryanenglish.com>
1 parent 3150eec commit 23e1d14

4 files changed

Lines changed: 299 additions & 25 deletions

File tree

‎README.md‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ started. Invalid options throw.
4747
| `serviceName` | string | `unknown_service:node` | Service name in exported resource attributes. The default follows the OpenTelemetry [semantic conventions](https://opentelemetry.io/docs/specs/semconv/general/trace/) for low-cardinality service names. |
4848
| `sdkName` | string | `microtel` | `telemetry.sdk.name` resource attribute, and the name of the instrumentation scope on exported spans. |
4949
| `sdkVersion` | string | this package's version | `telemetry.sdk.version` resource attribute, and the scope version. |
50-
| `filter` | string | all modules | Comma-separated list of modules to instrument. Supported values: `node:http`, `node:http/client`, `node:http/server`. |
50+
| `filter` | string | all modules | Comma-separated list of modules to instrument. Supported values: `node:http`, `node:http/client`, `node:http/server`, and `node:undici`. `node:fetch` is an alias for `node:undici`: both cover all undici requests, including `fetch()`, and either one enables them. |
5151
| `exclude` | string | none | Comma-separated list of modules to exclude from instrumentation. Same values as `filter`; takes precedence over it. |
5252
| `maxBufferSize` | number | `100` | Number of spans buffered before an immediate flush is triggered. Must be a positive integer. |
5353
| `flushInterval` | number | `10000` | Milliseconds between periodic flushes. Must be a positive integer. |
@@ -121,11 +121,14 @@ port that is not a decimal number is omitted.
121121

122122
### Undici/Fetch client
123123

124-
Not implemented yet. The instrumentation subscribes to the
125-
`undici:request:*` diagnostics channels and produces one CLIENT span per
126-
request, with the same attributes as the HTTP client spans plus
127-
`server.port`; the plan is documented as a TODO in
128-
`lib/instrumentations.js`.
124+
A span with kind `CLIENT` is created for each outgoing request made via
125+
`fetch()` or undici's `request()`. Error status, `exception` event behavior,
126+
and span end timing are the same as for the HTTP client spans above. Requests
127+
that fail before they are dispatched, such as a refused connection, produce
128+
no span: the undici channels only fire once the request is underway. The
129+
injected `traceparent` replaces any caller-supplied value rather than
130+
appending to it, and a stale `tracestate` is removed when the span has none
131+
to inherit.
129132

130133
Attributes set on undici/fetch client spans:
131134

@@ -152,9 +155,8 @@ fresh trace is started instead. The `tracestate` header, when present, is
152155
attached to the span and forwarded verbatim to outgoing requests. It is never
153156
parsed or modified.
154157
* **Outgoing requests**: The `traceparent` header is injected into outgoing
155-
HTTP requests, together with the stored `tracestate` when one
156-
is present, enabling distributed tracing across services. Undici and
157-
fetch requests are covered once that instrumentation lands.
158+
HTTP and undici/fetch requests, together with the stored `tracestate` when
159+
one is present, enabling distributed tracing across services.
158160

159161
## Benchmarks
160162

‎lib/instrumentations.js‎

Lines changed: 76 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -206,22 +206,73 @@ function onClientRequestError({ request, error }) {
206206
failClientSpan(span, error);
207207
}
208208

209-
// TODO(bengl): undici/fetch instrumentation. Subscribing to the
210-
// undici:request:create, undici:request:headers, undici:request:trailers,
211-
// and undici:request:error channels yields one CLIENT span per fetch()
212-
// or request() call, with the same shape as the node:http client spans
213-
// above: http.request.method and url.full (origin + path) attributes,
214-
// server.address set to the origin, server.port derived from it, and the
215-
// span ended on trailers, which fire once the response body completes.
216-
// The headers channel applies the response status; the error channel
217-
// reuses failClientSpan. Trace context injection must replace rather than
218-
// append headers: undici's addHeader appends, so a caller-supplied
219-
// traceparent has to be updated in place or it arrives duplicated. The
220-
// filter values node:undici and node:fetch enable it, as an alias pair.
221-
// A working implementation, including its test, exists in
222-
// nodejs/node#66587 (lib/internal/otel/instrumentations.js and
223-
// test/parallel/test-otel-undici.js); the PR's diff stays viewable even
224-
// once it is closed.
209+
// Undici's addHeader appends rather than replaces; update an existing
210+
// header in place so a caller-supplied value is overridden, not
211+
// duplicated.
212+
function setUndiciHeader(request, name, value) {
213+
const headers = request.headers;
214+
for (let i = 0; i < headers.length; i += 2) {
215+
if (headers[i] === name) {
216+
headers[i + 1] = value;
217+
return;
218+
}
219+
}
220+
request.addHeader(name, value);
221+
}
222+
223+
function removeUndiciHeader(request, name) {
224+
const headers = request.headers;
225+
for (let i = headers.length - 2; i >= 0; i -= 2) {
226+
if (headers[i] === name) headers.splice(i, 2);
227+
}
228+
}
229+
230+
function onUndiciRequestCreate({ request }) {
231+
const parent = spanStorage.getStore();
232+
233+
const method = request.method || 'GET';
234+
const span = new Span(method, SPAN_KIND_CLIENT, { parent });
235+
236+
span.setAttribute('http.request.method', method);
237+
238+
const { origin } = request;
239+
const path = request.path;
240+
if (origin) {
241+
span.setAttribute('server.address', origin);
242+
// The origin omits the port when it is the scheme default.
243+
const parsed = new URL(origin);
244+
span.setAttribute(
245+
'server.port',
246+
Number(parsed.port) || (parsed.protocol === 'https:' ? 443 : 80),
247+
);
248+
}
249+
span.setAttribute('url.full', `${origin}${path}`);
250+
251+
request[kSpan] = span;
252+
253+
if (request.addHeader) {
254+
setUndiciHeader(request, 'traceparent', span.toTraceparent());
255+
if (span.tracestate)
256+
setUndiciHeader(request, 'tracestate', span.tracestate);
257+
else removeUndiciHeader(request, 'tracestate');
258+
}
259+
}
260+
261+
function onUndiciRequestHeaders({ request, response }) {
262+
const span = request[kSpan];
263+
if (span == null) return;
264+
265+
applyClientResponse(span, response.statusCode);
266+
}
267+
268+
// undici:request:trailers fires when the response completes, including its
269+
// body, so the span duration covers the full transfer.
270+
function onUndiciRequestTrailers({ request }) {
271+
const span = request[kSpan];
272+
if (span == null) return;
273+
274+
span.end();
275+
}
225276

226277
function enableInstrumentations() {
227278
spanStorage = getSpanStorage();
@@ -236,6 +287,15 @@ function enableInstrumentations() {
236287
dc.subscribe('http.client.response.finish', onHttpClientResponseFinish);
237288
dc.subscribe('http.client.request.error', onClientRequestError);
238289
}
290+
291+
// node:fetch is an alias for node:undici: both enable the same
292+
// channels, which cover every undici request, fetch() included.
293+
if (isModuleEnabled('node:undici') || isModuleEnabled('node:fetch')) {
294+
dc.subscribe('undici:request:create', onUndiciRequestCreate);
295+
dc.subscribe('undici:request:headers', onUndiciRequestHeaders);
296+
dc.subscribe('undici:request:trailers', onUndiciRequestTrailers);
297+
dc.subscribe('undici:request:error', onClientRequestError);
298+
}
239299
}
240300

241301
module.exports = {

‎test/test-init.js‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,30 @@ describe('otel init activation', () => {
187187
);
188188
});
189189

190+
it('filter accepts node:fetch as an alias for undici', async () => {
191+
const { code, stdout } = await spawnMicrotel([
192+
fixture('state-child.js'),
193+
JSON.stringify([
194+
{
195+
endpoint: 'http://127.0.0.1:1',
196+
filter: 'node:fetch',
197+
},
198+
]),
199+
'node:fetch,node:http',
200+
]);
201+
202+
assert.strictEqual(code, 0);
203+
const lines = stdout.trim().split('\n');
204+
assert.match(
205+
lines.find((l) => l.startsWith('node:fetch:')),
206+
/node:fetch:true/,
207+
);
208+
assert.match(
209+
lines.find((l) => l.startsWith('node:http:')),
210+
/node:http:false/,
211+
);
212+
});
213+
190214
it('exclude disables modules without touching the rest', async () => {
191215
const { code, stdout } = await spawnMicrotel([
192216
fixture('state-child.js'),

‎test/test-undici.js‎

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
'use strict';
2+
3+
const assert = require('node:assert');
4+
const { after, before, describe, it } = require('node:test');
5+
const {
6+
startTracing,
7+
startHTTPServer,
8+
flushAndCollect,
9+
getClientSpan,
10+
rawRequest,
11+
spanAttrs,
12+
closeAll,
13+
} = require('./common/otel');
14+
15+
describe('otel undici spans', () => {
16+
let collector;
17+
18+
before(async () => {
19+
collector = await startTracing({
20+
filter: 'node:http,node:undici',
21+
});
22+
});
23+
24+
after(() => collector.close());
25+
26+
it(
27+
'creates client spans for fetch requests and overrides existing ' +
28+
'trace context headers',
29+
async () => {
30+
const receivedTraceparent = {};
31+
const target = await startHTTPServer((req, res) => {
32+
receivedTraceparent[req.url] = req.headers.traceparent;
33+
res.writeHead(200);
34+
res.end('ok');
35+
});
36+
37+
try {
38+
const res = await fetch(`http://127.0.0.1:${target.port}/fetch-test`);
39+
await res.text();
40+
} catch {}
41+
42+
// A caller-supplied traceparent must be overridden, not duplicated:
43+
// undici's addHeader appends, so a duplicate would arrive as a single
44+
// comma-separated value.
45+
try {
46+
const res = await fetch(
47+
`http://127.0.0.1:${target.port}/override-test`,
48+
{
49+
headers: {
50+
traceparent:
51+
'00-00000000000000000000000000000000-' + '0000000000000000-00',
52+
},
53+
},
54+
);
55+
await res.text();
56+
} catch {}
57+
58+
const spans = await flushAndCollect(collector);
59+
await target.close();
60+
61+
assert.ok(spans.length > 0, 'Expected at least one span from fetch');
62+
const clientSpan = getClientSpan(spans, '/fetch-test');
63+
assert.ok(clientSpan, 'Expected a CLIENT span from fetch');
64+
65+
assert.partialDeepStrictEqual(spanAttrs(clientSpan), {
66+
'http.request.method': 'GET',
67+
'server.address': `http://127.0.0.1:${target.port}`,
68+
'server.port': String(target.port),
69+
'url.full': `http://127.0.0.1:${target.port}/fetch-test`,
70+
'http.response.status_code': '200',
71+
});
72+
73+
const overridden = receivedTraceparent['/override-test'];
74+
assert.ok(overridden, 'Expected a traceparent on the override request');
75+
assert.match(overridden, /^00-[0-9a-f]{32}-[0-9a-f]{16}-0[01]$/);
76+
assert.ok(
77+
!overridden.includes(','),
78+
'traceparent must be overridden, not duplicated',
79+
);
80+
},
81+
);
82+
83+
it('creates an error span for a failed fetch', async () => {
84+
// A response that dies mid-transfer must end the span with the error
85+
// status and an exception event, through the undici:request:error
86+
// channel. A fetch that fails to connect never dispatches, so it
87+
// produces no span at all.
88+
const target = await startHTTPServer((req, res) => {
89+
res.writeHead(200);
90+
res.write('partial');
91+
setTimeout(() => res.destroy(), 50);
92+
});
93+
94+
await fetch(`http://127.0.0.1:${target.port}/destroy`)
95+
.then((res) => res.text())
96+
.catch(() => {});
97+
98+
const spans = await flushAndCollect(collector);
99+
await target.close();
100+
101+
const errorSpan = getClientSpan(spans, '/destroy');
102+
assert.ok(errorSpan, 'Expected an error client span for the failed fetch');
103+
104+
assert.ok(errorSpan.status, 'Error span should have status');
105+
assert.strictEqual(errorSpan.status.code, 2); // STATUS_ERROR
106+
assert.ok(
107+
errorSpan.events?.some((e) => e.name === 'exception'),
108+
'Should have an exception event',
109+
);
110+
});
111+
112+
it('overrides a stale tracestate with the inherited one', async () => {
113+
const incomingTraceId = 'a'.repeat(32);
114+
const incomingTracestate = 'vendor=inherited';
115+
116+
let seen;
117+
// The fetch target records the trace context headers it receives.
118+
const fetchTarget = await startHTTPServer((req, res) => {
119+
seen = {
120+
traceparent: req.headers.traceparent,
121+
tracestate: req.headers.tracestate,
122+
};
123+
res.writeHead(200);
124+
res.end('ok');
125+
});
126+
127+
// A fetch made while handling a traced request inherits the incoming
128+
// tracestate and must override the caller-supplied stale one.
129+
const frontend = await startHTTPServer((req, res) => {
130+
fetch(`http://127.0.0.1:${fetchTarget.port}/down`, {
131+
headers: { tracestate: 'stale=caller-value' },
132+
}).then((fr) => {
133+
fr.text().then(() => {
134+
res.writeHead(200);
135+
res.end('ok');
136+
});
137+
});
138+
});
139+
140+
await rawRequest(frontend.port, '/up', {
141+
traceparent: `00-${incomingTraceId}-0123456789abcdef-01`,
142+
tracestate: incomingTracestate,
143+
});
144+
145+
await flushAndCollect(collector);
146+
await closeAll(fetchTarget, frontend);
147+
148+
assert.ok(seen?.traceparent, 'Expected a traceparent on the fetch');
149+
assert.ok(
150+
seen.traceparent.includes(incomingTraceId),
151+
'The fetch must carry the incoming trace ID',
152+
);
153+
assert.strictEqual(
154+
seen.tracestate,
155+
incomingTracestate,
156+
'The inherited tracestate must win over the stale one',
157+
);
158+
});
159+
160+
it('removes a stale tracestate when the span has none', async () => {
161+
let receivedTracestate = 'sentinel';
162+
const target = await startHTTPServer((req, res) => {
163+
receivedTracestate = req.headers.tracestate;
164+
res.writeHead(200);
165+
res.end('ok');
166+
});
167+
168+
try {
169+
const res = await fetch(`http://127.0.0.1:${target.port}/stale`, {
170+
headers: { tracestate: 'vendor=state-for-an-old-trace' },
171+
});
172+
await res.text();
173+
} catch {}
174+
175+
const spans = await flushAndCollect(collector);
176+
await target.close();
177+
178+
assert.ok(getClientSpan(spans, '/stale'), 'Expected a client span');
179+
180+
// A stale tracestate from an unrelated trace must not travel with
181+
// the fresh trace context, even though the span has none to inject.
182+
assert.strictEqual(
183+
receivedTracestate,
184+
undefined,
185+
'stale tracestate must not be forwarded',
186+
);
187+
});
188+
});

0 commit comments

Comments
 (0)