Documentation should mention tracing.instrument-threads

[dstepanov]

Please consider to enable it by default in the next major version, with all kind of thread pools tracing is not much useful without it.

[graemerocher]

It was enabled in a previous version, but we encountered issues with open tracing. See #2126 and micronaut-projects/micronaut-gcp#15

If you are able to find a way to make it work with instrument-threads enabled and not causing the above issues then let us know :)

[dstepanov]

That's strange, looking into code I would suggest an improvement to use CurrentTraceContext#wrap instead of TracingRunnable and TracingCallable.

We are using tracing with zipkin publishing and new brave without problems, I have also disabled health spans sampling using new brave api.

[graemerocher]

Maybe that would work, I must admit it is very hard getting something working for both Brave/zipkin and OpenTracing/jaeger that works consistently for both. I think instrument-threads is possibly ok for the former. But any contributions and improvements are of course welcome.

[geemanjs]

Hey @graemerocher - we just hit something related to this

I think our use case is pretty simple. We have a service (service a) which exposes a http server controller applies some logic and as part of its processing calls another service (service b) using the micronaut http client. The other service (service b) receives the request, does some processing returns a success and or failure to the client (service a) which in turn completes its logic and returns to the initial caller. In our case - latency is of the highest importance so we have pretty extensive instrumentation and testing to find bottlenecks through our code.

For tracing we use OpenTracing/Jaeger and we have server.thread-selection: auto (originally due to #3651 - but we may be able to remove this now)

We recently "yolo upgraded" from micronaut 2.5.5 -> 2.5.11 and found all our tracing stopped carrying through services and after digging through a lot of code basically stumbled upon tracing,instrument-threads: true and enabling it fixed our issue.

However, before running around enabling it on all our services we're kean to understand how it works and the drawbacks / what work is outstanding - we may be able to help. I've been through the majority of the issue's mentioned above and they are all closed (and/or fixed) - is it fair to assume we can enable this without issue now? And perhaps set it's default to true?

Thanks 👍

[graemerocher]

In general thread based instrumentation can lead to leaking data in a reactive context which may be unsafe, hence why it is disabled by default.