From 5a81f18962c06c41ef5c6d8eb621532d7939b22a Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Thu, 5 Jan 2017 21:50:11 -0800 Subject: [PATCH 1/3] local info: refactor information about the local env This has been bothering me for a while. We are copying things like local zone, local cluster, etc. all over the place. It's also likely that we will add more things in the future. This introduces a LocalInfo abstract class which holds such information. The change is large by number of lines but it's simple. --- include/envoy/local_info/local_info.h | 20 ++++++++++++++ include/envoy/server/instance.h | 5 ++-- include/envoy/server/options.h | 15 ----------- include/envoy/upstream/cluster_manager.h | 1 - source/common/http/async_client_impl.cc | 14 +++++----- source/common/http/async_client_impl.h | 6 ++--- source/common/http/filter/ratelimit.cc | 4 +-- source/common/http/filter/ratelimit.h | 10 +++---- source/common/local_info/local_info_impl.h | 26 +++++++++++++++++++ source/common/router/router.cc | 18 ++++++------- source/common/router/router.h | 11 ++++---- source/common/stats/statsd.cc | 6 ++--- source/common/stats/statsd.h | 9 +++---- source/common/tracing/http_tracer_impl.cc | 6 ++--- source/common/tracing/http_tracer_impl.h | 8 +++--- .../common/upstream/cluster_manager_impl.cc | 15 +++++------ source/common/upstream/cluster_manager_impl.h | 15 ++++++----- source/common/upstream/sds.cc | 16 +++++++----- source/common/upstream/sds.h | 6 +++-- source/common/upstream/upstream_impl.cc | 3 ++- source/common/upstream/upstream_impl.h | 2 ++ source/exe/main.cc | 6 ++++- source/server/config/http/ratelimit.cc | 2 +- source/server/config/http/router.cc | 4 +-- .../config/network/http_connection_manager.cc | 6 +++-- source/server/configuration_impl.cc | 15 +++++------ source/server/http/admin.cc | 2 +- source/server/http/health_check.cc | 2 +- source/server/options_impl.h | 7 ++--- source/server/server.cc | 17 ++++++------ source/server/server.h | 7 ++--- test/CMakeLists.txt | 1 + test/common/http/async_client_impl_test.cc | 12 ++++----- test/common/http/filter/ratelimit_test.cc | 4 ++- test/common/router/router_test.cc | 24 +++++++++-------- test/common/stats/statsd_test.cc | 4 ++- test/common/tracing/http_tracer_impl_test.cc | 4 ++- .../upstream/cluster_manager_impl_test.cc | 8 +++--- test/common/upstream/sds_test.cc | 9 ++++--- test/integration/server.cc | 5 +++- test/integration/server.h | 6 ----- test/mocks/local_info/mocks.cc | 16 ++++++++++++ test/mocks/local_info/mocks.h | 23 ++++++++++++++++ test/mocks/server/mocks.cc | 8 ++---- test/mocks/server/mocks.h | 9 +++---- test/server/http/health_check_test.cc | 2 -- 46 files changed, 251 insertions(+), 168 deletions(-) create mode 100644 include/envoy/local_info/local_info.h create mode 100644 source/common/local_info/local_info_impl.h create mode 100644 test/mocks/local_info/mocks.cc create mode 100644 test/mocks/local_info/mocks.h diff --git a/include/envoy/local_info/local_info.h b/include/envoy/local_info/local_info.h new file mode 100644 index 0000000000000..f77c9ede3bd71 --- /dev/null +++ b/include/envoy/local_info/local_info.h @@ -0,0 +1,20 @@ +#pragma once + +#include "envoy/common/pure.h" + +namespace LocalInfo { + +/** + * + */ +class LocalInfo { +public: + virtual ~LocalInfo() {} + + virtual const std::string& address() const PURE; + virtual const std::string& zoneName() const PURE; + virtual const std::string& clusterName() const PURE; + virtual const std::string& nodeName() const PURE; +}; + +} // LocalInfo diff --git a/include/envoy/server/instance.h b/include/envoy/server/instance.h index 071c9d1d73090..63bb625415e68 100644 --- a/include/envoy/server/instance.h +++ b/include/envoy/server/instance.h @@ -2,6 +2,7 @@ #include "envoy/access_log/access_log.h" #include "envoy/api/api.h" +#include "envoy/local_info/local_info.h" #include "envoy/ratelimit/ratelimit.h" #include "envoy/runtime/runtime.h" #include "envoy/server/admin.h" @@ -160,9 +161,9 @@ class Instance { virtual ThreadLocal::Instance& threadLocal() PURE; /** - * @return the local IP address of the server + * @return information about the local environment the server is running in. */ - virtual const std::string& getLocalAddress() PURE; + virtual const LocalInfo::LocalInfo& localInfo() PURE; }; } // Server diff --git a/include/envoy/server/options.h b/include/envoy/server/options.h index 1f57c5325fb9b..11f0631273989 100644 --- a/include/envoy/server/options.h +++ b/include/envoy/server/options.h @@ -50,21 +50,6 @@ class Options { */ virtual uint64_t restartEpoch() PURE; - /** - * @return const std::string& the service cluster name where the server is running. - */ - virtual const std::string& serviceClusterName() PURE; - - /** - * @return const std::string& the service node name where the server is running. - */ - virtual const std::string& serviceNodeName() PURE; - - /** - * @return const std::string& the service zone where the server is running. - */ - virtual const std::string& serviceZone() PURE; - /** * @return std::chrono::milliseconds the duration in msec between log flushes. */ diff --git a/include/envoy/upstream/cluster_manager.h b/include/envoy/upstream/cluster_manager.h index d0de5ffa0c526..000cf7ef6ccf1 100644 --- a/include/envoy/upstream/cluster_manager.h +++ b/include/envoy/upstream/cluster_manager.h @@ -90,7 +90,6 @@ class ClusterManager { * Global configuration for any SDS clusters. */ struct SdsConfig { - std::string local_zone_name_; std::string sds_cluster_name_; std::chrono::milliseconds refresh_delay_; }; diff --git a/source/common/http/async_client_impl.cc b/source/common/http/async_client_impl.cc index 36a9708d46b46..802521e15f2fc 100644 --- a/source/common/http/async_client_impl.cc +++ b/source/common/http/async_client_impl.cc @@ -11,14 +11,14 @@ const AsyncRequestImpl::NullShadowPolicy AsyncRequestImpl::RouteEntryImpl::shado const AsyncRequestImpl::NullVirtualHost AsyncRequestImpl::RouteEntryImpl::virtual_host_; AsyncClientImpl::AsyncClientImpl(const Upstream::ClusterInfo& cluster, Stats::Store& stats_store, - Event::Dispatcher& dispatcher, const std::string& local_zone_name, + Event::Dispatcher& dispatcher, + const LocalInfo::LocalInfo& local_info, Upstream::ClusterManager& cm, Runtime::Loader& runtime, Runtime::RandomGenerator& random, - Router::ShadowWriterPtr&& shadow_writer, - const std::string& local_address) - : cluster_(cluster), config_("http.async-client.", local_zone_name, stats_store, cm, runtime, - random, std::move(shadow_writer), true), - dispatcher_(dispatcher), local_address_(local_address) {} + Router::ShadowWriterPtr&& shadow_writer) + : cluster_(cluster), config_("http.async-client.", local_info, stats_store, cm, runtime, random, + std::move(shadow_writer), true), + dispatcher_(dispatcher) {} AsyncClientImpl::~AsyncClientImpl() { while (!active_requests_.empty()) { @@ -50,7 +50,7 @@ AsyncRequestImpl::AsyncRequestImpl(MessagePtr&& request, AsyncClientImpl& parent router_.setDecoderFilterCallbacks(*this); request_->headers().insertEnvoyInternalRequest().value( Headers::get().EnvoyInternalRequestValues.True); - request_->headers().insertForwardedFor().value(parent_.local_address_); + request_->headers().insertForwardedFor().value(parent_.config_.local_info_.address()); router_.decodeHeaders(request_->headers(), !request_->body()); if (!complete_ && request_->body()) { router_.decodeData(*request_->body(), true); diff --git a/source/common/http/async_client_impl.h b/source/common/http/async_client_impl.h index 257e498d750be..3d8d95cbcd460 100644 --- a/source/common/http/async_client_impl.h +++ b/source/common/http/async_client_impl.h @@ -23,10 +23,9 @@ class AsyncRequestImpl; class AsyncClientImpl final : public AsyncClient { public: AsyncClientImpl(const Upstream::ClusterInfo& cluster, Stats::Store& stats_store, - Event::Dispatcher& dispatcher, const std::string& local_zone_name, + Event::Dispatcher& dispatcher, const LocalInfo::LocalInfo& local_info, Upstream::ClusterManager& cm, Runtime::Loader& runtime, - Runtime::RandomGenerator& random, Router::ShadowWriterPtr&& shadow_writer, - const std::string& local_address); + Runtime::RandomGenerator& random, Router::ShadowWriterPtr&& shadow_writer); ~AsyncClientImpl(); // Http::AsyncClient @@ -38,7 +37,6 @@ class AsyncClientImpl final : public AsyncClient { Router::FilterConfig config_; Event::Dispatcher& dispatcher_; std::list> active_requests_; - const std::string local_address_; friend class AsyncRequestImpl; }; diff --git a/source/common/http/filter/ratelimit.cc b/source/common/http/filter/ratelimit.cc index cba706126b6c9..a6364e608401d 100644 --- a/source/common/http/filter/ratelimit.cc +++ b/source/common/http/filter/ratelimit.cc @@ -30,8 +30,8 @@ FilterHeadersStatus Filter::decodeHeaders(HeaderMap& headers, bool) { fmt::format("ratelimit.{}.http_filter_enabled", route_key), 100)) { continue; } - rate_limit.populateDescriptors(*route, descriptors, config_->localServiceCluster(), headers, - callbacks_->downstreamAddress()); + rate_limit.populateDescriptors(*route, descriptors, config_->localInfo().clusterName(), + headers, callbacks_->downstreamAddress()); } if (!descriptors.empty()) { diff --git a/source/common/http/filter/ratelimit.h b/source/common/http/filter/ratelimit.h index ca19fefc152f5..14d7822c024d9 100644 --- a/source/common/http/filter/ratelimit.h +++ b/source/common/http/filter/ratelimit.h @@ -1,6 +1,7 @@ #pragma once #include "envoy/http/filter.h" +#include "envoy/local_info/local_info.h" #include "envoy/ratelimit/ratelimit.h" #include "envoy/runtime/runtime.h" @@ -15,14 +16,13 @@ namespace RateLimit { */ class FilterConfig { public: - FilterConfig(const Json::Object& config, const std::string& local_service_cluster, + FilterConfig(const Json::Object& config, const LocalInfo::LocalInfo& local_info, Stats::Store& stats_store, Runtime::Loader& runtime) : domain_(config.getString("domain")), stage_(config.getInteger("stage", 0)), - local_service_cluster_(local_service_cluster), stats_store_(stats_store), - runtime_(runtime) {} + local_info_(local_info), stats_store_(stats_store), runtime_(runtime) {} const std::string& domain() const { return domain_; } - const std::string& localServiceCluster() const { return local_service_cluster_; } + const LocalInfo::LocalInfo& localInfo() const { return local_info_; } int64_t stage() const { return stage_; } Runtime::Loader& runtime() { return runtime_; } Stats::Store& stats() { return stats_store_; } @@ -30,7 +30,7 @@ class FilterConfig { private: const std::string domain_; int64_t stage_; - const std::string local_service_cluster_; + const LocalInfo::LocalInfo& local_info_; Stats::Store& stats_store_; Runtime::Loader& runtime_; }; diff --git a/source/common/local_info/local_info_impl.h b/source/common/local_info/local_info_impl.h new file mode 100644 index 0000000000000..9f2ae0aa7eb7c --- /dev/null +++ b/source/common/local_info/local_info_impl.h @@ -0,0 +1,26 @@ +#pragma once + +#include "envoy/local_info/local_info.h" + +namespace LocalInfo { + +class LocalInfoImpl : public LocalInfo { +public: + LocalInfoImpl(const std::string address, const std::string zone_name, + const std::string cluster_name, const std::string node_name) + : address_(address), zone_name_(zone_name), cluster_name_(cluster_name), + node_name_(node_name) {} + + const std::string& address() const override { return address_; } + const std::string& zoneName() const override { return zone_name_; } + const std::string& clusterName() const override { return cluster_name_; } + const std::string& nodeName() const override { return node_name_; } + +private: + const std::string address_; + const std::string zone_name_; + const std::string cluster_name_; + const std::string node_name_; +}; + +} // LocalInfo diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 5e4a5e93f3fb5..7777ad6d74777 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -109,14 +109,14 @@ void Filter::chargeUpstreamCode(const Http::HeaderMap& response_headers, Http::CodeUtility::ResponseStatInfo info{ config_.stats_store_, cluster_->statPrefix(), response_headers, internal_request, route_->virtualHost().name(), request_vcluster_ ? request_vcluster_->name() : "", - config_.service_zone_, upstreamZone(upstream_host), is_canary}; + config_.local_info_.zoneName(), upstreamZone(upstream_host), is_canary}; Http::CodeUtility::chargeResponseStat(info); for (const std::string& alt_prefix : alt_stat_prefixes_) { - Http::CodeUtility::ResponseStatInfo info{config_.stats_store_, alt_prefix, response_headers, - internal_request, "", "", config_.service_zone_, - upstreamZone(upstream_host), is_canary}; + Http::CodeUtility::ResponseStatInfo info{ + config_.stats_store_, alt_prefix, response_headers, internal_request, "", "", + config_.local_info_.zoneName(), upstreamZone(upstream_host), is_canary}; Http::CodeUtility::chargeResponseStat(info); } @@ -481,16 +481,16 @@ void Filter::onUpstreamComplete() { Http::CodeUtility::ResponseTimingInfo info{ config_.stats_store_, cluster_->statPrefix(), response_time, upstream_request_->upstream_canary_, internal_request, route_->virtualHost().name(), - request_vcluster_ ? request_vcluster_->name() : "", config_.service_zone_, + request_vcluster_ ? request_vcluster_->name() : "", config_.local_info_.zoneName(), upstreamZone(upstream_request_->upstream_host_)}; Http::CodeUtility::chargeResponseTiming(info); for (const std::string& alt_prefix : alt_stat_prefixes_) { - Http::CodeUtility::ResponseTimingInfo info{config_.stats_store_, alt_prefix, response_time, - upstream_request_->upstream_canary_, - internal_request, "", "", config_.service_zone_, - upstreamZone(upstream_request_->upstream_host_)}; + Http::CodeUtility::ResponseTimingInfo info{ + config_.stats_store_, alt_prefix, response_time, upstream_request_->upstream_canary_, + internal_request, "", "", config_.local_info_.zoneName(), + upstreamZone(upstream_request_->upstream_host_)}; Http::CodeUtility::chargeResponseTiming(info); } diff --git a/source/common/router/router.h b/source/common/router/router.h index 10cd63eafa3f5..fe239f9ae96e4 100644 --- a/source/common/router/router.h +++ b/source/common/router/router.h @@ -3,6 +3,7 @@ #include "envoy/http/codec.h" #include "envoy/http/codes.h" #include "envoy/http/filter.h" +#include "envoy/local_info/local_info.h" #include "envoy/router/shadow_writer.h" #include "envoy/runtime/runtime.h" #include "envoy/stats/stats_macros.h" @@ -69,18 +70,18 @@ class FilterUtility { */ class FilterConfig { public: - FilterConfig(const std::string& stat_prefix, const std::string& service_zone, Stats::Store& stats, - Upstream::ClusterManager& cm, Runtime::Loader& runtime, + FilterConfig(const std::string& stat_prefix, const LocalInfo::LocalInfo& local_info, + Stats::Store& stats, Upstream::ClusterManager& cm, Runtime::Loader& runtime, Runtime::RandomGenerator& random, ShadowWriterPtr&& shadow_writer, bool emit_dynamic_stats) - : stats_store_(stats), service_zone_(service_zone), cm_(cm), runtime_(runtime), - random_(random), stats_{ALL_ROUTER_STATS(POOL_COUNTER_PREFIX(stats, stat_prefix))}, + : stats_store_(stats), local_info_(local_info), cm_(cm), runtime_(runtime), random_(random), + stats_{ALL_ROUTER_STATS(POOL_COUNTER_PREFIX(stats, stat_prefix))}, emit_dynamic_stats_(emit_dynamic_stats), shadow_writer_(std::move(shadow_writer)) {} ShadowWriter& shadowWriter() { return *shadow_writer_; } Stats::Store& stats_store_; - const std::string service_zone_; + const LocalInfo::LocalInfo& local_info_; Upstream::ClusterManager& cm_; Runtime::Loader& runtime_; Runtime::RandomGenerator& random_; diff --git a/source/common/stats/statsd.cc b/source/common/stats/statsd.cc index 45536f1f53257..b881c6fd6caca 100644 --- a/source/common/stats/statsd.cc +++ b/source/common/stats/statsd.cc @@ -54,17 +54,17 @@ void UdpStatsdSink::onTimespanComplete(const std::string& name, std::chrono::mil writer().writeTimer(name, ms); } -TcpStatsdSink::TcpStatsdSink(const std::string& stat_cluster, const std::string& stat_host, +TcpStatsdSink::TcpStatsdSink(const LocalInfo::LocalInfo& local_info, const std::string& cluster_name, ThreadLocal::Instance& tls, Upstream::ClusterManager& cluster_manager) - : stat_cluster_(stat_cluster), stat_host_(stat_host), cluster_name_(cluster_name), tls_(tls), + : local_info_(local_info), cluster_name_(cluster_name), tls_(tls), tls_slot_(tls.allocateSlot()), cluster_manager_(cluster_manager) { if (!cluster_manager.get(cluster_name)) { throw EnvoyException(fmt::format("unknown TCP statsd upstream cluster: {}", cluster_name)); } - if (stat_cluster.empty() || stat_host.empty()) { + if (local_info_.clusterName().empty() || local_info_.nodeName().empty()) { throw EnvoyException( fmt::format("TCP statsd requires setting --service-cluster and --service-node")); } diff --git a/source/common/stats/statsd.h b/source/common/stats/statsd.h index 899f86634d3f1..0f8a7fc55f660 100644 --- a/source/common/stats/statsd.h +++ b/source/common/stats/statsd.h @@ -1,5 +1,6 @@ #pragma once +#include "envoy/local_info/local_info.h" #include "envoy/network/connection.h" #include "envoy/stats/stats.h" #include "envoy/thread_local/thread_local.h" @@ -56,9 +57,8 @@ class UdpStatsdSink : public Sink { */ class TcpStatsdSink : public Sink { public: - TcpStatsdSink(const std::string& stat_cluster, const std::string& stat_host, - const std::string& cluster_name, ThreadLocal::Instance& tls, - Upstream::ClusterManager& cluster_manager); + TcpStatsdSink(const LocalInfo::LocalInfo& local_info, const std::string& cluster_name, + ThreadLocal::Instance& tls, Upstream::ClusterManager& cluster_manager); // Stats::Sink void flushCounter(const std::string& name, uint64_t delta) override { @@ -100,8 +100,7 @@ class TcpStatsdSink : public Sink { bool shutdown_{}; }; - std::string stat_cluster_; - std::string stat_host_; + const LocalInfo::LocalInfo& local_info_; std::string cluster_name_; ThreadLocal::Instance& tls_; uint32_t tls_slot_; diff --git a/source/common/tracing/http_tracer_impl.cc b/source/common/tracing/http_tracer_impl.cc index 2412b931c19d4..aefab82dfe322 100644 --- a/source/common/tracing/http_tracer_impl.cc +++ b/source/common/tracing/http_tracer_impl.cc @@ -191,13 +191,13 @@ LightStepSink::TlsLightStepTracer::TlsLightStepTracer(lightstep::Tracer tracer, : tracer_(tracer), sink_(sink) {} LightStepSink::LightStepSink(const Json::Object& config, Upstream::ClusterManager& cluster_manager, - Stats::Store& stats, const std::string& service_node, + Stats::Store& stats, const LocalInfo::LocalInfo& local_info, ThreadLocal::Instance& tls, Runtime::Loader& runtime, std::unique_ptr options) : collector_cluster_(config.getString("collector_cluster")), cm_(cluster_manager), stats_store_(stats), tracer_stats_{LIGHTSTEP_TRACER_STATS(POOL_COUNTER_PREFIX(stats, "tracing.lightstep."))}, - service_node_(service_node), tls_(tls), runtime_(runtime), options_(std::move(options)), + local_info_(local_info), tls_(tls), runtime_(runtime), options_(std::move(options)), tls_slot_(tls.allocateSlot()) { if (!cm_.get(collector_cluster_)) { throw EnvoyException(fmt::format("{} collector cluster is not defined on cluster manager level", @@ -257,7 +257,7 @@ void LightStepSink::flushTrace(const Http::HeaderMap& request_headers, const Htt lightstep::SetTag("downstream cluster", valueOrDefault(request_headers.EnvoyDownstreamServiceCluster(), "-")), lightstep::SetTag("user agent", valueOrDefault(request_headers.UserAgent(), "-")), - lightstep::SetTag("node id", service_node_)}); + lightstep::SetTag("node id", local_info_.nodeName())}); if (request_info.responseCode().valid() && Http::CodeUtility::is5xx(request_info.responseCode().value())) { diff --git a/source/common/tracing/http_tracer_impl.h b/source/common/tracing/http_tracer_impl.h index ce61957ba3e4b..d311d55078a87 100644 --- a/source/common/tracing/http_tracer_impl.h +++ b/source/common/tracing/http_tracer_impl.h @@ -1,5 +1,6 @@ #pragma once +#include "envoy/local_info/local_info.h" #include "envoy/runtime/runtime.h" #include "envoy/thread_local/thread_local.h" #include "envoy/tracing/http_tracer.h" @@ -101,8 +102,9 @@ class HttpTracerImpl : public HttpTracer { class LightStepSink : public HttpSink { public: LightStepSink(const Json::Object& config, Upstream::ClusterManager& cluster_manager, - Stats::Store& stats, const std::string& service_node, ThreadLocal::Instance& tls, - Runtime::Loader& runtime, std::unique_ptr options); + Stats::Store& stats, const LocalInfo::LocalInfo& local_info, + ThreadLocal::Instance& tls, Runtime::Loader& runtime, + std::unique_ptr options); // Tracer::HttpSink void flushTrace(const Http::HeaderMap& request_headers, const Http::HeaderMap& response_headers, @@ -133,7 +135,7 @@ class LightStepSink : public HttpSink { Upstream::ClusterManager& cm_; Stats::Store& stats_store_; LightstepTracerStats tracer_stats_; - const std::string service_node_; + const LocalInfo::LocalInfo& local_info_; ThreadLocal::Instance& tls_; Runtime::Loader& runtime_; std::unique_ptr options_; diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index 2f0b617ff05b4..94de9e34179fb 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -17,12 +17,10 @@ namespace Upstream { ClusterManagerImpl::ClusterManagerImpl(const Json::Object& config, ClusterManagerFactory& factory, Stats::Store& stats, ThreadLocal::Instance& tls, Runtime::Loader& runtime, Runtime::RandomGenerator& random, - const std::string& local_zone_name, - const std::string& local_address, + const LocalInfo::LocalInfo& local_info, AccessLog::AccessLogManager& log_manager) : factory_(factory), runtime_(runtime), stats_(stats), tls_(tls), random_(random), - thread_local_slot_(tls.allocateSlot()), local_zone_name_(local_zone_name), - local_address_(local_address) { + thread_local_slot_(tls.allocateSlot()), local_info_(local_info) { std::vector clusters = config.getObjectArray("clusters"); pending_cluster_init_ = clusters.size(); @@ -41,7 +39,7 @@ ClusterManagerImpl::ClusterManagerImpl(const Json::Object& config, ClusterManage loadCluster(*config.getObject("sds")->getObject("cluster"), false); SdsConfig sds_config{ - local_zone_name, config.getObject("sds")->getObject("cluster")->getString("name"), + config.getObject("sds")->getObject("cluster")->getString("name"), std::chrono::milliseconds(config.getObject("sds")->getInteger("refresh_delay_ms"))}; sds_config_.value(sds_config); @@ -328,10 +326,9 @@ ClusterManagerImpl::ThreadLocalClusterManagerImpl::ClusterEntry::ClusterEntry( ThreadLocalClusterManagerImpl& parent, ClusterInfoPtr cluster) : parent_(parent), cluster_info_(cluster), http_async_client_(*cluster, parent.parent_.stats_, parent.thread_local_dispatcher_, - parent.parent_.local_zone_name_, parent.parent_, parent.parent_.runtime_, + parent.parent_.local_info_, parent.parent_, parent.parent_.runtime_, parent.parent_.random_, - Router::ShadowWriterPtr{new Router::ShadowWriterImpl(parent.parent_)}, - parent.parent_.local_address_) { + Router::ShadowWriterPtr{new Router::ShadowWriterImpl(parent.parent_)}) { switch (cluster->lbType()) { case LoadBalancerType::LeastRequest: { @@ -402,7 +399,7 @@ ProdClusterManagerFactory::clusterFromJson(const Json::Object& cluster, ClusterM const Optional& sds_config, Outlier::EventLoggerPtr outlier_event_logger) { return ClusterImplBase::create(cluster, cm, stats_, tls_, dns_resolver_, ssl_context_manager_, - runtime_, random_, primary_dispatcher_, sds_config, + runtime_, random_, primary_dispatcher_, sds_config, local_info_, outlier_event_logger); } diff --git a/source/common/upstream/cluster_manager_impl.h b/source/common/upstream/cluster_manager_impl.h index ef6f58a9b6fb8..37e7a5336a18f 100644 --- a/source/common/upstream/cluster_manager_impl.h +++ b/source/common/upstream/cluster_manager_impl.h @@ -3,6 +3,7 @@ #include "sds.h" #include "envoy/http/codes.h" +#include "envoy/local_info/local_info.h" #include "envoy/runtime/runtime.h" #include "envoy/thread_local/thread_local.h" #include "envoy/upstream/cluster_manager.h" @@ -21,9 +22,11 @@ class ProdClusterManagerFactory : public ClusterManagerFactory { ThreadLocal::Instance& tls, Runtime::RandomGenerator& random, Network::DnsResolver& dns_resolver, Ssl::ContextManager& ssl_context_manager, - Event::Dispatcher& primary_dispatcher) + Event::Dispatcher& primary_dispatcher, + const LocalInfo::LocalInfo& local_info) : runtime_(runtime), stats_(stats), tls_(tls), random_(random), dns_resolver_(dns_resolver), - ssl_context_manager_(ssl_context_manager), primary_dispatcher_(primary_dispatcher) {} + ssl_context_manager_(ssl_context_manager), primary_dispatcher_(primary_dispatcher), + local_info_(local_info) {} // Upstream::ClusterManagerFactory Http::ConnectionPool::InstancePtr allocateConnPool(Event::Dispatcher& dispatcher, @@ -41,6 +44,7 @@ class ProdClusterManagerFactory : public ClusterManagerFactory { Network::DnsResolver& dns_resolver_; Ssl::ContextManager& ssl_context_manager_; Event::Dispatcher& primary_dispatcher_; + const LocalInfo::LocalInfo& local_info_; }; /** @@ -51,8 +55,8 @@ class ClusterManagerImpl : public ClusterManager { public: ClusterManagerImpl(const Json::Object& config, ClusterManagerFactory& factory, Stats::Store& stats, ThreadLocal::Instance& tls, Runtime::Loader& runtime, - Runtime::RandomGenerator& random, const std::string& local_zone_name, - const std::string& local_address, AccessLog::AccessLogManager& log_manager); + Runtime::RandomGenerator& random, const LocalInfo::LocalInfo& local_info, + AccessLog::AccessLogManager& log_manager); // Upstream::ClusterManager bool addOrUpdatePrimaryCluster(const Json::Object& config) override; @@ -155,8 +159,7 @@ class ClusterManagerImpl : public ClusterManager { Optional sds_config_; std::list secondary_init_clusters_; Outlier::EventLoggerPtr outlier_event_logger_; - const std::string local_zone_name_; - const std::string local_address_; + const LocalInfo::LocalInfo& local_info_; }; } // Upstream diff --git a/source/common/upstream/sds.cc b/source/common/upstream/sds.cc index 8ac2790f3e2eb..f12d21736b746 100644 --- a/source/common/upstream/sds.cc +++ b/source/common/upstream/sds.cc @@ -17,10 +17,12 @@ namespace Upstream { SdsClusterImpl::SdsClusterImpl(const Json::Object& config, Runtime::Loader& runtime, Stats::Store& stats, Ssl::ContextManager& ssl_context_manager, - const SdsConfig& sds_config, ClusterManager& cm, - Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random) + const SdsConfig& sds_config, const LocalInfo::LocalInfo& local_info, + ClusterManager& cm, Event::Dispatcher& dispatcher, + Runtime::RandomGenerator& random) : BaseDynamicClusterImpl(config, runtime, stats, ssl_context_manager), cm_(cm), - sds_config_(sds_config), service_name_(config.getString("service_name")), random_(random), + sds_config_(sds_config), local_info_(local_info), + service_name_(config.getString("service_name")), random_(random), refresh_timer_(dispatcher.createTimer([this]() -> void { refreshHosts(); })) {} SdsClusterImpl::~SdsClusterImpl() { @@ -80,7 +82,7 @@ void SdsClusterImpl::parseSdsResponse(Http::Message& response) { HostListsPtr per_zone(new std::vector>()); // If local zone name is not defined then skip populating per zone hosts. - if (!sds_config_.local_zone_name_.empty()) { + if (!local_info_.zoneName().empty()) { std::map> hosts_per_zone; for (HostPtr host : *current_hosts_copy) { @@ -88,11 +90,11 @@ void SdsClusterImpl::parseSdsResponse(Http::Message& response) { } // Populate per_zone hosts only if upstream cluster has hosts in the same zone. - if (hosts_per_zone.find(sds_config_.local_zone_name_) != hosts_per_zone.end()) { - per_zone->push_back(hosts_per_zone[sds_config_.local_zone_name_]); + if (hosts_per_zone.find(local_info_.zoneName()) != hosts_per_zone.end()) { + per_zone->push_back(hosts_per_zone[local_info_.zoneName()]); for (auto& entry : hosts_per_zone) { - if (sds_config_.local_zone_name_ != entry.first) { + if (local_info_.zoneName() != entry.first) { per_zone->push_back(entry.second); } } diff --git a/source/common/upstream/sds.h b/source/common/upstream/sds.h index c865c87d2ece2..b18f3763dc183 100644 --- a/source/common/upstream/sds.h +++ b/source/common/upstream/sds.h @@ -3,6 +3,7 @@ #include "upstream_impl.h" #include "envoy/http/async_client.h" +#include "envoy/local_info/local_info.h" #include "envoy/runtime/runtime.h" #include "envoy/upstream/cluster_manager.h" @@ -15,8 +16,8 @@ class SdsClusterImpl : public BaseDynamicClusterImpl, public Http::AsyncClient:: public: SdsClusterImpl(const Json::Object& config, Runtime::Loader& runtime, Stats::Store& stats, Ssl::ContextManager& ssl_context_manager, const SdsConfig& sds_config, - ClusterManager& cm, Event::Dispatcher& dispatcher, - Runtime::RandomGenerator& random); + const LocalInfo::LocalInfo& local_info, ClusterManager& cm, + Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random); ~SdsClusterImpl(); @@ -35,6 +36,7 @@ class SdsClusterImpl : public BaseDynamicClusterImpl, public Http::AsyncClient:: ClusterManager& cm_; const SdsConfig& sds_config_; + const LocalInfo::LocalInfo& local_info_; const std::string service_name_; Runtime::RandomGenerator& random_; Event::TimerPtr refresh_timer_; diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 827dc1829c44a..443b1db0aa89f 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -92,6 +92,7 @@ ClusterPtr ClusterImplBase::create(const Json::Object& cluster, ClusterManager& Runtime::Loader& runtime, Runtime::RandomGenerator& random, Event::Dispatcher& dispatcher, const Optional& sds_config, + const LocalInfo::LocalInfo& local_info, Outlier::EventLoggerPtr outlier_event_logger) { std::unique_ptr new_cluster; std::string string_type = cluster.getString("type"); @@ -109,7 +110,7 @@ ClusterPtr ClusterImplBase::create(const Json::Object& cluster, ClusterManager& } new_cluster.reset(new SdsClusterImpl(cluster, runtime, stats, ssl_context_manager, - sds_config.value(), cm, dispatcher, random)); + sds_config.value(), local_info, cm, dispatcher, random)); } else { throw EnvoyException(fmt::format("cluster: unknown cluster type '{}'", string_type)); } diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index 7c4f7c95a9549..3666bbc42c469 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -4,6 +4,7 @@ #include "resource_manager_impl.h" #include "envoy/event/timer.h" +#include "envoy/local_info/local_info.h" #include "envoy/network/dns.h" #include "envoy/runtime/runtime.h" #include "envoy/ssl/context_manager.h" @@ -219,6 +220,7 @@ class ClusterImplBase : public Cluster, Ssl::ContextManager& ssl_context_manager, Runtime::Loader& runtime, Runtime::RandomGenerator& random, Event::Dispatcher& dispatcher, const Optional& sds_config, + const LocalInfo::LocalInfo& local_info, Outlier::EventLoggerPtr outlier_event_logger); /** diff --git a/source/exe/main.cc b/source/exe/main.cc index ba40d0c4a8baf..ff143591df4b8 100644 --- a/source/exe/main.cc +++ b/source/exe/main.cc @@ -1,6 +1,8 @@ #include "hot_restart.h" #include "common/event/libevent.h" +#include "common/local_info/local_info_impl.h" +#include "common/network/utility.h" #include "common/ssl/openssl.h" #include "server/drain_manager_impl.h" #include "server/options_impl.h" @@ -41,8 +43,10 @@ int main(int argc, char** argv) { DefaultTestHooks default_test_hooks; Stats::ThreadLocalStoreImpl stats_store(restarter->statLock(), *restarter); Server::ProdComponentFactory component_factory; + LocalInfo::LocalInfoImpl local_info(Network::Utility::getLocalAddress(), options.serviceZone(), + options.serviceClusterName(), options.serviceNodeName()); Server::InstanceImpl server(options, default_test_hooks, *restarter, stats_store, - restarter->accessLogLock(), component_factory); + restarter->accessLogLock(), component_factory, local_info); server.run(); return 0; } diff --git a/source/server/config/http/ratelimit.cc b/source/server/config/http/ratelimit.cc index edfd76f42bd28..d86f273b57ebd 100644 --- a/source/server/config/http/ratelimit.cc +++ b/source/server/config/http/ratelimit.cc @@ -19,7 +19,7 @@ class RateLimitFilterConfig : public HttpFilterConfigFactory { } Http::RateLimit::FilterConfigPtr filter_config(new Http::RateLimit::FilterConfig( - config, server.options().serviceClusterName(), server.stats(), server.runtime())); + config, server.localInfo(), server.stats(), server.runtime())); return [filter_config, &server](Http::FilterChainFactoryCallbacks& callbacks) -> void { callbacks.addStreamDecoderFilter(Http::StreamDecoderFilterPtr{new Http::RateLimit::Filter( filter_config, server.rateLimitClient(std::chrono::milliseconds(20)))}); diff --git a/source/server/config/http/router.cc b/source/server/config/http/router.cc index c5a06e255e85b..17b5274158541 100644 --- a/source/server/config/http/router.cc +++ b/source/server/config/http/router.cc @@ -19,8 +19,8 @@ class FilterConfig : public HttpFilterConfigFactory { } Router::FilterConfigPtr config(new Router::FilterConfig( - stat_prefix, server.options().serviceZone(), server.stats(), server.clusterManager(), - server.runtime(), server.random(), + stat_prefix, server.localInfo(), server.stats(), server.clusterManager(), server.runtime(), + server.random(), Router::ShadowWriterPtr{new Router::ShadowWriterImpl(server.clusterManager())}, json_config.getBoolean("dynamic_stats", true))); diff --git a/source/server/config/network/http_connection_manager.cc b/source/server/config/network/http_connection_manager.cc index a87a5c1d13c2e..cb0f6120df41d 100644 --- a/source/server/config/network/http_connection_manager.cc +++ b/source/server/config/network/http_connection_manager.cc @@ -84,7 +84,7 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(const Json::Object& con } if (config.hasObject("add_user_agent") && config.getBoolean("add_user_agent")) { - user_agent_.value(server.options().serviceClusterName()); + user_agent_.value(server.localInfo().clusterName()); } if (config.hasObject("tracing")) { @@ -199,7 +199,9 @@ HttpFilterType HttpConnectionManagerConfig::stringToType(const std::string& type } } -const std::string& HttpConnectionManagerConfig::localAddress() { return server_.getLocalAddress(); } +const std::string& HttpConnectionManagerConfig::localAddress() { + return server_.localInfo().address(); +} } // Configuration } // Server diff --git a/source/server/configuration_impl.cc b/source/server/configuration_impl.cc index 0666e8783a7f2..c09ae94b0e804 100644 --- a/source/server/configuration_impl.cc +++ b/source/server/configuration_impl.cc @@ -28,11 +28,12 @@ MainImpl::MainImpl(Server::Instance& server) : server_(server) {} void MainImpl::initialize(const Json::Object& json) { cluster_manager_factory_.reset(new Upstream::ProdClusterManagerFactory( server_.runtime(), server_.stats(), server_.threadLocal(), server_.random(), - server_.dnsResolver(), server_.sslContextManager(), server_.dispatcher())); + server_.dnsResolver(), server_.sslContextManager(), server_.dispatcher(), + server_.localInfo())); cluster_manager_.reset(new Upstream::ClusterManagerImpl( *json.getObject("cluster_manager"), *cluster_manager_factory_, server_.stats(), - server_.threadLocal(), server_.runtime(), server_.random(), server_.options().serviceZone(), - server_.getLocalAddress(), server_.accessLogManager())); + server_.threadLocal(), server_.runtime(), server_.random(), server_.localInfo(), + server_.accessLogManager())); std::vector listeners = json.getObjectArray("listeners"); log().info("loading {} listener(s)", listeners.size()); @@ -96,14 +97,12 @@ void MainImpl::initializeTracers(const Json::Object& tracing_configuration) { opts->access_token = server_.api().fileReadToEnd(sink->getString("access_token_file")); StringUtil::rtrim(opts->access_token); - opts->tracer_attributes["lightstep.component_name"] = - server_.options().serviceClusterName(); + opts->tracer_attributes["lightstep.component_name"] = server_.localInfo().clusterName(); opts->guid_generator = [&rand]() { return rand.random(); }; http_tracer_->addSink(Tracing::HttpSinkPtr{new Tracing::LightStepSink( - *sink->getObject("config"), *cluster_manager_, server_.stats(), - server_.options().serviceNodeName(), server_.threadLocal(), server_.runtime(), - std::move(opts))}); + *sink->getObject("config"), *cluster_manager_, server_.stats(), server_.localInfo(), + server_.threadLocal(), server_.runtime(), std::move(opts))}); } else { throw EnvoyException(fmt::format("unsupported sink type: '{}'", type)); } diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index 1929c6a2afdb4..be180e2f2739b 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -361,6 +361,6 @@ Http::Code AdminImpl::runCallback(const std::string& path, Buffer::Instance& res return code; } -const std::string& AdminImpl::localAddress() { return server_.getLocalAddress(); } +const std::string& AdminImpl::localAddress() { return server_.localInfo().address(); } } // Server diff --git a/source/server/http/health_check.cc b/source/server/http/health_check.cc index 9a81504e6daab..8c6f68dae933b 100644 --- a/source/server/http/health_check.cc +++ b/source/server/http/health_check.cc @@ -115,7 +115,7 @@ Http::FilterHeadersStatus HealthCheckFilter::encodeHeaders(Http::HeaderMap& head static_cast(Http::Utility::getResponseStatus(headers))); } - headers.insertEnvoyUpstreamHealthCheckedCluster().value(server_.options().serviceClusterName()); + headers.insertEnvoyUpstreamHealthCheckedCluster().value(server_.localInfo().clusterName()); } return Http::FilterHeadersStatus::Continue; diff --git a/source/server/options_impl.h b/source/server/options_impl.h index 69c44601b1dcf..ce785a5533aa4 100644 --- a/source/server/options_impl.h +++ b/source/server/options_impl.h @@ -10,6 +10,10 @@ class OptionsImpl : public Server::Options { OptionsImpl(int argc, char** argv, const std::string& hot_restart_version, spdlog::level::level_enum default_log_level); + const std::string& serviceClusterName() { return service_cluster_; } + const std::string& serviceNodeName() { return service_node_; } + const std::string& serviceZone() { return service_zone_; } + // Server::Options uint64_t baseId() { return base_id_; } uint32_t concurrency() override { return concurrency_; } @@ -18,9 +22,6 @@ class OptionsImpl : public Server::Options { spdlog::level::level_enum logLevel() override { return log_level_; } std::chrono::seconds parentShutdownTime() override { return parent_shutdown_time_; } uint64_t restartEpoch() override { return restart_epoch_; } - const std::string& serviceClusterName() override { return service_cluster_; } - const std::string& serviceNodeName() override { return service_node_; } - const std::string& serviceZone() override { return service_zone_; } std::chrono::milliseconds fileFlushIntervalMsec() override { return file_flush_interval_msec_; } private: diff --git a/source/server/server.cc b/source/server/server.cc index 456d734326682..7d58a8624e03f 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -14,7 +14,6 @@ #include "common/common/utility.h" #include "common/common/version.h" #include "common/memory/stats.h" -#include "common/network/utility.h" #include "common/runtime/runtime_impl.h" #include "common/stats/stats_impl.h" #include "common/stats/statsd.h" @@ -23,13 +22,13 @@ namespace Server { InstanceImpl::InstanceImpl(Options& options, TestHooks& hooks, HotRestart& restarter, Stats::Store& store, Thread::BasicLockable& access_log_lock, - ComponentFactory& component_factory) + ComponentFactory& component_factory, + const LocalInfo::LocalInfo& local_info) : options_(options), restarter_(restarter), start_time_(time(nullptr)), original_start_time_(start_time_), stats_store_(store), server_stats_{ALL_SERVER_STATS(POOL_GAUGE_PREFIX(stats_store_, "server."))}, handler_(stats_store_, log(), Api::ApiPtr{new Api::Impl(options.fileFlushIntervalMsec())}), - dns_resolver_(handler_.dispatcher().createDnsResolver()), - local_address_(Network::Utility::getLocalAddress()), + dns_resolver_(handler_.dispatcher().createDnsResolver()), local_info_(local_info), access_log_manager_(handler_.api(), handler_.dispatcher(), access_log_lock, store) { failHealthcheck(false); @@ -40,7 +39,7 @@ InstanceImpl::InstanceImpl(Options& options, TestHooks& hooks, HotRestart& resta } server_stats_.version_.set(version_int); - if (local_address_.empty()) { + if (local_info_.address().empty()) { throw EnvoyException("could not resolve local address"); } @@ -238,7 +237,7 @@ Runtime::LoaderPtr InstanceUtil::createRuntime(Instance& server, log().info("runtime subdirectory: {}", config.runtime()->subdirectory()); std::string override_subdirectory = - config.runtime()->overrideSubdirectory() + "/" + server.options().serviceClusterName(); + config.runtime()->overrideSubdirectory() + "/" + server.localInfo().clusterName(); log().info("runtime override subdirectory: {}", override_subdirectory); return Runtime::LoaderPtr{new Runtime::LoaderImpl( @@ -258,9 +257,9 @@ void InstanceImpl::initializeStatSinks() { if (config_->statsdTcpClusterName().valid()) { log().info("statsd TCP cluster: {}", config_->statsdTcpClusterName().value()); - stat_sinks_.emplace_back(new Stats::Statsd::TcpStatsdSink( - options_.serviceClusterName(), options_.serviceNodeName(), - config_->statsdTcpClusterName().value(), thread_local_, config_->clusterManager())); + stat_sinks_.emplace_back( + new Stats::Statsd::TcpStatsdSink(local_info_, config_->statsdTcpClusterName().value(), + thread_local_, config_->clusterManager())); stats_store_.addSink(*stat_sinks_.back()); } } diff --git a/source/server/server.h b/source/server/server.h index bac3e9d6cd18a..d6f8c3f39d0ba 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -74,7 +74,8 @@ class InstanceUtil : Logger::Loggable { class InstanceImpl : Logger::Loggable, public Instance { public: InstanceImpl(Options& options, TestHooks& hooks, HotRestart& restarter, Stats::Store& store, - Thread::BasicLockable& access_log_lock, ComponentFactory& component_factory); + Thread::BasicLockable& access_log_lock, ComponentFactory& component_factory, + const LocalInfo::LocalInfo& local_info); ~InstanceImpl(); void run(); @@ -109,7 +110,7 @@ class InstanceImpl : Logger::Loggable, public Instance { Stats::Store& stats() override { return stats_store_; } Tracing::HttpTracer& httpTracer() override; ThreadLocal::Instance& threadLocal() override { return thread_local_; } - const std::string& getLocalAddress() override { return local_address_; } + const LocalInfo::LocalInfo& localInfo() override { return local_info_; } private: void flushStats(); @@ -138,7 +139,7 @@ class InstanceImpl : Logger::Loggable, public Instance { Event::SignalEventPtr sig_hup_; Network::DnsResolverPtr dns_resolver_; Event::TimerPtr stat_flush_timer_; - const std::string local_address_; + const LocalInfo::LocalInfo& local_info_; std::unique_ptr ssl_context_manager_; DrainManagerPtr drain_manager_; AccessLog::AccessLogManagerImpl access_log_manager_; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 908864a1d7b7a..f26b8274c2567 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -118,6 +118,7 @@ add_executable(envoy-test mocks/filesystem/mocks.cc mocks/grpc/mocks.cc mocks/http/mocks.cc + mocks/local_info/mocks.cc mocks/network/mocks.cc mocks/ratelimit/mocks.cc mocks/router/mocks.cc diff --git a/test/common/http/async_client_impl_test.cc b/test/common/http/async_client_impl_test.cc index 3355541f0c312..fd5804a8d6e13 100644 --- a/test/common/http/async_client_impl_test.cc +++ b/test/common/http/async_client_impl_test.cc @@ -7,6 +7,7 @@ #include "test/mocks/buffer/mocks.h" #include "test/mocks/common.h" #include "test/mocks/http/mocks.h" +#include "test/mocks/local_info/mocks.h" #include "test/mocks/router/mocks.h" #include "test/mocks/runtime/mocks.h" #include "test/mocks/stats/mocks.h" @@ -24,13 +25,12 @@ namespace Http { class AsyncClientImplTest : public testing::Test { public: AsyncClientImplTest() - : client_(*cm_.cluster_.info_, stats_store_, dispatcher_, "from_az", cm_, runtime_, random_, - Router::ShadowWriterPtr{new NiceMock()}, - "local_address") { + : client_(*cm_.cluster_.info_, stats_store_, dispatcher_, local_info_, cm_, runtime_, random_, + Router::ShadowWriterPtr{new NiceMock()}) { message_->headers().insertMethod().value(std::string("GET")); message_->headers().insertHost().value(std::string("host")); message_->headers().insertPath().value(std::string("/")); - ON_CALL(*cm_.conn_pool_.host_, zone()).WillByDefault(ReturnRef(upstream_zone_)); + ON_CALL(*cm_.conn_pool_.host_, zone()).WillByDefault(ReturnRef(local_info_.zoneName())); ON_CALL(*cm_.cluster_.info_, altStatName()).WillByDefault(ReturnRef(EMPTY_STRING)); } @@ -41,7 +41,6 @@ class AsyncClientImplTest : public testing::Test { })); } - std::string upstream_zone_{"to_az"}; MessagePtr message_{new RequestMessageImpl()}; MockAsyncClientCallbacks callbacks_; NiceMock cm_; @@ -52,6 +51,7 @@ class AsyncClientImplTest : public testing::Test { NiceMock runtime_; NiceMock random_; Stats::IsolatedStoreImpl stats_store_; + NiceMock local_info_; AsyncClientImpl client_; }; @@ -69,7 +69,7 @@ TEST_F(AsyncClientImplTest, Basic) { TestHeaderMapImpl copy(message_->headers()); copy.addViaCopy("x-envoy-internal", "true"); - copy.addViaCopy("x-forwarded-for", "local_address"); + copy.addViaCopy("x-forwarded-for", "127.0.0.1"); copy.addViaCopy(":scheme", "http"); EXPECT_CALL(stream_encoder_, encodeHeaders(HeaderMapEqualRef(©), false)); diff --git a/test/common/http/filter/ratelimit_test.cc b/test/common/http/filter/ratelimit_test.cc index 9f8bd1fb53e06..9e2bdcc903553 100644 --- a/test/common/http/filter/ratelimit_test.cc +++ b/test/common/http/filter/ratelimit_test.cc @@ -5,6 +5,7 @@ #include "common/stats/stats_impl.h" #include "test/mocks/http/mocks.h" +#include "test/mocks/local_info/mocks.h" #include "test/mocks/ratelimit/mocks.h" #include "test/mocks/runtime/mocks.h" #include "test/test_common/utility.h" @@ -34,7 +35,7 @@ class HttpRateLimitFilterTest : public testing::Test { void SetUpTest(const std::string json) { Json::ObjectPtr config = Json::Factory::LoadFromString(json); - config_.reset(new FilterConfig(*config, "service_cluster", stats_store_, runtime_)); + config_.reset(new FilterConfig(*config, local_info_, stats_store_, runtime_)); client_ = new ::RateLimit::MockClient(); filter_.reset(new Filter(config_, ::RateLimit::ClientPtr{client_})); @@ -61,6 +62,7 @@ class HttpRateLimitFilterTest : public testing::Test { NiceMock runtime_; NiceMock rate_limit_policy_entry_; std::vector<::RateLimit::Descriptor> descriptor_{{{{"descriptor_key", "descriptor_value"}}}}; + NiceMock local_info_; }; TEST_F(HttpRateLimitFilterTest, NoRoute) { diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index 8503ad4ddacab..3e16fe3b19921 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -3,6 +3,7 @@ #include "test/common/http/common.h" #include "test/mocks/http/mocks.h" +#include "test/mocks/local_info/mocks.h" #include "test/mocks/router/mocks.h" #include "test/mocks/runtime/mocks.h" #include "test/mocks/ssl/mocks.h" @@ -39,7 +40,7 @@ class RouterTest : public testing::Test { public: RouterTest() : shadow_writer_(new MockShadowWriter()), - config_("test.", "from_az", stats_store_, cm_, runtime_, random_, + config_("test.", local_info_, stats_store_, cm_, runtime_, random_, ShadowWriterPtr{shadow_writer_}, true), router_(config_) { router_.setDecoderFilterCallbacks(callbacks_); @@ -67,6 +68,7 @@ class RouterTest : public testing::Test { Http::ConnectionPool::MockCancellable cancellable_; NiceMock callbacks_; MockShadowWriter* shadow_writer_; + NiceMock local_info_; FilterConfig config_; TestFilter router_; Event::MockTimer* response_timeout_{}; @@ -597,10 +599,10 @@ TEST_F(RouterTest, RetryUpstream5xxNotComplete) { EXPECT_EQ(1U, stats_store_.counter("cluster.fake_cluster.retry.upstream_rq_503").value()); EXPECT_EQ(1U, stats_store_.counter("cluster.fake_cluster.upstream_rq_200").value()); - EXPECT_EQ( - 1U, stats_store_.counter("cluster.fake_cluster.zone.from_az.to_az.upstream_rq_200").value()); - EXPECT_EQ( - 1U, stats_store_.counter("cluster.fake_cluster.zone.from_az.to_az.upstream_rq_2xx").value()); + EXPECT_EQ(1U, stats_store_.counter("cluster.fake_cluster.zone.zone_name.to_az.upstream_rq_200") + .value()); + EXPECT_EQ(1U, stats_store_.counter("cluster.fake_cluster.zone.zone_name.to_az.upstream_rq_2xx") + .value()); } TEST_F(RouterTest, Shadow) { @@ -678,12 +680,12 @@ TEST_F(RouterTest, AltStatName) { .value()); EXPECT_EQ(1U, stats_store_.counter("cluster.fake_cluster.canary.upstream_rq_200").value()); EXPECT_EQ(1U, stats_store_.counter("cluster.fake_cluster.alt_stat.upstream_rq_200").value()); - EXPECT_EQ(1U, - stats_store_.counter("cluster.fake_cluster.alt_stat.zone.from_az.to_az.upstream_rq_200") - .value()); - EXPECT_EQ(1U, - stats_store_.counter("cluster.fake_cluster.alt_stat.zone.from_az.to_az.upstream_rq_200") - .value()); + EXPECT_EQ( + 1U, stats_store_.counter("cluster.fake_cluster.alt_stat.zone.zone_name.to_az.upstream_rq_200") + .value()); + EXPECT_EQ( + 1U, stats_store_.counter("cluster.fake_cluster.alt_stat.zone.zone_name.to_az.upstream_rq_200") + .value()); } TEST_F(RouterTest, Redirect) { diff --git a/test/common/stats/statsd_test.cc b/test/common/stats/statsd_test.cc index 509c91ebbf5c4..99e61aa2af982 100644 --- a/test/common/stats/statsd_test.cc +++ b/test/common/stats/statsd_test.cc @@ -2,6 +2,7 @@ #include "common/upstream/upstream_impl.h" #include "test/mocks/buffer/mocks.h" +#include "test/mocks/local_info/mocks.h" #include "test/mocks/network/mocks.h" #include "test/mocks/thread_local/mocks.h" #include "test/mocks/upstream/mocks.h" @@ -17,12 +18,13 @@ class TcpStatsdSinkTest : public testing::Test { public: TcpStatsdSinkTest() { EXPECT_CALL(cluster_manager_, get("statsd")); - sink_.reset(new TcpStatsdSink("cluster", "host", "statsd", tls_, cluster_manager_)); + sink_.reset(new TcpStatsdSink(local_info_, "statsd", tls_, cluster_manager_)); } NiceMock tls_; Upstream::MockClusterManager cluster_manager_; std::unique_ptr sink_; + NiceMock local_info_; }; TEST_F(TcpStatsdSinkTest, All) { diff --git a/test/common/tracing/http_tracer_impl_test.cc b/test/common/tracing/http_tracer_impl_test.cc index e45e068873626..9c2ca9c30fe55 100644 --- a/test/common/tracing/http_tracer_impl_test.cc +++ b/test/common/tracing/http_tracer_impl_test.cc @@ -5,6 +5,7 @@ #include "common/tracing/http_tracer_impl.h" #include "test/mocks/http/mocks.h" +#include "test/mocks/local_info/mocks.h" #include "test/mocks/runtime/mocks.h" #include "test/mocks/stats/mocks.h" #include "test/mocks/thread_local/mocks.h" @@ -318,7 +319,7 @@ class LightStepSinkTest : public Test { } sink_.reset( - new LightStepSink(config, cm_, stats_, "service_node", tls_, runtime_, std::move(opts))); + new LightStepSink(config, cm_, stats_, local_info_, tls_, runtime_, std::move(opts))); } void setupValidSink() { @@ -347,6 +348,7 @@ class LightStepSinkTest : public Test { NiceMock runtime_; NiceMock tls_; NiceMock context_; + NiceMock local_info_; }; TEST_F(LightStepSinkTest, InitializeSink) { diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index aa3d74a1a8738..96be01407017b 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -6,6 +6,7 @@ #include "test/mocks/access_log/mocks.h" #include "test/mocks/http/mocks.h" +#include "test/mocks/local_info/mocks.h" #include "test/mocks/network/mocks.h" #include "test/mocks/runtime/mocks.h" #include "test/mocks/thread_local/mocks.h" @@ -34,7 +35,7 @@ class TestClusterManagerFactory : public ClusterManagerFactory { Outlier::EventLoggerPtr outlier_event_logger) -> Cluster* { return ClusterImplBase::create(cluster, cm, stats_, tls_, dns_resolver_, ssl_context_manager_, runtime_, random_, dispatcher_, - sds_config, outlier_event_logger).release(); + sds_config, local_info_, outlier_event_logger).release(); })); } @@ -61,14 +62,15 @@ class TestClusterManagerFactory : public ClusterManagerFactory { NiceMock random_; Ssl::ContextManagerImpl ssl_context_manager_{runtime_}; NiceMock dispatcher_; + LocalInfo::MockLocalInfo local_info_; }; class ClusterManagerImplTest : public testing::Test { public: void create(const Json::Object& config) { cluster_manager_.reset(new ClusterManagerImpl(config, factory_, factory_.stats_, factory_.tls_, - factory_.runtime_, factory_.random_, "us-east-1d", - "local_address", log_manager_)); + factory_.runtime_, factory_.random_, + factory_.local_info_, log_manager_)); } NiceMock factory_; diff --git a/test/common/upstream/sds_test.cc b/test/common/upstream/sds_test.cc index d77c7d8f83ec7..2027039fce30c 100644 --- a/test/common/upstream/sds_test.cc +++ b/test/common/upstream/sds_test.cc @@ -3,6 +3,7 @@ #include "common/network/utility.h" #include "common/upstream/sds.h" +#include "test/mocks/local_info/mocks.h" #include "test/mocks/runtime/mocks.h" #include "test/mocks/ssl/mocks.h" #include "test/mocks/upstream/mocks.h" @@ -20,9 +21,7 @@ namespace Upstream { class SdsTest : public testing::Test { protected: - SdsTest() - : sds_config_{"us-east-1a", "sds", std::chrono::milliseconds(30000)}, - request_(&cm_.async_client_) { + SdsTest() : sds_config_{"sds", std::chrono::milliseconds(30000)}, request_(&cm_.async_client_) { std::string raw_config = R"EOF( { "name": "name", @@ -36,8 +35,9 @@ class SdsTest : public testing::Test { Json::ObjectPtr config = Json::Factory::LoadFromString(raw_config); timer_ = new Event::MockTimer(&dispatcher_); + local_info_.zone_name_ = "us-east-1a"; cluster_.reset(new SdsClusterImpl(*config, runtime_, stats_, ssl_context_manager_, sds_config_, - cm_, dispatcher_, random_)); + local_info_, cm_, dispatcher_, random_)); EXPECT_EQ(Cluster::InitializePhase::Secondary, cluster_->initializePhase()); } @@ -93,6 +93,7 @@ class SdsTest : public testing::Test { NiceMock random_; Http::MockAsyncClientRequest request_; NiceMock runtime_; + NiceMock local_info_; }; TEST_F(SdsTest, Shutdown) { diff --git a/test/integration/server.cc b/test/integration/server.cc index 3db9e6f5fd131..f69dce7b5953e 100644 --- a/test/integration/server.cc +++ b/test/integration/server.cc @@ -5,6 +5,7 @@ #include "envoy/http/header_map.h" #include "envoy/server/hot_restart.h" +#include "common/local_info/local_info_impl.h" #include "common/tracing/http_tracer_impl.h" namespace Server { @@ -53,7 +54,9 @@ void IntegrationTestServer::threadRoutine() { Thread::MutexBasicLockable lock; Stats::HeapRawStatDataAllocator stat_allocator; Stats::ThreadLocalStoreImpl stats_store(lock, stat_allocator); - server_.reset(new Server::InstanceImpl(options, *this, restarter, stats_store, lock, *this)); + LocalInfo::LocalInfoImpl local_info("127.0.0.1", "zone_name", "cluster_name", "node_name"); + server_.reset( + new Server::InstanceImpl(options, *this, restarter, stats_store, lock, *this, local_info)); server_->run(); server_.reset(); } diff --git a/test/integration/server.h b/test/integration/server.h index c0bfc2fe340fa..2e5476b981151 100644 --- a/test/integration/server.h +++ b/test/integration/server.h @@ -26,18 +26,12 @@ class TestOptionsImpl : public Options { spdlog::level::level_enum logLevel() override { NOT_IMPLEMENTED; } std::chrono::seconds parentShutdownTime() override { return std::chrono::seconds(0); } uint64_t restartEpoch() override { return 0; } - const std::string& serviceClusterName() override { return cluster_name_; } - const std::string& serviceNodeName() override { return node_name_; } - const std::string& serviceZone() override { return zone_name_; } std::chrono::milliseconds fileFlushIntervalMsec() override { return std::chrono::milliseconds(10000); } private: const std::string config_path_; - const std::string cluster_name_{"cluster_name"}; - const std::string node_name_{"node_name"}; - const std::string zone_name_{"zone_name"}; }; class TestDrainManager : public DrainManager { diff --git a/test/mocks/local_info/mocks.cc b/test/mocks/local_info/mocks.cc new file mode 100644 index 0000000000000..2d0f8ecb205d3 --- /dev/null +++ b/test/mocks/local_info/mocks.cc @@ -0,0 +1,16 @@ +#include "mocks.h" + +using testing::ReturnRef; + +namespace LocalInfo { + +MockLocalInfo::MockLocalInfo() { + ON_CALL(*this, address()).WillByDefault(ReturnRef(address_)); + ON_CALL(*this, zoneName()).WillByDefault(ReturnRef(zone_name_)); + ON_CALL(*this, clusterName()).WillByDefault(ReturnRef(cluster_name_)); + ON_CALL(*this, nodeName()).WillByDefault(ReturnRef(node_name_)); +} + +MockLocalInfo::~MockLocalInfo() {} + +} // LocalInfo diff --git a/test/mocks/local_info/mocks.h b/test/mocks/local_info/mocks.h new file mode 100644 index 0000000000000..f7e69d4ea1539 --- /dev/null +++ b/test/mocks/local_info/mocks.h @@ -0,0 +1,23 @@ +#pragma once + +#include "envoy/local_info/local_info.h" + +namespace LocalInfo { + +class MockLocalInfo : public LocalInfo { +public: + MockLocalInfo(); + ~MockLocalInfo(); + + MOCK_CONST_METHOD0(address, std::string&()); + MOCK_CONST_METHOD0(zoneName, std::string&()); + MOCK_CONST_METHOD0(clusterName, std::string&()); + MOCK_CONST_METHOD0(nodeName, std::string&()); + + std::string address_{"127.0.0.1"}; + std::string zone_name_{"zone_name"}; + std::string cluster_name_{"cluster_name"}; + std::string node_name_{"node_name"}; +}; + +} // LocalInfo diff --git a/test/mocks/server/mocks.cc b/test/mocks/server/mocks.cc index 86441e4a2d596..9ff8f8ba01a12 100644 --- a/test/mocks/server/mocks.cc +++ b/test/mocks/server/mocks.cc @@ -4,15 +4,11 @@ using testing::_; using testing::Return; using testing::ReturnNew; using testing::ReturnRef; -using testing::ReturnRefOfCopy; using testing::SaveArg; namespace Server { -MockOptions::MockOptions() { - ON_CALL(*this, serviceZone()).WillByDefault(ReturnRef(service_zone_)); -} - +MockOptions::MockOptions() {} MockOptions::~MockOptions() {} MockAdmin::MockAdmin() {} @@ -38,7 +34,7 @@ MockInstance::MockInstance() : ssl_context_manager_(runtime_loader_) { ON_CALL(*this, dispatcher()).WillByDefault(ReturnRef(dispatcher_)); ON_CALL(*this, hotRestart()).WillByDefault(ReturnRef(hot_restart_)); ON_CALL(*this, random()).WillByDefault(ReturnRef(random_)); - ON_CALL(*this, getLocalAddress()).WillByDefault(ReturnRefOfCopy(std::string("127.0.0.1"))); + ON_CALL(*this, localInfo()).WillByDefault(ReturnRef(local_info_)); ON_CALL(*this, options()).WillByDefault(ReturnRef(options_)); } diff --git a/test/mocks/server/mocks.h b/test/mocks/server/mocks.h index 3ea4b5c0d3330..ebaf26c392290 100644 --- a/test/mocks/server/mocks.h +++ b/test/mocks/server/mocks.h @@ -13,6 +13,7 @@ #include "test/mocks/access_log/mocks.h" #include "test/mocks/api/mocks.h" #include "test/mocks/http/mocks.h" +#include "test/mocks/local_info/mocks.h" #include "test/mocks/network/mocks.h" #include "test/mocks/runtime/mocks.h" #include "test/mocks/thread_local/mocks.h" @@ -35,12 +36,7 @@ class MockOptions : public Options { MOCK_METHOD0(logLevel, spdlog::level::level_enum()); MOCK_METHOD0(parentShutdownTime, std::chrono::seconds()); MOCK_METHOD0(restartEpoch, uint64_t()); - MOCK_METHOD0(serviceClusterName, const std::string&()); - MOCK_METHOD0(serviceNodeName, const std::string&()); - MOCK_METHOD0(serviceZone, const std::string&()); MOCK_METHOD0(fileFlushIntervalMsec, std::chrono::milliseconds()); - - std::string service_zone_; }; class MockAdmin : public Admin { @@ -116,7 +112,7 @@ class MockInstance : public Instance { MOCK_METHOD0(stats, Stats::Store&()); MOCK_METHOD0(httpTracer, Tracing::HttpTracer&()); MOCK_METHOD0(threadLocal, ThreadLocal::Instance&()); - MOCK_METHOD0(getLocalAddress, const std::string&()); + MOCK_METHOD0(localInfo, const LocalInfo::LocalInfo&()); testing::NiceMock thread_local_; Stats::IsolatedStoreImpl stats_store_; @@ -134,6 +130,7 @@ class MockInstance : public Instance { testing::NiceMock hot_restart_; testing::NiceMock options_; testing::NiceMock random_; + testing::NiceMock local_info_; }; } // Server diff --git a/test/server/http/health_check_test.cc b/test/server/http/health_check_test.cc index b70a7b59e2c07..3c539b44e2db9 100644 --- a/test/server/http/health_check_test.cc +++ b/test/server/http/health_check_test.cc @@ -16,7 +16,6 @@ class HealthCheckFilterTest : public testing::Test { HealthCheckFilterTest(bool pass_through, bool caching) : request_headers_{{":path", "/healthcheck"}}, request_headers_no_hc_{{":path", "/foo"}} { - ON_CALL(server_.options_, serviceClusterName()).WillByDefault(ReturnRef(cluster_name_)); if (caching) { cache_timer_ = new Event::MockTimer(&dispatcher_); EXPECT_CALL(*cache_timer_, enableTimer(_)); @@ -39,7 +38,6 @@ class HealthCheckFilterTest : public testing::Test { NiceMock callbacks_; Http::TestHeaderMapImpl request_headers_; Http::TestHeaderMapImpl request_headers_no_hc_; - std::string cluster_name_{"cluster_name"}; }; class HealthCheckFilterNoPassThroughTest : public HealthCheckFilterTest { From 27eaacc9cee99441e0576146da04e4bd2077badd Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Fri, 6 Jan 2017 10:16:16 -0800 Subject: [PATCH 2/3] comments --- include/envoy/local_info/local_info.h | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/include/envoy/local_info/local_info.h b/include/envoy/local_info/local_info.h index f77c9ede3bd71..a82aadbc14058 100644 --- a/include/envoy/local_info/local_info.h +++ b/include/envoy/local_info/local_info.h @@ -5,15 +5,30 @@ namespace LocalInfo { /** - * + * Information about the local environment. */ class LocalInfo { public: virtual ~LocalInfo() {} + /** + * Human readable network address. + */ virtual const std::string& address() const PURE; + + /** + * Human readable zone name. + */ virtual const std::string& zoneName() const PURE; + + /** + * Human readable cluster name. + */ virtual const std::string& clusterName() const PURE; + + /** + * Human readable individual node name. + */ virtual const std::string& nodeName() const PURE; }; From b0d001a585216c49cf042bcc9c6200bfa1c3ac5f Mon Sep 17 00:00:00 2001 From: Matt Klein Date: Fri, 6 Jan 2017 10:23:32 -0800 Subject: [PATCH 3/3] comments --- include/envoy/local_info/local_info.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/envoy/local_info/local_info.h b/include/envoy/local_info/local_info.h index a82aadbc14058..1ecdde43e36bb 100644 --- a/include/envoy/local_info/local_info.h +++ b/include/envoy/local_info/local_info.h @@ -12,22 +12,22 @@ class LocalInfo { virtual ~LocalInfo() {} /** - * Human readable network address. + * Human readable network address. E.g., "127.0.0.1". */ virtual const std::string& address() const PURE; /** - * Human readable zone name. + * Human readable zone name. E.g., "us-east-1a". */ virtual const std::string& zoneName() const PURE; /** - * Human readable cluster name. + * Human readable cluster name. E.g., "eta". */ virtual const std::string& clusterName() const PURE; /** - * Human readable individual node name. + * Human readable individual node name. E.g., "i-123456". */ virtual const std::string& nodeName() const PURE; };