Integrate OpenTelemetry

[mcollina]

OpenTelemetry (OTEL) has emerged as the de facto standard for observability. It's vendor-neutral and a CNCF project. Lately, Deno shipped support for OTEL.

Based on a few conversations in the hallway at the recent collaboration summit, there is interest in building this within Node.js. Moreover, it would simplify many integrations and improve overall performance.

[mcollina]

cc @bengl @targos @legendecas @cjihrig @nodejs/diagnostics

[cjihrig]

Would the plan be to vendor the JavaScript library (or maybe the C++ library)? One concern could be that @opentelemetry/core alone is 9.3MB according to pkg-size.dev. Maybe that's fine though?

It also looks like Deno has run into at least one rough edge here.

[mhdawson]

@mcollina thanks for opening the issue, worth discussing.

[davidfiala]

Would be good to ensure there's a compile time flag to exclude this entirely, not just a runtime flag. Mostly to save space and resources on embedded environments.

[Flarna]

Do you think about removing the "burden" to manually install OTel components like SDK, exporter,... by selecting a specific set of them and include it into node core?
I fear we will run into similar compat problems as that one linked above for deno. Also topics like long term stability need to be clarified. Quite some parts of OTel are still experimental and move/change fast.
OTel is quite extensible, e.g. it supports exporting data via HTTP, grpc,... therefore it's also important to choose wise what is included always and what can be extended by users using npm modules. Which again raises the compat topic between node internal implementation and OTel JS implementation.

Or is it more about creating spans for node internals (e.g. HTTP) using OTel API but leave the SDK, Exporter,.. installation/configuration up to user? So somewhat replace existing diagnostic channels by calls to OTel.

[rochdev]

I don't think integrating OpenTelemetry is the right way to go. What should be done instead is adding diagnostics_channel support everywhere telemetry is needed, which OpenTelemetry can then listen to when installed. This is also how it should be integrated into libraries instead of adding a dependency on OpenTelemetry everywhere as is currently done. This separates the concern of reporting the data from collecting and processing it.

I opened a similar issue a few years ago in open-telemetry/opentelemetry-js#1263.

[AbhiPrasad]

From the perspective of someone who's contributed to OpenTelemetry, and currently works at an observability vendor (Sentry), I would also advocate for putting diagnostics_channel everywhere we need telemetry. OpenTelemetry instrumentation (and other kind of telemetry data) can easily build integrations on-top of diagnostics_channel, which we've seen does work in use cases like Undici.

There are other kinds of telemetry than logs, traces, and metrics (:cough: errors) that could benefit from this, and defaulting to OTEL for observability means we restrict the kind of signals that can be collected from Node.js and it's internals. I'd prefer if Node.js kept this open, as it leaves room for innovation, although I am a bit biased working at Sentry.

Bundling in OTEL means quite a bit of API surface area that Node.js has to expose. It will also introduce possibilities of conflicting versions, especially for instrumentations and semantic conventions (semantic conventions frequently experience changes every release). We built the Sentry Node SDKs on-top of OpenTelemetry, but are now experiencing pains with the breaking change between 1.x and 2.x of the OTEL JS SDKs (it's very hard to support both in the way the packages are architected).

[bengl]

Overall I see a bunch of wins in doing this roughly similar to how Deno did it.

1. There would be less non-core code needed for OTel-style observability. This is great for scenarios like serverless where bundle sizes matter quite a bit.
2. Code in core can be optimized in ways that are difficult or impossible to do outside of core.
3. For observability vendors, zero-install onboarding is desirable, even if it only provides a subset of a vendor's offerings.

All that said, as @rochdev mentioned, as did @AbhiPrasad and @Flarna, there ought to be a focus on including diagnostics_channel support in as many places as possible, both in Node.js core and in popular packages. Rather than including OTel API components everywhere, diagnostics_channel already provides a sufficient API for tracing, logging, metrics, and a host of other observability possibilities. It's a lower-level API that's included, meaning OTel and other observability concepts can be build upon it relatively trivially. By building OTel data on top of it, it's also much easier to share the context objects, which is great for folks building on top of built-in instrumentations. Self-instrumentation is a goal of OTel, but it's better in a lot of ways to do it via diagnostics_channel.

But it's not an either-or situation. We can have both! To serve both these needs, I propose the following approach:

1. Ensure that diagnostics_channel is used appropriately in every place in core that would be useful for OTel-style observability. This primarily means HTTP inbounds and outbounds.
2. Create internals that subscribe to those channels, create those spans/traces, insert context propafation headers, and ship all that data via OTLP (via HTTP for now). This would all only be activated when an OTLP endpoint is provided via an environment variable.
3. Create a new public-facing API that takes arbitrary tracing channels, and subscribes to them and creates spans out of them when the OTel subsystem mentioned above is activated. I'm doing a lot of hand-waving here around converting diagnostics_channel data into span tags, but we ought to be able to have a mostly-declarative way of doing this.
4. As a stopgap, provide TracerProvider, etc., implementations to enable @opentelemetry/api usage.

This would create a subsystem that's runtime-optional, API compatible, can be optimized in core, and provides a nice OOTB experience, while ensuring that low-level observability primitives are still usable for all kinds of vendor needs, and providing a lighter-weight API for library authors to add their own instrumentation with.

I'm glossing over a lot of finer details and rough edges here because I wanted to chime in before the thread got too long 😄.

[cjihrig]

Another option could be for Node.js to develop a blessed OpenTelemetry integration that lives on npm, but is in the Node.js GitHub org. Node core could work backward from that to provide any missing telemetry data or APIs. It wouldn't be quite as simple to get started with, but maybe it means npm installing one thing instead of many things and dealing with the config headache that sometimes comes with OTel.

[legendecas]

Another option could be for Node.js to develop a blessed OpenTelemetry integration that lives on npm, but is in the Node.js GitHub org. Node core could work backward from that to provide any missing telemetry data or APIs. It wouldn't be quite as simple to get started with, but maybe it means npm installing one thing instead of many things and dealing with the config headache that sometimes comes with OTel.

I'm not sure I'm following the reason behind maintaining a https://github.com/nodejs version of OpenTelemetry. Node.js is a first class runtime that is officially supported by https://github.com/open-telemetry/opentelemetry-js and I don't think there is any reason that will block us from landing any improvement in the official repo or npm packages.

I'd agree with @bengl that the ultimate goal is to improve the onboarding setup for users to get a full observerbility experience. OpenTelemetry has its own API and SDK versioning pace and there are disparities between OTel's and Node.js'. I'd agree that the first step is to improve diagnostic_channel in Node.js and reduce the instrumentation cost in OpenTelemetry.

However, shipping OTel in Node.js core is not the only approach that we can do towards the goal of improving observerbility setup. We can also provide an integration stack, like a Docker image setup, to avoid coupling the versioning issue of the two project at the first stage.

Overall, I'm glad that this is brought up!

(/cc @open-telemetry/javascript-maintainers @blumamir @dyladan @JamieDanielson @pichlermarc @trentm)

[cjihrig]

I'm not sure I'm following the reason behind maintaining a https://github.com/nodejs version of OpenTelemetry. Node.js is a first class runtime that is officially supported by https://github.com/open-telemetry/opentelemetry-js and I don't think there is any reason that will block us from landing any improvement in the official repo or npm packages.

Sorry, I wasn't implying any changes to the OpenTelemetry packages. By "provide any missing telemetry data or APIs" I meant in core, such as additional diagnostic channels, etc.

As far as something that could be put on npm, what I meant is: Deno is offering zero config OTel integration. What if we could do something similar via a single npm package, assuming that OTel would not be integrated directly into core. I think that's something that could be useful to users due to the complexity and number of dependencies that need to be configured when setting up OTel by hand.

[Flarna]

Shipping preconfigured/zero config packages for OTel is usually done by APM tool vendors.
They know/decide on details like

• export protocol
• export endpoints
• authentication on these endpoints
• which propagators to use
• which resource detectors to use
• opinionated selection and config of 3rd party instrumentations
• ...
  to get results fitting best to their backend capabilities.

Sure, node project could provide one more opinion here. But why would node project do a better job here then OTel project could do?
There is already an npm package which takes care about most of it and allows config via env vars.

[cjihrig]

Shipping preconfigured/zero config packages for OTel is usually done by APM tool vendors.
They know/decide on details like

My understanding is that this issue is proposing something similar to what Deno is doing. If Deno can do it, so can Node even though we aren't an APM vendor. If you don't think this is a good idea, that is a fine opinion to have.

But why would node project do a better job here then OTel project could do? There is already an npm package which takes care about most of it and allows config via env vars.

My understanding from proposing auto-instrumentation for node:sqlite is that the OTel team isn't overly eager to support this (but maybe I'm wrong there).

[Qard]

Very much agreed with @bengl. We've talked at length about this direction in the past. 🙂

Definitely biased as the creator of diagnostics_channel here, but providing a base on which OTel can be built without its core needing to exist within Node.js core was literally the primary goal I had in mind when creating it.

I wanted to have an API that we could integrate in core with minimal dependencies or overhead, but with the flexibility that additional systems could be connected in userland or potentially exist in core too, but lazy-loaded.

With diagnostics_channel as the base architecture, the heavier capabilities of OTel can be layered on top without needing any of its infrastructure loaded or initialized until needed.

[Flarna]

My understanding from proposing auto-instrumentation for node:sqlite is that the OTel team isn't overly eager to support this (but maybe I'm wrong there).

A concrete instrumentation for node:sqlite is for sure a good thing. Wherever it is hosted it requires to be maintained. OTel project usually doesn't take the maintenance part for instrumentations but they offer to host it on their repo which comes with pros/conts.

Having diagnostic channels (or some other hooks) in node:sqlite would avoid the need for monkey patching without binding node core to OTel API.
On top of the same hooks other tools could also build their stuff.

Having instrumentation/monitoring friendly built-ins and maybe even implement/maintain some of these instrumentations is one topic. But IMHO providing a preconfigured OTel setup is a different beast by far harder to get right.