Fixed review comments.

open-telemetry · Jul 10, 2023 · c894d5b · c894d5b
1 parent f7c5320
commit c894d5b
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 2 deletions.
diff --git 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
@@ -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
@@ -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