Skip to content

Commit f98bcc0

Browse files
venilnoronhavenilnoronha
committed
Refactor featureEnabled to reduce code complexity
Signed-off-by: Venil Noronha <[email protected]>
1 parent f197fe2 commit f98bcc0

File tree

19 files changed

+123
-155
lines changed

19 files changed

+123
-155
lines changed

‎DEPRECATED.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,10 @@ A logged warning is expected for each deprecated item that is in deprecation win
2323
is deprecated. Please use the new `upgrade_configs` in the
2424
[HttpConnectionManager](https://github.com/envoyproxy/envoy/blob/master/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto)
2525
instead.
26-
* Setting hosts via `hosts` field in `Cluster` is deprecated. Use `load_assignment` instead.
2726
* Use of the integer `percent` field in [FaultDelay](https://github.com/envoyproxy/envoy/blob/master/api/envoy/config/filter/fault/v2/fault.proto)
2827
and in [FaultAbort](https://github.com/envoyproxy/envoy/blob/master/api/envoy/config/filter/http/fault/v2/fault.proto) is deprecated in favor
2928
of the new `FractionalPercent` based `percentage` field.
29+
* Setting hosts via `hosts` field in `Cluster` is deprecated. Use `load_assignment` instead.
3030

3131
## Version 1.7.0
3232

‎include/envoy/runtime/runtime.h‎

Lines changed: 9 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -90,17 +90,20 @@ class Snapshot {
9090
virtual bool featureEnabled(const std::string& key, uint64_t default_value) const PURE;
9191

9292
/**
93-
* Test if a feature is enabled using a supplied stable random value. This variant is used if
94-
* the caller wants a stable result over multiple calls.
93+
* Test if a feature is enabled using a supplied stable random value and total number of buckets
94+
* for sampling. This variant is used if the caller wants a stable result over multiple calls
95+
* and have more granularity for samples.
9596
* @param key supplies the feature key to lookup.
9697
* @param default_value supplies the default value that will be used if either the feature key
9798
* does not exist or it is not an integer.
9899
* @param random_value supplies the stable random value to use for determining whether the feature
99100
* is enabled.
101+
* @param num_buckets control max number of buckets for sampling. Sampled value will be in a range
102+
* of [0, num_buckets).
100103
* @return true if the feature is enabled.
101104
*/
102-
virtual bool featureEnabled(const std::string& key, uint64_t default_value,
103-
uint64_t random_value) const PURE;
105+
virtual bool featureEnabled(const std::string& key, uint64_t default_value, uint64_t random_value,
106+
uint64_t num_buckets) const PURE;
104107

105108
/**
106109
* Test if a feature is enabled using the built in random generator and total number of buckets
@@ -112,25 +115,8 @@ class Snapshot {
112115
* of [0, num_buckets).
113116
* @return true if the feature is enabled.
114117
*/
115-
virtual bool sampleFeatureEnabled(const std::string& key, uint64_t default_value,
116-
uint64_t num_buckets) const PURE;
117-
118-
/**
119-
* Test if a feature is enabled using a supplied stable random value and total number of buckets
120-
* for sampling.
121-
* This variant is used if the caller wants a stable result over multiple calls
122-
* and have more granularity for samples.
123-
* @param key supplies the feature key to lookup.
124-
* @param default_value supplies the default value that will be used if either the feature key
125-
* does not exist or it is not an integer.
126-
* @param random_value supplies the stable random value to use for determining whether the feature
127-
* is enabled.
128-
* @param num_buckets control max number of buckets for sampling. Sampled value will be in a range
129-
* of [0, num_buckets).
130-
* @return true if the feature is enabled.
131-
*/
132-
virtual bool sampleFeatureEnabled(const std::string& key, uint64_t default_value,
133-
uint64_t random_value, uint64_t num_buckets) const PURE;
118+
virtual bool featureEnabled(const std::string& key, uint64_t default_value,
119+
uint64_t num_buckets) const PURE;
134120

135121
/**
136122
* Fetch raw runtime data based on key.

‎source/common/access_log/access_log_impl.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ bool RuntimeFilter::evaluate(const RequestInfo::RequestInfo&,
118118
random_value = random_.random();
119119
}
120120

121-
return runtime_.snapshot().sampleFeatureEnabled(
121+
return runtime_.snapshot().featureEnabled(
122122
runtime_key_, percent_.numerator(), random_value,
123123
ProtobufPercentHelper::fractionalPercentDenominatorToInt(percent_));
124124
}

‎source/common/http/conn_manager_utility.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -204,15 +204,15 @@ void ConnectionManagerUtility::mutateTracingRequestHeader(Http::HeaderMap& reque
204204
UuidUtils::setTraceableUuid(x_request_id, UuidTraceStatus::Client);
205205
} else if (request_headers.EnvoyForceTrace()) {
206206
UuidUtils::setTraceableUuid(x_request_id, UuidTraceStatus::Forced);
207-
} else if (runtime.snapshot().sampleFeatureEnabled("tracing.random_sampling",
208-
config.tracingConfig()->random_sampling_,
209-
result, 10000)) {
207+
} else if (runtime.snapshot().featureEnabled("tracing.random_sampling",
208+
config.tracingConfig()->random_sampling_, result,
209+
10000)) {
210210
UuidUtils::setTraceableUuid(x_request_id, UuidTraceStatus::Sampled);
211211
}
212212
}
213213

214214
if (!runtime.snapshot().featureEnabled("tracing.global_enabled",
215-
config.tracingConfig()->overall_sampling_, result)) {
215+
config.tracingConfig()->overall_sampling_, result, 100)) {
216216
UuidUtils::setTraceableUuid(x_request_id, UuidTraceStatus::NoTrace);
217217
}
218218

‎source/common/router/config_impl.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ bool RouteEntryImplBase::matchRoute(const Http::HeaderMap& headers, uint64_t ran
342342

343343
if (runtime_) {
344344
matches &= loader_.snapshot().featureEnabled(runtime_.value().key_, runtime_.value().default_,
345-
random_value);
345+
random_value, 100);
346346
}
347347

348348
matches &= Http::HeaderUtility::matchHeaders(headers, config_headers_);

‎source/common/router/router.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ bool FilterUtility::shouldShadow(const ShadowPolicy& policy, Runtime::Loader& ru
5050
}
5151

5252
if (!policy.runtimeKey().empty() &&
53-
!runtime.snapshot().sampleFeatureEnabled(policy.runtimeKey(), 0, stable_random, 10000UL)) {
53+
!runtime.snapshot().featureEnabled(policy.runtimeKey(), 0, stable_random, 10000UL)) {
5454
return false;
5555
}
5656

‎source/common/runtime/runtime_impl.cc‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -146,13 +146,13 @@ std::string RandomGeneratorImpl::uuid() {
146146
return std::string(uuid, UUID_LENGTH);
147147
}
148148

149-
bool SnapshotImpl::sampleFeatureEnabled(const std::string& key, uint64_t default_value,
150-
uint64_t num_buckets) const {
151-
return sampleFeatureEnabled(key, default_value, generator_.random(), num_buckets);
149+
bool SnapshotImpl::featureEnabled(const std::string& key, uint64_t default_value,
150+
uint64_t num_buckets) const {
151+
return featureEnabled(key, default_value, generator_.random(), num_buckets);
152152
}
153153

154-
bool SnapshotImpl::sampleFeatureEnabled(const std::string& key, uint64_t default_value,
155-
uint64_t random_value, uint64_t num_buckets) const {
154+
bool SnapshotImpl::featureEnabled(const std::string& key, uint64_t default_value,
155+
uint64_t random_value, uint64_t num_buckets) const {
156156
return random_value % num_buckets < std::min(getInteger(key, default_value), num_buckets);
157157
}
158158

@@ -168,11 +168,6 @@ bool SnapshotImpl::featureEnabled(const std::string& key, uint64_t default_value
168168
}
169169
}
170170

171-
bool SnapshotImpl::featureEnabled(const std::string& key, uint64_t default_value,
172-
uint64_t random_value) const {
173-
return sampleFeatureEnabled(key, default_value, random_value, 100);
174-
}
175-
176171
const std::string& SnapshotImpl::get(const std::string& key) const {
177172
auto entry = values_.find(key);
178173
if (entry == values_.end()) {

‎source/common/runtime/runtime_impl.h‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,11 @@ class SnapshotImpl : public Snapshot, public ThreadLocal::ThreadLocalObject {
6464
std::vector<OverrideLayerConstPtr>&& layers);
6565

6666
// Runtime::Snapshot
67-
bool sampleFeatureEnabled(const std::string& key, uint64_t default_value,
68-
uint64_t num_buckets) const override;
69-
bool sampleFeatureEnabled(const std::string& key, uint64_t default_value, uint64_t random_value,
70-
uint64_t num_buckets) const override;
71-
bool featureEnabled(const std::string& key, uint64_t default_value) const override;
7267
bool featureEnabled(const std::string& key, uint64_t default_value,
73-
uint64_t random_value) const override;
68+
uint64_t num_buckets) const override;
69+
bool featureEnabled(const std::string& key, uint64_t default_value, uint64_t random_value,
70+
uint64_t num_buckets) const override;
71+
bool featureEnabled(const std::string& key, uint64_t default_value) const override;
7472
const std::string& get(const std::string& key) const override;
7573
uint64_t getInteger(const std::string& key, uint64_t default_value) const override;
7674
const std::vector<OverrideLayerConstPtr>& getLayers() const override;

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,11 +131,11 @@ Http::FilterHeadersStatus FaultFilter::decodeHeaders(Http::HeaderMap& headers, b
131131
}
132132

133133
bool FaultFilter::isDelayEnabled() {
134-
bool enabled = config_->runtime().snapshot().sampleFeatureEnabled(
134+
bool enabled = config_->runtime().snapshot().featureEnabled(
135135
DELAY_PERCENT_KEY, fault_settings_->delayPercentage().numerator(),
136136
ProtobufPercentHelper::fractionalPercentDenominatorToInt(fault_settings_->delayPercentage()));
137137
if (!downstream_cluster_delay_percent_key_.empty()) {
138-
enabled |= config_->runtime().snapshot().sampleFeatureEnabled(
138+
enabled |= config_->runtime().snapshot().featureEnabled(
139139
downstream_cluster_delay_percent_key_, fault_settings_->delayPercentage().numerator(),
140140
ProtobufPercentHelper::fractionalPercentDenominatorToInt(
141141
fault_settings_->delayPercentage()));
@@ -144,11 +144,11 @@ bool FaultFilter::isDelayEnabled() {
144144
}
145145

146146
bool FaultFilter::isAbortEnabled() {
147-
bool enabled = config_->runtime().snapshot().sampleFeatureEnabled(
147+
bool enabled = config_->runtime().snapshot().featureEnabled(
148148
ABORT_PERCENT_KEY, fault_settings_->abortPercentage().numerator(),
149149
ProtobufPercentHelper::fractionalPercentDenominatorToInt(fault_settings_->abortPercentage()));
150150
if (!downstream_cluster_abort_percent_key_.empty()) {
151-
enabled |= config_->runtime().snapshot().sampleFeatureEnabled(
151+
enabled |= config_->runtime().snapshot().featureEnabled(
152152
downstream_cluster_abort_percent_key_, fault_settings_->abortPercentage().numerator(),
153153
ProtobufPercentHelper::fractionalPercentDenominatorToInt(
154154
fault_settings_->abortPercentage()));

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -319,10 +319,10 @@ absl::optional<uint64_t> ProxyFilter::delayDuration() {
319319
return result;
320320
}
321321

322-
if (!runtime_.snapshot().sampleFeatureEnabled(
323-
MongoRuntimeConfig::get().FixedDelayPercent, fault_config_->delayPercentage().numerator(),
324-
ProtobufPercentHelper::fractionalPercentDenominatorToInt(
325-
fault_config_->delayPercentage()))) {
322+
if (!runtime_.snapshot().featureEnabled(MongoRuntimeConfig::get().FixedDelayPercent,
323+
fault_config_->delayPercentage().numerator(),
324+
ProtobufPercentHelper::fractionalPercentDenominatorToInt(
325+
fault_config_->delayPercentage()))) {
326326
return result;
327327
}
328328

0 commit comments

Comments
 (0)