Repository navigation
test(storage): join metrics reader threads before tests exit - #16604
kalragauri wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request re-enables several previously flaky gRPC metrics tests by introducing a ScopedProviderShutdown helper class. This helper ensures that process-wide MeterProvider instances are properly shut down when a test ends, preventing background collection threads from racing with test exit. The review feedback suggests optimizing the RegisteredAuthorityHonorsOption test by avoiding redundant calls to FindMeterProvider and instead reusing the retrieved provider.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #16604 +/- ##
========================================
Coverage 92.40% 92.40%
========================================
Files 2266 2266
Lines 220810 221043 +233
========================================
+ Hits 204036 204255 +219
- Misses 16774 16788 +14 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
EnableGrpcMetricsImpl()stores itsMeterProviderinExporterRegistry::Singleton()and registers it with gRPC viaOpenTelemetryPluginBuilder::BuildAndRegisterGlobal(). Neither singleton is ever destroyed, so the provider'sPeriodicExportingMetricReaderthread is still running when a test binary exits. That thread runs its first collection immediately upon startup; if the collection overlapsexit(),SyncMetricStorage::Collect()readscardinality_limit_from the function-local static inAggregationConfig::GetOrDefault()after~AggregationConfig()has poisoned it under MemorySanitizer. Three unit tests that callEnableGrpcMetricsImpl()hit this race intermittently and were skipped in #16588.This PR adds a
ScopedProviderShutdownRAII guard inchannel_metrics_test.ccandmetrics_exporter_impl_test.ccthat downcasts theMeterProvidertoopentelemetry::sdk::metrics::MeterProviderand callsShutdown()when the test ends (including after a failedASSERT_*), joining the reader thread beforeexit()runs.Fixes #16587