Should NoopContextManager.active() return the context set by context.with()

[Flarna]

Currently NoopContextManager.active() always returns ROOT_CONTEXT.

This is somewhat strange if you consider following code as it looks like a basic contract is broken:

api.context.with(ctx, () => {
  api.context.active() // returns ROOT_CONTEXT
});

I think even the NoopContextManager should at least fulfill the basic contract that context.active() within the with() callback returns the context set.

In special if I think about Behavior of the API in the absence of an installed SDK a typical piece of code could be something like this (only a propagator installed but no SDK):

const ctx = api.propagation.extract(ROOT_CONTEXT, carrier);
api.context.with(ctx, () => {
  ...
  api.propagation.inject(api.context.active(), carrier);
});

Or is this the intended setup and a user has to create/install a working context manager in addition to the propagator in such setups?

[vmarchaud]

I'm not sure, its quite tricky to handle because context loss can happen within with callback (simply with async/await) so is it worth to change this behavior ? I think that would be better to force the user to use a working context manager, that would avoid problems where the context api "kinda" work depending on what code is in the callback.

[Flarna]

I'm also unsure. If we change it we have to at least enable it here.

I did a fast look into Java and Python and it seems they instantiate and use a working ContextManager even in API only case. But I assume it's easier for them as they don't have to support different runtimes (Browser/NodeJs).

[obecny]

I think it is fine the way it works now. If we start doing some partial solution in api we might confuse user even more, (why it works for sync but not for async etc.). We should just explain that until the context manager is registered the default behaviour is just noop. What we could do instead is probably show some warning that noop is used, and if you want to change it please register context manager first.

[aabmass]

I think this is quite dangerous for libraries that propagate context even without the SDK. I guess such libraries should really use explicit context passing for anything propagation related and only use context.with() before calling user code, but its not obvious right now.

What we could do instead is probably show some warning that noop is used, and if you want to change it please register context manager first.

For something like an HTTP client that is instrumented directly, this would log warnings to all the users not using OpenTelemetry. They would have no clue what OpenTelemetry is and be confused. I imagine this would make library authors hesitant to instrument with the API directly.

[obecny]

For something like an HTTP client that is instrumented directly, this would log warnings to all the users not using OpenTelemetry. They would have no clue what OpenTelemetry is and be confused. I imagine this would make library authors hesitant to instrument with the API directly.

Do you know that by default nothing is being logged until you enable this explicitly ? so there is no such risk. Secondly if you are not using OpenTelemetry how would this log anything then ?

[dyladan]

I think this would be a breaking change and isn't something we've had people asking for so I'm going to close it.