diff --git a/changelogs/current.yaml b/changelogs/current.yaml index b91c0376257e..df7a493d29a0 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -91,5 +91,9 @@ new_features: - area: redis change: | Added support for multi-key commands on transactions. +- area: xds + change: | + Reporting a locality_stats to LRS server when rq_issued > 0, disable by setting runtime guard + ``envoy.reloadable_features.report_load_with_rq_issued`` to ``false``. deprecated: diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 2c32305a22d4..ec4dee996bf6 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -86,6 +86,7 @@ RUNTIME_GUARD(envoy_reloadable_features_quic_send_server_preferred_address_to_al RUNTIME_GUARD(envoy_reloadable_features_quic_support_certificate_compression); RUNTIME_GUARD(envoy_reloadable_features_quic_upstream_reads_fixed_number_packets); RUNTIME_GUARD(envoy_reloadable_features_quic_upstream_socket_use_address_cache_for_read); +RUNTIME_GUARD(envoy_reloadable_features_report_load_with_rq_issued); RUNTIME_GUARD(envoy_reloadable_features_report_stream_reset_error_code); RUNTIME_GUARD(envoy_reloadable_features_sanitize_sni_in_access_log); RUNTIME_GUARD(envoy_reloadable_features_shadow_policy_inherit_trace_sampling); diff --git a/source/common/upstream/load_stats_reporter.cc b/source/common/upstream/load_stats_reporter.cc index f25708e75c17..1a3251f7d9a5 100644 --- a/source/common/upstream/load_stats_reporter.cc +++ b/source/common/upstream/load_stats_reporter.cc @@ -107,7 +107,12 @@ void LoadStatsReporter::sendLoadStatsRequest() { } } } - if (rq_success + rq_error + rq_active != 0) { + bool should_send_locality_stats = rq_success + rq_error + rq_active != 0; + if (Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.report_load_with_rq_issued")) { + should_send_locality_stats = rq_issued != 0; + } + if (should_send_locality_stats) { auto* locality_stats = cluster_stats->add_upstream_locality_stats(); locality_stats->mutable_locality()->MergeFrom(hosts[0]->locality()); locality_stats->set_priority(host_set->priority()); diff --git a/test/common/upstream/BUILD b/test/common/upstream/BUILD index 8f77860fbf06..41ad4639f3c6 100644 --- a/test/common/upstream/BUILD +++ b/test/common/upstream/BUILD @@ -327,6 +327,7 @@ envoy_cc_test( "//test/mocks/upstream:cluster_manager_mocks", "//test/mocks/upstream:cluster_priority_set_mocks", "//test/test_common:simulated_time_system_lib", + "//test/test_common:test_runtime_lib", "//test/test_common:utility_lib", "@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto", "@envoy_api//envoy/service/load_stats/v3:pkg_cc_proto", diff --git a/test/common/upstream/load_stats_reporter_test.cc b/test/common/upstream/load_stats_reporter_test.cc index 0313ece0632e..0c6f621096b6 100644 --- a/test/common/upstream/load_stats_reporter_test.cc +++ b/test/common/upstream/load_stats_reporter_test.cc @@ -12,6 +12,7 @@ #include "test/mocks/upstream/cluster_manager.h" #include "test/mocks/upstream/cluster_priority_set.h" #include "test/test_common/simulated_time_system.h" +#include "test/test_common/test_runtime.h" #include "test/test_common/utility.h" #include "gmock/gmock.h" @@ -252,6 +253,9 @@ HostSharedPtr makeTestHost(const std::string& hostname, } void addStats(const HostSharedPtr& host, double a, double b = 0, double c = 0, double d = 0) { + if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.report_load_with_rq_issued")) { + host->stats().rq_total_.inc(); + } host->stats().rq_success_.inc(); host->loadMetricStats().add("metric_a", a); if (b != 0) { @@ -274,8 +278,22 @@ void addStatExpectation(envoy::config::endpoint::v3::UpstreamLocalityStats* stat metric->set_total_metric_value(total_metric_value); } +class LoadStatsReporterTestWithRqTotal : public LoadStatsReporterTest, + public testing::WithParamInterface { +public: + LoadStatsReporterTestWithRqTotal() { + scoped_runtime_.mergeValues( + {{"envoy.reloadable_features.report_load_with_rq_issued", GetParam() ? "true" : "false"}}); + } + TestScopedRuntime scoped_runtime_; +}; + +INSTANTIATE_TEST_SUITE_P(LoadStatsReporterTestWithRqTotal, LoadStatsReporterTestWithRqTotal, + ::testing::Bool()); + // Validate that per-locality metrics are aggregated across hosts and included in the load report. -TEST_F(LoadStatsReporterTest, UpstreamLocalityStats) { +TEST_P(LoadStatsReporterTestWithRqTotal, UpstreamLocalityStats) { + bool expects_rq_total = GetParam(); EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_)); expectSendMessage({}); createLoadStatsReporter(); @@ -313,6 +331,9 @@ TEST_F(LoadStatsReporterTest, UpstreamLocalityStats) { auto expected_locality0_stats = expected_cluster_stats.add_upstream_locality_stats(); expected_locality0_stats->mutable_locality()->set_region("mars"); expected_locality0_stats->set_total_successful_requests(3); + if (expects_rq_total) { + expected_locality0_stats->set_total_issued_requests(3); + } addStatExpectation(expected_locality0_stats, "metric_a", 3, 0.88888); addStatExpectation(expected_locality0_stats, "metric_b", 2, 1.12345); addStatExpectation(expected_locality0_stats, "metric_c", 1, 3.14159); @@ -320,6 +341,9 @@ TEST_F(LoadStatsReporterTest, UpstreamLocalityStats) { auto expected_locality1_stats = expected_cluster_stats.add_upstream_locality_stats(); expected_locality1_stats->mutable_locality()->set_region("jupiter"); expected_locality1_stats->set_total_successful_requests(1); + if (expects_rq_total) { + expected_locality1_stats->set_total_issued_requests(1); + } addStatExpectation(expected_locality1_stats, "metric_a", 1, 10.01); addStatExpectation(expected_locality1_stats, "metric_c", 1, 20.02); addStatExpectation(expected_locality1_stats, "metric_d", 1, 30.03); @@ -331,6 +355,10 @@ TEST_F(LoadStatsReporterTest, UpstreamLocalityStats) { // Traffic between previous request and next response. Previous latched metrics are cleared. host1->stats().rq_success_.inc(); + + if (expects_rq_total) { + host1->stats().rq_total_.inc(); + } host1->loadMetricStats().add("metric_a", 1.41421); host1->loadMetricStats().add("metric_e", 2.71828); @@ -348,6 +376,9 @@ TEST_F(LoadStatsReporterTest, UpstreamLocalityStats) { auto expected_locality0_stats = expected_cluster_stats.add_upstream_locality_stats(); expected_locality0_stats->mutable_locality()->set_region("mars"); expected_locality0_stats->set_total_successful_requests(1); + if (expects_rq_total) { + expected_locality0_stats->set_total_issued_requests(1); + } addStatExpectation(expected_locality0_stats, "metric_a", 1, 1.41421); addStatExpectation(expected_locality0_stats, "metric_e", 1, 2.71828);