Undici

[stefee]

Is your instrumentation request related to a problem? Please describe

When making server-side requests using undici.request the trace is not propagated across service boundaries. This is because undici does not use core http module for making requests (unlike other popular fetch libraries).

Is it applicable for Node or Browser or both?

Node

Do you expect this instrumentation to be commonly used?

Weekly Downloads:

What version of instrumentation are you interested in using?

Versions:

Additional context

https://github.com/nodejs/undici

[stefee]

See unofficial instrumentation https://github.com/gadget-inc/opentelemetry-instrumentations/tree/main/packages/opentelemetry-instrumentation-undici

[stefee]

See discussion over on undici project: nodejs/undici#1439

[stefee]

Following from the thread linked, I’m looking into implementing instrumentation using the Undici diagnostics channel. https://github.com/nodejs/undici/blob/main/docs/api/DiagnosticsChannel.md

The tricky thing I can see with the JS API for OTEL is that the only way to update the current active context is using the “with” method which takes a callback. https://open-telemetry.github.io/opentelemetry-js-api/classes/contextapi.html#with

See also: https://opentelemetry.io/docs/instrumentation/js/api/context/#active-context

In order to use the diagnostics channel we need a way of accessing the active context asynchronously, to set the active context during “request create” diagnostics function and restore the previous context during the “trailers” diagnostics - it feels like this what an async context is supposed to solve for in theory but the current context API doesn’t allow this.

The way this API works seems different to what the spec states, but I’m probably not understanding it. https://opentelemetry.io/docs/reference/specification/trace/api/#context-interaction

[vmarchaud]

I don't think this is feasible with the current context API, we'll need to move forward with open-telemetry/opentelemetry-js-api#123 that allows manually control the context like you said (insert at request create and remove afterwards)

[m1heng]

So once open-telemetry/opentelemetry-js-api#123 is merged, we can looking forward to have https://github.com/gadget-inc/opentelemetry-instrumentations/tree/main/packages/opentelemetry-instrumentation-undici merged into current repo and have it as part of auto-instrumentations?

[vmarchaud]

It can be upstreamed by @gadget-inc folks into this repo and added into auto instrumentations without requiring open-telemetry/opentelemetry-js-api#123 to be merged, it's just that it spans created by a undici request will not have the correct parent (which isn't a big issue i think since undici is just a http client).

[vmarchaud]

To be clear, when i said "I don't think this is feasible with the current context API", i was refering to:

In order to use the diagnostics channel we need a way of accessing the active context asynchronously, to set the active context during “request create” diagnostics function and restore the previous context during the “trailers” diagnostics - it feels like this what an async context is supposed to solve for in theory but the current context API doesn’t allow this.

The undici folks prefered using diag channel to support tracing instead of monkey patching (which is what the "unoficial" instrumentation do)

[m1heng]

I see, thank you @stefee and @vmarchaud for your contributions on this which would really solve my problem, I will try the "unofficial" one on my project first and see how it goes with splunk signalFX.
I still looking forward to see the context API PR get merged and we can have a comprehensive undici instrumentation and see it on auto meta.

[SimenB]

Was open-telemetry/opentelemetry-js-api#123 ever ported to https://github.com/open-telemetry/opentelemetry-js? I tried searching without luck

[mateus4k]

Is there any solution other than using the unofficial package?

[Flarna]

What's wrong with the "unofficial" package? As it is published to NPM it seems to be quite official.

Please note that this repo here is more a container for people to store/share packages. The actual ownership/maintainace/.. is not moved to Otel or or similar. There is also no stability guarantee or whatever if it is here compared to somewhere else.
see e.g. component_owners and here.

I would assume that some people even prefer to keep their otel addons in their own repos to keep full control regarding release,...

I don't think we should add one more undici instrumentation here just to have one here.

[MadLittleMods]

To better cross-link, I was able to get opentelemetry-instrumentation-fetch-node (instruments undici) working with Node.js 18 ✅

Whereas with the other unofficial suggestion, opentelemetry-instrumentation-undici, didn't seem to instrument anything (no new spans).

[joostmeijles]

To better cross-link, I was able to get opentelemetry-instrumentation-fetch-node (instruments undici) working with Node.js 18 ✅

Same here on NodeJS v18. Only https://www.npmjs.com/package/opentelemetry-instrumentation-fetch-node works (tested icw. NextJS v13).

It was quite a search to find a working NodeJS v18 fetch instrumentation solution.
Adding it as plugin to this repository would be helpful.

[dyladan]

If discoverability is the issue I would encourage any package authors to add their instrumentations to the registry which is intended for that purpose https://opentelemetry.io/ecosystem/registry/

[david-luna]

PR for undici and also global fetch API support #1951

[JamieDanielson]

#1951 has been merged 🎉 I will close this as completed, please comment if that is incorrect.