From 8326c67b9c39a8ed6cf7e5216ac0451e40ea02ca Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Sun, 31 Aug 2025 22:55:01 -0400 Subject: [PATCH] Deterministic render ordering --- src/metric.cpp | 84 ++++++++++++++++++++++++++----------- src/metric.hpp | 5 +++ tests/test_metric.cpp | 97 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 162 insertions(+), 24 deletions(-) diff --git a/src/metric.cpp b/src/metric.cpp index dc8be70..2b33d5b 100644 --- a/src/metric.cpp +++ b/src/metric.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -168,6 +169,21 @@ struct LabelsKey { } return true; } + + bool operator<(const LabelsKey &other) const { + if (labels.size() != other.labels.size()) { + return labels.size() < other.labels.size(); + } + for (size_t i = 0; i < labels.size(); ++i) { + if (labels[i].first != other.labels[i].first) { + return labels[i].first < other.labels[i].first; + } + if (labels[i].second != other.labels[i].second) { + return labels[i].second < other.labels[i].second; + } + } + return false; // They are equal + } }; } // namespace metric @@ -214,9 +230,8 @@ template <> struct Family::State { global_accumulated_values; // Callback-based metrics (global, not per-thread) - std::unordered_map< - LabelsKey, MetricCallback, std::hash, - std::equal_to, + std::map< + LabelsKey, MetricCallback, std::less, ArenaStlAllocator>>> callbacks; @@ -238,10 +253,8 @@ template <> struct Family::State { instances; // Callback-based metrics - std::unordered_map< - LabelsKey, MetricCallback, std::hash, - std::equal_to, - ArenaStlAllocator>>> + std::map, std::less, + ArenaStlAllocator>>> callbacks; State(ArenaAllocator &arena) @@ -321,9 +334,8 @@ struct Metric { // Function-local statics to avoid static initialization order fiasco static auto &get_counter_families() { - using FamilyMap = std::unordered_map< - std::string_view, Family::State *, std::hash, - std::equal_to, + using FamilyMap = std::map< + std::string_view, Family::State *, std::less, ArenaStlAllocator< std::pair::State *>>>; static FamilyMap *counterFamilies = new FamilyMap( @@ -334,9 +346,8 @@ struct Metric { } static auto &get_gauge_families() { - using FamilyMap = std::unordered_map< - std::string_view, Family::State *, std::hash, - std::equal_to, + using FamilyMap = std::map< + std::string_view, Family::State *, std::less, ArenaStlAllocator< std::pair::State *>>>; static FamilyMap *gaugeFamilies = new FamilyMap( @@ -347,11 +358,11 @@ struct Metric { } static auto &get_histogram_families() { - using FamilyMap = std::unordered_map< - std::string_view, Family::State *, - std::hash, std::equal_to, - ArenaStlAllocator< - std::pair::State *>>>; + using FamilyMap = + std::map::State *, + std::less, + ArenaStlAllocator::State *>>>; static FamilyMap *histogramFamilies = new FamilyMap( ArenaStlAllocator< std::pair::State *>>( @@ -997,9 +1008,8 @@ std::span render(ArenaAllocator &arena) { } // Aggregate all counter values (thread-local + global accumulated) - std::unordered_map, - std::equal_to, - ArenaStlAllocator>> + std::map, + ArenaStlAllocator>> aggregated_values{ ArenaStlAllocator>(&arena)}; @@ -1094,9 +1104,8 @@ std::span render(ArenaAllocator &arena) { AggregatedHistogram(ArenaAllocator &arena) : thresholds(&arena), counts(&arena), sum(0.0), observations(0) {} }; - std::unordered_map< - LabelsKey, AggregatedHistogram *, std::hash, - std::equal_to, + std::map< + LabelsKey, AggregatedHistogram *, std::less, ArenaStlAllocator>> aggregated_histograms{ArenaStlAllocator< std::pair>(&arena)}; @@ -1295,4 +1304,31 @@ void Family::register_callback( std::mutex Metric::mutex; thread_local Metric::ThreadInit Metric::thread_init; +void reset_metrics_for_testing() { + std::lock_guard _{Metric::mutex}; + + // WARNING: This function assumes no metric objects are in use! + // Clear all family maps - this will leak the Family::State objects but + // that's acceptable for testing since they were allocated in the global arena + + // Get references to the maps + auto &counter_families = Metric::get_counter_families(); + auto &gauge_families = Metric::get_gauge_families(); + auto &histogram_families = Metric::get_histogram_families(); + auto &interned_labels = Metric::get_interned_labels(); + + // Clear all family registrations + counter_families.clear(); + gauge_families.clear(); + histogram_families.clear(); + interned_labels.clear(); + + // Reset the global arena - this will invalidate all arena-allocated strings + // but since we're clearing everything, that's OK + Metric::get_global_arena().reset(); + + // Note: Thread-local arenas will be cleaned up by ThreadInit destructors + // when threads exit naturally +} + } // namespace metric diff --git a/src/metric.hpp b/src/metric.hpp index 3811653..b4fb87c 100644 --- a/src/metric.hpp +++ b/src/metric.hpp @@ -218,6 +218,11 @@ bool is_valid_metric_name(std::string_view name); bool is_valid_label_key(std::string_view key); bool is_valid_label_value(std::string_view value); +// Reset all metrics state - WARNING: Only safe for testing! +// This clears all registered families and metrics. Should only be called +// when no metric objects are in use and no concurrent render() calls. +void reset_metrics_for_testing(); + // Note: Histograms do not support callbacks due to their multi-value nature // (buckets + sum + count). Use static histogram metrics only. diff --git a/tests/test_metric.cpp b/tests/test_metric.cpp index 3074a49..0dc805d 100644 --- a/tests/test_metric.cpp +++ b/tests/test_metric.cpp @@ -7,7 +7,9 @@ #include #include #include +#include #include +#include #include #include @@ -657,3 +659,98 @@ TEST_CASE("memory management") { CHECK(final_output.size() > 0); } } + +TEST_CASE("render output deterministic order golden test") { + // Clean slate - reset all metrics before this test + metric::reset_metrics_for_testing(); + + ArenaAllocator arena; + + // Create a comprehensive set of metrics with deliberate ordering + // to test deterministic output + + // Create counters with different family names and labels + auto z_counter_family = + metric::create_counter("z_last_counter", "Last counter alphabetically"); + auto z_counter = + z_counter_family.create({{"method", "POST"}, {"handler", "api"}}); + z_counter.inc(42.0); + + auto a_counter_family = + metric::create_counter("a_first_counter", "First counter alphabetically"); + auto a_counter1 = a_counter_family.create({{"status", "200"}}); + auto a_counter2 = a_counter_family.create( + {{"method", "GET"}}); // Should come before status lexicographically + a_counter1.inc(100.0); + a_counter2.inc(200.0); + + // Create gauges with different orderings + auto m_gauge_family = metric::create_gauge("m_middle_gauge", "Middle gauge"); + auto m_gauge = m_gauge_family.create({{"type", "memory"}}); + m_gauge.set(1024.0); + + auto b_gauge_family = metric::create_gauge("b_second_gauge", "Second gauge"); + auto b_gauge = b_gauge_family.create({{"region", "us-west"}}); + b_gauge.set(256.0); + + // Create histograms + auto x_hist_family = metric::create_histogram("x_histogram", "Test histogram", + {0.1, 0.5, 1.0}); + auto x_hist = x_hist_family.create({{"endpoint", "/api/v1"}}); + x_hist.observe(0.25); + x_hist.observe(0.75); + + // Add some callbacks to test callback ordering + a_counter_family.register_callback({{"callback", "test"}}, + []() { return 123.0; }); + m_gauge_family.register_callback({{"callback", "dynamic"}}, + []() { return 456.0; }); + + // Render the metrics + auto output = metric::render(arena); + + // Concatenate all output into a single string + std::ostringstream oss; + for (const auto &line : output) { + oss << line; + } + std::string actual_output = oss.str(); + + // Define expected golden output - this represents the exact expected + // deterministic order + std::string expected_golden = + "# HELP a_first_counter First counter alphabetically\n" + "# TYPE a_first_counter counter\n" + "a_first_counter{callback=\"test\"} 123\n" + "a_first_counter{method=\"GET\"} 200\n" + "a_first_counter{status=\"200\"} 100\n" + "# HELP z_last_counter Last counter alphabetically\n" + "# TYPE z_last_counter counter\n" + "z_last_counter{handler=\"api\",method=\"POST\"} 42\n" + "# HELP b_second_gauge Second gauge\n" + "# TYPE b_second_gauge gauge\n" + "b_second_gauge{region=\"us-west\"} 256\n" + "# HELP m_middle_gauge Middle gauge\n" + "# TYPE m_middle_gauge gauge\n" + "m_middle_gauge{callback=\"dynamic\"} 456\n" + "m_middle_gauge{type=\"memory\"} 1024\n" + "# HELP x_histogram Test histogram\n" + "# TYPE x_histogram histogram\n" + "x_histogram_bucket{endpoint=\"/api/v1\",le=\"0.1\"} 0\n" + "x_histogram_bucket{endpoint=\"/api/v1\",le=\"0.5\"} 1\n" + "x_histogram_bucket{endpoint=\"/api/v1\",le=\"1.0\"} 2\n" + "x_histogram_bucket{endpoint=\"/api/v1\",le=\"+Inf\"} 2\n" + "x_histogram_sum{endpoint=\"/api/v1\"} 1\n" + "x_histogram_count{endpoint=\"/api/v1\"} 2\n"; + + // Check if output matches golden file + if (actual_output != expected_golden) { + MESSAGE("Render output does not match expected golden output."); + MESSAGE("This indicates the deterministic ordering has changed."); + MESSAGE("Expected output:\n" << expected_golden); + MESSAGE("Actual output:\n" << actual_output); + CHECK(false); // Force test failure + } else { + CHECK(true); // Test passes + } +}