From c894d5bb4dc5e7f37b500800c04a3ba58ac07da2 Mon Sep 17 00:00:00 2001
From: Marc Alff <marc.alff@oracle.com>
Date: Mon, 10 Jul 2023 10:02:24 +0200
Subject: [PATCH] Fixed review comments.

---
 ci/do_ci.sh                                 |  3 ++
 sdk/src/metrics/meter_context.cc            |  8 ++++-
 sdk/test/metrics/meter_provider_sdk_test.cc | 37 ++++++++++++++++++++-
 3 files changed, 46 insertions(+), 2 deletions(-)

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/sdk/src/metrics/meter_context.cc b/sdk/src/metrics/meter_context.cc
index c6526192ef..7a299bfc63 100644
--- a/sdk/src/metrics/meter_context.cc
+++ b/sdk/src/metrics/meter_context.cc
@@ -91,7 +91,13 @@ void MeterContext::RemoveMeter(nostd::string_view name,
   for (auto &meter : meters_)
   {
     auto scope = meter->GetInstrumentationScope();
-    if (!scope->equal(name, version, schema_url))
+    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);
     }
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> view{std::unique_ptr<View>()};
   // 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