From 958d3a1be02bc1ac87c301e334c780ab1ed0748c Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Mon, 16 Oct 2023 23:05:36 +0200 Subject: [PATCH 1/2] Fixes #2032 --- api/include/opentelemetry/trace/noop.h | 15 ++- .../opentelemetry/trace/tracer_provider.h | 99 ++++++++++++++++++- api/test/singleton/singleton_test.cc | 16 ++- api/test/trace/provider_test.cc | 17 +++- .../opentelemetry/sdk/trace/tracer_provider.h | 19 +++- sdk/src/trace/tracer_provider.cc | 37 ++++--- 6 files changed, 180 insertions(+), 23 deletions(-) diff --git a/api/include/opentelemetry/trace/noop.h b/api/include/opentelemetry/trace/noop.h index 2a2b7312ba..c6a5838867 100644 --- a/api/include/opentelemetry/trace/noop.h +++ b/api/include/opentelemetry/trace/noop.h @@ -108,12 +108,23 @@ class OPENTELEMETRY_EXPORT NoopTracerProvider final : public trace::TracerProvid : tracer_{nostd::shared_ptr(new trace::NoopTracer)} {} - nostd::shared_ptr GetTracer(nostd::string_view /* library_name */, - nostd::string_view /* library_version */, +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + nostd::shared_ptr GetTracer( + nostd::string_view /* name */, + nostd::string_view /* version */, + nostd::string_view /* schema_url */, + const common::KeyValueIterable * /* attributes */) noexcept override + { + return tracer_; + } +#else + nostd::shared_ptr GetTracer(nostd::string_view /* name */, + nostd::string_view /* version */, nostd::string_view /* schema_url */) noexcept override { return tracer_; } +#endif private: nostd::shared_ptr tracer_; diff --git a/api/include/opentelemetry/trace/tracer_provider.h b/api/include/opentelemetry/trace/tracer_provider.h index bc8ff8da89..059298d395 100644 --- a/api/include/opentelemetry/trace/tracer_provider.h +++ b/api/include/opentelemetry/trace/tracer_provider.h @@ -3,6 +3,8 @@ #pragma once +#include "opentelemetry/common/key_value_iterable.h" +#include "opentelemetry/common/key_value_iterable_view.h" #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/version.h" @@ -20,15 +22,106 @@ class OPENTELEMETRY_EXPORT TracerProvider { public: virtual ~TracerProvider() = default; + +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + + /** + * Gets or creates a named Tracer instance (ABI). + * + * @since ABI_VERSION 2 + * + * @param[in] name Tracer instrumentation scope + * @param[in] version Instrumentation scope version + * @param[in] schema_url Instrumentation scope schema URL + * @param[in] attributes Instrumentation scope attributes (optional, may be nullptr) + */ + virtual nostd::shared_ptr GetTracer( + nostd::string_view name, + nostd::string_view version, + nostd::string_view schema_url, + const common::KeyValueIterable *attributes) noexcept = 0; + + /** + * Gets or creates a named Tracer instance (API helper). + * + * @since ABI_VERSION 2 + * + * @param[in] name Tracer instrumentation scope + * @param[in] version Instrumentation scope version, optional + * @param[in] schema_url Instrumentation scope schema URL, optional + */ + nostd::shared_ptr GetTracer(nostd::string_view name, + nostd::string_view version = "", + nostd::string_view schema_url = "") + { + return GetTracer(name, version, schema_url, nullptr); + } + + /** + * Gets or creates a named Tracer instance (API helper). + * + * @since ABI_VERSION 2 + * + * @param[in] name Tracer instrumentation scope + * @param[in] version Instrumentation scope version + * @param[in] schema_url Instrumentation scope schema URL + * @param[in] attributes Instrumentation scope attributes + */ + nostd::shared_ptr GetTracer( + nostd::string_view name, + nostd::string_view version, + nostd::string_view schema_url, + std::initializer_list> attributes) + { + /* Build a container from std::initializer_list. */ + nostd::span> attributes_span{ + attributes.begin(), attributes.end()}; + + /* Build a view on the container. */ + common::KeyValueIterableView< + nostd::span>> + iterable_attributes{attributes_span}; + + /* Add attributes using the view. */ + return GetTracer(name, version, schema_url, &iterable_attributes); + } + + /** + * Gets or creates a named Tracer instance (API helper). + * + * @since ABI_VERSION 2 + * + * @param[in] name Tracer instrumentation scope + * @param[in] version Instrumentation scope version + * @param[in] schema_url Instrumentation scope schema URL + * @param[in] attributes Instrumentation scope attributes container + */ + template ::value> * = nullptr> + nostd::shared_ptr GetTracer(nostd::string_view name, + nostd::string_view version, + nostd::string_view schema_url, + const T &attributes) + { + /* Build a view on the container. */ + common::KeyValueIterableView iterable_attributes(attributes); + + /* Add attributes using the view. */ + return GetTracer(name, version, schema_url, &iterable_attributes); + } + +#else + /** * Gets or creates a named tracer instance. * * Optionally a version can be passed to create a named and versioned tracer * instance. */ - virtual nostd::shared_ptr GetTracer(nostd::string_view library_name, - nostd::string_view library_version = "", - nostd::string_view schema_url = "") noexcept = 0; + virtual nostd::shared_ptr GetTracer(nostd::string_view name, + nostd::string_view version = "", + nostd::string_view schema_url = "") noexcept = 0; +#endif }; } // namespace trace OPENTELEMETRY_END_NAMESPACE diff --git a/api/test/singleton/singleton_test.cc b/api/test/singleton/singleton_test.cc index 44a445e5bd..187e26f2b4 100644 --- a/api/test/singleton/singleton_test.cc +++ b/api/test/singleton/singleton_test.cc @@ -264,13 +264,25 @@ class MyTracerProvider : public trace::TracerProvider return result; } - nostd::shared_ptr GetTracer(nostd::string_view /* library_name */, - nostd::string_view /* library_version */, +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + nostd::shared_ptr GetTracer( + nostd::string_view /* name */, + nostd::string_view /* version */, + nostd::string_view /* schema_url */, + const common::KeyValueIterable * /* attributes */) noexcept override + { + nostd::shared_ptr result(new MyTracer()); + return result; + } +#else + nostd::shared_ptr GetTracer(nostd::string_view /* name */, + nostd::string_view /* version */, nostd::string_view /* schema_url */) noexcept override { nostd::shared_ptr result(new MyTracer()); return result; } +#endif }; void setup_otel() diff --git a/api/test/trace/provider_test.cc b/api/test/trace/provider_test.cc index 21e3619334..be2ca2842c 100644 --- a/api/test/trace/provider_test.cc +++ b/api/test/trace/provider_test.cc @@ -3,6 +3,7 @@ #include "opentelemetry/trace/provider.h" #include "opentelemetry/nostd/shared_ptr.h" +#include "opentelemetry/trace/tracer_provider.h" #include @@ -14,12 +15,24 @@ namespace nostd = opentelemetry::nostd; class TestProvider : public TracerProvider { - nostd::shared_ptr GetTracer(nostd::string_view /* library_name */, - nostd::string_view /* library_version */, + +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + nostd::shared_ptr GetTracer( + nostd::string_view /* name */, + nostd::string_view /* version */, + nostd::string_view /* schema_url */, + const opentelemetry::common::KeyValueIterable * /* attributes */) noexcept override + { + return nostd::shared_ptr(nullptr); + } +#else + nostd::shared_ptr GetTracer(nostd::string_view /* name */, + nostd::string_view /* version */, nostd::string_view /* schema_url */) noexcept override { return nostd::shared_ptr(nullptr); } +#endif }; TEST(Provider, GetTracerProviderDefault) diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h index 092db1eaf8..ce2a6d4401 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h @@ -63,10 +63,23 @@ class TracerProvider final : public opentelemetry::trace::TracerProvider ~TracerProvider() override; + /* + Make sure GetTracer() helpers from the API are seen in overload resolution. + */ + using opentelemetry::trace::TracerProvider::GetTracer; + +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 + opentelemetry::nostd::shared_ptr GetTracer( + nostd::string_view name, + nostd::string_view version, + nostd::string_view schema_url, + const opentelemetry::common::KeyValueIterable *attributes) noexcept override; +#else opentelemetry::nostd::shared_ptr GetTracer( - nostd::string_view library_name, - nostd::string_view library_version = "", - nostd::string_view schema_url = "") noexcept override; + nostd::string_view name, + nostd::string_view version = "", + nostd::string_view schema_url = "") noexcept override; +#endif /** * Attaches a span processor to list of configured processors for this tracer provider. diff --git a/sdk/src/trace/tracer_provider.cc b/sdk/src/trace/tracer_provider.cc index 13d56f9966..8569b63f23 100644 --- a/sdk/src/trace/tracer_provider.cc +++ b/sdk/src/trace/tracer_provider.cc @@ -53,17 +53,29 @@ TracerProvider::~TracerProvider() } } +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 nostd::shared_ptr TracerProvider::GetTracer( - nostd::string_view library_name, - nostd::string_view library_version, + nostd::string_view name, + nostd::string_view version, + nostd::string_view schema_url, + const opentelemetry::common::KeyValueIterable *attributes) noexcept +#else +nostd::shared_ptr TracerProvider::GetTracer( + nostd::string_view name, + nostd::string_view version, nostd::string_view schema_url) noexcept +#endif { - if (library_name.data() == nullptr) +#if OPENTELEMETRY_ABI_VERSION_NO < 2 + const opentelemetry::common::KeyValueIterable *attributes = nullptr; +#endif + + if (name.data() == nullptr) { OTEL_INTERNAL_LOG_ERROR("[TracerProvider::GetTracer] Library name is null."); - library_name = ""; + name = ""; } - else if (library_name == "") + else if (name == "") { OTEL_INTERNAL_LOG_ERROR("[TracerProvider::GetTracer] Library name is empty."); } @@ -72,17 +84,20 @@ nostd::shared_ptr TracerProvider::GetTracer( for (auto &tracer : tracers_) { - auto &tracer_lib = tracer->GetInstrumentationScope(); - if (tracer_lib.equal(library_name, library_version, schema_url)) + auto &tracer_scope = tracer->GetInstrumentationScope(); + if (tracer_scope.equal(name, version, schema_url)) { return nostd::shared_ptr{tracer}; } } - auto lib = InstrumentationScope::Create(library_name, library_version, schema_url); - tracers_.push_back(std::shared_ptr( - new sdk::trace::Tracer(context_, std::move(lib)))); - return nostd::shared_ptr{tracers_.back()}; + instrumentationscope::InstrumentationScopeAttributes attrs_map(attributes); + auto scope = + instrumentationscope::InstrumentationScope::Create(name, version, schema_url, attrs_map); + + auto tracer = std::shared_ptr(new Tracer(context_, std::move(scope))); + tracers_.push_back(tracer); + return nostd::shared_ptr{tracer}; } void TracerProvider::AddProcessor(std::unique_ptr processor) noexcept From ecf6db7f86807914f5661a7e948f96503de7c6f3 Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Wed, 18 Oct 2023 23:00:44 +0200 Subject: [PATCH 2/2] Added unit test, CHANGELOG --- CHANGELOG.md | 12 +++ sdk/test/trace/tracer_provider_test.cc | 137 +++++++++++++++++++++++++ 2 files changed, 149 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 48c379b7de..c267bb48da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,18 @@ Increment the: * [BUILD] Remove WITH_REMOVE_METER_PREVIEW, use WITH_ABI_VERSION_2 instead [#2370](https://github.com/open-telemetry/opentelemetry-cpp/pull/2370) +* [API] Add InstrumentationScope attributes in TracerProvider::GetTracer() + [#2371](https://github.com/open-telemetry/opentelemetry-cpp/pull/2371) + +Important changes: + +* [API] Add InstrumentationScope attributes in TracerProvider::GetTracer() + [#2371](https://github.com/open-telemetry/opentelemetry-cpp/pull/2371) + * TracerProvider::GetTracer() now accepts InstrumentationScope attributes. + * Because this is an `ABI` breaking change, the fix is only available + with the `CMake` option `WITH_ABI_VERSION_2=ON`. + * When building with `CMake` option `WITH_ABI_VERSION_1=ON` (by default) + the `ABI` is unchanged, and the fix is not available. Breaking changes: diff --git a/sdk/test/trace/tracer_provider_test.cc b/sdk/test/trace/tracer_provider_test.cc index 364efc0208..8e1b22fb62 100644 --- a/sdk/test/trace/tracer_provider_test.cc +++ b/sdk/test/trace/tracer_provider_test.cc @@ -78,6 +78,143 @@ TEST(TracerProvider, GetTracer) ASSERT_EQ(instrumentation_scope3.GetVersion(), "1.0.0"); } +#if OPENTELEMETRY_ABI_VERSION_NO >= 2 +TEST(TracerProvider, GetTracerAbiv2) +{ + std::unique_ptr processor(new SimpleSpanProcessor(nullptr)); + std::vector> processors; + processors.push_back(std::move(processor)); + std::unique_ptr context1( + new TracerContext(std::move(processors), Resource::Create({}))); + TracerProvider tp(std::move(context1)); + + auto t1 = tp.GetTracer("name1", "version1", "url1"); + ASSERT_NE(nullptr, t1); + + auto t2 = tp.GetTracer("name2", "version2", "url2", nullptr); + ASSERT_NE(nullptr, t2); + + auto t3 = tp.GetTracer("name3", "version3", "url3", {{"accept_single_attr", true}}); + ASSERT_NE(nullptr, t3); + { + auto tracer = static_cast(t3.get()); + auto scope = tracer->GetInstrumentationScope(); + auto attrs = scope.GetAttributes(); + ASSERT_EQ(attrs.size(), 1); + auto attr = attrs.find("accept_single_attr"); + ASSERT_FALSE(attr == attrs.end()); + ASSERT_TRUE(opentelemetry::nostd::holds_alternative(attr->second)); + EXPECT_EQ(opentelemetry::nostd::get(attr->second), true); + } + + std::pair attr4 = { + "accept_single_attr", true}; + auto t4 = tp.GetTracer("name4", "version4", "url4", {attr4}); + ASSERT_NE(nullptr, t4); + { + auto tracer = static_cast(t4.get()); + auto scope = tracer->GetInstrumentationScope(); + auto attrs = scope.GetAttributes(); + ASSERT_EQ(attrs.size(), 1); + auto attr = attrs.find("accept_single_attr"); + ASSERT_FALSE(attr == attrs.end()); + ASSERT_TRUE(opentelemetry::nostd::holds_alternative(attr->second)); + EXPECT_EQ(opentelemetry::nostd::get(attr->second), true); + } + + auto t5 = tp.GetTracer("name5", "version5", "url5", {{"foo", "1"}, {"bar", "2"}}); + ASSERT_NE(nullptr, t5); + { + auto tracer = static_cast(t5.get()); + auto scope = tracer->GetInstrumentationScope(); + auto attrs = scope.GetAttributes(); + ASSERT_EQ(attrs.size(), 2); + auto attr = attrs.find("bar"); + ASSERT_FALSE(attr == attrs.end()); + ASSERT_TRUE(opentelemetry::nostd::holds_alternative(attr->second)); + EXPECT_EQ(opentelemetry::nostd::get(attr->second), "2"); + } + + std::initializer_list< + std::pair> + attrs6 = {{"foo", "1"}, {"bar", 42}}; + + auto t6 = tp.GetTracer("name6", "version6", "url6", attrs6); + ASSERT_NE(nullptr, t6); + { + auto tracer = static_cast(t6.get()); + auto scope = tracer->GetInstrumentationScope(); + auto attrs = scope.GetAttributes(); + ASSERT_EQ(attrs.size(), 2); + auto attr = attrs.find("bar"); + ASSERT_FALSE(attr == attrs.end()); + ASSERT_TRUE(opentelemetry::nostd::holds_alternative(attr->second)); + EXPECT_EQ(opentelemetry::nostd::get(attr->second), 42); + } + + typedef std::pair KV; + + std::initializer_list attrs7 = {{"foo", 3.14}, {"bar", "2"}}; + auto t7 = tp.GetTracer("name7", "version7", "url7", attrs7); + ASSERT_NE(nullptr, t7); + { + auto tracer = static_cast(t7.get()); + auto scope = tracer->GetInstrumentationScope(); + auto attrs = scope.GetAttributes(); + ASSERT_EQ(attrs.size(), 2); + auto attr = attrs.find("foo"); + ASSERT_FALSE(attr == attrs.end()); + ASSERT_TRUE(opentelemetry::nostd::holds_alternative(attr->second)); + EXPECT_EQ(opentelemetry::nostd::get(attr->second), 3.14); + } + + auto t8 = tp.GetTracer("name8", "version8", "url8", + {{"a", "string"}, + {"b", false}, + {"c", 314159}, + {"d", (unsigned int)314159}, + {"e", (int32_t)-20}, + {"f", (uint32_t)20}, + {"g", (int64_t)-20}, + {"h", (uint64_t)20}, + {"i", 3.1}, + {"j", "string"}}); + ASSERT_NE(nullptr, t8); + { + auto tracer = static_cast(t8.get()); + auto scope = tracer->GetInstrumentationScope(); + auto attrs = scope.GetAttributes(); + ASSERT_EQ(attrs.size(), 10); + auto attr = attrs.find("e"); + ASSERT_FALSE(attr == attrs.end()); + ASSERT_TRUE(opentelemetry::nostd::holds_alternative(attr->second)); + EXPECT_EQ(opentelemetry::nostd::get(attr->second), -20); + } + + std::map attr9{ + {"a", "string"}, {"b", false}, {"c", 314159}, {"d", (unsigned int)314159}, + {"e", (int32_t)-20}, {"f", (uint32_t)20}, {"g", (int64_t)-20}, {"h", (uint64_t)20}, + {"i", 3.1}, {"j", "string"}}; + + auto t9 = tp.GetTracer("name9", "version9", "url9", attr9); + ASSERT_NE(nullptr, t9); + { + auto tracer = static_cast(t9.get()); + auto scope = tracer->GetInstrumentationScope(); + auto attrs = scope.GetAttributes(); + ASSERT_EQ(attrs.size(), 10); + auto attr = attrs.find("h"); + ASSERT_FALSE(attr == attrs.end()); + ASSERT_TRUE(opentelemetry::nostd::holds_alternative(attr->second)); + EXPECT_EQ(opentelemetry::nostd::get(attr->second), 20); + } + + // cleanup properly without crash + tp.ForceFlush(); + tp.Shutdown(); +} +#endif /* OPENTELEMETRY_ABI_VERSION_NO >= 2 */ + TEST(TracerProvider, Shutdown) { std::unique_ptr processor(new SimpleSpanProcessor(nullptr));