From fa5d9a2a4f6501ee435c4332dfc8be74cd2555c9 Mon Sep 17 00:00:00 2001 From: Tony Allen Date: Sun, 30 Sep 2018 04:30:44 -0700 Subject: [PATCH] route match: Add runtime_fraction field for more granular routing (#4560) This patch reintroduces PR #4217. Signed-off-by: Tony Allen Signed-off-by: Aaltan Ahmad --- DEPRECATED.md | 3 + api/envoy/api/v2/core/BUILD | 4 + api/envoy/api/v2/core/base.proto | 12 +++ api/envoy/api/v2/route/route.proto | 37 +++++-- source/common/access_log/access_log_impl.cc | 7 +- source/common/protobuf/utility.cc | 5 +- source/common/protobuf/utility.h | 5 +- source/common/router/config_impl.cc | 46 +++++++-- source/common/router/config_impl.h | 12 +-- .../filters/http/fault/fault_filter.cc | 10 +- .../filters/network/mongo_proxy/proxy.cc | 2 +- test/common/router/config_impl_test.cc | 96 ++++++++++++++++++- 12 files changed, 198 insertions(+), 41 deletions(-) diff --git a/DEPRECATED.md b/DEPRECATED.md index 770b0d954723..a4980347c1f3 100644 --- a/DEPRECATED.md +++ b/DEPRECATED.md @@ -29,6 +29,9 @@ A logged warning is expected for each deprecated item that is in deprecation win * Setting hosts via `hosts` field in `Cluster` is deprecated. Use `load_assignment` instead. * Use of `response_headers_to_*` and `request_headers_to_add` are deprecated at the `RouteAction` level. Please use the configuration options at the `Route` level. +* Use of `runtime` in `RouteMatch`, found in + [route.proto](https://github.com/envoyproxy/envoy/blob/master/api/envoy/api/v2/route/route.proto). + Set the `runtime_fraction` field instead. * Use of the string `user` field in `Authenticated` in [rbac.proto](https://github.com/envoyproxy/envoy/blob/master/api/envoy/config/rbac/v2alpha/rbac.proto) is deprecated in favor of the new `StringMatcher` based `principal_name` field. diff --git a/api/envoy/api/v2/core/BUILD b/api/envoy/api/v2/core/BUILD index 71a8d33f59d3..45251aebb4ba 100644 --- a/api/envoy/api/v2/core/BUILD +++ b/api/envoy/api/v2/core/BUILD @@ -37,11 +37,15 @@ api_proto_library_internal( visibility = [ ":friends", ], + deps = [ + "//envoy/type:percent", + ], ) api_go_proto_library( name = "base", proto = ":base", + deps = ["//envoy/type:percent_go_proto"], ) api_proto_library_internal( diff --git a/api/envoy/api/v2/core/base.proto b/api/envoy/api/v2/core/base.proto index 40204e38d87c..14c9a89a76a4 100644 --- a/api/envoy/api/v2/core/base.proto +++ b/api/envoy/api/v2/core/base.proto @@ -9,6 +9,8 @@ import "google/protobuf/wrappers.proto"; import "validate/validate.proto"; import "gogoproto/gogo.proto"; +import "envoy/type/percent.proto"; + option (gogoproto.equal_all) = true; // [#protodoc-title: Common types] @@ -216,3 +218,13 @@ message SocketOption { SocketState state = 6 [(validate.rules).message.required = true, (validate.rules).enum.defined_only = true]; } + +// Runtime derived FractionalPercent with defaults for when the numerator or denominator is not +// specified via a runtime key. +message RuntimeFractionalPercent { + // Default value if the runtime value's for the numerator/denominator keys are not available. + envoy.type.FractionalPercent default_value = 1 [(validate.rules).message.required = true]; + + // Runtime key for a YAML representation of a FractionalPercent. + string runtime_key = 2; +} diff --git a/api/envoy/api/v2/route/route.proto b/api/envoy/api/v2/route/route.proto index 51fb5ce5f91c..edb26bc1c391 100644 --- a/api/envoy/api/v2/route/route.proto +++ b/api/envoy/api/v2/route/route.proto @@ -291,16 +291,33 @@ message RouteMatch { // is true. google.protobuf.BoolValue case_sensitive = 4; - // Indicates that the route should additionally match on a runtime key. An - // integer between 0-100. Every time the route is considered for a match, a - // random number between 0-99 is selected. If the number is <= the value found - // in the key (checked first) or, if the key is not present, the default - // value, the route is a match (assuming everything also about the route - // matches). A runtime route configuration can be used to roll out route changes in a - // gradual manner without full code/config deploys. Refer to the - // :ref:`traffic shifting ` docs - // for additional documentation. - core.RuntimeUInt32 runtime = 5; + oneof runtime_specifier { + // Indicates that the route should additionally match on a runtime key. An integer between + // 0-100. Every time the route is considered for a match, a random number between 0-99 is + // selected. If the number is <= the value found in the key (checked first) or, if the key is + // not present, the default value, the route is a match (assuming everything also about the + // route matches). A runtime route configuration can be used to roll out route changes in a + // gradual manner without full code/config deploys. Refer to the :ref:`traffic shifting + // ` docs for additional + // documentation. + // + // .. attention:: + // + // **This field is deprecated**. Set the + // :ref:`runtime_fraction` field instead. + core.RuntimeUInt32 runtime = 5 [deprecated = true]; + + // Indicates that the route should additionally match on a runtime key. Every time the route + // is considered for a match, it must also fall under the percentage of matches indicated by + // this field. For some fraction N/D, a random number in the range [0,D) is selected. If the + // number is <= the value of the numberator N, or if the key is not present, the default + // value, the router continues to evaluate the remaining match criteria. A runtime_fraction + // route configuration can be used to roll out route changes in a gradual manner (with more + // granularity than the deprecated runtime field) without full code/config deploys. Refer to + // the :ref:`traffic shifting ` docs + // for additional documentation. + core.RuntimeFractionalPercent runtime_fraction = 9; + } // Specifies a set of headers that the route should match on. The router will // check the request’s headers against all the specified headers in the route diff --git a/source/common/access_log/access_log_impl.cc b/source/common/access_log/access_log_impl.cc index 163b695cf025..09db698dbcff 100644 --- a/source/common/access_log/access_log_impl.cc +++ b/source/common/access_log/access_log_impl.cc @@ -113,14 +113,15 @@ bool RuntimeFilter::evaluate(const RequestInfo::RequestInfo&, const Http::HeaderEntry* uuid = request_header.RequestId(); uint64_t random_value; if (use_independent_randomness_ || uuid == nullptr || - !UuidUtils::uuidModBy(uuid->value().c_str(), random_value, - ProtobufPercentHelper::fractionalPercentDenominatorToInt(percent_))) { + !UuidUtils::uuidModBy( + uuid->value().c_str(), random_value, + ProtobufPercentHelper::fractionalPercentDenominatorToInt(percent_.denominator()))) { random_value = random_.random(); } return runtime_.snapshot().featureEnabled( runtime_key_, percent_.numerator(), random_value, - ProtobufPercentHelper::fractionalPercentDenominatorToInt(percent_)); + ProtobufPercentHelper::fractionalPercentDenominatorToInt(percent_.denominator())); } OperatorFilter::OperatorFilter(const Protobuf::RepeatedPtrField< diff --git a/source/common/protobuf/utility.cc b/source/common/protobuf/utility.cc index b2b5b82faeaa..d8a3ab138f50 100644 --- a/source/common/protobuf/utility.cc +++ b/source/common/protobuf/utility.cc @@ -20,8 +20,9 @@ uint64_t convertPercent(double percent, uint64_t max_value) { return max_value * (percent / 100.0); } -uint64_t fractionalPercentDenominatorToInt(const envoy::type::FractionalPercent& percent) { - switch (percent.denominator()) { +uint64_t fractionalPercentDenominatorToInt( + const envoy::type::FractionalPercent::DenominatorType& denominator) { + switch (denominator) { case envoy::type::FractionalPercent::HUNDRED: return 100; case envoy::type::FractionalPercent::TEN_THOUSAND: diff --git a/source/common/protobuf/utility.h b/source/common/protobuf/utility.h index b615010f3c00..f34975f90e14 100644 --- a/source/common/protobuf/utility.h +++ b/source/common/protobuf/utility.h @@ -68,10 +68,11 @@ uint64_t convertPercent(double percent, uint64_t max_value); /** * Convert a fractional percent denominator enum into an integer. - * @param percent supplies percent to convert. + * @param denominator supplies denominator to convert. * @return the converted denominator. */ -uint64_t fractionalPercentDenominatorToInt(const envoy::type::FractionalPercent& percent); +uint64_t fractionalPercentDenominatorToInt( + const envoy::type::FractionalPercent::DenominatorType& denominator); } // namespace ProtobufPercentHelper } // namespace Envoy diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index 8324948be470..035c08f67cd1 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -11,6 +11,7 @@ #include "envoy/http/header_map.h" #include "envoy/runtime/runtime.h" +#include "envoy/type/percent.pb.validate.h" #include "envoy/upstream/cluster_manager.h" #include "envoy/upstream/upstream.h" @@ -18,6 +19,7 @@ #include "common/common/empty_string.h" #include "common/common/fmt.h" #include "common/common/hash.h" +#include "common/common/logger.h" #include "common/common/utility.h" #include "common/config/metadata.h" #include "common/config/rds_json.h" @@ -26,6 +28,7 @@ #include "common/http/headers.h" #include "common/http/utility.h" #include "common/http/websocket/ws_handler_impl.h" +#include "common/protobuf/protobuf.h" #include "common/protobuf/utility.h" #include "common/router/retry_state_impl.h" @@ -258,7 +261,7 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost, timeout_(PROTOBUF_GET_MS_OR_DEFAULT(route.route(), timeout, DEFAULT_ROUTE_TIMEOUT_MS)), idle_timeout_(PROTOBUF_GET_OPTIONAL_MS(route.route(), idle_timeout)), max_grpc_timeout_(PROTOBUF_GET_OPTIONAL_MS(route.route(), max_grpc_timeout)), - runtime_(loadRuntimeData(route.match())), loader_(factory_context.runtime()), + loader_(factory_context.runtime()), runtime_(loadRuntimeData(route.match())), host_redirect_(route.redirect().host_redirect()), path_redirect_(route.redirect().path_redirect()), https_redirect_(route.redirect().https_redirect()), @@ -346,8 +349,11 @@ bool RouteEntryImplBase::matchRoute(const Http::HeaderMap& headers, uint64_t ran bool matches = true; if (runtime_) { - matches &= loader_.snapshot().featureEnabled(runtime_.value().key_, runtime_.value().default_, - random_value); + matches &= random_value % runtime_->denominator_val_ < runtime_->numerator_val_; + if (!matches) { + // No need to waste further cycles calculating a route match. + return false; + } } if (match_grpc_) { @@ -409,14 +415,36 @@ void RouteEntryImplBase::finalizeResponseHeaders( absl::optional RouteEntryImplBase::loadRuntimeData(const envoy::api::v2::route::RouteMatch& route_match) { absl::optional runtime; - if (route_match.has_runtime()) { - RuntimeData data; - data.key_ = route_match.runtime().runtime_key(); - data.default_ = route_match.runtime().default_value(); - runtime = data; + RuntimeData runtime_data; + + if (route_match.runtime_specifier_case() == envoy::api::v2::route::RouteMatch::kRuntimeFraction) { + envoy::type::FractionalPercent fractional_percent; + const std::string& fraction_yaml = + loader_.snapshot().get(route_match.runtime_fraction().runtime_key()); + + try { + MessageUtil::loadFromYamlAndValidate(fraction_yaml, fractional_percent); + } catch (const EnvoyException& ex) { + ENVOY_LOG(error, "failed to parse string value for runtime key {}: {}", + route_match.runtime_fraction().runtime_key(), ex.what()); + fractional_percent = route_match.runtime_fraction().default_value(); + } + + runtime_data.numerator_val_ = fractional_percent.numerator(); + runtime_data.denominator_val_ = + ProtobufPercentHelper::fractionalPercentDenominatorToInt(fractional_percent.denominator()); + } else if (route_match.runtime_specifier_case() == envoy::api::v2::route::RouteMatch::kRuntime) { + // For backwards compatibility, the deprecated 'runtime' field must be converted to a + // RuntimeData format with a variable denominator type. The 'runtime' field assumes a percentage + // (0-100), so the hard-coded denominator value reflects this. + runtime_data.denominator_val_ = 100; + runtime_data.numerator_val_ = loader_.snapshot().getInteger( + route_match.runtime().runtime_key(), route_match.runtime().default_value()); + } else { + return runtime; } - return runtime; + return runtime_data; } void RouteEntryImplBase::finalizePathHeader(Http::HeaderMap& headers, diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index ce17b763ca01..c8a800553fa9 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -291,7 +291,8 @@ class RouteEntryImplBase : public RouteEntry, public DirectResponseEntry, public Route, public PathMatchCriterion, - public std::enable_shared_from_this { + public std::enable_shared_from_this, + Logger::Loggable { public: /** * @throw EnvoyException with reason if the route configuration contains any errors @@ -386,8 +387,8 @@ class RouteEntryImplBase : public RouteEntry, private: struct RuntimeData { - std::string key_{}; - uint64_t default_{}; + uint64_t numerator_val_{}; + uint64_t denominator_val_{}; }; class DynamicRouteEntry : public RouteEntry, public Route { @@ -518,8 +519,7 @@ class RouteEntryImplBase : public RouteEntry, typedef std::shared_ptr WeightedClusterEntrySharedPtr; - static absl::optional - loadRuntimeData(const envoy::api::v2::route::RouteMatch& route); + absl::optional loadRuntimeData(const envoy::api::v2::route::RouteMatch& route); static std::multimap parseOpaqueConfig(const envoy::api::v2::route::Route& route); @@ -540,8 +540,8 @@ class RouteEntryImplBase : public RouteEntry, const std::chrono::milliseconds timeout_; const absl::optional idle_timeout_; const absl::optional max_grpc_timeout_; - const absl::optional runtime_; Runtime::Loader& loader_; + const absl::optional runtime_; const std::string host_redirect_; const std::string path_redirect_; const bool https_redirect_; diff --git a/source/extensions/filters/http/fault/fault_filter.cc b/source/extensions/filters/http/fault/fault_filter.cc index 530156589561..1d9a6fe87e42 100644 --- a/source/extensions/filters/http/fault/fault_filter.cc +++ b/source/extensions/filters/http/fault/fault_filter.cc @@ -134,13 +134,14 @@ bool FaultFilter::isDelayEnabled() { bool enabled = config_->runtime().snapshot().featureEnabled( DELAY_PERCENT_KEY, fault_settings_->delayPercentage().numerator(), config_->randomGenerator().random(), - ProtobufPercentHelper::fractionalPercentDenominatorToInt(fault_settings_->delayPercentage())); + ProtobufPercentHelper::fractionalPercentDenominatorToInt( + fault_settings_->delayPercentage().denominator())); if (!downstream_cluster_delay_percent_key_.empty()) { enabled |= config_->runtime().snapshot().featureEnabled( downstream_cluster_delay_percent_key_, fault_settings_->delayPercentage().numerator(), config_->randomGenerator().random(), ProtobufPercentHelper::fractionalPercentDenominatorToInt( - fault_settings_->delayPercentage())); + fault_settings_->delayPercentage().denominator())); } return enabled; } @@ -149,13 +150,14 @@ bool FaultFilter::isAbortEnabled() { bool enabled = config_->runtime().snapshot().featureEnabled( ABORT_PERCENT_KEY, fault_settings_->abortPercentage().numerator(), config_->randomGenerator().random(), - ProtobufPercentHelper::fractionalPercentDenominatorToInt(fault_settings_->abortPercentage())); + ProtobufPercentHelper::fractionalPercentDenominatorToInt( + fault_settings_->abortPercentage().denominator())); if (!downstream_cluster_abort_percent_key_.empty()) { enabled |= config_->runtime().snapshot().featureEnabled( downstream_cluster_abort_percent_key_, fault_settings_->abortPercentage().numerator(), config_->randomGenerator().random(), ProtobufPercentHelper::fractionalPercentDenominatorToInt( - fault_settings_->abortPercentage())); + fault_settings_->abortPercentage().denominator())); } return enabled; } diff --git a/source/extensions/filters/network/mongo_proxy/proxy.cc b/source/extensions/filters/network/mongo_proxy/proxy.cc index 4b72b96a0e62..3c01ced80cf4 100644 --- a/source/extensions/filters/network/mongo_proxy/proxy.cc +++ b/source/extensions/filters/network/mongo_proxy/proxy.cc @@ -326,7 +326,7 @@ absl::optional ProxyFilter::delayDuration() { fault_config_->delayPercentage().numerator(), generator_.random(), ProtobufPercentHelper::fractionalPercentDenominatorToInt( - fault_config_->delayPercentage()))) { + fault_config_->delayPercentage().denominator()))) { return result; } diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index 0c64875cb929..919833172dd5 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -1970,16 +1970,104 @@ TEST(RouteMatcherTest, Runtime) { Runtime::MockSnapshot snapshot; ON_CALL(factory_context.runtime_loader_, snapshot()).WillByDefault(ReturnRef(snapshot)); + EXPECT_CALL(snapshot, getInteger("some_key", 50)) + .Times(testing::AtLeast(1)) + .WillRepeatedly(Return(42)); TestConfigImpl config(parseRouteConfigurationFromJson(json), factory_context, true); - EXPECT_CALL(snapshot, featureEnabled("some_key", 50, 10)).WillOnce(Return(true)); EXPECT_EQ("something_else", - config.route(genHeaders("www.lyft.com", "/", "GET"), 10)->routeEntry()->clusterName()); + config.route(genHeaders("www.lyft.com", "/", "GET"), 41)->routeEntry()->clusterName()); - EXPECT_CALL(snapshot, featureEnabled("some_key", 50, 20)).WillOnce(Return(false)); EXPECT_EQ("www2", - config.route(genHeaders("www.lyft.com", "/", "GET"), 20)->routeEntry()->clusterName()); + config.route(genHeaders("www.lyft.com", "/", "GET"), 43)->routeEntry()->clusterName()); +} + +TEST(RouteMatcherTest, FractionalRuntime) { + std::string yaml = R"EOF( +virtual_hosts: + - name: "www2" + domains: ["www.lyft.com"] + routes: + - match: + prefix: "/" + runtime_fraction: + default_value: + numerator: 50 + denominator: MILLION + runtime_key: "bogus_key" + route: + cluster: "something_else" + - match: + prefix: "/" + route: + cluster: "www2" + )EOF"; + + NiceMock factory_context; + Runtime::MockSnapshot snapshot; + ON_CALL(factory_context.runtime_loader_, snapshot()).WillByDefault(ReturnRef(snapshot)); + + const std::string runtime_fraction = R"EOF( + numerator: 42 + denominator: HUNDRED + )EOF"; + EXPECT_CALL(snapshot, get("bogus_key")) + .Times(testing::AtLeast(1)) + .WillRepeatedly(ReturnRef(runtime_fraction)); + + TestConfigImpl config(parseRouteConfigurationFromV2Yaml(yaml), factory_context, false); + + EXPECT_EQ( + "something_else", + config.route(genHeaders("www.lyft.com", "/foo", "GET"), 41)->routeEntry()->clusterName()); + + EXPECT_EQ( + "www2", + config.route(genHeaders("www.lyft.com", "/foo", "GET"), 43)->routeEntry()->clusterName()); +} + +TEST(RouteMatcherTest, FractionalRuntimeDefault) { + std::string yaml = R"EOF( +virtual_hosts: + - name: "www2" + domains: ["www.lyft.com"] + routes: + - match: + prefix: "/" + runtime_fraction: + default_value: + numerator: 50 + denominator: MILLION + runtime_key: "bogus_key" + route: + cluster: "something_else" + - match: + prefix: "/" + route: + cluster: "www2" + )EOF"; + + NiceMock factory_context; + Runtime::MockSnapshot snapshot; + ON_CALL(factory_context.runtime_loader_, snapshot()).WillByDefault(ReturnRef(snapshot)); + + const std::string runtime_fraction = R"EOF( + this string is nonsense and should fail parsing + )EOF"; + EXPECT_CALL(snapshot, get("bogus_key")) + .Times(testing::AtLeast(1)) + .WillRepeatedly(ReturnRef(runtime_fraction)); + + TestConfigImpl config(parseRouteConfigurationFromV2Yaml(yaml), factory_context, false); + + EXPECT_EQ( + "something_else", + config.route(genHeaders("www.lyft.com", "/foo", "GET"), 49)->routeEntry()->clusterName()); + + EXPECT_EQ( + "www2", + config.route(genHeaders("www.lyft.com", "/foo", "GET"), 51)->routeEntry()->clusterName()); } TEST(RouteMatcherTest, ShadowClusterNotFound) {