diff --git a/include/envoy/server/BUILD b/include/envoy/server/BUILD index fae78b50ab2a..33adc7526afb 100644 --- a/include/envoy/server/BUILD +++ b/include/envoy/server/BUILD @@ -111,6 +111,7 @@ envoy_cc_library( hdrs = ["options.h"], deps = [ "//include/envoy/network:address_interface", + "//include/envoy/stats:stats_interface", ], ) diff --git a/include/envoy/server/options.h b/include/envoy/server/options.h index 0d5a62e08491..88d43d8c6bf1 100644 --- a/include/envoy/server/options.h +++ b/include/envoy/server/options.h @@ -6,6 +6,7 @@ #include "envoy/common/pure.h" #include "envoy/network/address.h" +#include "envoy/stats/stats.h" #include "spdlog/spdlog.h" @@ -149,10 +150,10 @@ class Options { virtual uint64_t maxStats() const PURE; /** - * @return uint64_t the maximum name length of the name field in + * @return StatsOptions& the max stat name / suffix lengths for stats. * router/cluster/listener. */ - virtual uint64_t maxObjNameLength() const PURE; + virtual const Stats::StatsOptions& statsOptions() const PURE; /** * @return bool indicating whether the hot restart functionality has been disabled via cli flags. diff --git a/include/envoy/stats/stats.h b/include/envoy/stats/stats.h index 76e88253548d..31d0966d0cac 100644 --- a/include/envoy/stats/stats.h +++ b/include/envoy/stats/stats.h @@ -24,6 +24,43 @@ class Instance; namespace Stats { +/** + * Struct stored under Server::Options to hold information about the maximum object name length and + * maximum stat suffix length of a stat. These have defaults in StatsOptionsImpl, and the maximum + * object name length can be overridden. The default initialization is used in IsolatedStatImpl, and + * the user-overridden struct is stored in Options. + * + * As noted in the comment above StatsOptionsImpl in source/common/stats/stats_impl.h, a stat name + * often contains both a string whose length is user-defined (cluster_name in the below example), + * and a specific statistic name generated by Envoy. To make room for growth on both fronts, we + * limit the max allowed length of each separately. + * + * name / stat name + * |----------------------------------------------------------------| + * cluster..outlier_detection.ejections_consecutive_5xx + * |--------------------------------------| |-----------------------| + * object name suffix + */ +class StatsOptions { +public: + virtual ~StatsOptions() {} + + /** + * The max allowed length of a complete stat name, including suffix. + */ + virtual size_t maxNameLength() const PURE; + + /** + * The max allowed length of the object part of a stat name. + */ + virtual size_t maxObjNameLength() const PURE; + + /** + * The max allowed length of a stat suffix. + */ + virtual size_t maxStatSuffixLength() const PURE; +}; + /** * General representation of a tag. */ @@ -329,6 +366,12 @@ class Scope { * @return a histogram within the scope's namespace with a particular value type. */ virtual Histogram& histogram(const std::string& name) PURE; + + /** + * @return a reference to the top-level StatsOptions struct, containing information about the + * maximum allowable object name length and stat suffix length. + */ + virtual const Stats::StatsOptions& statsOptions() const PURE; }; /** diff --git a/source/common/common/BUILD b/source/common/common/BUILD index f2811d7a546a..b921a9d15444 100644 --- a/source/common/common/BUILD +++ b/source/common/common/BUILD @@ -235,6 +235,8 @@ envoy_cc_library( deps = [ ":assert_lib", ":logger_lib", + "//include/envoy/stats:stats_interface", + "//source/common/stats:stats_lib", ], ) diff --git a/source/common/common/block_memory_hash_set.h b/source/common/common/block_memory_hash_set.h index 1dab1e6c4c80..e28a1c59595e 100644 --- a/source/common/common/block_memory_hash_set.h +++ b/source/common/common/block_memory_hash_set.h @@ -5,10 +5,12 @@ #include #include "envoy/common/exception.h" +#include "envoy/stats/stats.h" #include "common/common/assert.h" #include "common/common/fmt.h" #include "common/common/logger.h" +#include "common/stats/stats_impl.h" #include "absl/strings/string_view.h" @@ -57,21 +59,24 @@ template class BlockMemoryHashSet : public Logger::Loggable class BlockMemoryHashSet : public Logger::Loggableoptions); } - - /** - * Returns the options structure that was used to construct the set. - */ - const BlockMemoryHashSetOptions& options() const { return control_->options; } + uint64_t numBytes(const Stats::StatsOptions& stats_options) const { + return numBytes(control_->hash_set_options, stats_options); + } /** Examines the data structures to see if they are sane, assert-failing on any trouble. */ void sanityCheck() { - RELEASE_ASSERT(control_->size <= control_->options.capacity, ""); + RELEASE_ASSERT(control_->size <= control_->hash_set_options.capacity, ""); // As a sanity check, make sure there are control_->size values // reachable from the slots, each of which has a valid @@ -107,11 +110,11 @@ template class BlockMemoryHashSet : public Logger::Loggableoptions.num_slots; ++slot) { + for (uint32_t slot = 0; slot < control_->hash_set_options.num_slots; ++slot) { uint32_t next = 0; // initialized to silence compilers. for (uint32_t cell_index = slots_[slot]; (cell_index != Sentinal) && (num_values <= control_->size); cell_index = next) { - RELEASE_ASSERT(cell_index < control_->options.capacity, ""); + RELEASE_ASSERT(cell_index < control_->hash_set_options.capacity, ""); Cell& cell = getCell(cell_index); absl::string_view key = cell.value.key(); RELEASE_ASSERT(computeSlot(key) == slot, ""); @@ -122,7 +125,7 @@ template class BlockMemoryHashSet : public Logger::Loggablesize, ""); uint32_t num_free_entries = 0; - uint32_t expected_free_entries = control_->options.capacity - control_->size; + uint32_t expected_free_entries = control_->hash_set_options.capacity - control_->size; // Don't infinite-loop with a corruption; break when we see there's a problem. for (uint32_t cell_index = control_->free_cell_index; @@ -152,7 +155,7 @@ template class BlockMemoryHashSet : public Logger::Loggablesize >= control_->options.capacity) { + if (control_->size >= control_->hash_set_options.capacity) { return ValueCreatedPair(nullptr, false); } const uint32_t slot = computeSlot(key); @@ -162,7 +165,7 @@ template class BlockMemoryHashSet : public Logger::Loggableinitialize(key); + value->truncateAndInit(key, stats_options_); ++control_->size; return ValueCreatedPair(value, true); } @@ -215,9 +218,9 @@ template class BlockMemoryHashSet : public Logger::Loggableoptions.toString(), - control_->hash_signature, numBytes()); + std::string version(const Stats::StatsOptions& stats_options) { + return fmt::format("options={} hash={} size={}", control_->hash_set_options.toString(), + control_->hash_signature, numBytes(stats_options)); } private: @@ -228,20 +231,20 @@ template class BlockMemoryHashSet : public Logger::Loggablehash_signature = Value::hash(signatureStringToHash()); - control_->num_bytes = numBytes(options); - control_->options = options; + control_->num_bytes = numBytes(hash_set_options, stats_options_); + control_->hash_set_options = hash_set_options; control_->size = 0; control_->free_cell_index = 0; // Initialize all the slots; - for (uint32_t slot = 0; slot < options.num_slots; ++slot) { + for (uint32_t slot = 0; slot < hash_set_options.num_slots; ++slot) { slots_[slot] = Sentinal; } // Initialize the free-cell list. - const uint32_t last_cell = options.capacity - 1; + const uint32_t last_cell = hash_set_options.capacity - 1; for (uint32_t cell_index = 0; cell_index < last_cell; ++cell_index) { Cell& cell = getCell(cell_index); cell.next_cell_index = cell_index + 1; @@ -254,10 +257,10 @@ template class BlockMemoryHashSet : public Logger::Loggablenum_bytes) { - ENVOY_LOG(error, "BlockMemoryHashSet unexpected memory size {} != {}", numBytes(options), - control_->num_bytes); + bool attach(const BlockMemoryHashSetOptions& hash_set_options) { + if (numBytes(hash_set_options, stats_options_) != control_->num_bytes) { + ENVOY_LOG(error, "BlockMemoryHashSet unexpected memory size {} != {}", + numBytes(hash_set_options, stats_options_), control_->num_bytes); return false; } if (Value::hash(signatureStringToHash()) != control_->hash_signature) { @@ -269,7 +272,7 @@ template class BlockMemoryHashSet : public Logger::Loggableoptions.num_slots; + return Value::hash(key) % control_->hash_set_options.num_slots; } /** @@ -290,11 +293,11 @@ template class BlockMemoryHashSet : public Logger::Loggable class BlockMemoryHashSet : public Logger::Loggable class BlockMemoryHashSet : public Logger::Loggable(cells_) + cellOffset(cell_index); + char* ptr = reinterpret_cast(cells_) + cellOffset(cell_index, stats_options_); RELEASE_ASSERT((reinterpret_cast(ptr) & (calculateAlignment() - 1)) == 0, ""); return *reinterpret_cast(ptr); } /** Maps out the segments of memory for us to work with. */ - void mapMemorySegments(const BlockMemoryHashSetOptions& options, uint8_t* memory) { + void mapMemorySegments(const BlockMemoryHashSetOptions& hash_set_options, uint8_t* memory) { // Note that we are not examining or mutating memory here, just looking at the pointer, // so we don't need to hold any locks. cells_ = reinterpret_cast(memory); // First because Value may need to be aligned. - memory += cellOffset(options.capacity); + memory += cellOffset(hash_set_options.capacity, stats_options_); control_ = reinterpret_cast(memory); memory += sizeof(Control); slots_ = reinterpret_cast(memory); @@ -356,6 +360,7 @@ template class BlockMemoryHashSet : public Logger::Loggable eds_config; @@ -24,7 +25,7 @@ void BootstrapJson::translateClusterManagerBootstrap( auto* cluster = bootstrap.mutable_static_resources()->mutable_clusters()->Add(); Config::CdsJson::translateCluster(*json_sds->getObject("cluster"), absl::optional(), - *cluster); + *cluster, stats_options); Config::Utility::translateEdsConfig( *json_sds, *bootstrap.mutable_dynamic_resources()->mutable_deprecated_v1()->mutable_sds_config()); @@ -34,7 +35,8 @@ void BootstrapJson::translateClusterManagerBootstrap( if (json_cluster_manager.hasObject("cds")) { const auto json_cds = json_cluster_manager.getObject("cds"); auto* cluster = bootstrap.mutable_static_resources()->mutable_clusters()->Add(); - Config::CdsJson::translateCluster(*json_cds->getObject("cluster"), eds_config, *cluster); + Config::CdsJson::translateCluster(*json_cds->getObject("cluster"), eds_config, *cluster, + stats_options); Config::Utility::translateCdsConfig( *json_cds, *bootstrap.mutable_dynamic_resources()->mutable_cds_config()); } @@ -42,7 +44,7 @@ void BootstrapJson::translateClusterManagerBootstrap( for (const Json::ObjectSharedPtr& json_cluster : json_cluster_manager.getObjectArray("clusters")) { auto* cluster = bootstrap.mutable_static_resources()->mutable_clusters()->Add(); - Config::CdsJson::translateCluster(*json_cluster, eds_config, *cluster); + Config::CdsJson::translateCluster(*json_cluster, eds_config, *cluster, stats_options); } auto* cluster_manager = bootstrap.mutable_cluster_manager(); @@ -54,10 +56,12 @@ void BootstrapJson::translateClusterManagerBootstrap( } void BootstrapJson::translateBootstrap(const Json::Object& json_config, - envoy::config::bootstrap::v2::Bootstrap& bootstrap) { + envoy::config::bootstrap::v2::Bootstrap& bootstrap, + const Stats::StatsOptions& stats_options) { json_config.validateSchema(Json::Schema::TOP_LEVEL_CONFIG_SCHEMA); - translateClusterManagerBootstrap(*json_config.getObject("cluster_manager"), bootstrap); + translateClusterManagerBootstrap(*json_config.getObject("cluster_manager"), bootstrap, + stats_options); if (json_config.hasObject("lds")) { auto* lds_config = bootstrap.mutable_dynamic_resources()->mutable_lds_config(); @@ -66,7 +70,7 @@ void BootstrapJson::translateBootstrap(const Json::Object& json_config, for (const auto json_listener : json_config.getObjectArray("listeners")) { auto* listener = bootstrap.mutable_static_resources()->mutable_listeners()->Add(); - Config::LdsJson::translateListener(*json_listener, *listener); + Config::LdsJson::translateListener(*json_listener, *listener, stats_options); } JSON_UTIL_SET_STRING(json_config, bootstrap, flags_path); diff --git a/source/common/config/bootstrap_json.h b/source/common/config/bootstrap_json.h index 80c567d7264a..dd77bc1f9e70 100644 --- a/source/common/config/bootstrap_json.h +++ b/source/common/config/bootstrap_json.h @@ -2,6 +2,7 @@ #include "envoy/config/bootstrap/v2/bootstrap.pb.h" #include "envoy/json/json_object.h" +#include "envoy/stats/stats.h" namespace Envoy { namespace Config { @@ -14,7 +15,8 @@ class BootstrapJson { * @param bootstrap destination v2 envoy::config::bootstrap::v2::Bootstrap. */ static void translateClusterManagerBootstrap(const Json::Object& json_cluster_manager, - envoy::config::bootstrap::v2::Bootstrap& bootstrap); + envoy::config::bootstrap::v2::Bootstrap& bootstrap, + const Stats::StatsOptions& stats_options); /** * Translate a v1 JSON static config object to v2 envoy::config::bootstrap::v2::Bootstrap. @@ -22,7 +24,8 @@ class BootstrapJson { * @param bootstrap destination v2 envoy::config::bootstrap::v2::Bootstrap. */ static void translateBootstrap(const Json::Object& json_config, - envoy::config::bootstrap::v2::Bootstrap& bootstrap); + envoy::config::bootstrap::v2::Bootstrap& bootstrap, + const Stats::StatsOptions& stats_options); }; } // namespace Config diff --git a/source/common/config/cds_json.cc b/source/common/config/cds_json.cc index 3cf33249a764..cbf9ede748c4 100644 --- a/source/common/config/cds_json.cc +++ b/source/common/config/cds_json.cc @@ -99,11 +99,12 @@ void CdsJson::translateOutlierDetection( void CdsJson::translateCluster(const Json::Object& json_cluster, const absl::optional& eds_config, - envoy::api::v2::Cluster& cluster) { + envoy::api::v2::Cluster& cluster, + const Stats::StatsOptions& stats_options) { json_cluster.validateSchema(Json::Schema::CLUSTER_SCHEMA); const std::string name = json_cluster.getString("name"); - Utility::checkObjNameLength("Invalid cluster name", name); + Utility::checkObjNameLength("Invalid cluster name", name, stats_options); cluster.set_name(name); const std::string string_type = json_cluster.getString("type"); diff --git a/source/common/config/cds_json.h b/source/common/config/cds_json.h index f2995074f79a..8d48d02ac6d6 100644 --- a/source/common/config/cds_json.h +++ b/source/common/config/cds_json.h @@ -3,6 +3,7 @@ #include "envoy/api/v2/cds.pb.h" #include "envoy/api/v2/cluster/circuit_breaker.pb.h" #include "envoy/json/json_object.h" +#include "envoy/stats/stats.h" #include "envoy/upstream/cluster_manager.h" #include "absl/types/optional.h" @@ -64,7 +65,8 @@ class CdsJson { */ static void translateCluster(const Json::Object& json_cluster, const absl::optional& eds_config, - envoy::api::v2::Cluster& cluster); + envoy::api::v2::Cluster& cluster, + const Stats::StatsOptions& stats_options); }; } // namespace Config diff --git a/source/common/config/filter_json.cc b/source/common/config/filter_json.cc index 761bbce080dc..a00b45280088 100644 --- a/source/common/config/filter_json.cc +++ b/source/common/config/filter_json.cc @@ -126,7 +126,8 @@ void FilterJson::translateAccessLog(const Json::Object& json_config, void FilterJson::translateHttpConnectionManager( const Json::Object& json_config, envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& - proto_config) { + proto_config, + const Stats::StatsOptions& stats_options) { json_config.validateSchema(Json::Schema::HTTP_CONN_NETWORK_FILTER_SCHEMA); envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager::CodecType @@ -138,7 +139,8 @@ void FilterJson::translateHttpConnectionManager( JSON_UTIL_SET_STRING(json_config, proto_config, stat_prefix); if (json_config.hasObject("rds")) { - Utility::translateRdsConfig(*json_config.getObject("rds"), *proto_config.mutable_rds()); + Utility::translateRdsConfig(*json_config.getObject("rds"), *proto_config.mutable_rds(), + stats_options); } if (json_config.hasObject("route_config")) { if (json_config.hasObject("rds")) { @@ -146,7 +148,7 @@ void FilterJson::translateHttpConnectionManager( "http connection manager must have either rds or route_config but not both"); } RdsJson::translateRouteConfiguration(*json_config.getObject("route_config"), - *proto_config.mutable_route_config()); + *proto_config.mutable_route_config(), stats_options); } for (const auto& json_filter : json_config.getObjectArray("filters", true)) { diff --git a/source/common/config/filter_json.h b/source/common/config/filter_json.h index 73ca0f7085dc..64ea883ac165 100644 --- a/source/common/config/filter_json.h +++ b/source/common/config/filter_json.h @@ -15,6 +15,7 @@ #include "envoy/config/filter/network/redis_proxy/v2/redis_proxy.pb.h" #include "envoy/config/filter/network/tcp_proxy/v2/tcp_proxy.pb.h" #include "envoy/json/json_object.h" +#include "envoy/stats/stats.h" namespace Envoy { namespace Config { @@ -49,7 +50,8 @@ class FilterJson { static void translateHttpConnectionManager( const Json::Object& json_config, envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& - proto_config); + proto_config, + const Stats::StatsOptions& stats_options); /** * Translate a v1 JSON Redis proxy object to v2 diff --git a/source/common/config/lds_json.cc b/source/common/config/lds_json.cc index c2bc7af1761b..5ca9f0f7cf8f 100644 --- a/source/common/config/lds_json.cc +++ b/source/common/config/lds_json.cc @@ -14,11 +14,12 @@ namespace Envoy { namespace Config { void LdsJson::translateListener(const Json::Object& json_listener, - envoy::api::v2::Listener& listener) { + envoy::api::v2::Listener& listener, + const Stats::StatsOptions& stats_options) { json_listener.validateSchema(Json::Schema::LISTENER_SCHEMA); const std::string name = json_listener.getString("name", ""); - Utility::checkObjNameLength("Invalid listener name", name); + Utility::checkObjNameLength("Invalid listener name", name, stats_options); listener.set_name(name); AddressJson::translateAddress(json_listener.getString("address"), true, true, diff --git a/source/common/config/lds_json.h b/source/common/config/lds_json.h index 4848192acf57..d15eab2fb3c6 100644 --- a/source/common/config/lds_json.h +++ b/source/common/config/lds_json.h @@ -3,6 +3,7 @@ #include "envoy/api/v2/lds.pb.h" #include "envoy/api/v2/listener/listener.pb.h" #include "envoy/json/json_object.h" +#include "envoy/stats/stats.h" namespace Envoy { namespace Config { @@ -15,7 +16,8 @@ class LdsJson { * @param listener destination v2 envoy::api::v2::Listener. */ static void translateListener(const Json::Object& json_listener, - envoy::api::v2::Listener& listener); + envoy::api::v2::Listener& listener, + const Stats::StatsOptions& stats_options); }; } // namespace Config diff --git a/source/common/config/rds_json.cc b/source/common/config/rds_json.cc index ceb892d69aaa..946623b54ba7 100644 --- a/source/common/config/rds_json.cc +++ b/source/common/config/rds_json.cc @@ -116,12 +116,13 @@ void RdsJson::translateQueryParameterMatcher( } void RdsJson::translateRouteConfiguration(const Json::Object& json_route_config, - envoy::api::v2::RouteConfiguration& route_config) { + envoy::api::v2::RouteConfiguration& route_config, + const Stats::StatsOptions& stats_options) { json_route_config.validateSchema(Json::Schema::ROUTE_CONFIGURATION_SCHEMA); for (const auto json_virtual_host : json_route_config.getObjectArray("virtual_hosts", true)) { auto* virtual_host = route_config.mutable_virtual_hosts()->Add(); - translateVirtualHost(*json_virtual_host, *virtual_host); + translateVirtualHost(*json_virtual_host, *virtual_host, stats_options); } for (const std::string& header : @@ -149,11 +150,12 @@ void RdsJson::translateRouteConfiguration(const Json::Object& json_route_config, } void RdsJson::translateVirtualHost(const Json::Object& json_virtual_host, - envoy::api::v2::route::VirtualHost& virtual_host) { + envoy::api::v2::route::VirtualHost& virtual_host, + const Stats::StatsOptions& stats_options) { json_virtual_host.validateSchema(Json::Schema::VIRTUAL_HOST_CONFIGURATION_SCHEMA); const std::string name = json_virtual_host.getString("name", ""); - Utility::checkObjNameLength("Invalid virtual host name", name); + Utility::checkObjNameLength("Invalid virtual host name", name, stats_options); virtual_host.set_name(name); for (const std::string& domain : json_virtual_host.getStringArray("domains", true)) { diff --git a/source/common/config/rds_json.h b/source/common/config/rds_json.h index dba4c8b5b618..987a99cf5d9f 100644 --- a/source/common/config/rds_json.h +++ b/source/common/config/rds_json.h @@ -3,6 +3,7 @@ #include "envoy/api/v2/rds.pb.h" #include "envoy/api/v2/route/route.pb.h" #include "envoy/json/json_object.h" +#include "envoy/stats/stats.h" namespace Envoy { namespace Config { @@ -64,7 +65,8 @@ class RdsJson { * @param route_config destination v2 envoy::api::v2::RouteConfiguration. */ static void translateRouteConfiguration(const Json::Object& json_route_config, - envoy::api::v2::RouteConfiguration& route_config); + envoy::api::v2::RouteConfiguration& route_config, + const Stats::StatsOptions& stats_options); /** * Translate a v1 JSON virtual host object to v2 envoy::api::v2::route::VirtualHost. @@ -72,7 +74,8 @@ class RdsJson { * @param virtual_host destination v2 envoy::api::v2::route::VirtualHost. */ static void translateVirtualHost(const Json::Object& json_virtual_host, - envoy::api::v2::route::VirtualHost& virtual_host); + envoy::api::v2::route::VirtualHost& virtual_host, + const Stats::StatsOptions& stats_options); /** * Translate a v1 JSON decorator object to v2 envoy::api::v2::route::Decorator. diff --git a/source/common/config/utility.cc b/source/common/config/utility.cc index 926362ff3853..007204761b8a 100644 --- a/source/common/config/utility.cc +++ b/source/common/config/utility.cc @@ -177,11 +177,12 @@ void Utility::translateCdsConfig(const Json::Object& json_config, void Utility::translateRdsConfig( const Json::Object& json_rds, - envoy::config::filter::network::http_connection_manager::v2::Rds& rds) { + envoy::config::filter::network::http_connection_manager::v2::Rds& rds, + const Stats::StatsOptions& stats_options) { json_rds.validateSchema(Json::Schema::RDS_CONFIGURATION_SCHEMA); const std::string name = json_rds.getString("route_config_name", ""); - checkObjNameLength("Invalid route_config name", name); + checkObjNameLength("Invalid route_config name", name, stats_options); rds.set_route_config_name(name); translateApiConfigSource(json_rds.getString("cluster"), @@ -204,11 +205,12 @@ Utility::createTagProducer(const envoy::config::bootstrap::v2::Bootstrap& bootst return std::make_unique(bootstrap.stats_config()); } -void Utility::checkObjNameLength(const std::string& error_prefix, const std::string& name) { - if (name.length() > Stats::RawStatData::maxObjNameLength()) { +void Utility::checkObjNameLength(const std::string& error_prefix, const std::string& name, + const Stats::StatsOptions& stats_options) { + if (name.length() > stats_options.maxNameLength()) { throw EnvoyException(fmt::format("{}: Length of {} ({}) exceeds allowed maximum length ({})", error_prefix, name, name.length(), - Stats::RawStatData::maxObjNameLength())); + stats_options.maxNameLength())); } } diff --git a/source/common/config/utility.h b/source/common/config/utility.h index d8536c2d1103..32797c4f627b 100644 --- a/source/common/config/utility.h +++ b/source/common/config/utility.h @@ -173,7 +173,8 @@ class Utility { */ static void translateRdsConfig(const Json::Object& json_rds, - envoy::config::filter::network::http_connection_manager::v2::Rds& rds); + envoy::config::filter::network::http_connection_manager::v2::Rds& rds, + const Stats::StatsOptions& stats_options); /** * Convert a v1 LDS JSON config to v2 LDS envoy::api::v2::core::ConfigSource. @@ -271,8 +272,11 @@ class Utility { * It should be within the configured length limit. Throws on error. * @param error_prefix supplies the prefix to use in error messages. * @param name supplies the name to check for length limits. + * @param stats_options the top-level statsOptions struct, which contains the max stat name / + * suffix lengths for stats. */ - static void checkObjNameLength(const std::string& error_prefix, const std::string& name); + static void checkObjNameLength(const std::string& error_prefix, const std::string& name, + const Stats::StatsOptions& stats_options); /** * Obtain gRPC async client factory from a envoy::api::v2::core::ApiConfigSource. diff --git a/source/common/router/BUILD b/source/common/router/BUILD index d4dfd9712910..d9313aa228cc 100644 --- a/source/common/router/BUILD +++ b/source/common/router/BUILD @@ -100,6 +100,7 @@ envoy_cc_library( hdrs = ["rds_subscription.h"], deps = [ "//include/envoy/config:subscription_interface", + "//include/envoy/stats:stats_interface", "//source/common/common:assert_lib", "//source/common/config:rds_json_lib", "//source/common/config:utility_lib", diff --git a/source/common/router/rds_impl.cc b/source/common/router/rds_impl.cc index 69ba8d9354cb..731d5d18be6c 100644 --- a/source/common/router/rds_impl.cc +++ b/source/common/router/rds_impl.cc @@ -72,7 +72,8 @@ RdsRouteConfigProviderImpl::RdsRouteConfigProviderImpl( &factory_context]() -> Envoy::Config::Subscription* { return new RdsSubscription(Envoy::Config::Utility::generateStats(*scope_), rds, factory_context.clusterManager(), factory_context.dispatcher(), - factory_context.random(), factory_context.localInfo()); + factory_context.random(), factory_context.localInfo(), + factory_context.scope()); }, "envoy.api.v2.RouteDiscoveryService.FetchRoutes", "envoy.api.v2.RouteDiscoveryService.StreamRoutes"); diff --git a/source/common/router/rds_subscription.cc b/source/common/router/rds_subscription.cc index fcfebd95e872..53d8b172a069 100644 --- a/source/common/router/rds_subscription.cc +++ b/source/common/router/rds_subscription.cc @@ -1,5 +1,7 @@ #include "common/router/rds_subscription.h" +#include "envoy/stats/stats.h" + #include "common/common/assert.h" #include "common/common/fmt.h" #include "common/config/rds_json.h" @@ -14,12 +16,12 @@ RdsSubscription::RdsSubscription( Envoy::Config::SubscriptionStats stats, const envoy::config::filter::network::http_connection_manager::v2::Rds& rds, Upstream::ClusterManager& cm, Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random, - const LocalInfo::LocalInfo& local_info) + const LocalInfo::LocalInfo& local_info, const Stats::Scope& scope) : RestApiFetcher(cm, rds.config_source().api_config_source().cluster_names()[0], dispatcher, random, Envoy::Config::Utility::apiConfigSourceRefreshDelay( rds.config_source().api_config_source())), - local_info_(local_info), stats_(stats) { + local_info_(local_info), stats_(stats), scope_(scope) { const auto& api_config_source = rds.config_source().api_config_source(); UNREFERENCED_PARAMETER(api_config_source); // If we are building an RdsSubscription, the ConfigSource should be REST_LEGACY. @@ -45,7 +47,8 @@ void RdsSubscription::parseResponse(const Http::Message& response) { const std::string response_body = response.bodyAsString(); Json::ObjectSharedPtr response_json = Json::Factory::loadFromString(response_body); Protobuf::RepeatedPtrField resources; - Envoy::Config::RdsJson::translateRouteConfiguration(*response_json, *resources.Add()); + Envoy::Config::RdsJson::translateRouteConfiguration(*response_json, *resources.Add(), + scope_.statsOptions()); resources[0].set_name(route_config_name_); std::pair hash = Envoy::Config::Utility::computeHashedVersion(response_body); diff --git a/source/common/router/rds_subscription.h b/source/common/router/rds_subscription.h index d27217af5616..132b7c42f9e0 100644 --- a/source/common/router/rds_subscription.h +++ b/source/common/router/rds_subscription.h @@ -5,6 +5,7 @@ #include "envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.pb.h" #include "envoy/config/subscription.h" +#include "envoy/stats/stats.h" #include "common/common/assert.h" #include "common/http/rest_api_fetcher.h" @@ -23,7 +24,8 @@ class RdsSubscription : public Http::RestApiFetcher, RdsSubscription(Envoy::Config::SubscriptionStats stats, const envoy::config::filter::network::http_connection_manager::v2::Rds& rds, Upstream::ClusterManager& cm, Event::Dispatcher& dispatcher, - Runtime::RandomGenerator& random, const LocalInfo::LocalInfo& local_info); + Runtime::RandomGenerator& random, const LocalInfo::LocalInfo& local_info, + const Stats::Scope& scope); private: // Config::Subscription @@ -55,6 +57,7 @@ class RdsSubscription : public Http::RestApiFetcher, const LocalInfo::LocalInfo& local_info_; Envoy::Config::SubscriptionCallbacks* callbacks_ = nullptr; Envoy::Config::SubscriptionStats stats_; + const Stats::Scope& scope_; }; } // namespace Router diff --git a/source/common/stats/stats_impl.cc b/source/common/stats/stats_impl.cc index 12ffbf2590f2..e0a370ae1672 100644 --- a/source/common/stats/stats_impl.cc +++ b/source/common/stats/stats_impl.cc @@ -37,32 +37,15 @@ bool regexStartsWithDot(absl::string_view regex) { } // namespace -uint64_t RawStatData::size() { - // Normally the compiler would do this, but because name_ is a flexible-array-length - // element, the compiler can't. RawStatData is put into an array in HotRestartImpl, so - // it's important that each element starts on the required alignment for the type. - return roundUpMultipleNaturalAlignment(sizeof(RawStatData) + nameSize()); +// Normally the compiler would do this, but because name_ is a flexible-array-length +// element, the compiler can't. RawStatData is put into an array in HotRestartImpl, so +// it's important that each element starts on the required alignment for the type. +uint64_t RawStatData::structSize(uint64_t name_size) { + return roundUpMultipleNaturalAlignment(sizeof(RawStatData) + name_size + 1); } -uint64_t& RawStatData::initializeAndGetMutableMaxObjNameLength(uint64_t configured_size) { - // Like CONSTRUCT_ON_FIRST_USE, but non-const so that the value can be changed by tests - static uint64_t size = configured_size; - return size; -} - -void RawStatData::configure(Server::Options& options) { - const uint64_t configured = options.maxObjNameLength(); - RELEASE_ASSERT(configured > 0, ""); - uint64_t max_obj_name_length = initializeAndGetMutableMaxObjNameLength(configured); - - // If this fails, it means that this function was called too late during - // startup because things were already using this size before it was set. - RELEASE_ASSERT(max_obj_name_length == configured, ""); -} - -void RawStatData::configureForTestsOnly(Server::Options& options) { - const uint64_t configured = options.maxObjNameLength(); - initializeAndGetMutableMaxObjNameLength(configured) = configured; +uint64_t RawStatData::structSizeWithOptions(const StatsOptions& stats_options) { + return structSize(stats_options.maxNameLength()); } std::string Utility::sanitizeStatsName(const std::string& name) { @@ -153,14 +136,13 @@ bool TagExtractorImpl::extractTag(const std::string& stat_name, std::vector } RawStatData* HeapRawStatDataAllocator::alloc(const std::string& name) { - RawStatData* data = static_cast(::calloc(RawStatData::size(), 1)); - data->initialize(name); - - // Because the RawStatData object is initialized with and contains a truncated - // version of the std::string name, storing the stats in a map would require - // storing the name twice. Performing a lookup on the set is similarly - // expensive to performing a map lookup, since both require copying a truncated version of the - // string before doing the hash lookup. + uint64_t num_bytes_to_allocate = RawStatData::structSize(name.size()); + RawStatData* data = static_cast(::calloc(num_bytes_to_allocate, 1)); + if (data == nullptr) { + throw std::bad_alloc(); + } + data->checkAndInit(name, num_bytes_to_allocate); + Thread::ReleasableLockGuard lock(mutex_); auto ret = stats_.insert(data); RawStatData* existing_data = *ret.first; @@ -353,20 +335,31 @@ void HeapRawStatDataAllocator::free(RawStatData& data) { ::free(&data); } -void RawStatData::initialize(absl::string_view key) { +void RawStatData::initialize(absl::string_view key, uint64_t xfer_size) { ASSERT(!initialized()); - if (key.size() > Stats::RawStatData::maxNameLength()) { + ref_count_ = 1; + memcpy(name_, key.data(), xfer_size); + name_[xfer_size] = '\0'; +} + +void RawStatData::checkAndInit(absl::string_view key, uint64_t num_bytes_allocated) { + uint64_t xfer_size = key.size(); + ASSERT(structSize(xfer_size) <= num_bytes_allocated); + + initialize(key, xfer_size); +} + +void RawStatData::truncateAndInit(absl::string_view key, const StatsOptions& stats_options) { + if (key.size() > stats_options.maxNameLength()) { ENVOY_LOG_MISC( warn, "Statistic '{}' is too long with {} characters, it will be truncated to {} characters", key, - key.size(), Stats::RawStatData::maxNameLength()); + key.size(), stats_options.maxNameLength()); } - ref_count_ = 1; // key is not necessarily nul-terminated, but we want to make sure name_ is. - uint64_t xfer_size = std::min(nameSize() - 1, key.size()); - memcpy(name_, key.data(), xfer_size); - name_[xfer_size] = '\0'; + uint64_t xfer_size = std::min(stats_options.maxNameLength(), key.size()); + initialize(key, xfer_size); } HistogramStatisticsImpl::HistogramStatisticsImpl(const histogram_t* histogram_ptr) diff --git a/source/common/stats/stats_impl.h b/source/common/stats/stats_impl.h index b9616e357246..70a37d18ba1a 100644 --- a/source/common/stats/stats_impl.h +++ b/source/common/stats/stats_impl.h @@ -32,6 +32,27 @@ namespace Envoy { namespace Stats { +// The max name length is based on current set of stats. +// As of now, the longest stat is +// cluster..outlier_detection.ejections_consecutive_5xx +// which is 52 characters long without the cluster name. +// The max stat name length is 127 (default). So, in order to give room +// for growth to both the envoy generated stat characters +// (e.g., outlier_detection...) and user supplied names (e.g., cluster name), +// we set the max user supplied name length to 60, and the max internally +// generated stat suffixes to 67 (15 more characters to grow). +// If you want to increase the max user supplied name length, use the compiler +// option ENVOY_DEFAULT_MAX_OBJ_NAME_LENGTH or the CLI option +// max-obj-name-len +struct StatsOptionsImpl : public StatsOptions { + size_t maxNameLength() const override { return max_obj_name_length_ + max_stat_suffix_length_; } + size_t maxObjNameLength() const override { return max_obj_name_length_; } + size_t maxStatSuffixLength() const override { return max_stat_suffix_length_; } + + size_t max_obj_name_length_ = 60; + size_t max_stat_suffix_length_ = 67; +}; + class TagExtractorImpl : public TagExtractor { public: /** @@ -170,7 +191,7 @@ class Utility { * it can be allocated from shared memory if needed. * * @note Due to name_ being variable size, sizeof(RawStatData) probably isn't useful. Use - * RawStatData::size() instead. + * RawStatData::structSize() or RawStatData::structSizeWithOptions() instead. */ struct RawStatData { @@ -182,54 +203,32 @@ struct RawStatData { ~RawStatData() = delete; /** - * Configure static settings. This MUST be called - * before any other static or instance methods. - */ - static void configure(Server::Options& options); - - /** - * Allow tests to re-configure this value after it has been set. - * This is unsafe in a non-test context. - */ - static void configureForTestsOnly(Server::Options& options); - - /** - * Returns the maximum length of the name of a stat. This length - * does not include a trailing NULL-terminator. - */ - static size_t maxNameLength() { return maxObjNameLength() + MAX_STAT_SUFFIX_LENGTH; } - - /** - * Returns the maximum length of a user supplied object (route/cluster/listener) - * name field in a stat. This length does not include a trailing NULL-terminator. - */ - static size_t maxObjNameLength() { - return initializeAndGetMutableMaxObjNameLength(DEFAULT_MAX_OBJ_NAME_LENGTH); - } - - /** - * Returns the maximum length of a stat suffix that Envoy generates (over the user supplied name). - * This length does not include a trailing NULL-terminator. + * Returns the size of this struct, accounting for the length of name_ + * and padding for alignment. Required for the HeapRawStatDataAllocator, which does not truncate + * at a maximum stat name length. */ - static size_t maxStatSuffixLength() { return MAX_STAT_SUFFIX_LENGTH; } + static uint64_t structSize(uint64_t name_size); /** - * size in bytes of name_ + * Wrapper for structSize, taking a StatsOptions struct. + * Required by BlockMemoryHashSet, which has the context to supply StatsOptions. */ - static size_t nameSize() { return maxNameLength() + 1; } + static uint64_t structSizeWithOptions(const StatsOptions& stats_options); /** - * Returns the size of this struct, accounting for the length of name_ - * and padding for alignment. This is required by BlockMemoryHashSet. + * Initializes this object to have the specified key, + * a refcount of 1, and all other values zero. Required for the HeapRawStatDataAllocator, which + * does not expect stat name truncation. We pass in the number of bytes allocated in order to + * assert the copy is safe inline. */ - static uint64_t size(); + void checkAndInit(absl::string_view key, uint64_t num_bytes_allocated); /** * Initializes this object to have the specified key, - * a refcount of 1, and all other values zero. This is required by - * BlockMemoryHashSet. + * a refcount of 1, and all other values zero. Required by the BlockMemoryHashSet. StatsOptions is + * used to truncate key inline, if necessary. */ - void initialize(absl::string_view key); + void truncateAndInit(absl::string_view key, const StatsOptions& stats_options); /** * Returns a hash of the key. This is required by BlockMemoryHashSet. @@ -242,11 +241,9 @@ struct RawStatData { bool initialized() { return name_[0] != '\0'; } /** - * Returns the name as a string_view. This is required by BlockMemoryHashSet. + * Returns the name as a string_view with no truncation. */ - absl::string_view key() const { - return absl::string_view(name_, strnlen(name_, maxNameLength())); - } + absl::string_view key() const { return absl::string_view(name_); } std::atomic value_; std::atomic pending_increment_; @@ -256,26 +253,7 @@ struct RawStatData { char name_[]; private: - // The max name length is based on current set of stats. - // As of now, the longest stat is - // cluster..outlier_detection.ejections_consecutive_5xx - // which is 52 characters long without the cluster name. - // The max stat name length is 127 (default). So, in order to give room - // for growth to both the envoy generated stat characters - // (e.g., outlier_detection...) and user supplied names (e.g., cluster name), - // we set the max user supplied name length to 60, and the max internally - // generated stat suffixes to 67 (15 more characters to grow). - // If you want to increase the max user supplied name length, use the compiler - // option ENVOY_DEFAULT_MAX_OBJ_NAME_LENGTH or the CLI option - // max-obj-name-len - static const size_t DEFAULT_MAX_OBJ_NAME_LENGTH = 60; - static const size_t MAX_STAT_SUFFIX_LENGTH = 67; - - /** - * @return uint64_t& a reference to the configured size, which can then be changed - * by callers. - */ - static uint64_t& initializeAndGetMutableMaxObjNameLength(uint64_t configured_size); + void initialize(absl::string_view key, uint64_t num_bytes_allocated); }; /** @@ -492,6 +470,7 @@ class IsolatedStoreImpl : public Store { Histogram& histogram = histograms_.get(name); return histogram; } + const Stats::StatsOptions& statsOptions() const override { return stats_options_; } // Stats::Store std::vector counters() const override { return counters_.toVector(); } @@ -515,6 +494,7 @@ class IsolatedStoreImpl : public Store { Histogram& histogram(const std::string& name) override { return parent_.histogram(prefix_ + name); } + const Stats::StatsOptions& statsOptions() const override { return parent_.statsOptions(); } IsolatedStoreImpl& parent_; const std::string prefix_; @@ -524,6 +504,7 @@ class IsolatedStoreImpl : public Store { IsolatedStatsCache counters_; IsolatedStatsCache gauges_; IsolatedStatsCache histograms_; + const Stats::StatsOptionsImpl stats_options_; }; } // namespace Stats diff --git a/source/common/stats/thread_local_store.cc b/source/common/stats/thread_local_store.cc index 681c9c9b7be3..7e6e448e63c8 100644 --- a/source/common/stats/thread_local_store.cc +++ b/source/common/stats/thread_local_store.cc @@ -12,8 +12,9 @@ namespace Envoy { namespace Stats { -ThreadLocalStoreImpl::ThreadLocalStoreImpl(StatDataAllocator& alloc) - : alloc_(alloc), default_scope_(createScope("")), +ThreadLocalStoreImpl::ThreadLocalStoreImpl(const Stats::StatsOptions& stats_options, + StatDataAllocator& alloc) + : stats_options_(stats_options), alloc_(alloc), default_scope_(createScope("")), tag_producer_(std::make_unique()), num_last_resort_stats_(default_scope_->counter("stats.overflow")), source_(*this) {} @@ -181,8 +182,14 @@ StatType& ThreadLocalStoreImpl::ScopeImpl::safeMakeStat( make_stat(parent_.alloc_, name, std::move(tag_extracted_name), std::move(tags)); if (stat == nullptr) { parent_.num_last_resort_stats_.inc(); + // TODO(jbuckland) Performing the fallback from non-heap allocator to heap allocator should be + // the responsibility of the non-heap allocator, not the client of the non-heap allocator. + // This branch will never be used in the non-hot-restart case, since the parent_.alloc_ object + // will throw instead of returning a nullptr; we should remove the assumption that this + // branching case is always available. stat = - make_stat(parent_.heap_allocator_, name, std::move(tag_extracted_name), std::move(tags)); + make_stat(parent_.heap_allocator_, name.substr(0, parent_.statsOptions().maxNameLength()), + std::move(tag_extracted_name), std::move(tags)); ASSERT(stat != nullptr); } central_ref = stat; diff --git a/source/common/stats/thread_local_store.h b/source/common/stats/thread_local_store.h index 05f8f16fbec1..69cebaeae32c 100644 --- a/source/common/stats/thread_local_store.h +++ b/source/common/stats/thread_local_store.h @@ -163,7 +163,7 @@ class TlsScope : public Scope { */ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRoot { public: - ThreadLocalStoreImpl(StatDataAllocator& alloc); + ThreadLocalStoreImpl(const Stats::StatsOptions& stats_options, StatDataAllocator& alloc); ~ThreadLocalStoreImpl(); // Stats::Scope @@ -195,6 +195,8 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo Source& source() override { return source_; } + const Stats::StatsOptions& statsOptions() const override { return stats_options_; } + private: struct TlsCacheEntry { std::unordered_map counters_; @@ -224,6 +226,7 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo Gauge& gauge(const std::string& name) override; Histogram& histogram(const std::string& name) override; Histogram& tlsHistogram(const std::string& name, ParentHistogramImpl& parent) override; + const Stats::StatsOptions& statsOptions() const override { return parent_.statsOptions(); } template using MakeStatFn = @@ -272,6 +275,7 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo void releaseScopeCrossThread(ScopeImpl* scope); void mergeInternal(PostMergeCb mergeCb); + const Stats::StatsOptions& stats_options_; StatDataAllocator& alloc_; Event::Dispatcher* main_thread_dispatcher_{}; ThreadLocal::SlotPtr tls_; diff --git a/source/common/upstream/cds_api_impl.cc b/source/common/upstream/cds_api_impl.cc index cc652c16ea35..aab3a313cbb2 100644 --- a/source/common/upstream/cds_api_impl.cc +++ b/source/common/upstream/cds_api_impl.cc @@ -35,10 +35,11 @@ CdsApiImpl::CdsApiImpl(const envoy::api::v2::core::ConfigSource& cds_config, subscription_ = Config::SubscriptionFactory::subscriptionFromConfigSource( cds_config, local_info.node(), dispatcher, cm, random, *scope_, - [this, &cds_config, &eds_config, &cm, &dispatcher, &random, - &local_info]() -> Config::Subscription* { + [this, &cds_config, &eds_config, &cm, &dispatcher, &random, &local_info, + &scope]() -> Config::Subscription* { return new CdsSubscription(Config::Utility::generateStats(*scope_), cds_config, - eds_config, cm, dispatcher, random, local_info); + eds_config, cm, dispatcher, random, local_info, + scope.statsOptions()); }, "envoy.api.v2.ClusterDiscoveryService.FetchClusters", "envoy.api.v2.ClusterDiscoveryService.StreamClusters"); diff --git a/source/common/upstream/cds_subscription.cc b/source/common/upstream/cds_subscription.cc index d3287a88e5d2..c571b855d313 100644 --- a/source/common/upstream/cds_subscription.cc +++ b/source/common/upstream/cds_subscription.cc @@ -16,10 +16,11 @@ CdsSubscription::CdsSubscription( Config::SubscriptionStats stats, const envoy::api::v2::core::ConfigSource& cds_config, const absl::optional& eds_config, ClusterManager& cm, Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random, - const LocalInfo::LocalInfo& local_info) + const LocalInfo::LocalInfo& local_info, const Stats::StatsOptions& stats_options) : RestApiFetcher(cm, cds_config.api_config_source().cluster_names()[0], dispatcher, random, Config::Utility::apiConfigSourceRefreshDelay(cds_config.api_config_source())), - local_info_(local_info), stats_(stats), eds_config_(eds_config) { + local_info_(local_info), stats_(stats), eds_config_(eds_config), + stats_options_(stats_options) { const auto& api_config_source = cds_config.api_config_source(); UNREFERENCED_PARAMETER(api_config_source); // If we are building an CdsSubscription, the ConfigSource should be REST_LEGACY. @@ -46,7 +47,7 @@ void CdsSubscription::parseResponse(const Http::Message& response) { Protobuf::RepeatedPtrField resources; for (const Json::ObjectSharedPtr& cluster : clusters) { - Config::CdsJson::translateCluster(*cluster, eds_config_, *resources.Add()); + Config::CdsJson::translateCluster(*cluster, eds_config_, *resources.Add(), stats_options_); } std::pair hash = diff --git a/source/common/upstream/cds_subscription.h b/source/common/upstream/cds_subscription.h index 8e4f1454612d..6359d473a6f3 100644 --- a/source/common/upstream/cds_subscription.h +++ b/source/common/upstream/cds_subscription.h @@ -26,7 +26,8 @@ class CdsSubscription : public Http::RestApiFetcher, const envoy::api::v2::core::ConfigSource& cds_config, const absl::optional& eds_config, ClusterManager& cm, Event::Dispatcher& dispatcher, - Runtime::RandomGenerator& random, const LocalInfo::LocalInfo& local_info); + Runtime::RandomGenerator& random, const LocalInfo::LocalInfo& local_info, + const Stats::StatsOptions& stats_options); private: // Config::Subscription @@ -55,6 +56,7 @@ class CdsSubscription : public Http::RestApiFetcher, Config::SubscriptionCallbacks* callbacks_ = nullptr; Config::SubscriptionStats stats_; const absl::optional& eds_config_; + const Stats::StatsOptions& stats_options_; }; } // namespace Upstream diff --git a/source/exe/main_common.cc b/source/exe/main_common.cc index cb3089f95493..5dd9a41e94c7 100644 --- a/source/exe/main_common.cc +++ b/source/exe/main_common.cc @@ -43,7 +43,6 @@ MainCommonBase::MainCommonBase(OptionsImpl& options) : options_(options) { Event::Libevent::Global::initialize(); RELEASE_ASSERT(Envoy::Server::validateProtoDescriptors(), ""); - Stats::RawStatData::configure(options_); switch (options_.mode()) { case Server::Mode::InitOnly: case Server::Mode::Serve: { @@ -62,7 +61,8 @@ MainCommonBase::MainCommonBase(OptionsImpl& options) : options_(options) { auto local_address = Network::Utility::getLocalAddress(options_.localAddressIpVersion()); Logger::Registry::initialize(options_.logLevel(), options_.logFormat(), log_lock); - stats_store_.reset(new Stats::ThreadLocalStoreImpl(restarter_->statsAllocator())); + stats_store_ = std::make_unique(options_.statsOptions(), + restarter_->statsAllocator()); server_.reset(new Server::InstanceImpl( options_, local_address, default_test_hooks_, *restarter_, *stats_store_, access_log_lock, component_factory_, std::make_unique(), *tls_)); diff --git a/source/extensions/filters/http/dynamo/BUILD b/source/extensions/filters/http/dynamo/BUILD index 3db5ce6089d5..e1ba97728475 100644 --- a/source/extensions/filters/http/dynamo/BUILD +++ b/source/extensions/filters/http/dynamo/BUILD @@ -41,7 +41,10 @@ envoy_cc_library( name = "dynamo_utility_lib", srcs = ["dynamo_utility.cc"], hdrs = ["dynamo_utility.h"], - deps = ["//source/common/stats:stats_lib"], + deps = [ + "//include/envoy/stats:stats_interface", + "//source/common/stats:stats_lib", + ], ) envoy_cc_library( diff --git a/source/extensions/filters/http/dynamo/dynamo_filter.cc b/source/extensions/filters/http/dynamo/dynamo_filter.cc index 5a6bd6905f2c..504fba6dce21 100644 --- a/source/extensions/filters/http/dynamo/dynamo_filter.cc +++ b/source/extensions/filters/http/dynamo/dynamo_filter.cc @@ -236,8 +236,9 @@ void DynamoFilter::chargeTablePartitionIdStats(const Json::Object& json_body) { std::vector partitions = RequestParser::parsePartitions(json_body); for (const RequestParser::PartitionDescriptor& partition : partitions) { - std::string scope_string = Utility::buildPartitionStatString( - stat_prefix_, table_descriptor_.table_name, operation_, partition.partition_id_); + std::string scope_string = + Utility::buildPartitionStatString(stat_prefix_, table_descriptor_.table_name, operation_, + partition.partition_id_, scope_.statsOptions()); scope_.counter(scope_string).add(partition.capacity_); } } diff --git a/source/extensions/filters/http/dynamo/dynamo_utility.cc b/source/extensions/filters/http/dynamo/dynamo_utility.cc index ceaa4085aafa..aa9cf79f7585 100644 --- a/source/extensions/filters/http/dynamo/dynamo_utility.cc +++ b/source/extensions/filters/http/dynamo/dynamo_utility.cc @@ -12,14 +12,15 @@ namespace Dynamo { std::string Utility::buildPartitionStatString(const std::string& stat_prefix, const std::string& table_name, const std::string& operation, - const std::string& partition_id) { + const std::string& partition_id, + const Stats::StatsOptions& stats_options) { // Use the last 7 characters of the partition id. std::string stats_partition_postfix = fmt::format(".capacity.{}.__partition_id={}", operation, partition_id.substr(partition_id.size() - 7, partition_id.size())); // Calculate how many characters are available for the table prefix. - size_t remaining_size = Stats::RawStatData::maxNameLength() - stats_partition_postfix.size(); + size_t remaining_size = stats_options.maxNameLength() - stats_partition_postfix.size(); std::string stats_table_prefix = fmt::format("{}table.{}", stat_prefix, table_name); // Truncate the table prefix if the current string is too large. diff --git a/source/extensions/filters/http/dynamo/dynamo_utility.h b/source/extensions/filters/http/dynamo/dynamo_utility.h index 87b4bd0bc119..c1609290a1ac 100644 --- a/source/extensions/filters/http/dynamo/dynamo_utility.h +++ b/source/extensions/filters/http/dynamo/dynamo_utility.h @@ -2,6 +2,8 @@ #include +#include "envoy/stats/stats.h" + namespace Envoy { namespace Extensions { namespace HttpFilters { @@ -23,7 +25,8 @@ class Utility { static std::string buildPartitionStatString(const std::string& stat_prefix, const std::string& table_name, const std::string& operation, - const std::string& partition_id); + const std::string& partition_id, + const Stats::StatsOptions& stats_options); }; } // namespace Dynamo diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index 49afe9880cb7..476efe295c74 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -84,7 +84,8 @@ HttpConnectionManagerFilterConfigFactory::createFilterFactoryFromProtoTyped( Network::FilterFactoryCb HttpConnectionManagerFilterConfigFactory::createFilterFactory( const Json::Object& json_config, Server::Configuration::FactoryContext& context) { envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager proto_config; - Config::FilterJson::translateHttpConnectionManager(json_config, proto_config); + Config::FilterJson::translateHttpConnectionManager(json_config, proto_config, + context.scope().statsOptions()); return createFilterFactoryFromProtoTyped(proto_config, context); } diff --git a/source/server/BUILD b/source/server/BUILD index 6ff07ca41dd8..31fd4036da0f 100644 --- a/source/server/BUILD +++ b/source/server/BUILD @@ -152,6 +152,7 @@ envoy_cc_library( deps = [ "//include/envoy/network:address_interface", "//include/envoy/server:options_interface", + "//include/envoy/stats:stats_interface", "//source/common/common:macros", "//source/common/common:version_lib", "//source/common/stats:stats_lib", diff --git a/source/server/hot_restart_impl.cc b/source/server/hot_restart_impl.cc index fd5bb7c425c4..e41db9566a6e 100644 --- a/source/server/hot_restart_impl.cc +++ b/source/server/hot_restart_impl.cc @@ -40,7 +40,7 @@ static BlockMemoryHashSetOptions blockMemHashOptions(uint64_t max_stats) { SharedMemory& SharedMemory::initialize(uint64_t stats_set_size, Options& options) { Api::OsSysCalls& os_sys_calls = Api::OsSysCallsSingleton::get(); - const uint64_t entry_size = Stats::RawStatData::size(); + const uint64_t entry_size = Stats::RawStatData::structSizeWithOptions(options.statsOptions()); const uint64_t total_size = sizeof(SharedMemory) + stats_set_size; int flags = O_RDWR; @@ -109,14 +109,16 @@ void SharedMemory::initializeMutex(pthread_mutex_t& mutex) { pthread_mutex_init(&mutex, &attribute); } -std::string SharedMemory::version(uint64_t max_num_stats, uint64_t max_stat_name_len) { +std::string SharedMemory::version(uint64_t max_num_stats, + const Stats::StatsOptions& stats_options) { return fmt::format("{}.{}.{}.{}", VERSION, sizeof(SharedMemory), max_num_stats, - max_stat_name_len); + stats_options.maxNameLength()); } HotRestartImpl::HotRestartImpl(Options& options) : options_(options), stats_set_options_(blockMemHashOptions(options.maxStats())), - shmem_(SharedMemory::initialize(RawStatDataSet::numBytes(stats_set_options_), options)), + shmem_(SharedMemory::initialize( + RawStatDataSet::numBytes(stats_set_options_, options_.statsOptions()), options_)), log_lock_(shmem_.log_lock_), access_log_lock_(shmem_.access_log_lock_), stat_lock_(shmem_.stat_lock_), init_lock_(shmem_.init_lock_) { { @@ -124,7 +126,7 @@ HotRestartImpl::HotRestartImpl(Options& options) // because it might be actively written to while we sanityCheck it. Thread::LockGuard lock(stat_lock_); stats_set_.reset(new RawStatDataSet(stats_set_options_, options.restartEpoch() == 0, - shmem_.stats_set_data_)); + shmem_.stats_set_data_, options_.statsOptions())); } my_domain_socket_ = bindDomainSocket(options.restartEpoch()); child_address_ = createDomainSocketAddress((options.restartEpoch() + 1)); @@ -143,8 +145,8 @@ Stats::RawStatData* HotRestartImpl::alloc(const std::string& name) { // Try to find the existing slot in shared memory, otherwise allocate a new one. Thread::LockGuard lock(stat_lock_); absl::string_view key = name; - if (key.size() > Stats::RawStatData::maxNameLength()) { - key.remove_suffix(key.size() - Stats::RawStatData::maxNameLength()); + if (key.size() > options_.statsOptions().maxNameLength()) { + key.remove_suffix(key.size() - options_.statsOptions().maxNameLength()); } auto value_created = stats_set_->insert(key); Stats::RawStatData* data = value_created.first; @@ -169,7 +171,8 @@ void HotRestartImpl::free(Stats::RawStatData& data) { } bool key_removed = stats_set_->remove(data.key()); ASSERT(key_removed); - memset(static_cast(&data), 0, Stats::RawStatData::size()); + memset(static_cast(&data), 0, + Stats::RawStatData::structSizeWithOptions(options_.statsOptions())); } int HotRestartImpl::bindDomainSocket(uint64_t id) { @@ -474,23 +477,30 @@ void HotRestartImpl::shutdown() { socket_event_.reset(); } std::string HotRestartImpl::version() { Thread::LockGuard lock(stat_lock_); - return versionHelper(shmem_.maxStats(), Stats::RawStatData::maxNameLength(), *stats_set_); + return versionHelper(shmem_.maxStats(), options_.statsOptions(), *stats_set_); } // Called from envoy --hot-restart-version -- needs to instantiate a RawStatDataSet so it // can generate the version string. std::string HotRestartImpl::hotRestartVersion(uint64_t max_num_stats, uint64_t max_stat_name_len) { - const BlockMemoryHashSetOptions options = blockMemHashOptions(max_num_stats); - const uint64_t bytes = RawStatDataSet::numBytes(options); + Stats::StatsOptionsImpl stats_options; + stats_options.max_obj_name_length_ = max_stat_name_len - stats_options.maxStatSuffixLength(); + + const BlockMemoryHashSetOptions hash_set_options = blockMemHashOptions(max_num_stats); + const uint64_t bytes = RawStatDataSet::numBytes(hash_set_options, stats_options); std::unique_ptr mem_buffer_for_dry_run_(new uint8_t[bytes]); - RawStatDataSet stats_set(options, true /* init */, mem_buffer_for_dry_run_.get()); - return versionHelper(max_num_stats, max_stat_name_len, stats_set); + RawStatDataSet stats_set(hash_set_options, true /* init */, mem_buffer_for_dry_run_.get(), + stats_options); + + return versionHelper(max_num_stats, stats_options, stats_set); } -std::string HotRestartImpl::versionHelper(uint64_t max_num_stats, uint64_t max_stat_name_len, +std::string HotRestartImpl::versionHelper(uint64_t max_num_stats, + const Stats::StatsOptions& stats_options, RawStatDataSet& stats_set) { - return SharedMemory::version(max_num_stats, max_stat_name_len) + "." + stats_set.version(); + return SharedMemory::version(max_num_stats, stats_options) + "." + + stats_set.version(stats_options); } } // namespace Server diff --git a/source/server/hot_restart_impl.h b/source/server/hot_restart_impl.h index 4fe2788fbcf6..39f537d55523 100644 --- a/source/server/hot_restart_impl.h +++ b/source/server/hot_restart_impl.h @@ -27,7 +27,7 @@ typedef BlockMemoryHashSet RawStatDataSet; class SharedMemory { public: static void configure(uint64_t max_num_stats, uint64_t max_stat_name_len); - static std::string version(uint64_t max_num_stats, uint64_t max_stat_name_len); + static std::string version(uint64_t max_num_stats, const Stats::StatsOptions& stats_options); // Made public for testing. static const uint64_t VERSION; @@ -203,7 +203,7 @@ class HotRestartImpl : public HotRestart, void onSocketEvent(); RpcBase* receiveRpc(bool block); void sendMessage(sockaddr_un& address, RpcBase& rpc); - static std::string versionHelper(uint64_t max_num_stats, uint64_t max_stat_name_len, + static std::string versionHelper(uint64_t max_num_stats, const Stats::StatsOptions& stats_options, RawStatDataSet& stats_set); Options& options_; diff --git a/source/server/lds_api.cc b/source/server/lds_api.cc index 87c7a5dd321d..43b6f725fd00 100644 --- a/source/server/lds_api.cc +++ b/source/server/lds_api.cc @@ -25,10 +25,10 @@ LdsApiImpl::LdsApiImpl(const envoy::api::v2::core::ConfigSource& lds_config, subscription_ = Envoy::Config::SubscriptionFactory::subscriptionFromConfigSource( lds_config, local_info.node(), dispatcher, cm, random, *scope_, - [this, &lds_config, &cm, &dispatcher, &random, - &local_info]() -> Config::Subscription* { + [this, &lds_config, &cm, &dispatcher, &random, &local_info, + &scope]() -> Config::Subscription* { return new LdsSubscription(Config::Utility::generateStats(*scope_), lds_config, cm, - dispatcher, random, local_info); + dispatcher, random, local_info, scope.statsOptions()); }, "envoy.api.v2.ListenerDiscoveryService.FetchListeners", "envoy.api.v2.ListenerDiscoveryService.StreamListeners"); diff --git a/source/server/lds_subscription.cc b/source/server/lds_subscription.cc index a825ddbff02b..757d4c8e43f6 100644 --- a/source/server/lds_subscription.cc +++ b/source/server/lds_subscription.cc @@ -16,10 +16,11 @@ LdsSubscription::LdsSubscription(Config::SubscriptionStats stats, const envoy::api::v2::core::ConfigSource& lds_config, Upstream::ClusterManager& cm, Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random, - const LocalInfo::LocalInfo& local_info) + const LocalInfo::LocalInfo& local_info, + const Stats::StatsOptions& stats_options) : RestApiFetcher(cm, lds_config.api_config_source().cluster_names()[0], dispatcher, random, Config::Utility::apiConfigSourceRefreshDelay(lds_config.api_config_source())), - local_info_(local_info), stats_(stats) { + local_info_(local_info), stats_(stats), stats_options_(stats_options) { const auto& api_config_source = lds_config.api_config_source(); UNREFERENCED_PARAMETER(lds_config); // If we are building an LdsSubscription, the ConfigSource should be REST_LEGACY. @@ -48,7 +49,7 @@ void LdsSubscription::parseResponse(const Http::Message& response) { Protobuf::RepeatedPtrField resources; for (const Json::ObjectSharedPtr& json_listener : json_listeners) { - Config::LdsJson::translateListener(*json_listener, *resources.Add()); + Config::LdsJson::translateListener(*json_listener, *resources.Add(), stats_options_); } std::pair hash = diff --git a/source/server/lds_subscription.h b/source/server/lds_subscription.h index 2dd39d873c69..1e7c3fc9f447 100644 --- a/source/server/lds_subscription.h +++ b/source/server/lds_subscription.h @@ -23,7 +23,8 @@ class LdsSubscription : public Http::RestApiFetcher, LdsSubscription(Config::SubscriptionStats stats, const envoy::api::v2::core::ConfigSource& lds_config, Upstream::ClusterManager& cm, Event::Dispatcher& dispatcher, - Runtime::RandomGenerator& random, const LocalInfo::LocalInfo& local_info); + Runtime::RandomGenerator& random, const LocalInfo::LocalInfo& local_info, + const Stats::StatsOptions& stats_options); private: // Config::Subscription @@ -51,6 +52,7 @@ class LdsSubscription : public Http::RestApiFetcher, const LocalInfo::LocalInfo& local_info_; Config::SubscriptionCallbacks* callbacks_ = nullptr; Config::SubscriptionStats stats_; + const Stats::StatsOptions& stats_options_; }; } // namespace Server diff --git a/source/server/options_impl.cc b/source/server/options_impl.cc index e66155781545..f052727ee7c7 100644 --- a/source/server/options_impl.cc +++ b/source/server/options_impl.cc @@ -194,13 +194,10 @@ OptionsImpl::OptionsImpl(int argc, const char* const* argv, drain_time_ = std::chrono::seconds(drain_time_s.getValue()); parent_shutdown_time_ = std::chrono::seconds(parent_shutdown_time_s.getValue()); max_stats_ = max_stats.getValue(); - max_obj_name_length_ = max_obj_name_len.getValue(); + stats_options_.max_obj_name_length_ = max_obj_name_len.getValue(); if (hot_restart_version_option.getValue()) { - Stats::RawStatData::configure(*this); - std::cerr << hot_restart_version_cb(max_stats.getValue(), - max_obj_name_len.getValue() + - Stats::RawStatData::maxStatSuffixLength(), + std::cerr << hot_restart_version_cb(max_stats.getValue(), stats_options_.maxNameLength(), !hot_restart_disabled_); throw NoServingException(); } diff --git a/source/server/options_impl.h b/source/server/options_impl.h index 201d3d8ff4c2..bbb97472ec8f 100644 --- a/source/server/options_impl.h +++ b/source/server/options_impl.h @@ -7,6 +7,8 @@ #include "envoy/common/exception.h" #include "envoy/server/options.h" +#include "common/stats/stats_impl.h" + #include "spdlog/spdlog.h" namespace Envoy { @@ -60,9 +62,7 @@ class OptionsImpl : public Server::Options { void setServiceNodeName(const std::string& service_node) { service_node_ = service_node; } void setServiceZone(const std::string& service_zone) { service_zone_ = service_zone; } void setMaxStats(uint64_t max_stats) { max_stats_ = max_stats; } - void setMaxObjNameLength(uint64_t max_obj_name_length) { - max_obj_name_length_ = max_obj_name_length; - } + void setStatsOptions(Stats::StatsOptionsImpl stats_options) { stats_options_ = stats_options; } void setHotRestartDisabled(bool hot_restart_disabled) { hot_restart_disabled_ = hot_restart_disabled; } @@ -91,7 +91,7 @@ class OptionsImpl : public Server::Options { const std::string& serviceNodeName() const override { return service_node_; } const std::string& serviceZone() const override { return service_zone_; } uint64_t maxStats() const override { return max_stats_; } - uint64_t maxObjNameLength() const override { return max_obj_name_length_; } + const Stats::StatsOptions& statsOptions() const override { return stats_options_; } bool hotRestartDisabled() const override { return hot_restart_disabled_; } private: @@ -114,7 +114,7 @@ class OptionsImpl : public Server::Options { std::chrono::seconds parent_shutdown_time_; Server::Mode mode_; uint64_t max_stats_; - uint64_t max_obj_name_length_; + Stats::StatsOptionsImpl stats_options_; bool hot_restart_disabled_; }; diff --git a/source/server/server.cc b/source/server/server.cc index 7c682ceed809..cc53893403c7 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -171,7 +171,7 @@ InstanceUtil::loadBootstrapConfig(envoy::config::bootstrap::v2::Bootstrap& boots throw EnvoyException("V1 config (detected) with --config-yaml is not supported"); } Json::ObjectSharedPtr config_json = Json::Factory::loadFromFile(options.configPath()); - Config::BootstrapJson::translateBootstrap(*config_json, bootstrap); + Config::BootstrapJson::translateBootstrap(*config_json, bootstrap, options.statsOptions()); MessageUtil::validate(bootstrap); return BootstrapVersion::V1; } diff --git a/test/common/common/BUILD b/test/common/common/BUILD index 19cdaf45284b..685f9970b5a2 100644 --- a/test/common/common/BUILD +++ b/test/common/common/BUILD @@ -156,8 +156,10 @@ envoy_cc_test( name = "block_memory_hash_set_test", srcs = ["block_memory_hash_set_test.cc"], deps = [ + "//include/envoy/stats:stats_interface", "//source/common/common:block_memory_hash_set_lib", "//source/common/common:hash_lib", + "//source/common/stats:stats_lib", ], ) diff --git a/test/common/common/block_memory_hash_set_test.cc b/test/common/common/block_memory_hash_set_test.cc index c4e96943902f..da5477cf8325 100644 --- a/test/common/common/block_memory_hash_set_test.cc +++ b/test/common/common/block_memory_hash_set_test.cc @@ -4,9 +4,12 @@ #include #include +#include "envoy/stats/stats.h" + #include "common/common/block_memory_hash_set.h" #include "common/common/fmt.h" #include "common/common/hash.h" +#include "common/stats/stats_impl.h" #include "absl/strings/string_view.h" #include "gtest/gtest.h" @@ -19,12 +22,15 @@ class BlockMemoryHashSetTest : public testing::Test { // TestValue that doesn't define a hash. struct TestValueBase { absl::string_view key() const { return name; } - void initialize(absl::string_view key) { - uint64_t xfer = std::min(sizeof(name) - 1, key.size()); + void truncateAndInit(absl::string_view key, const Stats::StatsOptions& stats_options) { + uint64_t xfer = std::min(stats_options.maxNameLength(), key.size()); memcpy(name, key.data(), xfer); name[xfer] = '\0'; } - static uint64_t size() { return sizeof(TestValue); } + static uint64_t structSizeWithOptions(const Stats::StatsOptions& stats_options) { + UNREFERENCED_PARAMETER(stats_options); + return sizeof(TestValue); + } int64_t number; char name[256]; @@ -43,9 +49,10 @@ class BlockMemoryHashSetTest : public testing::Test { typedef BlockMemoryHashSet::ValueCreatedPair ValueCreatedPair; template void setUp() { - options_.capacity = 100; - options_.num_slots = 5; - const uint32_t mem_size = BlockMemoryHashSet::numBytes(options_); + hash_set_options_.capacity = 100; + hash_set_options_.num_slots = 5; + const uint32_t mem_size = + BlockMemoryHashSet::numBytes(hash_set_options_, stats_options_); memory_.reset(new uint8_t[mem_size]); memset(memory_.get(), 0, mem_size); } @@ -59,10 +66,11 @@ class BlockMemoryHashSetTest : public testing::Test { std::string ret; static const uint32_t sentinal = BlockMemoryHashSet::Sentinal; std::string control_string = - fmt::format("{} size={} free_cell_index={}", hs.control_->options.toString(), + fmt::format("{} size={} free_cell_index={}", hs.control_->hash_set_options.toString(), hs.control_->size, hs.control_->free_cell_index); - ret = fmt::format("options={}\ncontrol={}\n", hs.control_->options.toString(), control_string); - for (uint32_t i = 0; i < hs.control_->options.num_slots; ++i) { + ret = fmt::format("options={}\ncontrol={}\n", hs.control_->hash_set_options.toString(), + control_string); + for (uint32_t i = 0; i < hs.control_->hash_set_options.num_slots; ++i) { ret += fmt::format("slot {}:", i); for (uint32_t j = hs.slots_[i]; j != sentinal; j = hs.getCell(j).next_cell_index) { ret += " " + std::string(hs.getCell(j).value.key()); @@ -72,23 +80,27 @@ class BlockMemoryHashSetTest : public testing::Test { return ret; } - BlockMemoryHashSetOptions options_; + BlockMemoryHashSetOptions hash_set_options_; + Stats::StatsOptionsImpl stats_options_; std::unique_ptr memory_; }; TEST_F(BlockMemoryHashSetTest, initAndAttach) { setUp(); { - BlockMemoryHashSet hash_set1(options_, true, memory_.get()); // init - BlockMemoryHashSet hash_set2(options_, false, memory_.get()); // attach + BlockMemoryHashSet hash_set1(hash_set_options_, true, memory_.get(), + stats_options_); // init + BlockMemoryHashSet hash_set2(hash_set_options_, false, memory_.get(), + stats_options_); // attach } // If we tweak an option, we can no longer attach it. bool constructor_completed = false; bool constructor_threw = false; try { - options_.capacity = 99; - BlockMemoryHashSet hash_set3(options_, false, memory_.get()); + hash_set_options_.capacity = 99; + BlockMemoryHashSet hash_set3(hash_set_options_, false, memory_.get(), + stats_options_); constructor_completed = false; } catch (const std::exception& e) { constructor_threw = true; @@ -100,7 +112,7 @@ TEST_F(BlockMemoryHashSetTest, initAndAttach) { TEST_F(BlockMemoryHashSetTest, putRemove) { setUp(); { - BlockMemoryHashSet hash_set1(options_, true, memory_.get()); + BlockMemoryHashSet hash_set1(hash_set_options_, true, memory_.get(), stats_options_); hash_set1.sanityCheck(); EXPECT_EQ(0, hash_set1.size()); EXPECT_EQ(nullptr, hash_set1.get("no such key")); @@ -121,7 +133,8 @@ TEST_F(BlockMemoryHashSetTest, putRemove) { { // Now init a new hash-map with the same memory. - BlockMemoryHashSet hash_set2(options_, false, memory_.get()); + BlockMemoryHashSet hash_set2(hash_set_options_, false, memory_.get(), + stats_options_); EXPECT_EQ(1, hash_set2.size()); EXPECT_EQ(nullptr, hash_set2.get("no such key")); EXPECT_EQ(6789, hash_set2.get("good key")->number) << hashSetToString(hash_set2); @@ -136,21 +149,21 @@ TEST_F(BlockMemoryHashSetTest, putRemove) { TEST_F(BlockMemoryHashSetTest, tooManyValues) { setUp(); - BlockMemoryHashSet hash_set1(options_, true, memory_.get()); + BlockMemoryHashSet hash_set1(hash_set_options_, true, memory_.get(), stats_options_); std::vector keys; - for (uint32_t i = 0; i < options_.capacity + 1; ++i) { + for (uint32_t i = 0; i < hash_set_options_.capacity + 1; ++i) { keys.push_back(fmt::format("key{}", i)); } - for (uint32_t i = 0; i < options_.capacity; ++i) { + for (uint32_t i = 0; i < hash_set_options_.capacity; ++i) { TestValue* value = hash_set1.insert(keys[i]).first; ASSERT_NE(nullptr, value); value->number = i; } hash_set1.sanityCheck(); - EXPECT_EQ(options_.capacity, hash_set1.size()); + EXPECT_EQ(hash_set_options_.capacity, hash_set1.size()); - for (uint32_t i = 0; i < options_.capacity; ++i) { + for (uint32_t i = 0; i < hash_set_options_.capacity; ++i) { const TestValue* value = hash_set1.get(keys[i]); ASSERT_NE(nullptr, value); EXPECT_EQ(i, value->number); @@ -158,29 +171,30 @@ TEST_F(BlockMemoryHashSetTest, tooManyValues) { hash_set1.sanityCheck(); // We can't fit one more value. - EXPECT_EQ(nullptr, hash_set1.insert(keys[options_.capacity]).first); + EXPECT_EQ(nullptr, hash_set1.insert(keys[hash_set_options_.capacity]).first); hash_set1.sanityCheck(); - EXPECT_EQ(options_.capacity, hash_set1.size()); + EXPECT_EQ(hash_set_options_.capacity, hash_set1.size()); // Now remove everything one by one. - for (uint32_t i = 0; i < options_.capacity; ++i) { + for (uint32_t i = 0; i < hash_set_options_.capacity; ++i) { EXPECT_TRUE(hash_set1.remove(keys[i])); } hash_set1.sanityCheck(); EXPECT_EQ(0, hash_set1.size()); // Now we can put in that last key we weren't able to before. - TestValue* value = hash_set1.insert(keys[options_.capacity]).first; + TestValue* value = hash_set1.insert(keys[hash_set_options_.capacity]).first; EXPECT_NE(nullptr, value); value->number = 314519; EXPECT_EQ(1, hash_set1.size()); - EXPECT_EQ(314519, hash_set1.get(keys[options_.capacity])->number); + EXPECT_EQ(314519, hash_set1.get(keys[hash_set_options_.capacity])->number); hash_set1.sanityCheck(); } TEST_F(BlockMemoryHashSetTest, severalKeysZeroHash) { setUp(); - BlockMemoryHashSet hash_set1(options_, true, memory_.get()); + BlockMemoryHashSet hash_set1(hash_set_options_, true, memory_.get(), + stats_options_); hash_set1.insert("one").first->number = 1; hash_set1.insert("two").first->number = 2; hash_set1.insert("three").first->number = 3; @@ -194,8 +208,9 @@ TEST_F(BlockMemoryHashSetTest, severalKeysZeroHash) { TEST_F(BlockMemoryHashSetTest, sanityCheckZeroedMemoryDeathTest) { setUp(); - BlockMemoryHashSet hash_set1(options_, true, memory_.get()); - memset(memory_.get(), 0, hash_set1.numBytes()); + BlockMemoryHashSet hash_set1(hash_set_options_, true, memory_.get(), + stats_options_); + memset(memory_.get(), 0, hash_set1.numBytes(stats_options_)); EXPECT_DEATH(hash_set1.sanityCheck(), ""); } diff --git a/test/common/config/utility_test.cc b/test/common/config/utility_test.cc index 9e45d3df7e99..26cef9151bc8 100644 --- a/test/common/config/utility_test.cc +++ b/test/common/config/utility_test.cc @@ -96,15 +96,16 @@ TEST(UtilityTest, createTagProducer) { } TEST(UtilityTest, ObjNameLength) { - - std::string name = "listenerwithareallyreallylongnamemorethanmaxcharsallowedbyschema"; + Stats::StatsOptionsImpl stats_options; + std::string name = "listenerwithareallyreallyreallyreallyreallyreallyreallyreallyreallyreallyreal" + "lyreallyreallyreallyreallyreallylongnamemorethanmaxcharsallowedbyschema"; std::string err_prefix; std::string err_suffix = fmt::format(": Length of {} ({}) exceeds allowed maximum length ({})", - name, name.length(), Stats::RawStatData::maxObjNameLength()); + name, name.length(), stats_options.maxNameLength()); { err_prefix = "test"; - EXPECT_THROW_WITH_MESSAGE(Utility::checkObjNameLength(err_prefix, name), EnvoyException, - err_prefix + err_suffix); + EXPECT_THROW_WITH_MESSAGE(Utility::checkObjNameLength(err_prefix, name, stats_options), + EnvoyException, err_prefix + err_suffix); } { @@ -114,8 +115,9 @@ TEST(UtilityTest, ObjNameLength) { auto json_object_ptr = Json::Factory::loadFromString(json); envoy::api::v2::Listener listener; - EXPECT_THROW_WITH_MESSAGE(Config::LdsJson::translateListener(*json_object_ptr, listener), - EnvoyException, err_prefix + err_suffix); + EXPECT_THROW_WITH_MESSAGE( + Config::LdsJson::translateListener(*json_object_ptr, listener, stats_options), + EnvoyException, err_prefix + err_suffix); } { @@ -123,8 +125,9 @@ TEST(UtilityTest, ObjNameLength) { std::string json = R"EOF({ "name": ")EOF" + name + R"EOF(", "domains": [], "routes": []})EOF"; auto json_object_ptr = Json::Factory::loadFromString(json); envoy::api::v2::route::VirtualHost vhost; - EXPECT_THROW_WITH_MESSAGE(Config::RdsJson::translateVirtualHost(*json_object_ptr, vhost), - EnvoyException, err_prefix + err_suffix); + EXPECT_THROW_WITH_MESSAGE( + Config::RdsJson::translateVirtualHost(*json_object_ptr, vhost, stats_options), + EnvoyException, err_prefix + err_suffix); } { @@ -136,8 +139,8 @@ TEST(UtilityTest, ObjNameLength) { envoy::api::v2::Cluster cluster; envoy::api::v2::core::ConfigSource eds_config; EXPECT_THROW_WITH_MESSAGE( - Config::CdsJson::translateCluster(*json_object_ptr, eds_config, cluster), EnvoyException, - err_prefix + err_suffix); + Config::CdsJson::translateCluster(*json_object_ptr, eds_config, cluster, stats_options), + EnvoyException, err_prefix + err_suffix); } { @@ -145,8 +148,9 @@ TEST(UtilityTest, ObjNameLength) { std::string json = R"EOF({ "route_config_name": ")EOF" + name + R"EOF(", "cluster": "foo"})EOF"; auto json_object_ptr = Json::Factory::loadFromString(json); envoy::config::filter::network::http_connection_manager::v2::Rds rds; - EXPECT_THROW_WITH_MESSAGE(Config::Utility::translateRdsConfig(*json_object_ptr, rds), - EnvoyException, err_prefix + err_suffix); + EXPECT_THROW_WITH_MESSAGE( + Config::Utility::translateRdsConfig(*json_object_ptr, rds, stats_options), EnvoyException, + err_prefix + err_suffix); } } @@ -160,9 +164,10 @@ TEST(UtilityTest, UnixClusterDns) { auto json_object_ptr = Json::Factory::loadFromString(json); envoy::api::v2::Cluster cluster; envoy::api::v2::core::ConfigSource eds_config; + Stats::StatsOptionsImpl stats_options; EXPECT_THROW_WITH_MESSAGE( - Config::CdsJson::translateCluster(*json_object_ptr, eds_config, cluster), EnvoyException, - "unresolved URL must be TCP scheme, got: unix:///test.sock"); + Config::CdsJson::translateCluster(*json_object_ptr, eds_config, cluster, stats_options), + EnvoyException, "unresolved URL must be TCP scheme, got: unix:///test.sock"); } TEST(UtilityTest, UnixClusterStatic) { @@ -175,7 +180,8 @@ TEST(UtilityTest, UnixClusterStatic) { auto json_object_ptr = Json::Factory::loadFromString(json); envoy::api::v2::Cluster cluster; envoy::api::v2::core::ConfigSource eds_config; - Config::CdsJson::translateCluster(*json_object_ptr, eds_config, cluster); + Stats::StatsOptionsImpl stats_options; + Config::CdsJson::translateCluster(*json_object_ptr, eds_config, cluster, stats_options); EXPECT_EQ("/test.sock", cluster.hosts(0).pipe().path()); } diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index 7f4f77e16d12..e6bd33829d6b 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -47,7 +47,9 @@ Http::TestHeaderMapImpl genHeaders(const std::string& host, const std::string& p envoy::api::v2::RouteConfiguration parseRouteConfigurationFromJson(const std::string& json_string) { envoy::api::v2::RouteConfiguration route_config; auto json_object_ptr = Json::Factory::loadFromString(json_string); - Envoy::Config::RdsJson::translateRouteConfiguration(*json_object_ptr, route_config); + Stats::StatsOptionsImpl stats_options; + Envoy::Config::RdsJson::translateRouteConfiguration(*json_object_ptr, route_config, + stats_options); return route_config; } diff --git a/test/common/router/rds_impl_test.cc b/test/common/router/rds_impl_test.cc index 9acd91d2960a..2d649ca392fe 100644 --- a/test/common/router/rds_impl_test.cc +++ b/test/common/router/rds_impl_test.cc @@ -35,12 +35,12 @@ namespace Router { namespace { envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager -parseHttpConnectionManagerFromJson(const std::string& json_string) { +parseHttpConnectionManagerFromJson(const std::string& json_string, const Stats::Scope& scope) { envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager http_connection_manager; auto json_object_ptr = Json::Factory::loadFromString(json_string); - Envoy::Config::FilterJson::translateHttpConnectionManager(*json_object_ptr, - http_connection_manager); + Envoy::Config::FilterJson::translateHttpConnectionManager( + *json_object_ptr, http_connection_manager, scope.statsOptions()); return http_connection_manager; } @@ -107,12 +107,13 @@ class RdsImplTest : public RdsTestBase { interval_timer_ = new Event::MockTimer(&factory_context_.dispatcher_); EXPECT_CALL(factory_context_.init_manager_, registerTarget(_)); rds_ = - RouteConfigProviderUtil::create(parseHttpConnectionManagerFromJson(config_json), + RouteConfigProviderUtil::create(parseHttpConnectionManagerFromJson(config_json, scope_), factory_context_, "foo.", *route_config_provider_manager_); expectRequest(); factory_context_.init_manager_.initialize(); } + NiceMock scope_; NiceMock server_; std::unique_ptr route_config_provider_manager_; RouteConfigProviderSharedPtr rds_; @@ -131,10 +132,10 @@ TEST_F(RdsImplTest, RdsAndStatic) { } )EOF"; - EXPECT_THROW(RouteConfigProviderUtil::create(parseHttpConnectionManagerFromJson(config_json), - factory_context_, "foo.", - *route_config_provider_manager_), - EnvoyException); + EXPECT_THROW( + RouteConfigProviderUtil::create(parseHttpConnectionManagerFromJson(config_json, scope_), + factory_context_, "foo.", *route_config_provider_manager_), + EnvoyException); } TEST_F(RdsImplTest, LocalInfoNotDefined) { @@ -154,10 +155,10 @@ TEST_F(RdsImplTest, LocalInfoNotDefined) { factory_context_.local_info_.node_.set_cluster(""); factory_context_.local_info_.node_.set_id(""); - EXPECT_THROW(RouteConfigProviderUtil::create(parseHttpConnectionManagerFromJson(config_json), - factory_context_, "foo.", - *route_config_provider_manager_), - EnvoyException); + EXPECT_THROW( + RouteConfigProviderUtil::create(parseHttpConnectionManagerFromJson(config_json, scope_), + factory_context_, "foo.", *route_config_provider_manager_), + EnvoyException); } TEST_F(RdsImplTest, UnknownCluster) { @@ -178,7 +179,7 @@ TEST_F(RdsImplTest, UnknownCluster) { Upstream::ClusterManager::ClusterInfoMap cluster_map; EXPECT_CALL(factory_context_.cluster_manager_, clusters()).WillOnce(Return(cluster_map)); EXPECT_THROW_WITH_MESSAGE( - RouteConfigProviderUtil::create(parseHttpConnectionManagerFromJson(config_json), + RouteConfigProviderUtil::create(parseHttpConnectionManagerFromJson(config_json, scope_), factory_context_, "foo.", *route_config_provider_manager_), EnvoyException, "envoy::api::v2::core::ConfigSource must have a statically defined non-EDS " @@ -362,7 +363,7 @@ class RouteConfigProviderManagerImplTest : public RdsTestBase { )EOF"; Json::ObjectSharedPtr config = Json::Factory::loadFromString(config_json); - Envoy::Config::Utility::translateRdsConfig(*config, rds_); + Envoy::Config::Utility::translateRdsConfig(*config, rds_, stats_options_); // Get a RouteConfigProvider. This one should create an entry in the RouteConfigProviderManager. Upstream::ClusterManager::ClusterInfoMap cluster_map; @@ -386,6 +387,7 @@ class RouteConfigProviderManagerImplTest : public RdsTestBase { ~RouteConfigProviderManagerImplTest() { factory_context_.thread_local_.shutdownThread(); } + Stats::StatsOptionsImpl stats_options_; envoy::config::filter::network::http_connection_manager::v2::Rds rds_; std::unique_ptr route_config_provider_manager_; RouteConfigProviderSharedPtr provider_; @@ -525,7 +527,7 @@ TEST_F(RouteConfigProviderManagerImplTest, Basic) { Json::ObjectSharedPtr config2 = Json::Factory::loadFromString(config_json2); envoy::config::filter::network::http_connection_manager::v2::Rds rds2; - Envoy::Config::Utility::translateRdsConfig(*config2, rds2); + Envoy::Config::Utility::translateRdsConfig(*config2, rds2, stats_options_); Upstream::ClusterManager::ClusterInfoMap cluster_map; Upstream::MockCluster cluster; diff --git a/test/common/router/router_ratelimit_test.cc b/test/common/router/router_ratelimit_test.cc index e2bb39813bcd..5142ae43d6fc 100644 --- a/test/common/router/router_ratelimit_test.cc +++ b/test/common/router/router_ratelimit_test.cc @@ -102,7 +102,8 @@ class RateLimitConfiguration : public testing::Test { void SetUpTest(const std::string json) { envoy::api::v2::RouteConfiguration route_config; auto json_object_ptr = Json::Factory::loadFromString(json); - Envoy::Config::RdsJson::translateRouteConfiguration(*json_object_ptr, route_config); + Envoy::Config::RdsJson::translateRouteConfiguration(*json_object_ptr, route_config, + stats_options); config_.reset(new ConfigImpl(route_config, factory_context_, true)); } @@ -111,6 +112,7 @@ class RateLimitConfiguration : public testing::Test { Http::TestHeaderMapImpl header_; const RouteEntry* route_; Network::Address::Ipv4Instance default_remote_address_{"10.0.0.1"}; + Stats::StatsOptionsImpl stats_options; }; TEST_F(RateLimitConfiguration, NoApplicableRateLimit) { diff --git a/test/common/stats/BUILD b/test/common/stats/BUILD index b3cc4299407e..863c7b626e10 100644 --- a/test/common/stats/BUILD +++ b/test/common/stats/BUILD @@ -27,6 +27,7 @@ envoy_cc_test( deps = [ "//source/common/stats:thread_local_store_lib", "//test/mocks/event:event_mocks", + "//test/mocks/server:server_mocks", "//test/mocks/stats:stats_mocks", "//test/mocks/thread_local:thread_local_mocks", "//test/test_common:utility_lib", diff --git a/test/common/stats/stats_impl_test.cc b/test/common/stats/stats_impl_test.cc index 687f3031ac6d..eb25227e5d58 100644 --- a/test/common/stats/stats_impl_test.cc +++ b/test/common/stats/stats_impl_test.cc @@ -485,7 +485,8 @@ TEST(RawStatDataTest, Consistency) { // vary only when the internal representation of a stat has been intentionally changed, in which // case SharedMemory::VERSION should be incremented as well. uint64_t expected_hash = 1874506077228772558; - uint64_t max_name_length = RawStatData::maxNameLength(); + Stats::StatsOptionsImpl stats_options; + uint64_t max_name_length = stats_options.maxNameLength(); const std::string name_1(max_name_length, 'A'); RawStatData* stat_1 = alloc.alloc(name_1); @@ -504,15 +505,6 @@ TEST(RawStatDataTest, Consistency) { alloc.free(*stat_2); } -// Validate truncation behavior of RawStatData. -TEST(RawStatDataTest, Truncate) { - HeapRawStatDataAllocator alloc; - const std::string long_string(RawStatData::maxNameLength() + 1, 'A'); - RawStatData* stat{}; - EXPECT_LOG_CONTAINS("warning", "is too long with", stat = alloc.alloc(long_string)); - alloc.free(*stat); -} - TEST(RawStatDataTest, HeapAlloc) { HeapRawStatDataAllocator alloc; RawStatData* stat_1 = alloc.alloc("ref_name"); @@ -529,6 +521,19 @@ TEST(RawStatDataTest, HeapAlloc) { alloc.free(*stat_3); } +TEST(RawStatDataTest, Truncate) { + // RawStatData::truncateAndInit(absl::string_view key, const StatsOptions& stats_options) will + // truncate and log to ENVOY_LOG_MISC if given a key longer than the allowed + // stats_options.maxNameLength(). This mechanism is also tested in HotRestartImplTest.truncateKey. + Stats::StatsOptionsImpl stats_options; + const std::string long_string(stats_options.maxNameLength() + 1, 'A'); + RawStatData* stat = + static_cast(::calloc(RawStatData::structSizeWithOptions(stats_options), 1)); + EXPECT_LOG_CONTAINS("warning", "is too long with", + stat->truncateAndInit(long_string, stats_options)); + ::free(stat); +} + TEST(SourceImplTest, Caching) { NiceMock store; std::vector stored_counters; diff --git a/test/common/stats/thread_local_store_test.cc b/test/common/stats/thread_local_store_test.cc index 37affaa4486b..98de7d312d0a 100644 --- a/test/common/stats/thread_local_store_test.cc +++ b/test/common/stats/thread_local_store_test.cc @@ -7,6 +7,7 @@ #include "common/stats/thread_local_store.h" #include "test/mocks/event/mocks.h" +#include "test/mocks/server/mocks.h" #include "test/mocks/stats/mocks.h" #include "test/mocks/thread_local/mocks.h" #include "test/test_common/utility.h" @@ -37,7 +38,7 @@ class StatsThreadLocalStoreTest : public testing::Test, public RawStatDataAlloca })); EXPECT_CALL(*this, alloc("stats.overflow")); - store_.reset(new ThreadLocalStoreImpl(*this)); + store_ = std::make_unique(options_, *this); store_->addSink(sink_); } @@ -46,6 +47,7 @@ class StatsThreadLocalStoreTest : public testing::Test, public RawStatDataAlloca NiceMock main_thread_dispatcher_; NiceMock tls_; + Stats::StatsOptionsImpl options_; TestAllocator alloc_; MockSink sink_; std::unique_ptr store_; @@ -65,7 +67,7 @@ class HistogramTest : public testing::Test, public RawStatDataAllocator { })); EXPECT_CALL(*this, alloc("stats.overflow")); - store_.reset(new ThreadLocalStoreImpl(*this)); + store_ = std::make_unique(options_, *this); store_->addSink(sink_); store_->initializeThreading(main_thread_dispatcher_, tls_); } @@ -157,6 +159,7 @@ class HistogramTest : public testing::Test, public RawStatDataAllocator { NiceMock main_thread_dispatcher_; NiceMock tls_; + Stats::StatsOptionsImpl options_; TestAllocator alloc_; MockSink sink_; std::unique_ptr store_; diff --git a/test/common/upstream/BUILD b/test/common/upstream/BUILD index 76dd45acc995..067ad5734c4b 100644 --- a/test/common/upstream/BUILD +++ b/test/common/upstream/BUILD @@ -30,6 +30,7 @@ envoy_cc_test( srcs = ["cluster_manager_impl_test.cc"], deps = [ ":utility_lib", + "//include/envoy/stats:stats_interface", "//include/envoy/upstream:upstream_interface", "//source/common/config:bootstrap_json_lib", "//source/common/config:utility_lib", @@ -337,9 +338,11 @@ envoy_cc_test_library( name = "utility_lib", hdrs = ["utility.h"], deps = [ + "//include/envoy/stats:stats_interface", "//source/common/config:cds_json_lib", "//source/common/json:json_loader_lib", "//source/common/network:utility_lib", + "//source/common/stats:stats_lib", "//source/common/upstream:upstream_includes", "//source/common/upstream:upstream_lib", ], diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index deb082bcb2b2..5afbfac84911 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -3,6 +3,7 @@ #include "envoy/admin/v2alpha/config_dump.pb.h" #include "envoy/network/listen_socket.h" +#include "envoy/stats/stats.h" #include "envoy/upstream/upstream.h" #include "common/config/bootstrap_json.h" @@ -164,7 +165,9 @@ class ClusterManagerImplTest : public testing::Test { envoy::config::bootstrap::v2::Bootstrap parseBootstrapFromJson(const std::string& json_string) { envoy::config::bootstrap::v2::Bootstrap bootstrap; auto json_object_ptr = Json::Factory::loadFromString(json_string); - Config::BootstrapJson::translateClusterManagerBootstrap(*json_object_ptr, bootstrap); + Stats::StatsOptionsImpl stats_options; + Config::BootstrapJson::translateClusterManagerBootstrap(*json_object_ptr, bootstrap, + stats_options); return bootstrap; } diff --git a/test/common/upstream/utility.h b/test/common/upstream/utility.h index 9d1c43d38240..716e2f2edee7 100644 --- a/test/common/upstream/utility.h +++ b/test/common/upstream/utility.h @@ -1,11 +1,13 @@ #pragma once +#include "envoy/stats/stats.h" #include "envoy/upstream/upstream.h" #include "common/common/utility.h" #include "common/config/cds_json.h" #include "common/json/json_loader.h" #include "common/network/utility.h" +#include "common/stats/stats_impl.h" #include "common/upstream/upstream_impl.h" #include "fmt/printf.h" @@ -46,8 +48,10 @@ inline std::string clustersJson(const std::vector& clusters) { inline envoy::api::v2::Cluster parseClusterFromJson(const std::string& json_string) { envoy::api::v2::Cluster cluster; auto json_object_ptr = Json::Factory::loadFromString(json_string); + Stats::StatsOptionsImpl stats_options; Config::CdsJson::translateCluster(*json_object_ptr, - absl::optional(), cluster); + absl::optional(), cluster, + stats_options); return cluster; } @@ -66,7 +70,8 @@ parseSdsClusterFromJson(const std::string& json_string, const envoy::api::v2::core::ConfigSource eds_config) { envoy::api::v2::Cluster cluster; auto json_object_ptr = Json::Factory::loadFromString(json_string); - Config::CdsJson::translateCluster(*json_object_ptr, eds_config, cluster); + Stats::StatsOptionsImpl stats_options; + Config::CdsJson::translateCluster(*json_object_ptr, eds_config, cluster, stats_options); return cluster; } diff --git a/test/extensions/filters/http/dynamo/BUILD b/test/extensions/filters/http/dynamo/BUILD index f4a644d0ff40..01797af74456 100644 --- a/test/extensions/filters/http/dynamo/BUILD +++ b/test/extensions/filters/http/dynamo/BUILD @@ -46,6 +46,7 @@ envoy_extension_cc_test( deps = [ "//source/common/stats:stats_lib", "//source/extensions/filters/http/dynamo:dynamo_utility_lib", + "//test/mocks/stats:stats_mocks", ], ) diff --git a/test/extensions/filters/http/dynamo/dynamo_filter_test.cc b/test/extensions/filters/http/dynamo/dynamo_filter_test.cc index 005822a42ec5..5215f814eee4 100644 --- a/test/extensions/filters/http/dynamo/dynamo_filter_test.cc +++ b/test/extensions/filters/http/dynamo/dynamo_filter_test.cc @@ -44,7 +44,7 @@ class DynamoFilterTest : public testing::Test { std::unique_ptr filter_; NiceMock loader_; std::string stat_prefix_{"prefix."}; - Stats::MockStore stats_; + NiceMock stats_; NiceMock decoder_callbacks_; NiceMock encoder_callbacks_; }; diff --git a/test/extensions/filters/http/dynamo/dynamo_utility_test.cc b/test/extensions/filters/http/dynamo/dynamo_utility_test.cc index 2661e1819a32..0e4b31872cce 100644 --- a/test/extensions/filters/http/dynamo/dynamo_utility_test.cc +++ b/test/extensions/filters/http/dynamo/dynamo_utility_test.cc @@ -4,9 +4,13 @@ #include "extensions/filters/http/dynamo/dynamo_utility.h" +#include "test/mocks/stats/mocks.h" + #include "gmock/gmock.h" #include "gtest/gtest.h" +using testing::NiceMock; +using testing::Return; using testing::_; namespace Envoy { @@ -15,17 +19,20 @@ namespace HttpFilters { namespace Dynamo { TEST(DynamoUtility, PartitionIdStatString) { + Stats::StatsOptionsImpl stats_options; + stats_options.max_obj_name_length_ = 60; + { std::string stat_prefix = "stat.prefix."; std::string table_name = "locations"; std::string operation = "GetItem"; std::string partition_id = "6235c781-1d0d-47a3-a4ea-eec04c5883ca"; - std::string partition_stat_string = - Utility::buildPartitionStatString(stat_prefix, table_name, operation, partition_id); + std::string partition_stat_string = Utility::buildPartitionStatString( + stat_prefix, table_name, operation, partition_id, stats_options); std::string expected_stat_string = "stat.prefix.table.locations.capacity.GetItem.__partition_id=c5883ca"; EXPECT_EQ(expected_stat_string, partition_stat_string); - EXPECT_TRUE(partition_stat_string.size() <= Stats::RawStatData::maxNameLength()); + EXPECT_TRUE(partition_stat_string.size() <= stats_options.maxNameLength()); } { @@ -34,13 +41,13 @@ TEST(DynamoUtility, PartitionIdStatString) { std::string operation = "GetItem"; std::string partition_id = "6235c781-1d0d-47a3-a4ea-eec04c5883ca"; - std::string partition_stat_string = - Utility::buildPartitionStatString(stat_prefix, table_name, operation, partition_id); + std::string partition_stat_string = Utility::buildPartitionStatString( + stat_prefix, table_name, operation, partition_id, stats_options); std::string expected_stat_string = "http.egress_dynamodb_iad.dynamodb.table.locations-sandbox-" "partition-test-iad-mytest-rea.capacity.GetItem.__partition_" "id=c5883ca"; EXPECT_EQ(expected_stat_string, partition_stat_string); - EXPECT_TRUE(partition_stat_string.size() == Stats::RawStatData::maxNameLength()); + EXPECT_TRUE(partition_stat_string.size() <= stats_options.maxNameLength()); } { std::string stat_prefix = "http.egress_dynamodb_iad.dynamodb."; @@ -48,14 +55,14 @@ TEST(DynamoUtility, PartitionIdStatString) { std::string operation = "GetItem"; std::string partition_id = "6235c781-1d0d-47a3-a4ea-eec04c5883ca"; - std::string partition_stat_string = - Utility::buildPartitionStatString(stat_prefix, table_name, operation, partition_id); + std::string partition_stat_string = Utility::buildPartitionStatString( + stat_prefix, table_name, operation, partition_id, stats_options); std::string expected_stat_string = "http.egress_dynamodb_iad.dynamodb.table.locations-sandbox-" "partition-test-iad-mytest-rea.capacity.GetItem.__partition_" "id=c5883ca"; EXPECT_EQ(expected_stat_string, partition_stat_string); - EXPECT_TRUE(partition_stat_string.size() == Stats::RawStatData::maxNameLength()); + EXPECT_TRUE(partition_stat_string.size() <= stats_options.maxNameLength()); } } diff --git a/test/extensions/filters/network/http_connection_manager/config_test.cc b/test/extensions/filters/network/http_connection_manager/config_test.cc index d6acb20c5b24..819f4221731e 100644 --- a/test/extensions/filters/network/http_connection_manager/config_test.cc +++ b/test/extensions/filters/network/http_connection_manager/config_test.cc @@ -29,7 +29,9 @@ parseHttpConnectionManagerFromJson(const std::string& json_string) { envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager http_connection_manager; auto json_object_ptr = Json::Factory::loadFromString(json_string); - Config::FilterJson::translateHttpConnectionManager(*json_object_ptr, http_connection_manager); + NiceMock scope; + Config::FilterJson::translateHttpConnectionManager(*json_object_ptr, http_connection_manager, + scope.statsOptions()); return http_connection_manager; } diff --git a/test/integration/BUILD b/test/integration/BUILD index 8dfbd2ba8727..05811781ad87 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -253,6 +253,7 @@ envoy_cc_test_library( "//include/envoy/server:configuration_interface", "//include/envoy/server:hot_restart_interface", "//include/envoy/server:options_interface", + "//include/envoy/stats:stats_interface", "//source/common/api:api_lib", "//source/common/buffer:buffer_lib", "//source/common/buffer:zero_copy_input_stream_lib", @@ -284,6 +285,7 @@ envoy_cc_test_library( "//test/common/upstream:utility_lib", "//test/config:utility_lib", "//test/mocks/buffer:buffer_mocks", + "//test/mocks/server:server_mocks", "//test/mocks/upstream:upstream_mocks", "//test/test_common:environment_lib", "//test/test_common:network_utility_lib", diff --git a/test/integration/server.cc b/test/integration/server.cc index ed0f40191beb..642dd6e0257f 100644 --- a/test/integration/server.cc +++ b/test/integration/server.cc @@ -15,6 +15,7 @@ #include "test/integration/integration.h" #include "test/integration/utility.h" #include "test/mocks/runtime/mocks.h" +#include "test/mocks/server/mocks.h" #include "test/test_common/environment.h" #include "gtest/gtest.h" @@ -92,7 +93,8 @@ void IntegrationTestServer::threadRoutine(const Network::Address::IpVersion vers ThreadLocal::InstanceImpl tls; Stats::HeapRawStatDataAllocator stats_allocator; - Stats::ThreadLocalStoreImpl stats_store(stats_allocator); + Stats::StatsOptionsImpl options_; + Stats::ThreadLocalStoreImpl stats_store(options_, stats_allocator); stat_store_ = &stats_store; Runtime::RandomGeneratorPtr random_generator; if (deterministic) { diff --git a/test/integration/server.h b/test/integration/server.h index 9919bbd370a8..80c3dc0cb198 100644 --- a/test/integration/server.h +++ b/test/integration/server.h @@ -7,6 +7,7 @@ #include #include "envoy/server/options.h" +#include "envoy/stats/stats.h" #include "common/common/assert.h" #include "common/common/lock_guard.h" @@ -62,7 +63,7 @@ class TestOptionsImpl : public Options { const std::string& serviceNodeName() const override { return service_node_name_; } const std::string& serviceZone() const override { return service_zone_; } uint64_t maxStats() const override { return 16384; } - uint64_t maxObjNameLength() const override { return 60; } + const Stats::StatsOptions& statsOptions() const override { return stats_options_; } bool hotRestartDisabled() const override { return false; } // asConfigYaml returns a new config that empties the configPath() and populates configYaml() @@ -76,6 +77,7 @@ class TestOptionsImpl : public Options { const std::string service_cluster_name_; const std::string service_node_name_; const std::string service_zone_; + Stats::StatsOptionsImpl stats_options_; const std::string log_path_; }; @@ -138,9 +140,12 @@ class TestScopeWrapper : public Scope { return wrapped_scope_->histogram(name); } + const Stats::StatsOptions& statsOptions() const override { return stats_options_; } + private: Thread::MutexBasicLockable& lock_; ScopePtr wrapped_scope_; + Stats::StatsOptionsImpl stats_options_; }; /** @@ -168,6 +173,7 @@ class TestIsolatedStoreImpl : public StoreRoot { Thread::LockGuard lock(lock_); return store_.histogram(name); } + const Stats::StatsOptions& statsOptions() const override { return stats_options_; } // Stats::Store std::vector counters() const override { @@ -196,6 +202,7 @@ class TestIsolatedStoreImpl : public StoreRoot { mutable Thread::MutexBasicLockable lock_; IsolatedStoreImpl store_; SourceImpl source_; + Stats::StatsOptionsImpl stats_options_; }; } // namespace Stats diff --git a/test/mocks/server/mocks.cc b/test/mocks/server/mocks.cc index 936bc7fcf8a8..46920ab56429 100644 --- a/test/mocks/server/mocks.cc +++ b/test/mocks/server/mocks.cc @@ -28,7 +28,7 @@ MockOptions::MockOptions(const std::string& config_path) : config_path_(config_p ON_CALL(*this, serviceZone()).WillByDefault(ReturnRef(service_zone_name_)); ON_CALL(*this, logPath()).WillByDefault(ReturnRef(log_path_)); ON_CALL(*this, maxStats()).WillByDefault(Return(1000)); - ON_CALL(*this, maxObjNameLength()).WillByDefault(Return(150)); + ON_CALL(*this, statsOptions()).WillByDefault(ReturnRef(stats_options_)); ON_CALL(*this, hotRestartDisabled()).WillByDefault(ReturnPointee(&hot_restart_disabled_)); } MockOptions::~MockOptions() {} diff --git a/test/mocks/server/mocks.h b/test/mocks/server/mocks.h index a11cc8a40a18..324088fbf111 100644 --- a/test/mocks/server/mocks.h +++ b/test/mocks/server/mocks.h @@ -68,7 +68,7 @@ class MockOptions : public Options { MOCK_CONST_METHOD0(serviceNodeName, const std::string&()); MOCK_CONST_METHOD0(serviceZone, const std::string&()); MOCK_CONST_METHOD0(maxStats, uint64_t()); - MOCK_CONST_METHOD0(maxObjNameLength, uint64_t()); + MOCK_CONST_METHOD0(statsOptions, const Stats::StatsOptions&()); MOCK_CONST_METHOD0(hotRestartDisabled, bool()); std::string config_path_; @@ -79,6 +79,7 @@ class MockOptions : public Options { std::string service_node_name_; std::string service_zone_name_; std::string log_path_; + Stats::StatsOptionsImpl stats_options_; bool hot_restart_disabled_{}; }; diff --git a/test/mocks/stats/mocks.cc b/test/mocks/stats/mocks.cc index 10fef6363055..9ec20c8dec6c 100644 --- a/test/mocks/stats/mocks.cc +++ b/test/mocks/stats/mocks.cc @@ -79,6 +79,7 @@ MockStore::MockStore() { histograms_.emplace_back(histogram); return *histogram; })); + ON_CALL(*this, statsOptions()).WillByDefault(ReturnRef(stats_options_)); } MockStore::~MockStore() {} diff --git a/test/mocks/stats/mocks.h b/test/mocks/stats/mocks.h index 5237d16ea557..c98fa2758f1b 100644 --- a/test/mocks/stats/mocks.h +++ b/test/mocks/stats/mocks.h @@ -145,9 +145,11 @@ class MockStore : public Store { MOCK_CONST_METHOD0(gauges, std::vector()); MOCK_METHOD1(histogram, Histogram&(const std::string& name)); MOCK_CONST_METHOD0(histograms, std::vector()); + MOCK_CONST_METHOD0(statsOptions, const Stats::StatsOptions&()); testing::NiceMock counter_; std::vector> histograms_; + StatsOptionsImpl stats_options_; }; /** diff --git a/test/server/hot_restart_impl_test.cc b/test/server/hot_restart_impl_test.cc index bda4509117b8..de7a1965d34c 100644 --- a/test/server/hot_restart_impl_test.cc +++ b/test/server/hot_restart_impl_test.cc @@ -14,6 +14,7 @@ using testing::Invoke; using testing::InvokeWithoutArgs; using testing::Return; +using testing::ReturnRef; using testing::WithArg; using testing::_; @@ -33,24 +34,17 @@ class HotRestartImplTest : public testing::Test { return buffer_.data(); })); EXPECT_CALL(os_sys_calls_, bind(_, _, _)); - - Stats::RawStatData::configureForTestsOnly(options_); + EXPECT_CALL(options_, statsOptions()).WillRepeatedly(ReturnRef(stats_options_)); // Test we match the correct stat with empty-slots before, after, or both. hot_restart_.reset(new HotRestartImpl(options_)); hot_restart_->drainParentListeners(); } - void TearDown() { - // Configure it back so that later tests don't get the wonky values - // used here - NiceMock default_options; - Stats::RawStatData::configureForTestsOnly(default_options); - } - Api::MockOsSysCalls os_sys_calls_; TestThreadsafeSingletonInjector os_calls{&os_sys_calls_}; NiceMock options_; + Stats::StatsOptionsImpl stats_options_; std::vector buffer_; std::unique_ptr hot_restart_; }; @@ -68,7 +62,7 @@ TEST_F(HotRestartImplTest, versionString) { version = hot_restart_->version(); EXPECT_TRUE(absl::StartsWith(version, fmt::format("{}.", SharedMemory::VERSION))) << version; max_stats = options_.maxStats(); // Save this so we can double it below. - max_obj_name_length = options_.maxObjNameLength(); + max_obj_name_length = options_.statsOptions().maxObjNameLength(); TearDown(); } @@ -86,7 +80,7 @@ TEST_F(HotRestartImplTest, versionString) { } { - ON_CALL(options_, maxObjNameLength()).WillByDefault(Return(2 * max_obj_name_length)); + stats_options_.max_obj_name_length_ = 2 * max_obj_name_length; setup(); EXPECT_NE(version, hot_restart_->version()) << "Version changes when max-obj-name-length changes"; @@ -123,7 +117,7 @@ TEST_F(HotRestartImplTest, crossAlloc) { TEST_F(HotRestartImplTest, truncateKey) { setup(); - std::string key1(Stats::RawStatData::maxNameLength(), 'a'); + std::string key1(options_.statsOptions().maxNameLength(), 'a'); Stats::RawStatData* stat1 = hot_restart_->alloc(key1); std::string key2 = key1 + "a"; Stats::RawStatData* stat2 = hot_restart_->alloc(key2); @@ -150,12 +144,15 @@ class HotRestartImplAlignmentTest : public HotRestartImplTest, public testing::WithParamInterface { public: HotRestartImplAlignmentTest() : name_len_(8 + GetParam()) { + stats_options_.max_obj_name_length_ = name_len_; + EXPECT_CALL(options_, statsOptions()).WillRepeatedly(ReturnRef(stats_options_)); EXPECT_CALL(options_, maxStats()).WillRepeatedly(Return(num_stats_)); - EXPECT_CALL(options_, maxObjNameLength()).WillRepeatedly(Return(name_len_)); + setup(); - EXPECT_EQ(name_len_, Stats::RawStatData::maxObjNameLength()); + EXPECT_EQ(name_len_ + stats_options_.maxStatSuffixLength(), stats_options_.maxNameLength()); } + Stats::StatsOptionsImpl stats_options_; static const uint64_t num_stats_ = 8; const uint64_t name_len_; }; @@ -185,7 +182,7 @@ TEST_P(HotRestartImplAlignmentTest, objectOverlap) { "zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz" "zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz", i) - .substr(0, Stats::RawStatData::maxNameLength()); + .substr(0, stats_options_.maxNameLength()); TestStat ts; ts.stat_ = hot_restart_->alloc(name); ts.name_ = ts.stat_->name_; @@ -193,7 +190,7 @@ TEST_P(HotRestartImplAlignmentTest, objectOverlap) { // If this isn't true then the hard coded part of the name isn't long enough to make the test // valid. - EXPECT_EQ(ts.name_.size(), Stats::RawStatData::maxNameLength()); + EXPECT_EQ(ts.name_.size(), stats_options_.maxNameLength()); stats.push_back(ts); } diff --git a/test/server/http/admin_test.cc b/test/server/http/admin_test.cc index d092c6d359fc..546724e98ed4 100644 --- a/test/server/http/admin_test.cc +++ b/test/server/http/admin_test.cc @@ -53,7 +53,7 @@ class AdminStatsTest : public testing::TestWithParam(options_, *this); store_->addSink(sink_); } @@ -71,6 +71,7 @@ class AdminStatsTest : public testing::TestWithParam tls_; Stats::TestAllocator alloc_; Stats::MockSink sink_; + Stats::StatsOptionsImpl options_; std::unique_ptr store_; }; diff --git a/test/server/options_impl_test.cc b/test/server/options_impl_test.cc index c6077699e41c..d5d7a6a5f4f0 100644 --- a/test/server/options_impl_test.cc +++ b/test/server/options_impl_test.cc @@ -37,16 +37,6 @@ std::unique_ptr createOptionsImpl(const std::string& args) { } // namespace TEST(OptionsImplTest, HotRestartVersion) { - // There's an evil static local in - // Stats::RawStatsData::initializeAndGetMutableMaxObjNameLength, which causes - // problems when all test.cc files are linked together for coverage-testing. - // This resets the static to the default options-value of 60. Note; this is only - // needed in coverage tests. - { - auto options = createOptionsImpl("envoy"); - Stats::RawStatData::configureForTestsOnly(*options); - } - EXPECT_THROW_WITH_REGEX(createOptionsImpl("envoy --hot-restart-version"), NoServingException, "NoServingException"); } @@ -113,6 +103,10 @@ TEST(OptionsImplTest, SetAll) { std::unique_ptr options = createOptionsImpl("envoy -c hello"); bool v2_config_only = options->v2ConfigOnly(); bool hot_restart_disabled = options->hotRestartDisabled(); + Stats::StatsOptionsImpl stats_options; + stats_options.max_obj_name_length_ = 54321; + stats_options.max_stat_suffix_length_ = 1234; + options->setBaseId(109876); options->setConcurrency(42); options->setConfigPath("foo"); @@ -132,7 +126,7 @@ TEST(OptionsImplTest, SetAll) { options->setServiceNodeName("node_foo"); options->setServiceZone("zone_foo"); options->setMaxStats(12345); - options->setMaxObjNameLength(54321); + options->setStatsOptions(stats_options); options->setHotRestartDisabled(!options->hotRestartDisabled()); EXPECT_EQ(109876, options->baseId()); @@ -154,7 +148,8 @@ TEST(OptionsImplTest, SetAll) { EXPECT_EQ("node_foo", options->serviceNodeName()); EXPECT_EQ("zone_foo", options->serviceZone()); EXPECT_EQ(12345U, options->maxStats()); - EXPECT_EQ(54321U, options->maxObjNameLength()); + EXPECT_EQ(stats_options.max_obj_name_length_, options->statsOptions().maxObjNameLength()); + EXPECT_EQ(stats_options.max_stat_suffix_length_, options->statsOptions().maxStatSuffixLength()); EXPECT_EQ(!hot_restart_disabled, options->hotRestartDisabled()); } diff --git a/test/server/utility.h b/test/server/utility.h index 9f9376e62cb1..76838f2c79c2 100644 --- a/test/server/utility.h +++ b/test/server/utility.h @@ -10,7 +10,8 @@ namespace { inline envoy::api::v2::Listener parseListenerFromJson(const std::string& json_string) { envoy::api::v2::Listener listener; auto json_object_ptr = Json::Factory::loadFromString(json_string); - Config::LdsJson::translateListener(*json_object_ptr, listener); + Stats::StatsOptionsImpl stats_options; + Config::LdsJson::translateListener(*json_object_ptr, listener, stats_options); return listener; } diff --git a/test/test_common/utility.cc b/test/test_common/utility.cc index 3dfdfeb0dfa0..85508ea5e675 100644 --- a/test/test_common/utility.cc +++ b/test/test_common/utility.cc @@ -143,7 +143,8 @@ envoy::config::bootstrap::v2::Bootstrap TestUtility::parseBootstrapFromJson(const std::string& json_string) { envoy::config::bootstrap::v2::Bootstrap bootstrap; auto json_object_ptr = Json::Factory::loadFromString(json_string); - Config::BootstrapJson::translateBootstrap(*json_object_ptr, bootstrap); + Stats::StatsOptionsImpl stats_options; + Config::BootstrapJson::translateBootstrap(*json_object_ptr, bootstrap, stats_options); return bootstrap; } diff --git a/test/test_common/utility.h b/test/test_common/utility.h index 72ca8699c47a..c972c9ad407c 100644 --- a/test/test_common/utility.h +++ b/test/test_common/utility.h @@ -342,10 +342,13 @@ class TestAllocator : public RawStatDataAllocator { ~TestAllocator() { EXPECT_TRUE(stats_.empty()); } RawStatData* alloc(const std::string& name) override { + Stats::StatsOptionsImpl stats_options; + stats_options.max_obj_name_length_ = 127; CSmartPtr& stat_ref = stats_[name]; if (!stat_ref) { - stat_ref.reset(static_cast(::calloc(RawStatData::size(), 1))); - stat_ref->initialize(name); + stat_ref.reset(static_cast( + ::calloc(RawStatData::structSizeWithOptions(stats_options), 1))); + stat_ref->truncateAndInit(name, stats_options); } else { stat_ref->ref_count_++; } diff --git a/test/tools/router_check/BUILD b/test/tools/router_check/BUILD index 90b29481696f..0b0ab09cbc3a 100644 --- a/test/tools/router_check/BUILD +++ b/test/tools/router_check/BUILD @@ -27,6 +27,7 @@ envoy_cc_test_library( "//source/common/http:headers_lib", "//source/common/json:json_loader_lib", "//source/common/router:config_lib", + "//source/common/stats:stats_lib", "//test/mocks/server:server_mocks", "//test/test_common:printers_lib", "//test/test_common:utility_lib", diff --git a/test/tools/router_check/router.cc b/test/tools/router_check/router.cc index 9947c285b0c1..1d9dbbfe5d03 100644 --- a/test/tools/router_check/router.cc +++ b/test/tools/router_check/router.cc @@ -7,6 +7,7 @@ #include "common/network/utility.h" #include "common/request_info/request_info_impl.h" +#include "common/stats/stats_impl.h" #include "test/test_common/printers.h" @@ -44,7 +45,9 @@ RouterCheckTool RouterCheckTool::create(const std::string& router_config_json) { // TODO(hennna): Allow users to load a full config and extract the route configuration from it. Json::ObjectSharedPtr loader = Json::Factory::loadFromFile(router_config_json); envoy::api::v2::RouteConfiguration route_config; - Config::RdsJson::translateRouteConfiguration(*loader, route_config); + // TODO(ambuc): Add a CLI option to allow for a maxStatNameLength constraint + Stats::StatsOptionsImpl stats_options; + Config::RdsJson::translateRouteConfiguration(*loader, route_config, stats_options); std::unique_ptr> factory_context( std::make_unique>()); diff --git a/test/tools/schema_validator/BUILD b/test/tools/schema_validator/BUILD index bea8d2dd0fbb..5d67c3230eb5 100644 --- a/test/tools/schema_validator/BUILD +++ b/test/tools/schema_validator/BUILD @@ -27,6 +27,7 @@ envoy_cc_test_library( "//source/common/config:rds_json_lib", "//source/common/json:json_loader_lib", "//source/common/router:config_lib", + "//source/common/stats:stats_lib", "//test/mocks/runtime:runtime_mocks", "//test/mocks/upstream:upstream_mocks", "//test/test_common:printers_lib", diff --git a/test/tools/schema_validator/validator.cc b/test/tools/schema_validator/validator.cc index fde2decc8eeb..6c6dc4e93376 100644 --- a/test/tools/schema_validator/validator.cc +++ b/test/tools/schema_validator/validator.cc @@ -1,6 +1,7 @@ #include "test/tools/schema_validator/validator.h" #include "common/router/config_impl.h" +#include "common/stats/stats_impl.h" #include "test/test_common/printers.h" @@ -55,7 +56,10 @@ void Validator::validate(const std::string& json_path, Schema::Type schema_type) // Construct a envoy::api::v2::RouteConfiguration to validate the Route configuration and // ignore the output since nothing will consume it. envoy::api::v2::RouteConfiguration route_config; - Config::RdsJson::translateRouteConfiguration(*loader, route_config); + // TODO(ambuc): Add a CLI option to the schema_validator to allow for a maxStatNameLength + // constraint + Stats::StatsOptionsImpl stats_options; + Config::RdsJson::translateRouteConfiguration(*loader, route_config, stats_options); break; } default: diff --git a/tools/BUILD b/tools/BUILD index f1896114d4e1..87880b11b6d5 100644 --- a/tools/BUILD +++ b/tools/BUILD @@ -41,6 +41,7 @@ envoy_cc_binary( "//source/common/config:bootstrap_json_lib", "//source/common/json:json_loader_lib", "//source/common/protobuf:utility_lib", + "//source/common/stats:stats_lib", "@envoy_api//envoy/config/bootstrap/v2:bootstrap_cc", ], ) diff --git a/tools/v1_to_bootstrap.cc b/tools/v1_to_bootstrap.cc index 20ec6e5ea516..e712e1bcb2c2 100644 --- a/tools/v1_to_bootstrap.cc +++ b/tools/v1_to_bootstrap.cc @@ -13,6 +13,7 @@ #include "common/config/bootstrap_json.h" #include "common/json/json_loader.h" #include "common/protobuf/utility.h" +#include "common/stats/stats_impl.h" // NOLINT(namespace-envoy) int main(int argc, char** argv) { @@ -23,7 +24,8 @@ int main(int argc, char** argv) { envoy::config::bootstrap::v2::Bootstrap bootstrap; auto config_json = Envoy::Json::Factory::loadFromFile(argv[1]); - Envoy::Config::BootstrapJson::translateBootstrap(*config_json, bootstrap); + Envoy::Stats::StatsOptionsImpl stats_options; + Envoy::Config::BootstrapJson::translateBootstrap(*config_json, bootstrap, stats_options); Envoy::MessageUtil::validate(bootstrap); std::cout << Envoy::MessageUtil::getJsonStringFromMessage(bootstrap, true);