From 1d9a4de137392e8054812a374978dfcff14a7dd8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 2 Jul 2025 01:16:25 +0000 Subject: [PATCH 1/3] Initial plan From 7b1872b45e5d329fe3bbcbae0aa41d84758e8c51 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 2 Jul 2025 01:24:52 +0000 Subject: [PATCH 2/3] Fix MeterProvider destructor warning when Shutdown() called manually - Add IsShutdown() method to MeterContext - Add atomic is_shutdown_ member to track shutdown state - Update MeterProvider destructor to check IsShutdown() before calling Shutdown() - Add test case for shutdown behavior - Follows same pattern as BatchSpanProcessor and SimpleLogRecordProcessor Co-authored-by: lalitb <1196320+lalitb@users.noreply.github.com> --- .../opentelemetry/sdk/metrics/meter_context.h | 7 +++++++ sdk/src/metrics/meter_context.cc | 6 ++++++ sdk/src/metrics/meter_provider.cc | 2 +- sdk/test/metrics/meter_provider_sdk_test.cc | 14 ++++++++++++++ 4 files changed, 28 insertions(+), 1 deletion(-) diff --git a/sdk/include/opentelemetry/sdk/metrics/meter_context.h b/sdk/include/opentelemetry/sdk/metrics/meter_context.h index 53f6298049..b05ae297eb 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter_context.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter_context.h @@ -170,6 +170,11 @@ class MeterContext : public std::enable_shared_from_this */ bool Shutdown(std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept; + /** + * Check if this meter context has been shut down. + */ + bool IsShutdown() const noexcept; + private: friend class ::testing::MetricCollectorTest; @@ -187,8 +192,10 @@ class MeterContext : public std::enable_shared_from_this #if defined(__cpp_lib_atomic_value_initialization) && \ __cpp_lib_atomic_value_initialization >= 201911L std::atomic_flag shutdown_latch_{}; + std::atomic is_shutdown_{false}; #else std::atomic_flag shutdown_latch_ = ATOMIC_FLAG_INIT; + std::atomic is_shutdown_{false}; #endif opentelemetry::common::SpinLockMutex forceflush_lock_; opentelemetry::common::SpinLockMutex meter_lock_; diff --git a/sdk/src/metrics/meter_context.cc b/sdk/src/metrics/meter_context.cc index 863ef21839..86b4c03d16 100644 --- a/sdk/src/metrics/meter_context.cc +++ b/sdk/src/metrics/meter_context.cc @@ -163,6 +163,7 @@ bool MeterContext::Shutdown(std::chrono::microseconds timeout) noexcept // Shutdown only once. if (!shutdown_latch_.test_and_set(std::memory_order_acquire)) { + is_shutdown_.store(true, std::memory_order_release); for (auto &collector : collectors_) { bool status = std::static_pointer_cast(collector)->Shutdown(timeout); @@ -180,6 +181,11 @@ bool MeterContext::Shutdown(std::chrono::microseconds timeout) noexcept return result; } +bool MeterContext::IsShutdown() const noexcept +{ + return is_shutdown_.load(std::memory_order_acquire); +} + bool MeterContext::ForceFlush(std::chrono::microseconds timeout) noexcept { bool result = true; diff --git a/sdk/src/metrics/meter_provider.cc b/sdk/src/metrics/meter_provider.cc index 92a6b033bc..b514331ec8 100644 --- a/sdk/src/metrics/meter_provider.cc +++ b/sdk/src/metrics/meter_provider.cc @@ -161,7 +161,7 @@ bool MeterProvider::ForceFlush(std::chrono::microseconds timeout) noexcept */ MeterProvider::~MeterProvider() { - if (context_) + if (context_ && !context_->IsShutdown()) { context_->Shutdown(); } diff --git a/sdk/test/metrics/meter_provider_sdk_test.cc b/sdk/test/metrics/meter_provider_sdk_test.cc index 0c1f4d7de1..2254be04d1 100644 --- a/sdk/test/metrics/meter_provider_sdk_test.cc +++ b/sdk/test/metrics/meter_provider_sdk_test.cc @@ -323,3 +323,17 @@ TEST(MeterProvider, GetMeterInequalityCheckAbiv2) } #endif /* OPENTELEMETRY_ABI_VERSION_NO >= 2 */ + +TEST(MeterProvider, ShutdownTwice) +{ + // Test that calling Shutdown twice doesn't emit warnings + MeterProvider mp; + + // First shutdown should succeed + EXPECT_TRUE(mp.Shutdown()); + + // Second shutdown should also succeed without warnings + EXPECT_TRUE(mp.Shutdown()); + + // Destructor should not emit warnings either since we already shut down +} From aff62f54400152937d583c1f2257251631c0f74b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 2 Jul 2025 23:42:53 +0000 Subject: [PATCH 3/3] Fix formatting - remove trailing whitespace from test Co-authored-by: ThomsonTan <1041795+ThomsonTan@users.noreply.github.com> --- sdk/test/metrics/meter_provider_sdk_test.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/test/metrics/meter_provider_sdk_test.cc b/sdk/test/metrics/meter_provider_sdk_test.cc index 2254be04d1..bf055721c5 100644 --- a/sdk/test/metrics/meter_provider_sdk_test.cc +++ b/sdk/test/metrics/meter_provider_sdk_test.cc @@ -328,12 +328,12 @@ TEST(MeterProvider, ShutdownTwice) { // Test that calling Shutdown twice doesn't emit warnings MeterProvider mp; - + // First shutdown should succeed EXPECT_TRUE(mp.Shutdown()); - + // Second shutdown should also succeed without warnings EXPECT_TRUE(mp.Shutdown()); - + // Destructor should not emit warnings either since we already shut down }