Skip to content

Commit

Permalink
Update Envoy to 38c5c86 (Mar 25, 2024) (#1110)
Browse files Browse the repository at this point in the history
- `.bazelrc` updated from upstream
- `tools/code_format/config.yaml` updated from upstream
- In an incredibly confusing refactor:
  - Envoy's `FaultFilterConfig` started taking a `CommonFactoryContext` instead of a `Runtime` and `TimeSource`. 
      - Updated `HttpDynamicDelayDecoderFilterConfig` to store a `CommonFactoryContext&` instead of a `TimeSource&`, so that it has a `CommonFactoryContext&` to use when constructing a `FaultFilterConfig`.
        - How to get a `CommonFactoryContext` when creating the `HttpDynamicDelayDecoderFilterConfig`? `HttpDynamicDelayDecoderFilterConfigFactory` has access to a `Envoy::Server::Configuration::FactoryContext`, which has a `serverFactoryContext()` method. `ServerFactoryContext` is a subclass of `CommonFactoryContext`.
  - Envoy has started calling `serverFactoryContext()` on the `NighthawkServerInstance`, which had a `PANIC` placeholder there. This was discovered in the integration tests.
    - Gave `NighthawkServerInstance` a reference to a `NighthawkServerFactoryContext` to return in  `serverFactoryContext()`, replacing the `PANIC` placeholder.
    - In order to remove a circular dependency, took away `NighthawkServerFactoryContext`'s reference to a  `NighthawkServerInstance`, to which it delegated many calls. Now `NighthawkServerFactoryContext` stores its own references to the underlying objects formerly wrapped in the `NighthawkServerInstance`.
  - I originally assumed the above two things were related, but it appears that both arose here by coincidence. 

Signed-off-by: eric846 <[email protected]>
  • Loading branch information
eric846 authored Mar 28, 2024
1 parent 6f3a946 commit 7a3e602
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 55 deletions.
4 changes: 2 additions & 2 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ build:compile-time-options --@envoy//source/extensions/filters/http/kill_request

# Docker sandbox
# NOTE: Update this from https://github.com/envoyproxy/envoy-build-tools/blob/main/toolchains/rbe_toolchains_config.bzl#L8
build:docker-sandbox --experimental_docker_image=envoyproxy/envoy-build-ubuntu:0ca52447572ee105a4730da5e76fe47c9c5a7c64@sha256:d736c58f06f36848e7966752cc7e01519cc1b5101a178d5c6634807e8ac3deab
build:docker-sandbox --experimental_docker_image=envoyproxy/envoy-build-ubuntu:f94a38f62220a2b017878b790b6ea98a0f6c5f9c@sha256:2dd96b6f43c08ccabd5f4747fce5854f5f96af509b32e5cf6493f136e9833649
build:docker-sandbox --spawn_strategy=docker
build:docker-sandbox --strategy=Javac=docker
build:docker-sandbox --strategy=Closure=docker
Expand Down Expand Up @@ -534,7 +534,7 @@ build:bes-envoy-engflow --bes_upload_mode=fully_async
build:rbe-envoy-engflow --config=cache-envoy-engflow
build:rbe-envoy-engflow --config=bes-envoy-engflow
build:rbe-envoy-engflow --remote_executor=grpcs://morganite.cluster.engflow.com
build:rbe-envoy-engflow --remote_default_exec_properties=container-image=docker://docker.io/envoyproxy/envoy-build-ubuntu:0ca52447572ee105a4730da5e76fe47c9c5a7c64@sha256:d736c58f06f36848e7966752cc7e01519cc1b5101a178d5c6634807e8ac3deab
build:rbe-envoy-engflow --remote_default_exec_properties=container-image=docker://docker.io/envoyproxy/envoy-build-ubuntu:f94a38f62220a2b017878b790b6ea98a0f6c5f9c@sha256:2dd96b6f43c08ccabd5f4747fce5854f5f96af509b32e5cf6493f136e9833649

#############################################################################
# debug: Various Bazel debugging flags
Expand Down
4 changes: 2 additions & 2 deletions bazel/repositories.bzl
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

ENVOY_COMMIT = "efd9bef80764663be31b115301812a5bb647fad5"
ENVOY_SHA = "424017e3dd804d3e0ec46aa84e009c413fb6a51ade43ac8339b5a73e1a5d3107"
ENVOY_COMMIT = "38c5c867ab92c60bb0fc32bde0f6565516ba0ff4"
ENVOY_SHA = "f6fb93e9c11c13bbee2234ffc43c6f525f3dd1294516885559bc930211f9b336"

HDR_HISTOGRAM_C_VERSION = "0.11.2" # October 12th, 2020
HDR_HISTOGRAM_C_SHA = "637f28b5f64de2e268131e4e34e6eef0b91cf5ff99167db447d9b2825eae6bad"
Expand Down
99 changes: 61 additions & 38 deletions source/client/process_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -130,20 +130,20 @@ class StatsConfigImpl : public Envoy::Server::Configuration::StatsConfig {
// when Nighthawk is running are implemented.
class NighthawkServerInstance : public Envoy::Server::Instance {
public:
NighthawkServerInstance(Envoy::OptRef<Envoy::Server::Admin> admin, Envoy::Api::Api& api,
Envoy::Event::Dispatcher& dispatcher,
Envoy::AccessLog::AccessLogManager& log_manager,
Envoy::Server::Options& options, Envoy::Runtime::Loader& runtime,
Envoy::Singleton::Manager& singleton_manager,
Envoy::ThreadLocal::Instance& tls,
Envoy::LocalInfo::LocalInfo& local_info,
Envoy::ProtobufMessage::ProdValidationContextImpl& validation_context,
Envoy::Grpc::Context& grpc_context,
Envoy::Router::Context& router_context)
NighthawkServerInstance(
Envoy::OptRef<Envoy::Server::Admin> admin, Envoy::Api::Api& api,
Envoy::Event::Dispatcher& dispatcher, Envoy::AccessLog::AccessLogManager& log_manager,
Envoy::Server::Options& options, Envoy::Runtime::Loader& runtime,
Envoy::Singleton::Manager& singleton_manager, Envoy::ThreadLocal::Instance& tls,
Envoy::LocalInfo::LocalInfo& local_info,
Envoy::ProtobufMessage::ProdValidationContextImpl& validation_context,
Envoy::Grpc::Context& grpc_context, Envoy::Router::Context& router_context,
Envoy::Server::Configuration::ServerFactoryContext& server_factory_context)
: admin_(admin), api_(api), dispatcher_(dispatcher), log_manager_(log_manager),
options_(options), runtime_(runtime), singleton_manager_(singleton_manager), tls_(tls),
local_info_(local_info), validation_context_(validation_context),
grpc_context_(grpc_context), router_context_(router_context) {}
grpc_context_(grpc_context), router_context_(router_context),
server_factory_context_(server_factory_context) {}

void run() override { PANIC("NighthawkServerInstance::run not implemented"); }
Envoy::OptRef<Envoy::Server::Admin> admin() override { return admin_; }
Expand Down Expand Up @@ -223,12 +223,14 @@ class NighthawkServerInstance : public Envoy::Server::Instance {
Envoy::ProtobufMessage::ValidationContext& messageValidationContext() override {
return validation_context_;
}
Envoy::Server::Configuration::StatsConfig& statsConfig() override { return stats_config_; }
Envoy::Server::Configuration::StatsConfig& statsConfig() override {
PANIC("NighthawkServerInstance::statsConfig not implemented");
}
envoy::config::bootstrap::v3::Bootstrap& bootstrap() override {
PANIC("NighthawkServerInstance::bootstrap not implemented");
}
Envoy::Server::Configuration::ServerFactoryContext& serverFactoryContext() override {
PANIC("NighthawkServerInstance::serverFactoryContext not implemented");
return server_factory_context_;
}
Envoy::Server::Configuration::TransportSocketFactoryContext&
transportSocketFactoryContext() override {
Expand Down Expand Up @@ -260,28 +262,39 @@ class NighthawkServerInstance : public Envoy::Server::Instance {
Envoy::ProtobufMessage::ProdValidationContextImpl& validation_context_;
Envoy::Grpc::Context& grpc_context_;
Envoy::Router::Context& router_context_;
StatsConfigImpl stats_config_;
Envoy::Server::Configuration::ServerFactoryContext& server_factory_context_;
};

// Implementation of Envoy::Server::Configuration::ServerFactoryContext.
class NighthawkServerFactoryContext : public Envoy::Server::Configuration::ServerFactoryContext {
public:
NighthawkServerFactoryContext(Envoy::Server::Instance& server, Envoy::Stats::Scope& server_scope)
: server_(server), server_scope_(server_scope) {}
NighthawkServerFactoryContext(
Envoy::OptRef<Envoy::Server::Admin> admin, Envoy::Api::Api& api,
Envoy::Event::Dispatcher& dispatcher, Envoy::AccessLog::AccessLogManager& log_manager,
Envoy::Server::Options& options, Envoy::Runtime::Loader& runtime,
Envoy::Singleton::Manager& singleton_manager, Envoy::ThreadLocal::Instance& tls,
Envoy::LocalInfo::LocalInfo& local_info,
Envoy::ProtobufMessage::ProdValidationContextImpl& validation_context,
Envoy::Grpc::Context& grpc_context, Envoy::Router::Context& router_context,
Envoy::Stats::Scope& server_scope)
: admin_(admin), api_(api), dispatcher_(dispatcher), log_manager_(log_manager),
options_(options), runtime_(runtime), singleton_manager_(singleton_manager), tls_(tls),
local_info_(local_info), validation_context_(validation_context),
grpc_context_(grpc_context), router_context_(router_context), server_scope_(server_scope) {}

const Envoy::Server::Options& options() override { return server_.options(); };
const Envoy::Server::Options& options() override { return options_; };

Envoy::Event::Dispatcher& mainThreadDispatcher() override { return server_.dispatcher(); }
Envoy::Event::Dispatcher& mainThreadDispatcher() override { return dispatcher_; }

Envoy::Api::Api& api() override { return server_.api(); }
Envoy::Api::Api& api() override { return api_; }

Envoy::LocalInfo::LocalInfo& localInfo() const override { return server_.localInfo(); }
Envoy::LocalInfo::LocalInfo& localInfo() const override { return local_info_; }

Envoy::OptRef<Envoy::Server::Admin> admin() override { return server_.admin(); }
Envoy::OptRef<Envoy::Server::Admin> admin() override { return admin_; }

Envoy::Runtime::Loader& runtime() override { return server_.runtime(); }
Envoy::Runtime::Loader& runtime() override { return runtime_; }

Envoy::Singleton::Manager& singletonManager() override { return server_.singletonManager(); }
Envoy::Singleton::Manager& singletonManager() override { return singleton_manager_; }

Envoy::ProtobufMessage::ValidationVisitor& messageValidationVisitor() override {
return Envoy::ProtobufMessage::getStrictValidationVisitor();
Expand All @@ -293,21 +306,19 @@ class NighthawkServerFactoryContext : public Envoy::Server::Configuration::Serve

Envoy::Stats::Scope& serverScope() override { return server_scope_; };

Envoy::ThreadLocal::SlotAllocator& threadLocal() override { return server_.threadLocal(); }
Envoy::ThreadLocal::SlotAllocator& threadLocal() override { return tls_; }

Envoy::Upstream::ClusterManager& clusterManager() override {
PANIC("NighthawkServerFactoryContext::clusterManager not implemented");
};

Envoy::ProtobufMessage::ValidationContext& messageValidationContext() override {
return server_.messageValidationContext();
return validation_context_;
};

Envoy::TimeSource& timeSource() override { return server_.timeSource(); };
Envoy::TimeSource& timeSource() override { return api_.timeSource(); };

Envoy::AccessLog::AccessLogManager& accessLogManager() override {
return server_.accessLogManager();
}
Envoy::AccessLog::AccessLogManager& accessLogManager() override { return log_manager_; }

Envoy::Server::ServerLifecycleNotifier& lifecycleNotifier() override {
PANIC("NighthawkServerFactoryContext::lifecycleNotifier not implemented");
Expand All @@ -321,9 +332,9 @@ class NighthawkServerFactoryContext : public Envoy::Server::Configuration::Serve
PANIC("NighthawkServerFactoryContext::initManager not implemented");
};

Envoy::Grpc::Context& grpcContext() override { return server_.grpcContext(); };
Envoy::Grpc::Context& grpcContext() override { return grpc_context_; };

Envoy::Router::Context& routerContext() override { return server_.routerContext(); };
Envoy::Router::Context& routerContext() override { return router_context_; };

Envoy::ProcessContextOptRef processContext() override {
PANIC("NighthawkServerFactoryContext::processContext not implemented");
Expand All @@ -333,9 +344,7 @@ class NighthawkServerFactoryContext : public Envoy::Server::Configuration::Serve
PANIC("NighthawkServerFactoryContext::drainManager not implemented");
};

Envoy::Server::Configuration::StatsConfig& statsConfig() override {
return server_.statsConfig();
}
Envoy::Server::Configuration::StatsConfig& statsConfig() override { return stats_config_; }

envoy::config::bootstrap::v3::Bootstrap& bootstrap() override {
PANIC("NighthawkServerFactoryContext::bootstrap not implemented");
Expand All @@ -354,7 +363,19 @@ class NighthawkServerFactoryContext : public Envoy::Server::Configuration::Serve
}

private:
Envoy::Server::Instance& server_;
Envoy::OptRef<Envoy::Server::Admin> admin_;
Envoy::Api::Api& api_;
Envoy::Event::Dispatcher& dispatcher_;
Envoy::AccessLog::AccessLogManager& log_manager_;
Envoy::Server::Options& options_;
Envoy::Runtime::Loader& runtime_;
Envoy::Singleton::Manager& singleton_manager_;
Envoy::ThreadLocal::Instance& tls_;
Envoy::LocalInfo::LocalInfo& local_info_;
Envoy::ProtobufMessage::ProdValidationContextImpl& validation_context_;
Envoy::Grpc::Context& grpc_context_;
Envoy::Router::Context& router_context_;
StatsConfigImpl stats_config_; // Using the default value.
Envoy::Stats::Scope& server_scope_;
};

Expand Down Expand Up @@ -832,12 +853,14 @@ bool ProcessImpl::runInternal(OutputCollector& collector, const UriPtr& tracing_
return false;
}

server_factory_context_ = std::make_unique<NighthawkServerFactoryContext>(
admin_, *api_, *dispatcher_, access_log_manager_, envoy_options_, *runtime_loader_.get(),
*singleton_manager_, tls_, *local_info_, validation_context_, grpc_context_,
router_context_, scope_root_);
server_ = std::make_unique<NighthawkServerInstance>(
admin_, *api_, *dispatcher_, access_log_manager_, envoy_options_, *runtime_loader_.get(),
*singleton_manager_, tls_, *local_info_, validation_context_, grpc_context_,
router_context_);
server_factory_context_ =
std::make_unique<NighthawkServerFactoryContext>(*server_, scope_root_);
router_context_, *server_factory_context_);
ssl_context_manager_ =
std::make_unique<Envoy::Extensions::TransportSockets::Tls::ContextManagerImpl>(
*server_factory_context_);
Expand Down
7 changes: 4 additions & 3 deletions source/server/http_dynamic_delay_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,11 @@ computeEffectiveConfiguration(std::shared_ptr<const DynamicDelayConfiguration> b

HttpDynamicDelayDecoderFilterConfig::HttpDynamicDelayDecoderFilterConfig(
const DynamicDelayConfiguration& proto_config, Envoy::Runtime::Loader& runtime,
const std::string& stats_prefix, Envoy::Stats::Scope& scope, Envoy::TimeSource& time_source)
const std::string& stats_prefix, Envoy::Stats::Scope& scope,
Envoy::Server::Configuration::CommonFactoryContext& common_factory_context)
: FilterConfigurationBase("dynamic-delay"), runtime_(runtime),
stats_prefix_(absl::StrCat(stats_prefix, fmt::format("{}.", filter_name()))), scope_(scope),
time_source_(time_source),
common_factory_context_(common_factory_context),
server_config_(std::make_shared<DynamicDelayConfiguration>(proto_config)) {}

std::shared_ptr<const DynamicDelayConfiguration>
Expand Down Expand Up @@ -165,7 +166,7 @@ HttpDynamicDelayDecoderFilter::translateOurConfigIntoFaultFilterConfig(
fault_config.mutable_delay()->mutable_percentage()->set_numerator(100);
fault_config.mutable_delay()->mutable_header_delay();
return std::make_shared<Envoy::Extensions::HttpFilters::Fault::FaultFilterConfig>(
fault_config, config.runtime(), config.stats_prefix(), config.scope(), config.time_source());
fault_config, config.stats_prefix(), config.scope(), config.common_factory_context());
}

void HttpDynamicDelayDecoderFilter::setDecoderFilterCallbacks(
Expand Down
13 changes: 8 additions & 5 deletions source/server/http_dynamic_delay_filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ class HttpDynamicDelayDecoderFilterConfig : public FilterConfigurationBase {
* @param stats_prefix Prefix to use by the filter when it names statistics. E.g.
* dynamic-delay.fault.delays_injected: 1
* @param scope Statistics scope to be used by the filter.
* @param time_source Time source to be used by the filter.
* @param common_factory_context CommonFactoryContext to be used by the filter.
*/
HttpDynamicDelayDecoderFilterConfig(
const nighthawk::server::DynamicDelayConfiguration& proto_config,
Envoy::Runtime::Loader& runtime, const std::string& stats_prefix, Envoy::Stats::Scope& scope,
Envoy::TimeSource& time_source);
Envoy::Server::Configuration::CommonFactoryContext& common_factory_context);
/**
* Increments the number of globally active filter instances.
*/
Expand Down Expand Up @@ -67,9 +67,12 @@ class HttpDynamicDelayDecoderFilterConfig : public FilterConfigurationBase {
Envoy::Stats::Scope& scope() { return scope_; }

/**
* @return Envoy::TimeSource& to be used by filter instantiations associated to this.
* @return Envoy::Server::Configuration::CommonFactoryContext& to be used by filter
* instantiations associated to this.
*/
Envoy::TimeSource& time_source() { return time_source_; }
Envoy::Server::Configuration::CommonFactoryContext& common_factory_context() {
return common_factory_context_;
}

/**
* @return std::string to be used by filter instantiations associated to this.
Expand All @@ -92,7 +95,7 @@ class HttpDynamicDelayDecoderFilterConfig : public FilterConfigurationBase {
Envoy::Runtime::Loader& runtime_;
const std::string stats_prefix_;
Envoy::Stats::Scope& scope_;
Envoy::TimeSource& time_source_;
Envoy::Server::Configuration::CommonFactoryContext& common_factory_context_;
std::shared_ptr<const nighthawk::server::DynamicDelayConfiguration> server_config_;
};

Expand Down
2 changes: 1 addition & 1 deletion source/server/http_dynamic_delay_filter_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class HttpDynamicDelayDecoderFilterConfigFactory
std::make_shared<Nighthawk::Server::HttpDynamicDelayDecoderFilterConfig>(
Nighthawk::Server::HttpDynamicDelayDecoderFilterConfig(
proto_config, context.serverFactoryContext().runtime(), "" /*stats_prefix*/,
context.scope(), context.serverFactoryContext().timeSource()));
context.scope(), context.serverFactoryContext()));

return [config](Envoy::Http::FilterChainFactoryCallbacks& callbacks) -> void {
auto* filter = new Nighthawk::Server::HttpDynamicDelayDecoderFilter(config);
Expand Down
7 changes: 3 additions & 4 deletions tools/code_format/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -133,19 +133,16 @@ paths:
- source/common/tcp_proxy/tcp_proxy.cc
- source/common/config/subscription_factory_impl.cc
- source/common/config/xds_resource.cc
- source/common/runtime/runtime_impl.cc
- source/common/filter/config_discovery_impl.cc
- source/common/json/json_internal.cc
- source/common/router/scoped_rds.cc
- source/common/router/config_impl.cc
- source/common/router/scoped_config_impl.cc
- source/common/router/router_ratelimit.cc
- source/common/router/header_parser.cc
- source/common/filesystem/inotify/watcher_impl.cc
- source/common/filesystem/posix/directory_iterator_impl.cc
- source/common/filesystem/inotify/watcher_impl.cc
- source/common/filesystem/kqueue/watcher_impl.cc
- source/common/filesystem/win32/directory_iterator_impl.cc
- source/common/filesystem/win32/watcher_impl.cc
- source/common/common/utility.cc
- source/common/common/regex.cc
- source/common/common/matchers.cc
Expand All @@ -154,6 +151,7 @@ paths:
- source/server/overload_manager_impl.cc
- source/server/config_validation/server.cc
- source/server/admin/html/active_stats.js
- source/server/admin/runtime_handler.cc
- source/server/server.cc
- source/server/hot_restarting_base.cc
- source/server/hot_restart_impl.cc
Expand All @@ -173,6 +171,7 @@ paths:
- source/common/local_reply/local_reply.cc
- source/common/tls/context_impl.cc
- source/common/tls/context_config_impl.cc
- source/common/config/watched_directory.cc

# Only one C++ file should instantiate grpc_init
grpc_init:
Expand Down

0 comments on commit 7a3e602

Please sign in to comment.