Skip to content

[Infra] Polyfill Stopwatch.GetElapsedTime#7193

Merged
martincostello merged 11 commits into
open-telemetry:mainfrom
martincostello:polyfill-stopwatch
Apr 30, 2026
Merged

[Infra] Polyfill Stopwatch.GetElapsedTime#7193
martincostello merged 11 commits into
open-telemetry:mainfrom
martincostello:polyfill-stopwatch

Conversation

@martincostello
Copy link
Copy Markdown
Member

Refactoring opportunity I spotted while doing other work that relates to open-telemetry/opentelemetry-dotnet-contrib#4300.

Changes

  • Add a polyfill for Stopwatch.GetElapsedTime() for netstandard2.0 and net462 for use instead of allocating Stopwatchs.
  • Simplify timeout computations and duplicative code.
  • Rename cur to current for readability.

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)

- Add a polyfill for `Stopwatch.GetElapsedTime()` for `netstandard2.0` and `net462` for use instead of allocating `Stopwatch`s.
- Simplify timeout computations and duplicative code.
- Rename `cur` to `current` for readability.
@github-actions github-actions Bot added pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package perf Performance related labels Apr 28, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 28, 2026

Codecov Report

❌ Patch coverage is 93.98496% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.75%. Comparing base (c4fa592) to head (ef3aec4).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...rc/OpenTelemetry/Internal/BatchExportTaskWorker.cs 25.00% 6 Missing ⚠️
.../OpenTelemetry/Internal/BatchExportThreadWorker.cs 84.61% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7193      +/-   ##
==========================================
+ Coverage   89.69%   89.75%   +0.05%     
==========================================
  Files         272      272              
  Lines       13434    13381      -53     
==========================================
- Hits        12050    12010      -40     
+ Misses       1384     1371      -13     
Flag Coverage Δ
unittests-Project-Experimental 89.56% <93.98%> (+0.01%) ⬆️
unittests-Project-Stable 89.72% <93.98%> (+0.16%) ⬆️
unittests-Solution 89.62% <93.98%> (+0.17%) ⬆️
unittests-UnstableCoreLibraries-Experimental 41.34% <38.40%> (+0.02%) ⬆️

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

Files with missing lines Coverage Δ
...tlpExporterPersistentStorageTransmissionHandler.cs 96.49% <100.00%> (+3.38%) ⬆️
...on/Transmission/OtlpExporterTransmissionHandler.cs 100.00% <100.00%> (ø)
src/OpenTelemetry/BatchExportProcessor.cs 96.15% <100.00%> (+1.32%) ⬆️
src/OpenTelemetry/CompositeProcessor.cs 95.45% <100.00%> (-0.14%) ⬇️
...nternal/PeriodicExportingMetricReaderTaskWorker.cs 75.75% <100.00%> (+0.75%) ⬆️
...ernal/PeriodicExportingMetricReaderThreadWorker.cs 78.43% <100.00%> (+2.95%) ⬆️
...emetry/Metrics/Reader/BaseExportingMetricReader.cs 90.90% <100.00%> (-1.40%) ⬇️
...nTelemetry/Metrics/Reader/CompositeMetricReader.cs 77.61% <100.00%> (+2.24%) ⬆️
...lemetry/Metrics/Reader/CompositeMetricReaderExt.cs 100.00% <100.00%> (ø)
src/OpenTelemetry/Metrics/Reader/MetricReader.cs 91.94% <100.00%> (-0.16%) ⬇️
... and 3 more

Not sure why this is appearing, but it doesn't apply to the stress tests anyway.
Remove need to suppress CA1515 by not using the polyfill.
Fix mistake from refactor that `Timeout.Infinite` was always used.
@martincostello martincostello marked this pull request as ready for review April 28, 2026 16:07
Copilot AI review requested due to automatic review settings April 28, 2026 16:07
@martincostello martincostello requested a review from a team as a code owner April 28, 2026 16:07
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a shared Stopwatch.GetElapsedTime polyfill for older TFMs and refactors multiple timeout/elapsed-time computations to avoid allocating Stopwatch instances, plus some small readability cleanups.

Changes:

  • Add src/Shared/StopwatchExtensions.cs to polyfill Stopwatch.GetElapsedTime(long) on non-NET TFMs and add a shared Stopwatch.Remaining(...) helper.
  • Replace Stopwatch.StartNew() allocations with timestamp-based elapsed/remaining computations across processors/readers/workers.
  • Rename loop variable cur to current in composite structures.

Reviewed changes

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

Show a summary per file
File Description
test/OpenTelemetry.Tests/Metrics/MetricApiTests.cs Uses timestamp + GetElapsedTime instead of allocating a Stopwatch for test timing output.
src/Shared/StopwatchExtensions.cs Adds GetElapsedTime polyfill (non-NET) and a Remaining(...) helper.
src/OpenTelemetry/OpenTelemetry.csproj Links the shared StopwatchExtensions.cs into the OpenTelemetry SDK project.
src/OpenTelemetry/Metrics/Reader/PeriodicExportingMetricReader.cs Simplifies shutdown timeout computation using timestamp + remaining helper.
src/OpenTelemetry/Metrics/Reader/MetricReader.cs Refactors collect timeout handling to timestamp + remaining helper and reduces duplicated logging branches.
src/OpenTelemetry/Metrics/Reader/CompositeMetricReaderExt.cs Renames cur to current for readability.
src/OpenTelemetry/Metrics/Reader/CompositeMetricReader.cs Refactors composite reader timeouts to timestamp-based logic and renames cur to current.
src/OpenTelemetry/Metrics/Reader/BaseExportingMetricReader.cs Refactors shutdown timeout handling using timestamp + remaining helper.
src/OpenTelemetry/Internal/PeriodicExportingMetricReaderThreadWorker.cs Refactors interval timeout computation to use GetElapsedTime instead of a Stopwatch instance.
src/OpenTelemetry/Internal/PeriodicExportingMetricReaderTaskWorker.cs Same interval timeout refactor for task-based worker.
src/OpenTelemetry/Internal/BatchExportThreadWorker.cs Refactors wait timeout logic to timestamp-based approach and simplifies Start/Shutdown.
src/OpenTelemetry/Internal/BatchExportTaskWorker.cs Same wait timeout refactor for task-based worker; also simplifies Start/Shutdown and adjusts cancellation guards.
src/OpenTelemetry/CompositeProcessor.cs Refactors force-flush/shutdown timeout logic and renames cur to current.
src/OpenTelemetry/BatchExportProcessor.cs Simplifies force-flush expression body and refactors shutdown timeout computation.
src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/Transmission/OtlpExporterTransmissionHandler.cs Minor simplification in request submission and refactors shutdown timeout with timestamp-based helper.
src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/Transmission/OtlpExporterPersistentStorageTransmissionHandler.cs Refactors shutdown timeout computation to timestamp-based helper.
OpenTelemetry.slnx Adds the new shared source file to the solution view.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/OpenTelemetry/Internal/PeriodicExportingMetricReaderThreadWorker.cs Outdated
Comment thread src/OpenTelemetry/CompositeProcessor.cs
Comment thread src/OpenTelemetry/CompositeProcessor.cs
Comment thread src/OpenTelemetry/Internal/PeriodicExportingMetricReaderTaskWorker.cs Outdated
Comment thread src/Shared/StopwatchExtensions.cs
Comment thread src/Shared/StopwatchExtensions.cs Outdated
Comment thread src/OpenTelemetry/Metrics/Reader/CompositeMetricReader.cs
Comment thread src/OpenTelemetry/Metrics/Reader/CompositeMetricReader.cs
Comment thread src/OpenTelemetry/Internal/BatchExportThreadWorker.cs
Comment thread src/OpenTelemetry/Internal/BatchExportTaskWorker.cs
@martincostello martincostello marked this pull request as draft April 28, 2026 17:24
- Fix accidentally making the timeout smaller and smaller over time.
- Avoid repeated frequency recalculation.
- Fix potential overflow.
@martincostello martincostello marked this pull request as ready for review April 28, 2026 17:48
Fix warning related to `extension`.
Disable for the whole file.
@martincostello
Copy link
Copy Markdown
Member Author

Need to make StyleCop happy with extension somehow...

@martincostello martincostello added this pull request to the merge queue Apr 30, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 30, 2026
@martincostello martincostello added this pull request to the merge queue Apr 30, 2026
Merged via the queue into open-telemetry:main with commit 9d9638e Apr 30, 2026
95 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

perf Performance related pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants