From 5675de51920a7c640dfbd3bfcaabfaf2681b4bdc Mon Sep 17 00:00:00 2001 From: Gregor Jasny Date: Wed, 27 Dec 2023 21:35:12 +0100 Subject: [PATCH] feat: don't create temporary objects for serialization Fix: #646 --- core/CMakeLists.txt | 2 +- core/include/prometheus/collectable.h | 9 +-- core/include/prometheus/family.h | 5 +- core/include/prometheus/iovector.h | 27 +++++++ core/include/prometheus/metric_family.h | 1 - core/include/prometheus/registry.h | 4 +- core/include/prometheus/serializer.h | 12 ++- core/include/prometheus/text_serializer.h | 18 ++++- core/src/family.cc | 13 ++-- core/src/iovector.cc | 58 +++++++++++++++ core/src/registry.cc | 22 ++---- core/src/serializer.cc | 13 ---- core/src/text_serializer.cc | 67 ++++++++++------- core/tests/CMakeLists.txt | 2 +- core/tests/builder_test.cc | 35 ++++++--- core/tests/family_test.cc | 81 ++++++++++++++++----- core/tests/iovector_test.cc | 89 +++++++++++++++++++++++ core/tests/mock_serializer.h | 18 +++++ core/tests/registry_test.cc | 66 +++++++++++++---- core/tests/serializer_test.cc | 76 ------------------- core/tests/text_serializer_test.cc | 36 ++++++++- pull/src/handler.cc | 85 +++++++++++++++------- pull/src/metrics_collector.cc | 10 +-- pull/src/metrics_collector.h | 4 +- push/src/detail/curl_wrapper.cc | 32 +++++++- push/src/detail/curl_wrapper.h | 3 +- push/src/gateway.cc | 18 +++-- 27 files changed, 551 insertions(+), 255 deletions(-) create mode 100644 core/include/prometheus/iovector.h create mode 100644 core/src/iovector.cc delete mode 100644 core/src/serializer.cc create mode 100644 core/tests/iovector_test.cc create mode 100644 core/tests/mock_serializer.h delete mode 100644 core/tests/serializer_test.cc diff --git a/core/CMakeLists.txt b/core/CMakeLists.txt index 24f91fd0..afce5b99 100644 --- a/core/CMakeLists.txt +++ b/core/CMakeLists.txt @@ -10,8 +10,8 @@ add_library(core src/gauge.cc src/histogram.cc src/info.cc + src/iovector.cc src/registry.cc - src/serializer.cc src/summary.cc src/text_serializer.cc ) diff --git a/core/include/prometheus/collectable.h b/core/include/prometheus/collectable.h index e268e508..33b31513 100644 --- a/core/include/prometheus/collectable.h +++ b/core/include/prometheus/collectable.h @@ -1,12 +1,7 @@ #pragma once -#include - #include "prometheus/detail/core_export.h" - -namespace prometheus { -struct MetricFamily; -} +#include "prometheus/serializer.h" namespace prometheus { @@ -19,7 +14,7 @@ class PROMETHEUS_CPP_CORE_EXPORT Collectable { virtual ~Collectable() = default; /// \brief Returns a list of metrics and their samples. - virtual std::vector Collect() const = 0; + virtual void Collect(const Serializer& out) const = 0; }; } // namespace prometheus diff --git a/core/include/prometheus/family.h b/core/include/prometheus/family.h index f5c887bb..5f234b74 100644 --- a/core/include/prometheus/family.h +++ b/core/include/prometheus/family.h @@ -4,7 +4,6 @@ #include #include #include -#include #include "prometheus/client_metric.h" #include "prometheus/collectable.h" @@ -12,7 +11,7 @@ #include "prometheus/detail/future_std.h" #include "prometheus/detail/utils.h" #include "prometheus/labels.h" -#include "prometheus/metric_family.h" +#include "prometheus/serializer.h" // IWYU pragma: no_include "prometheus/counter.h" // IWYU pragma: no_include "prometheus/gauge.h" @@ -140,7 +139,7 @@ class PROMETHEUS_CPP_CORE_EXPORT Family : public Collectable { /// Collect is called by the Registry when collecting metrics. /// /// \return Zero or more samples for each dimensional data. - std::vector Collect() const override; + void Collect(const Serializer& out) const override; private: std::unordered_map, detail::LabelHasher> metrics_; diff --git a/core/include/prometheus/iovector.h b/core/include/prometheus/iovector.h new file mode 100644 index 00000000..499868b0 --- /dev/null +++ b/core/include/prometheus/iovector.h @@ -0,0 +1,27 @@ +#pragma once + +#include +#include +#include +#include + +#include "prometheus/detail/core_export.h" + +namespace prometheus { + +struct PROMETHEUS_CPP_CORE_EXPORT IOVector { + using ByteVector = std::vector; + + bool empty() const; + + std::size_t size() const; + + std::size_t copy(std::size_t offset, void* buffer, + std::size_t bufferSize) const; + + void add(const std::string& str, std::size_t chunkSize); + + std::vector data; +}; + +} // namespace prometheus diff --git a/core/include/prometheus/metric_family.h b/core/include/prometheus/metric_family.h index 1ba3f5a3..b5d81038 100644 --- a/core/include/prometheus/metric_family.h +++ b/core/include/prometheus/metric_family.h @@ -13,6 +13,5 @@ struct PROMETHEUS_CPP_CORE_EXPORT MetricFamily { std::string name; std::string help; MetricType type = MetricType::Untyped; - std::vector metric; }; } // namespace prometheus diff --git a/core/include/prometheus/registry.h b/core/include/prometheus/registry.h index 6603f6c6..4964cc0c 100644 --- a/core/include/prometheus/registry.h +++ b/core/include/prometheus/registry.h @@ -9,7 +9,7 @@ #include "prometheus/detail/core_export.h" #include "prometheus/family.h" #include "prometheus/labels.h" -#include "prometheus/metric_family.h" +#include "prometheus/serializer.h" namespace prometheus { @@ -79,7 +79,7 @@ class PROMETHEUS_CPP_CORE_EXPORT Registry : public Collectable { /// function. /// /// \return Zero or more metrics and their samples. - std::vector Collect() const override; + void Collect(const Serializer& out) const override; /// \brief Removes a metrics family from the registry. /// diff --git a/core/include/prometheus/serializer.h b/core/include/prometheus/serializer.h index c55ae096..69c8bc04 100644 --- a/core/include/prometheus/serializer.h +++ b/core/include/prometheus/serializer.h @@ -1,9 +1,6 @@ #pragma once -#include -#include -#include - +#include "prometheus/client_metric.h" #include "prometheus/detail/core_export.h" #include "prometheus/metric_family.h" @@ -12,9 +9,10 @@ namespace prometheus { class PROMETHEUS_CPP_CORE_EXPORT Serializer { public: virtual ~Serializer() = default; - virtual std::string Serialize(const std::vector&) const; - virtual void Serialize(std::ostream& out, - const std::vector& metrics) const = 0; + + virtual void SerializeHelp(const MetricFamily& family) const = 0; + virtual void SerializeMetrics(const MetricFamily& family, + const ClientMetric& metric) const = 0; }; } // namespace prometheus diff --git a/core/include/prometheus/text_serializer.h b/core/include/prometheus/text_serializer.h index 315a164d..2ef155a2 100644 --- a/core/include/prometheus/text_serializer.h +++ b/core/include/prometheus/text_serializer.h @@ -1,9 +1,11 @@ #pragma once +#include #include -#include +#include "prometheus/client_metric.h" #include "prometheus/detail/core_export.h" +#include "prometheus/iovector.h" #include "prometheus/metric_family.h" #include "prometheus/serializer.h" @@ -11,9 +13,17 @@ namespace prometheus { class PROMETHEUS_CPP_CORE_EXPORT TextSerializer : public Serializer { public: - using Serializer::Serialize; - void Serialize(std::ostream& out, - const std::vector& metrics) const override; + TextSerializer(IOVector& ioVector); + + void SerializeHelp(const MetricFamily& family) const override; + void SerializeMetrics(const MetricFamily& family, + const ClientMetric& metric) const override; + + private: + void Add(const std::ostringstream& stream) const; + + IOVector& ioVector_; + static constexpr std::size_t chunkSize_ = 1 * 1024 * 1024; }; } // namespace prometheus diff --git a/core/src/family.cc b/core/src/family.cc index 8827fa47..ab3dfcd9 100644 --- a/core/src/family.cc +++ b/core/src/family.cc @@ -11,6 +11,7 @@ #include "prometheus/gauge.h" #include "prometheus/histogram.h" #include "prometheus/info.h" +#include "prometheus/metric_family.h" #include "prometheus/summary.h" namespace prometheus { @@ -86,22 +87,24 @@ const Labels& Family::GetConstantLabels() const { } template -std::vector Family::Collect() const { +void Family::Collect(const Serializer& out) const { std::lock_guard lock{mutex_}; if (metrics_.empty()) { - return {}; + return; } auto family = MetricFamily{}; family.name = name_; family.help = help_; family.type = T::metric_type; - family.metric.reserve(metrics_.size()); + + out.SerializeHelp(family); + for (const auto& m : metrics_) { - family.metric.push_back(std::move(CollectMetric(m.first, m.second.get()))); + auto&& metric = CollectMetric(m.first, m.second.get()); + out.SerializeMetrics(family, metric); } - return {family}; } template diff --git a/core/src/iovector.cc b/core/src/iovector.cc new file mode 100644 index 00000000..7e27b360 --- /dev/null +++ b/core/src/iovector.cc @@ -0,0 +1,58 @@ +#include "prometheus/iovector.h" + +#include +#include +#include +#include + +namespace prometheus { + +bool IOVector::empty() const { return data.empty() || !size(); } + +std::size_t IOVector::size() const { + return std::accumulate(begin(data), end(data), std::size_t{0}, + [](std::size_t size, const ByteVector& chunk) { + return size + chunk.size(); + }); +} + +std::size_t IOVector::copy(std::size_t offset, void* buffer, + std::size_t bufferSize) const { + std::size_t copied = 0; + for (const auto& chunk : data) { + if (offset >= chunk.size()) { + offset -= chunk.size(); + continue; + } + + auto chunkSize = std::min(chunk.size() - offset, bufferSize - copied); + std::copy_n(chunk.data() + offset, chunkSize, + reinterpret_cast(buffer) + copied); + copied += chunkSize; + offset = 0; + + if (copied == bufferSize) { + break; + } + } + return copied; +} + +void IOVector::add(const std::string& str, std::size_t chunkSize) { + std::size_t size = str.size(); + std::size_t offset = 0; + + while (size > 0U) { + if (data.empty() || data.back().size() >= chunkSize) { + data.emplace_back(); + data.back().reserve(chunkSize); + } + auto&& chunk = data.back(); + const auto toAdd = std::min(size, chunkSize - chunk.size()); + chunk.insert(chunk.end(), str.data() + offset, str.data() + offset + toAdd); + + size -= toAdd; + offset += toAdd; + } +} +} // namespace prometheus diff --git a/core/src/registry.cc b/core/src/registry.cc index 9e58b0bc..8fb0c792 100644 --- a/core/src/registry.cc +++ b/core/src/registry.cc @@ -3,7 +3,6 @@ #include #include #include -#include #include "prometheus/counter.h" #include "prometheus/detail/future_std.h" @@ -16,11 +15,9 @@ namespace prometheus { namespace { template -void CollectAll(std::vector& results, const T& families) { +void CollectAll(const Serializer& out, const T& families) { for (auto&& collectable : families) { - auto metrics = collectable->Collect(); - results.insert(results.end(), std::make_move_iterator(metrics.begin()), - std::make_move_iterator(metrics.end())); + collectable->Collect(out); } } @@ -43,17 +40,14 @@ Registry::Registry(InsertBehavior insert_behavior) Registry::~Registry() = default; -std::vector Registry::Collect() const { +void Registry::Collect(const Serializer& out) const { std::lock_guard lock{mutex_}; - auto results = std::vector{}; - CollectAll(results, counters_); - CollectAll(results, gauges_); - CollectAll(results, histograms_); - CollectAll(results, infos_); - CollectAll(results, summaries_); - - return results; + CollectAll(out, counters_); + CollectAll(out, gauges_); + CollectAll(out, histograms_); + CollectAll(out, infos_); + CollectAll(out, summaries_); } template <> diff --git a/core/src/serializer.cc b/core/src/serializer.cc deleted file mode 100644 index 8b14dc77..00000000 --- a/core/src/serializer.cc +++ /dev/null @@ -1,13 +0,0 @@ -#include "prometheus/serializer.h" - -#include // IWYU pragma: keep - -namespace prometheus { - -std::string Serializer::Serialize( - const std::vector& metrics) const { - std::ostringstream ss; - Serialize(ss, metrics); - return ss.str(); -} -} // namespace prometheus diff --git a/core/src/text_serializer.cc b/core/src/text_serializer.cc index 0656b505..3f9a66de 100644 --- a/core/src/text_serializer.cc +++ b/core/src/text_serializer.cc @@ -4,6 +4,7 @@ #include #include #include +#include #include #include "prometheus/client_metric.h" @@ -153,67 +154,77 @@ void SerializeHistogram(std::ostream& out, const MetricFamily& family, WriteTail(out, metric); } } +} // namespace + +TextSerializer::TextSerializer(IOVector& ioVector) : ioVector_(ioVector) {} + +void TextSerializer::SerializeHelp(const MetricFamily& family) const { + std::ostringstream out; -void SerializeFamily(std::ostream& out, const MetricFamily& family) { if (!family.help.empty()) { out << "# HELP " << family.name << " " << family.help << "\n"; } switch (family.type) { case MetricType::Counter: out << "# TYPE " << family.name << " counter\n"; - for (auto& metric : family.metric) { - SerializeCounter(out, family, metric); - } break; case MetricType::Gauge: out << "# TYPE " << family.name << " gauge\n"; - for (auto& metric : family.metric) { - SerializeGauge(out, family, metric); - } break; // info is not handled by prometheus, we use gauge as workaround // (https://github.com/OpenObservability/OpenMetrics/blob/98ae26c87b1c3bcf937909a880b32c8be643cc9b/specification/OpenMetrics.md#info-1) case MetricType::Info: out << "# TYPE " << family.name << " gauge\n"; - for (auto& metric : family.metric) { - SerializeInfo(out, family, metric); - } break; case MetricType::Summary: out << "# TYPE " << family.name << " summary\n"; - for (auto& metric : family.metric) { - SerializeSummary(out, family, metric); - } break; case MetricType::Untyped: out << "# TYPE " << family.name << " untyped\n"; - for (auto& metric : family.metric) { - SerializeUntyped(out, family, metric); - } break; case MetricType::Histogram: out << "# TYPE " << family.name << " histogram\n"; - for (auto& metric : family.metric) { - SerializeHistogram(out, family, metric); - } break; } + + Add(out); } -} // namespace -void TextSerializer::Serialize(std::ostream& out, - const std::vector& metrics) const { - auto saved_locale = out.getloc(); - auto saved_precision = out.precision(); +void TextSerializer::SerializeMetrics(const MetricFamily& family, + const ClientMetric& metric) const { + std::ostringstream out; out.imbue(std::locale::classic()); out.precision(std::numeric_limits::max_digits10 - 1); - for (auto& family : metrics) { - SerializeFamily(out, family); + switch (family.type) { + case MetricType::Counter: + SerializeCounter(out, family, metric); + break; + case MetricType::Gauge: + SerializeGauge(out, family, metric); + break; + // info is not handled by prometheus, we use gauge as workaround + // (https://github.com/OpenObservability/OpenMetrics/blob/98ae26c87b1c3bcf937909a880b32c8be643cc9b/specification/OpenMetrics.md#info-1) + case MetricType::Info: + SerializeInfo(out, family, metric); + break; + case MetricType::Summary: + SerializeSummary(out, family, metric); + break; + case MetricType::Untyped: + SerializeUntyped(out, family, metric); + break; + case MetricType::Histogram: + SerializeHistogram(out, family, metric); + break; } - out.imbue(saved_locale); - out.precision(saved_precision); + Add(out); } + +void TextSerializer::Add(const std::ostringstream& stream) const { + ioVector_.add(stream.str(), chunkSize_); +} + } // namespace prometheus diff --git a/core/tests/CMakeLists.txt b/core/tests/CMakeLists.txt index 7aa90d55..3c5f05f8 100644 --- a/core/tests/CMakeLists.txt +++ b/core/tests/CMakeLists.txt @@ -7,8 +7,8 @@ add_executable(prometheus_core_test family_test.cc gauge_test.cc histogram_test.cc + iovector_test.cc registry_test.cc - serializer_test.cc summary_test.cc text_serializer_test.cc utils_test.cc diff --git a/core/tests/builder_test.cc b/core/tests/builder_test.cc index 6fdffafa..60850b58 100644 --- a/core/tests/builder_test.cc +++ b/core/tests/builder_test.cc @@ -4,11 +4,11 @@ #include #include #include -#include #include #include #include +#include "mock_serializer.h" #include "prometheus/client_metric.h" #include "prometheus/counter.h" #include "prometheus/family.h" @@ -16,12 +16,20 @@ #include "prometheus/histogram.h" #include "prometheus/info.h" #include "prometheus/labels.h" +#include "prometheus/metric_family.h" #include "prometheus/registry.h" #include "prometheus/summary.h" namespace prometheus { namespace { +using ::testing::_; +using ::testing::AllOf; +using ::testing::ElementsAre; +using ::testing::Field; +using ::testing::InSequence; +using ::testing::UnorderedElementsAreArray; + class BuilderTest : public testing::Test { protected: std::vector getExpectedLabels() { @@ -40,15 +48,22 @@ class BuilderTest : public testing::Test { } void verifyCollectedLabels() { - const auto collected = registry.Collect(); - - ASSERT_EQ(1U, collected.size()); - EXPECT_EQ(name, collected.at(0).name); - EXPECT_EQ(help, collected.at(0).help); - ASSERT_EQ(1U, collected.at(0).metric.size()); - - EXPECT_THAT(collected.at(0).metric.at(0).label, - testing::UnorderedElementsAreArray(expected_labels)); + MockSerializer serializer; + + { + InSequence seq; + + EXPECT_CALL(serializer, + SerializeHelp(AllOf(Field(&MetricFamily::name, name), + Field(&MetricFamily::help, help)))); + EXPECT_CALL( + serializer, + SerializeMetrics( + _, AllOf(Field(&ClientMetric::label, + UnorderedElementsAreArray(expected_labels))))); + } + + registry.Collect(serializer); } Registry registry; diff --git a/core/tests/family_test.cc b/core/tests/family_test.cc index a263d4df..ab486f8e 100644 --- a/core/tests/family_test.cc +++ b/core/tests/family_test.cc @@ -5,6 +5,7 @@ #include +#include "mock_serializer.h" #include "prometheus/client_metric.h" #include "prometheus/counter.h" #include "prometheus/detail/future_std.h" @@ -15,6 +16,13 @@ namespace prometheus { namespace { +using ::testing::_; +using ::testing::AllOf; +using ::testing::ElementsAre; +using ::testing::Field; +using ::testing::InSequence; +using ::testing::Sequence; + TEST(FamilyTest, labels) { auto const_label = ClientMetric::Label{"component", "test"}; auto dynamic_label = ClientMetric::Label{"status", "200"}; @@ -23,11 +31,20 @@ TEST(FamilyTest, labels) { "Counts all requests", {{const_label.name, const_label.value}}}; family.Add({{dynamic_label.name, dynamic_label.value}}); - auto collected = family.Collect(); - ASSERT_GE(collected.size(), 1U); - ASSERT_GE(collected.at(0).metric.size(), 1U); - EXPECT_THAT(collected.at(0).metric.at(0).label, - ::testing::ElementsAre(const_label, dynamic_label)); + + MockSerializer serializer; + + { + InSequence seq; + EXPECT_CALL(serializer, SerializeHelp(_)); + EXPECT_CALL(serializer, + SerializeMetrics( + _, AllOf(Field(&ClientMetric::label, + ElementsAre(const_label, dynamic_label))))) + .Times(1); + } + + family.Collect(serializer); } TEST(FamilyTest, reject_same_label_keys) { @@ -41,10 +58,20 @@ TEST(FamilyTest, counter_value) { Family family{"total_requests", "Counts all requests", {}}; auto& counter = family.Add({}); counter.Increment(); - auto collected = family.Collect(); - ASSERT_GE(collected.size(), 1U); - ASSERT_GE(collected[0].metric.size(), 1U); - EXPECT_EQ(1, collected[0].metric.at(0).counter.value); + + MockSerializer serializer; + + { + InSequence seq; + EXPECT_CALL(serializer, SerializeHelp(_)); + EXPECT_CALL(serializer, SerializeMetrics( + Field(&MetricFamily::type, MetricType::Counter), + Field(&ClientMetric::counter, + Field(&ClientMetric::Counter::value, 1)))) + .Times(1); + } + + family.Collect(serializer); } TEST(FamilyTest, remove) { @@ -52,9 +79,16 @@ TEST(FamilyTest, remove) { auto& counter1 = family.Add({{"name", "counter1"}}); family.Add({{"name", "counter2"}}); family.Remove(&counter1); - auto collected = family.Collect(); - ASSERT_GE(collected.size(), 1U); - EXPECT_EQ(collected[0].metric.size(), 1U); + + MockSerializer serializer; + + { + InSequence seq; + EXPECT_CALL(serializer, SerializeHelp(_)); + EXPECT_CALL(serializer, SerializeMetrics(_, _)).Times(1); + } + + family.Collect(serializer); } TEST(FamilyTest, removeUnknownMetricMustNotCrash) { @@ -67,10 +101,21 @@ TEST(FamilyTest, Histogram) { auto& histogram1 = family.Add({{"name", "histogram1"}}, Histogram::BucketBoundaries{0, 1, 2}); histogram1.Observe(0); - auto collected = family.Collect(); - ASSERT_EQ(collected.size(), 1U); - ASSERT_GE(collected[0].metric.size(), 1U); - EXPECT_EQ(1U, collected[0].metric.at(0).histogram.sample_count); + + MockSerializer serializer; + + { + InSequence seq; + EXPECT_CALL(serializer, SerializeHelp(_)); + EXPECT_CALL(serializer, + SerializeMetrics( + Field(&MetricFamily::type, MetricType::Histogram), + Field(&ClientMetric::histogram, + Field(&ClientMetric::Histogram::sample_count, 1)))) + .Times(1); + } + + family.Collect(serializer); } TEST(FamilyTest, add_twice) { @@ -106,8 +151,8 @@ TEST(FamilyTest, should_throw_on_invalid_labels) { TEST(FamilyTest, should_not_collect_empty_metrics) { Family family{"total_requests", "Counts all requests", {}}; - auto collected = family.Collect(); - EXPECT_TRUE(collected.empty()); + MockSerializer serializer; + family.Collect(serializer); } TEST(FamilyTest, query_family_if_metric_already_exists) { diff --git a/core/tests/iovector_test.cc b/core/tests/iovector_test.cc new file mode 100644 index 00000000..a66f0463 --- /dev/null +++ b/core/tests/iovector_test.cc @@ -0,0 +1,89 @@ +#include "prometheus/iovector.h" + +#include + +#include + +namespace prometheus { +namespace { + +class IOVectorTest : public testing::Test { + public: + IOVector iov; +}; + +TEST_F(IOVectorTest, emptyWhenNew) { EXPECT_TRUE(iov.empty()); } + +TEST_F(IOVectorTest, emptyWhenVectorsEmpty) { + iov.data.resize(3); + EXPECT_TRUE(iov.empty()); +} + +TEST_F(IOVectorTest, notEmpty) { + iov.data.push_back(IOVector::ByteVector{0xAB}); + EXPECT_FALSE(iov.empty()); +} + +TEST_F(IOVectorTest, sizeIsZeroForNew) { EXPECT_EQ(0L, iov.size()); } + +TEST_F(IOVectorTest, sizeIsZeroWhenVectorsEmpty) { + iov.data.resize(3); + EXPECT_EQ(0L, iov.size()); +} + +TEST_F(IOVectorTest, addsChunkedData) { + const auto chunkSize = 2U; + iov.add("aa", chunkSize); + iov.add("b", chunkSize); + iov.add("bc", chunkSize); + + EXPECT_EQ(5U, iov.size()); + ASSERT_EQ(3U, iov.data.size()); + EXPECT_EQ(2U, iov.data.at(0).size()); + EXPECT_EQ(2U, iov.data.at(1).size()); + EXPECT_EQ(1U, iov.data.at(2).size()); + + static constexpr std::size_t bufferSize = 6; + char buffer[bufferSize + 1] = {}; + + iov.copy(0L, buffer, bufferSize); + EXPECT_STREQ("aabbc", buffer); +} + +class IOVectorCopyTest : public IOVectorTest { + public: + void SetUp() override { + iov.data.push_back(IOVector::ByteVector(1, 'a')); + iov.data.push_back(IOVector::ByteVector(2, 'b')); + iov.data.push_back(IOVector::ByteVector(3, 'c')); + } + + static constexpr std::size_t bufferSize = 6; + char buffer[bufferSize + 1] = {}; +}; + +TEST_F(IOVectorCopyTest, countSize) { EXPECT_EQ(6L, iov.size()); } + +TEST_F(IOVectorCopyTest, copiesData) { + iov.copy(0L, buffer, bufferSize); + EXPECT_STREQ("abbccc", buffer); +} + +TEST_F(IOVectorCopyTest, copiesDataWithOffset) { + iov.copy(2L, buffer, bufferSize); + EXPECT_STREQ("bccc", buffer); +} + +TEST_F(IOVectorCopyTest, copiesDataWithShortBuffer) { + iov.copy(2L, buffer, 2); + EXPECT_STREQ("bc", buffer); +} + +TEST_F(IOVectorCopyTest, skipsEmptyChunks) { + iov.data.at(1).clear(); + iov.copy(0L, buffer, bufferSize); + EXPECT_STREQ("accc", buffer); +} + +} // namespace +} // namespace prometheus diff --git a/core/tests/mock_serializer.h b/core/tests/mock_serializer.h new file mode 100644 index 00000000..a47d95e7 --- /dev/null +++ b/core/tests/mock_serializer.h @@ -0,0 +1,18 @@ +#pragma once + +#include + +#include "prometheus/serializer.h" + +namespace prometheus { + +class MockSerializer : public Serializer { + public: + MOCK_METHOD(void, SerializeHelp, (const MetricFamily& family), + (const, override)); + MOCK_METHOD(void, SerializeMetrics, + (const MetricFamily& family, const ClientMetric& metric), + (const, override)); +}; + +} // namespace prometheus diff --git a/core/tests/registry_test.cc b/core/tests/registry_test.cc index 45b98ff2..43356152 100644 --- a/core/tests/registry_test.cc +++ b/core/tests/registry_test.cc @@ -1,10 +1,11 @@ #include "prometheus/registry.h" -#include +#include #include #include +#include "mock_serializer.h" #include "prometheus/counter.h" #include "prometheus/gauge.h" #include "prometheus/histogram.h" @@ -14,21 +15,43 @@ namespace prometheus { namespace { +using ::testing::_; +using ::testing::AllOf; +using ::testing::ElementsAre; +using ::testing::Field; +using ::testing::InSequence; +using ::testing::Sequence; + TEST(RegistryTest, collect_single_metric_family) { Registry registry{}; auto& counter_family = BuildCounter().Name("test").Help("a test").Register(registry); counter_family.Add({{"name", "counter1"}}); counter_family.Add({{"name", "counter2"}}); - auto collected = registry.Collect(); - ASSERT_EQ(collected.size(), 1U); - EXPECT_EQ(collected[0].name, "test"); - EXPECT_EQ(collected[0].help, "a test"); - ASSERT_EQ(collected[0].metric.size(), 2U); - ASSERT_EQ(collected[0].metric.at(0).label.size(), 1U); - EXPECT_EQ(collected[0].metric.at(0).label.at(0).name, "name"); - ASSERT_EQ(collected[0].metric.at(1).label.size(), 1U); - EXPECT_EQ(collected[0].metric.at(1).label.at(0).name, "name"); + + MockSerializer serializer; + + { + Sequence s1, s2; + EXPECT_CALL( + serializer, + SerializeHelp(AllOf(Field(&MetricFamily::name, "test"), + Field(&MetricFamily::help, "a test"), + Field(&MetricFamily::type, MetricType::Counter)))) + .InSequence(s1, s2); + EXPECT_CALL(serializer, + SerializeMetrics(_, AllOf(Field(&ClientMetric::label, + ElementsAre(ClientMetric::Label{ + "name", "counter1"}))))) + .InSequence(s1); + EXPECT_CALL(serializer, + SerializeMetrics(_, AllOf(Field(&ClientMetric::label, + ElementsAre(ClientMetric::Label{ + "name", "counter2"}))))) + .InSequence(s2); + } + + registry.Collect(serializer); } TEST(RegistryTest, build_histogram_family) { @@ -38,8 +61,16 @@ TEST(RegistryTest, build_histogram_family) { auto& histogram = histogram_family.Add({{"name", "test_histogram_1"}}, Histogram::BucketBoundaries{0, 1, 2}); histogram.Observe(1.1); - auto collected = registry.Collect(); - ASSERT_EQ(collected.size(), 1U); + + MockSerializer serializer; + + { + InSequence seq; + EXPECT_CALL(serializer, SerializeHelp(_)); + EXPECT_CALL(serializer, SerializeMetrics(_, _)).Times(1); + } + + registry.Collect(serializer); } TEST(RegistryTest, unable_to_remove_family) { @@ -132,8 +163,15 @@ TEST(RegistryTest, merge_same_families) { .Add({{"name", "test_counter"}}); } - auto collected = registry.Collect(); - EXPECT_EQ(1U, collected.size()); + MockSerializer serializer; + + { + InSequence seq; + EXPECT_CALL(serializer, SerializeHelp(_)); + EXPECT_CALL(serializer, SerializeMetrics(_, _)).Times(1); + } + + registry.Collect(serializer); } TEST(RegistryTest, do_not_merge_families_with_different_labels) { diff --git a/core/tests/serializer_test.cc b/core/tests/serializer_test.cc deleted file mode 100644 index ebf8fd73..00000000 --- a/core/tests/serializer_test.cc +++ /dev/null @@ -1,76 +0,0 @@ -#include -#include - -#include -#include -#include -#include - -#include "prometheus/counter.h" -#include "prometheus/detail/future_std.h" -#include "prometheus/family.h" -#include "prometheus/metric_family.h" -#include "prometheus/text_serializer.h" -#include "raii_locale.h" - -namespace prometheus { -namespace { - -class SerializerTest : public testing::Test { - public: - void SetUp() override { - Family family{"requests_total", "", {}}; - auto& counter = family.Add({}); - counter.Increment(); - - collected = family.Collect(); - } - - std::vector collected; - TextSerializer textSerializer; -}; - -#ifndef _WIN32 -// This test expects a working German locale to test that floating -// point numbers do not use , but . as a delimiter. -// -// On Debian systems they can be generated by "locale-gen de_DE.UTF-8" -TEST_F(SerializerTest, shouldSerializeLocaleIndependent) { - std::unique_ptr localeWithCommaDecimalSeparator; - - // ignore missing locale and skip test if setup fails - try { - localeWithCommaDecimalSeparator = - detail::make_unique("de_DE.UTF-8"); - } catch (std::runtime_error&) { - GTEST_SKIP(); - } - - const auto serialized = textSerializer.Serialize(collected); - EXPECT_THAT(serialized, testing::HasSubstr(" 1\n")); -} -#endif - -TEST_F(SerializerTest, shouldRestoreStreamState) { - std::ostringstream os; - - // save stream state - auto saved_flags = os.flags(); - auto saved_precision = os.precision(); - auto saved_width = os.width(); - auto saved_fill = os.fill(); - auto saved_locale = os.getloc(); - - // serialize - textSerializer.Serialize(os, collected); - - // check for expected flags - EXPECT_EQ(os.flags(), saved_flags); - EXPECT_EQ(os.precision(), saved_precision); - EXPECT_EQ(os.width(), saved_width); - EXPECT_EQ(os.fill(), saved_fill); - EXPECT_EQ(os.getloc(), saved_locale); -} - -} // namespace -} // namespace prometheus diff --git a/core/tests/text_serializer_test.cc b/core/tests/text_serializer_test.cc index 470583c4..b699bf2f 100644 --- a/core/tests/text_serializer_test.cc +++ b/core/tests/text_serializer_test.cc @@ -5,14 +5,17 @@ #include #include +#include #include #include "prometheus/client_metric.h" +#include "prometheus/detail/future_std.h" #include "prometheus/histogram.h" #include "prometheus/info.h" #include "prometheus/metric_family.h" #include "prometheus/metric_type.h" #include "prometheus/summary.h" +#include "raii_locale.h" namespace prometheus { namespace { @@ -24,16 +27,20 @@ class TextSerializerTest : public testing::Test { metricFamily.name = name; metricFamily.help = "my metric help text"; metricFamily.type = type; - metricFamily.metric = std::vector{metric}; - std::vector families{metricFamily}; + IOVector serialized; + TextSerializer textSerializer{serialized}; - return textSerializer.Serialize(families); + textSerializer.SerializeHelp(metricFamily); + textSerializer.SerializeMetrics(metricFamily, metric); + + std::string out(serialized.size(), '\0'); + serialized.copy(0, &out.front(), out.size()); + return out; } const std::string name = "my_metric"; ClientMetric metric; - TextSerializer textSerializer; }; TEST_F(TextSerializerTest, shouldSerializeNotANumber) { @@ -128,5 +135,26 @@ TEST_F(TextSerializerTest, shouldSerializeSummary) { EXPECT_THAT(serialized, testing::HasSubstr(name + "{quantile=\"0.5\"} 0\n")); } +#ifndef _WIN32 +// This test expects a working German locale to test that floating +// point numbers do not use , but . as a delimiter. +// +// On Debian systems they can be generated by "locale-gen de_DE.UTF-8" +TEST_F(TextSerializerTest, shouldSerializeLocaleIndependent) { + std::unique_ptr localeWithCommaDecimalSeparator; + + // ignore missing locale and skip test if setup fails + try { + localeWithCommaDecimalSeparator = + detail::make_unique("de_DE.UTF-8"); + } catch (std::runtime_error&) { + GTEST_SKIP(); + } + + metric.counter.value = 0.5; + EXPECT_THAT(Serialize(MetricType::Counter), testing::HasSubstr(" 0.5\n")); +} +#endif + } // namespace } // namespace prometheus diff --git a/pull/src/handler.cc b/pull/src/handler.cc index 784fd5e7..61cee2e6 100644 --- a/pull/src/handler.cc +++ b/pull/src/handler.cc @@ -1,9 +1,11 @@ #include "handler.h" #include +#include #include #include #include +#include #include #ifdef HAVE_ZLIB @@ -14,6 +16,7 @@ #include "civetweb.h" #include "metrics_collector.h" #include "prometheus/counter.h" +#include "prometheus/iovector.h" #include "prometheus/metric_family.h" #include "prometheus/summary.h" #include "prometheus/text_serializer.h" @@ -57,7 +60,7 @@ static bool IsEncodingAccepted(struct mg_connection* conn, return std::strstr(accept_encoding, encoding) != nullptr; } -static std::vector GZipCompress(const std::string& input) { +static IOVector GZipCompress(const IOVector& input) { auto zs = z_stream{}; auto windowSize = 16 + MAX_WBITS; auto memoryLevel = 9; @@ -67,24 +70,46 @@ static std::vector GZipCompress(const std::string& input) { return {}; } - zs.next_in = (Bytef*)input.data(); - zs.avail_in = input.size(); + auto s = input.size(); int ret; - std::vector output; - output.reserve(input.size() / 2u); - do { - static const auto outputBytesPerRound = std::size_t{32768}; + IOVector output; - zs.avail_out = outputBytesPerRound; - output.resize(zs.total_out + zs.avail_out); - zs.next_out = reinterpret_cast(output.data() + zs.total_out); + for (std::size_t i = 0; i < input.data.size(); ++i) { + bool last = i == input.data.size() - 1U; + auto chunk = input.data[i]; - ret = deflate(&zs, Z_FINISH); + zs.next_in = + const_cast(reinterpret_cast(chunk.data())); + zs.avail_in = chunk.size(); - output.resize(zs.total_out); - } while (ret == Z_OK); + do { + static constexpr std::size_t maximumChunkSize = 1 * 1024 * 1024; + if (output.data.empty() || + output.data.back().size() >= maximumChunkSize) { + output.data.emplace_back(); + output.data.back().reserve(maximumChunkSize); + } + + auto&& chunk = output.data.back(); + + const auto previouslyUsed = chunk.size(); + const auto remainingChunkSize = maximumChunkSize - previouslyUsed; + + zs.avail_out = remainingChunkSize; + chunk.resize(chunk.size() + remainingChunkSize); + zs.next_out = reinterpret_cast(chunk.data() + previouslyUsed); + + ret = deflate(&zs, last ? Z_FINISH : Z_NO_FLUSH); + assert(ret != Z_STREAM_ERROR); + + chunk.resize(maximumChunkSize - zs.avail_out); + } while (zs.avail_out == 0U); + assert(zs.avail_in == 0); + } + assert(ret == Z_STREAM_END); + assert(zs.total_out == output.size()); deflateEnd(&zs); @@ -97,7 +122,7 @@ static std::vector GZipCompress(const std::string& input) { #endif static std::size_t WriteResponse(struct mg_connection* conn, - const std::string& body) { + const IOVector& body) { mg_printf(conn, "HTTP/1.1 200 OK\r\n" "Content-Type: text/plain; charset=utf-8\r\n"); @@ -108,20 +133,27 @@ static std::size_t WriteResponse(struct mg_connection* conn, if (acceptsGzip) { auto compressed = GZipCompress(body); if (!compressed.empty()) { + const std::size_t contentSize = compressed.size(); mg_printf(conn, "Content-Encoding: gzip\r\n" - "Content-Length: %lu\r\n\r\n", - static_cast(compressed.size())); - mg_write(conn, compressed.data(), compressed.size()); - return compressed.size(); + "Content-Length: %s\r\n\r\n", + std::to_string(contentSize).c_str()); + for (auto&& chunk : compressed.data) { + mg_write(conn, chunk.data(), chunk.size()); + } + return contentSize; } } #endif - mg_printf(conn, "Content-Length: %lu\r\n\r\n", - static_cast(body.size())); - mg_write(conn, body.data(), body.size()); - return body.size(); + std::size_t contentSize = body.size(); + + mg_printf(conn, "Content-Length: %s\r\n\r\n", + std::to_string(contentSize).c_str()); + for (auto&& chunk : body.data) { + mg_write(conn, chunk.data(), chunk.size()); + } + return contentSize; } void MetricsHandler::RegisterCollectable( @@ -148,16 +180,15 @@ void MetricsHandler::RemoveCollectable( bool MetricsHandler::handleGet(CivetServer*, struct mg_connection* conn) { auto start_time_of_request = std::chrono::steady_clock::now(); - std::vector metrics; + IOVector ioVector; + const auto serializer = TextSerializer{ioVector}; { std::lock_guard lock{collectables_mutex_}; - metrics = CollectMetrics(collectables_); + CollectMetrics(serializer, collectables_); } - const TextSerializer serializer; - - auto bodySize = WriteResponse(conn, serializer.Serialize(metrics)); + auto bodySize = WriteResponse(conn, ioVector); auto stop_time_of_request = std::chrono::steady_clock::now(); auto duration = std::chrono::duration_cast( diff --git a/pull/src/metrics_collector.cc b/pull/src/metrics_collector.cc index 0372d693..1e8855ee 100644 --- a/pull/src/metrics_collector.cc +++ b/pull/src/metrics_collector.cc @@ -7,7 +7,8 @@ namespace prometheus { namespace detail { -std::vector CollectMetrics( +void CollectMetrics( + const Serializer& out, const std::vector>& collectables) { auto collected_metrics = std::vector{}; @@ -17,13 +18,8 @@ std::vector CollectMetrics( continue; } - auto&& metrics = collectable->Collect(); - collected_metrics.insert(collected_metrics.end(), - std::make_move_iterator(metrics.begin()), - std::make_move_iterator(metrics.end())); + collectable->Collect(out); } - - return collected_metrics; } } // namespace detail diff --git a/pull/src/metrics_collector.h b/pull/src/metrics_collector.h index 4c3bb614..635863b1 100644 --- a/pull/src/metrics_collector.h +++ b/pull/src/metrics_collector.h @@ -4,11 +4,13 @@ #include #include "prometheus/metric_family.h" +#include "prometheus/serializer.h" namespace prometheus { class Collectable; namespace detail { -std::vector CollectMetrics( +void CollectMetrics( + const Serializer& out, const std::vector>& collectables); } // namespace detail } // namespace prometheus diff --git a/push/src/detail/curl_wrapper.cc b/push/src/detail/curl_wrapper.cc index e43ebb70..d3dbead1 100644 --- a/push/src/detail/curl_wrapper.cc +++ b/push/src/detail/curl_wrapper.cc @@ -5,6 +5,28 @@ namespace prometheus { namespace detail { +namespace { + +struct CurlReadData { + CurlReadData(const IOVector& data) : data{data}, size{data.size()} {} + + const IOVector& data; + const std::size_t size; + std::size_t offset = 0; +}; + +std::size_t ReadCallback(char* buffer, std::size_t size, std::size_t nmemb, + void* userdata) { + auto source = reinterpret_cast(userdata); + + const std::size_t realsize = size * nmemb; + std::size_t copied = source->data.copy(source->offset, buffer, realsize); + source->offset += copied; + + return copied; +} +} // namespace + static const char CONTENT_TYPE[] = "Content-Type: text/plain; version=0.0.4; charset=utf-8"; @@ -39,17 +61,21 @@ CurlWrapper::~CurlWrapper() { } int CurlWrapper::performHttpRequest(HttpMethod method, const std::string& uri, - const std::string& body, long timeout) { + const IOVector& body, long timeout) { std::lock_guard l(mutex_); + const CurlReadData readData{body}; + curl_easy_reset(curl_); curl_easy_setopt(curl_, CURLOPT_URL, uri.c_str()); curl_easy_setopt(curl_, CURLOPT_HTTPHEADER, optHttpHeader_); if (!body.empty()) { - curl_easy_setopt(curl_, CURLOPT_POSTFIELDSIZE, body.size()); - curl_easy_setopt(curl_, CURLOPT_POSTFIELDS, body.data()); + curl_easy_setopt(curl_, CURLOPT_POSTFIELDSIZE_LARGE, body.size()); + curl_easy_setopt(curl_, CURLOPT_POSTFIELDS, nullptr); + curl_easy_setopt(curl_, CURLOPT_READDATA, &readData); + curl_easy_setopt(curl_, CURLOPT_READFUNCTION, ReadCallback); } else { curl_easy_setopt(curl_, CURLOPT_POSTFIELDSIZE, 0L); } diff --git a/push/src/detail/curl_wrapper.h b/push/src/detail/curl_wrapper.h index 33e23f5b..253a7d08 100644 --- a/push/src/detail/curl_wrapper.h +++ b/push/src/detail/curl_wrapper.h @@ -4,6 +4,7 @@ #include #include "prometheus/detail/http_method.h" +#include "prometheus/iovector.h" namespace prometheus { namespace detail { @@ -20,7 +21,7 @@ class CurlWrapper { ~CurlWrapper(); int performHttpRequest(HttpMethod method, const std::string& uri, - const std::string& body, long timeout = 0L); + const IOVector& body, long timeout = 0L); bool addHttpHeader(const std::string& header); private: diff --git a/push/src/gateway.cc b/push/src/gateway.cc index 6b3bc683..cdcf1066 100644 --- a/push/src/gateway.cc +++ b/push/src/gateway.cc @@ -11,6 +11,7 @@ #include "detail/curl_wrapper.h" #include "detail/label_encoder.h" #include "prometheus/detail/future_std.h" +#include "prometheus/iovector.h" #include "prometheus/metric_family.h" // IWYU pragma: keep #include "prometheus/text_serializer.h" @@ -74,8 +75,6 @@ int Gateway::Push() { return push(detail::HttpMethod::Post); } int Gateway::PushAdd() { return push(detail::HttpMethod::Put); } int Gateway::push(detail::HttpMethod method) { - const auto serializer = TextSerializer{}; - std::lock_guard lock{mutex_}; for (auto& wcollectable : collectables_) { auto collectable = wcollectable.first.lock(); @@ -83,8 +82,10 @@ int Gateway::push(detail::HttpMethod method) { continue; } - auto metrics = collectable->Collect(); - auto body = serializer.Serialize(metrics); + IOVector body; + const auto serializer = TextSerializer{body}; + + collectable->Collect(serializer); auto uri = getUri(wcollectable); auto status_code = curlWrapper_->performHttpRequest(method, uri, body, timeout_.count()); @@ -106,22 +107,23 @@ std::future Gateway::AsyncPushAdd() { } std::future Gateway::async_push(detail::HttpMethod method) { - const auto serializer = TextSerializer{}; std::vector> futures; std::lock_guard lock{mutex_}; for (auto& wcollectable : collectables_) { + IOVector body; + const auto serializer = TextSerializer{body}; + auto collectable = wcollectable.first.lock(); if (!collectable) { continue; } - auto metrics = collectable->Collect(); - auto body = std::make_shared(serializer.Serialize(metrics)); + collectable->Collect(serializer); auto uri = getUri(wcollectable); futures.push_back(std::async(std::launch::async, [method, uri, body, this] { - return curlWrapper_->performHttpRequest(method, uri, *body, + return curlWrapper_->performHttpRequest(method, uri, body, timeout_.count()); })); }