Skip to content

stats: reproduce redundant TYPE tags problem in Prometheus. #27239

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jmarantz merged 2 commits into from
May 8, 2023
Merged

stats: reproduce redundant TYPE tags problem in Prometheus. #27239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cdd99c3
stats: reproduce redundant TYPE tags problem in Prometheus.
jmarantz May 7, 2023
6557b19
add commented out code variations to test streaming implementation.
jmarantz May 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions test/server/admin/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ envoy_cc_test(
name = "prometheus_stats_test",
srcs = envoy_select_admin_functionality(["prometheus_stats_test.cc"]),
deps = [
"//source/common/stats:tag_producer_lib",
"//source/common/stats:thread_local_store_lib",
"//source/server/admin:prometheus_stats_lib",
"//test/test_common:utility_lib",
],
Expand Down
47 changes: 47 additions & 0 deletions test/server/admin/prometheus_stats_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#include <vector>

#include "source/common/stats/custom_stat_namespaces_impl.h"
#include "source/common/stats/tag_producer_impl.h"
#include "source/common/stats/thread_local_store.h"
#include "source/server/admin/prometheus_stats.h"

#include "test/mocks/stats/mocks.h"
Expand Down Expand Up @@ -261,6 +263,51 @@ envoy_histogram1_count{} 0
EXPECT_EQ(expected_output, response.toString());
}

// Replicate bug https://github.com/envoyproxy/envoy/issues/27173 which fails to
// coalesce stats in different scopes with the same tag-extracted-name.
TEST_F(PrometheusStatsFormatterTest, DifferentNamedScopeSameStat) {
Stats::CustomStatNamespacesImpl custom_namespaces;
Stats::ThreadLocalStoreImpl store(alloc_);
envoy::config::metrics::v3::StatsConfig stats_config;
store.setTagProducer(std::make_unique<Stats::TagProducerImpl>(stats_config));
Stats::StatName name = pool_.add("default.total_match_count");

Stats::ScopeSharedPtr scope1 = store.rootScope()->createScope("cluster.a");
counters_.push_back(Stats::CounterSharedPtr(&scope1->counterFromStatName(name)));

// To reproduce the problem from we will render
// cluster.a.default.total_match_count before we discover the existence of
// cluster.x.default.total_match_count. That will happen because "d" in
// "default" comes before "x" with
// https://github.com/envoyproxy/envoy/pull/24998
Stats::ScopeSharedPtr scope2 = store.rootScope()->createScope("cluster.x");
counters_.push_back(Stats::CounterSharedPtr(&scope2->counterFromStatName(name)));

constexpr absl::string_view expected_output =
R"EOF(# TYPE envoy_cluster_default_total_match_count counter
envoy_cluster_default_total_match_count{envoy_cluster_name="a"} 0
envoy_cluster_default_total_match_count{envoy_cluster_name="x"} 0
)EOF";

// Note: in the version of prometheus_stats_test.cc that works with the
// streaming GroupedStatsRequest, the test code is slightly different;
// it's based on the local 'store' object rather than being based on
// the counters_ member variable.
// StatsParams params;
// params.type_ = StatsType::Counters;
// params.format_ = StatsFormat::Prometheus;
// auto request = std::make_unique<GroupedStatsRequest>(store, params, custom_namespaces_);
// EXPECT_EQ(expected_output, response(*request));
// This code is left here so that we can verify the bug is fixed if we decide to
// re-try the streaming Prometheus implementation.

Buffer::OwnedImpl response;
const uint64_t size = PrometheusStatsFormatter::statsAsPrometheus(
counters_, gauges_, histograms_, textReadouts_, response, StatsParams(), custom_namespaces);
EXPECT_EQ(1, size);
EXPECT_EQ(expected_output, response.toString());
}

TEST_F(PrometheusStatsFormatterTest, HistogramWithNonDefaultBuckets) {
Stats::CustomStatNamespacesImpl custom_namespaces;
HistogramWrapper h1_cumulative;
Expand Down