From 4128a69bf191ed9ac4fa3b96ed67ce32c0bd37ff Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Mon, 6 Jul 2020 19:35:55 +0000 Subject: [PATCH 01/31] added context header with dummy methods --- api/include/opentelemetry/context/TBD | 0 api/include/opentelemetry/context/context.h | 69 +++++++++++++++++++++ api/test/context/context_test.cc | 63 +++++++++++++++++++ 3 files changed, 132 insertions(+) delete mode 100644 api/include/opentelemetry/context/TBD create mode 100644 api/include/opentelemetry/context/context.h create mode 100644 api/test/context/context_test.cc diff --git a/api/include/opentelemetry/context/TBD b/api/include/opentelemetry/context/TBD deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h new file mode 100644 index 0000000000..35990e93e8 --- /dev/null +++ b/api/include/opentelemetry/context/context.h @@ -0,0 +1,69 @@ +#pragma once + +#include "opentelemetry/common/attribute_value.h" +#include "opentelemetry/nostd/string_view.h" +#include "opentelemetry/trace/key_value_iterable_view.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace context +{ + +// The context class provides a context identifier. +// This is a dummy class that is meant to be overridden, +// the methods return default values. +class Context +{ + +public: + // The Key class is used to obscure access from the + // user to the context map. The identifier is used as a key + // to the context map. + class Key + { + + public: + // Returns the key's identifier. + nostd::string_view GetIdentifier() { return ""; } + + // Returns the key's name + nostd::string_view GetName() { return ""; } + + Key() = default; + + // Constructs a new Key with the passed in name. Sets the identifier as + // the address of this object. + Key(nostd::string_view key_name) {} + + private: + friend class Context; + }; + + // Creates a key with the passed in name and returns it. + Key CreateKey(nostd::string_view key_name) { return Key(key_name); } + + Context() = default; + + // Contructor, creates a context object from a map of keys + // and identifiers. + template ::value> * = nullptr> + Context(const T &attributes) + {} + + // Accepts a new iterable and then returns a new context that + // contains both the original pairs and the new pair. + template ::value> * = nullptr> + Context WriteValues(T &attributes) noexcept + { + return Context(attributes); + } + + // Returns the value associated with the passed in key. + virtual common::AttributeValue GetValue(Key key) { return 0; } + + // Copy Constructors. + Context(const Context &other) = default; + Context &operator=(const Context &other) = default; +}; + +} // namespace context +OPENTELEMETRY_END_NAMESPACE diff --git a/api/test/context/context_test.cc b/api/test/context/context_test.cc new file mode 100644 index 0000000000..5ed00f81a8 --- /dev/null +++ b/api/test/context/context_test.cc @@ -0,0 +1,63 @@ +#include "opentelemetry/context/context.h" + +#include + +#include + +using namespace opentelemetry; + +// Tests that the context default constructor does not cause a crash +TEST(ContextTest, ContextDefaultConstructorCrash) +{ + context::Context context_test = context::Context(); +} + +// Tests that the context constructor does not cause a crash when a map +// is provided. +TEST(ContextTest, ContextIterableConstructorCrash) +{ + std::map map_test = {{"test_key", "123"}}; + context::Context context_test = context::Context(map_test); +} + +// Tests that the WriteValues method does not crash +TEST(ContextTest, ContextWriteValuesCrash) +{ + std::map map_test = {{"test_key", "123"}}; + std::map map_test_write = {{"foo_key", "456"}}; + context::Context context_test = context::Context(map_test); + context_test.WriteValues(map_test_write); +} + +// Tests that the CreateKey method does not crash +TEST(ContextTest, ContextCreateKeyCrash) +{ + context::Context context_test; + context::Context::Key test_key = context_test.CreateKey("key_name"); +} + +// Tests that the GetValue method does not crash +TEST(ContextTest, ContextGetValueCrash) +{ + std::map map_test = {{"test_key", "123"}}; + std::map map_test_write = {{"foo_key", "456"}}; + context::Context context_test = context::Context(map_test); + context::Context::Key test_key = context_test.CreateKey("key_name"); + EXPECT_EQ(nostd::get(context_test.GetValue(test_key)), 0); +} + +// Tests that the Context::Key GetIdentifier method does not crash +TEST(ContextTest, ContextKeyGetIdentifierCrash) +{ + context::Context context_test; + context::Context::Key test_key = context_test.CreateKey("key_name"); + EXPECT_EQ(test_key.GetIdentifier(), ""); +} + +// Tests that the Context::Key GetName method does not crash +TEST(ContextTest, ContextKeyGetNameCrash) +{ + context::Context context_test; + context::Context::Key test_key = context_test.CreateKey("key_name"); + EXPECT_EQ(test_key.GetName(), ""); +} From cba6d5f6d23acb27fb4d4b9ecbc1e32c887d8d3d Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Mon, 6 Jul 2020 19:40:31 +0000 Subject: [PATCH 02/31] Build and CMake files --- api/test/context/BUILD | 11 +++++++++++ api/test/context/CMakeLists.txt | 8 ++++++++ 2 files changed, 19 insertions(+) create mode 100644 api/test/context/BUILD create mode 100644 api/test/context/CMakeLists.txt diff --git a/api/test/context/BUILD b/api/test/context/BUILD new file mode 100644 index 0000000000..22ad910f0f --- /dev/null +++ b/api/test/context/BUILD @@ -0,0 +1,11 @@ +cc_test( + name = "context_test", + srcs = [ + "context_test.cc", + ], + deps = [ + "//api", + "@com_google_googletest//:gtest_main", + ], +) + diff --git a/api/test/context/CMakeLists.txt b/api/test/context/CMakeLists.txt new file mode 100644 index 0000000000..cb7786ade3 --- /dev/null +++ b/api/test/context/CMakeLists.txt @@ -0,0 +1,8 @@ +include(GoogleTest) + +foreach(testname context_test ) + add_executable(${testname} "${testname}.cc") + target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES} + ${CMAKE_THREAD_LIBS_INIT} opentelemetry_api) + gtest_add_tests(TARGET ${testname} TEST_PREFIX context. TEST_LIST ${testname}) +endforeach() From b2e876eda4c09e75564220910ca6f4296f6adc39 Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Mon, 6 Jul 2020 19:43:11 +0000 Subject: [PATCH 03/31] auto formatted --- api/test/context/BUILD | 1 - api/test/context/CMakeLists.txt | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/api/test/context/BUILD b/api/test/context/BUILD index 22ad910f0f..26728542b8 100644 --- a/api/test/context/BUILD +++ b/api/test/context/BUILD @@ -8,4 +8,3 @@ cc_test( "@com_google_googletest//:gtest_main", ], ) - diff --git a/api/test/context/CMakeLists.txt b/api/test/context/CMakeLists.txt index cb7786ade3..38d48ef0e8 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) add_executable(${testname} "${testname}.cc") target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT} opentelemetry_api) From a7480c81cde002bbcbeb31451885cc68bc250ffd Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Thu, 9 Jul 2020 20:03:37 +0000 Subject: [PATCH 04/31] changed variable name from attributes to values --- api/include/opentelemetry/context/context.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index 35990e93e8..478f9324a2 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -46,15 +46,15 @@ class Context // Contructor, creates a context object from a map of keys // and identifiers. template ::value> * = nullptr> - Context(const T &attributes) + Context(const T values) {} // Accepts a new iterable and then returns a new context that // contains both the original pairs and the new pair. template ::value> * = nullptr> - Context WriteValues(T &attributes) noexcept + Context WriteValues(T &values) noexcept { - return Context(attributes); + return Context(values); } // Returns the value associated with the passed in key. From 96cd32f712da569ae4fe7c7ae4c89bc475b4cdb5 Mon Sep 17 00:00:00 2001 From: Sam Atac <65615762+satac2@users.noreply.github.com> Date: Thu, 9 Jul 2020 15:06:30 -0500 Subject: [PATCH 05/31] Update api/include/opentelemetry/context/context.h Co-authored-by: Reiley Yang --- api/include/opentelemetry/context/context.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index 478f9324a2..4c421c469d 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -49,7 +49,7 @@ class Context Context(const T values) {} - // Accepts a new iterable and then returns a new context that + // Accepts a new iterable and then returns a new context that // contains both the original pairs and the new pair. template ::value> * = nullptr> Context WriteValues(T &values) noexcept From 5f0634bed72bd93a56555f3291d0bc99220f2879 Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Mon, 13 Jul 2020 04:02:35 +0000 Subject: [PATCH 06/31] changed to follow the spec and added context_value type --- api/include/opentelemetry/context/context.h | 117 ++++++++++-------- .../opentelemetry/context/context_value.h | 28 +++++ api/test/context/context_test.cc | 70 +++++------ 3 files changed, 124 insertions(+), 91 deletions(-) create mode 100644 api/include/opentelemetry/context/context_value.h diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index 478f9324a2..9ac35c226c 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -1,69 +1,78 @@ #pragma once #include "opentelemetry/common/attribute_value.h" +#include "opentelemetry/context/context_value.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/trace/key_value_iterable_view.h" +#include +#include + OPENTELEMETRY_BEGIN_NAMESPACE namespace context { -// The context class provides a context identifier. -// This is a dummy class that is meant to be overridden, -// the methods return default values. -class Context -{ - -public: - // The Key class is used to obscure access from the - // user to the context map. The identifier is used as a key - // to the context map. - class Key + // The context class provides a context identifier. + // This is a dummy class that is meant to be overridden, + // the methods return default values. + class Context { - public: - // Returns the key's identifier. - nostd::string_view GetIdentifier() { return ""; } - - // Returns the key's name - nostd::string_view GetName() { return ""; } - - Key() = default; - - // Constructs a new Key with the passed in name. Sets the identifier as - // the address of this object. - Key(nostd::string_view key_name) {} - - private: - friend class Context; + public: + + Context() = default; + + // Contructor, creates a context object from a map of keys + // and identifiers. + template ::value> * = nullptr> + Context(const T keys_and_values) + { + trace::KeyValueIterableView iterable{keys_and_values}; + iterable.ForEachKeyValue([&](nostd::string_view key, context::ContextValue value) noexcept { + context_map_[std::string(key)] = value; + return true; + }); + + } + + // Accepts a key and a value and then returns a new context that + // contains both the original pairs and the new pair. + template + Context SetValue(nostd::string_view key, T &value) noexcept + { + std::map context_map_copy = context_map_; + context_map_copy[std::string(key)] = value; + return Context(context_map_copy); + } + + // Accepts a new iterable and then returns a new context that + // contains both the original pairs and the new pair. + template ::value> * = nullptr> + Context SetValues(T &keys_and_values) noexcept + { + std::map context_map_copy = context_map_; + trace::KeyValueIterableView iterable{keys_and_values}; + + iterable.ForEachKeyValue([&](nostd::string_view key, context::ContextValue value) noexcept { + context_map_copy[std::string(key)] = value; + return true; + }); + + return Context(context_map_copy); + } + + // Returns the value associated with the passed in key. + context::ContextValue GetValue(nostd::string_view key) { + return context_map_[std::string(key)]; + } + + // Copy Constructors. + Context(const Context &other) = default; + Context &operator=(const Context &other) = default; + + private: + + std::map context_map_; }; - - // Creates a key with the passed in name and returns it. - Key CreateKey(nostd::string_view key_name) { return Key(key_name); } - - Context() = default; - - // Contructor, creates a context object from a map of keys - // and identifiers. - template ::value> * = nullptr> - Context(const T values) - {} - - // Accepts a new iterable and then returns a new context that - // contains both the original pairs and the new pair. - template ::value> * = nullptr> - Context WriteValues(T &values) noexcept - { - return Context(values); - } - - // Returns the value associated with the passed in key. - virtual common::AttributeValue GetValue(Key key) { return 0; } - - // Copy Constructors. - Context(const Context &other) = default; - Context &operator=(const Context &other) = default; -}; - } // namespace context OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/context/context_value.h b/api/include/opentelemetry/context/context_value.h new file mode 100644 index 0000000000..e4b966a4d9 --- /dev/null +++ b/api/include/opentelemetry/context/context_value.h @@ -0,0 +1,28 @@ +#pragma once + +#include + +#include "opentelemetry/nostd/span.h" +#include "opentelemetry/nostd/string_view.h" +#include "opentelemetry/nostd/variant.h" +#include "opentelemetry/version.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace context +{ + using ContextValue = nostd::variant, + nostd::span, + nostd::span, + nostd::span, + nostd::span, + nostd::span, + nostd::span>; +} // namespace common +OPENTELEMETRY_END_NAMESPACE diff --git a/api/test/context/context_test.cc b/api/test/context/context_test.cc index 5ed00f81a8..065be7f5be 100644 --- a/api/test/context/context_test.cc +++ b/api/test/context/context_test.cc @@ -6,58 +6,54 @@ using namespace opentelemetry; -// Tests that the context default constructor does not cause a crash -TEST(ContextTest, ContextDefaultConstructorCrash) -{ - context::Context context_test = context::Context(); -} -// Tests that the context constructor does not cause a crash when a map -// is provided. -TEST(ContextTest, ContextIterableConstructorCrash) +// Tests that the context constructor accepts an std::map +TEST(ContextTest, ContextIterableAcceptsMap) { - std::map map_test = {{"test_key", "123"}}; + std::map map_test = {{"test_key", "123"}}; context::Context context_test = context::Context(map_test); } -// Tests that the WriteValues method does not crash -TEST(ContextTest, ContextWriteValuesCrash) +// Tests that the GetValue method returns the expected value +TEST(ContextTest, ContextGetValueReturnsExpectedValue) { - std::map map_test = {{"test_key", "123"}}; - std::map map_test_write = {{"foo_key", "456"}}; + std::map map_test = {{"test_key", "123"},{"foo_key", "456"}}; + context::Context context_test = context::Context(map_test); - context_test.WriteValues(map_test_write); + EXPECT_EQ(nostd::get(context_test.GetValue("test_key")), "123"); + EXPECT_EQ(nostd::get(context_test.GetValue("foo_key")), "456"); } -// Tests that the CreateKey method does not crash -TEST(ContextTest, ContextCreateKeyCrash) +// Tests that the SetValues method accepts an std::map +TEST(ContextTest, ContextSetValuesAcceptsMap) { - context::Context context_test; - context::Context::Key test_key = context_test.CreateKey("key_name"); -} - -// Tests that the GetValue method does not crash -TEST(ContextTest, ContextGetValueCrash) -{ - std::map map_test = {{"test_key", "123"}}; - std::map map_test_write = {{"foo_key", "456"}}; + std::map map_test = {{"test_key", "123"}}; + std::map map_test_write = {{"foo_key", "456"}}; context::Context context_test = context::Context(map_test); - context::Context::Key test_key = context_test.CreateKey("key_name"); - EXPECT_EQ(nostd::get(context_test.GetValue(test_key)), 0); + context::Context foo_test = context_test.SetValues(map_test_write); + EXPECT_EQ(nostd::get(foo_test.GetValue("test_key")), "123"); + EXPECT_EQ(nostd::get(foo_test.GetValue("foo_key")), "456"); } -// Tests that the Context::Key GetIdentifier method does not crash -TEST(ContextTest, ContextKeyGetIdentifierCrash) +// Tests that the SetValues method accepts a nostd::string_view and +// context::ContextValue +TEST(ContextTest, ContextSetValuesAcceptsStringViewContextValue) { - context::Context context_test; - context::Context::Key test_key = context_test.CreateKey("key_name"); - EXPECT_EQ(test_key.GetIdentifier(), ""); + nostd::string_view string_view_test = "string_view"; + context::ContextValue context_value_test = "123"; + context::Context context_test = context::Context(); + context::Context foo_test = context_test.SetValue(string_view_test, context_value_test); + EXPECT_EQ(nostd::get(foo_test.GetValue(string_view_test)), "123"); } -// Tests that the Context::Key GetName method does not crash -TEST(ContextTest, ContextKeyGetNameCrash) +// Tests that the original context does not change when a value is +// written to it +TEST(ContextTest, ContextImmutability) { - context::Context context_test; - context::Context::Key test_key = context_test.CreateKey("key_name"); - EXPECT_EQ(test_key.GetName(), ""); + std::map map_test = {{"test_key", "123"}}; + context::Context context_test = context::Context(map_test); + + context::Context context_foo = context_test.SetValue("foo_key", "456"); + + EXPECT_THROW(nostd::get(context_test.GetValue("foo_key")), nostd::bad_variant_access); } From 68a1a70a57f33a6c2996dcaf589dd13c2d883593 Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Mon, 13 Jul 2020 14:14:54 +0000 Subject: [PATCH 07/31] removed tests for this PR --- api/test/context/context_test.cc | 59 -------------------------------- 1 file changed, 59 deletions(-) delete mode 100644 api/test/context/context_test.cc diff --git a/api/test/context/context_test.cc b/api/test/context/context_test.cc deleted file mode 100644 index 065be7f5be..0000000000 --- a/api/test/context/context_test.cc +++ /dev/null @@ -1,59 +0,0 @@ -#include "opentelemetry/context/context.h" - -#include - -#include - -using namespace opentelemetry; - - -// Tests that the context constructor accepts an std::map -TEST(ContextTest, ContextIterableAcceptsMap) -{ - std::map map_test = {{"test_key", "123"}}; - context::Context context_test = context::Context(map_test); -} - -// Tests that the GetValue method returns the expected value -TEST(ContextTest, ContextGetValueReturnsExpectedValue) -{ - std::map map_test = {{"test_key", "123"},{"foo_key", "456"}}; - - context::Context context_test = context::Context(map_test); - EXPECT_EQ(nostd::get(context_test.GetValue("test_key")), "123"); - EXPECT_EQ(nostd::get(context_test.GetValue("foo_key")), "456"); -} - -// Tests that the SetValues method accepts an std::map -TEST(ContextTest, ContextSetValuesAcceptsMap) -{ - std::map map_test = {{"test_key", "123"}}; - std::map map_test_write = {{"foo_key", "456"}}; - context::Context context_test = context::Context(map_test); - context::Context foo_test = context_test.SetValues(map_test_write); - EXPECT_EQ(nostd::get(foo_test.GetValue("test_key")), "123"); - EXPECT_EQ(nostd::get(foo_test.GetValue("foo_key")), "456"); -} - -// Tests that the SetValues method accepts a nostd::string_view and -// context::ContextValue -TEST(ContextTest, ContextSetValuesAcceptsStringViewContextValue) -{ - nostd::string_view string_view_test = "string_view"; - context::ContextValue context_value_test = "123"; - context::Context context_test = context::Context(); - context::Context foo_test = context_test.SetValue(string_view_test, context_value_test); - EXPECT_EQ(nostd::get(foo_test.GetValue(string_view_test)), "123"); -} - -// Tests that the original context does not change when a value is -// written to it -TEST(ContextTest, ContextImmutability) -{ - std::map map_test = {{"test_key", "123"}}; - context::Context context_test = context::Context(map_test); - - context::Context context_foo = context_test.SetValue("foo_key", "456"); - - EXPECT_THROW(nostd::get(context_test.GetValue("foo_key")), nostd::bad_variant_access); -} From 5e685743b98d65e6c909def3f1c3ad6bf34192de Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Mon, 13 Jul 2020 14:44:59 +0000 Subject: [PATCH 08/31] removed line --- api/include/opentelemetry/context/context.h | 1 - 1 file changed, 1 deletion(-) diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index 9ac35c226c..e5745c9a10 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -32,7 +32,6 @@ namespace context context_map_[std::string(key)] = value; return true; }); - } // Accepts a key and a value and then returns a new context that From 16a492ee65c021ffe933b1e57f08dd20e6e55f71 Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Mon, 13 Jul 2020 14:45:59 +0000 Subject: [PATCH 09/31] removed unnecessary include --- api/include/opentelemetry/context/context.h | 1 - 1 file changed, 1 deletion(-) diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index e5745c9a10..ecef96cd09 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -5,7 +5,6 @@ #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/trace/key_value_iterable_view.h" -#include #include OPENTELEMETRY_BEGIN_NAMESPACE From 497eb67c71567d52daee49a341d90766823d206b Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Mon, 13 Jul 2020 14:49:06 +0000 Subject: [PATCH 10/31] changed a function to pass by reference --- api/include/opentelemetry/context/context.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index ecef96cd09..968476c2b8 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -24,7 +24,7 @@ namespace context // Contructor, creates a context object from a map of keys // and identifiers. template ::value> * = nullptr> - Context(const T keys_and_values) + Context(const T &keys_and_values) { trace::KeyValueIterableView iterable{keys_and_values}; iterable.ForEachKeyValue([&](nostd::string_view key, context::ContextValue value) noexcept { From 479dead3acedcff2985d685dfe62c1737ac5bd29 Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Mon, 13 Jul 2020 16:32:01 +0000 Subject: [PATCH 11/31] formatted --- api/include/opentelemetry/context/context.h | 100 +++++++++--------- .../opentelemetry/context/context_value.h | 30 +++--- 2 files changed, 63 insertions(+), 67 deletions(-) diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index 968476c2b8..db4f0387ed 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -11,66 +11,62 @@ OPENTELEMETRY_BEGIN_NAMESPACE namespace context { - // The context class provides a context identifier. - // This is a dummy class that is meant to be overridden, - // the methods return default values. - class Context - { +// The context class provides a context identifier. +// This is a dummy class that is meant to be overridden, +// the methods return default values. +class Context +{ - public: +public: + Context() = default; - Context() = default; + // Contructor, creates a context object from a map of keys + // and identifiers. + template ::value> * = nullptr> + Context(const T &keys_and_values) + { + trace::KeyValueIterableView iterable{keys_and_values}; + iterable.ForEachKeyValue([&](nostd::string_view key, context::ContextValue value) noexcept { + context_map_[std::string(key)] = value; + return true; + }); + } - // Contructor, creates a context object from a map of keys - // and identifiers. - template ::value> * = nullptr> - Context(const T &keys_and_values) - { - trace::KeyValueIterableView iterable{keys_and_values}; - iterable.ForEachKeyValue([&](nostd::string_view key, context::ContextValue value) noexcept { - context_map_[std::string(key)] = value; - return true; - }); - } + // Accepts a key and a value and then returns a new context that + // contains both the original pairs and the new pair. + template + Context SetValue(nostd::string_view key, T &value) noexcept + { + std::map context_map_copy = context_map_; + context_map_copy[std::string(key)] = value; + return Context(context_map_copy); + } - // Accepts a key and a value and then returns a new context that - // contains both the original pairs and the new pair. - template - Context SetValue(nostd::string_view key, T &value) noexcept - { - std::map context_map_copy = context_map_; - context_map_copy[std::string(key)] = value; - return Context(context_map_copy); - } + // Accepts a new iterable and then returns a new context that + // contains both the original pairs and the new pair. + template ::value> * = nullptr> + Context SetValues(T &keys_and_values) noexcept + { + std::map context_map_copy = context_map_; + trace::KeyValueIterableView iterable{keys_and_values}; - // Accepts a new iterable and then returns a new context that - // contains both the original pairs and the new pair. - template ::value> * = nullptr> - Context SetValues(T &keys_and_values) noexcept - { - std::map context_map_copy = context_map_; - trace::KeyValueIterableView iterable{keys_and_values}; - - iterable.ForEachKeyValue([&](nostd::string_view key, context::ContextValue value) noexcept { - context_map_copy[std::string(key)] = value; - return true; - }); - - return Context(context_map_copy); - } + iterable.ForEachKeyValue([&](nostd::string_view key, context::ContextValue value) noexcept { + context_map_copy[std::string(key)] = value; + return true; + }); - // Returns the value associated with the passed in key. - context::ContextValue GetValue(nostd::string_view key) { - return context_map_[std::string(key)]; - } + return Context(context_map_copy); + } - // Copy Constructors. - Context(const Context &other) = default; - Context &operator=(const Context &other) = default; + // Returns the value associated with the passed in key. + context::ContextValue GetValue(nostd::string_view key) { return context_map_[std::string(key)]; } - private: + // Copy Constructors. + Context(const Context &other) = default; + Context &operator=(const Context &other) = default; - std::map context_map_; - }; +private: + std::map context_map_; +}; } // namespace context OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/context/context_value.h b/api/include/opentelemetry/context/context_value.h index e4b966a4d9..bccdac46d1 100644 --- a/api/include/opentelemetry/context/context_value.h +++ b/api/include/opentelemetry/context/context_value.h @@ -10,19 +10,19 @@ OPENTELEMETRY_BEGIN_NAMESPACE namespace context { - using ContextValue = nostd::variant, - nostd::span, - nostd::span, - nostd::span, - nostd::span, - nostd::span, - nostd::span>; -} // namespace common +using ContextValue = nostd::variant, + nostd::span, + nostd::span, + nostd::span, + nostd::span, + nostd::span, + nostd::span>; +} // namespace context OPENTELEMETRY_END_NAMESPACE From 2a78c48a8e03b1fe91ad5dda0280a4f7fdd76ce9 Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Mon, 13 Jul 2020 16:51:46 +0000 Subject: [PATCH 12/31] Added tests. --- api/test/context/context_test.cc | 101 +++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 api/test/context/context_test.cc diff --git a/api/test/context/context_test.cc b/api/test/context/context_test.cc new file mode 100644 index 0000000000..63010ec76a --- /dev/null +++ b/api/test/context/context_test.cc @@ -0,0 +1,101 @@ +#include "opentelemetry/context/context.h" + +#include + +#include + +using namespace opentelemetry; + +// Tests that the context constructor accepts an std::map. +TEST(ContextTest, ContextIterableAcceptsMap) +{ + std::map map_test = {{"test_key", "123"}}; + context::Context context_test = context::Context(map_test); +} + +// Tests that the GetValue method returns the expected value. +TEST(ContextTest, ContextGetValueReturnsExpectedValue) +{ + std::map map_test = {{"test_key", "123"}, {"foo_key", "456"}}; + + context::Context context_test = context::Context(map_test); + EXPECT_EQ(nostd::get(context_test.GetValue("test_key")), "123"); + EXPECT_EQ(nostd::get(context_test.GetValue("foo_key")), "456"); +} + +// Tests that the SetValues method accepts an std::map. +TEST(ContextTest, ContextSetValuesAcceptsMap) +{ + std::map map_test = {{"test_key", "123"}}; + std::map map_test_write = {{"foo_key", "456"}}; + context::Context context_test = context::Context(map_test); + context::Context foo_test = context_test.SetValues(map_test_write); + EXPECT_EQ(nostd::get(foo_test.GetValue("test_key")), "123"); + EXPECT_EQ(nostd::get(foo_test.GetValue("foo_key")), "456"); +} + +// Tests that the SetValues method accepts a nostd::string_view and +// context::ContextValue. +TEST(ContextTest, ContextSetValuesAcceptsStringViewContextValue) +{ + nostd::string_view string_view_test = "string_view"; + context::ContextValue context_value_test = "123"; + context::Context context_test = context::Context(); + context::Context context_foo = context_test.SetValue(string_view_test, context_value_test); + EXPECT_EQ(nostd::get(context_foo.GetValue(string_view_test)), "123"); +} + +// Tests that the original context does not change when a value is +// written to it. +TEST(ContextTest, ContextImmutability) +{ + std::map map_test = {{"test_key", "123"}}; + context::Context context_test = context::Context(map_test); + + context::Context context_foo = context_test.SetValue("foo_key", "456"); + + EXPECT_THROW(nostd::get(context_test.GetValue("foo_key")), + nostd::bad_variant_access); +} + +// Tests that writing the same to a context overwrites the original value. +TEST(ContextTest, ContextKeyOverwrite) +{ + std::map map_test = {{"test_key", "123"}}; + context::Context context_test = context::Context(map_test); + context::Context context_foo = context_test.SetValue("test_key", "456"); + + EXPECT_EQ(nostd::get(context_foo.GetValue("test_key")), "456"); +} + +// Tests that the new Context Objects inherits the keys and values +// of the original context object. +TEST(ContextTest, ContextInheritance) +{ + + using M = std::map; + context::Context test_context = context::Context(); + + M m1 = {{"test_key", "123"}, {"foo_key", "456"}}; + M m2 = {{"other_key", "789"}}; + + context::Context foo_context = test_context.SetValues(m1); + context::Context other_context = foo_context.SetValues(m2); + + EXPECT_EQ(nostd::get(other_context.GetValue("test_key")), "123"); + EXPECT_EQ(nostd::get(other_context.GetValue("foo_key")), "456"); +} + +// Tests that copying a context copies the key value pairs as expected. +TEST(ContextTest, ContextCopyOperator) +{ + std::map test_map = { + {"test_key", "123"}, {"foo_key", "456"}, {"other_key", "789"}}; + + context::Context test_context = context::Context(test_map); + context::Context copied_context = test_context; + + EXPECT_EQ(nostd::get(copied_context.GetValue("test_key")), "123"); + EXPECT_EQ(nostd::get(copied_context.GetValue("foo_key")), "456"); + EXPECT_EQ(nostd::get(copied_context.GetValue("other_key")), "789"); +} From f51748f614bd7b2f79dacafc6a1808dcf7b3ef23 Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Mon, 13 Jul 2020 17:48:52 +0000 Subject: [PATCH 13/31] avoiding ABI compatibility issues --- api/include/opentelemetry/context/context.h | 26 ++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index db4f0387ed..35b7920c09 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -5,6 +5,7 @@ #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/trace/key_value_iterable_view.h" +#include #include OPENTELEMETRY_BEGIN_NAMESPACE @@ -37,8 +38,18 @@ class Context template Context SetValue(nostd::string_view key, T &value) noexcept { - std::map context_map_copy = context_map_; - context_map_copy[std::string(key)] = value; + std::map context_map_copy; + trace::KeyValueIterableView> context_map_iterable{ + context_map_}; + + context_map_iterable.ForEachKeyValue([&](nostd::string_view key, + context::ContextValue value) noexcept { + context_map_copy[std::string(key)] = value; + return true; + }); + + context_map_copy[std::string(key)] = value; + return Context(context_map_copy); } @@ -47,7 +58,16 @@ class Context template ::value> * = nullptr> Context SetValues(T &keys_and_values) noexcept { - std::map context_map_copy = context_map_; + std::map context_map_copy; + trace::KeyValueIterableView> context_map_iterable{ + context_map_}; + + context_map_iterable.ForEachKeyValue([&](nostd::string_view key, + context::ContextValue value) noexcept { + context_map_copy[std::string(key)] = value; + return true; + }); + trace::KeyValueIterableView iterable{keys_and_values}; iterable.ForEachKeyValue([&](nostd::string_view key, context::ContextValue value) noexcept { From 3f85aa510e680191b8cfd6f8bebbaa0dcd8db8b4 Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Tue, 14 Jul 2020 14:50:59 +0000 Subject: [PATCH 14/31] added throw capture --- api/test/context/context_test.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/api/test/context/context_test.cc b/api/test/context/context_test.cc index 63010ec76a..de49e7ac4e 100644 --- a/api/test/context/context_test.cc +++ b/api/test/context/context_test.cc @@ -53,9 +53,12 @@ TEST(ContextTest, ContextImmutability) context::Context context_test = context::Context(map_test); context::Context context_foo = context_test.SetValue("foo_key", "456"); - +#if __EXCEPTIONS EXPECT_THROW(nostd::get(context_test.GetValue("foo_key")), nostd::bad_variant_access); +#else + EXPECT_DEATH({nostd::get(context_test.GetValue("foo_key"))}, ""); +#endif } // Tests that writing the same to a context overwrites the original value. From 21ea872e1af3d76687e2a315a01e27317c4d0cee Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Tue, 14 Jul 2020 15:06:53 +0000 Subject: [PATCH 15/31] minor exception syntax error --- api/test/context/context_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/test/context/context_test.cc b/api/test/context/context_test.cc index de49e7ac4e..ee366d1a1a 100644 --- a/api/test/context/context_test.cc +++ b/api/test/context/context_test.cc @@ -57,7 +57,7 @@ TEST(ContextTest, ContextImmutability) EXPECT_THROW(nostd::get(context_test.GetValue("foo_key")), nostd::bad_variant_access); #else - EXPECT_DEATH({nostd::get(context_test.GetValue("foo_key"))}, ""); + EXPECT_DEATH({ nostd::get(context_test.GetValue("foo_key")); }, ""); #endif } From 1f5e5fddfa658c0ec55d961e0065ef756d5a02c8 Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Tue, 14 Jul 2020 16:45:53 +0000 Subject: [PATCH 16/31] added runtime and threadlocal contexts --- .../opentelemetry/context/runtime_context.h | 31 ++++++++++ .../context/threadlocal_context.h | 62 +++++++++++++++++++ 2 files changed, 93 insertions(+) create mode 100644 api/include/opentelemetry/context/runtime_context.h create 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 new file mode 100644 index 0000000000..85807b407d --- /dev/null +++ b/api/include/opentelemetry/context/runtime_context.h @@ -0,0 +1,31 @@ +#pragma once + +#include "context.h" + +OPENTELEMETRY_BEGIN_NAMESPACE +namespace context +{ + + // The RuntimeContext class provides a wrapper for + // propogating context through cpp. + class RuntimeContext + { + public: + // The token class provides an identifier that is used by + // the attach and detach methods to keep track of context + // objects. + class Token; + + // Return the current context. + Context GetCurrent(); + + // Sets the current 'Context' object. Returns a token + // that can be used to reset to the previous Context. + Token Attach(Context &context); + + // Resets the context to a previous value stored in the + // passed in token. Returns zero if successful, -1 otherwise + int Detach(Token &token); + }; +} // namespace context +OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/context/threadlocal_context.h b/api/include/opentelemetry/context/threadlocal_context.h new file mode 100644 index 0000000000..2b5d354a0e --- /dev/null +++ b/api/include/opentelemetry/context/threadlocal_context.h @@ -0,0 +1,62 @@ +#pragma once + +#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. + class ThreadLocalContext : public RuntimeContext + { + public: + // The token class provides an identifier that is used by + // the attach and detach methods to keep track of context + // objects. + class Token + { + private: + friend class ThreadLocalContext; + + Context context_; + + // A constructor that sets the token's Context object to the + // one that was passed in. + Token(Context &context) { context_ = context; } + + // Returns the stored context object. + Context GetContext() { return context_; } + }; + + // Return the current context. + static Context GetCurrent() { return GetInstance(); } + + // Resets the context to a previous value stored in the + // passed in token. Returns zero if successful, -1 otherwise + static int Detach(Token &token) + { + // If the token context is the current context, return failure. + GetInstance() = token.GetContext(); + return 0; + } + + // Sets the current 'Context' object. Returns a token + // that can be used to reset to the previous Context. + static Token Attach(Context &context) + { + Token old_context = Token(GetInstance()); + GetInstance() = context; + return old_context; + } + + private: + // Provides storage and access to the thread_local context object. + static Context &GetInstance() + { + static thread_local Context instance; + return instance; + } + }; +} // namespace context +OPENTELEMETRY_END_NAMESPACE From bbf95598526e68dd6ae3316e1e055594e7ea4a88 Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Tue, 14 Jul 2020 17:12:43 +0000 Subject: [PATCH 17/31] add comparison operator to context api --- api/include/opentelemetry/context/context.h | 22 ++++++++++++++++++- .../context/threadlocal_context.h | 5 +++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index 35b7920c09..d15943ca18 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -7,6 +7,7 @@ #include #include +#include OPENTELEMETRY_BEGIN_NAMESPACE namespace context @@ -19,7 +20,9 @@ class Context { public: - Context() = default; + Context(){ + context_id_ = GetNextAvailableId(); + } // Contructor, creates a context object from a map of keys // and identifiers. @@ -31,6 +34,7 @@ class Context context_map_[std::string(key)] = value; return true; }); + context_id_ = GetNextAvailableId(); } // Accepts a key and a value and then returns a new context that @@ -84,9 +88,25 @@ class Context // Copy Constructors. Context(const Context &other) = default; Context &operator=(const Context &other) = default; + + // Comparison operator. + bool operator==(const Context &other){ + return (other.context_id_ == context_id_); + } private: std::map context_map_; + + int context_id_; + + static int GetNextAvailableId(){ + return next_id_++; + } + + static std::atomic next_id_; + }; + +std::atomic Context::next_id_{0}; } // namespace context OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/context/threadlocal_context.h b/api/include/opentelemetry/context/threadlocal_context.h index 2b5d354a0e..6b2205e3ec 100644 --- a/api/include/opentelemetry/context/threadlocal_context.h +++ b/api/include/opentelemetry/context/threadlocal_context.h @@ -37,6 +37,11 @@ namespace context static int Detach(Token &token) { // If the token context is the current context, return failure. + if (token.GetContext() == GetCurrent()) + { + return 1; + } + GetInstance() = token.GetContext(); return 0; } From c8b7ed692f3df6a82a5a8d5b82a2781cd821b24a Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Thu, 16 Jul 2020 03:41:01 +0000 Subject: [PATCH 18/31] added stack for local storage of context objects --- .../context/threadlocal_context.h | 81 +++++++++++++++---- 1 file changed, 64 insertions(+), 17 deletions(-) diff --git a/api/include/opentelemetry/context/threadlocal_context.h b/api/include/opentelemetry/context/threadlocal_context.h index 6b2205e3ec..8864c9a457 100644 --- a/api/include/opentelemetry/context/threadlocal_context.h +++ b/api/include/opentelemetry/context/threadlocal_context.h @@ -19,49 +19,96 @@ namespace context private: friend class ThreadLocalContext; - Context context_; + Context* context_; // A constructor that sets the token's Context object to the // one that was passed in. - Token(Context &context) { context_ = context; } - + Token(Context* context) { context_ = context; } + // Returns the stored context object. - Context GetContext() { return context_; } + Context* GetContext() { return context_; } }; // Return the current context. - static Context GetCurrent() { return GetInstance(); } + static Context* GetCurrent() { return Stack::Top(); } // Resets the context to a previous value stored in the // passed in token. Returns zero if successful, -1 otherwise static int Detach(Token &token) { - // If the token context is the current context, return failure. - if (token.GetContext() == GetCurrent()) + if (!(token.GetContext() == Stack::Top())) { return 1; } - - GetInstance() = token.GetContext(); + Stack::Pop(); return 0; } // Sets the current 'Context' object. Returns a token // that can be used to reset to the previous Context. - static Token Attach(Context &context) + static Token Attach(Context* context) { - Token old_context = Token(GetInstance()); - GetInstance() = context; + Stack::Push(context); + Token old_context = Token(context); return old_context; } private: - // Provides storage and access to the thread_local context object. - static Context &GetInstance() + + // A nested class to store the attached contexts in a stack. + class Stack { - static thread_local Context instance; - return instance; - } + friend class ThreadLocalContext; + + // Pops the top Context* off the stack and returns it. + static Context* Pop() + { + int index = size_ - 1; + size_--; + return base_[index]; + } + + // Returns the Context* at the top of the stack. + static Context* Top() + { + return base_[size_ - 1]; + } + + // Pushes the passed in context pointer to the top of the stack + // and resizes if necessary. + static void Push(Context* context) + { + size_++; + if(size_ > capacity_){ + Resize(size_*2); + } + base_[size_ - 1] = context; + } + + // Reallocates the storage array to the pass in new capacity size. + static void Resize(int new_capacity) + { + int old_size = size_ - 1; + if(new_capacity == 0){ + new_capacity = 2; + } + Context** temp = new Context*[new_capacity]; + memcpy(temp, base_, sizeof(Context**)*old_size); + delete [] base_; + base_ = temp; + } + + ~Stack(){ + delete [] base_; + } + + static thread_local int size_; + static thread_local int capacity_; + static thread_local Context** base_; + }; }; + thread_local int ThreadLocalContext::Stack::size_ = 0; + thread_local int ThreadLocalContext::Stack::capacity_ = 0; + thread_local Context** ThreadLocalContext::Stack::base_ = nullptr; } // namespace context OPENTELEMETRY_END_NAMESPACE From 8d8b91f275421dbfc53e4e7957433633de68ebe4 Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Thu, 16 Jul 2020 13:26:03 +0000 Subject: [PATCH 19/31] removed old context.h --- api/include/opentelemetry/context/context.h | 112 -------------------- 1 file changed, 112 deletions(-) delete mode 100644 api/include/opentelemetry/context/context.h diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h deleted file mode 100644 index d15943ca18..0000000000 --- a/api/include/opentelemetry/context/context.h +++ /dev/null @@ -1,112 +0,0 @@ -#pragma once - -#include "opentelemetry/common/attribute_value.h" -#include "opentelemetry/context/context_value.h" -#include "opentelemetry/nostd/string_view.h" -#include "opentelemetry/trace/key_value_iterable_view.h" - -#include -#include -#include - -OPENTELEMETRY_BEGIN_NAMESPACE -namespace context -{ - -// The context class provides a context identifier. -// This is a dummy class that is meant to be overridden, -// the methods return default values. -class Context -{ - -public: - Context(){ - context_id_ = GetNextAvailableId(); - } - - // Contructor, creates a context object from a map of keys - // and identifiers. - template ::value> * = nullptr> - Context(const T &keys_and_values) - { - trace::KeyValueIterableView iterable{keys_and_values}; - iterable.ForEachKeyValue([&](nostd::string_view key, context::ContextValue value) noexcept { - context_map_[std::string(key)] = value; - return true; - }); - context_id_ = GetNextAvailableId(); - } - - // Accepts a key and a value and then returns a new context that - // contains both the original pairs and the new pair. - template - Context SetValue(nostd::string_view key, T &value) noexcept - { - std::map context_map_copy; - trace::KeyValueIterableView> context_map_iterable{ - context_map_}; - - context_map_iterable.ForEachKeyValue([&](nostd::string_view key, - context::ContextValue value) noexcept { - context_map_copy[std::string(key)] = value; - return true; - }); - - context_map_copy[std::string(key)] = value; - - return Context(context_map_copy); - } - - // Accepts a new iterable and then returns a new context that - // contains both the original pairs and the new pair. - template ::value> * = nullptr> - Context SetValues(T &keys_and_values) noexcept - { - std::map context_map_copy; - trace::KeyValueIterableView> context_map_iterable{ - context_map_}; - - context_map_iterable.ForEachKeyValue([&](nostd::string_view key, - context::ContextValue value) noexcept { - context_map_copy[std::string(key)] = value; - return true; - }); - - trace::KeyValueIterableView iterable{keys_and_values}; - - iterable.ForEachKeyValue([&](nostd::string_view key, context::ContextValue value) noexcept { - context_map_copy[std::string(key)] = value; - return true; - }); - - return Context(context_map_copy); - } - - // Returns the value associated with the passed in key. - context::ContextValue GetValue(nostd::string_view key) { return context_map_[std::string(key)]; } - - // Copy Constructors. - Context(const Context &other) = default; - Context &operator=(const Context &other) = default; - - // Comparison operator. - bool operator==(const Context &other){ - return (other.context_id_ == context_id_); - } - -private: - std::map context_map_; - - int context_id_; - - static int GetNextAvailableId(){ - return next_id_++; - } - - static std::atomic next_id_; - -}; - -std::atomic Context::next_id_{0}; -} // namespace context -OPENTELEMETRY_END_NAMESPACE From 4b78ac8040dcea11c67178a2a64fca6d6bf45b80 Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Thu, 16 Jul 2020 13:27:19 +0000 Subject: [PATCH 20/31] changed runtime context interface to match --- 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 85807b407d..a027895d55 100644 --- a/api/include/opentelemetry/context/runtime_context.h +++ b/api/include/opentelemetry/context/runtime_context.h @@ -17,11 +17,11 @@ namespace context class Token; // Return the current context. - Context GetCurrent(); + Context* GetCurrent(); // Sets the current 'Context' object. Returns a token // that can be used to reset to the previous Context. - Token Attach(Context &context); + Token Attach(Context* context); // Resets the context to a previous value stored in the // passed in token. Returns zero if successful, -1 otherwise From d77e445153e91a7bdc3a19a5eb74b86a46cfb0bc Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Wed, 22 Jul 2020 03:20:12 +0000 Subject: [PATCH 21/31] formatted and added some size checks to prevent seg faults, also added an overloaded comparison to context API --- api/include/opentelemetry/context/context.h | 6 +- .../opentelemetry/context/runtime_context.h | 36 ++-- .../context/threadlocal_context.h | 187 +++++++++--------- api/test/context/BUILD | 11 ++ api/test/context/context_test.cc | 19 ++ api/test/nostd/string_view_test.cc | 2 +- 6 files changed, 150 insertions(+), 111 deletions(-) diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index 1f7caf0ee1..d25f8ed91e 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -16,6 +16,8 @@ class Context { public: + Context() = default; + // Creates a context object from a map of keys and identifiers, this will // hold a shared_ptr to the head of the DataList linked list template @@ -89,9 +91,9 @@ class Context return false; } -private: - Context() = default; + bool operator==(const Context &other) { return (head_ == other.head_); } +private: // A linked list to contain the keys and values of this context node class DataList { diff --git a/api/include/opentelemetry/context/runtime_context.h b/api/include/opentelemetry/context/runtime_context.h index a027895d55..747286ddc0 100644 --- a/api/include/opentelemetry/context/runtime_context.h +++ b/api/include/opentelemetry/context/runtime_context.h @@ -6,26 +6,26 @@ OPENTELEMETRY_BEGIN_NAMESPACE namespace context { - // The RuntimeContext class provides a wrapper for - // propogating context through cpp. - class RuntimeContext - { - public: - // The token class provides an identifier that is used by - // the attach and detach methods to keep track of context - // objects. - class Token; +// The RuntimeContext class provides a wrapper for +// propogating context through cpp. +class RuntimeContext +{ +public: + // The token class provides an identifier that is used by + // the attach and detach methods to keep track of context + // objects. + class Token; - // Return the current context. - Context* GetCurrent(); + // Return the current context. + Context *GetCurrent(); - // Sets the current 'Context' object. Returns a token - // that can be used to reset to the previous Context. - Token Attach(Context* context); + // Sets the current 'Context' object. Returns a token + // that can be used to reset to the previous Context. + Token Attach(Context *context); - // Resets the context to a previous value stored in the - // passed in token. Returns zero if successful, -1 otherwise - int Detach(Token &token); - }; + // Resets the context to a previous value stored in the + // passed in token. Returns zero if successful, -1 otherwise + int Detach(Token &token); +}; } // namespace context OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/context/threadlocal_context.h b/api/include/opentelemetry/context/threadlocal_context.h index 8864c9a457..23e5cb881f 100644 --- a/api/include/opentelemetry/context/threadlocal_context.h +++ b/api/include/opentelemetry/context/threadlocal_context.h @@ -6,109 +6,116 @@ 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. - class ThreadLocalContext : public RuntimeContext +// The ThreadLocalContext class is a derived class from RuntimeContext and +// provides a wrapper for propogating context through cpp thread locally. +class ThreadLocalContext : public RuntimeContext +{ +public: + // The token class provides an identifier that is used by + // the attach and detach methods to keep track of context + // objects. + class Token { - public: - // The token class provides an identifier that is used by - // the attach and detach methods to keep track of context - // objects. - class Token - { - private: - friend class ThreadLocalContext; + private: + friend class ThreadLocalContext; + + Context *context_; + + // A constructor that sets the token's Context object to the + // one that was passed in. + Token(Context *context) { context_ = context; } - Context* context_; + // Returns the stored context object. + Context *GetContext() { return context_; } + }; + + // Return the current context. + static Context *GetCurrent() { return Stack::Top(); } - // A constructor that sets the token's Context object to the - // one that was passed in. - Token(Context* context) { context_ = context; } - - // Returns the stored context object. - Context* GetContext() { return 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) + { + if (!(token.GetContext() == Stack::Top())) + { + return false; + } + Stack::Pop(); + return true; + } - // Return the current context. - static Context* GetCurrent() { return Stack::Top(); } + // Sets the current 'Context' object. Returns a token + // that can be used to reset to the previous Context. + static Token Attach(Context *context) + { + Stack::Push(context); + Token old_context = Token(context); + return old_context; + } - // Resets the context to a previous value stored in the - // passed in token. Returns zero if successful, -1 otherwise - static int Detach(Token &token) +private: + // A nested class to store the attached contexts in a stack. + class Stack + { + friend class ThreadLocalContext; + + // Pops the top Context* off the stack and returns it. + static Context *Pop() + { + if (size_ <= 0) { - if (!(token.GetContext() == Stack::Top())) - { - return 1; - } - Stack::Pop(); - return 0; + return nullptr; } + int index = size_ - 1; + size_--; + return base_[index]; + } - // Sets the current 'Context' object. Returns a token - // that can be used to reset to the previous Context. - static Token Attach(Context* context) + // Returns the Context* at the top of the stack. + static Context *Top() + { + if (size_ <= 0) { - Stack::Push(context); - Token old_context = Token(context); - return old_context; + return nullptr; } + return base_[size_ - 1]; + } - private: + // Pushes the passed in context pointer to the top of the stack + // and resizes if necessary. + static void Push(Context *context) + { + size_++; + if (size_ > capacity_) + { + Resize(size_ * 2); + } + base_[size_ - 1] = context; + } - // A nested class to store the attached contexts in a stack. - class Stack + // Reallocates the storage array to the pass in new capacity size. + static void Resize(int new_capacity) + { + int old_size = size_ - 1; + if (new_capacity == 0) { - friend class ThreadLocalContext; - - // Pops the top Context* off the stack and returns it. - static Context* Pop() - { - int index = size_ - 1; - size_--; - return base_[index]; - } - - // Returns the Context* at the top of the stack. - static Context* Top() - { - return base_[size_ - 1]; - } - - // Pushes the passed in context pointer to the top of the stack - // and resizes if necessary. - static void Push(Context* context) - { - size_++; - if(size_ > capacity_){ - Resize(size_*2); - } - base_[size_ - 1] = context; - } - - // Reallocates the storage array to the pass in new capacity size. - static void Resize(int new_capacity) - { - int old_size = size_ - 1; - if(new_capacity == 0){ - new_capacity = 2; - } - Context** temp = new Context*[new_capacity]; - memcpy(temp, base_, sizeof(Context**)*old_size); - delete [] base_; - base_ = temp; - } - - ~Stack(){ - delete [] base_; - } - - static thread_local int size_; - static thread_local int capacity_; - static thread_local Context** base_; - }; + new_capacity = 2; + } + Context **temp = new Context *[new_capacity]; + memcpy(temp, base_, sizeof(Context *) * old_size); + delete[] base_; + base_ = temp; + } + + ~Stack() { delete[] base_; } + + static thread_local int size_; + static thread_local int capacity_; + static thread_local Context **base_; }; - thread_local int ThreadLocalContext::Stack::size_ = 0; - thread_local int ThreadLocalContext::Stack::capacity_ = 0; - thread_local Context** ThreadLocalContext::Stack::base_ = nullptr; +}; +thread_local int ThreadLocalContext::Stack::size_ = 0; +thread_local int ThreadLocalContext::Stack::capacity_ = 0; +thread_local Context **ThreadLocalContext::Stack::base_ = nullptr; } // namespace context OPENTELEMETRY_END_NAMESPACE diff --git a/api/test/context/BUILD b/api/test/context/BUILD index b213710944..b0306e9a91 100644 --- a/api/test/context/BUILD +++ b/api/test/context/BUILD @@ -10,3 +10,14 @@ cc_test( "@com_google_googletest//:gtest_main", ], ) + +cc_test( + name = "threadlocal_context_test", + srcs = [ + "threadlocal_context_test.cc", + ], + deps = [ + "//api", + "@com_google_googletest//:gtest_main", + ], +) diff --git a/api/test/context/context_test.cc b/api/test/context/context_test.cc index 26fc5110bf..f9a6dfbfd4 100644 --- a/api/test/context/context_test.cc +++ b/api/test/context/context_test.cc @@ -118,3 +118,22 @@ TEST(ContextTest, ContextHasKey) EXPECT_TRUE(context_test.HasKey("test_key")); EXPECT_FALSE(context_test.HasKey("foo_key")); } + +// Tests that a copied context returns true when compared +TEST(ContextTest, ContextCopyCompare) +{ + std::map map_test = {{"test_key", (int64_t)123}}; + context::Context context_test = context::Context(map_test); + context::Context copied_test = context_test; + EXPECT_TRUE(context_test == copied_test); +} + +// Tests that two differently constructed contexts return false when compared +TEST(ContextTest, ContextDiffCompare) +{ + std::map map_test = {{"test_key", (int64_t)123}}; + std::map map_foo = {{"foo_key", (int64_t)123}}; + context::Context context_test = context::Context(map_test); + context::Context foo_test = context::Context(map_foo); + EXPECT_FALSE(context_test == foo_test); +} diff --git a/api/test/nostd/string_view_test.cc b/api/test/nostd/string_view_test.cc index 074d3bee49..364410a312 100644 --- a/api/test/nostd/string_view_test.cc +++ b/api/test/nostd/string_view_test.cc @@ -95,7 +95,7 @@ TEST(StringViewTest, Compare) TEST(StringViewTest, MapKeyOrdering) { std::map m = {{"bbb", 2}, {"aaa", 1}, {"ccc", 3}}; - size_t i = 1; + size_t i = 1; for (const auto &kv : m) { EXPECT_EQ(kv.second, i); From 0583e3c3fdf770fd5f34c0c5ed2e6fb1109e3e81 Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Wed, 22 Jul 2020 03:56:42 +0000 Subject: [PATCH 22/31] added tests --- api/test/context/threadlocal_context_test.cc | 65 ++++++++++++ api/test/nostd/string_view_test.cc | 104 ------------------- 2 files changed, 65 insertions(+), 104 deletions(-) create mode 100644 api/test/context/threadlocal_context_test.cc delete mode 100644 api/test/nostd/string_view_test.cc diff --git a/api/test/context/threadlocal_context_test.cc b/api/test/context/threadlocal_context_test.cc new file mode 100644 index 0000000000..2927e4a902 --- /dev/null +++ b/api/test/context/threadlocal_context_test.cc @@ -0,0 +1,65 @@ +#include "opentelemetry/context/threadlocal_context.h" + +#include + +using namespace opentelemetry; + +// Tests that GetCurrent returns the current context +TEST(ThreadLocalContextTest, ThreadLocalGetCurrent) +{ + std::map map_test = {{"test_key", (int64_t)123}}; + context::Context test_context = context::Context(map_test); + context::ThreadLocalContext::Token old_context = + context::ThreadLocalContext::Attach(&test_context); + EXPECT_TRUE(*context::ThreadLocalContext::GetCurrent() == test_context); + context::ThreadLocalContext::Detach(old_context); +} + +// Tests that detach resets the context to the previous context +TEST(ThreadLocalContextTest, ThreadLocalDetach) +{ + std::map map_test = {{"test_key", (int64_t)123}}; + context::Context test_context = context::Context(map_test); + context::Context foo_context = context::Context(map_test); + context::ThreadLocalContext::Token test_context_token = + context::ThreadLocalContext::Attach(&test_context); + context::ThreadLocalContext::Token foo_context_token = + context::ThreadLocalContext::Attach(&foo_context); + context::ThreadLocalContext::Detach(foo_context_token); + EXPECT_TRUE(*context::ThreadLocalContext::GetCurrent() == test_context); + context::ThreadLocalContext::Detach(test_context_token); +} + +// Tests that detach returns false when the wrong context is provided +TEST(ThreadLocalContextTest, ThreadLocalDetachWrongContext) +{ + std::map map_test = {{"test_key", (int64_t)123}}; + context::Context test_context = context::Context(map_test); + context::Context foo_context = context::Context(map_test); + context::ThreadLocalContext::Token test_context_token = + context::ThreadLocalContext::Attach(&test_context); + context::ThreadLocalContext::Token foo_context_token = + context::ThreadLocalContext::Attach(&foo_context); + EXPECT_FALSE(context::ThreadLocalContext::Detach(test_context_token)); + context::ThreadLocalContext::Detach(foo_context_token); + context::ThreadLocalContext::Detach(test_context_token); +} + +// Tests that the ThreadLocalContext can handle three attached contexts +TEST(ThreadLocalContextTest, ThreadLocalThreeAttachDetach) +{ + std::map map_test = {{"test_key", (int64_t)123}}; + context::Context test_context = context::Context(map_test); + context::Context foo_context = context::Context(map_test); + context::Context other_context = context::Context(map_test); + context::ThreadLocalContext::Token test_context_token = + context::ThreadLocalContext::Attach(&test_context); + context::ThreadLocalContext::Token foo_context_token = + context::ThreadLocalContext::Attach(&foo_context); + context::ThreadLocalContext::Token other_context_token = + context::ThreadLocalContext::Attach(&other_context); + + EXPECT_TRUE(context::ThreadLocalContext::Detach(other_context_token)); + EXPECT_TRUE(context::ThreadLocalContext::Detach(foo_context_token)); + EXPECT_TRUE(context::ThreadLocalContext::Detach(test_context_token)); +} diff --git a/api/test/nostd/string_view_test.cc b/api/test/nostd/string_view_test.cc deleted file mode 100644 index 364410a312..0000000000 --- a/api/test/nostd/string_view_test.cc +++ /dev/null @@ -1,104 +0,0 @@ -#include "opentelemetry/nostd/string_view.h" - -#include - -#include "map" - -using opentelemetry::nostd::string_view; - -TEST(StringViewTest, DefaultConstruction) -{ - string_view ref; - EXPECT_EQ(ref.data(), nullptr); - EXPECT_EQ(ref.length(), 0); -} - -TEST(StringViewTest, CStringInitialization) -{ - const char *val = "hello world"; - - string_view ref(val); - - EXPECT_EQ(ref.data(), val); - EXPECT_EQ(ref.length(), std::strlen(val)); -} - -TEST(StringViewTest, StdStringInitialization) -{ - const std::string val = "hello world"; - - string_view ref(val); - - EXPECT_EQ(ref.data(), val.data()); - EXPECT_EQ(ref.length(), val.size()); -} - -TEST(StringViewTest, Copy) -{ - const std::string val = "hello world"; - - string_view ref(val); - string_view cpy(ref); - - EXPECT_EQ(cpy.data(), val); - EXPECT_EQ(cpy.length(), val.length()); - EXPECT_EQ(cpy, val); -} - -TEST(StringViewTest, Accessor) -{ - string_view s = "abc123"; - EXPECT_EQ(s.data(), &s[0]); - EXPECT_EQ(s.data() + 1, &s[1]); -} - -TEST(StringViewTest, ExplicitStdStringConversion) -{ - std::string s = static_cast(string_view{"abc"}); - EXPECT_EQ(s, "abc"); -} - -TEST(StringViewTest, SubstrPortion) -{ - string_view s = "abc123"; - EXPECT_EQ("123", s.substr(3)); - EXPECT_EQ("12", s.substr(3, 2)); -} - -TEST(StringViewTest, SubstrOutOfRange) -{ - string_view s = "abc123"; -#if __EXCEPTIONS - EXPECT_THROW(s.substr(10), std::out_of_range); -#else - EXPECT_DEATH({ s.substr(10); }, ""); -#endif -} - -TEST(StringViewTest, Compare) -{ - string_view s1 = "aaa"; - string_view s2 = "bbb"; - string_view s3 = "aaa"; - - // Equals - EXPECT_EQ(s1, s3); - EXPECT_EQ(s1, s1); - - // Less then - EXPECT_LT(s1, s2); - - // Greater then - EXPECT_GT(s2, s1); -} - -TEST(StringViewTest, MapKeyOrdering) -{ - std::map m = {{"bbb", 2}, {"aaa", 1}, {"ccc", 3}}; - size_t i = 1; - for (const auto &kv : m) - { - EXPECT_EQ(kv.second, i); - i++; - } -} From e02fadb13af9b1d408e75168e4041b3acfbdc425 Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Thu, 23 Jul 2020 02:13:35 +0000 Subject: [PATCH 23/31] add a context_config file to choose which implementation the runtime context will use --- api/include/opentelemetry/context/context.h | 4 +- .../opentelemetry/context/context_config.h | 17 +++++ .../opentelemetry/context/runtime_context.h | 23 ++++--- .../context/threadlocal_context.h | 47 +++++++++----- api/test/context/BUILD | 4 +- api/test/context/runtime_context_test.cc | 59 +++++++++++++++++ api/test/context/threadlocal_context_test.cc | 65 ------------------- 7 files changed, 120 insertions(+), 99 deletions(-) create mode 100644 api/include/opentelemetry/context/context_config.h create mode 100644 api/test/context/runtime_context_test.cc delete mode 100644 api/test/context/threadlocal_context_test.cc diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index d25f8ed91e..9302fa9abe 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -16,8 +16,6 @@ class Context { public: - Context() = default; - // Creates a context object from a map of keys and identifiers, this will // hold a shared_ptr to the head of the DataList linked list template @@ -94,6 +92,8 @@ class Context bool operator==(const Context &other) { return (head_ == other.head_); } private: + Context() = default; + // A linked list to contain the keys and values of this context node class DataList { diff --git a/api/include/opentelemetry/context/context_config.h b/api/include/opentelemetry/context/context_config.h new file mode 100644 index 0000000000..4c76131ca7 --- /dev/null +++ b/api/include/opentelemetry/context/context_config.h @@ -0,0 +1,17 @@ +#pragma once + +//#include "opentelemetry/context/threadlocal_context.h" +#include "threadlocal_context.h" +OPENTELEMETRY_BEGIN_NAMESPACE +namespace context +{ +// The ContextType is used to determine the implementation that will be used +// for the RuntimeContext. It defaults to the ThreadLocalContext object. +typedef ThreadLocalContext ContextType; + +// The token type provides an identifier that is used by +// the attach and detach methods to keep track of context +// objects. +typedef ThreadLocalContext::Token ContextToken; +} // namespace context +OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/context/runtime_context.h b/api/include/opentelemetry/context/runtime_context.h index 747286ddc0..8358d72d5f 100644 --- a/api/include/opentelemetry/context/runtime_context.h +++ b/api/include/opentelemetry/context/runtime_context.h @@ -1,31 +1,30 @@ #pragma once -#include "context.h" +#include "opentelemetry/context/context.h" +#include "opentelemetry/context/context_config.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace context { -// The RuntimeContext class provides a wrapper for -// propogating context through cpp. class RuntimeContext { public: - // The token class provides an identifier that is used by - // the attach and detach methods to keep track of context - // objects. - class Token; - // Return the current context. - Context *GetCurrent(); + static Context *GetCurrent() { return context_handler_.GetCurrent(); } // Sets the current 'Context' object. Returns a token // that can be used to reset to the previous Context. - Token Attach(Context *context); + static ContextToken Attach(Context *context) { return context_handler_.Attach(context); } // Resets the context to a previous value stored in the - // passed in token. Returns zero if successful, -1 otherwise - int Detach(Token &token); + // passed in token. Returns true if successful, false otherwise + static bool Detach(ContextToken &token) { return context_handler_.Detach(token); } + + static ContextType context_handler_; }; + +ContextType RuntimeContext::context_handler_ = ContextType(); + } // namespace context OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/context/threadlocal_context.h b/api/include/opentelemetry/context/threadlocal_context.h index 23e5cb881f..626b536e17 100644 --- a/api/include/opentelemetry/context/threadlocal_context.h +++ b/api/include/opentelemetry/context/threadlocal_context.h @@ -5,12 +5,13 @@ 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. -class ThreadLocalContext : public RuntimeContext +class ThreadLocalContext { public: + ThreadLocalContext() {} + // The token class provides an identifier that is used by // the attach and detach methods to keep track of context // objects. @@ -30,25 +31,27 @@ class ThreadLocalContext : public RuntimeContext }; // Return the current context. - static Context *GetCurrent() { return Stack::Top(); } + Context *GetCurrent() { return stack_.Top(); } // Resets the context to a previous value stored in the // passed in token. Returns true if successful, false otherwise - static bool Detach(Token &token) + bool Detach(Token &token) { - if (!(token.GetContext() == Stack::Top())) + + if (!(token.GetContext() == stack_.Top())) { return false; } - Stack::Pop(); + + stack_.Pop(); return true; } // Sets the current 'Context' object. Returns a token // that can be used to reset to the previous Context. - static Token Attach(Context *context) + Token Attach(Context *context) { - Stack::Push(context); + stack_.Push(context); Token old_context = Token(context); return old_context; } @@ -59,8 +62,15 @@ class ThreadLocalContext : public RuntimeContext { friend class ThreadLocalContext; + Stack() + { + size_ = 0; + capacity_ = 0; + base_ = nullptr; + } + // Pops the top Context* off the stack and returns it. - static Context *Pop() + Context *Pop() { if (size_ <= 0) { @@ -72,7 +82,7 @@ class ThreadLocalContext : public RuntimeContext } // Returns the Context* at the top of the stack. - static Context *Top() + Context *Top() { if (size_ <= 0) { @@ -83,7 +93,7 @@ class ThreadLocalContext : public RuntimeContext // Pushes the passed in context pointer to the top of the stack // and resizes if necessary. - static void Push(Context *context) + void Push(Context *context) { size_++; if (size_ > capacity_) @@ -94,7 +104,7 @@ class ThreadLocalContext : public RuntimeContext } // Reallocates the storage array to the pass in new capacity size. - static void Resize(int new_capacity) + void Resize(int new_capacity) { int old_size = size_ - 1; if (new_capacity == 0) @@ -109,13 +119,14 @@ class ThreadLocalContext : public RuntimeContext ~Stack() { delete[] base_; } - static thread_local int size_; - static thread_local int capacity_; - static thread_local Context **base_; + int size_; + int capacity_; + Context **base_; }; + + static thread_local Stack stack_; }; -thread_local int ThreadLocalContext::Stack::size_ = 0; -thread_local int ThreadLocalContext::Stack::capacity_ = 0; -thread_local Context **ThreadLocalContext::Stack::base_ = nullptr; +thread_local ThreadLocalContext::Stack ThreadLocalContext::stack_ = ThreadLocalContext::Stack(); + } // namespace context OPENTELEMETRY_END_NAMESPACE diff --git a/api/test/context/BUILD b/api/test/context/BUILD index b0306e9a91..87452f4588 100644 --- a/api/test/context/BUILD +++ b/api/test/context/BUILD @@ -12,9 +12,9 @@ cc_test( ) cc_test( - name = "threadlocal_context_test", + name = "runtime_context_test", srcs = [ - "threadlocal_context_test.cc", + "runtime_context_test.cc", ], deps = [ "//api", diff --git a/api/test/context/runtime_context_test.cc b/api/test/context/runtime_context_test.cc new file mode 100644 index 0000000000..c34e44a119 --- /dev/null +++ b/api/test/context/runtime_context_test.cc @@ -0,0 +1,59 @@ +#include "opentelemetry/context/runtime_context.h" + +#include + +using namespace opentelemetry; + +// Tests that GetCurrent returns the current context +TEST(RuntimeContextTest, GetCurrent) +{ + std::map map_test = {{"test_key", (int64_t)123}}; + context::Context test_context = context::Context(map_test); + context::ContextToken old_context = context::RuntimeContext::Attach(&test_context); + EXPECT_TRUE(*context::RuntimeContext::GetCurrent() == test_context); + context::RuntimeContext::Detach(old_context); +} + +// Tests that detach resets the context to the previous context +TEST(RuntimeContextTest, Detach) +{ + std::map map_test = {{"test_key", (int64_t)123}}; + context::Context test_context = context::Context(map_test); + context::Context foo_context = context::Context(map_test); + + context::ContextToken test_context_token = context::RuntimeContext::Attach(&test_context); + context::ContextToken foo_context_token = context::RuntimeContext::Attach(&foo_context); + + context::RuntimeContext::Detach(foo_context_token); + EXPECT_TRUE(*context::RuntimeContext::GetCurrent() == test_context); + context::RuntimeContext::Detach(test_context_token); +} + +// Tests that detach returns false when the wrong context is provided +TEST(RuntimeContextTest, DetachWrongContext) +{ + std::map map_test = {{"test_key", (int64_t)123}}; + context::Context test_context = context::Context(map_test); + context::Context foo_context = context::Context(map_test); + context::ContextToken test_context_token = context::RuntimeContext::Attach(&test_context); + context::ContextToken foo_context_token = context::RuntimeContext::Attach(&foo_context); + EXPECT_FALSE(context::RuntimeContext::Detach(test_context_token)); + context::RuntimeContext::Detach(foo_context_token); + context::RuntimeContext::Detach(test_context_token); +} + +// Tests that the ThreadLocalContext can handle three attached contexts +TEST(RuntimeContextTest, ThreeAttachDetach) +{ + std::map map_test = {{"test_key", (int64_t)123}}; + context::Context test_context = context::Context(map_test); + context::Context foo_context = context::Context(map_test); + context::Context other_context = context::Context(map_test); + context::ContextToken test_context_token = context::RuntimeContext::Attach(&test_context); + context::ContextToken foo_context_token = context::RuntimeContext::Attach(&foo_context); + context::ContextToken other_context_token = context::RuntimeContext::Attach(&other_context); + + EXPECT_TRUE(context::RuntimeContext::Detach(other_context_token)); + EXPECT_TRUE(context::RuntimeContext::Detach(foo_context_token)); + EXPECT_TRUE(context::RuntimeContext::Detach(test_context_token)); +} diff --git a/api/test/context/threadlocal_context_test.cc b/api/test/context/threadlocal_context_test.cc deleted file mode 100644 index 2927e4a902..0000000000 --- a/api/test/context/threadlocal_context_test.cc +++ /dev/null @@ -1,65 +0,0 @@ -#include "opentelemetry/context/threadlocal_context.h" - -#include - -using namespace opentelemetry; - -// Tests that GetCurrent returns the current context -TEST(ThreadLocalContextTest, ThreadLocalGetCurrent) -{ - std::map map_test = {{"test_key", (int64_t)123}}; - context::Context test_context = context::Context(map_test); - context::ThreadLocalContext::Token old_context = - context::ThreadLocalContext::Attach(&test_context); - EXPECT_TRUE(*context::ThreadLocalContext::GetCurrent() == test_context); - context::ThreadLocalContext::Detach(old_context); -} - -// Tests that detach resets the context to the previous context -TEST(ThreadLocalContextTest, ThreadLocalDetach) -{ - std::map map_test = {{"test_key", (int64_t)123}}; - context::Context test_context = context::Context(map_test); - context::Context foo_context = context::Context(map_test); - context::ThreadLocalContext::Token test_context_token = - context::ThreadLocalContext::Attach(&test_context); - context::ThreadLocalContext::Token foo_context_token = - context::ThreadLocalContext::Attach(&foo_context); - context::ThreadLocalContext::Detach(foo_context_token); - EXPECT_TRUE(*context::ThreadLocalContext::GetCurrent() == test_context); - context::ThreadLocalContext::Detach(test_context_token); -} - -// Tests that detach returns false when the wrong context is provided -TEST(ThreadLocalContextTest, ThreadLocalDetachWrongContext) -{ - std::map map_test = {{"test_key", (int64_t)123}}; - context::Context test_context = context::Context(map_test); - context::Context foo_context = context::Context(map_test); - context::ThreadLocalContext::Token test_context_token = - context::ThreadLocalContext::Attach(&test_context); - context::ThreadLocalContext::Token foo_context_token = - context::ThreadLocalContext::Attach(&foo_context); - EXPECT_FALSE(context::ThreadLocalContext::Detach(test_context_token)); - context::ThreadLocalContext::Detach(foo_context_token); - context::ThreadLocalContext::Detach(test_context_token); -} - -// Tests that the ThreadLocalContext can handle three attached contexts -TEST(ThreadLocalContextTest, ThreadLocalThreeAttachDetach) -{ - std::map map_test = {{"test_key", (int64_t)123}}; - context::Context test_context = context::Context(map_test); - context::Context foo_context = context::Context(map_test); - context::Context other_context = context::Context(map_test); - context::ThreadLocalContext::Token test_context_token = - context::ThreadLocalContext::Attach(&test_context); - context::ThreadLocalContext::Token foo_context_token = - context::ThreadLocalContext::Attach(&foo_context); - context::ThreadLocalContext::Token other_context_token = - context::ThreadLocalContext::Attach(&other_context); - - EXPECT_TRUE(context::ThreadLocalContext::Detach(other_context_token)); - EXPECT_TRUE(context::ThreadLocalContext::Detach(foo_context_token)); - EXPECT_TRUE(context::ThreadLocalContext::Detach(test_context_token)); -} From c06edc559f792578b34f9b4d73c3ce7e8bcecd41 Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Thu, 23 Jul 2020 02:17:45 +0000 Subject: [PATCH 24/31] readding string_view_test --- api/test/nostd/string_view_test.cc | 104 +++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 api/test/nostd/string_view_test.cc diff --git a/api/test/nostd/string_view_test.cc b/api/test/nostd/string_view_test.cc new file mode 100644 index 0000000000..364410a312 --- /dev/null +++ b/api/test/nostd/string_view_test.cc @@ -0,0 +1,104 @@ +#include "opentelemetry/nostd/string_view.h" + +#include + +#include "map" + +using opentelemetry::nostd::string_view; + +TEST(StringViewTest, DefaultConstruction) +{ + string_view ref; + EXPECT_EQ(ref.data(), nullptr); + EXPECT_EQ(ref.length(), 0); +} + +TEST(StringViewTest, CStringInitialization) +{ + const char *val = "hello world"; + + string_view ref(val); + + EXPECT_EQ(ref.data(), val); + EXPECT_EQ(ref.length(), std::strlen(val)); +} + +TEST(StringViewTest, StdStringInitialization) +{ + const std::string val = "hello world"; + + string_view ref(val); + + EXPECT_EQ(ref.data(), val.data()); + EXPECT_EQ(ref.length(), val.size()); +} + +TEST(StringViewTest, Copy) +{ + const std::string val = "hello world"; + + string_view ref(val); + string_view cpy(ref); + + EXPECT_EQ(cpy.data(), val); + EXPECT_EQ(cpy.length(), val.length()); + EXPECT_EQ(cpy, val); +} + +TEST(StringViewTest, Accessor) +{ + string_view s = "abc123"; + EXPECT_EQ(s.data(), &s[0]); + EXPECT_EQ(s.data() + 1, &s[1]); +} + +TEST(StringViewTest, ExplicitStdStringConversion) +{ + std::string s = static_cast(string_view{"abc"}); + EXPECT_EQ(s, "abc"); +} + +TEST(StringViewTest, SubstrPortion) +{ + string_view s = "abc123"; + EXPECT_EQ("123", s.substr(3)); + EXPECT_EQ("12", s.substr(3, 2)); +} + +TEST(StringViewTest, SubstrOutOfRange) +{ + string_view s = "abc123"; +#if __EXCEPTIONS + EXPECT_THROW(s.substr(10), std::out_of_range); +#else + EXPECT_DEATH({ s.substr(10); }, ""); +#endif +} + +TEST(StringViewTest, Compare) +{ + string_view s1 = "aaa"; + string_view s2 = "bbb"; + string_view s3 = "aaa"; + + // Equals + EXPECT_EQ(s1, s3); + EXPECT_EQ(s1, s1); + + // Less then + EXPECT_LT(s1, s2); + + // Greater then + EXPECT_GT(s2, s1); +} + +TEST(StringViewTest, MapKeyOrdering) +{ + std::map m = {{"bbb", 2}, {"aaa", 1}, {"ccc", 3}}; + size_t i = 1; + for (const auto &kv : m) + { + EXPECT_EQ(kv.second, i); + i++; + } +} From fc42d91febdf460e4106461e6ba0baba54cc50bb Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Thu, 23 Jul 2020 19:30:06 +0000 Subject: [PATCH 25/31] threadlocalcontext is once again a derived class of RuntimeContext and ContextConfig has been removed --- .../opentelemetry/context/context_config.h | 17 ------- .../opentelemetry/context/runtime_context.h | 46 +++++++++++++++---- .../context/threadlocal_context.h | 35 +++++--------- api/test/context/runtime_context_test.cc | 22 +++++---- 4 files changed, 61 insertions(+), 59 deletions(-) delete mode 100644 api/include/opentelemetry/context/context_config.h diff --git a/api/include/opentelemetry/context/context_config.h b/api/include/opentelemetry/context/context_config.h deleted file mode 100644 index 4c76131ca7..0000000000 --- a/api/include/opentelemetry/context/context_config.h +++ /dev/null @@ -1,17 +0,0 @@ -#pragma once - -//#include "opentelemetry/context/threadlocal_context.h" -#include "threadlocal_context.h" -OPENTELEMETRY_BEGIN_NAMESPACE -namespace context -{ -// The ContextType is used to determine the implementation that will be used -// for the RuntimeContext. It defaults to the ThreadLocalContext object. -typedef ThreadLocalContext ContextType; - -// The token type provides an identifier that is used by -// the attach and detach methods to keep track of context -// objects. -typedef ThreadLocalContext::Token ContextToken; -} // namespace context -OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/context/runtime_context.h b/api/include/opentelemetry/context/runtime_context.h index 8358d72d5f..6f996f6a0c 100644 --- a/api/include/opentelemetry/context/runtime_context.h +++ b/api/include/opentelemetry/context/runtime_context.h @@ -1,30 +1,58 @@ #pragma once #include "opentelemetry/context/context.h" -#include "opentelemetry/context/context_config.h" OPENTELEMETRY_BEGIN_NAMESPACE namespace context { - +// 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. class RuntimeContext { public: + class Token + { + public: + bool operator==(const Context &other) { return (*context_ == other); } + + // A constructor that sets the token's Context object to the + // one that was passed in. + Token(Context *context) { context_ = context; } + + Token() = default; + + private: + friend class RuntimeContext; + + Context *context_; + + // Returns the stored context object. + Context *GetContext() { return context_; } + }; + // Return the current context. - static Context *GetCurrent() { return context_handler_.GetCurrent(); } + static Context *GetCurrent() { return context_handler_->InternalGetCurrent(); } + + // Dummy method to be overriden by derived class implementation + virtual Context *InternalGetCurrent() { return nullptr; } // Sets the current 'Context' object. Returns a token // that can be used to reset to the previous Context. - static ContextToken Attach(Context *context) { return context_handler_.Attach(context); } + static Token Attach(Context *context) { return context_handler_->InternalAttach(context); } + + // Dummy method to be overriden by derived class implementation + virtual Token InternalAttach(Context *context) { return Token(); } // Resets the context to a previous value stored in the // passed in token. Returns true if successful, false otherwise - static bool Detach(ContextToken &token) { return context_handler_.Detach(token); } + static bool Detach(Token &token) { return context_handler_->InternalDetach(token); } - static ContextType context_handler_; -}; - -ContextType RuntimeContext::context_handler_ = ContextType(); + // Dummy method to be overriden by derived class implementation + virtual bool InternalDetach(Token &token) { return false; } + static RuntimeContext *context_handler_; +}; } // namespace context OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/context/threadlocal_context.h b/api/include/opentelemetry/context/threadlocal_context.h index 626b536e17..6986e915d9 100644 --- a/api/include/opentelemetry/context/threadlocal_context.h +++ b/api/include/opentelemetry/context/threadlocal_context.h @@ -5,40 +5,25 @@ 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. -class ThreadLocalContext +// This file must be included to use the RuntimeContext class if another +// implementation has not been registered. +class ThreadLocalContext : public RuntimeContext { public: - ThreadLocalContext() {} - - // The token class provides an identifier that is used by - // the attach and detach methods to keep track of context - // objects. - class Token - { - private: - friend class ThreadLocalContext; - - Context *context_; - - // A constructor that sets the token's Context object to the - // one that was passed in. - Token(Context *context) { context_ = context; } - - // Returns the stored context object. - Context *GetContext() { return context_; } - }; + ThreadLocalContext() = default; // Return the current context. - Context *GetCurrent() { return stack_.Top(); } + Context *InternalGetCurrent() override { return stack_.Top(); } // Resets the context to a previous value stored in the // passed in token. Returns true if successful, false otherwise - bool Detach(Token &token) + bool InternalDetach(Token &token) override { - if (!(token.GetContext() == stack_.Top())) + if (!(token == *stack_.Top())) { return false; } @@ -49,7 +34,7 @@ class ThreadLocalContext // Sets the current 'Context' object. Returns a token // that can be used to reset to the previous Context. - Token Attach(Context *context) + Token InternalAttach(Context *context) override { stack_.Push(context); Token old_context = Token(context); @@ -128,5 +113,7 @@ class ThreadLocalContext }; 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 diff --git a/api/test/context/runtime_context_test.cc b/api/test/context/runtime_context_test.cc index c34e44a119..ed7f960f45 100644 --- a/api/test/context/runtime_context_test.cc +++ b/api/test/context/runtime_context_test.cc @@ -1,4 +1,4 @@ -#include "opentelemetry/context/runtime_context.h" +#include "opentelemetry/context/threadlocal_context.h" #include @@ -9,7 +9,7 @@ TEST(RuntimeContextTest, GetCurrent) { std::map map_test = {{"test_key", (int64_t)123}}; context::Context test_context = context::Context(map_test); - context::ContextToken old_context = context::RuntimeContext::Attach(&test_context); + context::RuntimeContext::Token old_context = context::RuntimeContext::Attach(&test_context); EXPECT_TRUE(*context::RuntimeContext::GetCurrent() == test_context); context::RuntimeContext::Detach(old_context); } @@ -21,8 +21,9 @@ TEST(RuntimeContextTest, Detach) context::Context test_context = context::Context(map_test); context::Context foo_context = context::Context(map_test); - context::ContextToken test_context_token = context::RuntimeContext::Attach(&test_context); - context::ContextToken foo_context_token = context::RuntimeContext::Attach(&foo_context); + context::RuntimeContext::Token test_context_token = + context::RuntimeContext::Attach(&test_context); + context::RuntimeContext::Token foo_context_token = context::RuntimeContext::Attach(&foo_context); context::RuntimeContext::Detach(foo_context_token); EXPECT_TRUE(*context::RuntimeContext::GetCurrent() == test_context); @@ -35,8 +36,9 @@ TEST(RuntimeContextTest, DetachWrongContext) std::map map_test = {{"test_key", (int64_t)123}}; context::Context test_context = context::Context(map_test); context::Context foo_context = context::Context(map_test); - context::ContextToken test_context_token = context::RuntimeContext::Attach(&test_context); - context::ContextToken foo_context_token = context::RuntimeContext::Attach(&foo_context); + context::RuntimeContext::Token test_context_token = + context::RuntimeContext::Attach(&test_context); + context::RuntimeContext::Token foo_context_token = context::RuntimeContext::Attach(&foo_context); EXPECT_FALSE(context::RuntimeContext::Detach(test_context_token)); context::RuntimeContext::Detach(foo_context_token); context::RuntimeContext::Detach(test_context_token); @@ -49,9 +51,11 @@ TEST(RuntimeContextTest, ThreeAttachDetach) context::Context test_context = context::Context(map_test); context::Context foo_context = context::Context(map_test); context::Context other_context = context::Context(map_test); - context::ContextToken test_context_token = context::RuntimeContext::Attach(&test_context); - context::ContextToken foo_context_token = context::RuntimeContext::Attach(&foo_context); - context::ContextToken other_context_token = context::RuntimeContext::Attach(&other_context); + context::RuntimeContext::Token test_context_token = + context::RuntimeContext::Attach(&test_context); + context::RuntimeContext::Token foo_context_token = context::RuntimeContext::Attach(&foo_context); + context::RuntimeContext::Token other_context_token = + context::RuntimeContext::Attach(&other_context); EXPECT_TRUE(context::RuntimeContext::Detach(other_context_token)); EXPECT_TRUE(context::RuntimeContext::Detach(foo_context_token)); From ce7858506dda118d406e99abee217b95f6db57d0 Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Thu, 23 Jul 2020 19:47:33 +0000 Subject: [PATCH 26/31] protected virtual members --- .../opentelemetry/context/runtime_context.h | 26 +++++++++++-------- .../context/threadlocal_context.h | 3 ++- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/api/include/opentelemetry/context/runtime_context.h b/api/include/opentelemetry/context/runtime_context.h index 6f996f6a0c..03442bca5f 100644 --- a/api/include/opentelemetry/context/runtime_context.h +++ b/api/include/opentelemetry/context/runtime_context.h @@ -17,15 +17,15 @@ class RuntimeContext public: bool operator==(const Context &other) { return (*context_ == other); } + private: + friend class RuntimeContext; + // A constructor that sets the token's Context object to the // one that was passed in. Token(Context *context) { context_ = context; } Token() = default; - private: - friend class RuntimeContext; - Context *context_; // Returns the stored context object. @@ -35,24 +35,28 @@ class RuntimeContext // Return the current context. static Context *GetCurrent() { return context_handler_->InternalGetCurrent(); } - // Dummy method to be overriden by derived class implementation - virtual Context *InternalGetCurrent() { return nullptr; } - // Sets the current 'Context' object. Returns a token // that can be used to reset to the previous Context. static Token Attach(Context *context) { return context_handler_->InternalAttach(context); } - // Dummy method to be overriden by derived class implementation - virtual Token InternalAttach(Context *context) { return Token(); } - // Resets the context to a previous value stored in the // passed in token. Returns true if successful, false otherwise static bool Detach(Token &token) { return context_handler_->InternalDetach(token); } + static RuntimeContext *context_handler_; + +protected: + // Provides a token with the passed in context + Token CreateToken(Context *context) { return Token(context); } + // Dummy method to be overriden by derived class implementation - virtual bool InternalDetach(Token &token) { return false; } + virtual Context *InternalGetCurrent() { return nullptr; } - static RuntimeContext *context_handler_; + // Dummy method to be overriden by derived class implementation + virtual Token InternalAttach(Context *context) { return Token(); } + + // Dummy method to be overriden by derived class implementation + virtual bool InternalDetach(Token &token) { return false; } }; } // namespace context OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/context/threadlocal_context.h b/api/include/opentelemetry/context/threadlocal_context.h index 6986e915d9..2ed1137d56 100644 --- a/api/include/opentelemetry/context/threadlocal_context.h +++ b/api/include/opentelemetry/context/threadlocal_context.h @@ -1,5 +1,6 @@ #pragma once +#include "opentelemetry/context/context.h" #include "opentelemetry/context/runtime_context.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -37,7 +38,7 @@ class ThreadLocalContext : public RuntimeContext Token InternalAttach(Context *context) override { stack_.Push(context); - Token old_context = Token(context); + Token old_context = CreateToken(context); return old_context; } From 02dbbdf9b33cf807d099cd52f044e9a45923f68d Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Thu, 23 Jul 2020 19:51:34 +0000 Subject: [PATCH 27/31] reset string_view_test --- api/test/nostd/string_view_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/test/nostd/string_view_test.cc b/api/test/nostd/string_view_test.cc index 364410a312..074d3bee49 100644 --- a/api/test/nostd/string_view_test.cc +++ b/api/test/nostd/string_view_test.cc @@ -95,7 +95,7 @@ TEST(StringViewTest, Compare) TEST(StringViewTest, MapKeyOrdering) { std::map m = {{"bbb", 2}, {"aaa", 1}, {"ccc", 3}}; - size_t i = 1; + size_t i = 1; for (const auto &kv : m) { EXPECT_EQ(kv.second, i); From 5a5df6e8849131336377ac01cefe5bff25647542 Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Fri, 24 Jul 2020 01:26:03 +0000 Subject: [PATCH 28/31] added destructor that detaches the context to the Token --- api/include/opentelemetry/context/runtime_context.h | 2 ++ api/include/opentelemetry/context/threadlocal_context.h | 6 ++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/api/include/opentelemetry/context/runtime_context.h b/api/include/opentelemetry/context/runtime_context.h index 03442bca5f..1b4d3c8223 100644 --- a/api/include/opentelemetry/context/runtime_context.h +++ b/api/include/opentelemetry/context/runtime_context.h @@ -17,6 +17,8 @@ class RuntimeContext public: bool operator==(const Context &other) { return (*context_ == other); } + ~Token() { Detach(*this); } + private: friend class RuntimeContext; diff --git a/api/include/opentelemetry/context/threadlocal_context.h b/api/include/opentelemetry/context/threadlocal_context.h index 2ed1137d56..f9e9cd278c 100644 --- a/api/include/opentelemetry/context/threadlocal_context.h +++ b/api/include/opentelemetry/context/threadlocal_context.h @@ -23,12 +23,14 @@ class ThreadLocalContext : public RuntimeContext // passed in token. Returns true if successful, false otherwise bool InternalDetach(Token &token) override { - + if (stack_.Top() == nullptr) + { + return false; + } if (!(token == *stack_.Top())) { return false; } - stack_.Pop(); return true; } From 1d7549f7eb29630b96692a8e8e5248432d4b4045 Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Sat, 25 Jul 2020 14:52:23 +0000 Subject: [PATCH 29/31] minor changes and switch from RuntimeContext accepting a ptr --- api/include/opentelemetry/context/context.h | 3 +- .../opentelemetry/context/runtime_context.h | 23 +++++----- .../context/threadlocal_context.h | 42 ++++++++++--------- api/test/context/runtime_context_test.cc | 24 +++++------ 4 files changed, 44 insertions(+), 48 deletions(-) diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index a50e32e0cd..a1379cdc64 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -16,6 +16,7 @@ class Context { public: + Context() = default; // Creates a context object from a map of keys and identifiers, this will // hold a shared_ptr to the head of the DataList linked list template @@ -92,8 +93,6 @@ class Context bool operator==(const Context &other) { return (head_ == other.head_); } private: - Context() = default; - // A linked list to contain the keys and values of this context node class DataList { diff --git a/api/include/opentelemetry/context/runtime_context.h b/api/include/opentelemetry/context/runtime_context.h index 1b4d3c8223..34f8f5b2d8 100644 --- a/api/include/opentelemetry/context/runtime_context.h +++ b/api/include/opentelemetry/context/runtime_context.h @@ -15,7 +15,7 @@ class RuntimeContext class Token { public: - bool operator==(const Context &other) { return (*context_ == other); } + bool operator==(const Context &other) { return context_ == other; } ~Token() { Detach(*this); } @@ -24,22 +24,22 @@ class RuntimeContext // A constructor that sets the token's Context object to the // one that was passed in. - Token(Context *context) { context_ = context; } + Token(Context context) { context_ = context; } Token() = default; - Context *context_; + Context context_; // Returns the stored context object. - Context *GetContext() { return context_; } + Context GetContext() { return context_; } }; // Return the current context. - static Context *GetCurrent() { return context_handler_->InternalGetCurrent(); } + static Context GetCurrent() { return context_handler_->InternalGetCurrent(); } // Sets the current 'Context' object. Returns a token // that can be used to reset to the previous Context. - static Token Attach(Context *context) { return context_handler_->InternalAttach(context); } + static Token Attach(Context context) { return context_handler_->InternalAttach(context); } // Resets the context to a previous value stored in the // passed in token. Returns true if successful, false otherwise @@ -49,16 +49,13 @@ class RuntimeContext protected: // Provides a token with the passed in context - Token CreateToken(Context *context) { return Token(context); } + Token CreateToken(Context context) { return Token(context); } - // Dummy method to be overriden by derived class implementation - virtual Context *InternalGetCurrent() { return nullptr; } + virtual Context InternalGetCurrent() = 0; - // Dummy method to be overriden by derived class implementation - virtual Token InternalAttach(Context *context) { return Token(); } + virtual Token InternalAttach(Context context) = 0; - // Dummy method to be overriden by derived class implementation - virtual bool InternalDetach(Token &token) { return false; } + virtual bool InternalDetach(Token &token) = 0; }; } // namespace context OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/context/threadlocal_context.h b/api/include/opentelemetry/context/threadlocal_context.h index f9e9cd278c..dec424279e 100644 --- a/api/include/opentelemetry/context/threadlocal_context.h +++ b/api/include/opentelemetry/context/threadlocal_context.h @@ -17,17 +17,13 @@ class ThreadLocalContext : public RuntimeContext ThreadLocalContext() = default; // Return the current context. - Context *InternalGetCurrent() override { return stack_.Top(); } + Context InternalGetCurrent() 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) override { - if (stack_.Top() == nullptr) - { - return false; - } - if (!(token == *stack_.Top())) + if (!(token == stack_.Top())) { return false; } @@ -37,7 +33,7 @@ class ThreadLocalContext : public RuntimeContext // Sets the current 'Context' object. Returns a token // that can be used to reset to the previous Context. - Token InternalAttach(Context *context) override + Token InternalAttach(Context context) override { stack_.Push(context); Token old_context = CreateToken(context); @@ -57,31 +53,31 @@ class ThreadLocalContext : public RuntimeContext base_ = nullptr; } - // Pops the top Context* off the stack and returns it. - Context *Pop() + // Pops the top Context off the stack and returns it. + Context Pop() { if (size_ <= 0) { - return nullptr; + return Context(); } int index = size_ - 1; size_--; return base_[index]; } - // Returns the Context* at the top of the stack. - Context *Top() + // Returns the Context at the top of the stack. + Context Top() { if (size_ <= 0) { - return nullptr; + 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) + void Push(Context context) { size_++; if (size_ > capacity_) @@ -99,17 +95,23 @@ class ThreadLocalContext : public RuntimeContext { new_capacity = 2; } - Context **temp = new Context *[new_capacity]; - memcpy(temp, base_, sizeof(Context *) * old_size); - delete[] base_; + Context *temp = new Context[new_capacity]; + if (base_ != nullptr) + { + for (auto i = 0; i <= old_size; i++) + { + temp[i] = base_[i]; + } + delete[] base_; + } base_ = temp; } ~Stack() { delete[] base_; } - int size_; - int capacity_; - Context **base_; + size_t size_; + size_t capacity_; + Context *base_; }; static thread_local Stack stack_; diff --git a/api/test/context/runtime_context_test.cc b/api/test/context/runtime_context_test.cc index ed7f960f45..b70d69dce8 100644 --- a/api/test/context/runtime_context_test.cc +++ b/api/test/context/runtime_context_test.cc @@ -1,3 +1,4 @@ +#include "opentelemetry/context/context.h" #include "opentelemetry/context/threadlocal_context.h" #include @@ -9,8 +10,8 @@ TEST(RuntimeContextTest, GetCurrent) { std::map map_test = {{"test_key", (int64_t)123}}; context::Context test_context = context::Context(map_test); - context::RuntimeContext::Token old_context = context::RuntimeContext::Attach(&test_context); - EXPECT_TRUE(*context::RuntimeContext::GetCurrent() == test_context); + context::RuntimeContext::Token old_context = context::RuntimeContext::Attach(test_context); + EXPECT_TRUE(context::RuntimeContext::GetCurrent() == test_context); context::RuntimeContext::Detach(old_context); } @@ -21,12 +22,11 @@ TEST(RuntimeContextTest, Detach) context::Context test_context = context::Context(map_test); context::Context foo_context = context::Context(map_test); - context::RuntimeContext::Token test_context_token = - context::RuntimeContext::Attach(&test_context); - context::RuntimeContext::Token foo_context_token = context::RuntimeContext::Attach(&foo_context); + context::RuntimeContext::Token test_context_token = context::RuntimeContext::Attach(test_context); + context::RuntimeContext::Token foo_context_token = context::RuntimeContext::Attach(foo_context); context::RuntimeContext::Detach(foo_context_token); - EXPECT_TRUE(*context::RuntimeContext::GetCurrent() == test_context); + EXPECT_TRUE(context::RuntimeContext::GetCurrent() == test_context); context::RuntimeContext::Detach(test_context_token); } @@ -36,9 +36,8 @@ TEST(RuntimeContextTest, DetachWrongContext) std::map map_test = {{"test_key", (int64_t)123}}; context::Context test_context = context::Context(map_test); context::Context foo_context = context::Context(map_test); - context::RuntimeContext::Token test_context_token = - context::RuntimeContext::Attach(&test_context); - context::RuntimeContext::Token foo_context_token = context::RuntimeContext::Attach(&foo_context); + context::RuntimeContext::Token test_context_token = context::RuntimeContext::Attach(test_context); + context::RuntimeContext::Token foo_context_token = context::RuntimeContext::Attach(foo_context); EXPECT_FALSE(context::RuntimeContext::Detach(test_context_token)); context::RuntimeContext::Detach(foo_context_token); context::RuntimeContext::Detach(test_context_token); @@ -51,11 +50,10 @@ TEST(RuntimeContextTest, ThreeAttachDetach) context::Context test_context = context::Context(map_test); context::Context foo_context = context::Context(map_test); context::Context other_context = context::Context(map_test); - context::RuntimeContext::Token test_context_token = - context::RuntimeContext::Attach(&test_context); - context::RuntimeContext::Token foo_context_token = context::RuntimeContext::Attach(&foo_context); + context::RuntimeContext::Token test_context_token = context::RuntimeContext::Attach(test_context); + context::RuntimeContext::Token foo_context_token = context::RuntimeContext::Attach(foo_context); context::RuntimeContext::Token other_context_token = - context::RuntimeContext::Attach(&other_context); + context::RuntimeContext::Attach(other_context); EXPECT_TRUE(context::RuntimeContext::Detach(other_context_token)); EXPECT_TRUE(context::RuntimeContext::Detach(foo_context_token)); From 090fbe7801dfd426b78b769ecbad82b249bf852d Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Tue, 28 Jul 2020 00:46:31 +0000 Subject: [PATCH 30/31] fixed nits --- .../opentelemetry/context/runtime_context.h | 28 ++++++++--------- .../context/threadlocal_context.h | 30 +++++++------------ 2 files changed, 25 insertions(+), 33 deletions(-) diff --git a/api/include/opentelemetry/context/runtime_context.h b/api/include/opentelemetry/context/runtime_context.h index 34f8f5b2d8..7ad3b73bdc 100644 --- a/api/include/opentelemetry/context/runtime_context.h +++ b/api/include/opentelemetry/context/runtime_context.h @@ -15,47 +15,47 @@ class RuntimeContext class Token { public: - bool operator==(const Context &other) { return context_ == other; } + bool operator==(const Context &other) noexcept { return context_ == other; } - ~Token() { Detach(*this); } + ~Token() noexcept { Detach(*this); } private: friend class RuntimeContext; // A constructor that sets the token's Context object to the // one that was passed in. - Token(Context context) { context_ = context; } + Token(Context context) noexcept : context_(context){}; - Token() = default; + Token() noexcept = default; Context context_; - - // Returns the stored context object. - Context GetContext() { return context_; } }; // Return the current context. - static Context GetCurrent() { return context_handler_->InternalGetCurrent(); } + static Context GetCurrent() noexcept { return context_handler_->InternalGetCurrent(); } // Sets the current 'Context' object. Returns a token // that can be used to reset to the previous Context. - static Token Attach(Context context) { return context_handler_->InternalAttach(context); } + static Token Attach(Context context) noexcept + { + return context_handler_->InternalAttach(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) { return context_handler_->InternalDetach(token); } + static bool Detach(Token &token) noexcept { return context_handler_->InternalDetach(token); } static RuntimeContext *context_handler_; protected: // Provides a token with the passed in context - Token CreateToken(Context context) { return Token(context); } + Token CreateToken(Context context) noexcept { return Token(context); } - virtual Context InternalGetCurrent() = 0; + virtual Context InternalGetCurrent() noexcept = 0; - virtual Token InternalAttach(Context context) = 0; + virtual Token InternalAttach(Context context) noexcept = 0; - virtual bool InternalDetach(Token &token) = 0; + virtual bool InternalDetach(Token &token) noexcept = 0; }; } // namespace context OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/context/threadlocal_context.h b/api/include/opentelemetry/context/threadlocal_context.h index dec424279e..9be14bcd32 100644 --- a/api/include/opentelemetry/context/threadlocal_context.h +++ b/api/include/opentelemetry/context/threadlocal_context.h @@ -14,14 +14,14 @@ namespace context class ThreadLocalContext : public RuntimeContext { public: - ThreadLocalContext() = default; + ThreadLocalContext() noexcept = default; // Return the current context. - Context InternalGetCurrent() override { return stack_.Top(); } + 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) override + bool InternalDetach(Token &token) noexcept override { if (!(token == stack_.Top())) { @@ -33,7 +33,7 @@ class ThreadLocalContext : public RuntimeContext // Sets the current 'Context' object. Returns a token // that can be used to reset to the previous Context. - Token InternalAttach(Context context) override + Token InternalAttach(Context context) noexcept override { stack_.Push(context); Token old_context = CreateToken(context); @@ -46,15 +46,10 @@ class ThreadLocalContext : public RuntimeContext { friend class ThreadLocalContext; - Stack() - { - size_ = 0; - capacity_ = 0; - base_ = nullptr; - } + Stack() noexcept : size_(0), capacity_(0), base_(nullptr){}; // Pops the top Context off the stack and returns it. - Context Pop() + Context Pop() noexcept { if (size_ <= 0) { @@ -66,7 +61,7 @@ class ThreadLocalContext : public RuntimeContext } // Returns the Context at the top of the stack. - Context Top() + Context Top() noexcept const { if (size_ <= 0) { @@ -77,7 +72,7 @@ class ThreadLocalContext : public RuntimeContext // Pushes the passed in context pointer to the top of the stack // and resizes if necessary. - void Push(Context context) + void Push(Context context) noexcept { size_++; if (size_ > capacity_) @@ -88,7 +83,7 @@ class ThreadLocalContext : public RuntimeContext } // Reallocates the storage array to the pass in new capacity size. - void Resize(int new_capacity) + void Resize(int new_capacity) noexcept { int old_size = size_ - 1; if (new_capacity == 0) @@ -98,16 +93,13 @@ class ThreadLocalContext : public RuntimeContext Context *temp = new Context[new_capacity]; if (base_ != nullptr) { - for (auto i = 0; i <= old_size; i++) - { - temp[i] = base_[i]; - } + std::copy(base_, base_ + old_size, temp); delete[] base_; } base_ = temp; } - ~Stack() { delete[] base_; } + ~Stack() noexcept { delete[] base_; } size_t size_; size_t capacity_; From 2ba7e022037dd9860f50a9ab721f046cf2ca3e40 Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Tue, 28 Jul 2020 00:52:16 +0000 Subject: [PATCH 31/31] added const to Top --- api/include/opentelemetry/context/threadlocal_context.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/include/opentelemetry/context/threadlocal_context.h b/api/include/opentelemetry/context/threadlocal_context.h index 9be14bcd32..febd89e63a 100644 --- a/api/include/opentelemetry/context/threadlocal_context.h +++ b/api/include/opentelemetry/context/threadlocal_context.h @@ -61,7 +61,7 @@ class ThreadLocalContext : public RuntimeContext } // Returns the Context at the top of the stack. - Context Top() noexcept const + Context Top() const noexcept { if (size_ <= 0) {