Use Dedicated Threads for BatchExportProcessor and PeriodicReader

[cijothomas]

For the 1.0 release:

Use dedicated threads for BatchExportProcessor and PeriodicReader. This is done to improve usability, telemetry reliability, simplify debugging, and eliminate issues stemming from asynchronous runtime dependencies and/or improper usage. Existing runtime integrations (eg: rt-tokio etc. will remain available under an experimental feature flag to ensure backward compatibility and allow time for further refinement.

1. Dedicated Background Threads as Default:

   • Both BatchExportProcessor and PeriodicReader will use dedicated background threads by default.

2. Existing Runtime - Offer under Experimental Feature flag:

   • For platforms with threading restrictions or users preferring OpenTelemetry to integrate with their existing async runtimes, we will continue to support current runtime-based configurations. These configurations will be moved under an experimental feature flag. This approach ensures that existing users are not disrupted while providing time to polish the APIs and functionalities beyond 1.0. Post 1.0, these can be advanced to be offered under stable feature flags.

3. Networking Library Issues:

   • Networking libraries like reqwest, hyper, and tonic used by our core Exporter (OTLP) present challenges when operating outside async runtimes:
     • reqwest: Replace with reqwest-blocking for dedicated threads. Given that the thread being blocked is OpenTelemetry's own background thread, this should not be a concern.
     • hyper and tonic: Evaluate their compatibility with dedicated threads and find appropriate solutions. If certain clients cannot be supported, users requiring those clients can revert to the experimental runtime feature.

Advantages of Dedicated Threads

1. Improved Reliability:

   • Telemetry processing becomes independent of shared thread pools (e.g., Tokio), ensuring it continues even when pools are saturated, avoiding issues like How to debug metrics not being sent to otel-collector #2058 (comment) and [Feature]: Support Metrics, BatchProcessor without async runtime #1437 (comment)

2. Eliminate a class of bugs:

   • Many shutdown and deadlock issues reported stem from incorrect use or combinations of async runtimes. Dedicated threads, combined with defaults that work for majority scenarios, eliminate this class of issues/bugs.

3. Simplified Debugging:

   • Debugging issues like "telemetry not flowing" becomes easier when telemetry processing occurs on a known, dedicated thread.

4. Ease of onboarding

   • By offering reasonable defaults, it is easy to onboard quickly to OpenTelemetry Rust + OTLP. Users do not have to worry about which async runtimes and networking client combinations to use. Currently there are no defaults and users are required to pick runtimes.

Implementation Plan

1. Start with Metrics:

   • Leverage the existing PeriodicReaderWithOwnThread, which already spawns its own thread, as a starting point.
   • Make it the default, and make sure OTLP Exporters can be used.

2. BatchExportProcessor:

   • Resurrect the existing work-in-progress pull request (PR) to enable dedicated thread support. This may be done in parallel as well. Focus on Logs only for 1.0

Migration for users

1. If users want telemetry to work with OTLP, remove all feature flags (like rt-tokio) and stop passing the rt argument to BatchExportProcessor and PeriodicReader. The defaults are sufficient for most scenarios.
2. If there is a need to use runtime like tokio, update the feature flags to their "experimental" prefixed versions..

[scottgerring]

This is an interesting one! Some thoughts:

• Thread management in high throughput environments - are we quickly going to end up in a position where we're manually running multiple-threads-per-exporter in order to keep up with telemetry throughput?
• Prior art - how do other language OTEL implementations manage this - for instance, I know you are working on dotnet @cijothomas ?
• Prior art - manual threading colocated with Tokio - I had a hunt around to see if this is "an established pattern" but couldn't find anything. Has anyone seen anything we can learn from?
• Aggregate resource consumption - I reckon would be higher, at least due to the increased memory impact of the additional threads. This may be something we'd end up wanting to measure.
• No-async - we could support no-async apps, which'd be cool, and I don't think can be done today?!

Feature Flags

Existing runtime integrations (eg: rt-tokio etc. will remain available under an experimental feature flag to ensure backward compatibility and allow time for further refinement.

What if this was the other way around - ThreadExportProcessor behind experimental until it stabilises? This feels like a big change, and it'd let the new impl get refined in the background.

Networking Libraries

hyper and tonic

Hyper is async-first, and tonic rests on hyper for gRPC. We'd have to change libraries here.

Concurrent runtimes as an alternative?

An alternative but potentially fraught option would be allowing the user to request a separate Tokio or async-std runtime. This would let us fence ourselves off from the "application" runtime, but keep all the async niceties and libraries, and not have to worry about thread management for e.g. parallel export. The change would then be essentially an override to specify a different runtime on the builders. However, this'd prevent us from providing a "no-async" runtime, if that was a goal.

[cijothomas]

Thread management in high throughput environments - are we quickly going to end up in a position where we're manually running multiple-threads-per-exporter in order to keep up with telemetry throughput?

Excellent point! As of now it'll be just one thread per PeriodicReader/BatchExportProcessor. If the throughput is not sufficient, we'll have to resort to some techniques to improve. If the solution is to use more threads or do some multiplexing, then it should be capped based on user preference. See more in my next reply below.

how do other language OTEL implementations manage this - for instance, I know you are working on dotnet

OTel .NET works exactly as proposed in this issue. It spins up own thread and makes blocking calls from it. So far, I am not aware of any issue with throughput. (IIUC, other languages are also working this way, but I need to check more to get confirmation)
Of course, .NET suffers from the problem that it does not work in environments where spinning up thread is not possible, like browser.

Resource consumption

I don't think this will be measurably high, as its one thread per signal, so at most 3 threads.

No-async - we could support no-async apps, which'd be cool, and I don't think can be done today?!

Correct. We currently mandates an async runtime for OTLP scenarios.

What if this was the other way around - ThreadExportProcessor behind experimental until it stabilises? This feels like a big change, and it'd let the new impl get refined in the background.

This was intentional! The current runtime abstractions and implementations for tokio, async-std exposes a lot of public API (runtime traits and implementation for multiple runtimes) that I am not confident declaring stable. (A very common challenge for folks like us who maintain libraries! committing a public API is a big commitment, as we can't break it ever!) DedicatedThread idea has no public API like this for now (eventually, it'll also let user control how to create the thread themselves, foe eg: if they want to set affinity etc.)
Since this should be the default, it can't be under experimental features. As of today, there are no defaults , forcing users to pickup some runtime themselves.
I agree with your point that the background thread one is new, so it is safe to wait some time before stabilizing it. This is why I put this change under 0.28 milestone itself, allowing time to polish it and gather feedbacks before 1.0. I'd call it a blocker for 1.0 - i.e if we are not confident to call it stable, then we delay 1.0 until it is stable.

The dedicated background thread was already implemented for Metrics (Logs is open PR), and I have been testing it for last several weeks.

Additional note about high throughput scenarios: For extremely high throughput scenarios, especially when running on powerful machines with 100s of CPU cores - BatchExportProcessor is really a bottleneck. Within Microsoft, we leverage exporters like ETW for such high throughput requirements. They have no batching need and no contention, and hence scale extremely well. Unfortunately, OTel Collector currently does not support accept ETW (or Linux counterpart user_events), but this is something I expect to change in next several months.

Hyper is async-first, and tonic rests on hyper for gRPC. We'd have to change libraries here.

We can probably keep the same ones, but make some tweaks to make this work. Based on my own testing, OTLP/gRPC with tonic works fine when called from the background thread, as long the channels/clients were initialized in a tokio context. i.e if user's main application is tokio::main, that is good enough. Of course, we have to test and make sure this is robust enough!
(Reqwest-Blocking, obviously works well. It also require a tweak to avoid creating the blocking client outside of background thread.)

Concurrent runtimes as an alternative?

Agree with the merits of this. I believe this can be an additive change post 1.0, if we see demand for it and prove it is better than own thread. The issue with relying on tokio/runtime thread pool for telemetry is that telemetry flow can be affected by thread pool issues when user makes mistakes... This is a well known problem, and I think it'd be good to make a statement like "OTel is a telemetry library that people often use to diagnose issues within their application. OTel's default design ensures that it can continue to operate even when the application's own thread pool misbehaves and OTel can report metrics/logs about this misbehavior, rather than OTel also stuck without the ability to report telemetry." We never wrote these down explicitly, but a lot of the implementations (esp. Metrics) is already ensuring this (and still improving)

[scottgerring]

Hey @cijothomas thanks for taking the time 💪
This all sounds very reasonable to me! The extra context you added here is helpful in understanding the goal:

The current runtime abstractions and implementations for tokio, async-std exposes a lot of public API (runtime traits and implementation for multiple runtimes) that I am not confident declaring stable.

If we want to support no-async, then this bit is a bit more nuanced, I reckon? :

We can probably keep the same ones, but make some tweaks to make this work. Based on my own testing, OTLP/gRPC with tonic works fine when called from the background thread, as long the channels/clients were initialized in a tokio context. i.e if user's main application is tokio::main, that is good enough. Of course, we have to test and make sure this is robust enough!

... but the work @lalitb 's done using futures_executor on #2390 seems relevant.

To put a line under it, we have:

• Use dedicated thread for Metrics PeriodicReader #2142 ,metrics threaded exporter impl from @cijothomas
• [DRAFT] Add Custom Thread-Based Runtime for OpenTelemetry batch Processing #2390, threaded task runner and logs threaded exporter impl by @lalitb. I gather this is a sketch of a otential post-thread-per-signal world, where we provide a minimal impl of a runtime and depend on that - is this accurate?

What's next - can we unbundle the work in here a bit?

[cijothomas]

What's next - can we unbundle the work in here a bit?

I'll open separate issues for each of the tasks to be done to close this issue soon. There will be many items that we can work in parallel.

[cijothomas]

I gather this is a sketch of a otential post-thread-per-signal world, where we provide a minimal impl of a runtime and depend on that - is this accurate?

Yes. Its is post 1.0 release. (I am not even sure if it will be a minimal implementation of a runtime, or something simpler than that or something else. As mentioned earlier, lot of public APIs exposed, we need time to make sure they are all needed.)