From 77d0e5c5832b0755a0ed6c8765a204088770e95f Mon Sep 17 00:00:00 2001 From: Tom Tan Date: Thu, 22 Apr 2021 15:49:47 -0700 Subject: [PATCH 01/11] Add InstrumentationLibrary to Tracer in API package --- api/include/opentelemetry/trace/tracer.h | 26 ++++++++++++++ api/test/CMakeLists.txt | 1 + api/test/instrumentationlibrary/BUILD | 12 +++++++ .../instrumentationlibrary/CMakeLists.txt | 12 +++++++ .../instrumentationlibrary_test.cc | 35 +++++++++++++++++++ sdk/include/opentelemetry/sdk/trace/tracer.h | 4 +-- sdk/src/trace/tracer_context.cc | 2 +- sdk/src/trace/tracer_provider.cc | 3 ++ 8 files changed, 92 insertions(+), 3 deletions(-) create mode 100644 api/test/instrumentationlibrary/BUILD create mode 100644 api/test/instrumentationlibrary/CMakeLists.txt create mode 100644 api/test/instrumentationlibrary/instrumentationlibrary_test.cc diff --git a/api/include/opentelemetry/trace/tracer.h b/api/include/opentelemetry/trace/tracer.h index e358100f4f..cacd6ce8fe 100644 --- a/api/include/opentelemetry/trace/tracer.h +++ b/api/include/opentelemetry/trace/tracer.h @@ -1,5 +1,6 @@ #pragma once +#include "opentelemetry/instrumentationlibrary/instrumentation_library.h" #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/nostd/unique_ptr.h" @@ -14,6 +15,9 @@ OPENTELEMETRY_BEGIN_NAMESPACE namespace trace { + +using InstrumentationLibrary = opentelemetry::instrumentationlibrary::InstrumentationLibrary; + /** * Handles span creation and in-process context propagation. * @@ -162,6 +166,25 @@ class Tracer } } + /** + * Get the instrumentation library associated with the current tracer. + * @return the instrumentation library for current tracer. + */ + InstrumentationLibrary *GetInstruemntationLibrary() const noexcept + { + return instrumentation_library_.get(); + } + + /** + * Set instrumentation library. + * @param instrumentation library to set on the current tracer. + */ + void SetInstrumentationLibrary( + nostd::unique_ptr &&instrumentation_library) noexcept + { + instrumentation_library_ = std::move(instrumentation_library); + } + /** * Force any buffered spans to flush. * @param timeout to complete the flush @@ -187,6 +210,9 @@ class Tracer } virtual void CloseWithMicroseconds(uint64_t timeout) noexcept = 0; + +private: + std::unique_ptr instrumentation_library_; }; } // namespace trace OPENTELEMETRY_END_NAMESPACE diff --git a/api/test/CMakeLists.txt b/api/test/CMakeLists.txt index eaf09e44e0..24f128694b 100644 --- a/api/test/CMakeLists.txt +++ b/api/test/CMakeLists.txt @@ -7,3 +7,4 @@ add_subdirectory(metrics) add_subdirectory(logs) add_subdirectory(common) add_subdirectory(baggage) +add_subdirectory(instrumentationlibrary) diff --git a/api/test/instrumentationlibrary/BUILD b/api/test/instrumentationlibrary/BUILD new file mode 100644 index 0000000000..98bb5d1697 --- /dev/null +++ b/api/test/instrumentationlibrary/BUILD @@ -0,0 +1,12 @@ +load("//bazel:otel_cc_benchmark.bzl", "otel_cc_benchmark") + +cc_test( + name = "instrumentationlibrary_test", + srcs = [ + "instrumentationlibrary_test.cc", + ], + deps = [ + "//api", + "@com_google_googletest//:gtest_main", + ], +) diff --git a/api/test/instrumentationlibrary/CMakeLists.txt b/api/test/instrumentationlibrary/CMakeLists.txt new file mode 100644 index 0000000000..d65599317d --- /dev/null +++ b/api/test/instrumentationlibrary/CMakeLists.txt @@ -0,0 +1,12 @@ +include(GoogleTest) + +foreach(testname instrumentationlibrary_test) + add_executable(${testname} "${testname}.cc") + target_link_libraries( + ${testname} ${GTEST_BOTH_LIBRARIES} ${CORE_RUNTIME_LIBS} + ${CMAKE_THREAD_LIBS_INIT} opentelemetry_api) + gtest_add_tests( + TARGET ${testname} + TEST_PREFIX baggage. + TEST_LIST ${testname}) +endforeach() diff --git a/api/test/instrumentationlibrary/instrumentationlibrary_test.cc b/api/test/instrumentationlibrary/instrumentationlibrary_test.cc new file mode 100644 index 0000000000..08e4acc20e --- /dev/null +++ b/api/test/instrumentationlibrary/instrumentationlibrary_test.cc @@ -0,0 +1,35 @@ +// Copyright 2021, OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "opentelemetry/nostd/string_view.h" + +#include +#include +#include + +#include "opentelemetry/instrumentationlibrary/instrumentation_library.h" + +using namespace opentelemetry; +using namespace opentelemetry::instrumentationlibrary; + +TEST(InstrumentationLibrary, CreateInstrumentationLibrary) +{ + + std::string library_name = "opentelemetry-cpp"; + std::string library_version = "0.1.0"; + auto instrumentation_library = InstrumentationLibrary::create(library_name, library_version); + + EXPECT_EQ(instrumentation_library->GetName(), library_name); + EXPECT_EQ(instrumentation_library->GetVersion(), library_version); +} diff --git a/sdk/include/opentelemetry/sdk/trace/tracer.h b/sdk/include/opentelemetry/sdk/trace/tracer.h index 39af27ff7f..f2054e1777 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer.h @@ -33,10 +33,10 @@ class Tracer final : public trace_api::Tracer, public std::enable_shared_from_th void CloseWithMicroseconds(uint64_t timeout) noexcept override; /** Returns the currently active span processor. */ - SpanProcessor &GetActiveProcessor() noexcept { return context_->GetActiveProcessor(); } + SpanProcessor &GetActiveProcessor() const noexcept { return context_->GetActiveProcessor(); } /** Returns the configured Id generator */ - IdGenerator &GetIdGenerator() noexcept { return context_->GetIdGenerator(); } + IdGenerator &GetIdGenerator() const noexcept { return context_->GetIdGenerator(); } // Note: Test only Sampler &GetSampler() { return context_->GetSampler(); } diff --git a/sdk/src/trace/tracer_context.cc b/sdk/src/trace/tracer_context.cc index 8dc5397dc9..c7bf1934e1 100644 --- a/sdk/src/trace/tracer_context.cc +++ b/sdk/src/trace/tracer_context.cc @@ -19,7 +19,7 @@ TracerContext::TracerContext(std::unique_ptr processor, Sampler &TracerContext::GetSampler() const noexcept { - return *sampler_.get(); + return *sampler_; } const opentelemetry::sdk::resource::Resource &TracerContext::GetResource() const noexcept diff --git a/sdk/src/trace/tracer_provider.cc b/sdk/src/trace/tracer_provider.cc index 479ba8a462..ee1ae54f47 100644 --- a/sdk/src/trace/tracer_provider.cc +++ b/sdk/src/trace/tracer_provider.cc @@ -23,6 +23,9 @@ opentelemetry::nostd::shared_ptr TracerProvider::G nostd::string_view library_name, nostd::string_view library_version) noexcept { + tracer_->SetInstrumentationLibrary( + opentelemetry::instrumentationlibrary::InstrumentationLibrary::create(library_name, + library_version)); return opentelemetry::nostd::shared_ptr(tracer_); } From 4ff42e6e6070a8a0f1d78c70f404a9a464b93ddd Mon Sep 17 00:00:00 2001 From: Tom Tan Date: Thu, 22 Apr 2021 16:18:40 -0700 Subject: [PATCH 02/11] More test case coverage for GetTracer. --- .../instrumentation_library.h | 55 +++++++++++++++++++ api/include/opentelemetry/trace/tracer.h | 2 +- sdk/test/trace/tracer_provider_test.cc | 13 ++++- 3 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 api/include/opentelemetry/instrumentationlibrary/instrumentation_library.h diff --git a/api/include/opentelemetry/instrumentationlibrary/instrumentation_library.h b/api/include/opentelemetry/instrumentationlibrary/instrumentation_library.h new file mode 100644 index 0000000000..d099fbc159 --- /dev/null +++ b/api/include/opentelemetry/instrumentationlibrary/instrumentation_library.h @@ -0,0 +1,55 @@ +// Copyright 2021, OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#pragma once + +#include "opentelemetry/nostd/unique_ptr.h" +#include "opentelemetry/nostd/string_view.h" +#include "opentelemetry/version.h" + +OPENTELEMETRY_BEGIN_NAMESPACE + +namespace instrumentationlibrary +{ + +class InstrumentationLibrary { +public: + InstrumentationLibrary(const InstrumentationLibrary &) = default; + + /** + * Returns a newly created InstrumentationLibrary with the specified library name and version. + * @param name name of the instrumentation library. + * @param version version of the instrumentation library. + * @returns the newly created InstrumentationLibrary. + */ + static nostd::unique_ptr create(nostd::string_view name, nostd::string_view version) + { + return nostd::unique_ptr(new InstrumentationLibrary{std::string{name}, std::string{version}}); + } + + const std::string &GetName() const { return name_; } + const std::string &GetVersion() const { return version_; } + +private: + InstrumentationLibrary(nostd::string_view name, nostd::string_view version): + name_(name), version_(version) {} + +private: + std::string name_; + std::string version_; +}; + +} // namespace instrumentationlibrary + +OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/trace/tracer.h b/api/include/opentelemetry/trace/tracer.h index cacd6ce8fe..b3cd6843bc 100644 --- a/api/include/opentelemetry/trace/tracer.h +++ b/api/include/opentelemetry/trace/tracer.h @@ -170,7 +170,7 @@ class Tracer * Get the instrumentation library associated with the current tracer. * @return the instrumentation library for current tracer. */ - InstrumentationLibrary *GetInstruemntationLibrary() const noexcept + InstrumentationLibrary *GetInstrumentationLibrary() const noexcept { return instrumentation_library_.get(); } diff --git a/sdk/test/trace/tracer_provider_test.cc b/sdk/test/trace/tracer_provider_test.cc index a1308827d5..cd15c44197 100644 --- a/sdk/test/trace/tracer_provider_test.cc +++ b/sdk/test/trace/tracer_provider_test.cc @@ -37,6 +37,17 @@ TEST(TracerProvider, GetTracer) std::unique_ptr(new RandomIdGenerator))); auto sdkTracer2 = dynamic_cast(tp2.GetTracer("test").get()); ASSERT_EQ("AlwaysOffSampler", sdkTracer2->GetSampler().GetDescription()); + + // TODO: return different tracer for different name. + // auto instrumentation_library1 = t1->GetInstrumentationLibrary(); + // ASSERT_NE(instrumentation_library1, nullptr); + // ASSERT_EQ(instrumentation_library1->GetName(), "test"); + // ASSERT_EQ(instrumentation_library1->GetVersion(), ""); + + auto instrumentation_library3 = t3->GetInstrumentationLibrary(); + ASSERT_NE(instrumentation_library3, nullptr); + ASSERT_EQ(instrumentation_library3->GetName(), "different"); + ASSERT_EQ(instrumentation_library3->GetVersion(), "1.0.0"); } TEST(TracerProvider, Shutdown) @@ -55,4 +66,4 @@ TEST(TracerProvider, ForceFlush) TracerProvider tp1(std::move(processor1)); EXPECT_TRUE(tp1.ForceFlush()); -} \ No newline at end of file +} From 6313490cc2e090c5bd6b865706db3e8ab822397e Mon Sep 17 00:00:00 2001 From: Tom Tan Date: Thu, 22 Apr 2021 16:43:52 -0700 Subject: [PATCH 03/11] Format --- .../instrumentation_library.h | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/api/include/opentelemetry/instrumentationlibrary/instrumentation_library.h b/api/include/opentelemetry/instrumentationlibrary/instrumentation_library.h index d099fbc159..93071e22a7 100644 --- a/api/include/opentelemetry/instrumentationlibrary/instrumentation_library.h +++ b/api/include/opentelemetry/instrumentationlibrary/instrumentation_library.h @@ -14,8 +14,8 @@ #pragma once -#include "opentelemetry/nostd/unique_ptr.h" #include "opentelemetry/nostd/string_view.h" +#include "opentelemetry/nostd/unique_ptr.h" #include "opentelemetry/version.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -23,7 +23,8 @@ OPENTELEMETRY_BEGIN_NAMESPACE namespace instrumentationlibrary { -class InstrumentationLibrary { +class InstrumentationLibrary +{ public: InstrumentationLibrary(const InstrumentationLibrary &) = default; @@ -33,17 +34,20 @@ class InstrumentationLibrary { * @param version version of the instrumentation library. * @returns the newly created InstrumentationLibrary. */ - static nostd::unique_ptr create(nostd::string_view name, nostd::string_view version) + static nostd::unique_ptr create(nostd::string_view name, + nostd::string_view version) { - return nostd::unique_ptr(new InstrumentationLibrary{std::string{name}, std::string{version}}); + return nostd::unique_ptr( + new InstrumentationLibrary{std::string{name}, std::string{version}}); } const std::string &GetName() const { return name_; } const std::string &GetVersion() const { return version_; } private: - InstrumentationLibrary(nostd::string_view name, nostd::string_view version): - name_(name), version_(version) {} + InstrumentationLibrary(nostd::string_view name, nostd::string_view version) + : name_(name), version_(version) + {} private: std::string name_; From a4c60a7afa76369836237fc749f882ed53f54187 Mon Sep 17 00:00:00 2001 From: Tom Tan Date: Fri, 23 Apr 2021 13:35:29 -0700 Subject: [PATCH 04/11] Move instrumentation library from API to SDK --- api/include/opentelemetry/trace/tracer.h | 25 ------------------- api/test/CMakeLists.txt | 1 - .../instrumentation_library.h | 15 ++++++++++- sdk/include/opentelemetry/sdk/trace/tracer.h | 15 ++++++++++- .../opentelemetry/sdk/trace/tracer_provider.h | 3 ++- sdk/src/trace/tracer.cc | 5 +++- sdk/src/trace/tracer_provider.cc | 23 +++++++++++++---- sdk/test/CMakeLists.txt | 1 + .../test/instrumentationlibrary/BUILD | 0 .../instrumentationlibrary/CMakeLists.txt | 2 +- .../instrumentationlibrary_test.cc | 4 +-- sdk/test/trace/tracer_provider_test.cc | 20 +++++++-------- sdk/test/trace/tracer_test.cc | 4 +-- 13 files changed, 67 insertions(+), 51 deletions(-) rename {api/include/opentelemetry => sdk/include/opentelemetry/sdk}/instrumentationlibrary/instrumentation_library.h (82%) rename {api => sdk}/test/instrumentationlibrary/BUILD (100%) rename {api => sdk}/test/instrumentationlibrary/CMakeLists.txt (89%) rename {api => sdk}/test/instrumentationlibrary/instrumentationlibrary_test.cc (89%) diff --git a/api/include/opentelemetry/trace/tracer.h b/api/include/opentelemetry/trace/tracer.h index b3cd6843bc..ab9c6f1675 100644 --- a/api/include/opentelemetry/trace/tracer.h +++ b/api/include/opentelemetry/trace/tracer.h @@ -1,6 +1,5 @@ #pragma once -#include "opentelemetry/instrumentationlibrary/instrumentation_library.h" #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/nostd/string_view.h" #include "opentelemetry/nostd/unique_ptr.h" @@ -16,8 +15,6 @@ OPENTELEMETRY_BEGIN_NAMESPACE namespace trace { -using InstrumentationLibrary = opentelemetry::instrumentationlibrary::InstrumentationLibrary; - /** * Handles span creation and in-process context propagation. * @@ -166,25 +163,6 @@ class Tracer } } - /** - * Get the instrumentation library associated with the current tracer. - * @return the instrumentation library for current tracer. - */ - InstrumentationLibrary *GetInstrumentationLibrary() const noexcept - { - return instrumentation_library_.get(); - } - - /** - * Set instrumentation library. - * @param instrumentation library to set on the current tracer. - */ - void SetInstrumentationLibrary( - nostd::unique_ptr &&instrumentation_library) noexcept - { - instrumentation_library_ = std::move(instrumentation_library); - } - /** * Force any buffered spans to flush. * @param timeout to complete the flush @@ -210,9 +188,6 @@ class Tracer } virtual void CloseWithMicroseconds(uint64_t timeout) noexcept = 0; - -private: - std::unique_ptr instrumentation_library_; }; } // namespace trace OPENTELEMETRY_END_NAMESPACE diff --git a/api/test/CMakeLists.txt b/api/test/CMakeLists.txt index 24f128694b..eaf09e44e0 100644 --- a/api/test/CMakeLists.txt +++ b/api/test/CMakeLists.txt @@ -7,4 +7,3 @@ add_subdirectory(metrics) add_subdirectory(logs) add_subdirectory(common) add_subdirectory(baggage) -add_subdirectory(instrumentationlibrary) diff --git a/api/include/opentelemetry/instrumentationlibrary/instrumentation_library.h b/sdk/include/opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h similarity index 82% rename from api/include/opentelemetry/instrumentationlibrary/instrumentation_library.h rename to sdk/include/opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h index 93071e22a7..611e81acb3 100644 --- a/api/include/opentelemetry/instrumentationlibrary/instrumentation_library.h +++ b/sdk/include/opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h @@ -20,6 +20,8 @@ OPENTELEMETRY_BEGIN_NAMESPACE +namespace sdk +{ namespace instrumentationlibrary { @@ -35,12 +37,22 @@ class InstrumentationLibrary * @returns the newly created InstrumentationLibrary. */ static nostd::unique_ptr create(nostd::string_view name, - nostd::string_view version) + nostd::string_view version = "") { return nostd::unique_ptr( new InstrumentationLibrary{std::string{name}, std::string{version}}); } + /** + * Compare 2 instrumentation libraries. + * @param other the instrumentation library to compare to. + * @returns true if the 2 instrumentation libraries are equal, false otherwise. + */ + bool operator==(const InstrumentationLibrary& other) const + { + return this->name_ == other.name_ && this->version_ == other.version_; + } + const std::string &GetName() const { return name_; } const std::string &GetVersion() const { return version_; } @@ -55,5 +67,6 @@ class InstrumentationLibrary }; } // namespace instrumentationlibrary +} // namespace sdk OPENTELEMETRY_END_NAMESPACE diff --git a/sdk/include/opentelemetry/sdk/trace/tracer.h b/sdk/include/opentelemetry/sdk/trace/tracer.h index f2054e1777..1e025ec89b 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer.h @@ -1,6 +1,7 @@ #pragma once #include "opentelemetry/sdk/common/atomic_shared_ptr.h" +#include "opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h" #include "opentelemetry/sdk/resource/resource.h" #include "opentelemetry/sdk/trace/processor.h" #include "opentelemetry/sdk/trace/samplers/always_on.h" @@ -16,11 +17,16 @@ namespace sdk { namespace trace { + +using namespace opentelemetry::sdk::instrumentationlibrary; + class Tracer final : public trace_api::Tracer, public std::enable_shared_from_this { public: /** Construct a new Tracer with the given context pipeline. */ - explicit Tracer(std::shared_ptr context) noexcept; + explicit Tracer(std::shared_ptr context, + std::unique_ptr instrumentation_library = + InstrumentationLibrary::create("")) noexcept; nostd::shared_ptr StartSpan( nostd::string_view name, @@ -38,11 +44,18 @@ class Tracer final : public trace_api::Tracer, public std::enable_shared_from_th /** Returns the configured Id generator */ IdGenerator &GetIdGenerator() const noexcept { return context_->GetIdGenerator(); } + /** Returns the associated instruementation library */ + const InstrumentationLibrary &GetInstrumentationLibrary() const noexcept + { + return *instrumentation_library_; + } + // Note: Test only Sampler &GetSampler() { return context_->GetSampler(); } private: std::shared_ptr context_; + std::shared_ptr instrumentation_library_; }; } // namespace trace } // namespace sdk diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h index a82e29ccef..6d12766ae3 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h @@ -3,6 +3,7 @@ #include #include #include +#include #include "opentelemetry/nostd/shared_ptr.h" #include "opentelemetry/sdk/resource/resource.h" @@ -73,7 +74,7 @@ class TracerProvider final : public opentelemetry::trace::TracerProvider private: std::shared_ptr context_; - std::shared_ptr tracer_; + std::vector> tracers_; }; } // namespace trace } // namespace sdk diff --git a/sdk/src/trace/tracer.cc b/sdk/src/trace/tracer.cc index 2d67321d83..e111357ec5 100644 --- a/sdk/src/trace/tracer.cc +++ b/sdk/src/trace/tracer.cc @@ -11,7 +11,10 @@ namespace sdk namespace trace { -Tracer::Tracer(std::shared_ptr context) noexcept : context_{context} {} +Tracer::Tracer(std::shared_ptr context, + std::unique_ptr instrumentation_library) noexcept + : context_{context}, instrumentation_library_{std::move(instrumentation_library)} +{} trace_api::SpanContext GetCurrentSpanContext(const trace_api::SpanContext &explicit_parent) { diff --git a/sdk/src/trace/tracer_provider.cc b/sdk/src/trace/tracer_provider.cc index ee1ae54f47..149df76193 100644 --- a/sdk/src/trace/tracer_provider.cc +++ b/sdk/src/trace/tracer_provider.cc @@ -6,7 +6,7 @@ namespace sdk namespace trace { TracerProvider::TracerProvider(std::shared_ptr context) noexcept - : context_{context}, tracer_(new Tracer(context)) + : context_{context} {} TracerProvider::TracerProvider(std::unique_ptr processor, @@ -23,10 +23,23 @@ opentelemetry::nostd::shared_ptr TracerProvider::G nostd::string_view library_name, nostd::string_view library_version) noexcept { - tracer_->SetInstrumentationLibrary( - opentelemetry::instrumentationlibrary::InstrumentationLibrary::create(library_name, - library_version)); - return opentelemetry::nostd::shared_ptr(tracer_); + // if (library_name == "") { + // // TODO: log invalid name. + // } + + auto lib = InstrumentationLibrary::create(library_name, library_version); + for (auto &tracer : tracers_) + { + auto &tracer_lib = tracer->GetInstrumentationLibrary(); + if (tracer_lib == *lib) + { + return opentelemetry::nostd::shared_ptr(tracer); + } + } + auto tracer = new sdk::trace::Tracer(context_, std::move(lib)); + tracers_.push_back(std::shared_ptr(std::move(tracer))); + + return tracers_.back(); } void TracerProvider::RegisterPipeline(std::unique_ptr processor) noexcept diff --git a/sdk/test/CMakeLists.txt b/sdk/test/CMakeLists.txt index eae1209f46..77047165d9 100644 --- a/sdk/test/CMakeLists.txt +++ b/sdk/test/CMakeLists.txt @@ -3,3 +3,4 @@ add_subdirectory(trace) add_subdirectory(metrics) add_subdirectory(logs) add_subdirectory(resource) +add_subdirectory(instrumentationlibrary) diff --git a/api/test/instrumentationlibrary/BUILD b/sdk/test/instrumentationlibrary/BUILD similarity index 100% rename from api/test/instrumentationlibrary/BUILD rename to sdk/test/instrumentationlibrary/BUILD diff --git a/api/test/instrumentationlibrary/CMakeLists.txt b/sdk/test/instrumentationlibrary/CMakeLists.txt similarity index 89% rename from api/test/instrumentationlibrary/CMakeLists.txt rename to sdk/test/instrumentationlibrary/CMakeLists.txt index d65599317d..50c2728a8a 100644 --- a/api/test/instrumentationlibrary/CMakeLists.txt +++ b/sdk/test/instrumentationlibrary/CMakeLists.txt @@ -7,6 +7,6 @@ foreach(testname instrumentationlibrary_test) ${CMAKE_THREAD_LIBS_INIT} opentelemetry_api) gtest_add_tests( TARGET ${testname} - TEST_PREFIX baggage. + TEST_PREFIX instrumentationlibrary. TEST_LIST ${testname}) endforeach() diff --git a/api/test/instrumentationlibrary/instrumentationlibrary_test.cc b/sdk/test/instrumentationlibrary/instrumentationlibrary_test.cc similarity index 89% rename from api/test/instrumentationlibrary/instrumentationlibrary_test.cc rename to sdk/test/instrumentationlibrary/instrumentationlibrary_test.cc index 08e4acc20e..75244e9804 100644 --- a/api/test/instrumentationlibrary/instrumentationlibrary_test.cc +++ b/sdk/test/instrumentationlibrary/instrumentationlibrary_test.cc @@ -18,10 +18,10 @@ #include #include -#include "opentelemetry/instrumentationlibrary/instrumentation_library.h" +#include "opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h" using namespace opentelemetry; -using namespace opentelemetry::instrumentationlibrary; +using namespace opentelemetry::sdk::instrumentationlibrary; TEST(InstrumentationLibrary, CreateInstrumentationLibrary) { diff --git a/sdk/test/trace/tracer_provider_test.cc b/sdk/test/trace/tracer_provider_test.cc index cd15c44197..cf241bc52c 100644 --- a/sdk/test/trace/tracer_provider_test.cc +++ b/sdk/test/trace/tracer_provider_test.cc @@ -24,8 +24,7 @@ TEST(TracerProvider, GetTracer) // Should return the same instance each time. ASSERT_EQ(t1, t2); - // TODO: t3 should be a different instance. - ASSERT_EQ(t1, t3); + ASSERT_NE(t1, t3); // Should be an sdk::trace::Tracer with the processor attached. auto sdkTracer1 = dynamic_cast(t1.get()); @@ -38,16 +37,15 @@ TEST(TracerProvider, GetTracer) auto sdkTracer2 = dynamic_cast(tp2.GetTracer("test").get()); ASSERT_EQ("AlwaysOffSampler", sdkTracer2->GetSampler().GetDescription()); - // TODO: return different tracer for different name. - // auto instrumentation_library1 = t1->GetInstrumentationLibrary(); - // ASSERT_NE(instrumentation_library1, nullptr); - // ASSERT_EQ(instrumentation_library1->GetName(), "test"); - // ASSERT_EQ(instrumentation_library1->GetVersion(), ""); + auto instrumentation_library1 = sdkTracer1->GetInstrumentationLibrary(); + ASSERT_EQ(instrumentation_library1.GetName(), "test"); + ASSERT_EQ(instrumentation_library1.GetVersion(), ""); - auto instrumentation_library3 = t3->GetInstrumentationLibrary(); - ASSERT_NE(instrumentation_library3, nullptr); - ASSERT_EQ(instrumentation_library3->GetName(), "different"); - ASSERT_EQ(instrumentation_library3->GetVersion(), "1.0.0"); + // Should be an sdk::trace::Tracer with the processor attached. + auto sdkTracer3 = dynamic_cast(t3.get()); + auto instrumentation_library3 = sdkTracer3->GetInstrumentationLibrary(); + ASSERT_EQ(instrumentation_library3.GetName(), "different"); + ASSERT_EQ(instrumentation_library3.GetVersion(), "1.0.0"); } TEST(TracerProvider, Shutdown) diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index dbfa3146f3..b2718cc004 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -513,7 +513,7 @@ TEST(Tracer, TestParentBasedSampler) std::unique_ptr exporter2(new InMemorySpanExporter()); std::shared_ptr span_data_parent_off = exporter2->GetData(); auto tracer_parent_off = initTracer(std::move(exporter2), - new ParentBasedSampler(std::make_shared())); + new ParentBasedSampler(std::make_shared())); auto span_parent_off_1 = tracer_parent_off->StartSpan("span 1"); auto span_parent_off_2 = tracer_parent_off->StartSpan("span 2"); @@ -593,4 +593,4 @@ TEST(Tracer, ExpectParent) EXPECT_EQ(spandata_first->GetSpanId(), spandata_second->GetParentSpanId()); EXPECT_EQ(spandata_second->GetSpanId(), spandata_third->GetParentSpanId()); -} \ No newline at end of file +} From c76e3c6e29ee3fa76b441efde81359685ed17a0d Mon Sep 17 00:00:00 2001 From: Tom Tan Date: Fri, 23 Apr 2021 14:21:33 -0700 Subject: [PATCH 05/11] Remove implicit type cast --- sdk/include/opentelemetry/sdk/trace/tracer_provider.h | 2 +- sdk/src/trace/tracer_provider.cc | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h index 6d12766ae3..6b8fc4fc3d 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h @@ -74,7 +74,7 @@ class TracerProvider final : public opentelemetry::trace::TracerProvider private: std::shared_ptr context_; - std::vector> tracers_; + std::vector> tracers_; }; } // namespace trace } // namespace sdk diff --git a/sdk/src/trace/tracer_provider.cc b/sdk/src/trace/tracer_provider.cc index 149df76193..3cd22caf3b 100644 --- a/sdk/src/trace/tracer_provider.cc +++ b/sdk/src/trace/tracer_provider.cc @@ -30,14 +30,15 @@ opentelemetry::nostd::shared_ptr TracerProvider::G auto lib = InstrumentationLibrary::create(library_name, library_version); for (auto &tracer : tracers_) { - auto &tracer_lib = tracer->GetInstrumentationLibrary(); + auto &tracer_lib = + dynamic_cast(tracer.get())->GetInstrumentationLibrary(); if (tracer_lib == *lib) { - return opentelemetry::nostd::shared_ptr(tracer); + return tracer; } } auto tracer = new sdk::trace::Tracer(context_, std::move(lib)); - tracers_.push_back(std::shared_ptr(std::move(tracer))); + tracers_.push_back(nostd::shared_ptr(std::move(tracer))); return tracers_.back(); } From 2084cb534ff3db3633903d080d6eac0ae743c693 Mon Sep 17 00:00:00 2001 From: Tom Tan Date: Fri, 23 Apr 2021 14:43:16 -0700 Subject: [PATCH 06/11] Apply format --- .../sdk/instrumentationlibrary/instrumentation_library.h | 4 ++-- sdk/include/opentelemetry/sdk/trace/tracer_provider.h | 2 ++ sdk/src/trace/tracer_provider.cc | 5 +++-- .../instrumentationlibrary/instrumentationlibrary_test.cc | 3 +-- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h b/sdk/include/opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h index 611e81acb3..a98cd05c02 100644 --- a/sdk/include/opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h +++ b/sdk/include/opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h @@ -48,9 +48,9 @@ class InstrumentationLibrary * @param other the instrumentation library to compare to. * @returns true if the 2 instrumentation libraries are equal, false otherwise. */ - bool operator==(const InstrumentationLibrary& other) const + bool operator==(const InstrumentationLibrary &other) const { - return this->name_ == other.name_ && this->version_ == other.version_; + return this->name_ == other.name_ && this->version_ == other.version_; } const std::string &GetName() const { return name_; } diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h index 6b8fc4fc3d..d5116692f7 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -75,6 +76,7 @@ class TracerProvider final : public opentelemetry::trace::TracerProvider private: std::shared_ptr context_; std::vector> tracers_; + std::mutex lock_; }; } // namespace trace } // namespace sdk diff --git a/sdk/src/trace/tracer_provider.cc b/sdk/src/trace/tracer_provider.cc index 3cd22caf3b..d2fe7b8399 100644 --- a/sdk/src/trace/tracer_provider.cc +++ b/sdk/src/trace/tracer_provider.cc @@ -27,6 +27,8 @@ opentelemetry::nostd::shared_ptr TracerProvider::G // // TODO: log invalid name. // } + const std::lock_guard guard(lock_); + auto lib = InstrumentationLibrary::create(library_name, library_version); for (auto &tracer : tracers_) { @@ -37,8 +39,7 @@ opentelemetry::nostd::shared_ptr TracerProvider::G return tracer; } } - auto tracer = new sdk::trace::Tracer(context_, std::move(lib)); - tracers_.push_back(nostd::shared_ptr(std::move(tracer))); + tracers_.push_back(nostd::shared_ptr(new sdk::trace::Tracer(context_, std::move(lib)))); return tracers_.back(); } diff --git a/sdk/test/instrumentationlibrary/instrumentationlibrary_test.cc b/sdk/test/instrumentationlibrary/instrumentationlibrary_test.cc index 75244e9804..a27ac0718f 100644 --- a/sdk/test/instrumentationlibrary/instrumentationlibrary_test.cc +++ b/sdk/test/instrumentationlibrary/instrumentationlibrary_test.cc @@ -13,13 +13,12 @@ // limitations under the License. #include "opentelemetry/nostd/string_view.h" +#include "opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h" #include #include #include -#include "opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h" - using namespace opentelemetry; using namespace opentelemetry::sdk::instrumentationlibrary; From b2dff232ff0f0bb23ae23ff1f87f8f560b321c84 Mon Sep 17 00:00:00 2001 From: Tom Tan Date: Fri, 23 Apr 2021 16:34:09 -0700 Subject: [PATCH 07/11] Store SdkTracer in SdkTracerProvider --- sdk/include/opentelemetry/sdk/trace/tracer_provider.h | 2 +- sdk/src/trace/tracer_provider.cc | 11 +++++------ sdk/test/trace/tracer_test.cc | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h index d5116692f7..6c1f8595fa 100644 --- a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h +++ b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h @@ -75,7 +75,7 @@ class TracerProvider final : public opentelemetry::trace::TracerProvider private: std::shared_ptr context_; - std::vector> tracers_; + std::vector> tracers_; std::mutex lock_; }; } // namespace trace diff --git a/sdk/src/trace/tracer_provider.cc b/sdk/src/trace/tracer_provider.cc index d2fe7b8399..80be9462d2 100644 --- a/sdk/src/trace/tracer_provider.cc +++ b/sdk/src/trace/tracer_provider.cc @@ -19,7 +19,7 @@ TracerProvider::TracerProvider(std::unique_ptr processor, std::move(id_generator))) {} -opentelemetry::nostd::shared_ptr TracerProvider::GetTracer( +nostd::shared_ptr TracerProvider::GetTracer( nostd::string_view library_name, nostd::string_view library_version) noexcept { @@ -32,16 +32,15 @@ opentelemetry::nostd::shared_ptr TracerProvider::G auto lib = InstrumentationLibrary::create(library_name, library_version); for (auto &tracer : tracers_) { - auto &tracer_lib = - dynamic_cast(tracer.get())->GetInstrumentationLibrary(); + auto &tracer_lib = tracer->GetInstrumentationLibrary(); if (tracer_lib == *lib) { return tracer; } } - tracers_.push_back(nostd::shared_ptr(new sdk::trace::Tracer(context_, std::move(lib)))); - - return tracers_.back(); + tracers_.push_back(std::shared_ptr( + new sdk::trace::Tracer(context_, std::move(lib)))); + return nostd::shared_ptr{tracers_.back()}; } void TracerProvider::RegisterPipeline(std::unique_ptr processor) noexcept diff --git a/sdk/test/trace/tracer_test.cc b/sdk/test/trace/tracer_test.cc index b2718cc004..0b3720b385 100644 --- a/sdk/test/trace/tracer_test.cc +++ b/sdk/test/trace/tracer_test.cc @@ -513,7 +513,7 @@ TEST(Tracer, TestParentBasedSampler) std::unique_ptr exporter2(new InMemorySpanExporter()); std::shared_ptr span_data_parent_off = exporter2->GetData(); auto tracer_parent_off = initTracer(std::move(exporter2), - new ParentBasedSampler(std::make_shared())); + new ParentBasedSampler(std::make_shared())); auto span_parent_off_1 = tracer_parent_off->StartSpan("span 1"); auto span_parent_off_2 = tracer_parent_off->StartSpan("span 2"); From 39eda7e5fd0411d80ad7fe4d8c18b45dbc85ca08 Mon Sep 17 00:00:00 2001 From: Tom Tan Date: Fri, 23 Apr 2021 16:39:12 -0700 Subject: [PATCH 08/11] Fix implicit type cast --- sdk/src/trace/tracer_provider.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/trace/tracer_provider.cc b/sdk/src/trace/tracer_provider.cc index 80be9462d2..ef520be074 100644 --- a/sdk/src/trace/tracer_provider.cc +++ b/sdk/src/trace/tracer_provider.cc @@ -35,7 +35,7 @@ nostd::shared_ptr TracerProvider::GetTracer( auto &tracer_lib = tracer->GetInstrumentationLibrary(); if (tracer_lib == *lib) { - return tracer; + return nostd::shared_ptr{tracer}; } } tracers_.push_back(std::shared_ptr( From 5e0f61310cc630c26aaf071252eaeb73f00737ac Mon Sep 17 00:00:00 2001 From: Tom Tan Date: Fri, 23 Apr 2021 16:52:32 -0700 Subject: [PATCH 09/11] Fix missing bazel dependence --- sdk/test/instrumentationlibrary/BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/test/instrumentationlibrary/BUILD b/sdk/test/instrumentationlibrary/BUILD index 98bb5d1697..68373e42c5 100644 --- a/sdk/test/instrumentationlibrary/BUILD +++ b/sdk/test/instrumentationlibrary/BUILD @@ -7,6 +7,7 @@ cc_test( ], deps = [ "//api", + "//sdk:headers", "@com_google_googletest//:gtest_main", ], ) From c792e3e636c9931557a269fbd07d9a4c7d915b41 Mon Sep 17 00:00:00 2001 From: Tom Tan Date: Fri, 23 Apr 2021 17:17:21 -0700 Subject: [PATCH 10/11] Update CHANGELOG file --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8efe92df8b..5b1193ac47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ Increment the: ## [Unreleased] +* [SDK] Add instrumentation library and multiple tracer support ([#693](https://github.com/open-telemetry/opentelemetry-cpp/pull/693)) + ## [0.4.0] 2021-04-12 * [EXPORTER] ETW Exporter enhancements ([#519](https://github.com/open-telemetry/opentelemetry-cpp/pull/519)) From c2fb193a736898d0343b1f129715d652bf791e6a Mon Sep 17 00:00:00 2001 From: Tom Tan Date: Sun, 25 Apr 2021 21:18:39 -0700 Subject: [PATCH 11/11] Delay heap allocation for existing tracer in GetTracer --- .../instrumentation_library.h | 15 ++++++++++++++- sdk/src/trace/tracer_provider.cc | 7 ++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/sdk/include/opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h b/sdk/include/opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h index a98cd05c02..ce9a89e9eb 100644 --- a/sdk/include/opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h +++ b/sdk/include/opentelemetry/sdk/instrumentationlibrary/instrumentation_library.h @@ -50,7 +50,20 @@ class InstrumentationLibrary */ bool operator==(const InstrumentationLibrary &other) const { - return this->name_ == other.name_ && this->version_ == other.version_; + return equal(other.name_, other.version_); + } + + /** + * Check whether the instrumentation library has given name and version. + * This could be used to check version equality and avoid heap allocation. + * @param name name of the instrumentation library to compare. + * @param version version of the instrumentatoin library to compare. + * @returns true if name and version in this instrumentation library are equal with the given name + * and version. + */ + bool equal(const nostd::string_view name, const nostd::string_view version) const + { + return this->name_ == name && this->version_ == version; } const std::string &GetName() const { return name_; } diff --git a/sdk/src/trace/tracer_provider.cc b/sdk/src/trace/tracer_provider.cc index ef520be074..47bbc54ecb 100644 --- a/sdk/src/trace/tracer_provider.cc +++ b/sdk/src/trace/tracer_provider.cc @@ -24,20 +24,21 @@ nostd::shared_ptr TracerProvider::GetTracer( nostd::string_view library_version) noexcept { // if (library_name == "") { - // // TODO: log invalid name. + // // TODO: log invalid library_name. // } const std::lock_guard guard(lock_); - auto lib = InstrumentationLibrary::create(library_name, library_version); for (auto &tracer : tracers_) { auto &tracer_lib = tracer->GetInstrumentationLibrary(); - if (tracer_lib == *lib) + if (tracer_lib.equal(library_name, library_version)) { return nostd::shared_ptr{tracer}; } } + + auto lib = InstrumentationLibrary::create(library_name, library_version); tracers_.push_back(std::shared_ptr( new sdk::trace::Tracer(context_, std::move(lib)))); return nostd::shared_ptr{tracers_.back()};