Merge branch 'main' into fix_get_tracer_abiv2_2032

open-telemetry · Oct 18, 2023 · fe36034 · fe36034
2 parents 958d3a1 + 231ca4a
commit fe36034
Show file tree

Hide file tree

Showing 12 changed files with 46 additions and 15 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,16 @@ Increment the:
 
 ## [Unreleased]
 
+* [BUILD] Remove WITH_REMOVE_METER_PREVIEW, use WITH_ABI_VERSION_2 instead
+  [#2370](https://github.com/open-telemetry/opentelemetry-cpp/pull/2370)
+
+Breaking changes:
+
+* [BUILD] Remove WITH_REMOVE_METER_PREVIEW, use WITH_ABI_VERSION_2 instead
+  [#2370](https://github.com/open-telemetry/opentelemetry-cpp/pull/2370)
+  * The experimental `CMake` option `WITH_REMOVE_METER_PREVIEW` is removed,
+    use option `WITH_ABI_VERSION_2` instead.
+
 ## [1.12.0] 2023-10-16
 
 * [BUILD] Support `pkg-config`

diff --git a/api/CMakeLists.txt b/api/CMakeLists.txt
@@ -114,11 +114,6 @@ 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()
-
 target_compile_definitions(
   opentelemetry_api
   INTERFACE OPENTELEMETRY_ABI_VERSION_NO=${OPENTELEMETRY_ABI_VERSION_NO})

diff --git a/api/include/opentelemetry/metrics/meter_provider.h b/api/include/opentelemetry/metrics/meter_provider.h
@@ -126,7 +126,19 @@ class MeterProvider
                                             nostd::string_view schema_url = "") noexcept = 0;
 #endif
 
-#ifdef ENABLE_REMOVE_METER_PREVIEW
+#if OPENTELEMETRY_ABI_VERSION_NO >= 2
+  /**
+   * Remove a named Meter instance (ABI).
+   *
+   * This API is experimental, see
+   * https://github.com/open-telemetry/opentelemetry-specification/issues/2232
+   *
+   * @since ABI_VERSION 2
+   *
+   * @param[in] name Meter instrumentation scope
+   * @param[in] version Instrumentation scope version, optional
+   * @param[in] schema_url Instrumentation scope schema URL, optional
+   */
   virtual void RemoveMeter(nostd::string_view name,
                            nostd::string_view version    = "",
                            nostd::string_view schema_url = "") noexcept = 0;

diff --git a/api/include/opentelemetry/metrics/noop.h b/api/include/opentelemetry/metrics/noop.h
@@ -214,7 +214,7 @@ class NoopMeterProvider final : public MeterProvider
   }
 #endif
 
-#ifdef ENABLE_REMOVE_METER_PREVIEW
+#if OPENTELEMETRY_ABI_VERSION_NO >= 2
   void RemoveMeter(nostd::string_view /* name */,
                    nostd::string_view /* version */,
                    nostd::string_view /* schema_url */) noexcept override

diff --git a/ci/do_ci.sh b/ci/do_ci.sh
@@ -112,7 +112,6 @@ 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 \
@@ -135,7 +134,6 @@ 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 \
@@ -159,7 +157,6 @@ 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 \
@@ -181,7 +178,6 @@ elif [[ "$1" == "cmake.maintainer.abiv2.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/sdk/include/opentelemetry/sdk/metrics/async_instruments.h b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h
@@ -25,6 +25,7 @@ class ObservableInstrument : public opentelemetry::metrics::ObservableInstrument
   ObservableInstrument(InstrumentDescriptor instrument_descriptor,
                        std::unique_ptr<AsyncWritableMetricStorage> storage,
                        std::shared_ptr<ObservableRegistry> observable_registry);
+  ~ObservableInstrument() override;
 
   void AddCallback(opentelemetry::metrics::ObservableCallbackPtr callback,
                    void *state) noexcept override;

diff --git a/sdk/include/opentelemetry/sdk/metrics/meter_provider.h b/sdk/include/opentelemetry/sdk/metrics/meter_provider.h
@@ -66,7 +66,7 @@ class MeterProvider final : public opentelemetry::metrics::MeterProvider
       nostd::string_view schema_url = "") noexcept override;
 #endif
 
-#ifdef ENABLE_REMOVE_METER_PREVIEW
+#if OPENTELEMETRY_ABI_VERSION_NO >= 2
   void RemoveMeter(nostd::string_view name,
                    nostd::string_view version,
                    nostd::string_view schema_url) noexcept override;

diff --git a/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h b/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h
@@ -35,6 +35,8 @@ class ObservableRegistry
                       void *state,
                       opentelemetry::metrics::ObservableInstrument *instrument);
 
+  void CleanupCallback(opentelemetry::metrics::ObservableInstrument *instrument);
+
   void Observe(opentelemetry::common::SystemTimestamp collection_ts);
 
 private:

diff --git a/sdk/src/metrics/async_instruments.cc b/sdk/src/metrics/async_instruments.cc
@@ -21,6 +21,11 @@ ObservableInstrument::ObservableInstrument(InstrumentDescriptor instrument_descr
 
 {}
 
+ObservableInstrument::~ObservableInstrument()
+{
+  observable_registry_->CleanupCallback(this);
+}
+
 void ObservableInstrument::AddCallback(opentelemetry::metrics::ObservableCallbackPtr callback,
                                        void *state) noexcept
 {

diff --git a/sdk/src/metrics/meter_provider.cc b/sdk/src/metrics/meter_provider.cc
@@ -75,7 +75,7 @@ nostd::shared_ptr<metrics_api::Meter> MeterProvider::GetMeter(
   return nostd::shared_ptr<metrics_api::Meter>{meter};
 }
 
-#ifdef ENABLE_REMOVE_METER_PREVIEW
+#if OPENTELEMETRY_ABI_VERSION_NO >= 2
 void MeterProvider::RemoveMeter(nostd::string_view name,
                                 nostd::string_view version,
                                 nostd::string_view schema_url) noexcept

diff --git a/sdk/src/metrics/state/observable_registry.cc b/sdk/src/metrics/state/observable_registry.cc
@@ -38,6 +38,16 @@ void ObservableRegistry::RemoveCallback(opentelemetry::metrics::ObservableCallba
   callbacks_.erase(new_end, callbacks_.end());
 }
 
+void ObservableRegistry::CleanupCallback(opentelemetry::metrics::ObservableInstrument *instrument)
+{
+  std::lock_guard<std::mutex> lock_guard{callbacks_m_};
+  auto iter = std::remove_if(callbacks_.begin(), callbacks_.end(),
+                             [instrument](const std::unique_ptr<ObservableCallbackRecord> &record) {
+                               return record->instrument == instrument;
+                             });
+  callbacks_.erase(iter, callbacks_.end());
+}
+
 void ObservableRegistry::Observe(opentelemetry::common::SystemTimestamp collection_ts)
 {
   std::lock_guard<std::mutex> lock_guard{callbacks_m_};

diff --git a/sdk/test/metrics/meter_provider_sdk_test.cc b/sdk/test/metrics/meter_provider_sdk_test.cc
@@ -191,7 +191,7 @@ TEST(MeterProvider, GetMeterAbiv2)
 }
 #endif /* OPENTELEMETRY_ABI_VERSION_NO >= 2 */
 
-#ifdef ENABLE_REMOVE_METER_PREVIEW
+#if OPENTELEMETRY_ABI_VERSION_NO >= 2
 TEST(MeterProvider, RemoveMeter)
 {
   MeterProvider mp;
@@ -225,4 +225,4 @@ TEST(MeterProvider, RemoveMeter)
   mp.ForceFlush();
   mp.Shutdown();
 }
-#endif
+#endif /* OPENTELEMETRY_ABI_VERSION_NO >= 2 */