Skip to content

metrics service: make metric predicate overridable #16725

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
snowp merged 5 commits into from
Jun 2, 2021
Merged

metrics service: make metric predicate overridable #16725

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b2be38a
metrics service: make metric predicate configurable
May 30, 2021
cc2e604
format
May 30, 2021
8a39320
add test
May 30, 2021
3a064cd
feedback: use helper to create metrics, add test case for always fals…
May 31, 2021
ff9037b
spelling
May 31, 2021
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
6 changes: 3 additions & 3 deletions source/extensions/stat_sinks/metrics_service/grpc_metrics_service_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,19 @@ MetricsPtr MetricsFlusher::flush(Stats::MetricSnapshot& snapshot) const {
snapshot.snapshotTime().time_since_epoch())
.count();
for (const auto& counter : snapshot.counters()) {
if (counter.counter_.get().used()) {
if (predicate_(counter.counter_.get())) {
flushCounter(*metrics->Add(), counter, snapshot_time_ms);
}
}

for (const auto& gauge : snapshot.gauges()) {
if (gauge.get().used()) {
if (predicate_(gauge)) {
flushGauge(*metrics->Add(), gauge.get(), snapshot_time_ms);
}
}

for (const auto& histogram : snapshot.histograms()) {
if (histogram.get().used()) {
if (predicate_(histogram.get())) {
flushHistogram(*metrics->Add(), *metrics->Add(), histogram.get(), snapshot_time_ms);
}
}
Expand Down
9 changes: 7 additions & 2 deletions source/extensions/stat_sinks/metrics_service/grpc_metrics_service_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,12 @@ using GrpcMetricsStreamerImplPtr = std::unique_ptr<GrpcMetricsStreamerImpl>;

class MetricsFlusher {
public:
MetricsFlusher(bool report_counters_as_deltas, bool emit_labels)
: report_counters_as_deltas_(report_counters_as_deltas), emit_labels_(emit_labels) {}
MetricsFlusher(
bool report_counters_as_deltas, bool emit_labels,
std::function<bool(const Stats::Metric&)> predicate =
[](const auto& metric) { return metric.used(); })
: report_counters_as_deltas_(report_counters_as_deltas), emit_labels_(emit_labels),
predicate_(predicate) {}

MetricsPtr flush(Stats::MetricSnapshot& snapshot) const;

Expand All @@ -105,6 +109,7 @@ class MetricsFlusher {

const bool report_counters_as_deltas_;
const bool emit_labels_;
const std::function<bool(const Stats::Metric&)> predicate_;
};

/**
Expand Down
130 changes: 70 additions & 60 deletions test/extensions/stats_sinks/metrics_service/grpc_metrics_service_impl_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,35 @@ class MockGrpcMetricsStreamer

class MetricsServiceSinkTest : public testing::Test {
public:
MetricsServiceSinkTest() = default;
void addCounterToSnapshot(const std::string& name, uint64_t delta, uint64_t value,
bool used = true) {
counter_storage_.emplace_back(std::make_unique<NiceMock<Stats::MockCounter>>());
counter_storage_.back()->name_ = name;
counter_storage_.back()->value_ = value;
counter_storage_.back()->used_ = used;

snapshot_.counters_.push_back({delta, *counter_storage_.back()});
}
void addGaugeToSnapshot(const std::string& name, uint64_t value, bool used = true) {
gauge_storage_.emplace_back(std::make_unique<NiceMock<Stats::MockGauge>>());
gauge_storage_.back()->name_ = name;
gauge_storage_.back()->value_ = value;
gauge_storage_.back()->used_ = used;

snapshot_.gauges_.push_back(*gauge_storage_.back());
}
void addHistogramToSnapshot(const std::string& name, bool used = true) {
histogram_storage_.emplace_back(std::make_unique<NiceMock<Stats::MockParentHistogram>>());
histogram_storage_.back()->name_ = name;
histogram_storage_.back()->used_ = used;

snapshot_.histograms_.push_back(*histogram_storage_.back());
}

NiceMock<Stats::MockMetricSnapshot> snapshot_;
std::vector<std::unique_ptr<NiceMock<Stats::MockCounter>>> counter_storage_;
std::vector<std::unique_ptr<NiceMock<Stats::MockGauge>>> gauge_storage_;
std::vector<std::unique_ptr<NiceMock<Stats::MockParentHistogram>>> histogram_storage_;
std::shared_ptr<MockGrpcMetricsStreamer> streamer_{new MockGrpcMetricsStreamer(
Grpc::AsyncClientFactoryPtr{new NiceMock<Grpc::MockAsyncClientFactory>()})};
};
Expand All @@ -113,21 +139,9 @@ TEST_F(MetricsServiceSinkTest, CheckSendCall) {
envoy::service::metrics::v3::StreamMetricsResponse>
sink(streamer_, false, false);

auto counter = std::make_shared<NiceMock<Stats::MockCounter>>();
counter->name_ = "test_counter";
counter->latch_ = 1;
counter->used_ = true;
snapshot_.counters_.push_back({1, *counter});

auto gauge = std::make_shared<NiceMock<Stats::MockGauge>>();
gauge->name_ = "test_gauge";
gauge->value_ = 1;
gauge->used_ = true;
snapshot_.gauges_.push_back(*gauge);

auto histogram = std::make_shared<NiceMock<Stats::MockParentHistogram>>();
histogram->name_ = "test_histogram";
histogram->used_ = true;
addCounterToSnapshot("test_counter", 1, 1);
addGaugeToSnapshot("test_gauge", 1);
addHistogramToSnapshot("test_histogram");

EXPECT_CALL(*streamer_, send(_));

Expand All @@ -139,25 +153,16 @@ TEST_F(MetricsServiceSinkTest, CheckStatsCount) {
envoy::service::metrics::v3::StreamMetricsResponse>
sink(streamer_, false, false);

auto counter = std::make_shared<NiceMock<Stats::MockCounter>>();
counter->name_ = "test_counter";
counter->value_ = 100;
counter->used_ = true;
snapshot_.counters_.push_back({1, *counter});

auto gauge = std::make_shared<NiceMock<Stats::MockGauge>>();
gauge->name_ = "test_gauge";
gauge->value_ = 1;
gauge->used_ = true;
snapshot_.gauges_.push_back(*gauge);
addCounterToSnapshot("test_counter", 1, 100);
addGaugeToSnapshot("test_gauge", 1);

EXPECT_CALL(*streamer_, send(_)).WillOnce(Invoke([](MetricsPtr&& metrics) {
EXPECT_EQ(2, metrics->size());
}));
sink.flush(snapshot_);

// Verify only newly added metrics come after endFlush call.
gauge->used_ = false;
gauge_storage_.back()->used_ = false;
EXPECT_CALL(*streamer_, send(_)).WillOnce(Invoke([](MetricsPtr&& metrics) {
EXPECT_EQ(1, metrics->size());
}));
Expand All @@ -170,11 +175,7 @@ TEST_F(MetricsServiceSinkTest, ReportCountersValues) {
envoy::service::metrics::v3::StreamMetricsResponse>
sink(streamer_, false, false);

auto counter = std::make_shared<NiceMock<Stats::MockCounter>>();
counter->name_ = "test_counter";
counter->value_ = 100;
counter->used_ = true;
snapshot_.counters_.push_back({1, *counter});
addCounterToSnapshot("test_counter", 1, 100);

EXPECT_CALL(*streamer_, send(_)).WillOnce(Invoke([](MetricsPtr&& metrics) {
EXPECT_EQ(1, metrics->size());
Expand All @@ -189,11 +190,7 @@ TEST_F(MetricsServiceSinkTest, ReportCountersAsDeltas) {
envoy::service::metrics::v3::StreamMetricsResponse>
sink(streamer_, true, false);

auto counter = std::make_shared<NiceMock<Stats::MockCounter>>();
counter->name_ = "test_counter";
counter->value_ = 100;
counter->used_ = true;
snapshot_.counters_.push_back({1, *counter});
addCounterToSnapshot("test_counter", 1, 100);

EXPECT_CALL(*streamer_, send(_)).WillOnce(Invoke([](MetricsPtr&& metrics) {
EXPECT_EQ(1, metrics->size());
Expand All @@ -204,28 +201,17 @@ TEST_F(MetricsServiceSinkTest, ReportCountersAsDeltas) {

// Test the behavior of tag emission based on the emit_tags_as_label flag.
TEST_F(MetricsServiceSinkTest, ReportMetricsWithTags) {
auto counter = std::make_shared<NiceMock<Stats::MockCounter>>();
counter->name_ = "full-counter-name";
counter->value_ = 100;
counter->used_ = true;
counter->setTagExtractedName("tag-counter-name");
counter->setTags({{"a", "b"}});
snapshot_.counters_.push_back({1, *counter});

auto gauge = std::make_shared<NiceMock<Stats::MockGauge>>();
gauge->name_ = "full-gauge-name";
gauge->value_ = 100;
gauge->used_ = true;
gauge->setTagExtractedName("tag-gauge-name");
gauge->setTags({{"a", "b"}});
snapshot_.gauges_.push_back({*gauge});

auto histogram = std::make_shared<NiceMock<Stats::MockParentHistogram>>();
histogram->name_ = "full-histogram-name";
histogram->used_ = true;
histogram->setTagExtractedName("tag-histogram-name");
histogram->setTags({{"a", "b"}});
snapshot_.histograms_.push_back({*histogram});
addCounterToSnapshot("full-counter-name", 1, 100);
counter_storage_.back()->setTagExtractedName("tag-counter-name");
counter_storage_.back()->setTags({{"a", "b"}});

addGaugeToSnapshot("full-gauge-name", 100);
gauge_storage_.back()->setTagExtractedName("tag-gauge-name");
gauge_storage_.back()->setTags({{"a", "b"}});

addHistogramToSnapshot("full-histogram-name");
histogram_storage_.back()->setTagExtractedName("tag-histogram-name");
histogram_storage_.back()->setTags({{"a", "b"}});

{
// When the emit_tags flag is false, we don't emit the tags and use the full name.
Expand Down Expand Up @@ -282,6 +268,30 @@ TEST_F(MetricsServiceSinkTest, ReportMetricsWithTags) {
sink.flush(snapshot_);
}

TEST_F(MetricsServiceSinkTest, FlushPredicate) {
addCounterToSnapshot("used_counter", 100, 1);
addCounterToSnapshot("unused_counter", 100, 1, false);

// Default predicate only accepts used metrics.
{
MetricsFlusher flusher(true, true);
auto metrics = flusher.flush(snapshot_);
EXPECT_EQ(1, metrics->size());
}

// Using a predicate that accepts all metrics, we'd flush both metrics.
{
MetricsFlusher flusher(true, true, [](const auto&) { return true; });
auto metrics = flusher.flush(snapshot_);
EXPECT_EQ(2, metrics->size());
}

// Using a predicate that rejects all metrics, we'd flush no metrics.
MetricsFlusher flusher(true, true, [](const auto&) { return false; });
auto metrics = flusher.flush(snapshot_);
EXPECT_EQ(0, metrics->size());
}

} // namespace
} // namespace MetricsService
} // namespace StatSinks
Expand Down