Skip to content

Commit 9db2dd5

Browse files
authored
feat: add undici and fetch instrumentation (#5)
Signed-off-by: Bryan English <bryan@bryanenglish.com>
1 parent 3150eec commit 9db2dd5

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)