Skip to content

Commit c97cd05

Browse files
authored
xds: deprecate flag report_load_with_rq_issued and remove legacy code paths (#41614)
## Description This PR removes the deprecated reloadable flag `report_load_with_rq_issued` and cleans up the legacy paths. Fix #41509 --- **Commit Message:** router: deprecate flag report_load_with_rq_issued and remove legacy code paths **Additional Description:** Remove the deprecated reloadable flag `report_load_with_rq_issued` and cleans up the legacy paths. **Risk Level:** Low **Testing:** CI **Docs Changes:** N/A **Release Notes:** Added Signed-off-by: Rohit Agrawal <[email protected]>
1 parent 25aedb7 commit c97cd05

File tree

4 files changed

+10
-36
lines changed

4 files changed

+10
-36
lines changed

changelogs/current.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,9 @@ removed_config_or_runtime:
6060
- area: http
6161
change: |
6262
Removed runtime guard ``envoy.reloadable_features.original_src_fix_port_exhaustion`` and legacy code paths.
63+
- area: xds
64+
change: |
65+
Removed runtime guard ``envoy.reloadable_features.report_load_with_rq_issued`` and legacy code paths.
6366
6467
new_features:
6568
- area: http

source/common/runtime/runtime_features.cc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@ RUNTIME_GUARD(envoy_reloadable_features_quic_signal_headers_only_to_http1_backen
6666
RUNTIME_GUARD(envoy_reloadable_features_quic_upstream_reads_fixed_number_packets);
6767
RUNTIME_GUARD(envoy_reloadable_features_quic_upstream_socket_use_address_cache_for_read);
6868
RUNTIME_GUARD(envoy_reloadable_features_reject_empty_trusted_ca_file);
69-
RUNTIME_GUARD(envoy_reloadable_features_report_load_with_rq_issued);
7069
RUNTIME_GUARD(envoy_reloadable_features_router_filter_resetall_on_local_reply);
7170
RUNTIME_GUARD(envoy_reloadable_features_safe_http2_options);
7271
RUNTIME_GUARD(envoy_reloadable_features_skip_dns_lookup_for_proxied_requests);

source/common/upstream/load_stats_reporter.cc

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -145,11 +145,7 @@ void LoadStatsReporter::sendLoadStatsRequest() {
145145
}
146146
}
147147

148-
bool should_send_locality_stats = rq_success + rq_error + rq_active != 0;
149-
if (Runtime::runtimeFeatureEnabled(
150-
"envoy.reloadable_features.report_load_with_rq_issued")) {
151-
should_send_locality_stats = rq_issued != 0;
152-
}
148+
bool should_send_locality_stats = rq_issued != 0;
153149
if (should_send_locality_stats) {
154150
locality_stats.set_total_successful_requests(rq_success);
155151
locality_stats.set_total_error_requests(rq_error);

test/common/upstream/load_stats_reporter_test.cc

Lines changed: 6 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -271,9 +271,7 @@ HostSharedPtr makeTestHost(const std::string& hostname,
271271
}
272272

273273
void addStats(const HostSharedPtr& host, double a, double b = 0, double c = 0, double d = 0) {
274-
if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.report_load_with_rq_issued")) {
275-
host->stats().rq_total_.inc();
276-
}
274+
host->stats().rq_total_.inc();
277275
host->stats().rq_success_.inc();
278276
host->loadMetricStats().add("metric_a", a);
279277
if (b != 0) {
@@ -426,22 +424,8 @@ TEST_F(LoadStatsReporterTest, EndpointLevelLoadStatsReportingNoUpdate) {
426424
response_timer_cb_();
427425
}
428426

429-
class LoadStatsReporterTestWithRqTotal : public LoadStatsReporterTest,
430-
public testing::WithParamInterface<bool> {
431-
public:
432-
LoadStatsReporterTestWithRqTotal() {
433-
scoped_runtime_.mergeValues(
434-
{{"envoy.reloadable_features.report_load_with_rq_issued", GetParam() ? "true" : "false"}});
435-
}
436-
TestScopedRuntime scoped_runtime_;
437-
};
438-
439-
INSTANTIATE_TEST_SUITE_P(LoadStatsReporterTestWithRqTotal, LoadStatsReporterTestWithRqTotal,
440-
::testing::Bool());
441-
442427
// Validate that per-locality metrics are aggregated across hosts and included in the load report.
443-
TEST_P(LoadStatsReporterTestWithRqTotal, UpstreamLocalityStats) {
444-
bool expects_rq_total = GetParam();
428+
TEST_F(LoadStatsReporterTest, UpstreamLocalityStats) {
445429
EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_));
446430
expectSendMessage({});
447431
createLoadStatsReporter();
@@ -479,19 +463,15 @@ TEST_P(LoadStatsReporterTestWithRqTotal, UpstreamLocalityStats) {
479463
auto expected_locality0_stats = expected_cluster_stats.add_upstream_locality_stats();
480464
expected_locality0_stats->mutable_locality()->set_region("mars");
481465
expected_locality0_stats->set_total_successful_requests(3);
482-
if (expects_rq_total) {
483-
expected_locality0_stats->set_total_issued_requests(3);
484-
}
466+
expected_locality0_stats->set_total_issued_requests(3);
485467
addStatExpectation(expected_locality0_stats, "metric_a", 3, 0.88888);
486468
addStatExpectation(expected_locality0_stats, "metric_b", 2, 1.12345);
487469
addStatExpectation(expected_locality0_stats, "metric_c", 1, 3.14159);
488470

489471
auto expected_locality1_stats = expected_cluster_stats.add_upstream_locality_stats();
490472
expected_locality1_stats->mutable_locality()->set_region("jupiter");
491473
expected_locality1_stats->set_total_successful_requests(1);
492-
if (expects_rq_total) {
493-
expected_locality1_stats->set_total_issued_requests(1);
494-
}
474+
expected_locality1_stats->set_total_issued_requests(1);
495475
addStatExpectation(expected_locality1_stats, "metric_a", 1, 10.01);
496476
addStatExpectation(expected_locality1_stats, "metric_c", 1, 20.02);
497477
addStatExpectation(expected_locality1_stats, "metric_d", 1, 30.03);
@@ -504,9 +484,7 @@ TEST_P(LoadStatsReporterTestWithRqTotal, UpstreamLocalityStats) {
504484
// Traffic between previous request and next response. Previous latched metrics are cleared.
505485
host1->stats().rq_success_.inc();
506486

507-
if (expects_rq_total) {
508-
host1->stats().rq_total_.inc();
509-
}
487+
host1->stats().rq_total_.inc();
510488
host1->loadMetricStats().add("metric_a", 1.41421);
511489
host1->loadMetricStats().add("metric_e", 2.71828);
512490

@@ -524,9 +502,7 @@ TEST_P(LoadStatsReporterTestWithRqTotal, UpstreamLocalityStats) {
524502
auto expected_locality0_stats = expected_cluster_stats.add_upstream_locality_stats();
525503
expected_locality0_stats->mutable_locality()->set_region("mars");
526504
expected_locality0_stats->set_total_successful_requests(1);
527-
if (expects_rq_total) {
528-
expected_locality0_stats->set_total_issued_requests(1);
529-
}
505+
expected_locality0_stats->set_total_issued_requests(1);
530506
addStatExpectation(expected_locality0_stats, "metric_a", 1, 1.41421);
531507
addStatExpectation(expected_locality0_stats, "metric_e", 1, 2.71828);
532508

0 commit comments

Comments
 (0)