diff --git a/CHANGELOG.md b/CHANGELOG.md index 4849ece1ef..5c5d9d9b04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,12 +21,23 @@ Increment the: * [API] Remove include_trace_context [#2194](https://github.com/open-telemetry/opentelemetry-cpp/pull/2194) +* [API] Remove Meters + [#2205](https://github.com/open-telemetry/opentelemetry-cpp/pull/2205) + * [SDK] MeterProvider should own MeterContext, not share it [#2218](https://github.com/open-telemetry/opentelemetry-cpp/pull/2218) * [SDK] TracerProvider should own TracerContext, not share it [#2221](https://github.com/open-telemetry/opentelemetry-cpp/pull/2221) +Important changes: + +* [API] Remove Meters + [#2205](https://github.com/open-telemetry/opentelemetry-cpp/pull/2205) + * The CMake option `WITH_REMOVE_METER_PREVIEW` was added. + * This option is experimental, and may change in the future. + * Enabling it is an ABI breaking change. + Breaking changes: * [REMOVAL] Remove the jaeger exporter diff --git a/CMakeLists.txt b/CMakeLists.txt index fb483f5a58..997c9440e8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -197,6 +197,14 @@ option(WITH_OPENTRACING "Whether to include the Opentracing shim" OFF) option(OTELCPP_VERSIONED_LIBS "Whether to generate the versioned shared libs" OFF) +# +# This option is experimental, subject to change in the spec: +# +# * https://github.com/open-telemetry/opentelemetry-specification/issues/2232 +# +option(WITH_REMOVE_METER_PREVIEW + "EXPERIMENTAL, ABI BREAKING: Allow to remove a meter" OFF) + if(OTELCPP_VERSIONED_LIBS AND NOT BUILD_SHARED_LIBS) message(FATAL_ERROR "OTELCPP_VERSIONED_LIBS=ON requires BUILD_SHARED_LIBS=ON") endif() diff --git a/api/CMakeLists.txt b/api/CMakeLists.txt index 91d129e179..d1a6a77fa3 100644 --- a/api/CMakeLists.txt +++ b/api/CMakeLists.txt @@ -98,6 +98,11 @@ if(WITH_ASYNC_EXPORT_PREVIEW) target_compile_definitions(opentelemetry_api INTERFACE ENABLE_ASYNC_EXPORT) endif() +if(WITH_REMOVE_METER_PREVIEW) + target_compile_definitions(opentelemetry_api + INTERFACE ENABLE_REMOVE_METER_PREVIEW) +endif() + # A better place should be in sdk, not api if(WITH_OTLP_HTTP_SSL_PREVIEW) target_compile_definitions(opentelemetry_api diff --git a/api/include/opentelemetry/metrics/meter_provider.h b/api/include/opentelemetry/metrics/meter_provider.h index 654c4022ea..e0b0285ef4 100644 --- a/api/include/opentelemetry/metrics/meter_provider.h +++ b/api/include/opentelemetry/metrics/meter_provider.h @@ -29,6 +29,11 @@ class MeterProvider virtual nostd::shared_ptr GetMeter(nostd::string_view library_name, nostd::string_view library_version = "", nostd::string_view schema_url = "") noexcept = 0; +#ifdef ENABLE_REMOVE_METER_PREVIEW + virtual void RemoveMeter(nostd::string_view library_name, + nostd::string_view library_version = "", + nostd::string_view schema_url = "") noexcept = 0; +#endif }; } // namespace metrics OPENTELEMETRY_END_NAMESPACE diff --git a/api/include/opentelemetry/metrics/noop.h b/api/include/opentelemetry/metrics/noop.h index 3a754a6e00..c5802f3dd3 100644 --- a/api/include/opentelemetry/metrics/noop.h +++ b/api/include/opentelemetry/metrics/noop.h @@ -203,6 +203,13 @@ class NoopMeterProvider final : public MeterProvider return meter_; } +#ifdef ENABLE_REMOVE_METER_PREVIEW + void RemoveMeter(nostd::string_view /* name */, + nostd::string_view /* version */, + nostd::string_view /* schema_url */) noexcept override + {} +#endif + private: nostd::shared_ptr meter_; }; diff --git a/ci/do_ci.sh b/ci/do_ci.sh index 42bf3df416..69a6eef0c9 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -106,6 +106,7 @@ elif [[ "$1" == "cmake.maintainer.sync.test" ]]; then -DWITH_OTLP_HTTP=ON \ -DWITH_OTLP_HTTP_SSL_PREVIEW=ON \ -DWITH_OTLP_HTTP_SSL_TLS_PREVIEW=ON \ + -DWITH_REMOVE_METER_PREVIEW=ON \ -DWITH_PROMETHEUS=ON \ -DWITH_EXAMPLES=ON \ -DWITH_EXAMPLES_HTTP=ON \ @@ -129,6 +130,7 @@ elif [[ "$1" == "cmake.maintainer.async.test" ]]; then -DWITH_OTLP_HTTP=ON \ -DWITH_OTLP_HTTP_SSL_PREVIEW=ON \ -DWITH_OTLP_HTTP_SSL_TLS_PREVIEW=ON \ + -DWITH_REMOVE_METER_PREVIEW=ON \ -DWITH_PROMETHEUS=ON \ -DWITH_EXAMPLES=ON \ -DWITH_EXAMPLES_HTTP=ON \ @@ -153,6 +155,7 @@ elif [[ "$1" == "cmake.maintainer.cpp11.async.test" ]]; then -DWITH_OTLP_HTTP=ON \ -DWITH_OTLP_HTTP_SSL_PREVIEW=ON \ -DWITH_OTLP_HTTP_SSL_TLS_PREVIEW=ON \ + -DWITH_REMOVE_METER_PREVIEW=ON \ -DWITH_PROMETHEUS=ON \ -DWITH_EXAMPLES=ON \ -DWITH_EXAMPLES_HTTP=ON \ diff --git a/exporters/otlp/test/otlp_grpc_log_record_exporter_test.cc b/exporters/otlp/test/otlp_grpc_log_record_exporter_test.cc index f2ed416327..518caaba5e 100644 --- a/exporters/otlp/test/otlp_grpc_log_record_exporter_test.cc +++ b/exporters/otlp/test/otlp_grpc_log_record_exporter_test.cc @@ -154,7 +154,7 @@ TEST_F(OtlpGrpcLogRecordExporterTestPeer, ExportIntegrationTest) std::unique_ptr trace_stub_interface( trace_mock_stub); - auto trace_provider = opentelemetry::nostd::shared_ptr( + auto trace_provider = opentelemetry::nostd::shared_ptr( opentelemetry::sdk::trace::TracerProviderFactory::Create( opentelemetry::sdk::trace::SimpleSpanProcessorFactory::Create( GetExporter(trace_stub_interface)))); @@ -174,7 +174,7 @@ TEST_F(OtlpGrpcLogRecordExporterTestPeer, ExportIntegrationTest) auto logger = provider->GetLogger("test", "opentelelemtry_library", "", schema_url, {{"scope_key1", "scope_value"}, {"scope_key2", 2}}); - std::unordered_map attributes; + std::unordered_map attributes; attributes["service.name"] = "unit_test_service"; attributes["tenant.id"] = "test_user"; attributes["bool_value"] = true; @@ -197,7 +197,7 @@ TEST_F(OtlpGrpcLogRecordExporterTestPeer, ExportIntegrationTest) opentelemetry::trace::Provider::SetTracerProvider( opentelemetry::nostd::shared_ptr( new opentelemetry::trace::NoopTracerProvider())); - trace_provider = opentelemetry::nostd::shared_ptr(); + trace_provider = opentelemetry::nostd::shared_ptr(); } } // namespace otlp diff --git a/sdk/include/opentelemetry/sdk/metrics/meter_context.h b/sdk/include/opentelemetry/sdk/metrics/meter_context.h index 8155b4da9f..336f5da7a0 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter_context.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter_context.h @@ -115,6 +115,10 @@ class MeterContext : public std::enable_shared_from_this */ void AddMeter(std::shared_ptr meter); + void RemoveMeter(nostd::string_view name, + nostd::string_view version, + nostd::string_view schema_url); + /** * Force all active Collectors to flush any buffered meter data * within the given timeout. diff --git a/sdk/include/opentelemetry/sdk/metrics/meter_provider.h b/sdk/include/opentelemetry/sdk/metrics/meter_provider.h index b27fcfb969..c74c37f8a4 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter_provider.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter_provider.h @@ -53,6 +53,12 @@ class MeterProvider final : public opentelemetry::metrics::MeterProvider nostd::string_view version = "", nostd::string_view schema_url = "") noexcept override; +#ifdef ENABLE_REMOVE_METER_PREVIEW + void RemoveMeter(nostd::string_view name, + nostd::string_view version, + nostd::string_view schema_url) noexcept override; +#endif + /** * Obtain the resource associated with this meter provider. * @return The resource for this meter provider. diff --git a/sdk/src/metrics/meter_context.cc b/sdk/src/metrics/meter_context.cc index eaf82232d6..7a299bfc63 100644 --- a/sdk/src/metrics/meter_context.cc +++ b/sdk/src/metrics/meter_context.cc @@ -3,6 +3,7 @@ #include "opentelemetry/sdk/metrics/meter_context.h" #include "opentelemetry/sdk/common/global_log_handler.h" +#include "opentelemetry/sdk/metrics/meter.h" #include "opentelemetry/sdk/metrics/metric_reader.h" #include "opentelemetry/sdk/metrics/state/metric_collector.h" #include "opentelemetry/sdk_config.h" @@ -79,6 +80,32 @@ void MeterContext::AddMeter(std::shared_ptr meter) meters_.push_back(meter); } +void MeterContext::RemoveMeter(nostd::string_view name, + nostd::string_view version, + nostd::string_view schema_url) +{ + std::lock_guard guard(meter_lock_); + + std::vector> filtered_meters; + + for (auto &meter : meters_) + { + auto scope = meter->GetInstrumentationScope(); + if (scope->equal(name, version, schema_url)) + { + OTEL_INTERNAL_LOG_INFO("[MeterContext::RemoveMeter] removing meter name <" + << name << ">, version <" << version << ">, URL <" << schema_url + << ">"); + } + else + { + filtered_meters.push_back(meter); + } + } + + meters_.swap(filtered_meters); +} + bool MeterContext::Shutdown() noexcept { bool result = true; diff --git a/sdk/src/metrics/meter_provider.cc b/sdk/src/metrics/meter_provider.cc index a36260e201..84ab58c4cf 100644 --- a/sdk/src/metrics/meter_provider.cc +++ b/sdk/src/metrics/meter_provider.cc @@ -59,6 +59,23 @@ nostd::shared_ptr MeterProvider::GetMeter( return nostd::shared_ptr{meter}; } +#ifdef ENABLE_REMOVE_METER_PREVIEW +void MeterProvider::RemoveMeter(nostd::string_view name, + nostd::string_view version, + nostd::string_view schema_url) noexcept +{ + if (name.data() == nullptr || name == "") + { + OTEL_INTERNAL_LOG_WARN("[MeterProvider::RemoveMeter] Library name is empty."); + name = ""; + } + + const std::lock_guard guard(lock_); + + context_->RemoveMeter(name, version, schema_url); +} +#endif + const resource::Resource &MeterProvider::GetResource() const noexcept { return context_->GetResource(); diff --git a/sdk/test/logs/log_record_test.cc b/sdk/test/logs/log_record_test.cc index 0e5de03050..4f7deb06d0 100644 --- a/sdk/test/logs/log_record_test.cc +++ b/sdk/test/logs/log_record_test.cc @@ -99,13 +99,13 @@ class TestBodyLogger : public opentelemetry::logs::Logger } } - const opentelemetry::v1::common::AttributeValue &GetLastLogRecord() const noexcept + const opentelemetry::common::AttributeValue &GetLastLogRecord() const noexcept { return last_body_; } private: - opentelemetry::v1::common::AttributeValue last_body_; + opentelemetry::common::AttributeValue last_body_; }; // Define a basic LoggerProvider class that returns an instance of the logger class defined above diff --git a/sdk/test/metrics/meter_provider_sdk_test.cc b/sdk/test/metrics/meter_provider_sdk_test.cc index bcdc6c4b79..da75916555 100644 --- a/sdk/test/metrics/meter_provider_sdk_test.cc +++ b/sdk/test/metrics/meter_provider_sdk_test.cc @@ -16,7 +16,6 @@ using namespace opentelemetry::sdk::metrics; TEST(MeterProvider, GetMeter) { - MeterProvider mp1; // std::unique_ptr view{std::unique_ptr()}; // MeterProvider mp1(std::move(exporters), std::move(readers), std::move(views); @@ -59,3 +58,39 @@ TEST(MeterProvider, GetMeter) mp1.ForceFlush(); mp1.Shutdown(); } + +#ifdef ENABLE_REMOVE_METER_PREVIEW +TEST(MeterProvider, RemoveMeter) +{ + MeterProvider mp; + + auto m1 = mp.GetMeter("test", "1", "URL"); + ASSERT_NE(nullptr, m1); + + // Will return the same meter + auto m2 = mp.GetMeter("test", "1", "URL"); + ASSERT_NE(nullptr, m2); + ASSERT_EQ(m1, m2); + + mp.RemoveMeter("unknown", "0", ""); + + // Will decrease use_count() on m1 and m2 + mp.RemoveMeter("test", "1", "URL"); + + // Will create a different meter + auto m3 = mp.GetMeter("test", "1", "URL"); + ASSERT_NE(nullptr, m3); + ASSERT_NE(m1, m3); + ASSERT_NE(m2, m3); + + // Will decrease use_count() on m3 + mp.RemoveMeter("test", "1", "URL"); + + // Will do nothing + mp.RemoveMeter("test", "1", "URL"); + + // cleanup properly without crash + mp.ForceFlush(); + mp.Shutdown(); +} +#endif