diff --git a/examples/metrics_simple/metrics_ostream.cc b/examples/metrics_simple/metrics_ostream.cc index d0e4dccfeb..ebb8b34b8a 100644 --- a/examples/metrics_simple/metrics_ostream.cc +++ b/examples/metrics_simple/metrics_ostream.cc @@ -30,7 +30,6 @@ namespace void initMetrics(const std::string &name) { std::unique_ptr exporter{new exportermetrics::OStreamMetricExporter}; - std::vector> 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 reader{ new metric_sdk::PeriodicExportingMetricReader(std::move(exporter), options)}; - auto provider = std::shared_ptr( - new metric_sdk::MeterProvider(std::move(exporters))); - auto p = std::static_pointer_cast(provider); + auto provider = std::shared_ptr(new metric_sdk::MeterProvider()); + auto p = std::static_pointer_cast(provider); p->AddMetricReader(std::move(reader)); // counter view diff --git a/sdk/include/opentelemetry/sdk/metrics/meter_context.h b/sdk/include/opentelemetry/sdk/metrics/meter_context.h index 74f67a1c46..e35700175d 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter_context.h +++ b/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 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> &&exporters, std::unique_ptr views = std::unique_ptr(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 */ 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 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 void AddMeter(std::shared_ptr 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> exporters_; std::vector> collectors_; std::unique_ptr views_; opentelemetry::common::SystemTimestamp sdk_start_ts_; diff --git a/sdk/include/opentelemetry/sdk/metrics/meter_provider.h b/sdk/include/opentelemetry/sdk/metrics/meter_provider.h index c6efba6222..685f43e747 100644 --- a/sdk/include/opentelemetry/sdk/metrics/meter_provider.h +++ b/sdk/include/opentelemetry/sdk/metrics/meter_provider.h @@ -22,7 +22,6 @@ namespace metrics // forward declaration class MetricCollector; -class MetricExporter; class MetricReader; class MeterProvider final : public opentelemetry::metrics::MeterProvider @@ -30,12 +29,10 @@ 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> &&exporters, std::unique_ptr views = std::unique_ptr(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 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 diff --git a/sdk/src/metrics/meter_context.cc b/sdk/src/metrics/meter_context.cc index f23135fa0c..73721e324d 100644 --- a/sdk/src/metrics/meter_context.cc +++ b/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> &&exporters, - std::unique_ptr views, +MeterContext::MeterContext(std::unique_ptr 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 exporter) noexcept -{ - exporters_.push_back(std::move(exporter)); -} - void MeterContext::AddMetricReader(std::unique_ptr reader) noexcept { auto collector = @@ -75,51 +65,41 @@ void MeterContext::AddMeter(std::shared_ptr 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(collector)->Shutdown(); - result_collector = result_reader && status; + bool status = std::static_pointer_cast(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 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(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 diff --git a/sdk/src/metrics/meter_provider.cc b/sdk/src/metrics/meter_provider.cc index 8a9572c88c..788811cd61 100644 --- a/sdk/src/metrics/meter_provider.cc +++ b/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 context) noexcept : context_{context} {} -MeterProvider::MeterProvider(std::vector> &&exporters, - std::unique_ptr views, +MeterProvider::MeterProvider(std::unique_ptr views, sdk::resource::Resource resource) noexcept - : context_(std::make_shared(std::move(exporters), std::move(views), resource)) + : context_(std::make_shared(std::move(views), resource)) {} nostd::shared_ptr MeterProvider::GetMeter( @@ -61,11 +59,6 @@ const resource::Resource &MeterProvider::GetResource() const noexcept return context_->GetResource(); } -void MeterProvider::AddMetricExporter(std::unique_ptr exporter) noexcept -{ - return context_->AddMetricExporter(std::move(exporter)); -} - void MeterProvider::AddMetricReader(std::unique_ptr reader) noexcept { return context_->AddMetricReader(std::move(reader)); diff --git a/sdk/test/metrics/async_metric_storage_test.cc b/sdk/test/metrics/async_metric_storage_test.cc index 512e24472c..f939035fb2 100644 --- a/sdk/test/metrics/async_metric_storage_test.cc +++ b/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> exporters; - std::shared_ptr meter_context(new MeterContext(std::move(exporters))); + std::shared_ptr meter_context(new MeterContext()); std::unique_ptr metric_reader(new MockMetricReader(AggregationTemporality::kDelta)); std::shared_ptr collector = std::shared_ptr( diff --git a/sdk/test/metrics/meter_provider_sdk_test.cc b/sdk/test/metrics/meter_provider_sdk_test.cc index 4a2792c3a7..b0fabe50bb 100644 --- a/sdk/test/metrics/meter_provider_sdk_test.cc +++ b/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 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 exporter_; }; TEST(MeterProvider, GetMeter) { - std::vector> exporters; - MeterProvider mp1(std::move(exporters)); + MeterProvider mp1; // std::unique_ptr view{std::unique_ptr()}; // 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(m1.get()); # endif ASSERT_NE(nullptr, sdkMeter1); - - std::unique_ptr exporter{new MockMetricExporter()}; - ASSERT_NO_THROW(mp1.AddMetricExporter(std::move(exporter))); - - std::unique_ptr reader{new MockMetricReader()}; + std::unique_ptr exporter(new MockMetricExporter()); + std::unique_ptr reader{new MockMetricReader(std::move(exporter))}; ASSERT_NO_THROW(mp1.AddMetricReader(std::move(reader))); std::unique_ptr view{std::unique_ptr()}; diff --git a/sdk/test/metrics/metric_reader_test.cc b/sdk/test/metrics/metric_reader_test.cc index 61a4364934..c9c30853df 100644 --- a/sdk/test/metrics/metric_reader_test.cc +++ b/sdk/test/metrics/metric_reader_test.cc @@ -27,12 +27,11 @@ TEST(MetricReaderTest, BasicTests) std::unique_ptr metric_reader1(new MockMetricReader(aggr_temporality)); EXPECT_EQ(metric_reader1->GetAggregationTemporality(), aggr_temporality); - std::vector> exporters; - std::shared_ptr meter_context1(new MeterContext(std::move(exporters))); + std::shared_ptr meter_context1(new MeterContext()); EXPECT_NO_THROW(meter_context1->AddMetricReader(std::move(metric_reader1))); std::unique_ptr metric_reader2(new MockMetricReader(aggr_temporality)); - std::shared_ptr meter_context2(new MeterContext(std::move(exporters))); + std::shared_ptr 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; }));