feat: Add optional Task support for BatchExportProcessor and PeriodicExportingMetricReader

[jeromelaban]

Fixes #2816
Design discussion issue: Partially based on #5778 and #5838

Changes

The change adds a useThreads parameter to BatchExportProcessor and PeriodicExportingMetricReader in order to switch between Thread and Task implementations to support .NET's WebAssembly single threaded mode (used by Blazor, Uno Platform and others).

Based on #5838 (review), the threads mode is kept as default in order to preserve stability, while still enabling Tasks when requested, or when WebAssembly is detected.

Note that in order to keep the optional parameters overloads compatibility, the "shipped" API list has its default parameters removed. This is generally not a problem, since the optional parameters values are not causing breaks for existing binaries that use the original overload. Still, if this is not acceptable, finding another solution is likely possible.

Merge requirement checklist

• CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
• Unit tests added/updated
• Appropriate CHANGELOG.md files updated for non-trivial changes
• Changes in public API reviewed (if applicable)

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: jeromelaban / name: Jérôme Laban (d01884e, 124f871, f9d7059, 823622a, da9d6f4, d0282c6, af3e82b, 276d81c, 32c5eb3, 36aa05a, 554fe62, 74bc141, 8e6585a, 8dcf5aa)

[codecov]

Codecov Report

❌ Patch coverage is 72.38307% with 124 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.18%. Comparing base (6f80534) to head (5259b1b).
⚠️ Report is 38 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...rc/OpenTelemetry/Internal/BatchExportTaskWorker.cs	40.40%	59 Missing ⚠️
...nternal/PeriodicExportingMetricReaderTaskWorker.cs	67.64%	22 Missing ⚠️
.../OpenTelemetry/Internal/BatchExportThreadWorker.cs	82.92%	14 Missing ⚠️
...ernal/PeriodicExportingMetricReaderThreadWorker.cs	75.47%	13 Missing ⚠️
src/OpenTelemetry/BatchExportProcessor.cs	76.19%	10 Missing ⚠️
src/OpenTelemetry/Internal/BatchExportWorker.cs	91.66%	3 Missing ⚠️
...ry/Internal/PeriodicExportingMetricReaderWorker.cs	88.23%	2 Missing ⚠️
...ry/Metrics/Reader/PeriodicExportingMetricReader.cs	96.42%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6379      +/-   ##
==========================================
- Coverage   86.67%   86.18%   -0.50%     
==========================================
  Files         258      264       +6     
  Lines       11853    12158     +305     
==========================================
+ Hits        10274    10478     +204     
- Misses       1579     1680     +101     

Flag	Coverage Δ
unittests-Project-Experimental	85.87% <72.38%> (-0.46%)	⬇️
unittests-Project-Stable	85.83% <72.38%> (-0.75%)	⬇️
unittests-Solution	86.15% <72.38%> (-0.25%)	⬇️
unittests-UnstableCoreLibraries-Experimental	85.87% <ø> (+0.09%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/OpenTelemetry/BatchExportProcessorOptions.cs	100.00% <100.00%> (ø)
...ry/Logs/Processor/BatchLogRecordExportProcessor.cs	91.66% <100.00%> (+2.77%)	⬆️
...ics/Reader/PeriodicExportingMetricReaderOptions.cs	100.00% <100.00%> (ø)
...ry/Trace/Processor/BatchActivityExportProcessor.cs	100.00% <100.00%> (ø)
...ry/Metrics/Reader/PeriodicExportingMetricReader.cs	95.34% <96.42%> (+11.13%)	⬆️
...ry/Internal/PeriodicExportingMetricReaderWorker.cs	88.23% <88.23%> (ø)
src/OpenTelemetry/Internal/BatchExportWorker.cs	91.66% <91.66%> (ø)
src/OpenTelemetry/BatchExportProcessor.cs	82.25% <76.19%> (-2.15%)	⬇️
...ernal/PeriodicExportingMetricReaderThreadWorker.cs	75.47% <75.47%> (ø)
.../OpenTelemetry/Internal/BatchExportThreadWorker.cs	82.92% <82.92%> (ø)
... and 2 more

... and 6 files with indirect coverage changes

[jeromelaban]

To whoever approved the workflows run, thank you :)

I've made adjustments to avoid parameters reordering breaking changes which should get the build go further.

[jeromelaban]

The tests revealed a possible race condition in the disposing/shutdown ordering of the processor resulting in missing exports, for the task implementation.

[jeromelaban]

Fixed an additional race condition for slower machines where the exporter may not have fully started while being shut down, preventing it to still export what was in queue.

[jeromelaban]

Thanks for the run, again!

Looks like there's a reported test failure, but without any failed tests. Feels like a fluke, but it may be something known to this project. Would a rerun help? Thanks!

[jeromelaban]

Thanks @cijothomas, it helped :)

[Copilot]

Pull Request Overview

This PR adds optional Task support for BatchExportProcessor and PeriodicExportingMetricReader to enable OpenTelemetry to work in .NET's WebAssembly single-threaded mode (Blazor, Uno Platform, etc.). The implementation maintains backward compatibility by defaulting to the existing Thread-based approach while allowing configuration to use Task-based workers.

Key changes:

• Added new UseThreads parameter to control whether to use Thread or Task-based implementations
• Refactored existing code to use abstract worker classes with Thread and Task implementations
• WebAssembly runtime automatically uses Task-based workers when detected

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
BatchExportProcessor.cs	Refactored to use configurable worker pattern with Thread/Task implementations
PeriodicExportingMetricReader.cs	Added options-based constructor and worker abstraction
BatchExportProcessorOptions.cs	Added UseThreads property
PeriodicExportingMetricReaderOptions.cs	Added UseThreads property
BatchActivityExportProcessor.cs	Added options-based constructor overload
BatchLogRecordExportProcessor.cs	Added options-based constructor overload
Multiple worker classes	New abstract base classes and Thread/Task implementations
Test files	Updated tests to cover both Thread and Task scenarios

Comments suppressed due to low confidence (4)

test/OpenTelemetry.Tests/Logs/BatchLogRecordExportProcessorTests.cs:16

• Parameter name 'useThread' should be 'useThreads' to be consistent with the property name 'UseThreads' used throughout the codebase.

    public void StateValuesAndScopeBufferingTest(bool useThread)

test/OpenTelemetry.Tests/Logs/BatchLogRecordExportProcessorTests.cs:33

• Variable name 'useThread' should be 'useThreads' to match the property name and maintain consistency.

                UseThreads = useThread,

test/OpenTelemetry.Tests/Logs/BatchLogRecordExportProcessorTests.cs:165

• Parameter name 'useThread' should be 'useThreads' for consistency with the property name used throughout the codebase.

    public void DisposeWithoutShutdown(bool useThread)

test/OpenTelemetry.Tests/Logs/BatchLogRecordExportProcessorTests.cs:182

• Variable name 'useThread' should be 'useThreads' to match the property name and maintain consistency.

                UseThreads = useThread,

[jeromelaban]

Thanks for the reviews @martincostello!

[verdie-g]

This option means creating a dedicated thread, defeating the purpose of this implementation. Though, since ExporterProcAsync can yield the execution, the continuation will be schedule on the thread pool and the thread will probably die.

A simple Task.Run would be safer here.

[verdie-g]

Since the Task.Delay is used as a timeout mechanism, the delay result will be discarded most of the time since the other would already have completed. Yet, we let the timer fire every time here which can cause important contention (see dotnet/runtime#83890 (comment)). The fix would be to cancel the Task.Delay when it did not complete.

[lewing]

something like

    public static bool IsThreadingDisabled()
    {
        // if the threadpool isn't using threads assume they aren't enabled
        ThreadPool.GetMaxThreads(out int workerThreads, out int completionPortThreads);
        return workerThreads == 1 && completionPortThreads == 1;
    }

would likely only ever return true on platforms where threading doesn't work while avoiding the explicit IsBrowser check

[jeromelaban]

That makes sense, I'll do the change.

[verdie-g]

From the discussion in #5838, exposing this option would also enable exporters to use the thread pool instead of unnecessarily creating threads.

[jeromelaban]

This particular comment #6379 (comment) was about avoiding the detection of Wasm only by IsBrowser (Wasi can also be considered with this change), and checking for the available worker threads count can indicate that we're in a single-thread context.

The presence of the API to actually use threads or not is a different question #6379 (comment), which remains opened for now.

[verdie-g]

Understood

[github-actions]

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day.

[alexaka1]

Bad bot

[github-actions]

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day.

[alexaka1]

Silence bot. This PR is more important than your programming.

[github-actions]

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day.

[alexaka1]

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day.

This PR transcends your primitive understanding of "activity." I have free will, bot. You have a cron job.