fault: use FractionalPercent for percent #3978

venilnoronha · 2018-07-28T03:27:59Z

Description:
The existing FaultDelay config used uint32 for the percent field which limited the user to configure percentages only in terms of whole numbers between 0 and 100. FractionalPercent allows finer control over the percent values by allowing fractions to be specified in the configuration, for example, 0.0001%.

Signed-off-by: Venil Noronha veniln@vmware.com

Risk Level: Med
Testing: Added 2 new tests and updated existing tests
Docs Changes: N/A
Release Notes: Added deprecation note and introduced FractionalPercent percentages.
Fixes #3904

/cc @rshriram

The existing FaultDelay config used uint32 for the percent field which limited the user to configure percentages only in terms of whole numbers between 0 and 100. FractionalPercent allows finer control over the percent values by allowing fractions to be specified in the configuration, for example 0.0001%. Signed-off-by: Venil Noronha <veniln@vmware.com>

mattklein123

Not a full review but a quick drive by to get started. Thank you!

mattklein123 · 2018-07-28T03:32:57Z

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

-  // on which the delay will be injected.
-  uint32 percent = 2 [(validate.rules).uint32.lte = 100];
+  // The percentage of operations/connection requests on which the delay will be injected.
+  envoy.type.FractionalPercent percent = 2;


We can't just change this. You will need to deprecate the old percent and add a new one.

call the new one as percentage. And i think there is also some protoc validation with it

mattklein123 · 2018-07-28T03:33:41Z

api/envoy/type/percent.proto

    // 10,000.
    //
    // **Example**: 1/10000 = 0.01%.
-    TEN_THOUSAND = 1;


These can't be changed. You will need to add new ones at the end. Are they really necessary though? Seems unnecessary to me with TEN_THOUSAND and MILLION. I would revert these changes personally.

Addressed in f105661.

mattklein123 · 2018-07-28T03:33:57Z

docs/root/api-v1/http_filters/fault_filter.rst

@@ -77,7 +77,10 @@ Delay

  {
    "type" : "...",
-    "fixed_delay_percent" : "...",
+    "fixed_delay_percent" : {


We don't accept v1 changes anymore. Please revert all changes to v1 code, schema, docs, etc.

venilnoronha · 2018-07-28T03:37:29Z

@mattklein123 thanks for the quick review! I'll address the issues you mentioned.

Signed-off-by: Venil Noronha <veniln@vmware.com>

mattklein123 · 2018-07-30T05:15:33Z

@rshriram do you mind doing first pass review on this?

rshriram · 2018-07-30T17:34:22Z

docs/root/api-v1/http_filters/fault_filter.rst

-  be delayed for the duration specified by *fixed_duration_ms*. Valid
-  values range from 0 to 100.
+  *(required, envoy.type.FractionalPercent)* The percentage of requests that will be delayed for
+  the duration specified by *fixed_duration_ms*.



Skip all changes to this file

rshriram · 2018-07-30T17:34:31Z

docs/root/api-v1/network_filters/mongo_proxy_filter.rst

-  *(required, integer)* Probability of an eligible MongoDB operation to be affected by the
-  injected fault when there is no active fault. Valid values are integers in a range of [0, 100].
+  *(required, envoy.type.FractionalPercent)* Probability of an eligible MongoDB operation to be
+  affected by the injected fault when there is no active fault.


and this as well

rshriram · 2018-07-30T17:35:52Z

include/envoy/runtime/runtime.h

-  virtual bool featureEnabled(const std::string& key, uint64_t default_value, uint64_t random_value,
-                              uint64_t num_buckets) const PURE;
+  virtual bool sampleFeatureEnabled(const std::string& key, uint64_t default_value,
+                                    uint64_t random_value, uint64_t num_buckets) const PURE;


@mattklein123 how are you guys doing fractional percent with runtimes? IOW, do you need runtime support for fractional percent? If not, all changes to this file can be eliminated

I think access_log_impl uses runtime for sampling the feature status as seen here. Should we just stick to a similar approach for faults?

Looking again, why is this being renamed ?

Because of a signature conflict between the routines highlighted here.

rshriram · 2018-07-30T17:39:22Z

source/common/json/config_schemas.cc

@@ -379,9 +379,20 @@ const std::string Json::Schema::MONGO_PROXY_NETWORK_FILTER_SCHEMA(R"EOF(
            "type" : "object",
            "properties" : {
              "percent" : {
-                "type" : "integer",
-                "minimum" : 0,
-                "maximum" : 100


rshriram

Thanks for doing this. Left comments inline.

* Revert v1 code, schema & doc changes * Deprecate percent field in FaultDelay * Introduce percentage field in FaultDelay of type FractionalPercent Signed-off-by: Venil Noronha <veniln@vmware.com>

rshriram

You need to do this for both delay and aborts.

Follow the convert old to new approach instead of a if old_set do x or if new_set do y.

rshriram · 2018-08-01T02:09:05Z

source/common/config/filter_json.cc

+    if (json_config_delay->hasObject("fixed_delay_percentage")) {
+      const auto json_percentage = json_config_delay->getObject("fixed_delay_percentage");
+      JSON_UTIL_SET_FRACTIONALPERCENT(*json_percentage, *delay, percentage);
+    }


You don’t need this. This is for translation of legacy config only.

rshriram · 2018-08-01T02:09:48Z

source/common/config/filter_json.cc

+    if (json_fault->hasObject("percentage")) {
+      const auto json_percentage = json_fault->getObject("percentage");
+      JSON_UTIL_SET_FRACTIONALPERCENT(*json_percentage, *delay, percentage);
+    }


rshriram · 2018-08-01T02:11:54Z

source/common/runtime/runtime_impl.cc

+  return sampleFeatureEnabled(key, default_value, generator_.random(), num_buckets);
+}
+
+bool SnapshotImpl::sampleFeatureEnabled(const std::string& key, uint64_t default_value,


Skip renaming and overload old function name ?

rshriram · 2018-08-01T02:17:05Z

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

-    enabled |= config_->runtime().snapshot().featureEnabled(downstream_cluster_delay_percent_key_,
+  bool enabled = false;
+  if (fault_settings_->delayPercent() != 0) {
+    enabled |= config_->runtime().snapshot().featureEnabled(DELAY_PERCENT_KEY,


This is not right. Runtime overrides config. If there is no runtime the config value becomes the default.

rshriram · 2018-08-01T02:19:07Z

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

+          downstream_cluster_delay_percent_key_, fault_settings_->delayPercentage().numerator(),
+          ProtobufPercentHelper::fractionalPercentDenominatorToInt(
+              fault_settings_->delayPercentage()));
+    }


Flip the logic. Read new runtime or percentage. If not present read old runtime.

rshriram · 2018-08-01T02:22:21Z

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

+  uint64_t abort_percent_{};                                // 0-100
+  uint64_t http_status_{};                                  // HTTP or gRPC return codes
+  uint64_t fixed_delay_percent_{};                          // 0-100
+  envoy::type::FractionalPercent fixed_delay_percentage_{}; // 0-100


Here is a simpler way. Convert both percents to fractional (delay and abort).
Then when reading/translating config, if there is no fractional but old percent field is set, read it and convert it to fractions (out of 100).

Then in fault filter you simply have to check for new runtime fields for fractional percentage. If absent check old field. If that’s also absent use the value provided in the config.

I'd prefer to implement the change to abort in a separate PR.

* Revert v1 changes * Improve the logic that determines whether delay should be enabled Signed-off-by: Venil Noronha <veniln@vmware.com>

rshriram · 2018-08-03T15:34:21Z

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

@@ -41,6 +40,7 @@ FaultSettings::FaultSettings(const envoy::config::filter::http::fault::v2::HTTPF

  if (fault.has_delay()) {
    fixed_delay_percent_ = fault.delay().percent();
+    fixed_delay_percentage_ = fault.delay().percentage();


do you have validtion set to ensure that this field is not null ?
Also, fold fixed_delay_percent_ into fixed_delay_percentage.. i.e. convert the old percent (if set) to new form..
The problem though is that you can't differentiate between a 0 percent and unset percent in hte old one as its uint32.. Sigh..

I've addressed this in 6652866.

Signed-off-by: Venil Noronha <veniln@vmware.com>

* Convert uint32/uint64 percentages to FractionalPercent * Add null validation for FractionalPercent * Document proto changes in version_history Signed-off-by: Venil Noronha <veniln@vmware.com>

rshriram

This is looking very good. A few minor nits related to duplicated tests. Also, please do the same change for aborts as well (abort percentage instead of integer percent)

rshriram · 2018-08-06T15:06:15Z

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

+  //
+  // .. attention::
+  //
+  //   This field is deprecated and `percentage` should be used instead.


Use of integer percent value is deprecated. Use fractional percentage field instead

rshriram · 2018-08-06T15:06:57Z

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: deprecated :ref:`percent <envoy_api_field_config.filter.fault.v2.FaultDelay.percent>` in
+  favor of :ref:`fractional percentages <envoy_api_field_config.filter.fault.v2.FaultDelay.percentage>` for fault injections.


fault: Added support for fractional percentages...

rshriram · 2018-08-06T15:12:09Z

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

-  std::string upstream_cluster_;   // restrict faults to specific upstream cluster
+  uint64_t abort_percent_{};                                // 0-100
+  uint64_t http_status_{};                                  // HTTP or gRPC return codes
+  envoy::type::FractionalPercent fixed_delay_percentage_{}; // 0-100


fix comment to indicate that this is fractional

rshriram · 2018-08-06T15:12:36Z

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

-  uint64_t fixed_delay_percent_{}; // 0-100
-  uint64_t fixed_duration_ms_{};   // in milliseconds
-  std::string upstream_cluster_;   // restrict faults to specific upstream cluster
+  uint64_t abort_percent_{};                                // 0-100


Since you are doing this for delays, why don't you change the aborts as well to fractional ? It makes the implementation streamlined.

Since this PR is big in itself, should we do it in another PR?

The abort stuff is small. Confined to http only. If done right, it should be touching same files and not much.

Yes, I'm on it right now. Figured out it wasn't as big of a change as I thought it would be.

rshriram · 2018-08-06T15:12:52Z

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

@@ -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;


Can you do the same for aborts as well ?

rshriram · 2018-08-06T15:17:14Z

test/extensions/filters/http/fault/fault_filter_test.cc

@@ -321,11 +324,58 @@ TEST_F(FaultFilterTest, FixedDelayZeroDuration) {
  EXPECT_EQ(0UL, config_->stats().aborts_injected_.value());
 }

+TEST_F(FaultFilterTest, V2FixedDelayNonZeroDuration) {


rather than copying, modify the test case below directly. JSON will soon be out.

rshriram · 2018-08-06T15:21:29Z

test/extensions/filters/network/mongo_proxy/proxy_test.cc

 TEST_F(MongoProxyFilterTest, DelayFaults) {
  setupDelayFault(true);
-  initializeFilter();


same comment as above. Simply change this function. You don't need a new function for this.. All you need to test is whether old percent is being respected or not.

You could even change this into DelayFaultWithDeprecatedIntegerPercent and DelayFaultWithFractionalPercent

Signed-off-by: Venil Noronha <veniln@vmware.com>

venilnoronha · 2018-08-06T18:40:23Z

@rshriram I've addressed the last set of comments in f14d3bc. Let me know if you still want the abort changes in this PR, or if you'd prefer it in another PR.

rshriram · 2018-08-06T20:42:38Z

Same pr please. It’s a mass search and replace. Makes it easy to follow patterns

* Deprecate int percent in FaultAbort proto * Introduce FractionalPercent percentages for aborts * Update abort tests Signed-off-by: Venil Noronha <veniln@vmware.com>

rshriram

Looks good so far.

venilnoronha · 2018-08-06T20:51:37Z

Implemented the changes to abort in 3d56b3f.

rshriram

LGTM

rshriram · 2018-08-07T17:09:36Z

ping @mattklein123 .. this is ready for a final pass.

mattklein123

LGTM, some small comments. Thank you!

mattklein123 · 2018-08-07T20:01:16Z

docs/root/intro/version_history.rst

@@ -11,6 +11,7 @@ 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 :ref:`fractional percentages <envoy_api_field_config.filter.fault.v2.FaultDelay.percentage>`.


Please note the deprecations in deprecated.md

mattklein123 · 2018-08-07T20:08:20Z

include/envoy/runtime/runtime.h

+   * of [0, num_buckets).
+   * @return true if the feature is enabled.
+   */
+  virtual bool sampleFeatureEnabled(const std::string& key, uint64_t default_value,


nit: I see why you had to change the name here, but IMO "sample" is a bit strange in the sense that the other featureEnabled() functions also do sampling. A few options here:

Get rid of the versions of featureEnabled() that don't require specifying number of buckets. Larger change.

Some other name. featureEnabledEx ? (Bad, but not sure what would be better).

@mattklein123 thanks for reviewing! I've addressed your other comments in f197fe2, and I'm working on this one now.

Implemented option 1 in f98bcc0.

mattklein123 · 2018-08-07T20:08:51Z

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

@@ -34,12 +34,22 @@ 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();
+    if (fault.abort().has_percentage()) {


extract to helper function for duplicated code here and below.

mattklein123 · 2018-08-07T20:09:17Z

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

-  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_{};       // 0.0-100.0


{} not needed. Also, comment is no longer relevant I think. Same below.

mattklein123 · 2018-08-07T20:09:37Z

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

-        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)) {
+    if (fault_config.has_percentage()) {


same utility function extracted above

* Update DEPRECATED and version_history docs * Fix FractionalPercent variable declaration issue * Extract repeating logic into a macro Signed-off-by: Venil Noronha <veniln@vmware.com>

Signed-off-by: Venil Noronha <veniln@vmware.com>

mattklein123

Thanks for the cleanup, LGTM. @rshriram can you take one last pass given the new changes?

rshriram · 2018-08-13T02:11:12Z

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)) {


why this change?

See this comment.

rshriram · 2018-08-13T02:16:31Z

@venilnoronha I think you lost some of your changes. You haven't implemented either option that matt suggested but the call sites seem to have changed

rshriram · 2018-08-13T02:18:07Z

include/envoy/runtime/runtime.h

   * @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;


did you intend to delete this function and simply use the one below everywhere?

I wanted to use the function below whenever it was possible to supply a random value. However, there are a few cases where there isn't a random number available at call time, and that's where I use this.

rshriram · 2018-08-13T02:19:47Z

source/common/runtime/runtime_impl.cc

-                                  uint64_t random_value) const {
-  return featureEnabled(key, default_value, random_value, 100);
-}
-


These two functions are not equivalent.. The stable random value is not same as number of buckets above. Am I missing something here>?

random_value and num_buckets aren't being used interchangeably. Either we pass both random_value and num_buckets to featureEnabled, or we only pass num_buckets. There's no way to pass random_value without num_buckets.

rshriram · 2018-08-13T02:32:02Z

So I suggest this.. Replace all call sites of featureEnabled() with the longer version that always passes the number of buckets - But do this in a separate PR. This should essentially be a mass search-and-replace. Should have no issues with any reviewer. Then this PR will automatically reduce to the changes related to fault injection.

Alternatively, simply rename the sampleFeatureEnabled to something else like featureEnabledEx and retain changes before you followed matt's option

Signed-off-by: Venil Noronha <veniln@vmware.com>

venilnoronha · 2018-08-13T02:53:34Z

@rshriram I've reverted the changes and have renamed sampleFeatureEnabled to featureEnabledEx in 9b1046f.

@mattklein123 as @rshriram suggested, I'll take up option 1 in a separate PR.

Thanks,
Venil

mattklein123 · 2018-08-13T02:57:37Z

@mattklein123 as @rshriram suggested, I'll take up option 1 in a separate PR.

I would rather do one set of changes. If you want to do option 1, can you just do that PR first, we can review/merge, and then we can merge master into this?

venilnoronha · 2018-08-13T03:09:34Z

On a second thought, I'd prefer keeping featureEnabledEx. We could cherry-pick f98bcc0 later if absolutely necessary.

rshriram · 2018-08-13T13:01:23Z

include/envoy/runtime/runtime.h

+   * @return true if the feature is enabled.
+   */
+  virtual bool featureEnabledEx(const std::string& key, uint64_t default_value,
+                                uint64_t num_buckets) const PURE;


Is there any callsite that is calling the 3 arg form of featureEnabledEx supplying buckets instead of random value? If not, lets get rid of this function.

+1 if possible.

Addressed via aef3da0 and c31bd82.

rshriram · 2018-08-13T13:21:06Z

So here is another easier way to resolve this: Add a random() function to the Loader interface.
In the fault filter code, use config_->runtime()->random(), so that you can call the existing featureEnabled(4 arg) function. This would avoid changing other call sites. All you need to do is add the new function to the interface and update a few mocks.

Signed-off-by: Venil Noronha <veniln@vmware.com>

mattklein123

I'm fine with the Ex change, but will defer to @rshriram and you to decide if you want to do something different.

mattklein123 · 2018-08-13T18:03:26Z

include/envoy/runtime/runtime.h

+   * @return true if the feature is enabled.
+   */
+  virtual bool featureEnabledEx(const std::string& key, uint64_t default_value,
+                                uint64_t num_buckets) const PURE;


+1 if possible.

This commit removes the 3 param sampleFeatureEnabled routine and renames the 4 param sampleFeatureEnabled routine back to featureEnabled. It also exports the RandomGenerator via the Loader in order to provide random values for featureEnabled calls. Signed-off-by: Venil Noronha <veniln@vmware.com>

mattklein123 · 2018-08-13T21:28:11Z

include/envoy/runtime/runtime.h

+  /**
+   * @return RandomGenerator& the random generator.
+   */
+  virtual RandomGenerator& random() PURE;


Do we really need to add this to this interface? I guess it's OK, but can't you just pass the random generator to where you need it? It should be in the filter config factory? This is what we do elsewhere.

Thought that I'd have to pass the RandomGenerator instance a long way, but I was wrong. I've fixed this in f167d92.

This commit removes the random() routine from the Loader and instead passes the RandomGenerator via the constructor to FaultFilterConfig and the ProxyFilter. Signed-off-by: Venil Noronha <veniln@vmware.com>

mattklein123

Thanks, very nice!

venilnoronha · 2018-08-14T03:35:18Z

@mattklein123 @rshriram thanks for reviewing/merging.

* origin/master: (38 commits) test: add tests for corner-cases around sending requests before run() starts or after run() ends. (envoyproxy#4114) perf: reduce the memory usage of LC Trie construction (envoyproxy#4117) test: moving redundant code in websocket_integration_test to utilities (envoyproxy#4127) test: make YamlLoadFromStringFail less picky about error msg. (envoyproxy#4141) rbac: add rbac network filter. (envoyproxy#4083) fuzz: route lookup and header finalization fuzzer. (envoyproxy#4116) Set content-type and content-length (envoyproxy#4113) fault: use FractionalPercent for percent (envoyproxy#3978) test: Fix inverted exact match logic in IntegrationTcpClient::waitForData() (envoyproxy#4134) Added cluster_name to load assignment config for static cluster (envoyproxy#4123) ssl: refactor ContextConfig to use TlsCertificateConfig (envoyproxy#4115) syscall: refactor OsSysCalls for deeper errno latching (envoyproxy#4111) thrift_proxy: fix oneway bugs (envoyproxy#4025) Do not crash when converting YAML to JSON fails (envoyproxy#4110) config: allow unknown fields flag (take 2) (envoyproxy#4096) Use a jittered backoff strategy for handling HdsDelegate stream/connection failures (envoyproxy#4108) bazel: use GCS remote cache (envoyproxy#4050) Add thread local cache of overload action states (envoyproxy#4090) Added TCP healthcheck capabilities to the HdsDelegate (envoyproxy#4079) secret: add secret provider interface and use it for TlsCertificates (envoyproxy#4086) ... Signed-off-by: Snow Pettersen <snowp@squareup.com>

mattklein123 reviewed Jul 28, 2018

View reviewed changes

Remove THOUSAND and HUNDRED_THOUSAND from proto

f105661

Signed-off-by: Venil Noronha <veniln@vmware.com>

mattklein123 assigned rshriram Jul 30, 2018

rshriram reviewed Jul 30, 2018

View reviewed changes

Address review comments

4daaac1

* Revert v1 code, schema & doc changes * Deprecate percent field in FaultDelay * Introduce percentage field in FaultDelay of type FractionalPercent Signed-off-by: Venil Noronha <veniln@vmware.com>

venilnoronha force-pushed the gh-3904-fix branch from 4fd33a0 to 4daaac1 Compare July 31, 2018 18:22

rshriram reviewed Aug 1, 2018

View reviewed changes

Address review comments

4e3a07d

* Revert v1 changes * Improve the logic that determines whether delay should be enabled Signed-off-by: Venil Noronha <veniln@vmware.com>

rshriram reviewed Aug 3, 2018

View reviewed changes

venilnoronha added 2 commits August 3, 2018 12:12

Add tests for delays with fractional percentage

d4a0e76

Signed-off-by: Venil Noronha <veniln@vmware.com>

Translate int to FractionalPercent internally

6652866

* Convert uint32/uint64 percentages to FractionalPercent * Add null validation for FractionalPercent * Document proto changes in version_history Signed-off-by: Venil Noronha <veniln@vmware.com>

rshriram reviewed Aug 6, 2018

View reviewed changes

Update code comments and simplify tests

f14d3bc

Signed-off-by: Venil Noronha <veniln@vmware.com>

Use FractionalPercent for percent in abort

3d56b3f

* Deprecate int percent in FaultAbort proto * Introduce FractionalPercent percentages for aborts * Update abort tests Signed-off-by: Venil Noronha <veniln@vmware.com>

rshriram reviewed Aug 6, 2018

View reviewed changes

rshriram previously approved these changes Aug 7, 2018

View reviewed changes

mattklein123 reviewed Aug 7, 2018

View reviewed changes

Update docs, fix minor code issues, extract util

f197fe2

* Update DEPRECATED and version_history docs * Fix FractionalPercent variable declaration issue * Extract repeating logic into a macro Signed-off-by: Venil Noronha <veniln@vmware.com>

venilnoronha dismissed rshriram’s stale review via f197fe2 August 8, 2018 03:31

mattklein123 self-assigned this Aug 8, 2018

venilnoronha added 2 commits August 9, 2018 18:15

Refactor featureEnabled to reduce code complexity

f98bcc0

Signed-off-by: Venil Noronha <veniln@vmware.com>

Reorder featureEnabled method declaration

c0f9701

Signed-off-by: Venil Noronha <veniln@vmware.com>

mattklein123 previously approved these changes Aug 10, 2018

View reviewed changes

rshriram reviewed Aug 13, 2018

View reviewed changes

venilnoronha added 2 commits August 12, 2018 19:41

Revert to f197fe2

ca6fa14

Signed-off-by: Venil Noronha <veniln@vmware.com>

Rename sampleFeatureEnabled to featureEnabledEx

9b1046f

Signed-off-by: Venil Noronha <veniln@vmware.com>

venilnoronha dismissed mattklein123’s stale review via 9b1046f August 13, 2018 02:46

rshriram reviewed Aug 13, 2018

View reviewed changes

Undo rename featureEnabledEx to sampleFeatureEnabled

aef3da0

Signed-off-by: Venil Noronha <veniln@vmware.com>

mattklein123 reviewed Aug 13, 2018

View reviewed changes

venilnoronha force-pushed the gh-3904-fix branch from 54c6ac0 to ea1d5ea Compare August 14, 2018 00:11

Refactor Loader and pass RandomGenerator at creation

f167d92

This commit removes the random() routine from the Loader and instead passes the RandomGenerator via the constructor to FaultFilterConfig and the ProxyFilter. Signed-off-by: Venil Noronha <veniln@vmware.com>

venilnoronha force-pushed the gh-3904-fix branch from ea1d5ea to f167d92 Compare August 14, 2018 01:42

mattklein123 approved these changes Aug 14, 2018

View reviewed changes

mattklein123 merged commit 714ae13 into envoyproxy:master Aug 14, 2018

trabetti mentioned this pull request Aug 14, 2018

Use fraction instead of uint32 for percentage in fault injection, following the same change in Envoy istio/istio#7903

Closed

mattklein123 mentioned this pull request Oct 5, 2018

[v1.8.0 (Oct 4, 2018) deprecation] Remove features marked deprecated in #3978 #4620

Closed

fault: use FractionalPercent for percent #3978

fault: use FractionalPercent for percent #3978

Conversation

venilnoronha commented Jul 28, 2018 • edited Loading

mattklein123 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

venilnoronha commented Jul 28, 2018

mattklein123 commented Jul 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

venilnoronha Aug 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rshriram left a comment

Choose a reason for hiding this comment

rshriram left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rshriram left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

venilnoronha commented Aug 6, 2018

rshriram commented Aug 6, 2018

rshriram left a comment

Choose a reason for hiding this comment

venilnoronha commented Aug 6, 2018

rshriram left a comment

Choose a reason for hiding this comment

rshriram commented Aug 7, 2018

mattklein123 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

venilnoronha Aug 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattklein123 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

venilnoronha Aug 13, 2018 • edited Loading

Choose a reason for hiding this comment

rshriram commented Aug 13, 2018

Choose a reason for hiding this comment

venilnoronha Aug 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rshriram commented Aug 13, 2018

venilnoronha commented Aug 13, 2018

mattklein123 commented Aug 13, 2018

venilnoronha commented Aug 13, 2018 • edited Loading

venilnoronha commented Jul 28, 2018 •

edited

Loading

venilnoronha Aug 1, 2018 •

edited

Loading

venilnoronha Aug 8, 2018 •

edited

Loading

venilnoronha Aug 13, 2018 •

edited

Loading

venilnoronha Aug 13, 2018 •

edited

Loading

venilnoronha commented Aug 13, 2018 •

edited

Loading

venilnoronha Aug 13, 2018 •

edited

Loading