From d63fec09e216b7ff16af5f02158812259ee769f5 Mon Sep 17 00:00:00 2001
From: alyssawilk <alyssar@chromium.org>
Date: Tue, 1 Mar 2022 12:44:58 -0500
Subject: [PATCH 01/14] test: adding a multi-envoy test (#20016)

Functionally this

handles the multi-envoy signal handler crash
skips instantiating a runtime singleton (off by default, must stay off until remove global runtime: rebase runtime features on ABSL_flags #19847 is closed)
Multi-envoy does not correctly support runtime flags or deprecation stats due to #19847 being incomplete. It can still handle proxy traffic client - L1 - L2 - upstream as shown in test.

Risk Level: low
Testing: yes
Docs Changes: n/a
Release Notes: n/a
Part of envoyproxy/envoy-mobile#2003

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>
---
 source/common/quic/active_quic_listener.cc    |  4 +-
 source/common/runtime/runtime_features.cc     |  3 +
 source/common/signal/fatal_error_handler.cc   | 10 ++-
 .../common/singleton/threadsafe_singleton.h   |  2 +
 source/server/config_validation/server.h      |  2 +-
 source/server/server.cc                       | 31 ++++++---
 source/server/server.h                        |  1 +
 test/common/signal/fatal_action_test.cc       |  7 --
 test/integration/BUILD                        | 10 +++
 test/integration/base_integration_test.cc     | 67 ++++++++++++-------
 test/integration/base_integration_test.h      | 15 ++++-
 test/integration/http_integration.cc          |  2 +-
 test/integration/multi_envoy_test.cc          | 65 ++++++++++++++++++
 13 files changed, 163 insertions(+), 56 deletions(-)
 create mode 100644 test/integration/multi_envoy_test.cc

diff --git a/source/common/quic/active_quic_listener.cc b/source/common/quic/active_quic_listener.cc
index 92d7c723ce5d..abc2b968dc01 100644
--- a/source/common/quic/active_quic_listener.cc
+++ b/source/common/quic/active_quic_listener.cc
@@ -59,9 +59,7 @@ ActiveQuicListener::ActiveQuicListener(
   ASSERT(GetQuicReloadableFlag(quic_single_ack_in_packet2));
   ASSERT(!GetQuicFlag(FLAGS_quic_header_size_limit_includes_overhead));
 
-  if (Runtime::LoaderSingleton::getExisting()) {
-    enabled_.emplace(Runtime::FeatureFlag(enabled, runtime));
-  }
+  enabled_.emplace(Runtime::FeatureFlag(enabled, runtime));
 
   quic::QuicRandom* const random = quic::QuicRandom::GetInstance();
   random->RandBytes(random_seed_, sizeof(random_seed_));
diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc
index 7ae10128a120..ab63a8c5a79a 100644
--- a/source/common/runtime/runtime_features.cc
+++ b/source/common/runtime/runtime_features.cc
@@ -63,6 +63,8 @@ FALSE_RUNTIME_GUARD(envoy_reloadable_features_test_feature_false);
 FALSE_RUNTIME_GUARD(envoy_reloadable_features_allow_multiple_dns_addresses);
 // TODO(adisuissa) reset to true to enable unified mux by default
 FALSE_RUNTIME_GUARD(envoy_reloadable_features_unified_mux);
+// TODO(alyssar) flip false once issue complete.
+FALSE_RUNTIME_GUARD(envoy_restart_features_no_runtime_singleton);
 
 // Block of non-boolean flags. These are deprecated. Do not add more.
 ABSL_FLAG(uint64_t, envoy_buffer_overflow_multiplier, 0, "");                        // NOLINT
@@ -169,6 +171,7 @@ constexpr absl::Flag<bool>* runtime_features[] = {
   &FLAGS_envoy_reloadable_features_validate_connect,
   &FLAGS_envoy_restart_features_explicit_wildcard_resource,
   &FLAGS_envoy_restart_features_use_apple_api_for_dns_lookups,
+  &FLAGS_envoy_restart_features_no_runtime_singleton,
 };
 // clang-format on
 
diff --git a/source/common/signal/fatal_error_handler.cc b/source/common/signal/fatal_error_handler.cc
index baf1ea267b57..ab1669c3b18f 100644
--- a/source/common/signal/fatal_error_handler.cc
+++ b/source/common/signal/fatal_error_handler.cc
@@ -153,12 +153,10 @@ void registerFatalActions(FatalAction::FatalActionPtrList safe_actions,
                           FatalAction::FatalActionPtrList unsafe_actions,
                           Thread::ThreadFactory& thread_factory) {
   // Create a FatalActionManager and store it.
-  FatalAction::FatalActionManager* previous_manager =
-      fatal_action_manager.exchange(new FatalAction::FatalActionManager(
-          std::move(safe_actions), std::move(unsafe_actions), thread_factory));
-
-  // Previous manager should be NULL.
-  ASSERT(!previous_manager);
+  if (!fatal_action_manager) {
+    fatal_action_manager.exchange(new FatalAction::FatalActionManager(
+        std::move(safe_actions), std::move(unsafe_actions), thread_factory));
+  }
 }
 
 FatalAction::Status runSafeActions() { return runFatalActions(FatalActionType::Safe); }
diff --git a/source/common/singleton/threadsafe_singleton.h b/source/common/singleton/threadsafe_singleton.h
index f88ae9ed161a..97afb282edfc 100644
--- a/source/common/singleton/threadsafe_singleton.h
+++ b/source/common/singleton/threadsafe_singleton.h
@@ -80,6 +80,8 @@ template <class T> class ScopedInjectableLoader {
   }
   ~ScopedInjectableLoader() { InjectableSingleton<T>::clear(); }
 
+  T& instance() { return *instance_; }
+
 private:
   std::unique_ptr<T> instance_;
 };
diff --git a/source/server/config_validation/server.h b/source/server/config_validation/server.h
index 082410352f8f..a8fa7aba5f31 100644
--- a/source/server/config_validation/server.h
+++ b/source/server/config_validation/server.h
@@ -91,7 +91,7 @@ class ValidationInstance final : Logger::Loggable<Logger::Id::main>,
   ServerLifecycleNotifier& lifecycleNotifier() override { return *this; }
   ListenerManager& listenerManager() override { return *listener_manager_; }
   Secret::SecretManager& secretManager() override { return *secret_manager_; }
-  Runtime::Loader& runtime() override { return Runtime::LoaderSingleton::get(); }
+  Runtime::Loader& runtime() override { return runtime_singleton_->instance(); }
   void shutdown() override;
   bool isShutdown() override { return false; }
   void shutdownAdmin() override {}
diff --git a/source/server/server.cc b/source/server/server.cc
index 10c1ffd4c91c..9871b5662e40 100644
--- a/source/server/server.cc
+++ b/source/server/server.cc
@@ -619,8 +619,12 @@ void InstanceImpl::initialize(Network::Address::InstanceConstSharedPtr local_add
 
   // Runtime gets initialized before the main configuration since during main configuration
   // load things may grab a reference to the loader for later use.
-  runtime_singleton_ = std::make_unique<Runtime::ScopedLoaderSingleton>(
-      component_factory.createRuntime(*this, initial_config));
+  Runtime::LoaderPtr runtime_ptr = component_factory.createRuntime(*this, initial_config);
+  if (runtime_ptr->snapshot().getBoolean("envoy.restart_features.no_runtime_singleton", false)) {
+    runtime_ = std::move(runtime_ptr);
+  } else {
+    runtime_singleton_ = std::make_unique<Runtime::ScopedLoaderSingleton>(std::move(runtime_ptr));
+  }
   initial_config.initAdminAccessLog(bootstrap_, *this);
   validation_context_.setRuntime(runtime());
 
@@ -648,10 +652,10 @@ void InstanceImpl::initialize(Network::Address::InstanceConstSharedPtr local_add
       dns_resolver_factory.createDnsResolver(dispatcher(), api(), typed_dns_resolver_config);
 
   cluster_manager_factory_ = std::make_unique<Upstream::ProdClusterManagerFactory>(
-      *admin_, Runtime::LoaderSingleton::get(), stats_store_, thread_local_, dns_resolver_,
-      *ssl_context_manager_, *dispatcher_, *local_info_, *secret_manager_,
-      messageValidationContext(), *api_, http_context_, grpc_context_, router_context_,
-      access_log_manager_, *singleton_manager_, options_, quic_stat_names_);
+      *admin_, runtime(), stats_store_, thread_local_, dns_resolver_, *ssl_context_manager_,
+      *dispatcher_, *local_info_, *secret_manager_, messageValidationContext(), *api_,
+      http_context_, grpc_context_, router_context_, access_log_manager_, *singleton_manager_,
+      options_, quic_stat_names_);
 
   // Now the configuration gets parsed. The configuration may start setting
   // thread local data per above. See MainImpl::initialize() for why ConfigImpl
@@ -675,7 +679,7 @@ void InstanceImpl::initialize(Network::Address::InstanceConstSharedPtr local_add
 
   // We have to defer RTDS initialization until after the cluster manager is
   // instantiated (which in turn relies on runtime...).
-  Runtime::LoaderSingleton::get().initialize(clusterManager());
+  runtime().initialize(clusterManager());
 
   clusterManager().setPrimaryClustersInitializedCb(
       [this]() { onClusterManagerPrimaryInitializationComplete(); });
@@ -706,7 +710,7 @@ void InstanceImpl::initialize(Network::Address::InstanceConstSharedPtr local_add
 
 void InstanceImpl::onClusterManagerPrimaryInitializationComplete() {
   // If RTDS was not configured the `onRuntimeReady` callback is immediately invoked.
-  Runtime::LoaderSingleton::get().startRtdsSubscriptions([this]() { onRuntimeReady(); });
+  runtime().startRtdsSubscriptions([this]() { onRuntimeReady(); });
 }
 
 void InstanceImpl::onRuntimeReady() {
@@ -730,8 +734,8 @@ void InstanceImpl::onRuntimeReady() {
           Config::Utility::factoryForGrpcApiConfigSource(*async_client_manager_, hds_config,
                                                          stats_store_, false)
               ->createUncachedRawAsyncClient(),
-          *dispatcher_, Runtime::LoaderSingleton::get(), stats_store_, *ssl_context_manager_,
-          info_factory_, access_log_manager_, *config_.clusterManager(), *local_info_, *admin_,
+          *dispatcher_, runtime(), stats_store_, *ssl_context_manager_, info_factory_,
+          access_log_manager_, *config_.clusterManager(), *local_info_, *admin_,
           *singleton_manager_, thread_local_, messageValidationContext().dynamicValidationVisitor(),
           *api_, options_);
     }
@@ -931,7 +935,12 @@ void InstanceImpl::terminate() {
   FatalErrorHandler::clearFatalActionsOnTerminate();
 }
 
-Runtime::Loader& InstanceImpl::runtime() { return Runtime::LoaderSingleton::get(); }
+Runtime::Loader& InstanceImpl::runtime() {
+  if (runtime_singleton_) {
+    return runtime_singleton_->instance();
+  }
+  return *runtime_;
+}
 
 void InstanceImpl::shutdown() {
   ENVOY_LOG(info, "shutting down server instance");
diff --git a/source/server/server.h b/source/server/server.h
index 6da357374d97..cef5cba3015b 100644
--- a/source/server/server.h
+++ b/source/server/server.h
@@ -357,6 +357,7 @@ class InstanceImpl final : Logger::Loggable<Logger::Id::main>,
   Singleton::ManagerPtr singleton_manager_;
   Network::ConnectionHandlerPtr handler_;
   std::unique_ptr<Runtime::ScopedLoaderSingleton> runtime_singleton_;
+  std::unique_ptr<Runtime::Loader> runtime_;
   std::unique_ptr<Ssl::ContextManager> ssl_context_manager_;
   ProdListenerComponentFactory listener_component_factory_;
   ProdWorkerFactory worker_factory_;
diff --git a/test/common/signal/fatal_action_test.cc b/test/common/signal/fatal_action_test.cc
index 5565c592eee5..b6b43470c60f 100644
--- a/test/common/signal/fatal_action_test.cc
+++ b/test/common/signal/fatal_action_test.cc
@@ -72,13 +72,6 @@ TEST_F(FatalActionTest, ShouldNotBeAbleToRunActionsBeforeRegistration) {
   EXPECT_EQ(FatalErrorHandler::runUnsafeActions(), Status::ActionManagerUnset);
 }
 
-TEST_F(FatalActionTest, ShouldOnlyBeAbleToRegisterFatalActionsOnce) {
-  // Register empty list of actions
-  FatalErrorHandler::registerFatalActions({}, {}, Thread::threadFactoryForTest());
-  EXPECT_DEBUG_DEATH(
-      { FatalErrorHandler::registerFatalActions({}, {}, Thread::threadFactoryForTest()); }, "");
-}
-
 TEST_F(FatalActionTest, CanCallRegisteredActions) {
   // Set up Fatal Actions
   safe_actions_.emplace_back(std::make_unique<TestFatalAction>(true, &counter_));
diff --git a/test/integration/BUILD b/test/integration/BUILD
index d93a55ba7dfd..27ad677a7867 100644
--- a/test/integration/BUILD
+++ b/test/integration/BUILD
@@ -554,6 +554,16 @@ envoy_cc_test(
     ],
 )
 
+envoy_cc_test(
+    name = "multi_envoy_test",
+    srcs = [
+        "multi_envoy_test.cc",
+    ],
+    deps = [
+        ":http_protocol_integration_lib",
+    ],
+)
+
 envoy_cc_test(
     name = "multiplexed_upstream_integration_test",
     srcs = [
diff --git a/test/integration/base_integration_test.cc b/test/integration/base_integration_test.cc
index 461bb39acab1..9fc56685856e 100644
--- a/test/integration/base_integration_test.cc
+++ b/test/integration/base_integration_test.cc
@@ -157,19 +157,14 @@ void BaseIntegrationTest::createUpstreams() {
   }
 }
 
-void BaseIntegrationTest::createEnvoy() {
-  std::vector<uint32_t> ports;
-  for (auto& upstream : fake_upstreams_) {
-    if (upstream->localAddress()->ip()) {
-      ports.push_back(upstream->localAddress()->ip()->port());
-    }
-  }
-
-  if (use_lds_) {
+std::string BaseIntegrationTest::finalizeConfigWithPorts(ConfigHelper& config_helper,
+                                                         std::vector<uint32_t>& ports,
+                                                         bool use_lds) {
+  if (use_lds) {
     ENVOY_LOG_MISC(debug, "Setting up file-based LDS");
     // Before finalization, set up a real lds path, replacing the default /dev/null
     std::string lds_path = TestEnvironment::temporaryPath(TestUtility::uniqueFilename());
-    config_helper_.addConfigModifier(
+    config_helper.addConfigModifier(
         [lds_path](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void {
           bootstrap.mutable_dynamic_resources()->mutable_lds_config()->set_resource_api_version(
               envoy::config::core::v3::V3);
@@ -183,16 +178,16 @@ void BaseIntegrationTest::createEnvoy() {
   // Note that finalize assumes that every fake_upstream_ must correspond to a bootstrap config
   // static entry. So, if you want to manually create a fake upstream without specifying it in the
   // config, you will need to do so *after* initialize() (which calls this function) is done.
-  config_helper_.finalize(ports);
+  config_helper.finalize(ports);
 
-  envoy::config::bootstrap::v3::Bootstrap bootstrap = config_helper_.bootstrap();
-  if (use_lds_) {
+  envoy::config::bootstrap::v3::Bootstrap bootstrap = config_helper.bootstrap();
+  if (use_lds) {
     // After the config has been finalized, write the final listener config to the lds file.
     const std::string lds_path =
-        config_helper_.bootstrap().dynamic_resources().lds_config().path_config_source().path();
+        config_helper.bootstrap().dynamic_resources().lds_config().path_config_source().path();
     envoy::service::discovery::v3::DiscoveryResponse lds;
     lds.set_version_info("0");
-    for (auto& listener : config_helper_.bootstrap().static_resources().listeners()) {
+    for (auto& listener : config_helper.bootstrap().static_resources().listeners()) {
       ProtobufWkt::Any* resource = lds.add_resources();
       resource->PackFrom(listener);
     }
@@ -208,6 +203,18 @@ void BaseIntegrationTest::createEnvoy() {
 
   const std::string bootstrap_path = TestEnvironment::writeStringToFileForTest(
       "bootstrap.pb", TestUtility::getProtobufBinaryStringFromMessage(bootstrap));
+  return bootstrap_path;
+}
+
+void BaseIntegrationTest::createEnvoy() {
+  std::vector<uint32_t> ports;
+  for (auto& upstream : fake_upstreams_) {
+    if (upstream->localAddress()->ip()) {
+      ports.push_back(upstream->localAddress()->ip()->port());
+    }
+  }
+
+  const std::string bootstrap_path = finalizeConfigWithPorts(config_helper_, ports, use_lds_);
 
   std::vector<std::string> named_ports;
   const auto& static_resources = config_helper_.bootstrap().static_resources();
@@ -295,12 +302,13 @@ void BaseIntegrationTest::setUpstreamAddress(
   socket_address->set_port_value(fake_upstreams_[upstream_index]->localAddress()->ip()->port());
 }
 
-void BaseIntegrationTest::registerTestServerPorts(const std::vector<std::string>& port_names) {
+void BaseIntegrationTest::registerTestServerPorts(const std::vector<std::string>& port_names,
+                                                  IntegrationTestServerPtr& test_server) {
   bool listeners_ready = false;
   absl::Mutex l;
   std::vector<std::reference_wrapper<Network::ListenerConfig>> listeners;
-  test_server_->server().dispatcher().post([this, &listeners, &listeners_ready, &l]() {
-    listeners = test_server_->server().listenerManager().listeners();
+  test_server->server().dispatcher().post([&listeners, &listeners_ready, &l, &test_server]() {
+    listeners = test_server->server().listenerManager().listeners();
     l.Lock();
     listeners_ready = true;
     l.Unlock();
@@ -318,7 +326,7 @@ void BaseIntegrationTest::registerTestServerPorts(const std::vector<std::string>
     }
   }
   const auto admin_addr =
-      test_server_->server().admin().socket().connectionInfoProvider().localAddress();
+      test_server->server().admin().socket().connectionInfoProvider().localAddress();
   if (admin_addr->type() == Network::Address::Type::Ip) {
     registerPort("admin", admin_addr->ip()->port());
   }
@@ -334,8 +342,15 @@ std::string getListenerDetails(Envoy::Server::Instance& server) {
 void BaseIntegrationTest::createGeneratedApiTestServer(
     const std::string& bootstrap_path, const std::vector<std::string>& port_names,
     Server::FieldValidationConfig validator_config, bool allow_lds_rejection) {
+  createGeneratedApiTestServer(bootstrap_path, port_names, validator_config, allow_lds_rejection,
+                               test_server_);
+}
 
-  test_server_ = IntegrationTestServer::create(
+void BaseIntegrationTest::createGeneratedApiTestServer(
+    const std::string& bootstrap_path, const std::vector<std::string>& port_names,
+    Server::FieldValidationConfig validator_config, bool allow_lds_rejection,
+    IntegrationTestServerPtr& test_server) {
+  test_server = IntegrationTestServer::create(
       bootstrap_path, version_, on_server_ready_function_, on_server_init_function_,
       deterministic_value_, timeSystem(), *api_, defer_listener_finalization_, process_object_,
       validator_config, concurrency_, drain_time_, drain_strategy_, proxy_buffer_factory_,
@@ -350,27 +365,27 @@ void BaseIntegrationTest::createGeneratedApiTestServer(
     Event::TestTimeSystem::RealTimeBound bound(2 * TestUtility::DefaultTimeout);
     const char* success = "listener_manager.listener_create_success";
     const char* rejected = "listener_manager.lds.update_rejected";
-    for (Stats::CounterSharedPtr success_counter = test_server_->counter(success),
-                                 rejected_counter = test_server_->counter(rejected);
+    for (Stats::CounterSharedPtr success_counter = test_server->counter(success),
+                                 rejected_counter = test_server->counter(rejected);
          (success_counter == nullptr ||
           success_counter->value() <
               concurrency_ * config_helper_.bootstrap().static_resources().listeners_size()) &&
          (!allow_lds_rejection || rejected_counter == nullptr || rejected_counter->value() == 0);
-         success_counter = test_server_->counter(success),
-                                 rejected_counter = test_server_->counter(rejected)) {
+         success_counter = test_server->counter(success),
+                                 rejected_counter = test_server->counter(rejected)) {
       if (!bound.withinBound()) {
         RELEASE_ASSERT(0, "Timed out waiting for listeners.");
       }
       if (!allow_lds_rejection) {
         RELEASE_ASSERT(rejected_counter == nullptr || rejected_counter->value() == 0,
                        absl::StrCat("Lds update failed. Details\n",
-                                    getListenerDetails(test_server_->server())));
+                                    getListenerDetails(test_server->server())));
       }
       // TODO(mattklein123): Switch to events and waitFor().
       time_system_.realSleepDoNotUseWithoutScrutiny(std::chrono::milliseconds(10));
     }
 
-    registerTestServerPorts(port_names);
+    registerTestServerPorts(port_names, test_server);
   }
 }
 
diff --git a/test/integration/base_integration_test.h b/test/integration/base_integration_test.h
index ecc5f538b494..2492031f3797 100644
--- a/test/integration/base_integration_test.h
+++ b/test/integration/base_integration_test.h
@@ -94,7 +94,11 @@ class BaseIntegrationTest : protected Logger::Loggable<Logger::Id::testing> {
   makeClientConnectionWithOptions(uint32_t port,
                                   const Network::ConnectionSocket::OptionsSharedPtr& options);
 
-  void registerTestServerPorts(const std::vector<std::string>& port_names);
+  void registerTestServerPorts(const std::vector<std::string>& port_names) {
+    registerTestServerPorts(port_names, test_server_);
+  }
+  void registerTestServerPorts(const std::vector<std::string>& port_names,
+                               IntegrationTestServerPtr& test_server);
   void createGeneratedApiTestServer(const std::string& bootstrap_path,
                                     const std::vector<std::string>& port_names,
                                     Server::FieldValidationConfig validator_config,
@@ -104,6 +108,12 @@ class BaseIntegrationTest : protected Logger::Loggable<Logger::Id::testing> {
                            Server::FieldValidationConfig validator_config,
                            bool allow_lds_rejection);
 
+  void createGeneratedApiTestServer(const std::string& bootstrap_path,
+                                    const std::vector<std::string>& port_names,
+                                    Server::FieldValidationConfig validator_config,
+                                    bool allow_lds_rejection,
+                                    IntegrationTestServerPtr& test_server);
+
   Event::TestTimeSystem& timeSystem() { return time_system_; }
 
   Stats::IsolatedStoreImpl stats_store_;
@@ -333,6 +343,9 @@ class BaseIntegrationTest : protected Logger::Loggable<Logger::Id::testing> {
   }
 
 protected:
+  static std::string finalizeConfigWithPorts(ConfigHelper& helper, std::vector<uint32_t>& ports,
+                                             bool use_lds);
+
   void setUdpFakeUpstream(absl::optional<FakeUpstreamConfig::UdpConfig> config) {
     upstream_config_.udp_fake_upstream_ = config;
   }
diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc
index fa2aa4d9a523..7c3fc675fd7f 100644
--- a/test/integration/http_integration.cc
+++ b/test/integration/http_integration.cc
@@ -357,7 +357,7 @@ void HttpIntegrationTest::initialize() {
   config_helper_.addQuicDownstreamTransportSocketConfig();
 
   BaseIntegrationTest::initialize();
-  registerTestServerPorts({"http"});
+  registerTestServerPorts({"http"}, test_server_);
 
   // Needs to outlive all QUIC connections.
   auto quic_connection_persistent_info =
diff --git a/test/integration/multi_envoy_test.cc b/test/integration/multi_envoy_test.cc
new file mode 100644
index 000000000000..f36c125d1fee
--- /dev/null
+++ b/test/integration/multi_envoy_test.cc
@@ -0,0 +1,65 @@
+#include "test/integration/http_protocol_integration.h"
+
+namespace Envoy {
+namespace {
+
+class MultiEnvoyTest : public HttpProtocolIntegrationTest {
+public:
+  ~MultiEnvoyTest() override { test_server_.reset(); }
+
+  // Create an envoy in front of the original Envoy.
+  void createL1Envoy();
+
+  IntegrationTestServerPtr l1_server_;
+  Thread::SkipAsserts skip_;
+};
+
+void MultiEnvoyTest::createL1Envoy() {
+  std::vector<uint32_t> ports;
+  std::vector<uint32_t> zero;
+  int l2_port = lookupPort("http");
+  for (auto& upstream : fake_upstreams_) {
+    if (upstream->localAddress()->ip()) {
+      ports.push_back(l2_port);
+      zero.push_back(0);
+    }
+  }
+  ConfigHelper l1_helper(version_, *api_,
+                         MessageUtil::getJsonStringFromMessageOrDie(config_helper_.bootstrap()));
+  l1_helper.setPorts(zero, true); // Zero out ports set by config_helper_'s finalize();
+  const std::string bootstrap_path = finalizeConfigWithPorts(l1_helper, ports, use_lds_);
+
+  std::vector<std::string> named_ports;
+  const auto& static_resources = config_helper_.bootstrap().static_resources();
+  named_ports.reserve(static_resources.listeners_size());
+  for (int i = 0; i < static_resources.listeners_size(); ++i) {
+    named_ports.push_back(static_resources.listeners(i).name() + "_l1");
+  }
+  createGeneratedApiTestServer(bootstrap_path, named_ports, {false, true, false}, false,
+                               l1_server_);
+}
+
+INSTANTIATE_TEST_SUITE_P(IpVersions, MultiEnvoyTest,
+                         testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams(
+                             {Http::CodecType::HTTP1}, {Http::CodecType::HTTP1})),
+                         HttpProtocolIntegrationTest::protocolTestParamsToString);
+
+// A simple test to make sure that traffic can flow between client - L1 - L2 - upstream.
+// This test does not currently support mixed protocol hops, or much of the other envoy test
+// framework knobs.
+TEST_P(MultiEnvoyTest, SimpleRequestAndResponse) {
+  config_helper_.addRuntimeOverride("envoy.restart_features.no_runtime_singleton", "true");
+  initialize();
+  createL1Envoy();
+
+  codec_client_ = makeHttpConnection(lookupPort("http_l1"));
+  auto response =
+      sendRequestAndWaitForResponse(default_request_headers_, 0, default_response_headers_, 0);
+  EXPECT_EQ(1, test_server_->counter("http.config_test.rq_total"));
+  EXPECT_EQ(1, l1_server_->counter("http.config_test.rq_total"));
+
+  l1_server_.reset();
+}
+
+} // namespace
+} // namespace Envoy

From 6a86a264d077ea1f4ea6076f646ba49ce5680213 Mon Sep 17 00:00:00 2001
From: Bin Wu <46450037+wu-bin@users.noreply.github.com>
Date: Tue, 1 Mar 2022 13:08:45 -0500
Subject: [PATCH 02/14] Add a congestionWindowInBytes method to
 Envoy::Network::Connection (#20105)

Signed-off-by: Bin Wu <wub@google.com>
Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>
---
 contrib/vcl/source/vcl_io_handle.cc           |  2 ++
 contrib/vcl/source/vcl_io_handle.h            |  1 +
 envoy/api/os_sys_calls.h                      |  3 +++
 envoy/network/connection.h                    |  8 ++++++
 envoy/network/io_handle.h                     |  8 ++++++
 envoy/network/listen_socket.h                 |  8 ++++++
 source/common/api/posix/os_sys_calls_impl.cc  |  4 +++
 source/common/api/win32/os_sys_calls_impl.cc  |  1 +
 source/common/network/connection_impl.cc      |  4 +++
 source/common/network/connection_impl.h       |  1 +
 .../network/happy_eyeballs_connection_impl.cc |  5 ++++
 .../network/happy_eyeballs_connection_impl.h  |  1 +
 .../common/network/io_socket_handle_impl.cc   |  9 +++++++
 source/common/network/io_socket_handle_impl.h |  1 +
 source/common/network/listen_socket_impl.h    |  4 +++
 .../quic_filter_manager_connection_impl.cc    | 27 +++++++++++++++++++
 .../quic_filter_manager_connection_impl.h     |  4 +--
 source/common/quic/quic_io_handle_wrapper.h   |  7 +++++
 .../io_socket/user_space/io_handle_impl.h     |  1 +
 source/server/api_listener_impl.h             |  3 ++-
 test/common/network/connection_impl_test.cc   | 17 ++++++++++++
 .../client_connection_factory_impl_test.cc    |  2 +-
 .../quic/envoy_quic_client_session_test.cc    | 11 ++++++++
 .../quic/envoy_quic_server_session_test.cc    | 12 +++++++++
 test/mocks/network/connection.h               |  1 +
 test/mocks/network/io_handle.h                |  1 +
 test/mocks/network/mocks.h                    |  1 +
 test/server/filter_chain_benchmark_test.cc    |  1 +
 tools/spelling/spelling_dictionary.txt        |  1 +
 29 files changed, 145 insertions(+), 4 deletions(-)

diff --git a/contrib/vcl/source/vcl_io_handle.cc b/contrib/vcl/source/vcl_io_handle.cc
index 1dc67293ba15..3caa617be5ee 100644
--- a/contrib/vcl/source/vcl_io_handle.cc
+++ b/contrib/vcl/source/vcl_io_handle.cc
@@ -765,6 +765,8 @@ IoHandlePtr VclIoHandle::duplicate() {
 
 absl::optional<std::chrono::milliseconds> VclIoHandle::lastRoundTripTime() { return {}; }
 
+absl::optional<uint64_t> VclIoHandle::congestionWindowInBytes() const { return {}; }
+
 } // namespace Vcl
 } // namespace Network
 } // namespace Extensions
diff --git a/contrib/vcl/source/vcl_io_handle.h b/contrib/vcl/source/vcl_io_handle.h
index f878c3647fdf..fc2dce4abfb5 100644
--- a/contrib/vcl/source/vcl_io_handle.h
+++ b/contrib/vcl/source/vcl_io_handle.h
@@ -45,6 +45,7 @@ class VclIoHandle : public Envoy::Network::IoHandle,
   Api::IoCallUint64Result recvmmsg(RawSliceArrays& slices, uint32_t self_port,
                                    RecvMsgOutput& output) override;
   absl::optional<std::chrono::milliseconds> lastRoundTripTime() override;
+  absl::optional<uint64_t> congestionWindowInBytes() const override;
 
   bool supportsMmsg() const override;
   bool supportsUdpGro() const override { return false; }
diff --git a/envoy/api/os_sys_calls.h b/envoy/api/os_sys_calls.h
index 3c9b6f9698ac..0421a389d811 100644
--- a/envoy/api/os_sys_calls.h
+++ b/envoy/api/os_sys_calls.h
@@ -19,6 +19,9 @@ namespace Api {
 
 struct EnvoyTcpInfo {
   std::chrono::microseconds tcpi_rtt;
+  // Congestion window, in bytes. Note that posix's TCP_INFO socket option returns cwnd in packets,
+  // we multiply it by MSS to get bytes.
+  uint32_t tcpi_snd_cwnd = 0;
 };
 
 // Small struct to avoid exposing ifaddrs -- which is not defined in all platforms -- to the
diff --git a/envoy/network/connection.h b/envoy/network/connection.h
index 79b25dac406c..433d16c3df4c 100644
--- a/envoy/network/connection.h
+++ b/envoy/network/connection.h
@@ -341,6 +341,14 @@ class Connection : public Event::DeferredDeletable,
    */
   virtual void configureInitialCongestionWindow(uint64_t bandwidth_bits_per_sec,
                                                 std::chrono::microseconds rtt) PURE;
+
+  /**
+   * @return the current congestion window in bytes, or unset if not available or not
+   * congestion-controlled.
+   * @note some congestion controller's cwnd is measured in number of packets, in that case the
+   * return value is cwnd(in packets) times the connection's MSS.
+   */
+  virtual absl::optional<uint64_t> congestionWindowInBytes() const PURE;
 };
 
 using ConnectionPtr = std::unique_ptr<Connection>;
diff --git a/envoy/network/io_handle.h b/envoy/network/io_handle.h
index 87a53a7709c3..db7dd90d30f5 100644
--- a/envoy/network/io_handle.h
+++ b/envoy/network/io_handle.h
@@ -327,6 +327,14 @@ class IoHandle {
    */
   virtual absl::optional<std::chrono::milliseconds> lastRoundTripTime() PURE;
 
+  /**
+   * @return the current congestion window in bytes, or unset if not available or not
+   * congestion-controlled.
+   * @note some congestion controller's cwnd is measured in number of packets, in that case the
+   * return value is cwnd(in packets) times the connection's MSS.
+   */
+  virtual absl::optional<uint64_t> congestionWindowInBytes() const PURE;
+
   /**
    * @return the interface name for the socket, if the OS supports it. Otherwise, absl::nullopt.
    */
diff --git a/envoy/network/listen_socket.h b/envoy/network/listen_socket.h
index a900ad7ee5e1..e600d2d0e42c 100644
--- a/envoy/network/listen_socket.h
+++ b/envoy/network/listen_socket.h
@@ -75,6 +75,14 @@ class ConnectionSocket : public virtual Socket, public virtual ScopeTrackedObjec
    *  returned.
    */
   virtual absl::optional<std::chrono::milliseconds> lastRoundTripTime() PURE;
+
+  /**
+   * @return the current congestion window in bytes, or unset if not available or not
+   * congestion-controlled.
+   * @note some congestion controller's cwnd is measured in number of packets, in that case the
+   * return value is cwnd(in packets) times the connection's MSS.
+   */
+  virtual absl::optional<uint64_t> congestionWindowInBytes() const PURE;
 };
 
 using ConnectionSocketPtr = std::unique_ptr<ConnectionSocket>;
diff --git a/source/common/api/posix/os_sys_calls_impl.cc b/source/common/api/posix/os_sys_calls_impl.cc
index 65fa8ecad506..a7c241bc2047 100644
--- a/source/common/api/posix/os_sys_calls_impl.cc
+++ b/source/common/api/posix/os_sys_calls_impl.cc
@@ -290,6 +290,10 @@ SysCallBoolResult OsSysCallsImpl::socketTcpInfo([[maybe_unused]] os_fd_t sockfd,
   auto result = ::getsockopt(sockfd, IPPROTO_TCP, TCP_INFO, &unix_tcp_info, &len);
   if (!SOCKET_FAILURE(result)) {
     tcp_info->tcpi_rtt = std::chrono::microseconds(unix_tcp_info.tcpi_rtt);
+
+    const uint64_t mss = (unix_tcp_info.tcpi_snd_mss > 0) ? unix_tcp_info.tcpi_snd_mss : 1460;
+    // Convert packets to bytes.
+    tcp_info->tcpi_snd_cwnd = unix_tcp_info.tcpi_snd_cwnd * mss;
   }
   return {!SOCKET_FAILURE(result), !SOCKET_FAILURE(result) ? 0 : errno};
 #endif
diff --git a/source/common/api/win32/os_sys_calls_impl.cc b/source/common/api/win32/os_sys_calls_impl.cc
index b08f07f8d858..fc8ac700d3e0 100644
--- a/source/common/api/win32/os_sys_calls_impl.cc
+++ b/source/common/api/win32/os_sys_calls_impl.cc
@@ -403,6 +403,7 @@ SysCallBoolResult OsSysCallsImpl::socketTcpInfo([[maybe_unused]] os_fd_t sockfd,
 
   if (!SOCKET_FAILURE(rc)) {
     tcp_info->tcpi_rtt = std::chrono::microseconds(win_tcpinfo.RttUs);
+    tcp_info->tcpi_snd_cwnd = win_tcpinfo.Cwnd;
   }
   return {!SOCKET_FAILURE(rc), !SOCKET_FAILURE(rc) ? 0 : ::WSAGetLastError()};
 #endif
diff --git a/source/common/network/connection_impl.cc b/source/common/network/connection_impl.cc
index 926457190801..a8c54da16736 100644
--- a/source/common/network/connection_impl.cc
+++ b/source/common/network/connection_impl.cc
@@ -788,6 +788,10 @@ void ConnectionImpl::configureInitialCongestionWindow(uint64_t bandwidth_bits_pe
   return transport_socket_->configureInitialCongestionWindow(bandwidth_bits_per_sec, rtt);
 }
 
+absl::optional<uint64_t> ConnectionImpl::congestionWindowInBytes() const {
+  return socket_->congestionWindowInBytes();
+}
+
 void ConnectionImpl::flushWriteBuffer() {
   if (state() == State::Open && write_buffer_->length() > 0) {
     onWriteReady();
diff --git a/source/common/network/connection_impl.h b/source/common/network/connection_impl.h
index a775d17c9d2c..0882ac9af802 100644
--- a/source/common/network/connection_impl.h
+++ b/source/common/network/connection_impl.h
@@ -101,6 +101,7 @@ class ConnectionImpl : public ConnectionImplBase, public TransportSocketCallback
   absl::optional<std::chrono::milliseconds> lastRoundTripTime() const override;
   void configureInitialCongestionWindow(uint64_t bandwidth_bits_per_sec,
                                         std::chrono::microseconds rtt) override;
+  absl::optional<uint64_t> congestionWindowInBytes() const override;
 
   // Network::FilterManagerConnection
   void rawWrite(Buffer::Instance& data, bool end_stream) override;
diff --git a/source/common/network/happy_eyeballs_connection_impl.cc b/source/common/network/happy_eyeballs_connection_impl.cc
index 39431d834175..cbe31161771c 100644
--- a/source/common/network/happy_eyeballs_connection_impl.cc
+++ b/source/common/network/happy_eyeballs_connection_impl.cc
@@ -282,6 +282,11 @@ absl::optional<std::chrono::milliseconds> HappyEyeballsConnectionImpl::lastRound
   return connections_[0]->lastRoundTripTime();
 }
 
+absl::optional<uint64_t> HappyEyeballsConnectionImpl::congestionWindowInBytes() const {
+  // Note, this value changes constantly even within the same connection.
+  return connections_[0]->congestionWindowInBytes();
+}
+
 void HappyEyeballsConnectionImpl::addConnectionCallbacks(ConnectionCallbacks& cb) {
   if (connect_finished_) {
     connections_[0]->addConnectionCallbacks(cb);
diff --git a/source/common/network/happy_eyeballs_connection_impl.h b/source/common/network/happy_eyeballs_connection_impl.h
index 57acb7fc13d6..d330bdcc5e8f 100644
--- a/source/common/network/happy_eyeballs_connection_impl.h
+++ b/source/common/network/happy_eyeballs_connection_impl.h
@@ -69,6 +69,7 @@ class HappyEyeballsConnectionImpl : public ClientConnection,
   bool startSecureTransport() override;
   absl::optional<std::chrono::milliseconds> lastRoundTripTime() const override;
   void configureInitialCongestionWindow(uint64_t, std::chrono::microseconds) override {}
+  absl::optional<uint64_t> congestionWindowInBytes() const override;
 
   // Simple getters which always delegate to the first connection in connections_.
   bool isHalfCloseEnabled() override;
diff --git a/source/common/network/io_socket_handle_impl.cc b/source/common/network/io_socket_handle_impl.cc
index c40e9f03d77e..9f7a2962fda0 100644
--- a/source/common/network/io_socket_handle_impl.cc
+++ b/source/common/network/io_socket_handle_impl.cc
@@ -576,6 +576,15 @@ absl::optional<std::chrono::milliseconds> IoSocketHandleImpl::lastRoundTripTime(
   return std::chrono::duration_cast<std::chrono::milliseconds>(info.tcpi_rtt);
 }
 
+absl::optional<uint64_t> IoSocketHandleImpl::congestionWindowInBytes() const {
+  Api::EnvoyTcpInfo info;
+  auto result = Api::OsSysCallsSingleton::get().socketTcpInfo(fd_, &info);
+  if (!result.return_value_) {
+    return {};
+  }
+  return info.tcpi_snd_cwnd;
+}
+
 absl::optional<std::string> IoSocketHandleImpl::interfaceName() {
   auto& os_syscalls_singleton = Api::OsSysCallsSingleton::get();
   if (!os_syscalls_singleton.supportsGetifaddrs()) {
diff --git a/source/common/network/io_socket_handle_impl.h b/source/common/network/io_socket_handle_impl.h
index fe57fff4073a..7bc007a2e7ed 100644
--- a/source/common/network/io_socket_handle_impl.h
+++ b/source/common/network/io_socket_handle_impl.h
@@ -80,6 +80,7 @@ class IoSocketHandleImpl : public IoHandle, protected Logger::Loggable<Logger::I
 
   Api::SysCallIntResult shutdown(int how) override;
   absl::optional<std::chrono::milliseconds> lastRoundTripTime() override;
+  absl::optional<uint64_t> congestionWindowInBytes() const override;
   absl::optional<std::string> interfaceName() override;
 
 protected:
diff --git a/source/common/network/listen_socket_impl.h b/source/common/network/listen_socket_impl.h
index 14e516671883..e190be417719 100644
--- a/source/common/network/listen_socket_impl.h
+++ b/source/common/network/listen_socket_impl.h
@@ -228,6 +228,10 @@ class ConnectionSocketImpl : public SocketImpl, public ConnectionSocket {
     return ioHandle().lastRoundTripTime();
   }
 
+  absl::optional<uint64_t> congestionWindowInBytes() const override {
+    return ioHandle().congestionWindowInBytes();
+  }
+
   void dumpState(std::ostream& os, int indent_level) const override {
     const char* spaces = spacesForLevel(indent_level);
     os << spaces << "ListenSocketImpl " << this << DUMP_MEMBER(transport_protocol_) << "\n";
diff --git a/source/common/quic/quic_filter_manager_connection_impl.cc b/source/common/quic/quic_filter_manager_connection_impl.cc
index 1cba93673f73..26d48eb50c97 100644
--- a/source/common/quic/quic_filter_manager_connection_impl.cc
+++ b/source/common/quic/quic_filter_manager_connection_impl.cc
@@ -217,6 +217,20 @@ void QuicFilterManagerConnectionImpl::onSendBufferLowWatermark() {
   }
 }
 
+absl::optional<std::chrono::milliseconds>
+QuicFilterManagerConnectionImpl::lastRoundTripTime() const {
+  if (quicConnection() == nullptr) {
+    return {};
+  }
+
+  const auto* rtt_stats = quicConnection()->sent_packet_manager().GetRttStats();
+  if (!rtt_stats->latest_rtt().IsZero()) {
+    return std::chrono::milliseconds(rtt_stats->latest_rtt().ToMilliseconds());
+  }
+
+  return std::chrono::milliseconds(rtt_stats->initial_rtt().ToMilliseconds());
+}
+
 void QuicFilterManagerConnectionImpl::configureInitialCongestionWindow(
     uint64_t bandwidth_bits_per_sec, std::chrono::microseconds rtt) {
   if (quicConnection() != nullptr) {
@@ -231,5 +245,18 @@ void QuicFilterManagerConnectionImpl::configureInitialCongestionWindow(
   }
 }
 
+absl::optional<uint64_t> QuicFilterManagerConnectionImpl::congestionWindowInBytes() const {
+  if (quicConnection() == nullptr) {
+    return {};
+  }
+
+  uint64_t cwnd = quicConnection()->sent_packet_manager().GetCongestionWindowInBytes();
+  if (cwnd == 0) {
+    return {};
+  }
+
+  return cwnd;
+}
+
 } // namespace Quic
 } // namespace Envoy
diff --git a/source/common/quic/quic_filter_manager_connection_impl.h b/source/common/quic/quic_filter_manager_connection_impl.h
index 4fe18f0abf75..04094e832dab 100644
--- a/source/common/quic/quic_filter_manager_connection_impl.h
+++ b/source/common/quic/quic_filter_manager_connection_impl.h
@@ -114,10 +114,10 @@ class QuicFilterManagerConnectionImpl : public Network::ConnectionImplBase,
   const StreamInfo::StreamInfo& streamInfo() const override { return stream_info_; }
   absl::string_view transportFailureReason() const override { return transport_failure_reason_; }
   bool startSecureTransport() override { return false; }
-  // TODO(#2557) Implement this.
-  absl::optional<std::chrono::milliseconds> lastRoundTripTime() const override { return {}; }
+  absl::optional<std::chrono::milliseconds> lastRoundTripTime() const override;
   void configureInitialCongestionWindow(uint64_t bandwidth_bits_per_sec,
                                         std::chrono::microseconds rtt) override;
+  absl::optional<uint64_t> congestionWindowInBytes() const override;
 
   // Network::FilterManagerConnection
   void rawWrite(Buffer::Instance& data, bool end_stream) override;
diff --git a/source/common/quic/quic_io_handle_wrapper.h b/source/common/quic/quic_io_handle_wrapper.h
index db9087daff10..9e41647b625a 100644
--- a/source/common/quic/quic_io_handle_wrapper.h
+++ b/source/common/quic/quic_io_handle_wrapper.h
@@ -5,6 +5,7 @@
 
 #include "envoy/network/io_handle.h"
 
+#include "source/common/common/assert.h"
 #include "source/common/network/io_socket_error_impl.h"
 
 namespace Envoy {
@@ -142,6 +143,12 @@ class QuicIoHandleWrapper : public Network::IoHandle {
 
   Api::SysCallIntResult shutdown(int how) override { return io_handle_.shutdown(how); }
   absl::optional<std::chrono::milliseconds> lastRoundTripTime() override { return {}; }
+  absl::optional<uint64_t> congestionWindowInBytes() const override {
+    // QUIC should get congestion window from QuicFilterManagerConnectionImpl, which implements the
+    // Envoy::Network::Connection::congestionWindowInBytes interface.
+    IS_ENVOY_BUG("QuicIoHandleWrapper does not implement congestionWindowInBytes.");
+    return {};
+  }
 
 private:
   Network::IoHandle& io_handle_;
diff --git a/source/extensions/io_socket/user_space/io_handle_impl.h b/source/extensions/io_socket/user_space/io_handle_impl.h
index 9f7ab9010b8b..5b0bdf1daa31 100644
--- a/source/extensions/io_socket/user_space/io_handle_impl.h
+++ b/source/extensions/io_socket/user_space/io_handle_impl.h
@@ -88,6 +88,7 @@ class IoHandleImpl final : public Network::IoHandle,
 
   Api::SysCallIntResult shutdown(int how) override;
   absl::optional<std::chrono::milliseconds> lastRoundTripTime() override { return absl::nullopt; }
+  absl::optional<uint64_t> congestionWindowInBytes() const override { return absl::nullopt; }
   absl::optional<std::string> interfaceName() override { return absl::nullopt; }
 
   void setWatermarks(uint32_t watermark) { pending_received_data_.setWatermarks(watermark); }
diff --git a/source/server/api_listener_impl.h b/source/server/api_listener_impl.h
index d96c8b442268..34bb413bdb4d 100644
--- a/source/server/api_listener_impl.h
+++ b/source/server/api_listener_impl.h
@@ -160,8 +160,9 @@ class ApiListenerImplBase : public ApiListener,
         IS_ENVOY_BUG("Unexpected function call");
         return false;
       }
-      absl::optional<std::chrono::milliseconds> lastRoundTripTime() const override { return {}; };
+      absl::optional<std::chrono::milliseconds> lastRoundTripTime() const override { return {}; }
       void configureInitialCongestionWindow(uint64_t, std::chrono::microseconds) override {}
+      absl::optional<uint64_t> congestionWindowInBytes() const override { return {}; }
       // ScopeTrackedObject
       void dumpState(std::ostream& os, int) const override { os << "SyntheticConnection"; }
 
diff --git a/test/common/network/connection_impl_test.cc b/test/common/network/connection_impl_test.cc
index 104e2a532fb7..344f14f4ab35 100644
--- a/test/common/network/connection_impl_test.cc
+++ b/test/common/network/connection_impl_test.cc
@@ -303,6 +303,23 @@ TEST_P(ConnectionImplTest, UniqueId) {
   disconnect(false);
 }
 
+TEST_P(ConnectionImplTest, GetCongestionWindow) {
+  setUpBasicConnection();
+  connect();
+
+// Congestion window is available on Posix(guarded by TCP_INFO) and Windows(guarded by
+// SIO_TCP_INFO).
+#if defined(TCP_INFO) || defined(SIO_TCP_INFO)
+  EXPECT_GT(client_connection_->congestionWindowInBytes().value(), 500);
+  EXPECT_GT(server_connection_->congestionWindowInBytes().value(), 500);
+#else
+  EXPECT_FALSE(client_connection_->congestionWindowInBytes().has_value());
+  EXPECT_FALSE(server_connection_->congestionWindowInBytes().has_value());
+#endif
+
+  disconnect(true);
+}
+
 TEST_P(ConnectionImplTest, CloseDuringConnectCallback) {
   setUpBasicConnection();
 
diff --git a/test/common/quic/client_connection_factory_impl_test.cc b/test/common/quic/client_connection_factory_impl_test.cc
index 0fd809b8eb4b..ef10af1ea228 100644
--- a/test/common/quic/client_connection_factory_impl_test.cc
+++ b/test/common/quic/client_connection_factory_impl_test.cc
@@ -87,7 +87,7 @@ TEST_P(QuicNetworkConnectionTest, BufferLimits) {
   ASSERT(session != nullptr);
   EXPECT_EQ(highWatermark(session), 45);
   EXPECT_EQ(absl::nullopt, session->unixSocketPeerCredentials());
-  EXPECT_EQ(absl::nullopt, session->lastRoundTripTime());
+  EXPECT_NE(absl::nullopt, session->lastRoundTripTime());
   client_connection->close(Network::ConnectionCloseType::NoFlush);
 }
 
diff --git a/test/common/quic/envoy_quic_client_session_test.cc b/test/common/quic/envoy_quic_client_session_test.cc
index 62a5579b6abd..2c7e38c51bc5 100644
--- a/test/common/quic/envoy_quic_client_session_test.cc
+++ b/test/common/quic/envoy_quic_client_session_test.cc
@@ -348,5 +348,16 @@ TEST_P(EnvoyQuicClientSessionTest, IncomingUnidirectionalReadStream) {
   envoy_quic_session_.OnStreamFrame(stream_frame2);
 }
 
+TEST_P(EnvoyQuicClientSessionTest, GetRttAndCwnd) {
+  EXPECT_GT(envoy_quic_session_.lastRoundTripTime().value(), std::chrono::microseconds(0));
+  // Just make sure the CWND is non-zero. We don't want to make strong assertions on what the value
+  // should be in this test, that is the job the congestion controllers' tests.
+  EXPECT_GT(envoy_quic_session_.congestionWindowInBytes().value(), 500);
+
+  envoy_quic_session_.configureInitialCongestionWindow(8000000, std::chrono::microseconds(1000000));
+  EXPECT_GT(envoy_quic_session_.congestionWindowInBytes().value(),
+            quic::kInitialCongestionWindow * quic::kDefaultTCPMSS);
+}
+
 } // namespace Quic
 } // namespace Envoy
diff --git a/test/common/quic/envoy_quic_server_session_test.cc b/test/common/quic/envoy_quic_server_session_test.cc
index db9cfea306ad..0a551e3928c3 100644
--- a/test/common/quic/envoy_quic_server_session_test.cc
+++ b/test/common/quic/envoy_quic_server_session_test.cc
@@ -1033,5 +1033,17 @@ TEST_F(EnvoyQuicServerSessionTest, IncomingUnidirectionalReadStream) {
   envoy_quic_session_.OnStreamFrame(stream_frame);
 }
 
+TEST_F(EnvoyQuicServerSessionTest, GetRttAndCwnd) {
+  installReadFilter();
+  EXPECT_GT(envoy_quic_session_.lastRoundTripTime().value(), std::chrono::microseconds(0));
+  // Just make sure the CWND is non-zero. We don't want to make strong assertions on what the value
+  // should be in this test, that is the job the congestion controllers' tests.
+  EXPECT_GT(envoy_quic_session_.congestionWindowInBytes().value(), 500);
+
+  envoy_quic_session_.configureInitialCongestionWindow(8000000, std::chrono::microseconds(1000000));
+  EXPECT_GT(envoy_quic_session_.congestionWindowInBytes().value(),
+            quic::kInitialCongestionWindow * quic::kDefaultTCPMSS);
+}
+
 } // namespace Quic
 } // namespace Envoy
diff --git a/test/mocks/network/connection.h b/test/mocks/network/connection.h
index a98e298dcb58..e939c4395ba1 100644
--- a/test/mocks/network/connection.h
+++ b/test/mocks/network/connection.h
@@ -88,6 +88,7 @@ class MockConnectionBase {
   MOCK_METHOD(absl::optional<std::chrono::milliseconds>, lastRoundTripTime, (), (const));          \
   MOCK_METHOD(void, configureInitialCongestionWindow,                                              \
               (uint64_t bandwidth_bits_per_sec, std::chrono::microseconds rtt), ());               \
+  MOCK_METHOD(absl::optional<uint64_t>, congestionWindowInBytes, (), (const));                     \
   MOCK_METHOD(void, dumpState, (std::ostream&, int), (const));
 
 class MockConnection : public Connection, public MockConnectionBase {
diff --git a/test/mocks/network/io_handle.h b/test/mocks/network/io_handle.h
index 1ccb724d53fe..aff62657ae89 100644
--- a/test/mocks/network/io_handle.h
+++ b/test/mocks/network/io_handle.h
@@ -61,6 +61,7 @@ class MockIoHandle : public IoHandle {
   MOCK_METHOD(void, resetFileEvents, ());
   MOCK_METHOD(Api::SysCallIntResult, shutdown, (int how));
   MOCK_METHOD(absl::optional<std::chrono::milliseconds>, lastRoundTripTime, ());
+  MOCK_METHOD(absl::optional<uint64_t>, congestionWindowInBytes, (), (const));
   MOCK_METHOD(Api::SysCallIntResult, ioctl,
               (unsigned long, void*, unsigned long, void*, unsigned long, unsigned long*));
   MOCK_METHOD(absl::optional<std::string>, interfaceName, ());
diff --git a/test/mocks/network/mocks.h b/test/mocks/network/mocks.h
index c5c1ab979ada..2acb8562d3e1 100644
--- a/test/mocks/network/mocks.h
+++ b/test/mocks/network/mocks.h
@@ -355,6 +355,7 @@ class MockConnectionSocket : public ConnectionSocket {
               (unsigned long, void*, unsigned long, void*, unsigned long, unsigned long*));
   MOCK_METHOD(Api::SysCallIntResult, setBlockingForTest, (bool));
   MOCK_METHOD(absl::optional<std::chrono::milliseconds>, lastRoundTripTime, ());
+  MOCK_METHOD(absl::optional<uint64_t>, congestionWindowInBytes, (), (const));
   MOCK_METHOD(void, dumpState, (std::ostream&, int), (const));
 
   IoHandlePtr io_handle_;
diff --git a/test/server/filter_chain_benchmark_test.cc b/test/server/filter_chain_benchmark_test.cc
index e2d4a9b891e1..f793cb07bf23 100644
--- a/test/server/filter_chain_benchmark_test.cc
+++ b/test/server/filter_chain_benchmark_test.cc
@@ -128,6 +128,7 @@ class MockConnectionSocket : public Network::ConnectionSocket {
   }
   Api::SysCallIntResult setBlockingForTest(bool) override { return {0, 0}; }
   absl::optional<std::chrono::milliseconds> lastRoundTripTime() override { return {}; }
+  absl::optional<uint64_t> congestionWindowInBytes() const override { return {}; }
   void dumpState(std::ostream&, int) const override {}
 
 private:
diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt
index 3cd5e6b9fe15..4eb45381497c 100644
--- a/tools/spelling/spelling_dictionary.txt
+++ b/tools/spelling/spelling_dictionary.txt
@@ -321,6 +321,7 @@ SIGINT
 SIGPIPE
 SIGSEGV
 SIGTERM
+SIO
 SMTP
 SNI
 SOTW

From e7f53dd996c3cf34814cfcbe69656e6d9790589b Mon Sep 17 00:00:00 2001
From: alyssawilk <alyssar@chromium.org>
Date: Tue, 1 Mar 2022 13:47:59 -0500
Subject: [PATCH 03/14] Update QUICHE from 50f15e7a5 to cf1588207 (#20154)

https://github.com/google/quiche/compare/50f15e7a5..cf1588207

$ git log 50f15e7a5..cf1588207 --date=short --no-merges --format="%ad %al %s"

2022-02-28 wub Deprecate --gfe2_reloadable_flag_quic_crypto_noop_if_disconnected_after_process_chlo.
2022-02-27 vasilvv Remove QuicheMemSlice(QuicUniqueBufferPtr, size_t) constructor.
2022-02-26 fayang Use std::string instead of absl::string_view in CryptoBufferMap.
2022-02-25 bnc Ignore incoming HTTP/3 MAX_PUSH_ID frames.
2022-02-25 bnc Remove Http3DebugVisitor::OnMaxPushIdFrameSent().
2022-02-25 bnc Remove QuicSpdySession::CanCreatePushStreamWithId().
2022-02-25 fayang Deprecate gfe2_reloadable_flag_quic_single_ack_in_packet2.

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>
---
 bazel/repository_locations.bzl                |  6 ++---
 source/common/quic/active_quic_listener.cc    |  1 -
 .../quic/client_connection_factory_impl.cc    |  1 -
 .../common/quic/platform/quiche_flags_impl.cc |  2 --
 .../quic/platform/quiche_mem_slice_impl.cc    | 24 ++++++++++---------
 .../quic/platform/quiche_mem_slice_impl.h     |  2 +-
 .../quic/envoy_quic_client_stream_test.cc     |  1 -
 7 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl
index bfc38797f8ad..efa24cb15710 100644
--- a/bazel/repository_locations.bzl
+++ b/bazel/repository_locations.bzl
@@ -824,12 +824,12 @@ REPOSITORY_LOCATIONS_SPEC = dict(
         project_name = "QUICHE",
         project_desc = "QUICHE (QUIC, HTTP/2, Etc) is Google‘s implementation of QUIC and related protocols",
         project_url = "https://github.com/google/quiche",
-        version = "50f15e7a5655ed94e369ea62de0702822a3ad91e",
-        sha256 = "5997844144628bf4b9dbe53bb438a081ebc4e03d62af213ee20806faf1ee5fef",
+        version = "cf1588207faa6771333052330846acd0c45a045c",
+        sha256 = "c4f61b07c2f061f8d4669b88fd9a5eb19f2d407b0b0912f5feac1f149ad6f390",
         urls = ["https://github.com/google/quiche/archive/{version}.tar.gz"],
         strip_prefix = "quiche-{version}",
         use_category = ["dataplane_core"],
-        release_date = "2022-02-25",
+        release_date = "2022-02-28",
         cpe = "N/A",
     ),
     com_googlesource_googleurl = dict(
diff --git a/source/common/quic/active_quic_listener.cc b/source/common/quic/active_quic_listener.cc
index abc2b968dc01..17775ef19f66 100644
--- a/source/common/quic/active_quic_listener.cc
+++ b/source/common/quic/active_quic_listener.cc
@@ -56,7 +56,6 @@ ActiveQuicListener::ActiveQuicListener(
       kernel_worker_routing_(kernel_worker_routing),
       packets_to_read_to_connection_count_ratio_(packets_to_read_to_connection_count_ratio),
       crypto_server_stream_factory_(crypto_server_stream_factory) {
-  ASSERT(GetQuicReloadableFlag(quic_single_ack_in_packet2));
   ASSERT(!GetQuicFlag(FLAGS_quic_header_size_limit_includes_overhead));
 
   enabled_.emplace(Runtime::FeatureFlag(enabled, runtime));
diff --git a/source/common/quic/client_connection_factory_impl.cc b/source/common/quic/client_connection_factory_impl.cc
index 1fd7bbf5b6a3..aab72a68533b 100644
--- a/source/common/quic/client_connection_factory_impl.cc
+++ b/source/common/quic/client_connection_factory_impl.cc
@@ -31,7 +31,6 @@ std::unique_ptr<Network::ClientConnection> createQuicNetworkConnection(
     Network::Address::InstanceConstSharedPtr server_addr,
     Network::Address::InstanceConstSharedPtr local_addr, QuicStatNames& quic_stat_names,
     OptRef<Http::AlternateProtocolsCache> rtt_cache, Stats::Scope& scope) {
-  ASSERT(GetQuicReloadableFlag(quic_single_ack_in_packet2));
   ASSERT(crypto_config != nullptr);
   PersistentQuicInfoImpl* info_impl = reinterpret_cast<PersistentQuicInfoImpl*>(&info);
   quic::ParsedQuicVersionVector quic_versions = quic::CurrentSupportedHttp3Versions();
diff --git a/source/common/quic/platform/quiche_flags_impl.cc b/source/common/quic/platform/quiche_flags_impl.cc
index a4185cc7069d..73710d23254d 100644
--- a/source/common/quic/platform/quiche_flags_impl.cc
+++ b/source/common/quic/platform/quiche_flags_impl.cc
@@ -35,8 +35,6 @@ absl::flat_hash_map<absl::string_view, Flag*> makeFlagMap() {
   // Envoy only supports RFC-v1 in the long term, so disable IETF draft 29 implementation by
   // default.
   FLAGS_quic_reloadable_flag_quic_disable_version_draft_29->setValue(true);
-  // This flag fixes a QUICHE issue which may crash Envoy during connection close.
-  FLAGS_quic_reloadable_flag_quic_single_ack_in_packet2->setValue(true);
   // This flag enables BBR, otherwise QUIC will use Cubic which is less performant.
   FLAGS_quic_reloadable_flag_quic_default_to_bbr->setValue(true);
 
diff --git a/source/common/quic/platform/quiche_mem_slice_impl.cc b/source/common/quic/platform/quiche_mem_slice_impl.cc
index 3062835a4d66..020901925a7a 100644
--- a/source/common/quic/platform/quiche_mem_slice_impl.cc
+++ b/source/common/quic/platform/quiche_mem_slice_impl.cc
@@ -12,18 +12,20 @@
 
 namespace quiche {
 
-QuicheMemSliceImpl::QuicheMemSliceImpl(quic::QuicUniqueBufferPtr buffer, size_t length)
-    : fragment_(std::make_unique<Envoy::Buffer::BufferFragmentImpl>(
-          buffer.get(), length,
-          // TODO(danzh) change the buffer fragment constructor to take the lambda by move instead
-          // of copy, so that the ownership of |buffer| can be transferred to lambda via capture
-          // here and below to unify and simplify the constructor implementations.
-          [allocator = buffer.get_deleter().allocator()](const void* p, size_t,
+QuicheMemSliceImpl::QuicheMemSliceImpl(quic::QuicBuffer buffer) {
+  size_t length = buffer.size();
+  quic::QuicUniqueBufferPtr buffer_ptr = buffer.Release();
+  fragment_ = std::make_unique<Envoy::Buffer::BufferFragmentImpl>(
+      buffer_ptr.get(), length,
+      // TODO(danzh) change the buffer fragment constructor to take the lambda by move instead
+      // of copy, so that the ownership of |buffer| can be transferred to lambda via capture
+      // here and below to unify and simplify the constructor implementations.
+      [allocator = buffer_ptr.get_deleter().allocator()](const void* p, size_t,
                                                          const Envoy::Buffer::BufferFragmentImpl*) {
-            quic::QuicBufferDeleter deleter(allocator);
-            deleter(const_cast<char*>(static_cast<const char*>(p)));
-          })) {
-  buffer.release();
+        quic::QuicBufferDeleter deleter(allocator);
+        deleter(const_cast<char*>(static_cast<const char*>(p)));
+      });
+  buffer_ptr.release();
   single_slice_buffer_.addBufferFragment(*fragment_);
   ASSERT(this->length() == length);
 }
diff --git a/source/common/quic/platform/quiche_mem_slice_impl.h b/source/common/quic/platform/quiche_mem_slice_impl.h
index b1582a8f746a..3ebfd973de13 100644
--- a/source/common/quic/platform/quiche_mem_slice_impl.h
+++ b/source/common/quic/platform/quiche_mem_slice_impl.h
@@ -26,7 +26,7 @@ class QuicheMemSliceImpl {
   ~QuicheMemSliceImpl();
 
   // Constructs a QuicheMemSliceImpl by taking ownership of the memory in |buffer|.
-  QuicheMemSliceImpl(quic::QuicUniqueBufferPtr buffer, size_t length);
+  QuicheMemSliceImpl(quic::QuicBuffer buffer);
   QuicheMemSliceImpl(std::unique_ptr<char[]> buffer, size_t length);
 
   // Constructs a QuicheMemSliceImpl from a Buffer::Instance with first |length| bytes in it.
diff --git a/test/common/quic/envoy_quic_client_stream_test.cc b/test/common/quic/envoy_quic_client_stream_test.cc
index 9a33dcd17785..b1a45dc0cdbb 100644
--- a/test/common/quic/envoy_quic_client_stream_test.cc
+++ b/test/common/quic/envoy_quic_client_stream_test.cc
@@ -50,7 +50,6 @@ class EnvoyQuicClientStreamTest : public testing::Test {
                                                stats_, http3_options_)),
         request_headers_{{":authority", host_}, {":method", "POST"}, {":path", "/"}},
         request_trailers_{{"trailer-key", "trailer-value"}} {
-    SetQuicReloadableFlag(quic_single_ack_in_packet2, false);
     quic_stream_->setResponseDecoder(stream_decoder_);
     quic_stream_->addCallbacks(stream_callbacks_);
     quic_session_.ActivateStream(std::unique_ptr<EnvoyQuicClientStream>(quic_stream_));

From d25664f45eea7c4f54ce7fafd8a92f7085b5495c Mon Sep 17 00:00:00 2001
From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com>
Date: Tue, 1 Mar 2022 19:38:58 +0000
Subject: [PATCH 04/14] build(deps): bump actions/stale from 4.1.0 to 5
 (#20159)

Bumps [actions/stale](https://github.com/actions/stale) from 4.1.0 to 5.
- [Release notes](https://github.com/actions/stale/releases)
- [Changelog](https://github.com/actions/stale/blob/main/CHANGELOG.md)
- [Commits](https://github.com/actions/stale/compare/v4.1.0...v5)

---
updated-dependencies:
- dependency-name: actions/stale
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>
---
 .github/workflows/stale.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml
index 933c34993a70..dc094c6094a5 100644
--- a/.github/workflows/stale.yml
+++ b/.github/workflows/stale.yml
@@ -11,7 +11,7 @@ jobs:
 
     steps:
     - name: Prune Stale
-      uses: actions/stale@v4.1.0
+      uses: actions/stale@v5
       with:
         repo-token: ${{ secrets.GITHUB_TOKEN }}
         # Different amounts of days for issues/PRs are not currently supported but there is a PR

From f176746a68ee96ce8a87c9ee294bf710f9dda04a Mon Sep 17 00:00:00 2001
From: Joshua Marantz <jmarantz@google.com>
Date: Tue, 1 Mar 2022 14:48:31 -0500
Subject: [PATCH 05/14] admin: improve test coverage and increase the
 coverage-percent threshold (#20025)

Adds a missing test for recent lookups now that there are no more fake symbol tables. Adds tests for a variety of override methods defined in admin.h that were previously hard to hit.

Adds a benchmark test to establish a baseline for the speedups in #19693

Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>
---
 source/server/admin/admin.h                   |  17 +--
 source/server/admin/stats_handler.cc          |  40 +++---
 source/server/admin/stats_handler.h           |  15 ++-
 test/per_file_coverage.sh                     |   2 +-
 test/server/admin/BUILD                       |  13 ++
 test/server/admin/admin_test.cc               |  68 +++++++++-
 test/server/admin/stats_handler_speed_test.cc | 125 ++++++++++++++++++
 test/server/admin/stats_handler_test.cc       |  43 ++++--
 8 files changed, 276 insertions(+), 47 deletions(-)
 create mode 100644 test/server/admin/stats_handler_speed_test.cc

diff --git a/source/server/admin/admin.h b/source/server/admin/admin.h
index 0eccd8c34921..1f9b13513ea2 100644
--- a/source/server/admin/admin.h
+++ b/source/server/admin/admin.h
@@ -200,19 +200,12 @@ class AdminImpl : public Admin,
                      Http::ResponseHeaderMap& response_headers, std::string& body) override;
   void closeSocket();
   void addListenerToHandler(Network::ConnectionHandler* handler) override;
-  Server::Instance& server() { return server_; }
 
   GenHandlerCb createHandlerFunction() {
     return [this](absl::string_view path_and_query, AdminStream& admin_stream) -> HandlerPtr {
       return findHandler(path_and_query, admin_stream);
     };
   }
-  AdminFilter::AdminServerCallbackFunction createCallbackFunction() {
-    return [this](absl::string_view path_and_query, Http::ResponseHeaderMap& response_headers,
-                  Buffer::OwnedImpl& response, AdminFilter& filter) -> Http::Code {
-      return runCallback(path_and_query, response_headers, response, filter);
-    };
-  }
   uint64_t maxRequestsPerConnection() const override { return 0; }
   const HttpConnectionManagerProto::ProxyStatusConfig* proxyStatusConfig() const override {
     return proxy_status_config_.get();
@@ -227,6 +220,8 @@ class AdminImpl : public Admin,
   static HandlerPtr makeStaticTextHandler(absl::string_view response_text, Http::Code code);
 
 private:
+  friend class AdminTestingPeer;
+
   /**
    * Individual admin handler including prefix, help text, and callback.
    */
@@ -295,8 +290,8 @@ class AdminImpl : public Admin,
    * OverloadManager keeps the admin interface accessible even when the proxy is overloaded.
    */
   struct NullOverloadManager : public OverloadManager {
-    struct NullThreadLocalOverloadState : public ThreadLocalOverloadState {
-      NullThreadLocalOverloadState(Event::Dispatcher& dispatcher) : dispatcher_(dispatcher) {}
+    struct OverloadState : public ThreadLocalOverloadState {
+      OverloadState(Event::Dispatcher& dispatcher) : dispatcher_(dispatcher) {}
       const OverloadActionState& getState(const std::string&) override { return inactive_; }
       bool tryAllocateResource(OverloadProactiveResourceName, int64_t) override { return false; }
       bool tryDeallocateResource(OverloadProactiveResourceName, int64_t) override { return false; }
@@ -310,12 +305,12 @@ class AdminImpl : public Admin,
 
     void start() override {
       tls_->set([](Event::Dispatcher& dispatcher) -> ThreadLocal::ThreadLocalObjectSharedPtr {
-        return std::make_shared<NullThreadLocalOverloadState>(dispatcher);
+        return std::make_shared<OverloadState>(dispatcher);
       });
     }
 
     ThreadLocalOverloadState& getThreadLocalOverloadState() override {
-      return tls_->getTyped<NullThreadLocalOverloadState>();
+      return tls_->getTyped<OverloadState>();
     }
 
     Event::ScaledRangeTimerManagerFactory scaledTimerFactory() override { return nullptr; }
diff --git a/source/server/admin/stats_handler.cc b/source/server/admin/stats_handler.cc
index 9e803ea4885c..af6b3675b67e 100644
--- a/source/server/admin/stats_handler.cc
+++ b/source/server/admin/stats_handler.cc
@@ -86,18 +86,33 @@ Http::Code StatsHandler::handlerStats(absl::string_view url,
   }
 
   const absl::optional<std::string> format_value = Utility::formatParam(params);
-  if (format_value.has_value() && format_value.value() == "prometheus") {
-    return handlerPrometheusStats(url, response_headers, response, admin_stream);
+  bool json = false;
+  if (format_value.has_value()) {
+    if (format_value.value() == "prometheus") {
+      return handlerPrometheusStats(url, response_headers, response, admin_stream);
+    } else if (format_value.value() == "json") {
+      json = true;
+    } else {
+      response.add("usage: /stats?format=json  or /stats?format=prometheus \n");
+      response.add("\n");
+      return Http::Code::NotFound;
+    }
   }
+  return handlerStats(server_.stats(), used_only, json, regex, response_headers, response);
+}
 
+Http::Code StatsHandler::handlerStats(Stats::Store& stats, bool used_only, bool json,
+                                      const absl::optional<std::regex>& regex,
+                                      Http::ResponseHeaderMap& response_headers,
+                                      Buffer::Instance& response) {
   std::map<std::string, uint64_t> all_stats;
-  for (const Stats::CounterSharedPtr& counter : server_.stats().counters()) {
+  for (const Stats::CounterSharedPtr& counter : stats.counters()) {
     if (shouldShowMetric(*counter, used_only, regex)) {
       all_stats.emplace(counter->name(), counter->value());
     }
   }
 
-  for (const Stats::GaugeSharedPtr& gauge : server_.stats().gauges()) {
+  for (const Stats::GaugeSharedPtr& gauge : stats.gauges()) {
     if (shouldShowMetric(*gauge, used_only, regex)) {
       ASSERT(gauge->importMode() != Stats::Gauge::ImportMode::Uninitialized);
       all_stats.emplace(gauge->name(), gauge->value());
@@ -105,33 +120,26 @@ Http::Code StatsHandler::handlerStats(absl::string_view url,
   }
 
   std::map<std::string, std::string> text_readouts;
-  for (const auto& text_readout : server_.stats().textReadouts()) {
+  for (const auto& text_readout : stats.textReadouts()) {
     if (shouldShowMetric(*text_readout, used_only, regex)) {
       text_readouts.emplace(text_readout->name(), text_readout->value());
     }
   }
 
-  Stats::Store& stats = server_.stats();
   std::vector<Stats::ParentHistogramSharedPtr> histograms = stats.histograms();
   stats.symbolTable().sortByStatNames<Stats::ParentHistogramSharedPtr>(
       histograms.begin(), histograms.end(),
       [](const Stats::ParentHistogramSharedPtr& a) -> Stats::StatName { return a->statName(); });
 
-  if (!format_value.has_value()) {
+  if (!json) {
     // Display plain stats if format query param is not there.
     statsAsText(all_stats, text_readouts, histograms, used_only, regex, response);
     return Http::Code::OK;
   }
 
-  if (format_value.value() == "json") {
-    response_headers.setReferenceContentType(Http::Headers::get().ContentTypeValues.Json);
-    response.add(statsAsJson(all_stats, text_readouts, histograms, used_only, regex));
-    return Http::Code::OK;
-  }
-
-  response.add("usage: /stats?format=json  or /stats?format=prometheus \n");
-  response.add("\n");
-  return Http::Code::NotFound;
+  response_headers.setReferenceContentType(Http::Headers::get().ContentTypeValues.Json);
+  response.add(statsAsJson(all_stats, text_readouts, histograms, used_only, regex));
+  return Http::Code::OK;
 }
 
 Http::Code StatsHandler::handlerPrometheusStats(absl::string_view path_and_query,
diff --git a/source/server/admin/stats_handler.h b/source/server/admin/stats_handler.h
index 796bd35bc8ff..d9521bd1fe7f 100644
--- a/source/server/admin/stats_handler.h
+++ b/source/server/admin/stats_handler.h
@@ -40,6 +40,11 @@ class StatsHandler : public HandlerContextBase {
   Http::Code handlerStats(absl::string_view path_and_query,
                           Http::ResponseHeaderMap& response_headers, Buffer::Instance& response,
                           AdminStream&);
+  static Http::Code handlerStats(Stats::Store& stats, bool used_only, bool json,
+                                 const absl::optional<std::regex>& filter,
+                                 Http::ResponseHeaderMap& response_headers,
+                                 Buffer::Instance& response);
+
   Http::Code handlerPrometheusStats(absl::string_view path_and_query,
                                     Http::ResponseHeaderMap& response_headers,
                                     Buffer::Instance& response, AdminStream&);
@@ -63,11 +68,11 @@ class StatsHandler : public HandlerContextBase {
                                  bool used_only, const absl::optional<std::regex>& regex,
                                  bool pretty_print = false);
 
-  void statsAsText(const std::map<std::string, uint64_t>& all_stats,
-                   const std::map<std::string, std::string>& text_readouts,
-                   const std::vector<Stats::ParentHistogramSharedPtr>& all_histograms,
-                   bool used_only, const absl::optional<std::regex>& regex,
-                   Buffer::Instance& response);
+  static void statsAsText(const std::map<std::string, uint64_t>& all_stats,
+                          const std::map<std::string, std::string>& text_readouts,
+                          const std::vector<Stats::ParentHistogramSharedPtr>& all_histograms,
+                          bool used_only, const absl::optional<std::regex>& regex,
+                          Buffer::Instance& response);
 };
 
 } // namespace Server
diff --git a/test/per_file_coverage.sh b/test/per_file_coverage.sh
index 404fbe995a9b..5bc6559ab9da 100755
--- a/test/per_file_coverage.sh
+++ b/test/per_file_coverage.sh
@@ -82,7 +82,7 @@ declare -a KNOWN_LOW_COVERAGE=(
 "source/extensions/watchdog:83.3" # Death tests within extensions
 "source/extensions/watchdog/profile_action:83.3"
 "source/server:93.3" # flaky: be careful adjusting. See https://github.com/envoyproxy/envoy/issues/15239
-"source/server/admin:95.3"
+"source/server/admin:97.6"
 "source/server/config_validation:74.8"
 )
 
diff --git a/test/server/admin/BUILD b/test/server/admin/BUILD
index 9998c58138dc..9f1747ecebbd 100644
--- a/test/server/admin/BUILD
+++ b/test/server/admin/BUILD
@@ -1,5 +1,6 @@
 load(
     "//bazel:envoy_build_system.bzl",
+    "envoy_cc_benchmark_binary",
     "envoy_cc_test",
     "envoy_cc_test_library",
     "envoy_package",
@@ -68,6 +69,18 @@ envoy_cc_test(
     ],
 )
 
+envoy_cc_benchmark_binary(
+    name = "stats_handler_speed_test",
+    srcs = ["stats_handler_speed_test.cc"],
+    deps = [
+        "//source/common/buffer:buffer_lib",
+        "//source/common/http:header_map_lib",
+        "//source/common/stats:thread_local_store_lib",
+        "//source/server/admin:stats_handler_lib",
+        "//test/mocks/server:admin_stream_mocks",
+    ],
+)
+
 envoy_cc_test(
     name = "runtime_handler_test",
     srcs = ["runtime_handler_test.cc"],
diff --git a/test/server/admin/admin_test.cc b/test/server/admin/admin_test.cc
index d9086e45bff4..14f9fd908030 100644
--- a/test/server/admin/admin_test.cc
+++ b/test/server/admin/admin_test.cc
@@ -66,7 +66,7 @@ TEST_P(AdminInstanceTest, WriteAddressToFile) {
 }
 
 TEST_P(AdminInstanceTest, AdminAddress) {
-  std::string address_out_path = TestEnvironment::temporaryPath("admin.address");
+  const std::string address_out_path = TestEnvironment::temporaryPath("admin.address");
   AdminImpl admin_address_out_path(cpu_profile_path_, server_, false);
   std::list<AccessLog::InstanceSharedPtr> access_logs;
   Filesystem::FilePathAndType file_info{Filesystem::DestinationType::File, "/dev/null"};
@@ -81,7 +81,8 @@ TEST_P(AdminInstanceTest, AdminAddress) {
 }
 
 TEST_P(AdminInstanceTest, AdminBadAddressOutPath) {
-  std::string bad_path = TestEnvironment::temporaryPath("some/unlikely/bad/path/admin.address");
+  const std::string bad_path =
+      TestEnvironment::temporaryPath("some/unlikely/bad/path/admin.address");
   AdminImpl admin_bad_address_out_path(cpu_profile_path_, server_, false);
   std::list<AccessLog::InstanceSharedPtr> access_logs;
   Filesystem::FilePathAndType file_info{Filesystem::DestinationType::File, "/dev/null"};
@@ -220,5 +221,68 @@ TEST_P(AdminInstanceTest, HelpUsesFormForMutations) {
   EXPECT_NE(-1, response.search(stats_href.data(), stats_href.size(), 0, 0));
 }
 
+class AdminTestingPeer {
+public:
+  AdminTestingPeer(AdminImpl& admin)
+      : admin_(admin), server_(admin.server_), listener_scope_(server_.stats().createScope("")) {}
+  AdminImpl::NullRouteConfigProvider& routeConfigProvider() { return route_config_provider_; }
+  AdminImpl::NullScopedRouteConfigProvider& scopedRouteConfigProvider() {
+    return scoped_route_config_provider_;
+  }
+  AdminImpl::NullOverloadManager& overloadManager() { return admin_.null_overload_manager_; }
+  AdminImpl::NullOverloadManager::OverloadState& overloadState() { return overload_state_; }
+  AdminImpl::AdminListenSocketFactory& socketFactory() { return socket_factory_; }
+  AdminImpl::AdminListener& listener() { return listener_; }
+
+private:
+  AdminImpl& admin_;
+  Server::Instance& server_;
+  AdminImpl::NullRouteConfigProvider route_config_provider_{server_.timeSource()};
+  AdminImpl::NullScopedRouteConfigProvider scoped_route_config_provider_{server_.timeSource()};
+  AdminImpl::NullOverloadManager::OverloadState overload_state_{server_.dispatcher()};
+  AdminImpl::AdminListenSocketFactory socket_factory_{nullptr};
+  Stats::ScopeSharedPtr listener_scope_;
+  AdminImpl::AdminListener listener_{admin_, std::move(listener_scope_)};
+};
+
+// Covers override methods for admin-specific implementations of classes in
+// admin.h, reducing a major source of reduced coverage-percent expectations in
+// source/server/admin. There remain a few uncovered lines that are somewhat
+// harder to construct tests for.
+TEST_P(AdminInstanceTest, Overrides) {
+  admin_.http1Settings();
+  admin_.originalIpDetectionExtensions();
+
+  AdminTestingPeer peer(admin_);
+
+  peer.routeConfigProvider().config();
+  peer.routeConfigProvider().configInfo();
+  peer.routeConfigProvider().lastUpdated();
+  peer.routeConfigProvider().onConfigUpdate();
+
+  peer.scopedRouteConfigProvider().lastUpdated();
+  peer.scopedRouteConfigProvider().getConfigProto();
+  peer.scopedRouteConfigProvider().getConfigVersion();
+  peer.scopedRouteConfigProvider().getConfig();
+  peer.scopedRouteConfigProvider().apiType();
+  peer.scopedRouteConfigProvider().getConfigProtos();
+
+  auto overload_name = Server::OverloadProactiveResourceName::GlobalDownstreamMaxConnections;
+  peer.overloadState().tryAllocateResource(overload_name, 0);
+  peer.overloadState().tryDeallocateResource(overload_name, 0);
+  peer.overloadState().isResourceMonitorEnabled(overload_name);
+
+  peer.overloadManager().scaledTimerFactory();
+
+  peer.socketFactory().clone();
+  peer.socketFactory().closeAllSockets();
+  peer.socketFactory().doFinalPreWorkerInit();
+
+  peer.listener().name();
+  peer.listener().udpListenerConfig();
+  peer.listener().direction();
+  peer.listener().tcpBacklogSize();
+}
+
 } // namespace Server
 } // namespace Envoy
diff --git a/test/server/admin/stats_handler_speed_test.cc b/test/server/admin/stats_handler_speed_test.cc
new file mode 100644
index 000000000000..3a6669ebb770
--- /dev/null
+++ b/test/server/admin/stats_handler_speed_test.cc
@@ -0,0 +1,125 @@
+// Note: this should be run with --compilation_mode=opt, and would benefit from a
+// quiescent system with disabled cstate power management.
+
+#include "source/common/buffer/buffer_impl.h"
+#include "source/common/http/header_map_impl.h"
+#include "source/common/stats/thread_local_store.h"
+#include "source/server/admin/stats_handler.h"
+
+#include "test/mocks/server/admin_stream.h"
+
+#include "benchmark/benchmark.h"
+
+namespace Envoy {
+namespace Server {
+
+class StatsHandlerTest {
+public:
+  StatsHandlerTest() : alloc_(symbol_table_), store_(alloc_) {
+    // Benchmark will be 10k clusters each with 100 counters, with 100+
+    // character names. The first counter in each scope will be given a value so
+    // it will be included in 'usedonly'.
+    std::string prefix(100, 'a');
+    for (uint32_t s = 0; s < 10000; ++s) {
+      Stats::ScopeSharedPtr scope = store_.createScope(absl::StrCat("scope_", s));
+      scopes_.emplace_back(scope);
+      for (uint32_t c = 0; c < 100; ++c) {
+        Stats::Counter& counter = scope->counterFromString(absl::StrCat(prefix, "_", c));
+        if (c == 0) {
+          counter.inc();
+        }
+      }
+    }
+  }
+
+  /**
+   * Issues an admin request against the stats saved in store_.
+   *
+   * @param path the admin endpoint to query.
+   * @return the Http Code and the response body as a string.
+   */
+  uint64_t handlerStats(bool used_only, bool json, const absl::optional<std::regex>& filter) {
+    Buffer::OwnedImpl data;
+    auto response_headers = Http::ResponseHeaderMapImpl::create();
+    StatsHandler::handlerStats(store_, used_only, json, filter, *response_headers, data);
+    return data.length();
+  }
+
+  Stats::SymbolTableImpl symbol_table_;
+  Stats::AllocatorImpl alloc_;
+  Stats::ThreadLocalStoreImpl store_;
+  std::vector<Stats::ScopeSharedPtr> scopes_;
+};
+
+} // namespace Server
+} // namespace Envoy
+
+Envoy::Server::StatsHandlerTest& testContext() {
+  MUTABLE_CONSTRUCT_ON_FIRST_USE(Envoy::Server::StatsHandlerTest);
+}
+
+// NOLINTNEXTLINE(readability-identifier-naming)
+static void BM_AllCountersText(benchmark::State& state) {
+  Envoy::Server::StatsHandlerTest& test_context = testContext();
+  for (auto _ : state) { // NOLINT
+    uint64_t count = test_context.handlerStats(false, false, absl::nullopt);
+    RELEASE_ASSERT(count > 100 * 1000 * 1000, "expected count > 100M"); // actual = 117,789,000
+  }
+}
+BENCHMARK(BM_AllCountersText)->Unit(benchmark::kMillisecond);
+
+// NOLINTNEXTLINE(readability-identifier-naming)
+static void BM_UsedCountersText(benchmark::State& state) {
+  Envoy::Server::StatsHandlerTest& test_context = testContext();
+  for (auto _ : state) { // NOLINT
+    uint64_t count = test_context.handlerStats(true, false, absl::nullopt);
+    RELEASE_ASSERT(count > 1000 * 1000, "expected count > 1M");
+    RELEASE_ASSERT(count < 2 * 1000 * 1000, "expected count < 2M"); // actual = 1,168,890
+  }
+}
+BENCHMARK(BM_UsedCountersText)->Unit(benchmark::kMillisecond);
+
+// NOLINTNEXTLINE(readability-identifier-naming)
+static void BM_FilteredCountersText(benchmark::State& state) {
+  Envoy::Server::StatsHandlerTest& test_context = testContext();
+  absl::optional<std::regex> filter(std::regex("no-match"));
+
+  for (auto _ : state) { // NOLINT
+    uint64_t count = test_context.handlerStats(false, false, filter);
+    RELEASE_ASSERT(count == 0, "expected count == 0");
+  }
+}
+BENCHMARK(BM_FilteredCountersText)->Unit(benchmark::kMillisecond);
+
+// NOLINTNEXTLINE(readability-identifier-naming)
+static void BM_AllCountersJson(benchmark::State& state) {
+  Envoy::Server::StatsHandlerTest& test_context = testContext();
+  for (auto _ : state) { // NOLINT
+    uint64_t count = test_context.handlerStats(false, true, absl::nullopt);
+    RELEASE_ASSERT(count > 130 * 1000 * 1000, "expected count > 1130M"); // actual = 135,789,011
+  }
+}
+BENCHMARK(BM_AllCountersJson)->Unit(benchmark::kMillisecond);
+
+// NOLINTNEXTLINE(readability-identifier-naming)
+static void BM_UsedCountersJson(benchmark::State& state) {
+  Envoy::Server::StatsHandlerTest& test_context = testContext();
+  for (auto _ : state) { // NOLINT
+    uint64_t count = test_context.handlerStats(true, true, absl::nullopt);
+    RELEASE_ASSERT(count > 1000 * 1000, "expected count > 1M");
+    RELEASE_ASSERT(count < 2 * 1000 * 1000, "expected count < 2M"); // actual = 1,348,901
+  }
+}
+BENCHMARK(BM_UsedCountersJson)->Unit(benchmark::kMillisecond);
+
+// NOLINTNEXTLINE(readability-identifier-naming)
+static void BM_FilteredCountersJson(benchmark::State& state) {
+  Envoy::Server::StatsHandlerTest& test_context = testContext();
+  absl::optional<std::regex> filter(std::regex("no-match"));
+
+  for (auto _ : state) { // NOLINT
+    uint64_t count = test_context.handlerStats(false, true, filter);
+    RELEASE_ASSERT(count < 100, "expected count < 100"); // actual = 12
+  }
+}
+BENCHMARK(BM_FilteredCountersJson)->Unit(benchmark::kMillisecond);
diff --git a/test/server/admin/stats_handler_test.cc b/test/server/admin/stats_handler_test.cc
index 209cf5523280..33d00bd8eec4 100644
--- a/test/server/admin/stats_handler_test.cc
+++ b/test/server/admin/stats_handler_test.cc
@@ -120,7 +120,7 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, AdminStatsTest,
 
 TEST_P(AdminStatsTest, HandlerStatsInvalidFormat) {
   const std::string url = "/stats?format=blergh";
-  CodeResponse code_response(handlerStats(url));
+  const CodeResponse code_response(handlerStats(url));
   EXPECT_EQ(Http::Code::NotFound, code_response.first);
   EXPECT_EQ("usage: /stats?format=json  or /stats?format=prometheus \n\n", code_response.second);
 }
@@ -186,7 +186,7 @@ TEST_P(AdminStatsTest, HandlerStatsJson) {
 
   store_->mergeHistograms([]() -> void {});
 
-  CodeResponse code_response = handlerStats(url);
+  const CodeResponse code_response = handlerStats(url);
   EXPECT_EQ(Http::Code::OK, code_response.first);
 
   const std::string expected_json_old = R"EOF({
@@ -710,7 +710,7 @@ TEST_P(AdminStatsTest, SortedCountersAndGauges) {
   store_->counterFromString("s1");
   store_->gaugeFromString("s2", Stats::Gauge::ImportMode::Accumulate);
   for (const std::string& url : {"/stats", "/stats?format=json"}) {
-    CodeResponse code_response = handlerStats(url);
+    const CodeResponse code_response = handlerStats(url);
     ASSERT_EQ(Http::Code::OK, code_response.first);
     checkOrder(code_response.second, {"s1", "s2", "s3", "s4"});
   }
@@ -723,7 +723,7 @@ TEST_P(AdminStatsTest, SortedTextReadouts) {
   store_->textReadoutFromString("t1");
   store_->textReadoutFromString("t2");
   for (const std::string& url : {"/stats", "/stats?format=json"}) {
-    CodeResponse code_response = handlerStats(url);
+    const CodeResponse code_response = handlerStats(url);
     ASSERT_EQ(Http::Code::OK, code_response.first);
     checkOrder(code_response.second, {"t1", "t2", "t3", "t4"});
   }
@@ -735,7 +735,7 @@ TEST_P(AdminStatsTest, SortedHistograms) {
   store_->histogramFromString("h1", Stats::Histogram::Unit::Unspecified);
   store_->histogramFromString("h2", Stats::Histogram::Unit::Unspecified);
   for (const std::string& url : {"/stats", "/stats?format=json"}) {
-    CodeResponse code_response = handlerStats(url);
+    const CodeResponse code_response = handlerStats(url);
     ASSERT_EQ(Http::Code::OK, code_response.first);
     checkOrder(code_response.second, {"h1", "h2", "h3", "h4"});
   }
@@ -800,8 +800,17 @@ TEST_P(AdminInstanceTest, RecentLookups) {
   EXPECT_THAT(body, HasSubstr("Lookup tracking is not enabled"));
   EXPECT_THAT(std::string(response_headers.getContentTypeValue()), HasSubstr("text/plain"));
 
-  // We can't test RecentLookups in admin unit tests as it doesn't work with a
-  // fake symbol table. However we cover this solidly in integration tests.
+  EXPECT_EQ(Http::Code::OK,
+            admin_.request("/stats/recentlookups/enable", "POST", response_headers, body));
+  Stats::StatNamePool pool(server_.stats().symbolTable());
+  pool.add("alpha");
+  pool.add("beta");
+  pool.add("gamma");
+  pool.add("alpha");
+  pool.add("beta");
+  pool.add("alpha");
+  EXPECT_EQ(Http::Code::OK, admin_.request("/stats/recentlookups", "GET", response_headers, body));
+  EXPECT_THAT(body, HasSubstr("       1 gamma\n       2 beta\n       3 alpha\n"));
 }
 
 class StatsHandlerPrometheusTest : public StatsHandlerTest {
@@ -839,7 +848,7 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, StatsHandlerPrometheusDefaultTest,
                          TestUtility::ipTestParamsToString);
 
 TEST_P(StatsHandlerPrometheusDefaultTest, StatsHandlerPrometheusDefaultTest) {
-  std::string url = "/stats?format=prometheus";
+  const std::string url = "/stats?format=prometheus";
 
   createTestStats();
 
@@ -853,9 +862,19 @@ envoy_cluster_upstream_cx_active{cluster="c2"} 12
 
 )EOF";
 
-  CodeResponse code_response = handlerStats(url);
+  const CodeResponse code_response = handlerStats(url);
   EXPECT_EQ(Http::Code::OK, code_response.first);
-  EXPECT_THAT(expected_response, code_response.second);
+  EXPECT_EQ(expected_response, code_response.second);
+}
+
+TEST_P(StatsHandlerPrometheusDefaultTest, StatsHandlerPrometheusInvalidRegex) {
+  const std::string url = "/stats?format=prometheus&filter=(+invalid)";
+
+  createTestStats();
+
+  const CodeResponse code_response = handlerStats(url);
+  EXPECT_EQ(Http::Code::BadRequest, code_response.first);
+  EXPECT_THAT(code_response.second, HasSubstr("Invalid regex"));
 }
 
 class StatsHandlerPrometheusWithTextReadoutsTest
@@ -871,7 +890,7 @@ INSTANTIATE_TEST_SUITE_P(
                                      "/stats?format=prometheus&text_readouts=abc")));
 
 TEST_P(StatsHandlerPrometheusWithTextReadoutsTest, StatsHandlerPrometheusWithTextReadoutsTest) {
-  std::string url = std::get<1>(GetParam());
+  const std::string url = std::get<1>(GetParam());
 
   createTestStats();
 
@@ -888,7 +907,7 @@ envoy_control_plane_identifier{cluster="c1",text_value="cp-1"} 0
 
 )EOF";
 
-  CodeResponse code_response = handlerStats(url);
+  const CodeResponse code_response = handlerStats(url);
   EXPECT_EQ(Http::Code::OK, code_response.first);
   EXPECT_THAT(expected_response, code_response.second);
 }

From fdec21589cf8e9ecc089068ebf556490938da78c Mon Sep 17 00:00:00 2001
From: alyssawilk <alyssar@chromium.org>
Date: Wed, 2 Mar 2022 01:48:12 -0500
Subject: [PATCH 06/14] test: removing a bunch of direct runtime singleton
 access (#19993)

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>
---
 test/benchmark/main.cc                        |  7 ++----
 test/common/common/dns_utils_test.cc          |  9 +++----
 test/common/common/regex_test.cc              |  9 +++----
 .../delta_subscription_state_old_test.cc      |  4 +--
 .../config/subscription_factory_impl_test.cc  |  3 +--
 test/common/event/dispatcher_impl_test.cc     |  3 +--
 test/common/event/file_event_impl_test.cc     |  2 --
 .../grpc/async_client_manager_impl_test.cc    |  2 +-
 test/common/http/conn_manager_utility_test.cc | 15 +++++------
 test/common/http/header_map_impl_fuzz_test.cc |  7 +++---
 test/common/http/header_map_impl_test.cc      |  4 +--
 test/common/http/http1/codec_impl_test.cc     |  6 -----
 test/common/http/http2/conn_pool_test.cc      |  2 --
 test/common/network/connection_impl_test.cc   |  5 ++--
 test/common/network/listener_impl_test.cc     | 15 ++++-------
 test/common/protobuf/utility_test.cc          | 25 ++++++++-----------
 test/common/router/router_test.cc             |  4 +--
 .../upstream/cluster_manager_impl_test.cc     |  3 +--
 test/common/upstream/eds_speed_test.cc        |  2 +-
 test/common/upstream/eds_test.cc              |  8 +++---
 .../dns_cache_impl_test.cc                    |  6 ++---
 .../filters/http/buffer/buffer_filter_test.cc |  3 ---
 .../filters/http/ext_authz/ext_authz_test.cc  |  2 +-
 .../filters/http/wasm/wasm_filter_test.cc     | 12 ---------
 .../network/common/redis/fault_test.cc        |  5 ----
 test/server/connection_handler_test.cc        |  4 +--
 test/server/listener_manager_impl_test.cc     | 20 ++++++---------
 .../test_common/simulated_time_system_test.cc |  2 --
 test/test_common/test_runtime.h               | 12 +++++----
 29 files changed, 68 insertions(+), 133 deletions(-)

diff --git a/test/benchmark/main.cc b/test/benchmark/main.cc
index 69c117d12bbf..17780ea14d20 100644
--- a/test/benchmark/main.cc
+++ b/test/benchmark/main.cc
@@ -82,15 +82,12 @@ int main(int argc, char** argv) {
 
   skip_expensive_benchmarks = skip_switch.getValue();
 
+  TestScopedRuntime runtime;
   // Initialize scoped_runtime if a runtime_feature argument is present. This
   // allows benchmarks to use their own scoped_runtime in case no runtime flag is
   // passed as an argument.
-  std::unique_ptr<TestScopedRuntime> scoped_runtime = nullptr;
   const auto& runtime_features_args = runtime_features.getValue();
   for (const absl::string_view runtime_feature_arg : runtime_features_args) {
-    if (scoped_runtime == nullptr) {
-      scoped_runtime = std::make_unique<TestScopedRuntime>();
-    }
     // Make sure the argument contains a single ":" character.
     const std::vector<std::string> runtime_feature_split = absl::StrSplit(runtime_feature_arg, ':');
     if (runtime_feature_split.size() != 2) {
@@ -102,7 +99,7 @@ int main(int argc, char** argv) {
     }
     const auto feature_name = runtime_feature_split[0];
     const auto feature_val = runtime_feature_split[1];
-    Runtime::LoaderSingleton::getExisting()->mergeValues({{feature_name, feature_val}});
+    runtime.mergeValues({{feature_name, feature_val}});
   }
 
   if (skip_expensive_benchmarks) {
diff --git a/test/common/common/dns_utils_test.cc b/test/common/common/dns_utils_test.cc
index 3ea82592e71b..784a817fc17e 100644
--- a/test/common/common/dns_utils_test.cc
+++ b/test/common/common/dns_utils_test.cc
@@ -11,8 +11,7 @@ namespace {
 
 TEST(DnsUtils, LegacyGenerateTest) {
   TestScopedRuntime scoped_runtime;
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.reloadable_features.allow_multiple_dns_addresses", "false"}});
+  scoped_runtime.mergeValues({{"envoy.reloadable_features.allow_multiple_dns_addresses", "false"}});
 
   std::list<Network::DnsResponse> responses =
       TestUtility::makeDnsResponse({"10.0.0.1", "10.0.0.2"});
@@ -23,8 +22,7 @@ TEST(DnsUtils, LegacyGenerateTest) {
 
 TEST(DnsUtils, MultipleGenerateTest) {
   TestScopedRuntime scoped_runtime;
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.reloadable_features.allow_multiple_dns_addresses", "true"}});
+  scoped_runtime.mergeValues({{"envoy.reloadable_features.allow_multiple_dns_addresses", "true"}});
 
   std::list<Network::DnsResponse> responses =
       TestUtility::makeDnsResponse({"10.0.0.1", "10.0.0.2"});
@@ -37,8 +35,7 @@ TEST(DnsUtils, MultipleGenerateTest) {
 
 TEST(DnsUtils, ListChanged) {
   TestScopedRuntime scoped_runtime;
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.reloadable_features.allow_multiple_dns_addresses", "true"}});
+  scoped_runtime.mergeValues({{"envoy.reloadable_features.allow_multiple_dns_addresses", "true"}});
 
   Network::Address::InstanceConstSharedPtr address1 =
       Network::Utility::parseInternetAddress("10.0.0.1");
diff --git a/test/common/common/regex_test.cc b/test/common/common/regex_test.cc
index b6d36c3cd44d..7330b1050c79 100644
--- a/test/common/common/regex_test.cc
+++ b/test/common/common/regex_test.cc
@@ -73,8 +73,7 @@ TEST(Utility, ParseRegex) {
   // The deprecated field codepath precedes any runtime settings.
   {
     TestScopedRuntime scoped_runtime;
-    Runtime::LoaderSingleton::getExisting()->mergeValues(
-        {{"re2.max_program_size.error_level", "3"}});
+    scoped_runtime.mergeValues({{"re2.max_program_size.error_level", "3"}});
     envoy::type::matcher::v3::RegexMatcher matcher;
     matcher.set_regex("/asdf/.*");
     matcher.mutable_google_re2()->mutable_max_program_size()->set_value(1);
@@ -90,8 +89,7 @@ TEST(Utility, ParseRegex) {
   // Verify that an exception is thrown for the error level max program size.
   {
     TestScopedRuntime scoped_runtime;
-    Runtime::LoaderSingleton::getExisting()->mergeValues(
-        {{"re2.max_program_size.error_level", "1"}});
+    scoped_runtime.mergeValues({{"re2.max_program_size.error_level", "1"}});
     envoy::type::matcher::v3::RegexMatcher matcher;
     matcher.set_regex("/asdf/.*");
     matcher.mutable_google_re2();
@@ -127,8 +125,7 @@ TEST(Utility, ParseRegex) {
   // Verify that a warning is logged for the warn level max program size.
   {
     TestScopedRuntime scoped_runtime;
-    Runtime::LoaderSingleton::getExisting()->mergeValues(
-        {{"re2.max_program_size.warn_level", "1"}});
+    scoped_runtime.mergeValues({{"re2.max_program_size.warn_level", "1"}});
     envoy::type::matcher::v3::RegexMatcher matcher;
     matcher.set_regex("/asdf/.*");
     matcher.mutable_google_re2();
diff --git a/test/common/config/delta_subscription_state_old_test.cc b/test/common/config/delta_subscription_state_old_test.cc
index 8e61015e7130..fa564c6ca21a 100644
--- a/test/common/config/delta_subscription_state_old_test.cc
+++ b/test/common/config/delta_subscription_state_old_test.cc
@@ -38,8 +38,8 @@ class OldDeltaSubscriptionStateTestBase : public testing::Test {
     // Disable the explicit wildcard resource feature, so OldDeltaSubscriptionState will be picked
     // up.
     {
-      TestScopedRuntime scoped_runtime_;
-      Runtime::LoaderSingleton::getExisting()->mergeValues({
+      TestScopedRuntime scoped_runtime;
+      scoped_runtime.mergeValues({
           {"envoy.restart_features.explicit_wildcard_resource", "false"},
       });
       state_ = std::make_unique<Envoy::Config::DeltaSubscriptionState>(type_url, callbacks_,
diff --git a/test/common/config/subscription_factory_impl_test.cc b/test/common/config/subscription_factory_impl_test.cc
index c27f407b1a7a..334b39c5805d 100644
--- a/test/common/config/subscription_factory_impl_test.cc
+++ b/test/common/config/subscription_factory_impl_test.cc
@@ -80,8 +80,7 @@ class SubscriptionFactoryTestUnifiedOrLegacyMux
 public:
   SubscriptionFactoryTestUnifiedOrLegacyMux() {
     if (GetParam() == LegacyOrUnified::Unified) {
-      Runtime::LoaderSingleton::getExisting()->mergeValues(
-          {{"envoy.reloadable_features.unified_mux", "true"}});
+      scoped_runtime_.mergeValues({{"envoy.reloadable_features.unified_mux", "true"}});
     }
   }
 
diff --git a/test/common/event/dispatcher_impl_test.cc b/test/common/event/dispatcher_impl_test.cc
index 7538a8c495c0..a7c84bb3a588 100644
--- a/test/common/event/dispatcher_impl_test.cc
+++ b/test/common/event/dispatcher_impl_test.cc
@@ -14,10 +14,10 @@
 #include "source/common/stats/isolated_store_impl.h"
 
 #include "test/mocks/common.h"
+#include "test/mocks/event/mocks.h"
 #include "test/mocks/server/watch_dog.h"
 #include "test/mocks/stats/mocks.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"
@@ -907,7 +907,6 @@ class TimerImplTest : public testing::Test {
     requested_advance_ = absl::ZeroDuration();
   }
 
-  TestScopedRuntime scoped_runtime_;
   absl::Duration requested_advance_ = absl::ZeroDuration();
   std::vector<std::function<void()>> check_callbacks_;
   bool in_event_loop_{};
diff --git a/test/common/event/file_event_impl_test.cc b/test/common/event/file_event_impl_test.cc
index d3a842ada010..0f1c7194eb72 100644
--- a/test/common/event/file_event_impl_test.cc
+++ b/test/common/event/file_event_impl_test.cc
@@ -8,7 +8,6 @@
 
 #include "test/mocks/common.h"
 #include "test/test_common/environment.h"
-#include "test/test_common/test_runtime.h"
 #include "test/test_common/utility.h"
 
 #include "gtest/gtest.h"
@@ -72,7 +71,6 @@ class FileEventImplActivateTest : public testing::TestWithParam<Network::Address
 
 protected:
   Api::OsSysCalls& os_sys_calls_;
-  TestScopedRuntime scoped_runtime_;
 };
 
 INSTANTIATE_TEST_SUITE_P(IpVersions, FileEventImplActivateTest,
diff --git a/test/common/grpc/async_client_manager_impl_test.cc b/test/common/grpc/async_client_manager_impl_test.cc
index c3ffb0d79572..9d01bdbbcdc0 100644
--- a/test/common/grpc/async_client_manager_impl_test.cc
+++ b/test/common/grpc/async_client_manager_impl_test.cc
@@ -129,7 +129,7 @@ TEST_F(AsyncClientManagerImplTest, EnvoyGrpcOk) {
 
 TEST_F(AsyncClientManagerImplTest, DisableRawAsyncClientCache) {
   TestScopedRuntime scoped_runtime;
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
+  scoped_runtime.mergeValues(
       {{"envoy.reloadable_features.enable_grpc_async_client_cache", "false"}});
   envoy::config::core::v3::GrpcService grpc_service;
   grpc_service.mutable_envoy_grpc()->set_cluster_name("foo");
diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc
index b96f2056d1d2..4ccaddffb23a 100644
--- a/test/common/http/conn_manager_utility_test.cc
+++ b/test/common/http/conn_manager_utility_test.cc
@@ -240,8 +240,7 @@ TEST_F(ConnectionManagerUtilityTest, UseRemoteAddressWhenNotLocalHostRemoteAddre
 
 TEST_F(ConnectionManagerUtilityTest, RemoveRefererIfUrlInvalid) {
   TestScopedRuntime scoped_runtime;
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.reloadable_features.sanitize_http_header_referer", "true"}});
+  scoped_runtime.mergeValues({{"envoy.reloadable_features.sanitize_http_header_referer", "true"}});
 
   connection_.stream_info_.downstream_connection_info_provider_->setRemoteAddress(
       std::make_shared<Network::Address::Ipv4Instance>("10.0.0.1"));
@@ -254,8 +253,7 @@ TEST_F(ConnectionManagerUtilityTest, RemoveRefererIfUrlInvalid) {
 
 TEST_F(ConnectionManagerUtilityTest, RemoveRefererIfMultipleEntriesAreFound) {
   TestScopedRuntime scoped_runtime;
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.reloadable_features.sanitize_http_header_referer", "true"}});
+  scoped_runtime.mergeValues({{"envoy.reloadable_features.sanitize_http_header_referer", "true"}});
 
   connection_.stream_info_.downstream_connection_info_provider_->setRemoteAddress(
       std::make_shared<Network::Address::Ipv4Instance>("10.0.0.1"));
@@ -269,8 +267,7 @@ TEST_F(ConnectionManagerUtilityTest, RemoveRefererIfMultipleEntriesAreFound) {
 
 TEST_F(ConnectionManagerUtilityTest, ValidRefererPassesSanitization) {
   TestScopedRuntime scoped_runtime;
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.reloadable_features.sanitize_http_header_referer", "true"}});
+  scoped_runtime.mergeValues({{"envoy.reloadable_features.sanitize_http_header_referer", "true"}});
 
   connection_.stream_info_.downstream_connection_info_provider_->setRemoteAddress(
       std::make_shared<Network::Address::Ipv4Instance>("10.0.0.1"));
@@ -1970,7 +1967,7 @@ TEST_F(ConnectionManagerUtilityTest, RejectPathWithFragmentByDefault) {
 
 TEST_F(ConnectionManagerUtilityTest, DropFragmentFromPathWithOverride) {
   TestScopedRuntime scoped_runtime;
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
+  scoped_runtime.mergeValues(
       {{"envoy.reloadable_features.http_reject_path_with_fragment", "false"}});
 
   TestRequestHeaderMapImpl header_map{{":path", "/foo/bar#boom"}};
@@ -2003,9 +2000,9 @@ TEST_F(ConnectionManagerUtilityTest, DropFragmentFromPathWithOverride) {
 
 TEST_F(ConnectionManagerUtilityTest, KeepFragmentFromPathWithBothOverrides) {
   TestScopedRuntime scoped_runtime;
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
+  scoped_runtime.mergeValues(
       {{"envoy.reloadable_features.http_reject_path_with_fragment", "false"}});
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
+  scoped_runtime.mergeValues(
       {{"envoy.reloadable_features.http_strip_fragment_from_path_unsafe_if_disabled", "false"}});
 
   TestRequestHeaderMapImpl header_map{{":path", "/foo/bar#boom"}};
diff --git a/test/common/http/header_map_impl_fuzz_test.cc b/test/common/http/header_map_impl_fuzz_test.cc
index d769eb225879..62fc452a420d 100644
--- a/test/common/http/header_map_impl_fuzz_test.cc
+++ b/test/common/http/header_map_impl_fuzz_test.cc
@@ -17,12 +17,11 @@ namespace Envoy {
 
 // Fuzz the header map implementation.
 DEFINE_PROTO_FUZZER(const test::common::http::HeaderMapImplFuzzTestCase& input) {
-  TestScopedRuntime runtime;
+  TestScopedRuntime scoped_runtime;
   // Set the lazy header-map threshold if found.
   if (input.has_config()) {
-    Runtime::LoaderSingleton::getExisting()->mergeValues(
-        {{"envoy.http.headermap.lazy_map_min_size",
-          absl::StrCat(input.config().lazy_map_min_size())}});
+    scoped_runtime.mergeValues({{"envoy.http.headermap.lazy_map_min_size",
+                                 absl::StrCat(input.config().lazy_map_min_size())}});
   }
 
   auto header_map = Http::RequestHeaderMapImpl::create();
diff --git a/test/common/http/header_map_impl_test.cc b/test/common/http/header_map_impl_test.cc
index 1b95f4b49ad1..57561ec98ead 100644
--- a/test/common/http/header_map_impl_test.cc
+++ b/test/common/http/header_map_impl_test.cc
@@ -372,7 +372,7 @@ class HeaderMapImplTest : public testing::TestWithParam<uint32_t> {
 public:
   HeaderMapImplTest() {
     // Set the lazy map threshold using the test parameter.
-    Runtime::LoaderSingleton::getExisting()->mergeValues(
+    scoped_runtime_.mergeValues(
         {{"envoy.http.headermap.lazy_map_min_size", absl::StrCat(GetParam())}});
   }
 
@@ -380,7 +380,7 @@ class HeaderMapImplTest : public testing::TestWithParam<uint32_t> {
     return absl::StrCat(params.param);
   }
 
-  TestScopedRuntime runtime;
+  TestScopedRuntime scoped_runtime_;
 };
 
 INSTANTIATE_TEST_SUITE_P(HeaderMapThreshold, HeaderMapImplTest,
diff --git a/test/common/http/http1/codec_impl_test.cc b/test/common/http/http1/codec_impl_test.cc
index 825b6431c335..36726e28f325 100644
--- a/test/common/http/http1/codec_impl_test.cc
+++ b/test/common/http/http1/codec_impl_test.cc
@@ -1118,10 +1118,6 @@ TEST_F(Http1ServerConnectionImplTest, HostHeaderTranslation) {
 // Ensures that requests with invalid HTTP header values are properly rejected
 // when the runtime guard is enabled for the feature.
 TEST_F(Http1ServerConnectionImplTest, HeaderInvalidCharsRejection) {
-  TestScopedRuntime scoped_runtime;
-  // When the runtime-guarded feature is enabled, invalid header values
-  // should result in a rejection.
-
   initialize();
 
   MockRequestDecoder decoder;
@@ -1211,8 +1207,6 @@ TEST_F(Http1ServerConnectionImplTest, HeaderNameWithUnderscoreCauseRequestReject
 }
 
 TEST_F(Http1ServerConnectionImplTest, HeaderInvalidAuthority) {
-  TestScopedRuntime scoped_runtime;
-
   initialize();
 
   MockRequestDecoder decoder;
diff --git a/test/common/http/http2/conn_pool_test.cc b/test/common/http/http2/conn_pool_test.cc
index 9e186ed3e924..97baf19266e6 100644
--- a/test/common/http/http2/conn_pool_test.cc
+++ b/test/common/http/http2/conn_pool_test.cc
@@ -1443,7 +1443,6 @@ TEST_F(Http2ConnPoolImplTest, PreconnectWithoutMultiplexing) {
 }
 
 TEST_F(Http2ConnPoolImplTest, IncreaseCapacityWithSettingsFrame) {
-  TestScopedRuntime scoped_runtime;
   cluster_->http2_options_.mutable_max_concurrent_streams()->set_value(100);
   ON_CALL(*cluster_, perUpstreamPreconnectRatio).WillByDefault(Return(1));
 
@@ -1486,7 +1485,6 @@ TEST_F(Http2ConnPoolImplTest, IncreaseCapacityWithSettingsFrame) {
 }
 
 TEST_F(Http2ConnPoolImplTest, DisconnectWithNegativeCapacity) {
-  TestScopedRuntime scoped_runtime;
   cluster_->http2_options_.mutable_max_concurrent_streams()->set_value(6);
   ON_CALL(*cluster_, perUpstreamPreconnectRatio).WillByDefault(Return(1));
 
diff --git a/test/common/network/connection_impl_test.cc b/test/common/network/connection_impl_test.cc
index 344f14f4ab35..ef046c48d51c 100644
--- a/test/common/network/connection_impl_test.cc
+++ b/test/common/network/connection_impl_test.cc
@@ -3030,9 +3030,8 @@ class InternalClientConnectionImplTest : public testing::Test {
 
 TEST_F(InternalClientConnectionImplTest,
        CannotCreateConnectionToInternalAddressWithInternalAddressEnabled) {
-  auto scoped_runtime_guard = std::make_unique<TestScopedRuntime>();
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.reloadable_features.internal_address", "true"}});
+  auto runtime = std::make_unique<TestScopedRuntime>();
+  runtime->mergeValues({{"envoy.reloadable_features.internal_address", "true"}});
 
   const Network::SocketInterface* sock_interface = Network::socketInterface(
       "envoy.extensions.network.socket_interface.default_socket_interface");
diff --git a/test/common/network/listener_impl_test.cc b/test/common/network/listener_impl_test.cc
index 15beded05f3b..fde37f55c4d0 100644
--- a/test/common/network/listener_impl_test.cc
+++ b/test/common/network/listener_impl_test.cc
@@ -124,8 +124,7 @@ TEST_P(TcpListenerImplTest, GlobalConnectionLimitEnforcement) {
   // Required to manipulate runtime values when there is no test server.
   TestScopedRuntime scoped_runtime;
 
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"overload.global_downstream_max_connections", "2"}});
+  scoped_runtime.mergeValues({{"overload.global_downstream_max_connections", "2"}});
   auto socket = std::make_shared<Network::Test::TcpListenSocketImmediateListen>(
       Network::Test::getCanonicalLoopbackAddress(version_));
   Network::MockTcpListenerCallbacks listener_callbacks;
@@ -161,8 +160,7 @@ TEST_P(TcpListenerImplTest, GlobalConnectionLimitEnforcement) {
   EXPECT_EQ(2, server_connections.size());
 
   // Let's increase the allowed connections and try sending more connections.
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"overload.global_downstream_max_connections", "3"}});
+  scoped_runtime.mergeValues({{"overload.global_downstream_max_connections", "3"}});
   initiate_connections(5);
   EXPECT_CALL(listener_callbacks, onReject(TcpListenerCallbacks::RejectCause::GlobalCxLimit))
       .Times(4);
@@ -171,8 +169,7 @@ TEST_P(TcpListenerImplTest, GlobalConnectionLimitEnforcement) {
   EXPECT_EQ(3, server_connections.size());
 
   // Clear the limit and verify there's no longer a limit.
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"overload.global_downstream_max_connections", ""}});
+  scoped_runtime.mergeValues({{"overload.global_downstream_max_connections", ""}});
   initiate_connections(10);
   dispatcher_->run(Event::Dispatcher::RunType::Block);
 
@@ -185,16 +182,14 @@ TEST_P(TcpListenerImplTest, GlobalConnectionLimitEnforcement) {
     conn->close(ConnectionCloseType::NoFlush);
   }
 
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"overload.global_downstream_max_connections", ""}});
+  scoped_runtime.mergeValues({{"overload.global_downstream_max_connections", ""}});
 }
 
 TEST_P(TcpListenerImplTest, GlobalConnectionLimitListenerOptOut) {
   // Required to manipulate runtime values when there is no test server.
   TestScopedRuntime scoped_runtime;
 
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"overload.global_downstream_max_connections", "1"}});
+  scoped_runtime.mergeValues({{"overload.global_downstream_max_connections", "1"}});
   auto socket = std::make_shared<Network::Test::TcpListenSocketImmediateListen>(
       Network::Test::getCanonicalLoopbackAddress(version_));
   Network::MockTcpListenerCallbacks listener_callbacks;
diff --git a/test/common/protobuf/utility_test.cc b/test/common/protobuf/utility_test.cc
index 644a92d57312..7d259f621c86 100644
--- a/test/common/protobuf/utility_test.cc
+++ b/test/common/protobuf/utility_test.cc
@@ -1725,7 +1725,7 @@ TEST_F(DeprecatedFieldsTest, IndividualFieldDeprecatedEmitsCrash) {
   base.set_is_deprecated("foo");
   // Non-fatal checks for a deprecated field should throw an exception if the
   // runtime flag is enabled..
-  Runtime::LoaderSingleton::getExisting()->mergeValues({
+  mergeValues({
       {"envoy.features.fail_on_any_deprecated_feature", "true"},
   });
   EXPECT_THROW_WITH_REGEX(
@@ -1756,9 +1756,8 @@ TEST_F(DeprecatedFieldsTest,
   // The config will be rejected, so the feature will not be used.
 
   // Now create a new snapshot with this feature allowed.
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.deprecated_features:envoy.test.deprecation_test.Base.is_deprecated_fatal",
-        "True "}});
+  mergeValues({{"envoy.deprecated_features:envoy.test.deprecation_test.Base.is_deprecated_fatal",
+                "True "}});
 
   // Now the same deprecation check should only trigger a warning.
   EXPECT_LOG_CONTAINS(
@@ -1780,8 +1779,7 @@ TEST_F(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(IndividualFieldDisallowedWi
   // The config will be rejected, so the feature will not be used.
 
   // Now create a new snapshot with this all features allowed.
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.features.enable_all_deprecated_features", "true"}});
+  mergeValues({{"envoy.features.enable_all_deprecated_features", "true"}});
 
   // Now the same deprecation check should only trigger a warning.
   EXPECT_LOG_CONTAINS(
@@ -1800,7 +1798,7 @@ TEST_F(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(DisallowViaRuntime)) {
                       checkForDeprecation(base));
 
   // Now create a new snapshot with this feature disallowed.
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
+  mergeValues(
       {{"envoy.deprecated_features:envoy.test.deprecation_test.Base.is_deprecated", " false"}});
 
   EXPECT_THROW_WITH_REGEX(
@@ -1809,8 +1807,7 @@ TEST_F(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(DisallowViaRuntime)) {
 
   // Verify that even when the enable_all_deprecated_features is enabled the
   // feature is disallowed.
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.features.enable_all_deprecated_features", "true"}});
+  mergeValues({{"envoy.features.enable_all_deprecated_features", "true"}});
 
   EXPECT_THROW_WITH_REGEX(
       checkForDeprecation(base), Envoy::ProtobufMessage::DeprecatedProtoFieldException,
@@ -1915,7 +1912,7 @@ TEST_F(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(RuntimeOverrideEnumDefault)
   envoy::test::deprecation_test::Base base;
   base.mutable_enum_container();
 
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
+  mergeValues(
       {{"envoy.deprecated_features:envoy.test.deprecation_test.Base.DEPRECATED_DEFAULT", "false"}});
 
   // Make sure this is set up right.
@@ -1925,8 +1922,7 @@ TEST_F(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(RuntimeOverrideEnumDefault)
 
   // Verify that even when the enable_all_deprecated_features is enabled the
   // enum is disallowed.
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.features.enable_all_deprecated_features", "true"}});
+  mergeValues({{"envoy.features.enable_all_deprecated_features", "true"}});
 
   EXPECT_THROW_WITH_REGEX(checkForDeprecation(base),
                           Envoy::ProtobufMessage::DeprecatedProtoFieldException,
@@ -1942,7 +1938,7 @@ TEST_F(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(FatalEnum)) {
                           Envoy::ProtobufMessage::DeprecatedProtoFieldException,
                           "Using deprecated value DEPRECATED_FATAL");
 
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
+  mergeValues(
       {{"envoy.deprecated_features:envoy.test.deprecation_test.Base.DEPRECATED_FATAL", "true"}});
 
   EXPECT_LOG_CONTAINS(
@@ -1963,8 +1959,7 @@ TEST_F(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(FatalEnumGlobalOverride)) {
                           Envoy::ProtobufMessage::DeprecatedProtoFieldException,
                           "Using deprecated value DEPRECATED_FATAL");
 
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.features.enable_all_deprecated_features", "true"}});
+  mergeValues({{"envoy.features.enable_all_deprecated_features", "true"}});
 
   EXPECT_LOG_CONTAINS(
       "warning",
diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc
index dd74cba502a9..4a8f08bf4864 100644
--- a/test/common/router/router_test.cc
+++ b/test/common/router/router_test.cc
@@ -4324,7 +4324,7 @@ TEST_F(RouterTest, InternalRedirectStripsFragment) {
 
 TEST_F(RouterTest, InternalRedirectKeepsFragmentWithOveride) {
   TestScopedRuntime scoped_runtime;
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
+  scoped_runtime.mergeValues(
       {{"envoy.reloadable_features.http_reject_path_with_fragment", "false"}});
   enableRedirects();
   default_request_headers_.setForwardedProto("http");
@@ -5914,7 +5914,7 @@ TEST_F(RouterTest, ExpectedUpstreamTimeoutUpdatedDuringRetries) {
 
 TEST_F(RouterTest, ExpectedUpstreamTimeoutNotUpdatedDuringRetriesWhenRuntimeGuardDisabled) {
   TestScopedRuntime scoped_runtime;
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
+  scoped_runtime.mergeValues(
       {{"envoy.reloadable_features.update_expected_rq_timeout_on_retry", "false"}});
 
   auto retry_options_predicate = std::make_shared<MockRetryOptionsPredicate>();
diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc
index cfec77e8f83f..bbe5a9d7fd23 100644
--- a/test/common/upstream/cluster_manager_impl_test.cc
+++ b/test/common/upstream/cluster_manager_impl_test.cc
@@ -5380,8 +5380,7 @@ TEST_F(ClusterManagerImplTest, ConnPoolsNotDrainedOnHostSetChange) {
 
 TEST_F(ClusterManagerImplTest, ConnPoolsIdleDeleted) {
   TestScopedRuntime scoped_runtime;
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.reloadable_features.conn_pool_delete_when_idle", "true"}});
+  scoped_runtime.mergeValues({{"envoy.reloadable_features.conn_pool_delete_when_idle", "true"}});
 
   const std::string yaml = R"EOF(
   static_resources:
diff --git a/test/common/upstream/eds_speed_test.cc b/test/common/upstream/eds_speed_test.cc
index c3d6a43e27ef..64a2ebffd463 100644
--- a/test/common/upstream/eds_speed_test.cc
+++ b/test/common/upstream/eds_speed_test.cc
@@ -45,6 +45,7 @@ class EdsSpeedTest {
         type_url_("type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment"),
         subscription_stats_(Config::Utility::generateStats(stats_)),
         api_(Api::createApiForTest(stats_)), async_client_(new Grpc::MockAsyncClient()) {
+    TestDeprecatedV2Api::allowDeprecatedV2();
     if (use_unified_mux_) {
       grpc_mux_.reset(new Config::XdsMux::GrpcMuxSotw(
           std::unique_ptr<Grpc::MockAsyncClient>(async_client_), dispatcher_,
@@ -150,7 +151,6 @@ class EdsSpeedTest {
            num_hosts);
   }
 
-  TestDeprecatedV2Api _deprecated_v2_api_;
   State& state_;
   bool use_unified_mux_;
   const std::string type_url_;
diff --git a/test/common/upstream/eds_test.cc b/test/common/upstream/eds_test.cc
index c57da0bb13e1..863901a2f9e6 100644
--- a/test/common/upstream/eds_test.cc
+++ b/test/common/upstream/eds_test.cc
@@ -127,7 +127,7 @@ class EdsTest : public testing::Test {
     Envoy::Server::Configuration::TransportSocketFactoryContextImpl factory_context(
         admin_, ssl_context_manager_, *scope, cm_, local_info_, dispatcher_, stats_,
         singleton_manager_, tls_, validation_visitor_, *api_, options_);
-    cluster_ = std::make_shared<EdsClusterImpl>(eds_cluster_, runtime_, factory_context,
+    cluster_ = std::make_shared<EdsClusterImpl>(eds_cluster_, runtime_.loader(), factory_context,
                                                 std::move(scope), false);
     EXPECT_EQ(initialize_phase, cluster_->initializePhase());
     eds_callbacks_ = cm_.subscription_factory_.callbacks_;
@@ -154,7 +154,7 @@ class EdsTest : public testing::Test {
   EdsClusterImplSharedPtr cluster_;
   Config::SubscriptionCallbacks* eds_callbacks_{};
   NiceMock<Random::MockRandomGenerator> random_;
-  NiceMock<Runtime::MockLoader> runtime_;
+  TestScopedRuntime runtime_;
   NiceMock<LocalInfo::MockLocalInfo> local_info_;
   NiceMock<Server::MockAdmin> admin_;
   Singleton::ManagerImpl singleton_manager_{Thread::threadFactoryForTest()};
@@ -1520,8 +1520,7 @@ TEST_F(EdsTest, EndpointLocalityUpdated) {
 // Unlike EndpointLocalityUpdated, runtime feature flag is disabled this time and then it is
 // verified that locality update does not happen on eds cluster endpoints.
 TEST_F(EdsTest, EndpointLocalityNotUpdatedIfFixDisabled) {
-  TestScopedRuntime runtime;
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
+  runtime_.mergeValues(
       {{"envoy.reloadable_features.support_locality_update_on_eds_cluster_endpoints", "false"}});
   envoy::config::endpoint::v3::ClusterLoadAssignment cluster_load_assignment;
   cluster_load_assignment.set_cluster_name("fare");
@@ -1579,7 +1578,6 @@ TEST_F(EdsTest, EndpointLocalityNotUpdatedIfFixDisabled) {
     EXPECT_EQ("hello", locality.zone());
     EXPECT_EQ("world", locality.sub_zone());
   }
-  Runtime::LoaderSingleton::clear();
 }
 
 // Validate that onConfigUpdate() does not propagate locality weights to the host set when
diff --git a/test/extensions/common/dynamic_forward_proxy/dns_cache_impl_test.cc b/test/extensions/common/dynamic_forward_proxy/dns_cache_impl_test.cc
index c53b7a8342ce..f836716e882c 100644
--- a/test/extensions/common/dynamic_forward_proxy/dns_cache_impl_test.cc
+++ b/test/extensions/common/dynamic_forward_proxy/dns_cache_impl_test.cc
@@ -1182,8 +1182,7 @@ TEST(UtilityTest, PrepareDnsRefreshStrategy) {
 
 TEST_F(DnsCacheImplTest, ResolveSuccessWithCaching) {
   TestScopedRuntime scoped_runtime;
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.reloadable_features.allow_multiple_dns_addresses", "true"}});
+  scoped_runtime.mergeValues({{"envoy.reloadable_features.allow_multiple_dns_addresses", "true"}});
 
   auto* time_source = new NiceMock<MockTimeSystem>();
   context_.dispatcher_.time_system_.reset(time_source);
@@ -1299,8 +1298,7 @@ TEST_F(DnsCacheImplTest, ResolveSuccessWithCaching) {
 
 TEST_F(DnsCacheImplTest, CacheLoad) {
   TestScopedRuntime scoped_runtime;
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.reloadable_features.allow_multiple_dns_addresses", "true"}});
+  scoped_runtime.mergeValues({{"envoy.reloadable_features.allow_multiple_dns_addresses", "true"}});
 
   auto* time_source = new NiceMock<MockTimeSystem>();
   context_.dispatcher_.time_system_.reset(time_source);
diff --git a/test/extensions/filters/http/buffer/buffer_filter_test.cc b/test/extensions/filters/http/buffer/buffer_filter_test.cc
index fd672c624299..85d3976b19ca 100644
--- a/test/extensions/filters/http/buffer/buffer_filter_test.cc
+++ b/test/extensions/filters/http/buffer/buffer_filter_test.cc
@@ -11,7 +11,6 @@
 #include "test/mocks/buffer/mocks.h"
 #include "test/mocks/http/mocks.h"
 #include "test/test_common/printers.h"
-#include "test/test_common/test_runtime.h"
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -40,8 +39,6 @@ class BufferFilterTest : public testing::Test {
   NiceMock<Http::MockStreamDecoderFilterCallbacks> callbacks_;
   BufferFilterConfigSharedPtr config_;
   BufferFilter filter_;
-  // Create a runtime loader, so that tests can manually manipulate runtime guarded features.
-  TestScopedRuntime scoped_runtime;
 };
 
 TEST_F(BufferFilterTest, HeaderOnlyRequest) {
diff --git a/test/extensions/filters/http/ext_authz/ext_authz_test.cc b/test/extensions/filters/http/ext_authz/ext_authz_test.cc
index 314a1c9f328a..00591fcebfa7 100644
--- a/test/extensions/filters/http/ext_authz/ext_authz_test.cc
+++ b/test/extensions/filters/http/ext_authz/ext_authz_test.cc
@@ -1691,7 +1691,7 @@ TEST_P(HttpFilterTestParam, NoRoute) {
 // `envoy.reloadable_features.http_ext_authz_do_not_skip_direct_response_and_redirect` is false.
 TEST_P(HttpFilterTestParam, NoRouteWithSkipAuth) {
   TestScopedRuntime scoped_runtime;
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
+  scoped_runtime.mergeValues(
       {{"envoy.reloadable_features.http_ext_authz_do_not_skip_direct_response_and_redirect",
         "false"}});
   EXPECT_CALL(*decoder_filter_callbacks_.route_, routeEntry()).WillOnce(Return(nullptr));
diff --git a/test/extensions/filters/http/wasm/wasm_filter_test.cc b/test/extensions/filters/http/wasm/wasm_filter_test.cc
index b6652a14964d..c867272aec80 100644
--- a/test/extensions/filters/http/wasm/wasm_filter_test.cc
+++ b/test/extensions/filters/http/wasm/wasm_filter_test.cc
@@ -6,7 +6,6 @@
 #include "test/extensions/common/wasm/wasm_runtime.h"
 #include "test/mocks/network/connection.h"
 #include "test/mocks/router/mocks.h"
-#include "test/test_common/test_runtime.h"
 #include "test/test_common/wasm_base.h"
 
 using testing::Eq;
@@ -940,7 +939,6 @@ TEST_P(WasmHttpFilterTest, GrpcCall) {
     proto_or_cluster.push_back("grpc_call_proto");
   }
   for (const auto& id : proto_or_cluster) {
-    TestScopedRuntime scoped_runtime;
     setupTest(id);
     setupFilter();
 
@@ -1018,7 +1016,6 @@ TEST_P(WasmHttpFilterTest, GrpcCallBadCall) {
     proto_or_cluster.push_back("grpc_call_proto");
   }
   for (const auto& id : proto_or_cluster) {
-    TestScopedRuntime scoped_runtime;
     setupTest(id);
     setupFilter();
 
@@ -1060,7 +1057,6 @@ TEST_P(WasmHttpFilterTest, GrpcCallFailure) {
     proto_or_cluster.push_back("grpc_call_proto");
   }
   for (const auto& id : proto_or_cluster) {
-    TestScopedRuntime scoped_runtime;
     setupTest(id);
     setupFilter();
 
@@ -1150,7 +1146,6 @@ TEST_P(WasmHttpFilterTest, GrpcCallCancel) {
     proto_or_cluster.push_back("grpc_call_proto");
   }
   for (const auto& id : proto_or_cluster) {
-    TestScopedRuntime scoped_runtime;
     setupTest(id);
     setupFilter();
 
@@ -1209,7 +1204,6 @@ TEST_P(WasmHttpFilterTest, GrpcCallClose) {
     proto_or_cluster.push_back("grpc_call_proto");
   }
   for (const auto& id : proto_or_cluster) {
-    TestScopedRuntime scoped_runtime;
     setupTest(id);
     setupFilter();
 
@@ -1268,7 +1262,6 @@ TEST_P(WasmHttpFilterTest, GrpcCallAfterDestroyed) {
     proto_or_cluster.push_back("grpc_call_proto");
   }
   for (const auto& id : proto_or_cluster) {
-    TestScopedRuntime scoped_runtime;
     setupTest(id);
     setupFilter();
 
@@ -1372,7 +1365,6 @@ TEST_P(WasmHttpFilterTest, GrpcStream) {
     proto_or_cluster.push_back("grpc_stream_proto");
   }
   for (const auto& id : proto_or_cluster) {
-    TestScopedRuntime scoped_runtime;
     Grpc::RawAsyncStreamCallbacks* callbacks = nullptr;
     setupGrpcStreamTest(callbacks, id);
 
@@ -1434,7 +1426,6 @@ TEST_P(WasmHttpFilterTest, GrpcStreamCloseLocal) {
     proto_or_cluster.push_back("grpc_stream_proto");
   }
   for (const auto& id : proto_or_cluster) {
-    TestScopedRuntime scoped_runtime;
     Grpc::RawAsyncStreamCallbacks* callbacks = nullptr;
     setupGrpcStreamTest(callbacks, id);
 
@@ -1495,7 +1486,6 @@ TEST_P(WasmHttpFilterTest, GrpcStreamCloseRemote) {
     proto_or_cluster.push_back("grpc_stream_proto");
   }
   for (const auto& id : proto_or_cluster) {
-    TestScopedRuntime scoped_runtime;
     Grpc::RawAsyncStreamCallbacks* callbacks = nullptr;
     setupGrpcStreamTest(callbacks, id);
 
@@ -1555,7 +1545,6 @@ TEST_P(WasmHttpFilterTest, GrpcStreamCancel) {
     proto_or_cluster.push_back("grpc_stream_proto");
   }
   for (const auto& id : proto_or_cluster) {
-    TestScopedRuntime scoped_runtime;
     Grpc::RawAsyncStreamCallbacks* callbacks = nullptr;
     setupGrpcStreamTest(callbacks, id);
 
@@ -1606,7 +1595,6 @@ TEST_P(WasmHttpFilterTest, GrpcStreamOpenAtShutdown) {
     proto_or_cluster.push_back("grpc_stream_proto");
   }
   for (const auto& id : proto_or_cluster) {
-    TestScopedRuntime scoped_runtime;
     Grpc::RawAsyncStreamCallbacks* callbacks = nullptr;
     setupGrpcStreamTest(callbacks, id);
 
diff --git a/test/extensions/filters/network/common/redis/fault_test.cc b/test/extensions/filters/network/common/redis/fault_test.cc
index da80a05d7a9f..f8c67a2130b7 100644
--- a/test/extensions/filters/network/common/redis/fault_test.cc
+++ b/test/extensions/filters/network/common/redis/fault_test.cc
@@ -92,7 +92,6 @@ TEST_F(FaultTest, NoFaults) {
   RedisProxy redis_config;
   auto* faults = redis_config.mutable_faults();
 
-  TestScopedRuntime scoped_runtime;
   FaultManagerImpl fault_manager = FaultManagerImpl(random_, runtime_, *faults);
 
   const Fault* fault_ptr = fault_manager.getFaultForCommand("get");
@@ -104,7 +103,6 @@ TEST_F(FaultTest, SingleCommandFaultNotEnabled) {
   auto* faults = redis_config.mutable_faults();
   createCommandFault(faults->Add(), "get", 0, 0, FractionalPercent::HUNDRED, RUNTIME_KEY);
 
-  TestScopedRuntime scoped_runtime;
   FaultManagerImpl fault_manager = FaultManagerImpl(random_, runtime_, *faults);
 
   EXPECT_CALL(random_, random()).WillOnce(Return(0));
@@ -120,7 +118,6 @@ TEST_F(FaultTest, SingleCommandFault) {
   auto* faults = redis_config.mutable_faults();
   createCommandFault(faults->Add(), "ttl", 0, 5000, FractionalPercent::TEN_THOUSAND, RUNTIME_KEY);
 
-  TestScopedRuntime scoped_runtime;
   FaultManagerImpl fault_manager = FaultManagerImpl(random_, runtime_, *faults);
 
   EXPECT_CALL(random_, random()).WillOnce(Return(1));
@@ -136,7 +133,6 @@ TEST_F(FaultTest, SingleCommandFaultWithNoDefaultValueOrRuntimeValue) {
   auto* faults = redis_config.mutable_faults();
   createCommandFault(faults->Add(), "ttl", 0, absl::nullopt, absl::nullopt, absl::nullopt);
 
-  TestScopedRuntime scoped_runtime;
   FaultManagerImpl fault_manager = FaultManagerImpl(random_, runtime_, *faults);
 
   EXPECT_CALL(random_, random()).WillOnce(Return(1));
@@ -154,7 +150,6 @@ TEST_F(FaultTest, MultipleFaults) {
   createCommandFault(faults->Add(), "get", 0, 25, FractionalPercent::HUNDRED, RUNTIME_KEY);
   createAllKeyFault(faults->Add(), 2, 25, FractionalPercent::HUNDRED, absl::nullopt);
 
-  TestScopedRuntime scoped_runtime;
   FaultManagerImpl fault_manager = FaultManagerImpl(random_, runtime_, *faults);
   const Fault* fault_ptr;
 
diff --git a/test/server/connection_handler_test.cc b/test/server/connection_handler_test.cc
index 3313b1118166..9f86c5c76e42 100644
--- a/test/server/connection_handler_test.cc
+++ b/test/server/connection_handler_test.cc
@@ -982,7 +982,7 @@ TEST_F(ConnectionHandlerTest, EnsureNotMatchStoppedAnyAddressListener) {
 }
 
 TEST_F(ConnectionHandlerTest, EnsureNotMatchStoppedAnyAddressListenerOnOldBehavior) {
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
+  scoped_runtime_.mergeValues(
       {{"envoy.reloadable_features.listener_wildcard_match_ip_family", "false"}});
 
   Network::TcpListenerCallbacks* listener_callbacks;
@@ -1103,7 +1103,7 @@ TEST_F(ConnectionHandlerTest, FallbackToWildcardListener) {
 }
 
 TEST_F(ConnectionHandlerTest, OldBehaviorWildcardListener) {
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
+  scoped_runtime_.mergeValues(
       {{"envoy.reloadable_features.listener_wildcard_match_ip_family", "false"}});
 
   Network::TcpListenerCallbacks* listener_callbacks1;
diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc
index 2f9c18052998..5d67dca2f1da 100644
--- a/test/server/listener_manager_impl_test.cc
+++ b/test/server/listener_manager_impl_test.cc
@@ -547,10 +547,9 @@ bind_to_port: false
 }
 
 TEST_F(ListenerManagerImplTest, UnsupportedInternalListener) {
-  auto scoped_runtime_guard = std::make_unique<TestScopedRuntime>();
+  auto scoped_runtime = std::make_unique<TestScopedRuntime>();
   // Workaround of triggering death at windows platform.
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.reloadable_features.internal_address", "false"}});
+  scoped_runtime->mergeValues({{"envoy.reloadable_features.internal_address", "false"}});
 
   const std::string yaml = R"EOF(
     name: "foo"
@@ -565,9 +564,8 @@ TEST_F(ListenerManagerImplTest, UnsupportedInternalListener) {
 }
 
 TEST_F(ListenerManagerImplTest, RejectListenerWithSocketAddressWithInternalListenerConfig) {
-  auto scoped_runtime_guard = std::make_unique<TestScopedRuntime>();
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.reloadable_features.internal_address", "true"}});
+  auto scoped_runtime = std::make_unique<TestScopedRuntime>();
+  scoped_runtime->mergeValues({{"envoy.reloadable_features.internal_address", "true"}});
 
   const std::string yaml = R"EOF(
     name: "foo"
@@ -587,9 +585,8 @@ TEST_F(ListenerManagerImplTest, RejectListenerWithSocketAddressWithInternalListe
 }
 
 TEST_F(ListenerManagerImplTest, RejectTcpOptionsWithInternalListenerConfig) {
-  auto scoped_runtime_guard = std::make_unique<TestScopedRuntime>();
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.reloadable_features.internal_address", "true"}});
+  auto scoped_runtime = std::make_unique<TestScopedRuntime>();
+  scoped_runtime->mergeValues({{"envoy.reloadable_features.internal_address", "true"}});
 
   const std::string yaml = R"EOF(
     name: "foo"
@@ -4874,9 +4871,8 @@ name: test_api_listener_2
 }
 
 TEST_F(ListenerManagerImplWithRealFiltersTest, AddOrUpdateInternalListener) {
-  auto scoped_runtime_guard = std::make_unique<TestScopedRuntime>();
-  Runtime::LoaderSingleton::getExisting()->mergeValues(
-      {{"envoy.reloadable_features.internal_address", "true"}});
+  auto scoped_runtime = std::make_unique<TestScopedRuntime>();
+  scoped_runtime->mergeValues({{"envoy.reloadable_features.internal_address", "true"}});
   time_system_.setSystemTime(std::chrono::milliseconds(1001001001001));
 
   InSequence s;
diff --git a/test/test_common/simulated_time_system_test.cc b/test/test_common/simulated_time_system_test.cc
index 422669b0725e..03720a5a767b 100644
--- a/test/test_common/simulated_time_system_test.cc
+++ b/test/test_common/simulated_time_system_test.cc
@@ -6,7 +6,6 @@
 #include "test/mocks/common.h"
 #include "test/mocks/event/mocks.h"
 #include "test/test_common/simulated_time_system.h"
-#include "test/test_common/test_runtime.h"
 #include "test/test_common/utility.h"
 
 #include "event2/event.h"
@@ -59,7 +58,6 @@ class SimulatedTimeSystemTest : public testing::Test {
     base_scheduler_.run(Dispatcher::RunType::NonBlock);
   }
 
-  TestScopedRuntime scoped_runtime_;
   Event::MockDispatcher dispatcher_;
   LibeventScheduler base_scheduler_;
   SimulatedTimeSystem time_system_;
diff --git a/test/test_common/test_runtime.h b/test/test_common/test_runtime.h
index a55c526b0683..d763309c3be7 100644
--- a/test/test_common/test_runtime.h
+++ b/test/test_common/test_runtime.h
@@ -3,11 +3,8 @@
 // As long as this class is in scope one can do runtime feature overrides:
 //
 //  TestScopedRuntime scoped_runtime;
-//  Runtime::LoaderSingleton::getExisting()->mergeValues(
+//  scoped_runtime.mergeValues(
 //      {{"envoy.reloadable_features.test_feature_true", "false"}});
-//
-//  As long as a TestScopedRuntime exists, Runtime::LoaderSingleton::getExisting()->mergeValues()
-//  can safely be called to override runtime values.
 
 #pragma once
 
@@ -42,6 +39,10 @@ class TestScopedRuntime {
 
   Runtime::Loader& loader() { return *Runtime::LoaderSingleton::getExisting(); }
 
+  void mergeValues(const absl::node_hash_map<std::string, std::string>& values) {
+    loader().mergeValues(values);
+  }
+
   ~TestScopedRuntime() {
     Runtime::RuntimeFeatures features;
     features.restoreDefaults();
@@ -60,7 +61,8 @@ class TestScopedRuntime {
 
 class TestDeprecatedV2Api : public TestScopedRuntime {
 public:
-  TestDeprecatedV2Api() {
+  TestDeprecatedV2Api() { allowDeprecatedV2(); }
+  static void allowDeprecatedV2() {
     Runtime::LoaderSingleton::getExisting()->mergeValues({
         {"envoy.test_only.broken_in_production.enable_deprecated_v2_api", "true"},
         {"envoy.features.enable_all_deprecated_features", "true"},

From fb6f157a8c02b6419822249e561707f1ae076948 Mon Sep 17 00:00:00 2001
From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com>
Date: Wed, 2 Mar 2022 10:00:46 +0000
Subject: [PATCH 07/14] build(deps): bump grpcio-tools in
 /examples/grpc-bridge/client (#20040)

Bumps [grpcio-tools](https://github.com/grpc/grpc) from 1.43.0 to 1.44.0.
- [Release notes](https://github.com/grpc/grpc/releases)
- [Changelog](https://github.com/grpc/grpc/blob/master/doc/grpc_release_schedule.md)
- [Commits](https://github.com/grpc/grpc/compare/v1.43.0...v1.44.0)

---
updated-dependencies:
- dependency-name: grpcio-tools
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>
---
 examples/grpc-bridge/client/requirements.txt | 90 ++++++++++----------
 1 file changed, 45 insertions(+), 45 deletions(-)

diff --git a/examples/grpc-bridge/client/requirements.txt b/examples/grpc-bridge/client/requirements.txt
index be2b75815c9b..aca3d1c8cc88 100644
--- a/examples/grpc-bridge/client/requirements.txt
+++ b/examples/grpc-bridge/client/requirements.txt
@@ -60,51 +60,51 @@ grpcio==1.44.0 \
     # via
     #   -r requirements.in
     #   grpcio-tools
-grpcio-tools==1.43.0 \
-    --hash=sha256:010a4be6a2fccbd6741a4809c5da7f2e39a1e9e227745e6b495be567638bbeb9 \
-    --hash=sha256:019f55929e963214471825c7a4cdab7a57069109d5621b24e4db7b428b5fe47d \
-    --hash=sha256:04f100c1f6a7c72c537760c33582f6970070bd6fa6676b529bccfa31cc58bc79 \
-    --hash=sha256:05550ba473cff7c09e905fcfb2263fd1f7600389660194ec022b5d5a3802534b \
-    --hash=sha256:09464c6b17663088144b7e6ea10e9465efdcee03d4b2ffefab39a799bd8360f8 \
-    --hash=sha256:178a881db5de0f89abf3aeeb260ecfd1116cc31f88fb600a45fb5b19c3323b33 \
-    --hash=sha256:18870dcc8369ac4c37213e6796d8dc20494ea770670204f5e573f88e69eaaf0b \
-    --hash=sha256:1adb0dbcc1c10b86dcda910b8f56e39210e401bcee923dba166ba923a5f4696a \
-    --hash=sha256:1def9b68ac9e62674929bc6590a33d89635f1cf16016657d9e16a69f41aa5c36 \
-    --hash=sha256:234c7a5af653357df5c616e013173eddda6193146c8ab38f3108c4784f66be26 \
-    --hash=sha256:2458d6b0404f83d95aef00cec01f310d30e9719564a25be50e39b259f6a2da5d \
-    --hash=sha256:2737f749a6ab965748629e619b35f3e1cbe5820fc79e34c88f73cb99efc71dde \
-    --hash=sha256:2e9bb5da437364b7dcd2d3c6850747081ecbec0ba645c96c6d471f7e21fdcadb \
-    --hash=sha256:3eb4aa5b0e578c3d9d9da8e37a2ef73654287a498b8081543acd0db7f0ec1a9c \
-    --hash=sha256:426f16b6b14d533ce61249a18fbcd1a23a4fa0c71a6d7ab347b1c7f862847bb8 \
-    --hash=sha256:4c5c80098fa69593b828d119973744de03c3f9a6935df8a02e4329a39b7072f5 \
-    --hash=sha256:53f7dcaa4218df1b64b39d0fc7236a8270e8ab2db4ab8cd1d2fda0e6d4544946 \
-    --hash=sha256:55c2e604536e06248e2f81e549737fb3a180c8117832e494a0a8a81fbde44837 \
-    --hash=sha256:5be6d402b0cafef20ba3abb3baa37444961d9a9c4a6434d3d7c1f082f7697deb \
-    --hash=sha256:5f2e584d7644ef924e9e042fa151a3bb9f7c28ef1ae260ee6c9cb327982b5e94 \
-    --hash=sha256:61ef6cb6ccf9b9c27bb85fffc5338194bcf444df502196c2ad0ff8df4706d41e \
-    --hash=sha256:63862a441a77f6326ea9fe4bb005882f0e363441a5968d9cf8621c34d3dadc2b \
-    --hash=sha256:671e61bbc91d8d568f12c3654bb5a91fce9f3fdfd5ec2cfc60c2d3a840449aa6 \
-    --hash=sha256:6dea0cb2e79b67593553ed8662f70e4310599fa8850fc0e056b19fcb63572b7f \
-    --hash=sha256:6eaf97414237b8670ae9fa623879a26eabcc4c635b550c79a81e17eb600d6ae3 \
-    --hash=sha256:766771ef5b60ebcba0a3bdb302dd92fda988552eb8508451ff6d97371eac38e5 \
-    --hash=sha256:8953fdebef6905d7ff13a5a376b21b6fecd808d18bf4f0d3990ffe4a215d56eb \
-    --hash=sha256:98dcb5b756855110fb661ccd6a93a716610b7efcd5720a3aec01358a1a892c30 \
-    --hash=sha256:9dbb6d1f58f26d88ae689f1b49de84cfaf4786c81c01b9001d3ceea178116a07 \
-    --hash=sha256:b68cc0c95a0f8c757e8d69b5fa46111d5c9d887ae62af28f827649b1d1b70fe1 \
-    --hash=sha256:ba3da574eb08fcaed541b3fc97ce217360fd86d954fa9ad6a604803d57a2e049 \
-    --hash=sha256:c39cbe7b902bb92f9afaa035091f5e2b8be35acbac501fec8cb6a0be7d7cdbbd \
-    --hash=sha256:ce13a922db8f5f95c5041d3a4cbf04d942b353f0cba9b251a674f69a31a2d3a6 \
-    --hash=sha256:d21928b680e6e29538688cffbf53f3d5a53cff0ec8f0c33139641700045bdf1a \
-    --hash=sha256:d7173ed19854d1066bce9bdc09f735ca9c13e74a25d47a1cc5d1fe803b53bffb \
-    --hash=sha256:d7e3662f62d410b3f81823b5fa0f79c6e0e250977a1058e4131867b85138a661 \
-    --hash=sha256:e6c0e1d1b47554c580882d392b739df91a55b6a8ec696b2b2e1bbc127d63df2c \
-    --hash=sha256:e956b5c3b586d7b27eae49fb06f544a26288596fe12e22ffec768109717276d1 \
-    --hash=sha256:ebfb94ddb454a6dc3a505d9531dc81c948e6364e181b8795bfad3f3f479974dc \
-    --hash=sha256:efd1eb5880001f5189cfa3a774675cc9bbc8cc51586a3e90fe796394ac8626b8 \
-    --hash=sha256:f19d40690c97365c1c1bde81474e6f496d7ab76f87e6d2889c72ad01bac98f2d \
-    --hash=sha256:f42f1d713096808b1b0472dd2a3749b712d13f0092dab9442d9c096446e860b2 \
-    --hash=sha256:f974cb0bea88bac892c3ed16da92c6ac88cff0fea17f24bf0e1892eb4d27cd00 \
-    --hash=sha256:f97f9ffa49348fb24692751d2d4455ef2968bd07fe536d65597caaec14222629
+grpcio-tools==1.44.0 \
+    --hash=sha256:121c9765cee8636201cf0d4e80bc7b509813194919bccdb66e9671c4ece6dac3 \
+    --hash=sha256:1972caf8f695b91edc6444134445798692fe71276f0cde7604d55e65179adf93 \
+    --hash=sha256:1d120082236f8d2877f8a19366476b82c3562423b877b7c471a142432e31c2c4 \
+    --hash=sha256:1f87fc86d0b4181b6b4da6ec6a29511dca000e6b5694fdd6bbf87d125128bc41 \
+    --hash=sha256:2b211f12e4cbc0fde8e0f982b0f581cce38874666a02ebfed93c23dcaeb8a4e0 \
+    --hash=sha256:2c516124356476d9afa126acce10ce568733120afbd9ae17ee01d44b9da20a67 \
+    --hash=sha256:33d93027840a873c7b59402fe6db8263b88c56e2f84aa0b6281c05cc8bd314a1 \
+    --hash=sha256:37045ba850d423cdacede77b266b127025818a5a36d80f1fd7a5a1614a6a0de5 \
+    --hash=sha256:395609c06f69fbc79518b30a01931127088a3f9ef2cc2a35269c5f187eefd38c \
+    --hash=sha256:398eda759194d355eb09f7beabae6e4fb45b3877cf7efe505b49095fa4889cef \
+    --hash=sha256:3c0be60721ae1ba09c4f29572a145f412e561b9201e19428758893709827f472 \
+    --hash=sha256:3c9abc4a40c62f46d5e43e49c7afc567dedf12eeef95933ac9ea2986baa2420b \
+    --hash=sha256:3d6c8548b199591757dbfe89ed14e23782d6079d6d201c6c314c72f4086883aa \
+    --hash=sha256:3e16260dfe6e997330473863e01466b0992369ae2337a0249b390b4651cff424 \
+    --hash=sha256:3f0e1d1f3f5a6f0c9f8b5441819dbec831ce7e9ffe04768e4b0d965a95fbbe5e \
+    --hash=sha256:4eb93619c8cb3773fb899504e3e30a0dc79d3904fd7a84091d15552178e1e920 \
+    --hash=sha256:5ade6b13dc4e148f400c8f55a6ef0b14216a3371d7a9e559571d5981b6cec36b \
+    --hash=sha256:5caef118deb8cdee1978fd3d8e388a9b256cd8d34e4a8895731ac0e86fa5e47c \
+    --hash=sha256:608414cc1093e1e9e5980c97a6ee78e51dffff359e7a3f123d1fb9d95b8763a5 \
+    --hash=sha256:6138d2c7eec7ed57585bc58e2dbcb65635a2d574ac632abd29949d3e68936bab \
+    --hash=sha256:614c427ff235d92f103e9189f0230197c8f2f817d0dd9fd078f5d2ea4d920d02 \
+    --hash=sha256:65c2fe3cdc5425180f01dd303e28d4f363d38f4c2e3a7e1a87caedd5417e23bb \
+    --hash=sha256:674fb8d9c0e2d75166c4385753962485b757897223fc92a19c9e513ab80b96f7 \
+    --hash=sha256:69bfa6fc1515c202fe428ba9f99e2b2f947b01bafc15d868798235b2e2d36baa \
+    --hash=sha256:6cdf72947c6b0b03aa6dac06117a095947d02d43a5c6343051f4ce161fd0abcb \
+    --hash=sha256:71fb6e7e66b918803b1bebd0231560981ab86c2546a3318a45822ce94de5e83d \
+    --hash=sha256:7c04ec47905c4f6d6dad34d29f6ace652cc1ddc986f55aaa5559b72104c3f5cf \
+    --hash=sha256:90d1fac188bac838c4169eb3b67197887fa0572ea8a90519a20cddb080800549 \
+    --hash=sha256:9b421dc9b27bcaff4c73644cd3801e4893b11ba3eb39729246fd3de98d9f685b \
+    --hash=sha256:9f58529e24f613019a85c258a274d441d89e0cad8cf7fca21ef3807ba5840c5d \
+    --hash=sha256:a169bfd7a1fe8cc11472eeeeab3088b3c5d56caac12b2192a920b73adcbc974c \
+    --hash=sha256:a58aaaec0d846d142edd8e794ebb80aa429abfd581f4493a60a603aac0c50ac8 \
+    --hash=sha256:b41c419829f01734d65958ba9b01b759061d8f7e0698f9612ba6b8837269f7a9 \
+    --hash=sha256:b73fd87a44ba1b91866b0254193c37cdb001737759b77b637cebe0c816d38342 \
+    --hash=sha256:be37f458ea510c9a8f1caabbc2b258d12e55d189a567f5edcace90f27dc0efbf \
+    --hash=sha256:c13e0cb486cfa15320ddcd70452a4d736e6ce319c03d6b3c0c2513ec8d2748fb \
+    --hash=sha256:c3253bee8b68fe422754faf0f286aa068861c926a7b11e4daeb44b9af767c7f1 \
+    --hash=sha256:cb8baa1d4cea35ca662c24098377bdd9514c56f227da0e38b43cd9b8223bfcc6 \
+    --hash=sha256:ceb6441c24176705c5ab056e65a8b330e107107c5a492ba094d1b862a136d15d \
+    --hash=sha256:e44b9572c2226b85976e0d6054e22d7c59ebd6c9425ee71e5bc8910434aee3e1 \
+    --hash=sha256:ea36a294f7c70fd2f2bfb5dcf08602006304aa65b055ebd4f7c709e2a89deba7 \
+    --hash=sha256:f7ce16766b24b88ec0e4355f5dd66c2eee6af210e889fcb7961c9c4634c687de \
+    --hash=sha256:f9f0c5b4567631fec993826e694e83d86a972b3e2e9b05cb0c56839b0316d26c \
+    --hash=sha256:fb8c7b9d24e2c4dc77e7800e83b68081729ac6094b781b2afdabf08af18c3b28
     # via -r requirements.in
 idna==3.2 \
     --hash=sha256:14475042e284991034cb48e06f6851428fb14c4dc953acd9be9a5e95c7b6dd7a \

From c1206673ba36bf6b85cb0dfd8d109515484e94cf Mon Sep 17 00:00:00 2001
From: Rex Chang <58710378+rexnp@users.noreply.github.com>
Date: Wed, 2 Mar 2022 01:28:44 +0000
Subject: [PATCH 08/14] adds to spellcheck

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>
---
 tools/spelling/spelling_dictionary.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt
index 4eb45381497c..a4bffd6110f0 100644
--- a/tools/spelling/spelling_dictionary.txt
+++ b/tools/spelling/spelling_dictionary.txt
@@ -402,6 +402,7 @@ XML
 XN
 XNOR
 XSS
+Xray
 YAML
 ZXID
 absl
@@ -1176,6 +1177,7 @@ subnets
 suboptimal
 subsecond
 subseconds
+subsegment
 subsequence
 subsetting
 substr

From 7e8134f7983b52fb5fe47e1303ed7b6acea644b6 Mon Sep 17 00:00:00 2001
From: Rex Chang <58710378+rexnp@users.noreply.github.com>
Date: Tue, 21 Dec 2021 10:34:05 -0800
Subject: [PATCH 09/14] xray tracer: set subsegment type for child spans (#2)

* xray tracer: set subsegment type for child spans

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

* adds test coverage

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>
---
 source/extensions/tracers/xray/daemon.proto |  3 +++
 source/extensions/tracers/xray/tracer.cc    |  5 ++++-
 source/extensions/tracers/xray/tracer.h     | 13 +++++++++++++
 test/extensions/tracers/xray/tracer_test.cc |  8 ++++++++
 4 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/source/extensions/tracers/xray/daemon.proto b/source/extensions/tracers/xray/daemon.proto
index 2054bced7630..8f66f972684f 100644
--- a/source/extensions/tracers/xray/daemon.proto
+++ b/source/extensions/tracers/xray/daemon.proto
@@ -43,6 +43,9 @@ message Segment {
   }
   // Object containing one or more fields that X-Ray indexes for use with filter expressions.
   map<string, string> annotations = 8;
+  // Set type to "subsegment" when sending a child span so Xray treats it as a subsegment.
+  // https://docs.aws.amazon.com/xray/latest/devguide/xray-api-segmentdocuments.html#api-segmentdocuments-subsegments
+  string type = 14;
 }
 
 message Header {
diff --git a/source/extensions/tracers/xray/tracer.cc b/source/extensions/tracers/xray/tracer.cc
index b6bfb4da427e..e8d6bcdd124d 100644
--- a/source/extensions/tracers/xray/tracer.cc
+++ b/source/extensions/tracers/xray/tracer.cc
@@ -71,7 +71,9 @@ void Span::finishSpan() {
   s.set_error(clientError());
   s.set_fault(serverError());
   s.set_throttle(isThrottled());
-
+  if (type() == Subsegment) {
+    s.set_type(std::string(Subsegment));
+  }
   auto* aws = s.mutable_aws()->mutable_fields();
   for (const auto& field : aws_metadata_) {
     aws->insert({field.first, field.second});
@@ -115,6 +117,7 @@ Tracing::SpanPtr Span::spawnChild(const Tracing::Config& config, const std::stri
   child_span->setParentId(id());
   child_span->setTraceId(traceId());
   child_span->setSampled(sampled());
+  child_span->setType(Subsegment);
   return child_span;
 }
 
diff --git a/source/extensions/tracers/xray/tracer.h b/source/extensions/tracers/xray/tracer.h
index f53c4ca91a12..e156eab4665e 100644
--- a/source/extensions/tracers/xray/tracer.h
+++ b/source/extensions/tracers/xray/tracer.h
@@ -26,6 +26,7 @@ namespace Tracers {
 namespace XRay {
 
 constexpr auto XRayTraceHeader = "x-amzn-trace-id";
+constexpr absl::string_view Subsegment = "subsegment";
 
 class Span : public Tracing::Span, Logger::Loggable<Logger::Id::config> {
 public:
@@ -101,6 +102,12 @@ class Span : public Tracing::Span, Logger::Loggable<Logger::Id::config> {
     parent_segment_id_ = std::string(parent_segment_id);
   }
 
+  /**
+   * Sets the type of the Span. In Xray, an independent subsegment has a type of ``subsegment``.
+   * https://docs.aws.amazon.com/xray/latest/devguide/xray-api-segmentdocuments.html#api-segmentdocuments-subsegments
+   */
+  void setType(absl::string_view type) { type_ = std::string(type); }
+
   /**
    * Sets the aws metadata field of the Span.
    */
@@ -156,6 +163,11 @@ class Span : public Tracing::Span, Logger::Loggable<Logger::Id::config> {
    */
   const std::string& direction() const { return direction_; }
 
+  /**
+   * Gets this Span's type.
+   */
+  const std::string& type() const { return type_; }
+
   /**
    * Gets this Span's name.
    */
@@ -216,6 +228,7 @@ class Span : public Tracing::Span, Logger::Loggable<Logger::Id::config> {
   std::string parent_segment_id_;
   std::string name_;
   std::string origin_;
+  std::string type_;
   absl::flat_hash_map<std::string, ProtobufWkt::Value> aws_metadata_;
   absl::flat_hash_map<std::string, ProtobufWkt::Value> http_request_annotations_;
   absl::flat_hash_map<std::string, ProtobufWkt::Value> http_response_annotations_;
diff --git a/test/extensions/tracers/xray/tracer_test.cc b/test/extensions/tracers/xray/tracer_test.cc
index f77d28e40724..d2b58951f1fe 100644
--- a/test/extensions/tracers/xray/tracer_test.cc
+++ b/test/extensions/tracers/xray/tracer_test.cc
@@ -97,6 +97,7 @@ TEST_F(XRayTracerTest, SerializeSpanTest) {
     EXPECT_FALSE(s.id().empty());
     EXPECT_EQ(2, s.annotations().size());
     EXPECT_TRUE(s.parent_id().empty());
+    EXPECT_TRUE(s.type().empty());
     EXPECT_FALSE(s.fault());    /*server error*/
     EXPECT_FALSE(s.error());    /*client error*/
     EXPECT_FALSE(s.throttle()); /*request throttled*/
@@ -142,6 +143,7 @@ TEST_F(XRayTracerTest, SerializeSpanTestServerError) {
     EXPECT_FALSE(s.trace_id().empty());
     EXPECT_FALSE(s.id().empty());
     EXPECT_TRUE(s.parent_id().empty());
+    EXPECT_TRUE(s.type().empty());
     EXPECT_TRUE(s.fault());  /*server error*/
     EXPECT_FALSE(s.error()); /*client error*/
     EXPECT_EQ(expected_status_code,
@@ -175,6 +177,7 @@ TEST_F(XRayTracerTest, SerializeSpanTestClientError) {
     EXPECT_FALSE(s.trace_id().empty());
     EXPECT_FALSE(s.id().empty());
     EXPECT_TRUE(s.parent_id().empty());
+    EXPECT_TRUE(s.type().empty());
     EXPECT_FALSE(s.fault());    /*server error*/
     EXPECT_TRUE(s.error());     /*client error*/
     EXPECT_FALSE(s.throttle()); /*request throttled*/
@@ -208,6 +211,7 @@ TEST_F(XRayTracerTest, SerializeSpanTestClientErrorWithThrottle) {
     EXPECT_FALSE(s.trace_id().empty());
     EXPECT_FALSE(s.id().empty());
     EXPECT_TRUE(s.parent_id().empty());
+    EXPECT_TRUE(s.type().empty());
     EXPECT_FALSE(s.fault());   /*server error*/
     EXPECT_TRUE(s.error());    /*client error*/
     EXPECT_TRUE(s.throttle()); /*request throttled*/
@@ -239,6 +243,7 @@ TEST_F(XRayTracerTest, SerializeSpanTestWithEmptyValue) {
     EXPECT_FALSE(s.trace_id().empty());
     EXPECT_FALSE(s.id().empty());
     EXPECT_TRUE(s.parent_id().empty());
+    EXPECT_TRUE(s.type().empty());
     EXPECT_FALSE(s.http().request().fields().contains(Tracing::Tags::get().Status));
   };
 
@@ -270,6 +275,7 @@ TEST_F(XRayTracerTest, SerializeSpanTestWithStatusCodeNotANumber) {
     EXPECT_FALSE(s.trace_id().empty());
     EXPECT_FALSE(s.id().empty());
     EXPECT_TRUE(s.parent_id().empty());
+    EXPECT_TRUE(s.type().empty());
     EXPECT_FALSE(s.http().request().fields().contains(Tracing::Tags::get().Status));
     EXPECT_FALSE(s.http().request().fields().contains("content_length"));
   };
@@ -347,6 +353,8 @@ TEST_F(XRayTracerTest, ChildSpanHasParentInfo) {
     // Hex encoded 64 bit identifier
     EXPECT_STREQ("00000000000003e7", s.parent_id().c_str());
     EXPECT_EQ(expected_->span_name, s.name().c_str());
+    EXPECT_TRUE(xray_parent_span->type().empty());
+    EXPECT_EQ(Subsegment, s.type().c_str());
     EXPECT_STREQ(xray_parent_span->traceId().c_str(), s.trace_id().c_str());
     EXPECT_STREQ("0000003d25bebe62", s.id().c_str());
   };

From 607c763d4c08cdeb4ca81f97a542967272bf3443 Mon Sep 17 00:00:00 2001
From: Rex Chang <58710378+rexnp@users.noreply.github.com>
Date: Tue, 1 Mar 2022 16:03:32 -0800
Subject: [PATCH 10/14] Xray subsegment (#3)

* xray tracer: set subsegment type for child spans

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

* adds test coverage

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

* updates xray subsegment name to use operation name (instead of parent's span name)

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

* updates doc

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

* updates doc

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>
---
 docs/root/version_history/current.rst       | 1 +
 source/extensions/tracers/xray/tracer.cc    | 2 +-
 source/extensions/tracers/xray/tracer.h     | 2 +-
 test/extensions/tracers/xray/tracer_test.cc | 2 +-
 4 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst
index b53bfa4846fa..6356d06305e3 100644
--- a/docs/root/version_history/current.rst
+++ b/docs/root/version_history/current.rst
@@ -38,6 +38,7 @@ Bug Fixes
 * tls: fix a bug while matching a certificate SAN with an exact value in ``match_typed_subject_alt_names`` of a listener where wildcard ``*`` character is not the only character of the dns label. Example, ``baz*.example.net`` and ``*baz.example.net`` and ``b*z.example.net`` will match ``baz1.example.net`` and ``foobaz.example.net`` and ``buzz.example.net``, respectively.
 * upstream: fix stack overflow when a cluster with large number of idle connections is removed.
 * xray: fix the AWS X-Ray tracer extension to not sample the trace if ``sampled=`` keyword is not present in the header ``x-amzn-trace-id``.
+* xray: fix the AWS X-Ray tracer extension to annotate a child span with ``type=subsegment`` to correctly relate subsegments to a parent segment. Previously a subsegment would be treated as an independent segment.
 
 Removed Config or Runtime
 -------------------------
diff --git a/source/extensions/tracers/xray/tracer.cc b/source/extensions/tracers/xray/tracer.cc
index e8d6bcdd124d..3cc7f770ce64 100644
--- a/source/extensions/tracers/xray/tracer.cc
+++ b/source/extensions/tracers/xray/tracer.cc
@@ -110,7 +110,7 @@ void Span::injectContext(Tracing::TraceContext& trace_context) {
 Tracing::SpanPtr Span::spawnChild(const Tracing::Config& config, const std::string& operation_name,
                                   Envoy::SystemTime start_time) {
   auto child_span = std::make_unique<XRay::Span>(time_source_, random_, broker_);
-  child_span->setName(name());
+  child_span->setName(operation_name);
   child_span->setOperation(operation_name);
   child_span->setDirection(Tracing::HttpTracerUtility::toString(config.operationName()));
   child_span->setStartTime(start_time);
diff --git a/source/extensions/tracers/xray/tracer.h b/source/extensions/tracers/xray/tracer.h
index e156eab4665e..8fe8bdd457e1 100644
--- a/source/extensions/tracers/xray/tracer.h
+++ b/source/extensions/tracers/xray/tracer.h
@@ -103,7 +103,7 @@ class Span : public Tracing::Span, Logger::Loggable<Logger::Id::config> {
   }
 
   /**
-   * Sets the type of the Span. In Xray, an independent subsegment has a type of ``subsegment``.
+   * Sets the type of the Span. In Xray, an independent subsegment has a type of "subsegment".
    * https://docs.aws.amazon.com/xray/latest/devguide/xray-api-segmentdocuments.html#api-segmentdocuments-subsegments
    */
   void setType(absl::string_view type) { type_ = std::string(type); }
diff --git a/test/extensions/tracers/xray/tracer_test.cc b/test/extensions/tracers/xray/tracer_test.cc
index d2b58951f1fe..6fa8cfb430ac 100644
--- a/test/extensions/tracers/xray/tracer_test.cc
+++ b/test/extensions/tracers/xray/tracer_test.cc
@@ -352,7 +352,7 @@ TEST_F(XRayTracerTest, ChildSpanHasParentInfo) {
     TestUtility::validate(s);
     // Hex encoded 64 bit identifier
     EXPECT_STREQ("00000000000003e7", s.parent_id().c_str());
-    EXPECT_EQ(expected_->span_name, s.name().c_str());
+    EXPECT_EQ(expected_->operation_name, s.name().c_str());
     EXPECT_TRUE(xray_parent_span->type().empty());
     EXPECT_EQ(Subsegment, s.type().c_str());
     EXPECT_STREQ(xray_parent_span->traceId().c_str(), s.trace_id().c_str());

From 491abf16841344d087e3bf5e637a7cf72a7f59a5 Mon Sep 17 00:00:00 2001
From: Rex Chang <58710378+rexnp@users.noreply.github.com>
Date: Tue, 1 Mar 2022 17:30:48 -0800
Subject: [PATCH 11/14] Xray subsegment (#4)

* xray tracer: set subsegment type for child spans

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

* adds test coverage

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

* updates xray subsegment name to use operation name (instead of parent's span name)

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

* updates doc

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

* updates doc

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

* adds to spell check dictionary

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>
---
 tools/spelling/spelling_dictionary.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt
index a4bffd6110f0..93d391425e7c 100644
--- a/tools/spelling/spelling_dictionary.txt
+++ b/tools/spelling/spelling_dictionary.txt
@@ -401,6 +401,7 @@ XFF
 XML
 XN
 XNOR
+XRay
 XSS
 Xray
 YAML

From 5be2062c54bc5602f07cea45d51cce674045f5b8 Mon Sep 17 00:00:00 2001
From: Rex Chang <58710378+rexnp@users.noreply.github.com>
Date: Wed, 2 Mar 2022 18:13:12 +0000
Subject: [PATCH 12/14] fixes spellcheck

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>
---
 source/extensions/tracers/xray/daemon.proto | 2 +-
 source/extensions/tracers/xray/tracer.h     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/source/extensions/tracers/xray/daemon.proto b/source/extensions/tracers/xray/daemon.proto
index 8f66f972684f..6d1f17e409a9 100644
--- a/source/extensions/tracers/xray/daemon.proto
+++ b/source/extensions/tracers/xray/daemon.proto
@@ -43,7 +43,7 @@ message Segment {
   }
   // Object containing one or more fields that X-Ray indexes for use with filter expressions.
   map<string, string> annotations = 8;
-  // Set type to "subsegment" when sending a child span so Xray treats it as a subsegment.
+  // Set type to "subsegment" when sending a child span so XRay treats it as a subsegment.
   // https://docs.aws.amazon.com/xray/latest/devguide/xray-api-segmentdocuments.html#api-segmentdocuments-subsegments
   string type = 14;
 }
diff --git a/source/extensions/tracers/xray/tracer.h b/source/extensions/tracers/xray/tracer.h
index 8fe8bdd457e1..cbb25ac3294b 100644
--- a/source/extensions/tracers/xray/tracer.h
+++ b/source/extensions/tracers/xray/tracer.h
@@ -103,7 +103,7 @@ class Span : public Tracing::Span, Logger::Loggable<Logger::Id::config> {
   }
 
   /**
-   * Sets the type of the Span. In Xray, an independent subsegment has a type of "subsegment".
+   * Sets the type of the Span. In XRay, an independent subsegment has a type of "subsegment".
    * https://docs.aws.amazon.com/xray/latest/devguide/xray-api-segmentdocuments.html#api-segmentdocuments-subsegments
    */
   void setType(absl::string_view type) { type_ = std::string(type); }

From a26b9b9ae521cf488b3af9c7471a6f41d034ca45 Mon Sep 17 00:00:00 2001
From: Rex Chang <58710378+rexnp@users.noreply.github.com>
Date: Wed, 2 Mar 2022 01:28:44 +0000
Subject: [PATCH 13/14] adds to spellcheck

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

xray tracer: set subsegment type for child spans (#2)

* xray tracer: set subsegment type for child spans

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

* adds test coverage

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

Xray subsegment (#3)

* xray tracer: set subsegment type for child spans

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

* adds test coverage

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

* updates xray subsegment name to use operation name (instead of parent's span name)

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

* updates doc

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

* updates doc

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

Xray subsegment (#4)

* xray tracer: set subsegment type for child spans

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

* adds test coverage

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

* updates xray subsegment name to use operation name (instead of parent's span name)

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

* updates doc

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

* updates doc

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

* adds to spell check dictionary

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

fixes spellcheck

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>
---
 docs/root/version_history/current.rst       |  1 +
 source/extensions/tracers/xray/daemon.proto |  3 +++
 source/extensions/tracers/xray/tracer.cc    |  7 +++++--
 source/extensions/tracers/xray/tracer.h     | 13 +++++++++++++
 test/extensions/tracers/xray/tracer_test.cc | 10 +++++++++-
 tools/spelling/spelling_dictionary.txt      |  3 +++
 6 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst
index b53bfa4846fa..6356d06305e3 100644
--- a/docs/root/version_history/current.rst
+++ b/docs/root/version_history/current.rst
@@ -38,6 +38,7 @@ Bug Fixes
 * tls: fix a bug while matching a certificate SAN with an exact value in ``match_typed_subject_alt_names`` of a listener where wildcard ``*`` character is not the only character of the dns label. Example, ``baz*.example.net`` and ``*baz.example.net`` and ``b*z.example.net`` will match ``baz1.example.net`` and ``foobaz.example.net`` and ``buzz.example.net``, respectively.
 * upstream: fix stack overflow when a cluster with large number of idle connections is removed.
 * xray: fix the AWS X-Ray tracer extension to not sample the trace if ``sampled=`` keyword is not present in the header ``x-amzn-trace-id``.
+* xray: fix the AWS X-Ray tracer extension to annotate a child span with ``type=subsegment`` to correctly relate subsegments to a parent segment. Previously a subsegment would be treated as an independent segment.
 
 Removed Config or Runtime
 -------------------------
diff --git a/source/extensions/tracers/xray/daemon.proto b/source/extensions/tracers/xray/daemon.proto
index 2054bced7630..6d1f17e409a9 100644
--- a/source/extensions/tracers/xray/daemon.proto
+++ b/source/extensions/tracers/xray/daemon.proto
@@ -43,6 +43,9 @@ message Segment {
   }
   // Object containing one or more fields that X-Ray indexes for use with filter expressions.
   map<string, string> annotations = 8;
+  // Set type to "subsegment" when sending a child span so XRay treats it as a subsegment.
+  // https://docs.aws.amazon.com/xray/latest/devguide/xray-api-segmentdocuments.html#api-segmentdocuments-subsegments
+  string type = 14;
 }
 
 message Header {
diff --git a/source/extensions/tracers/xray/tracer.cc b/source/extensions/tracers/xray/tracer.cc
index b6bfb4da427e..3cc7f770ce64 100644
--- a/source/extensions/tracers/xray/tracer.cc
+++ b/source/extensions/tracers/xray/tracer.cc
@@ -71,7 +71,9 @@ void Span::finishSpan() {
   s.set_error(clientError());
   s.set_fault(serverError());
   s.set_throttle(isThrottled());
-
+  if (type() == Subsegment) {
+    s.set_type(std::string(Subsegment));
+  }
   auto* aws = s.mutable_aws()->mutable_fields();
   for (const auto& field : aws_metadata_) {
     aws->insert({field.first, field.second});
@@ -108,13 +110,14 @@ void Span::injectContext(Tracing::TraceContext& trace_context) {
 Tracing::SpanPtr Span::spawnChild(const Tracing::Config& config, const std::string& operation_name,
                                   Envoy::SystemTime start_time) {
   auto child_span = std::make_unique<XRay::Span>(time_source_, random_, broker_);
-  child_span->setName(name());
+  child_span->setName(operation_name);
   child_span->setOperation(operation_name);
   child_span->setDirection(Tracing::HttpTracerUtility::toString(config.operationName()));
   child_span->setStartTime(start_time);
   child_span->setParentId(id());
   child_span->setTraceId(traceId());
   child_span->setSampled(sampled());
+  child_span->setType(Subsegment);
   return child_span;
 }
 
diff --git a/source/extensions/tracers/xray/tracer.h b/source/extensions/tracers/xray/tracer.h
index f53c4ca91a12..cbb25ac3294b 100644
--- a/source/extensions/tracers/xray/tracer.h
+++ b/source/extensions/tracers/xray/tracer.h
@@ -26,6 +26,7 @@ namespace Tracers {
 namespace XRay {
 
 constexpr auto XRayTraceHeader = "x-amzn-trace-id";
+constexpr absl::string_view Subsegment = "subsegment";
 
 class Span : public Tracing::Span, Logger::Loggable<Logger::Id::config> {
 public:
@@ -101,6 +102,12 @@ class Span : public Tracing::Span, Logger::Loggable<Logger::Id::config> {
     parent_segment_id_ = std::string(parent_segment_id);
   }
 
+  /**
+   * Sets the type of the Span. In XRay, an independent subsegment has a type of "subsegment".
+   * https://docs.aws.amazon.com/xray/latest/devguide/xray-api-segmentdocuments.html#api-segmentdocuments-subsegments
+   */
+  void setType(absl::string_view type) { type_ = std::string(type); }
+
   /**
    * Sets the aws metadata field of the Span.
    */
@@ -156,6 +163,11 @@ class Span : public Tracing::Span, Logger::Loggable<Logger::Id::config> {
    */
   const std::string& direction() const { return direction_; }
 
+  /**
+   * Gets this Span's type.
+   */
+  const std::string& type() const { return type_; }
+
   /**
    * Gets this Span's name.
    */
@@ -216,6 +228,7 @@ class Span : public Tracing::Span, Logger::Loggable<Logger::Id::config> {
   std::string parent_segment_id_;
   std::string name_;
   std::string origin_;
+  std::string type_;
   absl::flat_hash_map<std::string, ProtobufWkt::Value> aws_metadata_;
   absl::flat_hash_map<std::string, ProtobufWkt::Value> http_request_annotations_;
   absl::flat_hash_map<std::string, ProtobufWkt::Value> http_response_annotations_;
diff --git a/test/extensions/tracers/xray/tracer_test.cc b/test/extensions/tracers/xray/tracer_test.cc
index f77d28e40724..6fa8cfb430ac 100644
--- a/test/extensions/tracers/xray/tracer_test.cc
+++ b/test/extensions/tracers/xray/tracer_test.cc
@@ -97,6 +97,7 @@ TEST_F(XRayTracerTest, SerializeSpanTest) {
     EXPECT_FALSE(s.id().empty());
     EXPECT_EQ(2, s.annotations().size());
     EXPECT_TRUE(s.parent_id().empty());
+    EXPECT_TRUE(s.type().empty());
     EXPECT_FALSE(s.fault());    /*server error*/
     EXPECT_FALSE(s.error());    /*client error*/
     EXPECT_FALSE(s.throttle()); /*request throttled*/
@@ -142,6 +143,7 @@ TEST_F(XRayTracerTest, SerializeSpanTestServerError) {
     EXPECT_FALSE(s.trace_id().empty());
     EXPECT_FALSE(s.id().empty());
     EXPECT_TRUE(s.parent_id().empty());
+    EXPECT_TRUE(s.type().empty());
     EXPECT_TRUE(s.fault());  /*server error*/
     EXPECT_FALSE(s.error()); /*client error*/
     EXPECT_EQ(expected_status_code,
@@ -175,6 +177,7 @@ TEST_F(XRayTracerTest, SerializeSpanTestClientError) {
     EXPECT_FALSE(s.trace_id().empty());
     EXPECT_FALSE(s.id().empty());
     EXPECT_TRUE(s.parent_id().empty());
+    EXPECT_TRUE(s.type().empty());
     EXPECT_FALSE(s.fault());    /*server error*/
     EXPECT_TRUE(s.error());     /*client error*/
     EXPECT_FALSE(s.throttle()); /*request throttled*/
@@ -208,6 +211,7 @@ TEST_F(XRayTracerTest, SerializeSpanTestClientErrorWithThrottle) {
     EXPECT_FALSE(s.trace_id().empty());
     EXPECT_FALSE(s.id().empty());
     EXPECT_TRUE(s.parent_id().empty());
+    EXPECT_TRUE(s.type().empty());
     EXPECT_FALSE(s.fault());   /*server error*/
     EXPECT_TRUE(s.error());    /*client error*/
     EXPECT_TRUE(s.throttle()); /*request throttled*/
@@ -239,6 +243,7 @@ TEST_F(XRayTracerTest, SerializeSpanTestWithEmptyValue) {
     EXPECT_FALSE(s.trace_id().empty());
     EXPECT_FALSE(s.id().empty());
     EXPECT_TRUE(s.parent_id().empty());
+    EXPECT_TRUE(s.type().empty());
     EXPECT_FALSE(s.http().request().fields().contains(Tracing::Tags::get().Status));
   };
 
@@ -270,6 +275,7 @@ TEST_F(XRayTracerTest, SerializeSpanTestWithStatusCodeNotANumber) {
     EXPECT_FALSE(s.trace_id().empty());
     EXPECT_FALSE(s.id().empty());
     EXPECT_TRUE(s.parent_id().empty());
+    EXPECT_TRUE(s.type().empty());
     EXPECT_FALSE(s.http().request().fields().contains(Tracing::Tags::get().Status));
     EXPECT_FALSE(s.http().request().fields().contains("content_length"));
   };
@@ -346,7 +352,9 @@ TEST_F(XRayTracerTest, ChildSpanHasParentInfo) {
     TestUtility::validate(s);
     // Hex encoded 64 bit identifier
     EXPECT_STREQ("00000000000003e7", s.parent_id().c_str());
-    EXPECT_EQ(expected_->span_name, s.name().c_str());
+    EXPECT_EQ(expected_->operation_name, s.name().c_str());
+    EXPECT_TRUE(xray_parent_span->type().empty());
+    EXPECT_EQ(Subsegment, s.type().c_str());
     EXPECT_STREQ(xray_parent_span->traceId().c_str(), s.trace_id().c_str());
     EXPECT_STREQ("0000003d25bebe62", s.id().c_str());
   };
diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt
index 4eb45381497c..93d391425e7c 100644
--- a/tools/spelling/spelling_dictionary.txt
+++ b/tools/spelling/spelling_dictionary.txt
@@ -401,7 +401,9 @@ XFF
 XML
 XN
 XNOR
+XRay
 XSS
+Xray
 YAML
 ZXID
 absl
@@ -1176,6 +1178,7 @@ subnets
 suboptimal
 subsecond
 subseconds
+subsegment
 subsequence
 subsetting
 substr

From 83e97f89a0fc0dc97c53af96f53dad9a63d94b10 Mon Sep 17 00:00:00 2001
From: Rex Chang <58710378+rexnp@users.noreply.github.com>
Date: Fri, 11 Mar 2022 18:33:06 +0000
Subject: [PATCH 14/14] fixes spell check

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>
---
 source/extensions/tracers/xray/daemon.proto | 2 +-
 source/extensions/tracers/xray/tracer.h     | 2 +-
 tools/spelling/spelling_dictionary.txt      | 2 --
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/source/extensions/tracers/xray/daemon.proto b/source/extensions/tracers/xray/daemon.proto
index 6d1f17e409a9..02c853cb47b4 100644
--- a/source/extensions/tracers/xray/daemon.proto
+++ b/source/extensions/tracers/xray/daemon.proto
@@ -43,7 +43,7 @@ message Segment {
   }
   // Object containing one or more fields that X-Ray indexes for use with filter expressions.
   map<string, string> annotations = 8;
-  // Set type to "subsegment" when sending a child span so XRay treats it as a subsegment.
+  // Set type to "subsegment" when sending a child span so X-Ray treats it as a subsegment.
   // https://docs.aws.amazon.com/xray/latest/devguide/xray-api-segmentdocuments.html#api-segmentdocuments-subsegments
   string type = 14;
 }
diff --git a/source/extensions/tracers/xray/tracer.h b/source/extensions/tracers/xray/tracer.h
index cbb25ac3294b..81206cc2fa2f 100644
--- a/source/extensions/tracers/xray/tracer.h
+++ b/source/extensions/tracers/xray/tracer.h
@@ -103,7 +103,7 @@ class Span : public Tracing::Span, Logger::Loggable<Logger::Id::config> {
   }
 
   /**
-   * Sets the type of the Span. In XRay, an independent subsegment has a type of "subsegment".
+   * Sets the type of the Span. In X-Ray, an independent subsegment has a type of "subsegment".
    * https://docs.aws.amazon.com/xray/latest/devguide/xray-api-segmentdocuments.html#api-segmentdocuments-subsegments
    */
   void setType(absl::string_view type) { type_ = std::string(type); }
diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt
index 93d391425e7c..8c25806f93d1 100644
--- a/tools/spelling/spelling_dictionary.txt
+++ b/tools/spelling/spelling_dictionary.txt
@@ -401,9 +401,7 @@ XFF
 XML
 XN
 XNOR
-XRay
 XSS
-Xray
 YAML
 ZXID
 absl