From 4128a69bf191ed9ac4fa3b96ed67ce32c0bd37ff Mon Sep 17 00:00:00 2001 From: Sam Atac Date: Mon, 6 Jul 2020 19:35:55 +0000 Subject: [PATCH 01/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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 }