diff --git a/api/envoy/config/metrics/v3/metrics_service.proto b/api/envoy/config/metrics/v3/metrics_service.proto index ad9879055ba3c..0e078c0916f8a 100644 --- a/api/envoy/config/metrics/v3/metrics_service.proto +++ b/api/envoy/config/metrics/v3/metrics_service.proto @@ -4,6 +4,8 @@ package envoy.config.metrics.v3; import "envoy/config/core/v3/grpc_service.proto"; +import "google/protobuf/wrappers.proto"; + import "udpa/annotations/status.proto"; import "udpa/annotations/versioning.proto"; import "validate/validate.proto"; @@ -25,4 +27,10 @@ message MetricsServiceConfig { // The upstream gRPC cluster that hosts the metrics service. core.v3.GrpcService grpc_service = 1 [(validate.rules).message = {required: true}]; + + // If true, counters are reported as the delta between flushing intervals. Otherwise, the current + // counter value is reported. Defaults to false. + // Eventually (https://github.com/envoyproxy/envoy/issues/10968) if this value is not set, the + // sink will take updates from the :ref:`MetricsResponse `. + google.protobuf.BoolValue report_counters_as_deltas = 2; } diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 2449c881fa101..cd9551605f4a7 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -7,7 +7,7 @@ Changes * access loggers: added GRPC_STATUS operator on logging format. * access loggers: extened specifier for FilterStateFormatter to output :ref:`unstructured log string `. * dynamic forward proxy: added :ref:`SNI based dynamic forward proxy ` support. -* fault: added support for controlling the percentage of requests that abort, delay and response rate limits faults +* fault: added support for controlling the percentage of requests that abort, delay and response rate limits faults are applied to using :ref:`HTTP headers ` to the HTTP fault filter. * filter: add `upstram_rq_time` stats to the GPRC stats filter. Disabled by default and can be enabled via :ref:`enable_upstream_stats `. @@ -28,6 +28,7 @@ Changes tracing is not forced. * router: allow retries of streaming or incomplete requests. This removes stat `rq_retry_skipped_request_not_complete`. * router: allow retries by default when upstream responds with :ref:`x-envoy-overloaded `. +* stats: added the option to :ref:`report counters as deltas ` to the metrics service stats sink. * tracing: tracing configuration has been made fully dynamic and every HTTP connection manager can now have a separate :ref:`tracing provider `. * upstream: fixed a bug where Envoy would panic when receiving a GRPC SERVICE_UNKNOWN status on the health check. diff --git a/generated_api_shadow/envoy/config/metrics/v3/metrics_service.proto b/generated_api_shadow/envoy/config/metrics/v3/metrics_service.proto index ad9879055ba3c..0e078c0916f8a 100644 --- a/generated_api_shadow/envoy/config/metrics/v3/metrics_service.proto +++ b/generated_api_shadow/envoy/config/metrics/v3/metrics_service.proto @@ -4,6 +4,8 @@ package envoy.config.metrics.v3; import "envoy/config/core/v3/grpc_service.proto"; +import "google/protobuf/wrappers.proto"; + import "udpa/annotations/status.proto"; import "udpa/annotations/versioning.proto"; import "validate/validate.proto"; @@ -25,4 +27,10 @@ message MetricsServiceConfig { // The upstream gRPC cluster that hosts the metrics service. core.v3.GrpcService grpc_service = 1 [(validate.rules).message = {required: true}]; + + // If true, counters are reported as the delta between flushing intervals. Otherwise, the current + // counter value is reported. Defaults to false. + // Eventually (https://github.com/envoyproxy/envoy/issues/10968) if this value is not set, the + // sink will take updates from the :ref:`MetricsResponse `. + google.protobuf.BoolValue report_counters_as_deltas = 2; } diff --git a/source/extensions/stat_sinks/metrics_service/config.cc b/source/extensions/stat_sinks/metrics_service/config.cc index 4f8402e201b0e..69f0860d228b0 100644 --- a/source/extensions/stat_sinks/metrics_service/config.cc +++ b/source/extensions/stat_sinks/metrics_service/config.cc @@ -33,7 +33,9 @@ Stats::SinkPtr MetricsServiceSinkFactory::createStatsSink(const Protobuf::Messag grpc_service, server.stats(), false), server.localInfo()); - return std::make_unique(grpc_metrics_streamer, server.timeSource()); + return std::make_unique( + grpc_metrics_streamer, server.timeSource(), + PROTOBUF_GET_WRAPPED_OR_DEFAULT(sink_config, report_counters_as_deltas, false)); } ProtobufTypes::MessagePtr MetricsServiceSinkFactory::createEmptyConfigProto() { diff --git a/source/extensions/stat_sinks/metrics_service/grpc_metrics_service_impl.cc b/source/extensions/stat_sinks/metrics_service/grpc_metrics_service_impl.cc index cfea996f40d78..85f2a63b7fb3d 100644 --- a/source/extensions/stat_sinks/metrics_service/grpc_metrics_service_impl.cc +++ b/source/extensions/stat_sinks/metrics_service/grpc_metrics_service_impl.cc @@ -34,19 +34,26 @@ void GrpcMetricsStreamerImpl::send(envoy::service::metrics::v3::StreamMetricsMes } MetricsServiceSink::MetricsServiceSink(const GrpcMetricsStreamerSharedPtr& grpc_metrics_streamer, - TimeSource& time_source) - : grpc_metrics_streamer_(grpc_metrics_streamer), time_source_(time_source) {} + TimeSource& time_source, + const bool report_counters_as_deltas) + : grpc_metrics_streamer_(grpc_metrics_streamer), time_source_(time_source), + report_counters_as_deltas_(report_counters_as_deltas) {} -void MetricsServiceSink::flushCounter(const Stats::Counter& counter) { +void MetricsServiceSink::flushCounter( + const Stats::MetricSnapshot::CounterSnapshot& counter_snapshot) { io::prometheus::client::MetricFamily* metrics_family = message_.add_envoy_metrics(); metrics_family->set_type(io::prometheus::client::MetricType::COUNTER); - metrics_family->set_name(counter.name()); + metrics_family->set_name(counter_snapshot.counter_.get().name()); auto* metric = metrics_family->add_metric(); metric->set_timestamp_ms(std::chrono::duration_cast( time_source_.systemTime().time_since_epoch()) .count()); auto* counter_metric = metric->mutable_counter(); - counter_metric->set_value(counter.value()); + if (report_counters_as_deltas_) { + counter_metric->set_value(counter_snapshot.delta_); + } else { + counter_metric->set_value(counter_snapshot.counter_.get().value()); + } } void MetricsServiceSink::flushGauge(const Stats::Gauge& gauge) { @@ -110,7 +117,7 @@ void MetricsServiceSink::flush(Stats::MetricSnapshot& snapshot) { snapshot.histograms().size()); for (const auto& counter : snapshot.counters()) { if (counter.counter_.get().used()) { - flushCounter(counter.counter_.get()); + flushCounter(counter); } } diff --git a/source/extensions/stat_sinks/metrics_service/grpc_metrics_service_impl.h b/source/extensions/stat_sinks/metrics_service/grpc_metrics_service_impl.h index f8d500a058496..84c2d19695f48 100644 --- a/source/extensions/stat_sinks/metrics_service/grpc_metrics_service_impl.h +++ b/source/extensions/stat_sinks/metrics_service/grpc_metrics_service_impl.h @@ -73,11 +73,11 @@ class MetricsServiceSink : public Stats::Sink { public: // MetricsService::Sink MetricsServiceSink(const GrpcMetricsStreamerSharedPtr& grpc_metrics_streamer, - TimeSource& time_system); + TimeSource& time_system, const bool report_counters_as_deltas); void flush(Stats::MetricSnapshot& snapshot) override; void onHistogramComplete(const Stats::Histogram&, uint64_t) override {} - void flushCounter(const Stats::Counter& counter); + void flushCounter(const Stats::MetricSnapshot::CounterSnapshot& counter_snapshot); void flushGauge(const Stats::Gauge& gauge); void flushHistogram(const Stats::ParentHistogram& envoy_histogram); @@ -85,6 +85,7 @@ class MetricsServiceSink : public Stats::Sink { GrpcMetricsStreamerSharedPtr grpc_metrics_streamer_; envoy::service::metrics::v3::StreamMetricsMessage message_; TimeSource& time_source_; + const bool report_counters_as_deltas_; }; } // namespace MetricsService diff --git a/test/extensions/stats_sinks/metrics_service/grpc_metrics_service_impl_test.cc b/test/extensions/stats_sinks/metrics_service/grpc_metrics_service_impl_test.cc index c7543903d0f90..d0e5325607e98 100644 --- a/test/extensions/stats_sinks/metrics_service/grpc_metrics_service_impl_test.cc +++ b/test/extensions/stats_sinks/metrics_service/grpc_metrics_service_impl_test.cc @@ -90,35 +90,29 @@ class MockGrpcMetricsStreamer : public GrpcMetricsStreamer { MOCK_METHOD(void, send, (envoy::service::metrics::v3::StreamMetricsMessage & message)); }; -class TestGrpcMetricsStreamer : public GrpcMetricsStreamer { +class MetricsServiceSinkTest : public testing::Test { public: - int metric_count; - // GrpcMetricsStreamer - void send(envoy::service::metrics::v3::StreamMetricsMessage& message) override { - metric_count = message.envoy_metrics_size(); - } -}; + MetricsServiceSinkTest() = default; -class MetricsServiceSinkTest : public testing::Test {}; - -TEST(MetricsServiceSinkTest, CheckSendCall) { - NiceMock snapshot; - Event::SimulatedTimeSystem time_system; + NiceMock snapshot_; + Event::SimulatedTimeSystem time_system_; std::shared_ptr streamer_{new MockGrpcMetricsStreamer()}; +}; - MetricsServiceSink sink(streamer_, time_system); +TEST_F(MetricsServiceSinkTest, CheckSendCall) { + MetricsServiceSink sink(streamer_, time_system_, false); auto counter = std::make_shared>(); counter->name_ = "test_counter"; counter->latch_ = 1; counter->used_ = true; - snapshot.counters_.push_back({1, *counter}); + snapshot_.counters_.push_back({1, *counter}); auto gauge = std::make_shared>(); gauge->name_ = "test_gauge"; gauge->value_ = 1; gauge->used_ = true; - snapshot.gauges_.push_back(*gauge); + snapshot_.gauges_.push_back(*gauge); auto histogram = std::make_shared>(); histogram->name_ = "test_histogram"; @@ -126,35 +120,73 @@ TEST(MetricsServiceSinkTest, CheckSendCall) { EXPECT_CALL(*streamer_, send(_)); - sink.flush(snapshot); + sink.flush(snapshot_); } -TEST(MetricsServiceSinkTest, CheckStatsCount) { - NiceMock snapshot; - Event::SimulatedTimeSystem time_system; - std::shared_ptr streamer_{new TestGrpcMetricsStreamer()}; - - MetricsServiceSink sink(streamer_, time_system); +TEST_F(MetricsServiceSinkTest, CheckStatsCount) { + MetricsServiceSink sink(streamer_, time_system_, false); auto counter = std::make_shared>(); counter->name_ = "test_counter"; - counter->latch_ = 1; + counter->value_ = 100; counter->used_ = true; - snapshot.counters_.push_back({1, *counter}); + snapshot_.counters_.push_back({1, *counter}); auto gauge = std::make_shared>(); gauge->name_ = "test_gauge"; gauge->value_ = 1; gauge->used_ = true; - snapshot.gauges_.push_back(*gauge); + snapshot_.gauges_.push_back(*gauge); - sink.flush(snapshot); - EXPECT_EQ(2, (*streamer_).metric_count); + EXPECT_CALL(*streamer_, send(_)) + .WillOnce(Invoke([](envoy::service::metrics::v3::StreamMetricsMessage& message) { + EXPECT_EQ(2, message.envoy_metrics_size()); + })); + sink.flush(snapshot_); // Verify only newly added metrics come after endFlush call. gauge->used_ = false; - sink.flush(snapshot); - EXPECT_EQ(1, (*streamer_).metric_count); + EXPECT_CALL(*streamer_, send(_)) + .WillOnce(Invoke([](envoy::service::metrics::v3::StreamMetricsMessage& message) { + EXPECT_EQ(1, message.envoy_metrics_size()); + })); + sink.flush(snapshot_); +} + +// Test that verifies counters are correctly reported as current value when configured to do so. +TEST_F(MetricsServiceSinkTest, ReportCountersValues) { + MetricsServiceSink sink(streamer_, time_system_, false); + + auto counter = std::make_shared>(); + counter->name_ = "test_counter"; + counter->value_ = 100; + counter->used_ = true; + snapshot_.counters_.push_back({1, *counter}); + + EXPECT_CALL(*streamer_, send(_)) + .WillOnce(Invoke([](envoy::service::metrics::v3::StreamMetricsMessage& message) { + EXPECT_EQ(1, message.envoy_metrics_size()); + EXPECT_EQ(100, message.envoy_metrics(0).metric(0).counter().value()); + })); + sink.flush(snapshot_); +} + +// Test that verifies counters are reported as the delta between flushes when configured to do so. +TEST_F(MetricsServiceSinkTest, ReportCountersAsDeltas) { + MetricsServiceSink sink(streamer_, time_system_, true); + + auto counter = std::make_shared>(); + counter->name_ = "test_counter"; + counter->value_ = 100; + counter->used_ = true; + snapshot_.counters_.push_back({1, *counter}); + + EXPECT_CALL(*streamer_, send(_)) + .WillOnce(Invoke([](envoy::service::metrics::v3::StreamMetricsMessage& message) { + EXPECT_EQ(1, message.envoy_metrics_size()); + EXPECT_EQ(1, message.envoy_metrics(0).metric(0).counter().value()); + })); + sink.flush(snapshot_); } } // namespace