Simplifying SDK Configuration: Removing Exporter Registration from Meter Provider

examples/metrics_simple/metrics_ostream.cc

@@ -30,7 +30,6 @@ namespace
 void initMetrics(const std::string &name)
 {
   std::unique_ptr<metric_sdk::MetricExporter> exporter{new exportermetrics::OStreamMetricExporter};
-  std::vector<std::unique_ptr<metric_sdk::MetricExporter>> exporters;
 
   std::string version{"1.2.0"};
   std::string schema{"https://opentelemetry.io/schemas/1.2.0"};
@@ -41,9 +40,8 @@ void initMetrics(const std::string &name)
   options.export_timeout_millis  = std::chrono::milliseconds(500);
   std::unique_ptr<metric_sdk::MetricReader> reader{
       new metric_sdk::PeriodicExportingMetricReader(std::move(exporter), options)};
-  auto provider = std::shared_ptr<metrics_api::MeterProvider>(
-      new metric_sdk::MeterProvider(std::move(exporters)));
-  auto p = std::static_pointer_cast<metric_sdk::MeterProvider>(provider);
+  auto provider = std::shared_ptr<metrics_api::MeterProvider>(new metric_sdk::MeterProvider());
+  auto p        = std::static_pointer_cast<metric_sdk::MeterProvider>(provider);
   p->AddMetricReader(std::move(reader));
 
   // counter view

sdk/include/opentelemetry/sdk/metrics/meter_context.h

@@ -24,7 +24,6 @@ namespace metrics
 
 // forward declaration
 class Meter;
-class MetricExporter;
 class MetricReader;
 
 /**
@@ -36,13 +35,11 @@ class MeterContext : public std::enable_shared_from_this<MeterContext>
 public:
   /**
    * Initialize a new meter provider
-   * @param exporters The exporters to be configured with meter context
    * @param readers The readers to be configured with meter context.
    * @param views The views to be configured with meter context.
    * @param resource  The resource for this meter context.
    */
   MeterContext(
-      std::vector<std::unique_ptr<sdk::metrics::MetricExporter>> &&exporters,
       std::unique_ptr<ViewRegistry> views = std::unique_ptr<ViewRegistry>(new ViewRegistry()),
       opentelemetry::sdk::resource::Resource resource =
           opentelemetry::sdk::resource::Resource::Create({})) noexcept;
@@ -77,16 +74,6 @@ class MeterContext : public std::enable_shared_from_this<MeterContext>
    */
   opentelemetry::common::SystemTimestamp GetSDKStartTime() noexcept;
 
-  /**
-   * Attaches a metric exporter to list of configured exporters for this Meter context.
-   * @param exporter The metric exporter for this meter context. This
-   * must not be a nullptr.
-   *
-   * Note: This exporter may not receive any in-flight meter data, but will get newly created meter
-   * data. Note: This method is not thread safe, and should ideally be called from main thread.
-   */
-  void AddMetricExporter(std::unique_ptr<MetricExporter> exporter) noexcept;
-
   /**
    * Attaches a metric reader to list of configured readers for this Meter context.
    * @param reader The metric reader for this meter context. This
@@ -118,20 +105,19 @@ class MeterContext : public std::enable_shared_from_this<MeterContext>
   void AddMeter(std::shared_ptr<Meter> meter);
 
   /**
-   * Force all active Exporters and Collectors to flush any buffered meter data
+   * Force all active Collectors to flush any buffered meter data
    * within the given timeout.
    */
 
   bool ForceFlush(std::chrono::microseconds timeout = (std::chrono::microseconds::max)()) noexcept;
 
   /**
-   * Shutdown the Exporters and Collectors associated with this meter provider.
+   * Shutdown the Collectors associated with this meter provider.
    */
   bool Shutdown() noexcept;
 
 private:
   opentelemetry::sdk::resource::Resource resource_;
-  std::vector<std::unique_ptr<MetricExporter>> exporters_;
   std::vector<std::shared_ptr<CollectorHandle>> collectors_;
   std::unique_ptr<ViewRegistry> views_;
   opentelemetry::common::SystemTimestamp sdk_start_ts_;

sdk/include/opentelemetry/sdk/metrics/meter_provider.h

@@ -22,20 +22,17 @@ namespace metrics
 
 // forward declaration
 class MetricCollector;
-class MetricExporter;
 class MetricReader;
 
 class MeterProvider final : public opentelemetry::metrics::MeterProvider
 {
 public:
   /**
    * Initialize a new meter provider
-   * @param exporters The span exporters for this meter provider
    * @param views The views for this meter provider
    * @param resource  The resources for this meter provider.
    */
   MeterProvider(
-      std::vector<std::unique_ptr<MetricExporter>> &&exporters,
       std::unique_ptr<ViewRegistry> views = std::unique_ptr<ViewRegistry>(new ViewRegistry()),
       sdk::resource::Resource resource    = sdk::resource::Resource::Create({})) noexcept;
 
@@ -56,16 +53,6 @@ class MeterProvider final : public opentelemetry::metrics::MeterProvider
    */
   const sdk::resource::Resource &GetResource() const noexcept;
 
-  /**
-   * Attaches a metric exporter to list of configured exporters for this Meter provider.
-   * @param exporter The metric exporter for this meter provider. This
-   * must not be a nullptr.
-   *
-   * Note: This exporter may not receive any in-flight meter data, but will get newly created meter
-   * data. Note: This method is not thread safe, and should ideally be called from main thread.
-   */
-  void AddMetricExporter(std::unique_ptr<MetricExporter> exporter) noexcept;
-
   /**
    * Attaches a metric reader to list of configured readers for this Meter providers.
    * @param reader The metric reader for this meter provider. This

sdk/src/metrics/meter_context.cc

@@ -4,7 +4,6 @@
 #ifndef ENABLE_METRICS_PREVIEW
 #  include "opentelemetry/sdk/metrics/meter_context.h"
 #  include "opentelemetry/sdk/common/global_log_handler.h"
-#  include "opentelemetry/sdk/metrics/metric_exporter.h"
 #  include "opentelemetry/sdk/metrics/metric_reader.h"
 #  include "opentelemetry/sdk_config.h"
 #  include "opentelemetry/version.h"
@@ -15,13 +14,9 @@ namespace sdk
 namespace metrics
 {
 
-MeterContext::MeterContext(std::vector<std::unique_ptr<MetricExporter>> &&exporters,
-                           std::unique_ptr<ViewRegistry> views,
+MeterContext::MeterContext(std::unique_ptr<ViewRegistry> views,
                            opentelemetry::sdk::resource::Resource resource) noexcept
-    : resource_{resource},
-      exporters_(std::move(exporters)),
-      views_(std::move(views)),
-      sdk_start_ts_{std::chrono::system_clock::now()}
+    : resource_{resource}, views_(std::move(views)), sdk_start_ts_{std::chrono::system_clock::now()}
 {}
 
 const resource::Resource &MeterContext::GetResource() const noexcept
@@ -49,11 +44,6 @@ opentelemetry::common::SystemTimestamp MeterContext::GetSDKStartTime() noexcept
   return sdk_start_ts_;
 }
 
-void MeterContext::AddMetricExporter(std::unique_ptr<MetricExporter> exporter) noexcept
-{
-  exporters_.push_back(std::move(exporter));
-}
-
 void MeterContext::AddMetricReader(std::unique_ptr<MetricReader> reader) noexcept
 {
   auto collector =
@@ -75,51 +65,41 @@ void MeterContext::AddMeter(std::shared_ptr<Meter> meter)
 
 bool MeterContext::Shutdown() noexcept
 {
-  bool return_status = true;
+  bool result = true;
   if (!shutdown_latch_.test_and_set(std::memory_order_acquire))
   {
-    bool result_exporter  = true;
-    bool result_reader    = true;
-    bool result_collector = true;
 
-    for (auto &exporter : exporters_)
-    {
-      bool status     = exporter->Shutdown();
-      result_exporter = result_exporter && status;
-    }
-    if (!result_exporter)
-    {
-      OTEL_INTERNAL_LOG_WARN("[MeterContext::Shutdown] Unable to shutdown all metric exporters");
-    }
     for (auto &collector : collectors_)
     {
-      bool status      = std::static_pointer_cast<MetricCollector>(collector)->Shutdown();
-      result_collector = result_reader && status;
+      bool status = std::static_pointer_cast<MetricCollector>(collector)->Shutdown();
+      result      = result && status;
     }
-    if (!result_collector)
+    if (!result)
     {
       OTEL_INTERNAL_LOG_WARN("[MeterContext::Shutdown] Unable to shutdown all metric readers");
     }
-    return_status = result_exporter && result_collector;
   }
-  return return_status;
+  return result;
 }
 
 bool MeterContext::ForceFlush(std::chrono::microseconds timeout) noexcept
 {
   // TODO - Implement timeout logic.
-  const std::lock_guard<opentelemetry::common::SpinLockMutex> locked(forceflush_lock_);
-  bool result_exporter = true;
-  for (auto &exporter : exporters_)
-  {
-    bool status     = exporter->ForceFlush(timeout);
-    result_exporter = result_exporter && status;
-  }
-  if (!result_exporter)
+  bool result = true;
+  if (!shutdown_latch_.test_and_set(std::memory_order_acquire))
   {
-    OTEL_INTERNAL_LOG_WARN("[MeterContext::ForceFlush] Unable to force-flush all metric exporters");
+
+    for (auto &collector : collectors_)
+    {
+      bool status = std::static_pointer_cast<MetricCollector>(collector)->ForceFlush(timeout);
+      result      = result && status;
+    }
+    if (!result)
+    {
+      OTEL_INTERNAL_LOG_WARN("[MeterContext::ForceFlush] Unable to ForceFlush all metric readers");
+    }
   }
-  return result_exporter;
+  return result;
 }
 
 }  // namespace metrics

sdk/src/metrics/meter_provider.cc

@@ -4,7 +4,6 @@
 #ifndef ENABLE_METRICS_PREVIEW
 #  include "opentelemetry/sdk/metrics/meter_provider.h"
 #  include "opentelemetry/metrics/meter.h"
-#  include "opentelemetry/sdk/metrics/metric_exporter.h"
 #  include "opentelemetry/sdk/metrics/metric_reader.h"
 
 #  include "opentelemetry/sdk/common/global_log_handler.h"
@@ -23,10 +22,9 @@ namespace metrics_api = opentelemetry::metrics;
 
 MeterProvider::MeterProvider(std::shared_ptr<MeterContext> context) noexcept : context_{context} {}
 
-MeterProvider::MeterProvider(std::vector<std::unique_ptr<MetricExporter>> &&exporters,
-                             std::unique_ptr<ViewRegistry> views,
+MeterProvider::MeterProvider(std::unique_ptr<ViewRegistry> views,
                              sdk::resource::Resource resource) noexcept
-    : context_(std::make_shared<MeterContext>(std::move(exporters), std::move(views), resource))
+    : context_(std::make_shared<MeterContext>(std::move(views), resource))
 {}
 
 nostd::shared_ptr<metrics_api::Meter> MeterProvider::GetMeter(
@@ -61,11 +59,6 @@ const resource::Resource &MeterProvider::GetResource() const noexcept
   return context_->GetResource();
 }
 
-void MeterProvider::AddMetricExporter(std::unique_ptr<MetricExporter> exporter) noexcept
-{
-  return context_->AddMetricExporter(std::move(exporter));
-}
-
 void MeterProvider::AddMetricReader(std::unique_ptr<MetricReader> reader) noexcept
 {
   return context_->AddMetricReader(std::move(reader));

sdk/test/metrics/async_metric_storage_test.cc

@@ -46,8 +46,7 @@ TEST(AsyncMetricStorageTest, BasicTests)
   // Some computation here
   auto collection_ts = std::chrono::system_clock::now() + std::chrono::seconds(5);
 
-  std::vector<std::unique_ptr<opentelemetry::sdk::metrics::MetricExporter>> exporters;
-  std::shared_ptr<MeterContext> meter_context(new MeterContext(std::move(exporters)));
+  std::shared_ptr<MeterContext> meter_context(new MeterContext());
   std::unique_ptr<MetricReader> metric_reader(new MockMetricReader(AggregationTemporality::kDelta));
 
   std::shared_ptr<CollectorHandle> collector = std::shared_ptr<CollectorHandle>(

sdk/test/metrics/meter_provider_sdk_test.cc

@@ -38,16 +38,19 @@ class MockMetricExporter : public MetricExporter
 class MockMetricReader : public MetricReader
 {
 public:
+  MockMetricReader(std::unique_ptr<MetricExporter> exporter) : exporter_(std::move(exporter)) {}
   virtual bool OnForceFlush(std::chrono::microseconds timeout) noexcept override { return true; }
   virtual bool OnShutDown(std::chrono::microseconds timeout) noexcept override { return true; }
   virtual void OnInitialized() noexcept override {}
+
+private:
+  std::unique_ptr<MetricExporter> exporter_;
 };
 
 TEST(MeterProvider, GetMeter)
 {
-  std::vector<std::unique_ptr<MetricExporter>> exporters;
 
-  MeterProvider mp1(std::move(exporters));
+  MeterProvider mp1;
   //   std::unique_ptr<View> view{std::unique_ptr<View>()};
   // MeterProvider mp1(std::move(exporters), std::move(readers), std::move(views);
   auto m1 = mp1.GetMeter("test");
@@ -74,11 +77,8 @@ TEST(MeterProvider, GetMeter)
   auto sdkMeter1 = static_cast<Meter *>(m1.get());
 #  endif
   ASSERT_NE(nullptr, sdkMeter1);
-
-  std::unique_ptr<MetricExporter> exporter{new MockMetricExporter()};
-  ASSERT_NO_THROW(mp1.AddMetricExporter(std::move(exporter)));
-
-  std::unique_ptr<MetricReader> reader{new MockMetricReader()};
+  std::unique_ptr<MockMetricExporter> exporter(new MockMetricExporter());
+  std::unique_ptr<MetricReader> reader{new MockMetricReader(std::move(exporter))};
   ASSERT_NO_THROW(mp1.AddMetricReader(std::move(reader)));
 
   std::unique_ptr<View> view{std::unique_ptr<View>()};

sdk/test/metrics/metric_reader_test.cc

@@ -27,12 +27,11 @@ TEST(MetricReaderTest, BasicTests)
   std::unique_ptr<MetricReader> metric_reader1(new MockMetricReader(aggr_temporality));
   EXPECT_EQ(metric_reader1->GetAggregationTemporality(), aggr_temporality);
 
-  std::vector<std::unique_ptr<sdk::metrics::MetricExporter>> exporters;
-  std::shared_ptr<MeterContext> meter_context1(new MeterContext(std::move(exporters)));
+  std::shared_ptr<MeterContext> meter_context1(new MeterContext());
   EXPECT_NO_THROW(meter_context1->AddMetricReader(std::move(metric_reader1)));
 
   std::unique_ptr<MetricReader> metric_reader2(new MockMetricReader(aggr_temporality));
-  std::shared_ptr<MeterContext> meter_context2(new MeterContext(std::move(exporters)));
+  std::shared_ptr<MeterContext> meter_context2(new MeterContext());
   MetricProducer *metric_producer =
       new MetricCollector(std::move(meter_context2), std::move(metric_reader2));
   EXPECT_NO_THROW(metric_producer->Collect([](ResourceMetrics &metric_data) { return true; }));