fault: use FractionalPercent for percent

DEPRECATED.md

@@ -23,6 +23,9 @@ A logged warning is expected for each deprecated item that is in deprecation win
   is deprecated. Please use the new `upgrade_configs` in the
   [HttpConnectionManager](https://github.com/envoyproxy/envoy/blob/master/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto)
   instead.
+* Use of the integer `percent` field in [FaultDelay](https://github.com/envoyproxy/envoy/blob/master/api/envoy/config/filter/fault/v2/fault.proto)
+  and in [FaultAbort](https://github.com/envoyproxy/envoy/blob/master/api/envoy/config/filter/http/fault/v2/fault.proto) is deprecated in favor
+  of the new `FractionalPercent` based `percentage` field.
 * Setting hosts via `hosts` field in `Cluster` is deprecated. Use `load_assignment` instead.
 
 ## Version 1.7.0

api/envoy/config/filter/fault/v2/BUILD

@@ -9,4 +9,5 @@ api_proto_library_internal(
         "//envoy/config/filter/http/fault/v2:__pkg__",
         "//envoy/config/filter/network/mongo_proxy/v2:__pkg__",
     ],
+    deps = ["//envoy/type:percent"],
 )

api/envoy/config/filter/fault/v2/fault.proto

@@ -3,6 +3,8 @@ syntax = "proto3";
 package envoy.config.filter.fault.v2;
 option go_package = "v2";
 
+import "envoy/type/percent.proto";
+
 import "google/protobuf/duration.proto";
 
 import "validate/validate.proto";
@@ -24,7 +26,11 @@ message FaultDelay {
 
   // An integer between 0-100 indicating the percentage of operations/connection requests
   // on which the delay will be injected.
-  uint32 percent = 2 [(validate.rules).uint32.lte = 100];
+  //
+  // .. attention::
+  //
+  //   Use of integer `percent` value is deprecated. Use fractional `percentage` field instead.
+  uint32 percent = 2 [(validate.rules).uint32.lte = 100, deprecated = true];
 
   oneof fault_delay_secifier {
     option (validate.required) = true;
@@ -37,4 +43,7 @@ message FaultDelay {
     google.protobuf.Duration fixed_delay = 3
         [(validate.rules).duration.gt = {}, (gogoproto.stdduration) = true];
   }
+
+  // The percentage of operations/connection requests on which the delay will be injected.
+  envoy.type.FractionalPercent percentage = 4;
 }

api/envoy/config/filter/http/fault/v2/BUILD

@@ -8,5 +8,6 @@ api_proto_library_internal(
     deps = [
         "//envoy/api/v2/route",
         "//envoy/config/filter/fault/v2:fault",
+        "//envoy/type:percent",
     ],
 )

api/envoy/config/filter/http/fault/v2/fault.proto

@@ -3,8 +3,9 @@ syntax = "proto3";
 package envoy.config.filter.http.fault.v2;
 option go_package = "v2";
 
-import "envoy/config/filter/fault/v2/fault.proto";
 import "envoy/api/v2/route/route.proto";
+import "envoy/config/filter/fault/v2/fault.proto";
+import "envoy/type/percent.proto";
 
 import "validate/validate.proto";
 
@@ -14,14 +15,22 @@ import "validate/validate.proto";
 message FaultAbort {
   // An integer between 0-100 indicating the percentage of requests/operations/connections
   // that will be aborted with the error code provided.
-  uint32 percent = 1 [(validate.rules).uint32.lte = 100];
+  //
+  // .. attention::
+  //
+  //   Use of integer `percent` value is deprecated. Use fractional `percentage` field instead.
+  uint32 percent = 1 [(validate.rules).uint32.lte = 100, deprecated = true];
 
   oneof error_type {
     option (validate.required) = true;
 
     // HTTP status code to use to abort the HTTP request.
     uint32 http_status = 2 [(validate.rules).uint32 = {gte: 200, lt: 600}];
   }
+
+  // The percentage of requests/operations/connections that will be aborted with the error code
+  // provided.
+  envoy.type.FractionalPercent percentage = 3;
 }
 
 message HTTPFault {

docs/root/intro/version_history.rst

@@ -11,6 +11,8 @@ Version history
   `google.api.HttpBody <https://github.com/googleapis/googleapis/blob/master/google/api/httpbody.proto>`_.
 * config: v1 disabled by default. v1 support remains available until October via flipping --v2-config-only=false.
 * config: v1 disabled by default. v1 support remains available until October via setting :option:`--allow-deprecated-v1-api`.
+* fault: added support for fractional percentages in :ref:`FaultDelay <envoy_api_field_config.filter.fault.v2.FaultDelay.percentage>`
+  and in :ref:`FaultAbort <envoy_api_field_config.filter.http.fault.v2.FaultAbort.percentage>`.
 * health check: added support for :ref:`custom health check <envoy_api_field_core.HealthCheck.custom_health_check>`.
 * health check: added support for :ref:`specifying jitter as a percentage <envoy_api_field_core.HealthCheck.interval_jitter_percent>`.
 * health_check: added support for :ref:`health check event logging <arch_overview_health_check_logging>`.

include/envoy/runtime/runtime.h

@@ -90,17 +90,17 @@ class Snapshot {
   virtual bool featureEnabled(const std::string& key, uint64_t default_value) const PURE;
 
   /**
-   * Test if a feature is enabled using a supplied stable random value. This variant is used if
-   * the caller wants a stable result over multiple calls.
+   * Test if a feature is enabled using the built in random generator and total number of buckets
+   * for sampling.
    * @param key supplies the feature key to lookup.
    * @param default_value supplies the default value that will be used if either the feature key
    *        does not exist or it is not an integer.
-   * @param random_value supplies the stable random value to use for determining whether the feature
-   *        is enabled.
+   * @param num_buckets control max number of buckets for sampling. Sampled value will be in a range
+   *        of [0, num_buckets).
    * @return true if the feature is enabled.
    */
   virtual bool featureEnabled(const std::string& key, uint64_t default_value,
-                              uint64_t random_value) const PURE;
+                              uint64_t num_buckets) const PURE;
 
   /**
    * Test if a feature is enabled using a supplied stable random value and total number of buckets
@@ -112,8 +112,8 @@ class Snapshot {
    *        does not exist or it is not an integer.
    * @param random_value supplies the stable random value to use for determining whether the feature
    *        is enabled.
-   * @param control max number of buckets for sampling. Sampled value will be in a range of
-   *        [0, num_buckets).
+   * @param num_buckets control max number of buckets for sampling. Sampled value will be in a range
+   *        of [0, num_buckets).
    * @return true if the feature is enabled.
    */
   virtual bool featureEnabled(const std::string& key, uint64_t default_value, uint64_t random_value,

source/common/http/conn_manager_utility.cc

@@ -212,7 +212,7 @@ void ConnectionManagerUtility::mutateTracingRequestHeader(Http::HeaderMap& reque
   }
 
   if (!runtime.snapshot().featureEnabled("tracing.global_enabled",
-                                         config.tracingConfig()->overall_sampling_, result)) {
+                                         config.tracingConfig()->overall_sampling_, result, 100)) {
     UuidUtils::setTraceableUuid(x_request_id, UuidTraceStatus::NoTrace);
   }
 

source/common/protobuf/utility.h

@@ -48,6 +48,16 @@
   ((message).has_##field_name() ? DurationUtil::durationToSeconds((message).field_name())          \
                                 : throw MissingFieldException(#field_name, (message)))
 
+// Set the value of a FractionalPercent field with the value from a protobuf message if present.
+// Otherwise, convert the default field value into FractionalPercent and set it.
+#define PROTOBUF_SET_FRACTIONAL_PERCENT_OR_DEFAULT(field, message, field_name, default_field_name) \
+  if ((message).has_##field_name()) {                                                              \
+    field = (message).field_name();                                                                \
+  } else {                                                                                         \
+    field.set_numerator((message).default_field_name());                                           \
+    field.set_denominator(envoy::type::FractionalPercent::HUNDRED);                                \
+  }
+
 namespace Envoy {
 namespace ProtobufPercentHelper {
 

source/common/router/config_impl.cc

@@ -342,7 +342,7 @@ bool RouteEntryImplBase::matchRoute(const Http::HeaderMap& headers, uint64_t ran
 
   if (runtime_) {
     matches &= loader_.snapshot().featureEnabled(runtime_.value().key_, runtime_.value().default_,
-                                                 random_value);
+                                                 random_value, 100);
   }
 
   matches &= Http::HeaderUtility::matchHeaders(headers, config_headers_);

source/common/runtime/runtime_impl.cc

@@ -146,6 +146,11 @@ std::string RandomGeneratorImpl::uuid() {
   return std::string(uuid, UUID_LENGTH);
 }
 
+bool SnapshotImpl::featureEnabled(const std::string& key, uint64_t default_value,
+                                  uint64_t num_buckets) const {
+  return featureEnabled(key, default_value, generator_.random(), num_buckets);
+}
+
 bool SnapshotImpl::featureEnabled(const std::string& key, uint64_t default_value,
                                   uint64_t random_value, uint64_t num_buckets) const {
   return random_value % num_buckets < std::min(getInteger(key, default_value), num_buckets);
@@ -163,11 +168,6 @@ bool SnapshotImpl::featureEnabled(const std::string& key, uint64_t default_value
   }
 }
 
-bool SnapshotImpl::featureEnabled(const std::string& key, uint64_t default_value,
-                                  uint64_t random_value) const {
-  return featureEnabled(key, default_value, random_value, 100);
-}
-
 const std::string& SnapshotImpl::get(const std::string& key) const {
   auto entry = values_.find(key);
   if (entry == values_.end()) {

source/common/runtime/runtime_impl.h

@@ -64,11 +64,11 @@ class SnapshotImpl : public Snapshot, public ThreadLocal::ThreadLocalObject {
                std::vector<OverrideLayerConstPtr>&& layers);
 
   // Runtime::Snapshot
+  bool featureEnabled(const std::string& key, uint64_t default_value,
+                      uint64_t num_buckets) const override;
   bool featureEnabled(const std::string& key, uint64_t default_value, uint64_t random_value,
                       uint64_t num_buckets) const override;
   bool featureEnabled(const std::string& key, uint64_t default_value) const override;
-  bool featureEnabled(const std::string& key, uint64_t default_value,
-                      uint64_t random_value) const override;
   const std::string& get(const std::string& key) const override;
   uint64_t getInteger(const std::string& key, uint64_t default_value) const override;
   const std::vector<OverrideLayerConstPtr>& getLayers() const override;

source/extensions/filters/http/fault/fault_filter.cc

@@ -34,12 +34,14 @@ const std::string FaultFilter::ABORT_HTTP_STATUS_KEY = "fault.http.abort.http_st
 FaultSettings::FaultSettings(const envoy::config::filter::http::fault::v2::HTTPFault& fault) {
 
   if (fault.has_abort()) {
-    abort_percent_ = fault.abort().percent();
+    PROTOBUF_SET_FRACTIONAL_PERCENT_OR_DEFAULT(abort_percentage_, fault.abort(), percentage,
+                                               percent);
     http_status_ = fault.abort().http_status();
   }
 
   if (fault.has_delay()) {
-    fixed_delay_percent_ = fault.delay().percent();
+    PROTOBUF_SET_FRACTIONAL_PERCENT_OR_DEFAULT(fixed_delay_percentage_, fault.delay(), percentage,
+                                               percent);
     const auto& delay = fault.delay();
     fixed_duration_ms_ = PROTOBUF_GET_MS_OR_DEFAULT(delay, fixed_delay, 0);
   }
@@ -129,26 +131,28 @@ Http::FilterHeadersStatus FaultFilter::decodeHeaders(Http::HeaderMap& headers, b
 }
 
 bool FaultFilter::isDelayEnabled() {
-  bool enabled = config_->runtime().snapshot().featureEnabled(DELAY_PERCENT_KEY,
-                                                              fault_settings_->delayPercent());
-
+  bool enabled = config_->runtime().snapshot().featureEnabled(
+      DELAY_PERCENT_KEY, fault_settings_->delayPercentage().numerator(),
+      ProtobufPercentHelper::fractionalPercentDenominatorToInt(fault_settings_->delayPercentage()));
   if (!downstream_cluster_delay_percent_key_.empty()) {
-    enabled |= config_->runtime().snapshot().featureEnabled(downstream_cluster_delay_percent_key_,
-                                                            fault_settings_->delayPercent());
+    enabled |= config_->runtime().snapshot().featureEnabled(
+        downstream_cluster_delay_percent_key_, fault_settings_->delayPercentage().numerator(),
+        ProtobufPercentHelper::fractionalPercentDenominatorToInt(
+            fault_settings_->delayPercentage()));
   }
-
   return enabled;
 }
 
 bool FaultFilter::isAbortEnabled() {
-  bool enabled = config_->runtime().snapshot().featureEnabled(ABORT_PERCENT_KEY,
-                                                              fault_settings_->abortPercent());
-
+  bool enabled = config_->runtime().snapshot().featureEnabled(
+      ABORT_PERCENT_KEY, fault_settings_->abortPercentage().numerator(),
+      ProtobufPercentHelper::fractionalPercentDenominatorToInt(fault_settings_->abortPercentage()));
   if (!downstream_cluster_abort_percent_key_.empty()) {
-    enabled |= config_->runtime().snapshot().featureEnabled(downstream_cluster_abort_percent_key_,
-                                                            fault_settings_->abortPercent());
+    enabled |= config_->runtime().snapshot().featureEnabled(
+        downstream_cluster_abort_percent_key_, fault_settings_->abortPercentage().numerator(),
+        ProtobufPercentHelper::fractionalPercentDenominatorToInt(
+            fault_settings_->abortPercentage()));
   }
-
   return enabled;
 }
 

source/extensions/filters/http/fault/fault_filter.h

@@ -45,19 +45,19 @@ class FaultSettings : public Router::RouteSpecificFilterConfig {
   const std::vector<Http::HeaderUtility::HeaderData>& filterHeaders() const {
     return fault_filter_headers_;
   }
-  uint64_t abortPercent() const { return abort_percent_; }
-  uint64_t delayPercent() const { return fixed_delay_percent_; }
+  envoy::type::FractionalPercent abortPercentage() const { return abort_percentage_; }
+  envoy::type::FractionalPercent delayPercentage() const { return fixed_delay_percentage_; }
   uint64_t delayDuration() const { return fixed_duration_ms_; }
   uint64_t abortCode() const { return http_status_; }
   const std::string& upstreamCluster() const { return upstream_cluster_; }
   const std::unordered_set<std::string>& downstreamNodes() const { return downstream_nodes_; }
 
 private:
-  uint64_t abort_percent_{};       // 0-100
-  uint64_t http_status_{};         // HTTP or gRPC return codes
-  uint64_t fixed_delay_percent_{}; // 0-100
-  uint64_t fixed_duration_ms_{};   // in milliseconds
-  std::string upstream_cluster_;   // restrict faults to specific upstream cluster
+  envoy::type::FractionalPercent abort_percentage_;
+  uint64_t http_status_{}; // HTTP or gRPC return codes
+  envoy::type::FractionalPercent fixed_delay_percentage_;
+  uint64_t fixed_duration_ms_{}; // in milliseconds
+  std::string upstream_cluster_; // restrict faults to specific upstream cluster
   std::vector<Http::HeaderUtility::HeaderData> fault_filter_headers_;
   std::unordered_set<std::string> downstream_nodes_{}; // Inject failures for specific downstream
 };

source/extensions/filters/network/mongo_proxy/proxy.cc

@@ -320,7 +320,9 @@ absl::optional<uint64_t> ProxyFilter::delayDuration() {
   }
 
   if (!runtime_.snapshot().featureEnabled(MongoRuntimeConfig::get().FixedDelayPercent,
-                                          fault_config_->delayPercent())) {
+                                          fault_config_->delayPercentage().numerator(),
+                                          ProtobufPercentHelper::fractionalPercentDenominatorToInt(
+                                              fault_config_->delayPercentage()))) {
     return result;
   }
 

source/extensions/filters/network/mongo_proxy/proxy.h

@@ -102,13 +102,15 @@ typedef std::shared_ptr<AccessLog> AccessLogSharedPtr;
 class FaultConfig {
 public:
   FaultConfig(const envoy::config::filter::fault::v2::FaultDelay& fault_config)
-      : delay_percent_(fault_config.percent()),
-        duration_ms_(PROTOBUF_GET_MS_REQUIRED(fault_config, fixed_delay)) {}
-  uint32_t delayPercent() const { return delay_percent_; }
+      : duration_ms_(PROTOBUF_GET_MS_REQUIRED(fault_config, fixed_delay)) {
+    PROTOBUF_SET_FRACTIONAL_PERCENT_OR_DEFAULT(delay_percentage_, fault_config, percentage,
+                                               percent);
+  }
+  envoy::type::FractionalPercent delayPercentage() const { return delay_percentage_; }
   uint64_t delayDuration() const { return duration_ms_; }
 
 private:
-  const uint32_t delay_percent_;
+  envoy::type::FractionalPercent delay_percentage_;
   const uint64_t duration_ms_;
 };
 

test/common/http/conn_manager_impl_test.cc

@@ -564,7 +564,7 @@ TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlow) {
   EXPECT_CALL(*span, setTag(":method", "GET"));
   // Verify if the activeSpan interface returns reference to the current span.
   EXPECT_CALL(*span, setTag("service-cluster", "scoobydoo"));
-  EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _))
+  EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _, 100))
       .WillOnce(Return(true));
   EXPECT_CALL(*span, setOperation(_)).Times(0);
 
@@ -629,7 +629,7 @@ TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowIngressDecorat
           Invoke([&](const Tracing::Span& applyToSpan) -> void { EXPECT_EQ(span, &applyToSpan); }));
   EXPECT_CALL(*span, finishSpan());
   EXPECT_CALL(*span, setTag(_, _)).Times(testing::AnyNumber());
-  EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _))
+  EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _, 100))
       .WillOnce(Return(true));
   EXPECT_CALL(*span, setOperation(_)).Times(0);
 
@@ -691,7 +691,7 @@ TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowIngressDecorat
           Invoke([&](const Tracing::Span& applyToSpan) -> void { EXPECT_EQ(span, &applyToSpan); }));
   EXPECT_CALL(*span, finishSpan());
   EXPECT_CALL(*span, setTag(_, _)).Times(testing::AnyNumber());
-  EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _))
+  EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _, 100))
       .WillOnce(Return(true));
   EXPECT_CALL(*span, setOperation("testOp"));
 
@@ -758,7 +758,7 @@ TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowEgressDecorato
           Invoke([&](const Tracing::Span& applyToSpan) -> void { EXPECT_EQ(span, &applyToSpan); }));
   EXPECT_CALL(*span, finishSpan());
   EXPECT_CALL(*span, setTag(_, _)).Times(testing::AnyNumber());
-  EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _))
+  EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _, 100))
       .WillOnce(Return(true));
   EXPECT_CALL(*span, setOperation(_)).Times(0);
 
@@ -825,7 +825,7 @@ TEST_F(HttpConnectionManagerImplTest, StartAndFinishSpanNormalFlowEgressDecorato
           Invoke([&](const Tracing::Span& applyToSpan) -> void { EXPECT_EQ(span, &applyToSpan); }));
   EXPECT_CALL(*span, finishSpan());
   EXPECT_CALL(*span, setTag(_, _)).Times(testing::AnyNumber());
-  EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _))
+  EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _, 100))
       .WillOnce(Return(true));
   // Verify that span operation overridden by value supplied in response header.
   EXPECT_CALL(*span, setOperation("testOp"));
@@ -871,7 +871,7 @@ TEST_F(HttpConnectionManagerImplTest,
   tracing_config_.reset(new TracingConnectionManagerConfig(
       {Tracing::OperationName::Egress, {LowerCaseString(":method")}, 100, 10000, 100}));
 
-  EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _))
+  EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _, 100))
       .WillOnce(Return(false));
   std::shared_ptr<MockStreamDecoderFilter> filter(new NiceMock<MockStreamDecoderFilter>());
 
@@ -1042,7 +1042,7 @@ TEST_F(HttpConnectionManagerImplTest, DoNotStartSpanIfTracingIsNotEnabled) {
   tracing_config_.reset();
 
   EXPECT_CALL(tracer_, startSpan_(_, _, _, _)).Times(0);
-  ON_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _))
+  ON_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _, 100))
       .WillByDefault(Return(true));
 
   std::shared_ptr<MockStreamDecoderFilter> filter(new NiceMock<MockStreamDecoderFilter>());