From d9a3d9fa2342949fd7fcc20c2191dd2f78167acf Mon Sep 17 00:00:00 2001 From: Lalit Date: Mon, 13 Feb 2023 18:36:41 -0800 Subject: [PATCH 1/8] Fix --- exporters/prometheus/src/collector.cc | 3 +++ exporters/prometheus/src/exporter.cc | 18 ++++++++++++++++-- exporters/prometheus/test/exporter_test.cc | 5 +++-- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/exporters/prometheus/src/collector.cc b/exporters/prometheus/src/collector.cc index 77d4255679..b798d63a7c 100644 --- a/exporters/prometheus/src/collector.cc +++ b/exporters/prometheus/src/collector.cc @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 #include "opentelemetry/exporters/prometheus/collector.h" +#include "opentelemetry/sdk/common/global_log_handler.h" namespace metric_sdk = opentelemetry::sdk::metrics; @@ -27,6 +28,8 @@ std::vector PrometheusCollector::Collect() cons { if (reader_->IsShutdown()) { + OTEL_INTERNAL_LOG_WARN("[Prometheus Exporter] ERROR: Collect: " + "Exporter is shutdown, cann't invoke collect operation."); return {}; } collection_lock_.lock(); diff --git a/exporters/prometheus/src/exporter.cc b/exporters/prometheus/src/exporter.cc index 946d84bf91..235a3d8b0f 100644 --- a/exporters/prometheus/src/exporter.cc +++ b/exporters/prometheus/src/exporter.cc @@ -2,6 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 #include "opentelemetry/exporters/prometheus/exporter.h" +#include "prometheus/CivetServer.h" +#include "opentelemetry/sdk/common/global_log_handler.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -16,7 +18,18 @@ namespace metrics */ PrometheusExporter::PrometheusExporter(const PrometheusExporterOptions &options) : options_(options) { - exposer_ = std::unique_ptr<::prometheus::Exposer>(new ::prometheus::Exposer{options_.url}); + try { + exposer_ = std::unique_ptr<::prometheus::Exposer>(new ::prometheus::Exposer{options_.url}); + } + catch (const CivetException &ex) + { + std::cout << "Exception caught\n"; + exposer_ = nullptr; + OTEL_INTERNAL_LOG_ERROR("[Prometheus Exporter] ERROR: Init: " + << " Cannot Initialize start HTTP server exception :"<< ex.what() << std::endl); + Shutdown(); // set MetricReader in shutdown state. + return; + } collector_ = std::shared_ptr(new PrometheusCollector(this)); exposer_->RegisterCollectable(collector_); @@ -36,7 +49,8 @@ bool PrometheusExporter::OnForceFlush(std::chrono::microseconds /* timeout */) n bool PrometheusExporter::OnShutDown(std::chrono::microseconds /* timeout */) noexcept { - exposer_->RemoveCollectable(collector_); + if (exposer_ != nullptr) + exposer_->RemoveCollectable(collector_); return true; } diff --git a/exporters/prometheus/test/exporter_test.cc b/exporters/prometheus/test/exporter_test.cc index 3bda4c3d7b..d29f8b0b91 100644 --- a/exporters/prometheus/test/exporter_test.cc +++ b/exporters/prometheus/test/exporter_test.cc @@ -28,11 +28,12 @@ using opentelemetry::sdk::metrics::InstrumentType; TEST(PrometheusExporter, InitializeConstructorIsNotShutdown) { PrometheusExporterOptions options; - options.url = "localhost:8081"; + options.url = "localhost:8084"; PrometheusExporter exporter(options); // // Asserts that the exporter is not shutdown. - // ASSERT_TRUE(!exporter.IsShutdown()); + ASSERT_TRUE(exporter.IsShutdown()); exporter.Shutdown(); + ASSERT_TRUE(exporter.IsShutdown()); } /** From cb06cab366f9f8c0ded450d167bc7f1cfe3479fe Mon Sep 17 00:00:00 2001 From: Lalit Date: Tue, 14 Feb 2023 23:11:46 -0800 Subject: [PATCH 2/8] fix --- examples/prometheus/README.md | 24 ++---------------------- examples/prometheus/main.cc | 2 +- exporters/prometheus/src/exporter.cc | 16 ++++++++-------- 3 files changed, 11 insertions(+), 31 deletions(-) diff --git a/examples/prometheus/README.md b/examples/prometheus/README.md index 782e874644..a47098cc57 100644 --- a/examples/prometheus/README.md +++ b/examples/prometheus/README.md @@ -11,31 +11,12 @@ ## Export metrics from the application -It is highly recommended to go over the [ostream-metrics](../metrics_simple/README.md) -doc before following along this document. - Run the application with: ```sh bazel run //examples/prometheus:prometheus_example ``` -The main difference between the [ostream-metrics](../metrics_simple/README.md) -example with this one is that the line below is replaced: - -```cpp -std::unique_ptr exporter{ - new exportermetrics::OStreamMetricExporter}; - -``` - -with - -```cpp -std::unique_ptr exporter{ - new metrics_exporter::PrometheusExporter(opts)}; -``` - OpenTelemetry `PrometheusExporter` will export data via the endpoint defined by `metrics_exporter::PrometheusExporterOptions::url`, @@ -46,8 +27,7 @@ graph LR subgraph SDK MeterProvider - MetricReader[PeriodicExportingMetricReader] - PrometheusExporter["PrometheusExporter
(http://localhost:9464/)"] + MetricReader[PrometheusExporter
(http://localhost:9464/)"] end subgraph API @@ -56,7 +36,7 @@ end Instrument --> | Measurements | MeterProvider -MeterProvider --> | Metrics | MetricReader --> | Pull | PrometheusExporter +MeterProvider --> | Metrics | MetricReader ``` Also, for our learning purpose, we use a while-loop to keep recoring random diff --git a/examples/prometheus/main.cc b/examples/prometheus/main.cc index 7a23006b6a..243e58e723 100644 --- a/examples/prometheus/main.cc +++ b/examples/prometheus/main.cc @@ -78,7 +78,7 @@ void CleanupMetrics() int main(int argc, char **argv) { std::string example_type; - std::string addr{"localhost:8080"}; + std::string addr{"localhost:9464"}; if (argc == 1) { std::puts("usage: $prometheus_example "); diff --git a/exporters/prometheus/src/exporter.cc b/exporters/prometheus/src/exporter.cc index 235a3d8b0f..bbbddb287e 100644 --- a/exporters/prometheus/src/exporter.cc +++ b/exporters/prometheus/src/exporter.cc @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 #include "opentelemetry/exporters/prometheus/exporter.h" -#include "prometheus/CivetServer.h" #include "opentelemetry/sdk/common/global_log_handler.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -18,16 +17,17 @@ namespace metrics */ PrometheusExporter::PrometheusExporter(const PrometheusExporterOptions &options) : options_(options) { - try { - exposer_ = std::unique_ptr<::prometheus::Exposer>(new ::prometheus::Exposer{options_.url}); + try + { + exposer_ = std::unique_ptr<::prometheus::Exposer>(new ::prometheus::Exposer{options_.url}); } - catch (const CivetException &ex) + catch (const std::exception &ex) // const CivetException &ex) { - std::cout << "Exception caught\n"; exposer_ = nullptr; - OTEL_INTERNAL_LOG_ERROR("[Prometheus Exporter] ERROR: Init: " - << " Cannot Initialize start HTTP server exception :"<< ex.what() << std::endl); - Shutdown(); // set MetricReader in shutdown state. + OTEL_INTERNAL_LOG_ERROR("[Prometheus Exporter] " + << "Can't initialize prometheus exposer with endpoint: " << options_.url + << "\nError: " << ex.what()); + Shutdown(); // set MetricReader in shutdown state. return; } collector_ = std::shared_ptr(new PrometheusCollector(this)); From 42dede4ca47baa3700dc549800e8a972eeada5a6 Mon Sep 17 00:00:00 2001 From: Lalit Date: Tue, 14 Feb 2023 23:30:55 -0800 Subject: [PATCH 3/8] fix --- examples/prometheus/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/prometheus/README.md b/examples/prometheus/README.md index a47098cc57..d36228a5b7 100644 --- a/examples/prometheus/README.md +++ b/examples/prometheus/README.md @@ -27,7 +27,7 @@ graph LR subgraph SDK MeterProvider - MetricReader[PrometheusExporter
(http://localhost:9464/)"] + MetricReader["PrometheusExporter
(http://localhost:9464/)"] end subgraph API From 54ac2dc09e4acbf01e039598197f176f362aa3ac Mon Sep 17 00:00:00 2001 From: Lalit Date: Tue, 14 Feb 2023 23:46:32 -0800 Subject: [PATCH 4/8] fix --- exporters/prometheus/src/exporter.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/exporters/prometheus/src/exporter.cc b/exporters/prometheus/src/exporter.cc index bbbddb287e..7691e32f39 100644 --- a/exporters/prometheus/src/exporter.cc +++ b/exporters/prometheus/src/exporter.cc @@ -21,9 +21,9 @@ PrometheusExporter::PrometheusExporter(const PrometheusExporterOptions &options) { exposer_ = std::unique_ptr<::prometheus::Exposer>(new ::prometheus::Exposer{options_.url}); } - catch (const std::exception &ex) // const CivetException &ex) + catch (const std::exception &ex) { - exposer_ = nullptr; + exposer_.reset(nullptr); OTEL_INTERNAL_LOG_ERROR("[Prometheus Exporter] " << "Can't initialize prometheus exposer with endpoint: " << options_.url << "\nError: " << ex.what()); From f82bd51791a0161789024b13390deaee29f95d1f Mon Sep 17 00:00:00 2001 From: Lalit Date: Tue, 14 Feb 2023 23:51:03 -0800 Subject: [PATCH 5/8] fix --- CHANGELOG.md | 1 + exporters/prometheus/src/collector.cc | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ecbc24737..de2f099dce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ Increment the: * [CI] Enforce copyright check in CI [#1965](https://github.com/open-telemetry/opentelemetry-cpp/pull/1965) * [SEMANTIC CONVENTIONS] Upgrade to version 1.18.0 [#1974](https://github.com/open-telemetry/opentelemetry-cpp/pull/1974) +* Fix Prometheus server crash on listening to already used port [#1986](https://github.com/open-telemetry/opentelemetry-cpp/pull/1986) ## [1.8.2] 2023-01-31 diff --git a/exporters/prometheus/src/collector.cc b/exporters/prometheus/src/collector.cc index b798d63a7c..912514d5d3 100644 --- a/exporters/prometheus/src/collector.cc +++ b/exporters/prometheus/src/collector.cc @@ -28,8 +28,9 @@ std::vector PrometheusCollector::Collect() cons { if (reader_->IsShutdown()) { - OTEL_INTERNAL_LOG_WARN("[Prometheus Exporter] ERROR: Collect: " - "Exporter is shutdown, cann't invoke collect operation."); + OTEL_INTERNAL_LOG_WARN( + "[Prometheus Exporter] ERROR: Collect: " + "Exporter is shutdown, cann't invoke collect operation."); return {}; } collection_lock_.lock(); From f2cba1018fe59e32a941762c3c9104d514fb5978 Mon Sep 17 00:00:00 2001 From: Lalit Date: Tue, 14 Feb 2023 23:55:18 -0800 Subject: [PATCH 6/8] fix comment --- exporters/prometheus/src/collector.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporters/prometheus/src/collector.cc b/exporters/prometheus/src/collector.cc index 912514d5d3..f1ddb93870 100644 --- a/exporters/prometheus/src/collector.cc +++ b/exporters/prometheus/src/collector.cc @@ -29,7 +29,7 @@ std::vector PrometheusCollector::Collect() cons if (reader_->IsShutdown()) { OTEL_INTERNAL_LOG_WARN( - "[Prometheus Exporter] ERROR: Collect: " + "[Prometheus Exporter] Collect: " "Exporter is shutdown, cann't invoke collect operation."); return {}; } From 23ff03e163ace273ab943a60ab7319250208a30c Mon Sep 17 00:00:00 2001 From: Lalit Date: Wed, 15 Feb 2023 10:51:36 -0800 Subject: [PATCH 7/8] revert unit test for unwanted changes --- exporters/prometheus/test/exporter_test.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/exporters/prometheus/test/exporter_test.cc b/exporters/prometheus/test/exporter_test.cc index d29f8b0b91..3bda4c3d7b 100644 --- a/exporters/prometheus/test/exporter_test.cc +++ b/exporters/prometheus/test/exporter_test.cc @@ -28,12 +28,11 @@ using opentelemetry::sdk::metrics::InstrumentType; TEST(PrometheusExporter, InitializeConstructorIsNotShutdown) { PrometheusExporterOptions options; - options.url = "localhost:8084"; + options.url = "localhost:8081"; PrometheusExporter exporter(options); // // Asserts that the exporter is not shutdown. - ASSERT_TRUE(exporter.IsShutdown()); + // ASSERT_TRUE(!exporter.IsShutdown()); exporter.Shutdown(); - ASSERT_TRUE(exporter.IsShutdown()); } /** From 074ce2d8952f3eb6ad50fcd389b8bb9aaf5fcd62 Mon Sep 17 00:00:00 2001 From: Lalit Date: Wed, 15 Feb 2023 11:26:25 -0800 Subject: [PATCH 8/8] review comment cann't -> can not --- exporters/prometheus/src/collector.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporters/prometheus/src/collector.cc b/exporters/prometheus/src/collector.cc index f1ddb93870..29c0bc8db2 100644 --- a/exporters/prometheus/src/collector.cc +++ b/exporters/prometheus/src/collector.cc @@ -30,7 +30,7 @@ std::vector PrometheusCollector::Collect() cons { OTEL_INTERNAL_LOG_WARN( "[Prometheus Exporter] Collect: " - "Exporter is shutdown, cann't invoke collect operation."); + "Exporter is shutdown, can not invoke collect operation."); return {}; } collection_lock_.lock();