Skip to content

Fix MeterProvider destructor warning when Shutdown() called manually #3513

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions sdk/include/opentelemetry/sdk/metrics/meter_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,11 @@ class MeterContext : public std::enable_shared_from_this<MeterContext>
*/
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;

Expand All @@ -187,8 +192,10 @@ class MeterContext : public std::enable_shared_from_this<MeterContext>
#if defined(__cpp_lib_atomic_value_initialization) && \
__cpp_lib_atomic_value_initialization >= 201911L
std::atomic_flag shutdown_latch_{};
std::atomic<bool> is_shutdown_{false};
#else
std::atomic_flag shutdown_latch_ = ATOMIC_FLAG_INIT;
std::atomic<bool> is_shutdown_{false};
#endif
opentelemetry::common::SpinLockMutex forceflush_lock_;
opentelemetry::common::SpinLockMutex meter_lock_;
Expand Down
6 changes: 6 additions & 0 deletions sdk/src/metrics/meter_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<MetricCollector>(collector)->Shutdown(timeout);
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion sdk/src/metrics/meter_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ bool MeterProvider::ForceFlush(std::chrono::microseconds timeout) noexcept
*/
MeterProvider::~MeterProvider()
{
if (context_)
if (context_ && !context_->IsShutdown())
{
context_->Shutdown();
}
Expand Down
14 changes: 14 additions & 0 deletions sdk/test/metrics/meter_provider_sdk_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Loading