#62 document OpenTelemetry auto discovery NPE's

[steven-aerts]

During auto-discovery it is possible that you see NPE's in OpenTelemetry when the VertxContextProvider picked up as the default ContextProvder. This behavior can be prevented with a system property.

[tsegismont]

Thank you @steven-aerts

Can you share the full stack trace please?

[tsegismont]

I guess this comes from current context which is null.

We should update the implementation so that the noop scope is used if the current context is null. And also, log a trace level message.

Can you take care of it?

[tsegismont]

Comment about the doc

[tsegismont]

If you agree to modify the code as suggested, I think this part should be re-written to indicate that the VertxContextStorage is fine with pure Vert.x application but it might not be the best choice for mixed apps. And then the user needs to select the appropriate storage via the sysprop

[steven-aerts]

Both we, as the reporter of #62 seem to have seen this in a pure Vert.x applications, but OpenTelemetry itself is also creating traces for the actions it does, and those traces need a ContextStorage which works for them.
I will see if I can try to reword it.

[tsegismont]

Then perhaps the Vertx Context Storage can delegate to the default storage when not running on the Vert.x thread?

[steven-aerts]

I guess this comes from current context which is null.

We should update the implementation so that the noop scope is used if the current context is null. And also, log a trace level message.

Can you take care of it?

Stacktrace and more context in #62.
Yes, the context is null, but that does not mean we can just ignore things, as it still needs to act as a ContextStorage.
I am more wondering if the SPI is necessary for Vert.x as it is more or less saying here is a ContextStorage for everyone to use. Which is of course not the case.
I have the feeling that by just removing the SPI, things might just keep on working, as Vert.x will still attach the correct storage, but it will not be picked up automagically.

This might solve this issue without any need to define a system property.

[tsegismont]

I have the feeling that by just removing the SPI, things might just keep on working, as Vert.x will still attach the correct storage, but it will not be picked up automagically.

If we do this, then it's no longer possible to call Span.current() and retrieve the span created by Vert.x

[steven-aerts]

That is sad, Span.current() works when the system property is set.

I am not too familiar with the opentelemetry code base, to understand how to best fall back. For example in more complex examples, like how to behave if a third ContextProvider is injected?

Do you think it is still usefull to update the documentation? Or do you think this is something which needs need be addressed in code?

[Traderjoe95]

I am not too familiar with the opentelemetry code base, to understand how to best fall back. For example in more complex examples, like how to behave if a third ContextProvider is injected?

That's indeed a good question. But I think it's one we can leave up to the application to decide. A good default behavior for the Vert.x Context Storage is to delegate to OpenTelemetry's default context storage. But maybe we can make that configurable. i.e. let the application configure an alternative fallback in OpenTelemetryOptions, if required. But we could run into difficulties with the singleton implementation of VertxContextStorage then. What do you think, @tsegismont?

[tsegismont]

@steven-aerts thank you for bringing this to our attention.

I will close this PR in favor of #72 (which makes the Vert.x context storage provider use the default provider when not invoked on a Vert.x thread).