Different globals can not be set with seperate versions of the @opentelemetry/api package

[lucacasonato]

As I demoed in some previous meetings, Deno now has support for OpenTelemetry out of the box (see docs). Some APIs are automatically instrumented out of the box, but a key part of this feature is the integration with @opentelemetry/api: when a user has this feature enabled, we automatically inject a global TraceProvider, a global MeterProvider and a global ContextManager for @opentelemetry/api to use.

This is done with the semantic equivalent of a bundled @opentelemtry/api (in reality we do not actually bundle, but we re-implement the global setter logic in the global-utils.ts file), which on startup sets these global providers / managers. We chose this approach, because it prevents the hazard where users have to ensure that all OTEL providers / managers are set up before instrumented code is loaded, which can be quite cumbersome.

There are two issues with this approach, both relating to checks in OpenTelemetry:

1. To get globals, a user can not use a @opentelemetry/api version with a higher version than the version that set a globals in the first place (check here). This is presumably done to prevent users from receiving a provider / manager that their copy of the @opentelemetry/api types / API says has one set of methods, but actually has a different (smaller) set of methods. I.e. to enforce that only backwards compatibility is needed in API interfaces, but not forwards compatibility.
2. To set new globals, a user can not use a @opentelemetry/api version with a different version the version that set any globals in the first place (check here). This means that because we provider tracer / meter / context globals, the user can not provide their own propagators for example.

The first problem is a major blocker for us, as we can not reasonably ensure that Deno always has a version of the @opentelemetry/api bundled that is higher than the current latest version on NPM. As such, we lie when setting the global, and set the global version to 1.999.999. I don't think there is anything else we can do here without breaking compat with existing @opentelemetry/api users, but we are absolutely open to ideas.

We initially did not think the second problem was an issue until a user raised the propagators case. We need to support users specifying some globals themselves (such as the propagators). See the user report here: denoland/deno#28082.

Do you all have ideas what we could do to support this use case? Should there be a different set of fallback globals that can be overwritten by default? Should either or both of the checks be removed? Should we be doing something completely different?

I was chatting with @legendecas and he thinks that at least the second check (in the setter) can probably safely removed.

---

• This only affects the JavaScript OpenTelemetry library
• This may affect other libraries, but I would like to get opinions here first

[pichlermarc]

1. To get globals, a user can not use a @opentelemetry/api version with a higher version than the version that set a globals in the first place (check here). This is presumably done to prevent users from receiving a provider / manager that their copy of the @opentelemetry/api types / API says has one set of methods, but actually has a different (smaller) set of methods. I.e. to enforce that only backwards compatibility is needed in API interfaces, but not forwards compatibility.

Yes, this limitation is enforced by the API as the specification allows changes that are breaking for implementers of API interfaces in minor versions of the API. It is my observation that whole OTel specification process kind of sits on top of this assumption and that is the same for all implementations of OTel that I know of. That's something that must be raised with the spec if it needs to be changed and would likely be a very difficult process.

It's also one of the reasons why almost all of our packages leave the choice of @opentelemetry/api to the end-user (limited through peer-dependency ranges).

2. To set new globals, a user can not use a @opentelemetry/api version with a different version the version that set any globals in the first place (check here). This means that because we provider tracer / meter / context globals, the user can not provide their own propagators for example.

Hmm, some workaround idea: wouldn't it be possible to offer a different API that - instead of overriding would just extend a custom, run-time appendable and deno-specific CompositePropgator instead? That has some drawbacks as the SDK registration process differs, but that is expected for different runtimes and does not fragment the OTel ecosystem in a meaningful way - what is important is that all instrumentation code only references @opentelemetry/api and is SDK agnostic to keep compatible with and first or third-party SDKs after setup has been completed, anything that happens before registration is fair-game.

I would assume that only propagators are the only "real" problem in deno, right?

I was chatting with @legendecas and he thinks that at least the second check (in the setter) can probably safely removed.

Yes, I think I agree with that. If an end-user wants to downgrade the currently registered globals to an older version that still delivers the full feature-set needed by the installed version of the API we should not stand in their way. 👍

[lucacasonato]

Hmm, some workaround idea: wouldn't it be possible to offer a different API that - instead of overriding would just extend a custom, run-time appendable and deno-specific CompositePropgator instead?

I would assume that only propagators are the only "real" problem in deno, right?

@pichlermarc I think this could work, yeah. I think this is particularly important considering the fact that we should default the propagators to the value of OTEL_PROPAGATORS, and you then want to append to the list rather than replace it. I will see what that could look like on our side. Probably something along the lines of Deno.telemetry.addPropagator()

I do think it would still make sense to lift the restriction in @opentelemetry/api regardless, because I could imagine some power user wanting to override the JS exposed TraceProvider or MetricProvider interfaces with a wrapper (for example to work around some buggy instrumentation library).

[pichlermarc]

I do think it would still make sense to lift the restriction in @opentelemetry/api regardless, because I could imagine some power user wanting to override the JS exposed TraceProvider or MetricProvider interfaces with a wrapper (for example to work around some buggy instrumentation library).

Yep, I agree about the second check. Do you want to open a separate feature-request for that with some info on the concrete steps to take and link to the code that needs to be changed so that we can track this change? 🙂

I usually try to bring API changes like that up with a few more people before saying yes to double-check that we're not missing anything. A concrete "change x to y because of z" helps communicate that a bit more easily. 🙂

[lucacasonato]

Done: #5459

[pichlermarc]

Hi @lucacasonato thanks for opening the other issue. It seems that I misunderstood earlier - we cannot fully remove the check there as it may influence other parts of the app that are trying to obtain a global. So I don't think we'd be able to fully remove that check.

Additionally, there's some difficulty that comes from the fact that Deno's registered SDK pretends to be registered by a later API version than what is possible. As I've previously stated, the API may introduce breaking changes for implementers (like Deno's SDK) in minor version (relevant spec sections: [1], [2]) - so a 1.9.0 implementation acting like a 1.999.999 implementation of the API may therefore break end-users when used with @opentelemetry/1.10.0+ since members that are now required are not implemented.

An example: the specification introduces a function Tracer#addFoo() that all SDKs should implement. In this case we would add this to Tracer and release as  @opentelemetry/api@1.10.0. Even though it is breaking for implementers, end-users may continue to use the Tracer as if it was @opentelemetry/api@1.9.0 as it does not implement Tracer#addFoo().

[lucacasonato]

I understand this risk - unfortunately I do not think there is any way to avoid doing this "high version" trick from our side, because it is not reasonable for users to not be able to run their app every time @opentelemetry/api is updated.

It is already frequently the case in the web space that the types the user sees may not line up with what is actually implemented in any given runtime - that is the case here too.

[raphael-theriault-swi]

As a vendor I have to say I am very strongly opposed to removing that check. Issues where multiple versions of the API package are present in the same app and are very subtly incompatible are not common but they do occur and are very hard to troubleshoot, and removing that check would make them even harder to troubleshoot.

I think the common wisdom at the moment is to either requiring end users to provide the API package themselves and list it as a peer dep, or re-exporting the version that is being depended on. Personally I'd argue it falls on Deno (and on anyone else bundling the API) to figure out how to deal with scenarios where end users also depend on the API.

We do actually have some logic for that in our Lambda instrumentation, although it isn't really applicable to Deno given the way y'all do module resolution, but it gives a general idea.