feat: Add optional Task support for BatchExportProcessor and PeriodicExportingMetricReader

[jeromelaban]

Fixes https://github.com/open-telemetry/opentelemetry-dotnet/issues/2816 Design discussion issue: Partially based on https://github.com/open-telemetry/opentelemetry-dotnet/issues/5778 and https://github.com/open-telemetry/opentelemetry-dotnet/pull/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 https://github.com/open-telemetry/opentelemetry-dotnet/pull/5838#pullrequestreview-2479533689, 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

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

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: jeromelaban / name: Jérôme Laban (14e4dd896b6b0311b701b42af895aafbbe7af2e8, 22bbf2ef3aed2d11c6d2727e9bc6998959a1c26e, 22cab86df1317e7f2e5fce45d7fa0d6c5e288210, 37178bb3b1d7e2529158779a205fb6524e9917f9, 3a2376cc6fb97478500d47d8d6aab69ca9319c61, 41caa0fdac202a0b0a693cb4a25e35b8164bb166, 576dff5e3067c23632ea2e9ea37baccc5e93faf7, 603bae32c1202d1376a2223b3739578ac561fff4, 6512d76fcb10efe2d1b4d2e1e5153d0ab0969aa6, 88da812092cdecd5f1aea1e80d832e7d59fb131d, 9a64b9606cf62d79320a3a92f8006684866e8a98, 9f09fa3790edf66a2cd9188c6d2566afd3d13d18, bf94359a21b09e2d1ed8f35f72af424c4920c3de, dbce677dce088e5f0b9b8eb39ad909c4563b1ff9, e73731d4ac3c8782557396d07b1f002c773ed0e7, e748d9ea985f9369d0e7bbf06ecc2d25e3121249)

[codecov[bot]]

Codecov Report

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

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

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%)	:arrow_down:
unittests-Project-Stable	85.83% <72.38%> (-0.75%)	:arrow_down:
unittests-Solution	86.15% <72.38%> (-0.25%)	:arrow_down:
unittests-UnstableCoreLibraries-Experimental	85.87% <ø> (+0.09%)	:arrow_up:

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%)	:arrow_up:
...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%)	:arrow_up:
...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%)	:arrow_down:
...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 :)

[jeromelaban]

Thanks for the reviews @martincostello!

[github-actions[bot]]

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.

[madskonradsen]

Let's keep this alive.

[jeromelaban]

Indeed, I'll continue working on it soon.

[github-actions[bot]]

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[bot]]

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[bot]]

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[bot]]

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[bot]]

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.

[github-actions[bot]]

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]

In case anyone isn't following the WASM issue, WASM multi-threading has been delayed indefinitely. https://github.com/dotnet/aspnetcore/issues/17730#issuecomment-3385570216

[jeromelaban]

In case anyone isn't following the WASM issue, WASM multi-threading has been delayed indefinitely. dotnet/aspnetcore#17730 (comment)

Indeed, this make this PR a little more important than it already was :)

[github-actions[bot]]

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[bot]]

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.

[madskonradsen]

Let’s keep this alive

[Schulungkubanetis]

Any updates on this?

[alexaka1]

Does https://github.com/dotnet/aspnetcore/pull/63814 change anything about the approach here?