From 061e56486185ebcdf4e5d94345f94b15a33cd141 Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Wed, 10 Jun 2020 17:48:20 +0000 Subject: [PATCH 1/2] reversed public/private order --- api/include/opentelemetry/context/context.h | 80 ++++++++++----------- 1 file changed, 39 insertions(+), 41 deletions(-) diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index 222872274c..f0241a20f2 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -16,19 +16,6 @@ namespace context /*The context class provides a context identifier*/ class Context{ - private: - - /*The identifier itself*/ - std::map ctx_map_; - - /*Used to track that last ContextKey identifier and create the next one */ - static int last_key_identifier_; - - /* Context: A constructor that accepts a key/value map*/ - Context(std::map ctx_map){ - ctx_map_ = ctx_map; - } - public: /*The ContextKey class is used to obscure access from the @@ -49,8 +36,8 @@ namespace context return identifier_; } - /* ContextKey: constructs a new ContextKey with the - * passed in name and identifier. + /* Constructs a new ContextKey with the passed in name and + * identifier. */ ContextKey(std::string key_name, int identifier){ key_name_ = key_name; @@ -59,9 +46,8 @@ namespace context public: - /* ContextKey: Consructs a new ContextKey with the passed in name - * and increments the identifier then assigns it to be the key's - * identifier. + /* Consructs a new ContextKey with the passed in name and increments + * the identifier then assigns it to be the key's identifier. */ ContextKey(std::string key_name){ key_name_ = key_name; @@ -78,14 +64,14 @@ namespace context }; - /* Context: contructor, creates a context object with no key/value pairs + /* Creates a context object with no key/value pairs */ Context(){ ctx_map_ = std::map {}; } - /* Context: contructor, creates a context object from a map + /* Contructor, creates a context object from a map * of keys and identifiers */ Context(ContextKey key, int value){ @@ -93,7 +79,7 @@ namespace context } - /* WriteValue: accepts a new key/value pair and then returns a new + /* Accepts a new key/value pair and then returns a new * context that contains both the original pairs and the new pair. */ Context WriteValue(ContextKey key, int value){ @@ -105,13 +91,12 @@ namespace context } - /* GetValue: Returns the value associated with the passed in key - */ + /* Returns the value associated with the passed in key */ int GetValue(ContextKey key){ return ctx_map_[key.GetIdentifier()]; } - /* CreateKey: Returns a ContextKey that has the passed in name and the + /* Returns a ContextKey that has the passed in name and the * next available identifier.*/ ContextKey CreateKey(std::string key_name){ int id; @@ -127,6 +112,19 @@ namespace context return ContextKey(key_name,id); } + private: + + /*The identifier itself*/ + std::map ctx_map_; + + /*Used to track that last ContextKey identifier and create the next one */ + static int last_key_identifier_; + + /* A constructor that accepts a key/value map*/ + Context(std::map ctx_map){ + ctx_map_ = ctx_map; + } + }; @@ -137,53 +135,48 @@ namespace context * objects.*/ class Token{ - private: - - Context ctx_; public: - /* Token: A constructor that sets the token's Context object to the + /* A constructor that sets the token's Context object to the * one that was passed in. */ Token(Context &ctx){ ctx_ = ctx; } - /* GetContext: Returns the stored context object */ + /* Returns the stored context object */ Context GetContext(){ return ctx_; } + private: + + Context ctx_; }; /* The RuntimeContext class provides a wrapper for * propogating context through cpp*/ class RuntimeContext { - private: - - static thread_local Context context_; public: - /* RuntimeContext: A default constructor that will set the context to + /* A default constructor that will set the context to * an empty context object. */ RuntimeContext(){ context_ = Context(); } - /* RuntimeContext: A constructor that will set the context as - * the passed in context. - */ + /* A constructor that will set the context as the passed in context.*/ RuntimeContext(Context &context){ context_ = context; } - /* attach: Sets the current 'Context' object. Returns a token + /* Sets the current 'Context' object. Returns a token * that can be used to reset to the previous Context. */ Token Attach(Context &context){ @@ -197,7 +190,7 @@ namespace context } - /* GetCurrent: Return the current context. + /* Return the current context. */ static Context GetCurrent(){ Context context = context_; @@ -205,13 +198,18 @@ namespace context } - /* Detach: Resets the context to a previous value stored in the - * passed in token. + /* Resets the context to a previous value stored in the + * passed in token. */ - void Detach(Token &token){ + int Detach(Token &token){ context_ = token.GetContext(); } + + private: + + static thread_local Context context_; + }; thread_local Context RuntimeContext::context_ = Context(); From b2717b31ae3ed37a4a89d91574c2192abab3b434 Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Wed, 10 Jun 2020 18:16:15 +0000 Subject: [PATCH 2/2] Added detach test --- api/include/opentelemetry/context/context.h | 43 ++++++++++++++------- api/test/context/runtimeContext_test.cc | 4 +- 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index 3d03ed1a09..ddb6227095 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -1,6 +1,5 @@ #pragma once - #include #include #include @@ -13,7 +12,7 @@ namespace context std::mutex context_id_mutex; - /*The context class provides a context identifier*/ + /*The context class provides a context identifier */ class Context{ public: @@ -31,7 +30,7 @@ namespace context int identifier_; - /* GetIdentifier: returns the identifier*/ + /* GetIdentifier: returns the identifier */ int GetIdentifier(){ return identifier_; } @@ -64,8 +63,7 @@ namespace context }; - /* Creates a context object with no key/value pairs - */ + /* Creates a context object with no key/value pairs */ Context(){ ctx_map_ = std::map {}; @@ -90,6 +88,15 @@ namespace context return Context(temp_map); } + /* Class comparator to see if the context maps are the same. */ + bool operator == (const Context &context){ + if(context.ctx_map_ == ctx_map_){ + return true; + } + else{ + return false; + } + } /* Returns the value associated with the passed in key */ int GetValue(ContextKey key){ @@ -114,13 +121,13 @@ namespace context private: - /*The identifier itself*/ + /* The identifier itself */ std::map ctx_map_; /*Used to track that last ContextKey identifier and create the next one */ static int last_key_identifier_; - /* A constructor that accepts a key/value map*/ + /* A constructor that accepts a key/value map */ Context(std::map ctx_map){ ctx_map_ = ctx_map; } @@ -132,7 +139,8 @@ namespace context /* The token class provides an identifier that is used by * the attach and detach methods to keep track of context - * objects.*/ + * objects. + */ class Token{ @@ -157,7 +165,7 @@ namespace context /* The RuntimeContext class provides a wrapper for - * propogating context through cpp*/ + * propogating context through cpp. */ class RuntimeContext { public: @@ -170,7 +178,7 @@ namespace context context_ = Context(); } - /* A constructor that will set the context as the passed in context.*/ + /* A constructor that will set the context as the passed in context. */ RuntimeContext(Context &context){ context_ = context; } @@ -188,8 +196,7 @@ namespace context } - /* Return the current context. - */ + /* Return the current context. */ static Context GetCurrent(){ Context context = context_; return context_; @@ -197,10 +204,18 @@ namespace context /* Resets the context to a previous value stored in the - * passed in token. + * passed in token. Returns zero if successful, -1 otherwise */ int Detach(Token &token){ - context_ = token.GetContext(); + + if(token.GetContext() == context_){ + + return -1; + } + + context_ = token.GetContext(); + + return 0; } diff --git a/api/test/context/runtimeContext_test.cc b/api/test/context/runtimeContext_test.cc index 682443cf48..e283071c1d 100644 --- a/api/test/context/runtimeContext_test.cc +++ b/api/test/context/runtimeContext_test.cc @@ -49,8 +49,10 @@ TEST(runtimeContext_test, attach_detach_context) EXPECT_EQ(test_runtime.GetCurrent().GetValue(foo_key), foo_context.GetValue(foo_key)); - test_runtime.Detach(test_token); + int detach_result = test_runtime.Detach(test_token); + EXPECT_EQ(detach_result, 0); + EXPECT_EQ(test_runtime.GetCurrent().GetValue(test_key), test_context.GetValue(test_key)); EXPECT_NE(test_runtime.GetCurrent().GetValue(foo_key),