From 9b8a4dcdc7b863cfaa60f50ffc8f1f7ebd049b2a Mon Sep 17 00:00:00 2001
From: Ehsan Saei <71217171+esigo@users.noreply.github.com>
Date: Tue, 25 Oct 2022 19:02:05 +0200
Subject: [PATCH] [Metrics API/SDK] Change Meter API/SDK to return
 nostd::unique_ptr for Sync Instruments (#1707)

---
 api/include/opentelemetry/metrics/meter.h     | 19 ++++++------
 api/include/opentelemetry/metrics/noop.h      | 24 +++++++--------
 sdk/include/opentelemetry/sdk/metrics/meter.h | 12 ++++----
 sdk/src/metrics/meter.cc                      | 24 +++++++--------
 sdk/test/metrics/BUILD                        | 30 +++++++++++++++++++
 sdk/test/metrics/meter_test.cc                |  4 +--
 6 files changed, 72 insertions(+), 41 deletions(-)

diff --git a/api/include/opentelemetry/metrics/meter.h b/api/include/opentelemetry/metrics/meter.h
index b9830ed2d3..96fcb4724b 100644
--- a/api/include/opentelemetry/metrics/meter.h
+++ b/api/include/opentelemetry/metrics/meter.h
@@ -9,6 +9,7 @@
 #  include "opentelemetry/nostd/shared_ptr.h"
 #  include "opentelemetry/nostd/span.h"
 #  include "opentelemetry/nostd/string_view.h"
+#  include "opentelemetry/nostd/unique_ptr.h"
 #  include "opentelemetry/version.h"
 
 OPENTELEMETRY_BEGIN_NAMESPACE
@@ -28,7 +29,7 @@ class Meter
   virtual ~Meter() = default;
 
   /**
-   * Creates a Counter with the passed characteristics and returns a shared_ptr to that Counter.
+   * Creates a Counter with the passed characteristics and returns a unique_ptr to that Counter.
    *
    * @param name the name of the new Counter.
    * @param description a brief description of what the Counter is used for.
@@ -36,12 +37,12 @@ class Meter
    * @return a shared pointer to the created Counter.
    */
 
-  virtual nostd::shared_ptr<Counter<uint64_t>> CreateUInt64Counter(
+  virtual nostd::unique_ptr<Counter<uint64_t>> CreateUInt64Counter(
       nostd::string_view name,
       nostd::string_view description = "",
       nostd::string_view unit        = "") noexcept = 0;
 
-  virtual nostd::shared_ptr<Counter<double>> CreateDoubleCounter(
+  virtual nostd::unique_ptr<Counter<double>> CreateDoubleCounter(
       nostd::string_view name,
       nostd::string_view description = "",
       nostd::string_view unit        = "") noexcept = 0;
@@ -65,19 +66,19 @@ class Meter
       nostd::string_view unit        = "") noexcept = 0;
 
   /**
-   * Creates a Histogram with the passed characteristics and returns a shared_ptr to that Histogram.
+   * Creates a Histogram with the passed characteristics and returns a unique_ptr to that Histogram.
    *
    * @param name the name of the new Histogram.
    * @param description a brief description of what the Histogram is used for.
    * @param unit the unit of metric values following https://unitsofmeasure.org/ucum.html.
    * @return a shared pointer to the created Histogram.
    */
-  virtual nostd::shared_ptr<Histogram<uint64_t>> CreateUInt64Histogram(
+  virtual nostd::unique_ptr<Histogram<uint64_t>> CreateUInt64Histogram(
       nostd::string_view name,
       nostd::string_view description = "",
       nostd::string_view unit        = "") noexcept = 0;
 
-  virtual nostd::shared_ptr<Histogram<double>> CreateDoubleHistogram(
+  virtual nostd::unique_ptr<Histogram<double>> CreateDoubleHistogram(
       nostd::string_view name,
       nostd::string_view description = "",
       nostd::string_view unit        = "") noexcept = 0;
@@ -101,7 +102,7 @@ class Meter
       nostd::string_view unit        = "") noexcept = 0;
 
   /**
-   * Creates an UpDownCounter with the passed characteristics and returns a shared_ptr to that
+   * Creates an UpDownCounter with the passed characteristics and returns a unique_ptr to that
    * UpDownCounter.
    *
    * @param name the name of the new UpDownCounter.
@@ -109,12 +110,12 @@ class Meter
    * @param unit the unit of metric values following https://unitsofmeasure.org/ucum.html.
    * @return a shared pointer to the created UpDownCounter.
    */
-  virtual nostd::shared_ptr<UpDownCounter<int64_t>> CreateInt64UpDownCounter(
+  virtual nostd::unique_ptr<UpDownCounter<int64_t>> CreateInt64UpDownCounter(
       nostd::string_view name,
       nostd::string_view description = "",
       nostd::string_view unit        = "") noexcept = 0;
 
-  virtual nostd::shared_ptr<UpDownCounter<double>> CreateDoubleUpDownCounter(
+  virtual nostd::unique_ptr<UpDownCounter<double>> CreateDoubleUpDownCounter(
       nostd::string_view name,
       nostd::string_view description = "",
       nostd::string_view unit        = "") noexcept = 0;
diff --git a/api/include/opentelemetry/metrics/noop.h b/api/include/opentelemetry/metrics/noop.h
index abe841db95..1200c5c6f1 100644
--- a/api/include/opentelemetry/metrics/noop.h
+++ b/api/include/opentelemetry/metrics/noop.h
@@ -87,20 +87,20 @@ class NoopObservableInstrument : public ObservableInstrument
 class NoopMeter final : public Meter
 {
 public:
-  nostd::shared_ptr<Counter<uint64_t>> CreateUInt64Counter(
+  nostd::unique_ptr<Counter<uint64_t>> CreateUInt64Counter(
       nostd::string_view name,
       nostd::string_view description = "",
       nostd::string_view unit        = "") noexcept override
   {
-    return nostd::shared_ptr<Counter<uint64_t>>{new NoopCounter<uint64_t>(name, description, unit)};
+    return nostd::unique_ptr<Counter<uint64_t>>{new NoopCounter<uint64_t>(name, description, unit)};
   }
 
-  nostd::shared_ptr<Counter<double>> CreateDoubleCounter(
+  nostd::unique_ptr<Counter<double>> CreateDoubleCounter(
       nostd::string_view name,
       nostd::string_view description = "",
       nostd::string_view unit        = "") noexcept override
   {
-    return nostd::shared_ptr<Counter<double>>{new NoopCounter<double>(name, description, unit)};
+    return nostd::unique_ptr<Counter<double>>{new NoopCounter<double>(name, description, unit)};
   }
 
   nostd::shared_ptr<ObservableInstrument> CreateInt64ObservableCounter(
@@ -121,21 +121,21 @@ class NoopMeter final : public Meter
         new NoopObservableInstrument(name, description, unit));
   }
 
-  nostd::shared_ptr<Histogram<uint64_t>> CreateUInt64Histogram(
+  nostd::unique_ptr<Histogram<uint64_t>> CreateUInt64Histogram(
       nostd::string_view name,
       nostd::string_view description = "",
       nostd::string_view unit        = "") noexcept override
   {
-    return nostd::shared_ptr<Histogram<uint64_t>>{
+    return nostd::unique_ptr<Histogram<uint64_t>>{
         new NoopHistogram<uint64_t>(name, description, unit)};
   }
 
-  nostd::shared_ptr<Histogram<double>> CreateDoubleHistogram(
+  nostd::unique_ptr<Histogram<double>> CreateDoubleHistogram(
       nostd::string_view name,
       nostd::string_view description = "",
       nostd::string_view unit        = "") noexcept override
   {
-    return nostd::shared_ptr<Histogram<double>>{new NoopHistogram<double>(name, description, unit)};
+    return nostd::unique_ptr<Histogram<double>>{new NoopHistogram<double>(name, description, unit)};
   }
 
   nostd::shared_ptr<ObservableInstrument> CreateInt64ObservableGauge(
@@ -156,21 +156,21 @@ class NoopMeter final : public Meter
         new NoopObservableInstrument(name, description, unit));
   }
 
-  nostd::shared_ptr<UpDownCounter<int64_t>> CreateInt64UpDownCounter(
+  nostd::unique_ptr<UpDownCounter<int64_t>> CreateInt64UpDownCounter(
       nostd::string_view name,
       nostd::string_view description = "",
       nostd::string_view unit        = "") noexcept override
   {
-    return nostd::shared_ptr<UpDownCounter<int64_t>>{
+    return nostd::unique_ptr<UpDownCounter<int64_t>>{
         new NoopUpDownCounter<int64_t>(name, description, unit)};
   }
 
-  nostd::shared_ptr<UpDownCounter<double>> CreateDoubleUpDownCounter(
+  nostd::unique_ptr<UpDownCounter<double>> CreateDoubleUpDownCounter(
       nostd::string_view name,
       nostd::string_view description = "",
       nostd::string_view unit        = "") noexcept override
   {
-    return nostd::shared_ptr<UpDownCounter<double>>{
+    return nostd::unique_ptr<UpDownCounter<double>>{
         new NoopUpDownCounter<double>(name, description, unit)};
   }
 
diff --git a/sdk/include/opentelemetry/sdk/metrics/meter.h b/sdk/include/opentelemetry/sdk/metrics/meter.h
index b2e8e49709..c9d9fe1566 100644
--- a/sdk/include/opentelemetry/sdk/metrics/meter.h
+++ b/sdk/include/opentelemetry/sdk/metrics/meter.h
@@ -35,12 +35,12 @@ class Meter final : public opentelemetry::metrics::Meter
       std::unique_ptr<opentelemetry::sdk::instrumentationscope::InstrumentationScope> scope =
           opentelemetry::sdk::instrumentationscope::InstrumentationScope::Create("")) noexcept;
 
-  nostd::shared_ptr<opentelemetry::metrics::Counter<uint64_t>> CreateUInt64Counter(
+  nostd::unique_ptr<opentelemetry::metrics::Counter<uint64_t>> CreateUInt64Counter(
       nostd::string_view name,
       nostd::string_view description = "",
       nostd::string_view unit        = "") noexcept override;
 
-  nostd::shared_ptr<opentelemetry::metrics::Counter<double>> CreateDoubleCounter(
+  nostd::unique_ptr<opentelemetry::metrics::Counter<double>> CreateDoubleCounter(
       nostd::string_view name,
       nostd::string_view description = "",
       nostd::string_view unit        = "") noexcept override;
@@ -55,12 +55,12 @@ class Meter final : public opentelemetry::metrics::Meter
       nostd::string_view description = "",
       nostd::string_view unit        = "") noexcept override;
 
-  nostd::shared_ptr<opentelemetry::metrics::Histogram<uint64_t>> CreateUInt64Histogram(
+  nostd::unique_ptr<opentelemetry::metrics::Histogram<uint64_t>> CreateUInt64Histogram(
       nostd::string_view name,
       nostd::string_view description = "",
       nostd::string_view unit        = "") noexcept override;
 
-  nostd::shared_ptr<opentelemetry::metrics::Histogram<double>> CreateDoubleHistogram(
+  nostd::unique_ptr<opentelemetry::metrics::Histogram<double>> CreateDoubleHistogram(
       nostd::string_view name,
       nostd::string_view description = "",
       nostd::string_view unit        = "") noexcept override;
@@ -75,12 +75,12 @@ class Meter final : public opentelemetry::metrics::Meter
       nostd::string_view description = "",
       nostd::string_view unit        = "") noexcept override;
 
-  nostd::shared_ptr<opentelemetry::metrics::UpDownCounter<int64_t>> CreateInt64UpDownCounter(
+  nostd::unique_ptr<opentelemetry::metrics::UpDownCounter<int64_t>> CreateInt64UpDownCounter(
       nostd::string_view name,
       nostd::string_view description = "",
       nostd::string_view unit        = "") noexcept override;
 
-  nostd::shared_ptr<opentelemetry::metrics::UpDownCounter<double>> CreateDoubleUpDownCounter(
+  nostd::unique_ptr<opentelemetry::metrics::UpDownCounter<double>> CreateDoubleUpDownCounter(
       nostd::string_view name,
       nostd::string_view description = "",
       nostd::string_view unit        = "") noexcept override;
diff --git a/sdk/src/metrics/meter.cc b/sdk/src/metrics/meter.cc
index 86ba798e03..bff0cf5535 100644
--- a/sdk/src/metrics/meter.cc
+++ b/sdk/src/metrics/meter.cc
@@ -34,7 +34,7 @@ Meter::Meter(
       observable_registry_(new ObservableRegistry())
 {}
 
-nostd::shared_ptr<metrics::Counter<uint64_t>> Meter::CreateUInt64Counter(
+nostd::unique_ptr<metrics::Counter<uint64_t>> Meter::CreateUInt64Counter(
     nostd::string_view name,
     nostd::string_view description,
     nostd::string_view unit) noexcept
@@ -43,11 +43,11 @@ nostd::shared_ptr<metrics::Counter<uint64_t>> Meter::CreateUInt64Counter(
       std::string{name.data(), name.size()}, std::string{description.data(), description.size()},
       std::string{unit.data(), unit.size()}, InstrumentType::kCounter, InstrumentValueType::kLong};
   auto storage = RegisterSyncMetricStorage(instrument_descriptor);
-  return nostd::shared_ptr<metrics::Counter<uint64_t>>(
+  return nostd::unique_ptr<metrics::Counter<uint64_t>>(
       new LongCounter<uint64_t>(instrument_descriptor, std::move(storage)));
 }
 
-nostd::shared_ptr<metrics::Counter<double>> Meter::CreateDoubleCounter(
+nostd::unique_ptr<metrics::Counter<double>> Meter::CreateDoubleCounter(
     nostd::string_view name,
     nostd::string_view description,
     nostd::string_view unit) noexcept
@@ -57,7 +57,7 @@ nostd::shared_ptr<metrics::Counter<double>> Meter::CreateDoubleCounter(
       std::string{unit.data(), unit.size()}, InstrumentType::kCounter,
       InstrumentValueType::kDouble};
   auto storage = RegisterSyncMetricStorage(instrument_descriptor);
-  return nostd::shared_ptr<metrics::Counter<double>>{
+  return nostd::unique_ptr<metrics::Counter<double>>{
       new DoubleCounter(instrument_descriptor, std::move(storage))};
 }
 
@@ -89,7 +89,7 @@ Meter::CreateDoubleObservableCounter(nostd::string_view name,
       new ObservableInstrument(instrument_descriptor, std::move(storage), observable_registry_)};
 }
 
-nostd::shared_ptr<metrics::Histogram<uint64_t>> Meter::CreateUInt64Histogram(
+nostd::unique_ptr<metrics::Histogram<uint64_t>> Meter::CreateUInt64Histogram(
     nostd::string_view name,
     nostd::string_view description,
     nostd::string_view unit) noexcept
@@ -99,11 +99,11 @@ nostd::shared_ptr<metrics::Histogram<uint64_t>> Meter::CreateUInt64Histogram(
       std::string{unit.data(), unit.size()}, InstrumentType::kHistogram,
       InstrumentValueType::kLong};
   auto storage = RegisterSyncMetricStorage(instrument_descriptor);
-  return nostd::shared_ptr<metrics::Histogram<uint64_t>>{
+  return nostd::unique_ptr<metrics::Histogram<uint64_t>>{
       new LongHistogram<uint64_t>(instrument_descriptor, std::move(storage))};
 }
 
-nostd::shared_ptr<metrics::Histogram<double>> Meter::CreateDoubleHistogram(
+nostd::unique_ptr<metrics::Histogram<double>> Meter::CreateDoubleHistogram(
     nostd::string_view name,
     nostd::string_view description,
     nostd::string_view unit) noexcept
@@ -113,7 +113,7 @@ nostd::shared_ptr<metrics::Histogram<double>> Meter::CreateDoubleHistogram(
       std::string{unit.data(), unit.size()}, InstrumentType::kHistogram,
       InstrumentValueType::kDouble};
   auto storage = RegisterSyncMetricStorage(instrument_descriptor);
-  return nostd::shared_ptr<metrics::Histogram<double>>{
+  return nostd::unique_ptr<metrics::Histogram<double>>{
       new DoubleHistogram(instrument_descriptor, std::move(storage))};
 }
 
@@ -145,7 +145,7 @@ nostd::shared_ptr<opentelemetry::metrics::ObservableInstrument> Meter::CreateDou
       new ObservableInstrument(instrument_descriptor, std::move(storage), observable_registry_)};
 }
 
-nostd::shared_ptr<metrics::UpDownCounter<int64_t>> Meter::CreateInt64UpDownCounter(
+nostd::unique_ptr<metrics::UpDownCounter<int64_t>> Meter::CreateInt64UpDownCounter(
     nostd::string_view name,
     nostd::string_view description,
     nostd::string_view unit) noexcept
@@ -155,11 +155,11 @@ nostd::shared_ptr<metrics::UpDownCounter<int64_t>> Meter::CreateInt64UpDownCount
       std::string{unit.data(), unit.size()}, InstrumentType::kUpDownCounter,
       InstrumentValueType::kLong};
   auto storage = RegisterSyncMetricStorage(instrument_descriptor);
-  return nostd::shared_ptr<metrics::UpDownCounter<int64_t>>{
+  return nostd::unique_ptr<metrics::UpDownCounter<int64_t>>{
       new LongUpDownCounter(instrument_descriptor, std::move(storage))};
 }
 
-nostd::shared_ptr<metrics::UpDownCounter<double>> Meter::CreateDoubleUpDownCounter(
+nostd::unique_ptr<metrics::UpDownCounter<double>> Meter::CreateDoubleUpDownCounter(
     nostd::string_view name,
     nostd::string_view description,
     nostd::string_view unit) noexcept
@@ -169,7 +169,7 @@ nostd::shared_ptr<metrics::UpDownCounter<double>> Meter::CreateDoubleUpDownCount
       std::string{unit.data(), unit.size()}, InstrumentType::kUpDownCounter,
       InstrumentValueType::kDouble};
   auto storage = RegisterSyncMetricStorage(instrument_descriptor);
-  return nostd::shared_ptr<metrics::UpDownCounter<double>>{
+  return nostd::unique_ptr<metrics::UpDownCounter<double>>{
       new DoubleUpDownCounter(instrument_descriptor, std::move(storage))};
 }
 
diff --git a/sdk/test/metrics/BUILD b/sdk/test/metrics/BUILD
index 336fbc8b03..5d3076b8be 100644
--- a/sdk/test/metrics/BUILD
+++ b/sdk/test/metrics/BUILD
@@ -1,5 +1,20 @@
 load("//bazel:otel_cc_benchmark.bzl", "otel_cc_benchmark")
 
+cc_test(
+    name = "meter_test",
+    srcs = [
+        "meter_test.cc",
+    ],
+    tags = [
+        "metrics",
+        "test",
+    ],
+    deps = [
+        "//sdk/src/metrics",
+        "@com_google_googletest//:gtest_main",
+    ],
+)
+
 cc_test(
     name = "meter_provider_sdk_test",
     srcs = [
@@ -112,6 +127,21 @@ cc_test(
     ],
 )
 
+cc_test(
+    name = "async_instruments_test",
+    srcs = [
+        "async_instruments_test.cc",
+    ],
+    tags = [
+        "metrics",
+        "test",
+    ],
+    deps = [
+        "//sdk/src/metrics",
+        "@com_google_googletest//:gtest_main",
+    ],
+)
+
 cc_test(
     name = "async_metric_storage_test",
     srcs = [
diff --git a/sdk/test/metrics/meter_test.cc b/sdk/test/metrics/meter_test.cc
index ec78a23707..521c1d0c1c 100644
--- a/sdk/test/metrics/meter_test.cc
+++ b/sdk/test/metrics/meter_test.cc
@@ -82,12 +82,12 @@ TEST(MeterTest, StressMultiThread)
   MetricReader *metric_reader_ptr = nullptr;
   auto meter                      = InitMeter(&metric_reader_ptr, "stress_test_meter");
   std::atomic<unsigned> threadCount(0);
-  size_t numIterations = MAX_ITERATIONS_MT;
+  std::atomic<size_t> numIterations(MAX_ITERATIONS_MT);
   std::atomic<bool> do_collect{false}, do_sync_create{true}, do_async_create{false};
   std::vector<nostd::shared_ptr<opentelemetry::metrics::ObservableInstrument>>
       observable_instruments;
   std::vector<std::thread> meter_operation_threads;
-  size_t instrument_id = 0;
+  std::atomic<size_t> instrument_id(0);
   while (numIterations--)
   {
     for (size_t i = 0; i < MAX_THREADS; i++)