From a9110274684f6276ab82808309c00aecc60b5a68 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Fri, 4 Sep 2020 09:31:48 -0700 Subject: [PATCH 01/11] Remove static global context_handler_ variable --- .../opentelemetry/context/runtime_context.h | 213 ++++++++++++++++-- .../context/threadlocal_context.h | 116 ---------- 2 files changed, 197 insertions(+), 132 deletions(-) delete mode 100644 api/include/opentelemetry/context/threadlocal_context.h diff --git a/api/include/opentelemetry/context/runtime_context.h b/api/include/opentelemetry/context/runtime_context.h index cc4b3bb485..3df6df962b 100644 --- a/api/include/opentelemetry/context/runtime_context.h +++ b/api/include/opentelemetry/context/runtime_context.h @@ -15,7 +15,7 @@ class Token bool operator==(const Context &other) noexcept { return context_ == other; } private: - friend class RuntimeContext; + friend class RuntimeContextStorage; // The ContextDetacher object automatically attempts to detach // the Token when all copies of the Token are out of scope. @@ -45,28 +45,67 @@ class Token nostd::shared_ptr detacher_; }; -// Provides a wrapper for propagating the context object globally. In order -// to use either the threadlocal_context.h file must be included or another -// implementation which must be derived from the RuntimeContext can be -// provided. +/** + * RuntimeContextStorage is used by RuntimeContext to store Context frames. + * + * Custom context management strategies can be implemented by deriving from + * this class and passing an initialized RuntimeContextStorage object to + * RuntimeContext::SetRuntimeContextStorage. + */ +class RuntimeContextStorage +{ +public: + /** + * Return the current context. + * @return the current context + */ + virtual Context GetCurrent() noexcept = 0; + + /** + * Set the current context. + * @param the new current context + */ + virtual Token Attach(Context context) noexcept = 0; + + /** + * Detach the context related to the given token. + * @param token a token related to a context + * @return true if the context could be detached + */ + virtual bool Detach(Token &token) noexcept = 0; + +protected: + Token CreateToken(Context context) noexcept { return Token(context); } +}; + +/** + * Construct and return the default RuntimeContextStorage + * @return a ThreadLocalContextStorage + */ +static RuntimeContextStorage *GetDefaultStorage() noexcept; + +// Provides a wrapper for propagating the context object globally. +// +// By default, a thread-local runtime context storage is used. class RuntimeContext { public: // Return the current context. - static Context GetCurrent() noexcept { return context_handler_->InternalGetCurrent(); } + static Context GetCurrent() noexcept { return GetRuntimeContextStorage()->GetCurrent(); } // Sets the current 'Context' object. Returns a token // that can be used to reset to the previous Context. static Token Attach(Context context) noexcept { - return context_handler_->InternalAttach(context); + return GetRuntimeContextStorage()->Attach(context); } // Resets the context to a previous value stored in the // passed in token. Returns true if successful, false otherwise - static bool Detach(Token &token) noexcept { return context_handler_->InternalDetach(token); } - - static RuntimeContext *context_handler_; + static bool Detach(Token &token) noexcept + { + return GetRuntimeContextStorage()->Detach(token); + } // Sets the Key and Value into the passed in context or if a context is not // passed in, the RuntimeContext. @@ -108,15 +147,46 @@ class RuntimeContext return temp_context.GetValue(key); } -protected: - // Provides a token with the passed in context - Token CreateToken(Context context) noexcept { return Token(context); } + /** + * Provide a custom runtime context storage. + * + * This provides a possibility to override the default thread-local runtime + * context storage. This has to be set before any spans are created by the + * application, otherwise context state information will be discarded, which + * might lead to unexpected results. + * + * @param storage a custom runtime context storage + */ + static void SetRuntimeContextStorage(nostd::shared_ptr storage) noexcept + { + while (GetLock().test_and_set(std::memory_order_acquire)) + ; + GetStorage() = storage; + GetLock().clear(std::memory_order_release); + } - virtual Context InternalGetCurrent() noexcept = 0; +private: + static nostd::shared_ptr GetRuntimeContextStorage() noexcept + { + while (GetLock().test_and_set(std::memory_order_acquire)) + ; + auto storage = nostd::shared_ptr(GetStorage()); + GetLock().clear(std::memory_order_release); - virtual Token InternalAttach(Context context) noexcept = 0; + return storage; + } - virtual bool InternalDetach(Token &token) noexcept = 0; + static nostd::shared_ptr &GetStorage() noexcept + { + static nostd::shared_ptr context(GetDefaultStorage()); + return context; + } + + static std::atomic_flag &GetLock() noexcept + { + static std::atomic_flag lock = ATOMIC_FLAG_INIT; + return lock; + } }; inline Token::ContextDetacher::~ContextDetacher() @@ -125,5 +195,116 @@ inline Token::ContextDetacher::~ContextDetacher() token.context_ = context_; context::RuntimeContext::Detach(token); } + +// The ThreadLocalContextStorage class is a derived class from RuntimeContext and +// provides a wrapper for propogating context through cpp thread locally. +// This file must be included to use the RuntimeContext class if another +// implementation has not been registered. +class ThreadLocalContextStorage : public RuntimeContextStorage +{ +public: + ThreadLocalContextStorage() noexcept = default; + + // Return the current context. + Context GetCurrent() noexcept override { return GetStack().Top(); } + + // Resets the context to a previous value stored in the + // passed in token. Returns true if successful, false otherwise + bool Detach(Token &token) noexcept override + { + if (!(token == GetStack().Top())) + { + return false; + } + GetStack().Pop(); + return true; + } + + // Sets the current 'Context' object. Returns a token + // that can be used to reset to the previous Context. + Token Attach(Context context) noexcept override + { + GetStack().Push(context); + Token old_context = CreateToken(context); + return old_context; + } + +private: + // A nested class to store the attached contexts in a stack. + class Stack + { + friend class ThreadLocalContextStorage; + + Stack() noexcept : size_(0), capacity_(0), base_(nullptr){}; + + // Pops the top Context off the stack and returns it. + Context Pop() noexcept + { + if (size_ <= 0) + { + return Context(); + } + int index = size_ - 1; + size_--; + return base_[index]; + } + + // Returns the Context at the top of the stack. + Context Top() const noexcept + { + if (size_ <= 0) + { + return Context(); + } + return base_[size_ - 1]; + } + + // Pushes the passed in context pointer to the top of the stack + // and resizes if necessary. + void Push(Context context) noexcept + { + size_++; + if (size_ > capacity_) + { + Resize(size_ * 2); + } + base_[size_ - 1] = context; + } + + // Reallocates the storage array to the pass in new capacity size. + void Resize(int new_capacity) noexcept + { + int old_size = size_ - 1; + if (new_capacity == 0) + { + new_capacity = 2; + } + Context *temp = new Context[new_capacity]; + if (base_ != nullptr) + { + std::copy(base_, base_ + old_size, temp); + delete[] base_; + } + base_ = temp; + } + + ~Stack() noexcept { delete[] base_; } + + size_t size_; + size_t capacity_; + Context *base_; + }; + + Stack &GetStack() + { + static thread_local Stack stack_ = Stack(); + return stack_; + } +}; + +static RuntimeContextStorage *GetDefaultStorage() noexcept +{ + return new ThreadLocalContextStorage(); +} } // namespace context OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/context/threadlocal_context.h b/api/include/opentelemetry/context/threadlocal_context.h deleted file mode 100644 index febd89e63a..0000000000 --- a/api/include/opentelemetry/context/threadlocal_context.h +++ /dev/null @@ -1,116 +0,0 @@ -#pragma once - -#include "opentelemetry/context/context.h" -#include "opentelemetry/context/runtime_context.h" - -OPENTELEMETRY_BEGIN_NAMESPACE -namespace context -{ - -// The ThreadLocalContext class is a derived class from RuntimeContext and -// provides a wrapper for propogating context through cpp thread locally. -// This file must be included to use the RuntimeContext class if another -// implementation has not been registered. -class ThreadLocalContext : public RuntimeContext -{ -public: - ThreadLocalContext() noexcept = default; - - // Return the current context. - Context InternalGetCurrent() noexcept override { return stack_.Top(); } - - // Resets the context to a previous value stored in the - // passed in token. Returns true if successful, false otherwise - bool InternalDetach(Token &token) noexcept override - { - if (!(token == stack_.Top())) - { - return false; - } - stack_.Pop(); - return true; - } - - // Sets the current 'Context' object. Returns a token - // that can be used to reset to the previous Context. - Token InternalAttach(Context context) noexcept override - { - stack_.Push(context); - Token old_context = CreateToken(context); - return old_context; - } - -private: - // A nested class to store the attached contexts in a stack. - class Stack - { - friend class ThreadLocalContext; - - Stack() noexcept : size_(0), capacity_(0), base_(nullptr){}; - - // Pops the top Context off the stack and returns it. - Context Pop() noexcept - { - if (size_ <= 0) - { - return Context(); - } - int index = size_ - 1; - size_--; - return base_[index]; - } - - // Returns the Context at the top of the stack. - Context Top() const noexcept - { - if (size_ <= 0) - { - return Context(); - } - return base_[size_ - 1]; - } - - // Pushes the passed in context pointer to the top of the stack - // and resizes if necessary. - void Push(Context context) noexcept - { - size_++; - if (size_ > capacity_) - { - Resize(size_ * 2); - } - base_[size_ - 1] = context; - } - - // Reallocates the storage array to the pass in new capacity size. - void Resize(int new_capacity) noexcept - { - int old_size = size_ - 1; - if (new_capacity == 0) - { - new_capacity = 2; - } - Context *temp = new Context[new_capacity]; - if (base_ != nullptr) - { - std::copy(base_, base_ + old_size, temp); - delete[] base_; - } - base_ = temp; - } - - ~Stack() noexcept { delete[] base_; } - - size_t size_; - size_t capacity_; - Context *base_; - }; - - static thread_local Stack stack_; -}; -thread_local ThreadLocalContext::Stack ThreadLocalContext::stack_ = ThreadLocalContext::Stack(); - -// Registers the ThreadLocalContext as the context handler for the RuntimeContext -RuntimeContext *RuntimeContext::context_handler_ = new ThreadLocalContext(); -} // namespace context -OPENTELEMETRY_END_NAMESPACE From f526460634eb945bbac147ee24f62f926af6b01d Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Fri, 4 Sep 2020 09:58:36 -0700 Subject: [PATCH 02/11] Remove obsolete include --- .../opentelemetry/context/runtime_context.h | 15 ++++++--------- api/test/context/runtime_context_test.cc | 1 - api/test/plugin/dynamic_load_test.cc | 1 - examples/batch/main.cc | 2 -- examples/otlp/foo_library/foo_library.cc | 1 - examples/plugin/load/main.cc | 1 - examples/simple/main.cc | 1 - examples/zpages/zpages_example.cc | 1 - exporters/ostream/test/ostream_span_test.cc | 1 - exporters/otlp/test/otlp_exporter_benchmark.cc | 1 - exporters/otlp/test/otlp_exporter_test.cc | 1 - exporters/otlp/test/recordable_test.cc | 1 - ext/test/zpages/threadsafe_span_data_test.cc | 1 - ext/test/zpages/tracez_data_aggregator_test.cc | 1 - ext/test/zpages/tracez_processor_test.cc | 1 - sdk/test/trace/always_off_sampler_test.cc | 1 - sdk/test/trace/always_on_sampler_test.cc | 1 - sdk/test/trace/attribute_utils_test.cc | 1 - sdk/test/trace/batch_span_processor_test.cc | 1 - sdk/test/trace/parent_or_else_sampler_test.cc | 1 - sdk/test/trace/probability_sampler_test.cc | 1 - sdk/test/trace/sampler_benchmark.cc | 1 - sdk/test/trace/simple_processor_test.cc | 1 - sdk/test/trace/span_data_test.cc | 1 - sdk/test/trace/tracer_provider_test.cc | 1 - sdk/test/trace/tracer_test.cc | 1 - 26 files changed, 6 insertions(+), 35 deletions(-) diff --git a/api/include/opentelemetry/context/runtime_context.h b/api/include/opentelemetry/context/runtime_context.h index 3df6df962b..43f6b160e6 100644 --- a/api/include/opentelemetry/context/runtime_context.h +++ b/api/include/opentelemetry/context/runtime_context.h @@ -45,8 +45,8 @@ class Token nostd::shared_ptr detacher_; }; -/** - * RuntimeContextStorage is used by RuntimeContext to store Context frames. +/** + * RuntimeContextStorage is used by RuntimeContext to store Context frames. * * Custom context management strategies can be implemented by deriving from * this class and passing an initialized RuntimeContextStorage object to @@ -78,7 +78,7 @@ class RuntimeContextStorage Token CreateToken(Context context) noexcept { return Token(context); } }; -/** +/** * Construct and return the default RuntimeContextStorage * @return a ThreadLocalContextStorage */ @@ -102,10 +102,7 @@ class RuntimeContext // Resets the context to a previous value stored in the // passed in token. Returns true if successful, false otherwise - static bool Detach(Token &token) noexcept - { - return GetRuntimeContextStorage()->Detach(token); - } + static bool Detach(Token &token) noexcept { return GetRuntimeContextStorage()->Detach(token); } // Sets the Key and Value into the passed in context or if a context is not // passed in, the RuntimeContext. @@ -147,14 +144,14 @@ class RuntimeContext return temp_context.GetValue(key); } - /** + /** * Provide a custom runtime context storage. * * This provides a possibility to override the default thread-local runtime * context storage. This has to be set before any spans are created by the * application, otherwise context state information will be discarded, which * might lead to unexpected results. - * + * * @param storage a custom runtime context storage */ static void SetRuntimeContextStorage(nostd::shared_ptr storage) noexcept diff --git a/api/test/context/runtime_context_test.cc b/api/test/context/runtime_context_test.cc index f9e229d57c..30ad420220 100644 --- a/api/test/context/runtime_context_test.cc +++ b/api/test/context/runtime_context_test.cc @@ -1,5 +1,4 @@ #include "opentelemetry/context/context.h" -#include "opentelemetry/context/threadlocal_context.h" #include diff --git a/api/test/plugin/dynamic_load_test.cc b/api/test/plugin/dynamic_load_test.cc index 2e630f6649..3e0e45ecc1 100644 --- a/api/test/plugin/dynamic_load_test.cc +++ b/api/test/plugin/dynamic_load_test.cc @@ -1,5 +1,4 @@ #include "opentelemetry/plugin/dynamic_load.h" -#include "opentelemetry/context/threadlocal_context.h" #include diff --git a/examples/batch/main.cc b/examples/batch/main.cc index e159984ddf..7d7fa57f36 100644 --- a/examples/batch/main.cc +++ b/examples/batch/main.cc @@ -4,8 +4,6 @@ #include "opentelemetry/exporters/ostream/span_exporter.h" #include "opentelemetry/sdk/trace/batch_span_processor.h" -#include "opentelemetry/context/threadlocal_context.h" - #include #include diff --git a/examples/otlp/foo_library/foo_library.cc b/examples/otlp/foo_library/foo_library.cc index fb79781635..fdbf5b892b 100644 --- a/examples/otlp/foo_library/foo_library.cc +++ b/examples/otlp/foo_library/foo_library.cc @@ -1,4 +1,3 @@ -#include "opentelemetry/context/threadlocal_context.h" #include "opentelemetry/trace/provider.h" namespace trace = opentelemetry::trace; diff --git a/examples/plugin/load/main.cc b/examples/plugin/load/main.cc index 5348838942..3c6799fed0 100644 --- a/examples/plugin/load/main.cc +++ b/examples/plugin/load/main.cc @@ -1,4 +1,3 @@ -#include "opentelemetry/context/threadlocal_context.h" #include "opentelemetry/plugin/dynamic_load.h" #include diff --git a/examples/simple/main.cc b/examples/simple/main.cc index 572c3be1a7..485d31c2ec 100644 --- a/examples/simple/main.cc +++ b/examples/simple/main.cc @@ -1,4 +1,3 @@ -#include "opentelemetry/context/threadlocal_context.h" #include "opentelemetry/sdk/trace/simple_processor.h" #include "opentelemetry/sdk/trace/tracer_provider.h" #include "opentelemetry/trace/provider.h" diff --git a/examples/zpages/zpages_example.cc b/examples/zpages/zpages_example.cc index f89f8f95d9..72ca3fda75 100644 --- a/examples/zpages/zpages_example.cc +++ b/examples/zpages/zpages_example.cc @@ -5,7 +5,6 @@ #include #include #include -#include "opentelemetry/context/threadlocal_context.h" #include "opentelemetry/ext/zpages/zpages.h" // Required file include for zpages diff --git a/exporters/ostream/test/ostream_span_test.cc b/exporters/ostream/test/ostream_span_test.cc index 5dfecde265..e1f0c6b937 100644 --- a/exporters/ostream/test/ostream_span_test.cc +++ b/exporters/ostream/test/ostream_span_test.cc @@ -1,4 +1,3 @@ -#include "opentelemetry/context/threadlocal_context.h" #include "opentelemetry/sdk/trace/recordable.h" #include "opentelemetry/sdk/trace/simple_processor.h" #include "opentelemetry/sdk/trace/span_data.h" diff --git a/exporters/otlp/test/otlp_exporter_benchmark.cc b/exporters/otlp/test/otlp_exporter_benchmark.cc index 30f9046e34..0accff0bf4 100644 --- a/exporters/otlp/test/otlp_exporter_benchmark.cc +++ b/exporters/otlp/test/otlp_exporter_benchmark.cc @@ -1,4 +1,3 @@ -#include "opentelemetry/context/threadlocal_context.h" #include "opentelemetry/exporters/otlp/otlp_exporter.h" #include "opentelemetry/exporters/otlp/recordable.h" diff --git a/exporters/otlp/test/otlp_exporter_test.cc b/exporters/otlp/test/otlp_exporter_test.cc index a79046f1d3..a221365179 100644 --- a/exporters/otlp/test/otlp_exporter_test.cc +++ b/exporters/otlp/test/otlp_exporter_test.cc @@ -1,5 +1,4 @@ #include "opentelemetry/exporters/otlp/otlp_exporter.h" -#include "opentelemetry/context/threadlocal_context.h" #include "opentelemetry/proto/collector/trace/v1/trace_service_mock.grpc.pb.h" #include "opentelemetry/sdk/trace/simple_processor.h" #include "opentelemetry/sdk/trace/tracer_provider.h" diff --git a/exporters/otlp/test/recordable_test.cc b/exporters/otlp/test/recordable_test.cc index a349c1f50b..8a3d07369c 100644 --- a/exporters/otlp/test/recordable_test.cc +++ b/exporters/otlp/test/recordable_test.cc @@ -1,6 +1,5 @@ #include "opentelemetry/exporters/otlp/recordable.h" #include -#include "opentelemetry/context/threadlocal_context.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace exporter diff --git a/ext/test/zpages/threadsafe_span_data_test.cc b/ext/test/zpages/threadsafe_span_data_test.cc index 12ad3e5a45..5a4791d527 100644 --- a/ext/test/zpages/threadsafe_span_data_test.cc +++ b/ext/test/zpages/threadsafe_span_data_test.cc @@ -1,5 +1,4 @@ #include "opentelemetry/ext/zpages/threadsafe_span_data.h" -#include "opentelemetry/context/threadlocal_context.h" #include "opentelemetry/nostd/variant.h" #include "opentelemetry/trace/span_id.h" #include "opentelemetry/trace/trace_id.h" diff --git a/ext/test/zpages/tracez_data_aggregator_test.cc b/ext/test/zpages/tracez_data_aggregator_test.cc index 64a873d2fb..1a1cc92dcc 100644 --- a/ext/test/zpages/tracez_data_aggregator_test.cc +++ b/ext/test/zpages/tracez_data_aggregator_test.cc @@ -2,7 +2,6 @@ #include -#include "opentelemetry/context/threadlocal_context.h" #include "opentelemetry/ext/zpages/tracez_processor.h" #include "opentelemetry/sdk/trace/recordable.h" #include "opentelemetry/sdk/trace/tracer.h" diff --git a/ext/test/zpages/tracez_processor_test.cc b/ext/test/zpages/tracez_processor_test.cc index ad356d880f..ca7b8b3738 100644 --- a/ext/test/zpages/tracez_processor_test.cc +++ b/ext/test/zpages/tracez_processor_test.cc @@ -1,5 +1,4 @@ #include "opentelemetry/ext/zpages/tracez_processor.h" -#include "opentelemetry/context/threadlocal_context.h" #include diff --git a/sdk/test/trace/always_off_sampler_test.cc b/sdk/test/trace/always_off_sampler_test.cc index 28a6c5fb3c..dec684f934 100644 --- a/sdk/test/trace/always_off_sampler_test.cc +++ b/sdk/test/trace/always_off_sampler_test.cc @@ -1,4 +1,3 @@ -#include "opentelemetry/context/threadlocal_context.h" #include "opentelemetry/sdk/trace/samplers/always_off.h" #include diff --git a/sdk/test/trace/always_on_sampler_test.cc b/sdk/test/trace/always_on_sampler_test.cc index 5f6443c096..a933cd0073 100644 --- a/sdk/test/trace/always_on_sampler_test.cc +++ b/sdk/test/trace/always_on_sampler_test.cc @@ -1,4 +1,3 @@ -#include "opentelemetry/context/threadlocal_context.h" #include "opentelemetry/nostd/span.h" #include "opentelemetry/sdk/trace/samplers/always_on.h" diff --git a/sdk/test/trace/attribute_utils_test.cc b/sdk/test/trace/attribute_utils_test.cc index a042fcd7d9..43d6fd048c 100644 --- a/sdk/test/trace/attribute_utils_test.cc +++ b/sdk/test/trace/attribute_utils_test.cc @@ -1,5 +1,4 @@ #include "opentelemetry/sdk/trace/attribute_utils.h" -#include "opentelemetry/context/threadlocal_context.h" #include diff --git a/sdk/test/trace/batch_span_processor_test.cc b/sdk/test/trace/batch_span_processor_test.cc index d240d06b81..8e9e43c735 100644 --- a/sdk/test/trace/batch_span_processor_test.cc +++ b/sdk/test/trace/batch_span_processor_test.cc @@ -1,5 +1,4 @@ #include "opentelemetry/sdk/trace/batch_span_processor.h" -#include "opentelemetry/context/threadlocal_context.h" #include "opentelemetry/sdk/trace/span_data.h" #include "opentelemetry/sdk/trace/tracer.h" diff --git a/sdk/test/trace/parent_or_else_sampler_test.cc b/sdk/test/trace/parent_or_else_sampler_test.cc index cf7a208aa8..b5eee8bc58 100644 --- a/sdk/test/trace/parent_or_else_sampler_test.cc +++ b/sdk/test/trace/parent_or_else_sampler_test.cc @@ -1,6 +1,5 @@ #include #include -#include "opentelemetry/context/threadlocal_context.h" #include "opentelemetry/sdk/trace/samplers/always_off.h" #include "opentelemetry/sdk/trace/samplers/always_on.h" #include "opentelemetry/sdk/trace/samplers/parent_or_else.h" diff --git a/sdk/test/trace/probability_sampler_test.cc b/sdk/test/trace/probability_sampler_test.cc index 6e617db3b1..5062fb3b3a 100644 --- a/sdk/test/trace/probability_sampler_test.cc +++ b/sdk/test/trace/probability_sampler_test.cc @@ -1,4 +1,3 @@ -#include "opentelemetry/context/threadlocal_context.h" #include "opentelemetry/sdk/trace/samplers/probability.h" #include "src/common/random.h" diff --git a/sdk/test/trace/sampler_benchmark.cc b/sdk/test/trace/sampler_benchmark.cc index 2068846a00..2dad8c8128 100644 --- a/sdk/test/trace/sampler_benchmark.cc +++ b/sdk/test/trace/sampler_benchmark.cc @@ -1,4 +1,3 @@ -#include "opentelemetry/context/threadlocal_context.h" #include "opentelemetry/sdk/trace/sampler.h" #include "opentelemetry/sdk/trace/samplers/always_off.h" #include "opentelemetry/sdk/trace/samplers/always_on.h" diff --git a/sdk/test/trace/simple_processor_test.cc b/sdk/test/trace/simple_processor_test.cc index b1da619e64..b4819aa0ae 100644 --- a/sdk/test/trace/simple_processor_test.cc +++ b/sdk/test/trace/simple_processor_test.cc @@ -1,5 +1,4 @@ #include "opentelemetry/sdk/trace/simple_processor.h" -#include "opentelemetry/context/threadlocal_context.h" #include "opentelemetry/nostd/span.h" #include "opentelemetry/sdk/trace/span_data.h" diff --git a/sdk/test/trace/span_data_test.cc b/sdk/test/trace/span_data_test.cc index a235d53938..a8cf244aed 100644 --- a/sdk/test/trace/span_data_test.cc +++ b/sdk/test/trace/span_data_test.cc @@ -1,5 +1,4 @@ #include "opentelemetry/sdk/trace/span_data.h" -#include "opentelemetry/context/threadlocal_context.h" #include "opentelemetry/nostd/variant.h" #include "opentelemetry/trace/span_id.h" #include "opentelemetry/trace/trace_id.h" diff --git a/sdk/test/trace/tracer_provider_test.cc b/sdk/test/trace/tracer_provider_test.cc index 5840e3f9e0..dca3bc3607 100644 --- a/sdk/test/trace/tracer_provider_test.cc +++ b/sdk/test/trace/tracer_provider_test.cc @@ -1,5 +1,4 @@ #include "opentelemetry/sdk/trace/tracer_provider.h" -#include "opentelemetry/context/threadlocal_context.h" #include "opentelemetry/sdk/trace/samplers/always_off.h" #include "opentelemetry/sdk/trace/samplers/always_on.h" #include "opentelemetry/sdk/trace/simple_processor.h" diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index 498d22e6fc..ba196c67c1 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -1,5 +1,4 @@ #include "opentelemetry/sdk/trace/tracer.h" -#include "opentelemetry/context/threadlocal_context.h" #include "opentelemetry/sdk/trace/samplers/always_off.h" #include "opentelemetry/sdk/trace/samplers/always_on.h" #include "opentelemetry/sdk/trace/samplers/parent_or_else.h" From 767ef52fda1649bab6371a6784118202d44ab51e Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Fri, 4 Sep 2020 10:14:18 -0700 Subject: [PATCH 03/11] Fix build --- api/test/context/CMakeLists.txt | 2 +- api/test/context/runtime_context_test.cc | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/api/test/context/CMakeLists.txt b/api/test/context/CMakeLists.txt index 38d48ef0e8..2edbbc6a86 100644 --- a/api/test/context/CMakeLists.txt +++ b/api/test/context/CMakeLists.txt @@ -1,6 +1,6 @@ include(GoogleTest) -foreach(testname context_test) +foreach(testname context_test runtime_context_test) add_executable(${testname} "${testname}.cc") target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} opentelemetry_api) diff --git a/api/test/context/runtime_context_test.cc b/api/test/context/runtime_context_test.cc index 30ad420220..7924294185 100644 --- a/api/test/context/runtime_context_test.cc +++ b/api/test/context/runtime_context_test.cc @@ -1,3 +1,4 @@ +#include "opentelemetry/context/runtime_context.h" #include "opentelemetry/context/context.h" #include From c91350286bae73a61a8e11f555bf5df303446216 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Fri, 4 Sep 2020 10:32:13 -0700 Subject: [PATCH 04/11] Add missing include --- api/include/opentelemetry/context/runtime_context.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/include/opentelemetry/context/runtime_context.h b/api/include/opentelemetry/context/runtime_context.h index 43f6b160e6..513ffcf99b 100644 --- a/api/include/opentelemetry/context/runtime_context.h +++ b/api/include/opentelemetry/context/runtime_context.h @@ -1,5 +1,7 @@ #pragma once +#include + #include "opentelemetry/context/context.h" OPENTELEMETRY_BEGIN_NAMESPACE From f7f2fbdfba9dc4190cd80fcbcfc9754eb77e2696 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Mon, 14 Sep 2020 16:13:19 -0700 Subject: [PATCH 05/11] PR comments --- api/include/opentelemetry/context/runtime_context.h | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/api/include/opentelemetry/context/runtime_context.h b/api/include/opentelemetry/context/runtime_context.h index 513ffcf99b..6658d497ba 100644 --- a/api/include/opentelemetry/context/runtime_context.h +++ b/api/include/opentelemetry/context/runtime_context.h @@ -239,19 +239,18 @@ class ThreadLocalContextStorage : public RuntimeContextStorage // Pops the top Context off the stack and returns it. Context Pop() noexcept { - if (size_ <= 0) + if (size_ == 0) { return Context(); } - int index = size_ - 1; - size_--; - return base_[index]; + size -= 1; + return base_[size]; } // Returns the Context at the top of the stack. Context Top() const noexcept { - if (size_ <= 0) + if (size_ == 0) { return Context(); } From 121cc3edacfac641b35c7f1537438c8daef9c71c Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Mon, 14 Sep 2020 16:21:18 -0700 Subject: [PATCH 06/11] PR comments --- api/include/opentelemetry/context/runtime_context.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/include/opentelemetry/context/runtime_context.h b/api/include/opentelemetry/context/runtime_context.h index 6658d497ba..b7223aed05 100644 --- a/api/include/opentelemetry/context/runtime_context.h +++ b/api/include/opentelemetry/context/runtime_context.h @@ -243,8 +243,8 @@ class ThreadLocalContextStorage : public RuntimeContextStorage { return Context(); } - size -= 1; - return base_[size]; + size_ -= 1; + return base_[size_]; } // Returns the Context at the top of the stack. From 130ff0a6246937c08ba977809d6c7df2d06ccaa7 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Tue, 15 Sep 2020 09:07:26 -0700 Subject: [PATCH 07/11] Merge --- api/test/trace/scope_test.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/api/test/trace/scope_test.cc b/api/test/trace/scope_test.cc index 5c3ae25897..7c68d94abe 100644 --- a/api/test/trace/scope_test.cc +++ b/api/test/trace/scope_test.cc @@ -1,6 +1,5 @@ #include "opentelemetry/trace/scope.h" #include "opentelemetry/context/context.h" -#include "opentelemetry/context/threadlocal_context.h" #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/trace/noop.h" From 06d799c900889e423c3050239a5245fad051e5b5 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Thu, 17 Sep 2020 13:38:18 -0700 Subject: [PATCH 08/11] Remove locking around GetRuntimeContextStorage --- .../opentelemetry/context/runtime_context.h | 27 +++++-------------- 1 file changed, 6 insertions(+), 21 deletions(-) diff --git a/api/include/opentelemetry/context/runtime_context.h b/api/include/opentelemetry/context/runtime_context.h index b7223aed05..8b5d5bb1c3 100644 --- a/api/include/opentelemetry/context/runtime_context.h +++ b/api/include/opentelemetry/context/runtime_context.h @@ -151,28 +151,19 @@ class RuntimeContext * * This provides a possibility to override the default thread-local runtime * context storage. This has to be set before any spans are created by the - * application, otherwise context state information will be discarded, which - * might lead to unexpected results. + * application, otherwise the behavior is undefined. * * @param storage a custom runtime context storage */ static void SetRuntimeContextStorage(nostd::shared_ptr storage) noexcept { - while (GetLock().test_and_set(std::memory_order_acquire)) - ; GetStorage() = storage; - GetLock().clear(std::memory_order_release); } private: static nostd::shared_ptr GetRuntimeContextStorage() noexcept { - while (GetLock().test_and_set(std::memory_order_acquire)) - ; - auto storage = nostd::shared_ptr(GetStorage()); - GetLock().clear(std::memory_order_release); - - return storage; + return GetStorage(); } static nostd::shared_ptr &GetStorage() noexcept @@ -180,12 +171,6 @@ class RuntimeContext static nostd::shared_ptr context(GetDefaultStorage()); return context; } - - static std::atomic_flag &GetLock() noexcept - { - static std::atomic_flag lock = ATOMIC_FLAG_INIT; - return lock; - } }; inline Token::ContextDetacher::~ContextDetacher() @@ -195,10 +180,10 @@ inline Token::ContextDetacher::~ContextDetacher() context::RuntimeContext::Detach(token); } -// The ThreadLocalContextStorage class is a derived class from RuntimeContext and -// provides a wrapper for propogating context through cpp thread locally. -// This file must be included to use the RuntimeContext class if another -// implementation has not been registered. +// The ThreadLocalContextStorage class is a derived class from +// RuntimeContextStorage and provides a wrapper for propogating context through +// cpp thread locally. This file must be included to use the RuntimeContext +// class if another implementation has not been registered. class ThreadLocalContextStorage : public RuntimeContextStorage { public: From 5ef902e9bc7342eff8d9519e4758bd2b05a1c6f3 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Thu, 17 Sep 2020 13:44:30 -0700 Subject: [PATCH 09/11] Format --- api/include/opentelemetry/context/runtime_context.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/include/opentelemetry/context/runtime_context.h b/api/include/opentelemetry/context/runtime_context.h index 8b5d5bb1c3..a0ef10b686 100644 --- a/api/include/opentelemetry/context/runtime_context.h +++ b/api/include/opentelemetry/context/runtime_context.h @@ -180,7 +180,7 @@ inline Token::ContextDetacher::~ContextDetacher() context::RuntimeContext::Detach(token); } -// The ThreadLocalContextStorage class is a derived class from +// The ThreadLocalContextStorage class is a derived class from // RuntimeContextStorage and provides a wrapper for propogating context through // cpp thread locally. This file must be included to use the RuntimeContext // class if another implementation has not been registered. From 934322bb53c2947acc8de5358a70d94f897fd659 Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Thu, 17 Sep 2020 14:08:09 -0700 Subject: [PATCH 10/11] Remove obsolete header --- api/include/opentelemetry/context/runtime_context.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/api/include/opentelemetry/context/runtime_context.h b/api/include/opentelemetry/context/runtime_context.h index a0ef10b686..b987c7f98f 100644 --- a/api/include/opentelemetry/context/runtime_context.h +++ b/api/include/opentelemetry/context/runtime_context.h @@ -1,7 +1,5 @@ #pragma once -#include - #include "opentelemetry/context/context.h" OPENTELEMETRY_BEGIN_NAMESPACE From 97e7d42cdf0c942dee259c8b2dccbe528b5089cc Mon Sep 17 00:00:00 2001 From: Johannes Tax Date: Mon, 21 Sep 2020 10:37:56 -0700 Subject: [PATCH 11/11] Add valgrind suppressions for grpc flakiness --- ci/do_ci.sh | 2 +- ci/valgrind-suppressions | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 ci/valgrind-suppressions diff --git a/ci/do_ci.sh b/ci/do_ci.sh index e97686b0b7..493e31f554 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -125,7 +125,7 @@ elif [[ "$1" == "bazel.tsan" ]]; then exit 0 elif [[ "$1" == "bazel.valgrind" ]]; then bazel build $BAZEL_OPTIONS //... - bazel test --run_under="/usr/bin/valgrind --leak-check=full --error-exitcode=1" $BAZEL_TEST_OPTIONS //... + bazel test --run_under="/usr/bin/valgrind --leak-check=full --error-exitcode=1 --suppressions=\"${SRC_DIR}/ci/valgrind-suppressions\"" $BAZEL_TEST_OPTIONS //... exit 0 elif [[ "$1" == "benchmark" ]]; then [ -z "${BENCHMARK_DIR}" ] && export BENCHMARK_DIR=$HOME/benchmark diff --git a/ci/valgrind-suppressions b/ci/valgrind-suppressions new file mode 100644 index 0000000000..28741e7a17 --- /dev/null +++ b/ci/valgrind-suppressions @@ -0,0 +1,15 @@ +{ + + Memcheck:Leak + ... + fun:grpc_shutdown + ... +} + +{ + + Memcheck:Leak + ... + fun:grpc_init + ... +}