Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

route match: Add runtime_fraction field for more granular routing #4217

Merged
merged 15 commits into from
Sep 19, 2018
2 changes: 2 additions & 0 deletions api/envoy/api/v2/route/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ api_proto_library_internal(
deps = [
"//envoy/api/v2/core:base",
"//envoy/type:range",
"//envoy/type:runtime_percent",
],
)

Expand All @@ -18,5 +19,6 @@ api_go_proto_library(
deps = [
"//envoy/api/v2/core:base_go_proto",
"//envoy/type:range_go_proto",
"//envoy/type:runtime_percent",
],
)
20 changes: 19 additions & 1 deletion api/envoy/api/v2/route/route.proto
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ option java_generic_services = true;

import "envoy/api/v2/core/base.proto";
import "envoy/type/range.proto";
import "envoy/type/runtime_percent.proto";

import "google/protobuf/duration.proto";
import "google/protobuf/struct.proto";
Expand Down Expand Up @@ -283,7 +284,14 @@ message RouteMatch {
// gradual manner without full code/config deploys. Refer to the
// :ref:`traffic shifting <config_http_conn_man_route_table_traffic_splitting_shift>` docs
// for additional documentation.
core.RuntimeUInt32 runtime = 5;
//
// .. attention::
//
// **This field is deprecated**. Set the
// :ref:`runtime_fraction<envoy_api_field_route.RouteMatch.runtime_fraction>` field instead.
// This field is ignored if
// :ref:`runtime_fraction<envoy_api_field_route.RouteMatch.runtime_fraction>` is set.
core.RuntimeUInt32 runtime = 5 [deprecated = true];

// 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
Expand All @@ -298,6 +306,16 @@ message RouteMatch {
// query parameters is nonzero, they all must match the *path* header's
// query string for a match to occur.
repeated QueryParameterMatcher query_parameters = 7;

// 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
// <config_http_conn_man_route_table_traffic_splitting_shift>` docs for additional documentation.
envoy.type.RuntimeFractionalPercent runtime_fraction = 8;
tonya11en marked this conversation as resolved.
Show resolved Hide resolved
}

// [#comment:next free field: 9]
Expand Down
12 changes: 12 additions & 0 deletions api/envoy/type/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,18 @@ api_go_proto_library(
proto = ":http_status",
)

api_proto_library_internal(
name = "runtime_percent",
srcs = ["runtime_percent.proto"],
visibility = ["//visibility:public"],
deps = [":percent"],
)

api_go_proto_library(
name = "runtime_percent",
proto = ":runtime_percent",
)

api_proto_library_internal(
name = "percent",
srcs = ["percent.proto"],
Expand Down
27 changes: 27 additions & 0 deletions api/envoy/type/runtime_percent.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
syntax = "proto3";

package envoy.type;

import "validate/validate.proto";
import "gogoproto/gogo.proto";

import "envoy/type/percent.proto";

option (gogoproto.equal_all) = true;

// [#protodoc-title: RuntimePercent]

// Runtime derived FractionalPercent with defaults for when the numerator or denominator is not
// specified via a runtime key.
message RuntimeFractionalPercent {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be in api/envoy/api/v2/core/base.proto so as to be next to RuntimeUint32?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't do that because I didn't want to introduce a dependency on percent.proto. It was originally in percent.proto, but an earlier comment from @danielhochman suggested I move it out into its own file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fine for base to depend on percent.proto (just not the other way around).

// 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 to get numerator value for comparison. This value is used if defined.
string numerator_key = 2;

// Runtime key to get denominator type for comparison. This value is used if defined. Valid field
// value strings must match valid :ref:`denominator
// types<envoy_api_enum_type.FractionalPercent.DenominatorType>`.
string denominator_key = 3;
}
1 change: 1 addition & 0 deletions docs/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ PROTO_RST="
/envoy/service/auth/v2alpha/external_auth/envoy/service/auth/v2alpha/external_auth.proto.rst
/envoy/type/http_status/envoy/type/http_status.proto.rst
/envoy/type/percent/envoy/type/percent.proto.rst
/envoy/type/runtime_percent/envoy/type/runtime_percent.proto.rst
/envoy/type/range/envoy/type/range.proto.rst
/envoy/type/matcher/metadata/envoy/type/matcher/metadata.proto.rst
/envoy/type/matcher/value/envoy/type/matcher/value.proto.rst
Expand Down
1 change: 1 addition & 0 deletions docs/root/api-v2/types/types.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Types

../type/http_status.proto
../type/percent.proto
../type/runtime_percent.proto
../type/range.proto
../type/matcher/metadata.proto
../type/matcher/number.proto
Expand Down
7 changes: 4 additions & 3 deletions source/common/access_log/access_log_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<
Expand Down
5 changes: 3 additions & 2 deletions source/common/protobuf/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
5 changes: 3 additions & 2 deletions source/common/protobuf/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
38 changes: 29 additions & 9 deletions source/common/router/config_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,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()),
Expand Down Expand Up @@ -345,8 +345,9 @@ 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 &= loader_.snapshot().featureEnabled(runtime_.value().numerator_key_,
runtime_.value().numerator_default_, random_value,
runtime_.value().denominator_val_);
}

matches &= Http::HeaderUtility::matchHeaders(headers, config_headers_);
Expand Down Expand Up @@ -404,14 +405,33 @@ void RouteEntryImplBase::finalizeResponseHeaders(
absl::optional<RouteEntryImplBase::RuntimeData>
RouteEntryImplBase::loadRuntimeData(const envoy::api::v2::route::RouteMatch& route_match) {
absl::optional<RuntimeData> 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 data;

if (route_match.has_runtime_fraction()) {
data.numerator_key_ = route_match.runtime_fraction().numerator_key();
data.numerator_default_ = route_match.runtime_fraction().default_value().numerator();

const std::string& denominator_key = route_match.runtime_fraction().denominator_key();
const std::string& denominator_data = loader_.snapshot().get(denominator_key);
using FractionalPercent = envoy::type::FractionalPercent;
FractionalPercent::DenominatorType denominator_type;
if (!FractionalPercent::DenominatorType_Parse(denominator_data, &denominator_type)) {
denominator_type = route_match.runtime_fraction().default_value().denominator();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I read this right, failure to convert the denominator key into a valid denominator value causes us to use the numerator key's value with the default value's denominator. That seems like it might cause some pretty unexpected behavior. Is it reasonable to just switch using the default value altogether in this case?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what you propose is the sane thing to do. I'll change the logic.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually- scratch that. A user would have unexpected behavior either way if they misspell a denominator type and it switches to a default value. I'm not sure one way is better than the other. @htuch what do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what the best practice to alert when runtime is wrong; how do folks operationally determine when a runtime change has taken effect? Via /runtime?

One thought that occurred to me just now is that instead of having separate values for numerator/denominator, we have a single values, which is a textual representation of a FractionalPercent proto. That way, we can just use PGV validation on this to check sanity. It also solves an update atomicity issue.

data.denominator_val_ =
ProtobufPercentHelper::fractionalPercentDenominatorToInt(std::move(denominator_type));
tonya11en marked this conversation as resolved.
Show resolved Hide resolved
} else if (route_match.has_runtime()) {
// For backwards compatibility, the deprecated 'runtime' field must be converted to a
// RuntimeData format with a variable denominator type. This field assumes a percentage (0-100),
// so the hard-coded denominator value reflects this.
data.numerator_key_ = route_match.runtime().runtime_key();
data.numerator_default_ = route_match.runtime().default_value();
data.denominator_val_ = 100;
} else {
return runtime;
}

return runtime;
return data;
}

void RouteEntryImplBase::finalizePathHeader(Http::HeaderMap& headers,
Expand Down
10 changes: 5 additions & 5 deletions source/common/router/config_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -362,8 +362,9 @@ class RouteEntryImplBase : public RouteEntry,

private:
struct RuntimeData {
std::string key_{};
uint64_t default_{};
std::string numerator_key_{};
uint64_t numerator_default_{};
uint64_t denominator_val_{};
};

class DynamicRouteEntry : public RouteEntry, public Route {
Expand Down Expand Up @@ -494,8 +495,7 @@ class RouteEntryImplBase : public RouteEntry,

typedef std::shared_ptr<WeightedClusterEntry> WeightedClusterEntrySharedPtr;

static absl::optional<RuntimeData>
loadRuntimeData(const envoy::api::v2::route::RouteMatch& route);
absl::optional<RuntimeData> loadRuntimeData(const envoy::api::v2::route::RouteMatch& route);

static std::multimap<std::string, std::string>
parseOpaqueConfig(const envoy::api::v2::route::Route& route);
Expand All @@ -516,8 +516,8 @@ class RouteEntryImplBase : public RouteEntry,
const std::chrono::milliseconds timeout_;
const absl::optional<std::chrono::milliseconds> idle_timeout_;
const absl::optional<std::chrono::milliseconds> max_grpc_timeout_;
const absl::optional<RuntimeData> runtime_;
Runtime::Loader& loader_;
const absl::optional<RuntimeData> runtime_;
const std::string host_redirect_;
const std::string path_redirect_;
const bool https_redirect_;
Expand Down
10 changes: 6 additions & 4 deletions source/extensions/filters/http/fault/fault_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion source/extensions/filters/network/mongo_proxy/proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ absl::optional<uint64_t> ProxyFilter::delayDuration() {
fault_config_->delayPercentage().numerator(),
generator_.random(),
ProtobufPercentHelper::fractionalPercentDenominatorToInt(
fault_config_->delayPercentage()))) {
fault_config_->delayPercentage().denominator()))) {
return result;
}

Expand Down
Loading