Skip to content

fault: use FractionalPercent for percent #3978

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.

Sign up for GitHub

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

Merged
mattklein123 merged 16 commits into from
Aug 14, 2018
Merged

fault: use FractionalPercent for percent #3978

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
35dc15a
fault: use FractionalPercent for percent
venilnoronha Jul 28, 2018
f105661
Remove THOUSAND and HUNDRED_THOUSAND from proto
venilnoronha Jul 28, 2018
4daaac1
Address review comments
venilnoronha Jul 31, 2018
4e3a07d
Address review comments
venilnoronha Aug 3, 2018
d4a0e76
Add tests for delays with fractional percentage
venilnoronha Aug 3, 2018
6652866
Translate int to FractionalPercent internally
venilnoronha Aug 3, 2018
f14d3bc
Update code comments and simplify tests
venilnoronha Aug 6, 2018
3d56b3f
Use FractionalPercent for percent in abort
venilnoronha Aug 6, 2018
f197fe2
Update docs, fix minor code issues, extract util
venilnoronha Aug 7, 2018
f98bcc0
Refactor featureEnabled to reduce code complexity
venilnoronha Aug 10, 2018
c0f9701
Reorder featureEnabled method declaration
venilnoronha Aug 10, 2018
ca6fa14
Revert to f197fe25
venilnoronha Aug 13, 2018
9b1046f
Rename sampleFeatureEnabled to featureEnabledEx
venilnoronha Aug 13, 2018
aef3da0
Undo rename featureEnabledEx to sampleFeatureEnabled
venilnoronha Aug 13, 2018
c31bd82
Refactor featureEnabled and impl Loader:random()
venilnoronha Aug 13, 2018
f167d92
Refactor Loader and pass RandomGenerator at creation
venilnoronha Aug 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api/envoy/config/filter/fault/v2/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
)
11 changes: 10 additions & 1 deletion api/envoy/config/filter/fault/v2/fault.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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;
Expand All @@ -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;

@rshriram rshriram Aug 6, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you do the same for aborts as well ?

}
1 change: 1 addition & 0 deletions docs/root/intro/version_history.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>`.

@mattklein123 mattklein123 Aug 7, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please note the deprecations in deprecated.md

* 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>`.
Expand Down
21 changes: 17 additions & 4 deletions include/envoy/runtime/runtime.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,19 @@ class Snapshot {
virtual bool featureEnabled(const std::string& key, uint64_t default_value,
uint64_t random_value) const PURE;

/**
* 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 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 sampleFeatureEnabled(const std::string& key, uint64_t default_value,

@mattklein123 mattklein123 Aug 7, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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:

  1. Get rid of the versions of featureEnabled() that don't require specifying number of buckets. Larger change.
  2. Some other name. featureEnabledEx ? (Bad, but not sure what would be better).

@venilnoronha venilnoronha Aug 8, 2018 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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

@venilnoronha venilnoronha Aug 10, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Implemented option 1 in f98bcc0.

uint64_t num_buckets) const PURE;

/**
* Test if a feature is enabled using a supplied stable random value and total number of buckets
* for sampling.
Expand All @@ -112,12 +125,12 @@ 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,
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;

@rshriram rshriram Jul 30, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@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

@venilnoronha venilnoronha Jul 30, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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?

@rshriram rshriram Aug 1, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking again, why is this being renamed ?

@venilnoronha venilnoronha Aug 1, 2018 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Because of a signature conflict between the routines highlighted here.


/**
* Fetch raw runtime data based on key.
Expand Down
2 changes: 1 addition & 1 deletion source/common/access_log/access_log_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ bool RuntimeFilter::evaluate(const RequestInfo::RequestInfo&,
random_value = random_.random();
}

return runtime_.snapshot().featureEnabled(
return runtime_.snapshot().sampleFeatureEnabled(
runtime_key_, percent_.numerator(), random_value,
ProtobufPercentHelper::fractionalPercentDenominatorToInt(percent_));
}
Expand Down
6 changes: 3 additions & 3 deletions source/common/http/conn_manager_utility.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,9 @@ void ConnectionManagerUtility::mutateTracingRequestHeader(Http::HeaderMap& reque
UuidUtils::setTraceableUuid(x_request_id, UuidTraceStatus::Client);
} else if (request_headers.EnvoyForceTrace()) {
UuidUtils::setTraceableUuid(x_request_id, UuidTraceStatus::Forced);
} else if (runtime.snapshot().featureEnabled("tracing.random_sampling",
config.tracingConfig()->random_sampling_, result,
10000)) {
} else if (runtime.snapshot().sampleFeatureEnabled("tracing.random_sampling",
config.tracingConfig()->random_sampling_,
result, 10000)) {
UuidUtils::setTraceableUuid(x_request_id, UuidTraceStatus::Sampled);
}
}
Expand Down
2 changes: 1 addition & 1 deletion source/common/router/router.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ bool FilterUtility::shouldShadow(const ShadowPolicy& policy, Runtime::Loader& ru
}

if (!policy.runtimeKey().empty() &&
!runtime.snapshot().featureEnabled(policy.runtimeKey(), 0, stable_random, 10000UL)) {
!runtime.snapshot().sampleFeatureEnabled(policy.runtimeKey(), 0, stable_random, 10000UL)) {
return false;
}

Expand Down
11 changes: 8 additions & 3 deletions source/common/runtime/runtime_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,13 @@ std::string RandomGeneratorImpl::uuid() {
return std::string(uuid, UUID_LENGTH);
}

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

bool SnapshotImpl::sampleFeatureEnabled(const std::string& key, uint64_t default_value,

@rshriram rshriram Aug 1, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Skip renaming and overload old function name ?

@rshriram rshriram Aug 1, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ignore

uint64_t random_value, uint64_t num_buckets) const {
return random_value % num_buckets < std::min(getInteger(key, default_value), num_buckets);
}

Expand All @@ -165,7 +170,7 @@ 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);
return sampleFeatureEnabled(key, default_value, random_value, 100);
}

@rshriram rshriram Aug 13, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

@venilnoronha venilnoronha Aug 13, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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.

const std::string& SnapshotImpl::get(const std::string& key) const {
Expand Down
6 changes: 4 additions & 2 deletions source/common/runtime/runtime_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,10 @@ 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 random_value,
uint64_t num_buckets) const override;
bool sampleFeatureEnabled(const std::string& key, uint64_t default_value,
uint64_t num_buckets) const override;
bool sampleFeatureEnabled(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;
Expand Down
20 changes: 13 additions & 7 deletions source/extensions/filters/http/fault/fault_filter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,12 @@ FaultSettings::FaultSettings(const envoy::config::filter::http::fault::v2::HTTPF
}

if (fault.has_delay()) {
fixed_delay_percent_ = fault.delay().percent();
if (fault.delay().has_percentage()) {
fixed_delay_percentage_ = fault.delay().percentage();
} else {
fixed_delay_percentage_.set_numerator(fault.delay().percent());
fixed_delay_percentage_.set_denominator(envoy::type::FractionalPercent::HUNDRED);
}
const auto& delay = fault.delay();
fixed_duration_ms_ = PROTOBUF_GET_MS_OR_DEFAULT(delay, fixed_delay, 0);
}
Expand Down Expand Up @@ -129,14 +134,15 @@ 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().sampleFeatureEnabled(
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().sampleFeatureEnabled(
downstream_cluster_delay_percent_key_, fault_settings_->delayPercentage().numerator(),
ProtobufPercentHelper::fractionalPercentDenominatorToInt(
fault_settings_->delayPercentage()));
}

return enabled;
}

Expand Down
12 changes: 6 additions & 6 deletions source/extensions/filters/http/fault/fault_filter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,18 +46,18 @@ class FaultSettings : public Router::RouteSpecificFilterConfig {
return fault_filter_headers_;
}
uint64_t abortPercent() const { return abort_percent_; }
uint64_t delayPercent() const { return fixed_delay_percent_; }
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
uint64_t abort_percent_{}; // 0-100

@rshriram rshriram Aug 6, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

@venilnoronha venilnoronha Aug 6, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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

@rshriram rshriram Aug 6, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

@venilnoronha venilnoronha Aug 6, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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

uint64_t http_status_{}; // HTTP or gRPC return codes
envoy::type::FractionalPercent fixed_delay_percentage_{}; // 0.0-100.0
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
};
Expand Down
6 changes: 4 additions & 2 deletions source/extensions/filters/network/mongo_proxy/proxy.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,8 +319,10 @@ absl::optional<uint64_t> ProxyFilter::delayDuration() {
return result;
}

if (!runtime_.snapshot().featureEnabled(MongoRuntimeConfig::get().FixedDelayPercent,
fault_config_->delayPercent())) {
if (!runtime_.snapshot().sampleFeatureEnabled(
MongoRuntimeConfig::get().FixedDelayPercent, fault_config_->delayPercentage().numerator(),
ProtobufPercentHelper::fractionalPercentDenominatorToInt(
fault_config_->delayPercentage()))) {
return result;
}

Expand Down
14 changes: 10 additions & 4 deletions source/extensions/filters/network/mongo_proxy/proxy.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,19 @@ 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)) {
if (fault_config.has_percentage()) {

@mattklein123 mattklein123 Aug 7, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same utility function extracted above

delay_percentage_ = fault_config.percentage();
} else {
delay_percentage_.set_numerator(static_cast<uint32_t>(fault_config.percent()));
delay_percentage_.set_denominator(envoy::type::FractionalPercent::HUNDRED);
}
}
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_;
};

Expand Down
20 changes: 10 additions & 10 deletions test/common/access_log/access_log_impl_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,25 +204,25 @@ TEST_F(AccessLogImplTest, RuntimeFilter) {

// Value is taken from random generator.
EXPECT_CALL(context_.random_, random()).WillOnce(Return(42));
EXPECT_CALL(runtime_.snapshot_, featureEnabled("access_log.test_key", 0, 42, 100))
EXPECT_CALL(runtime_.snapshot_, sampleFeatureEnabled("access_log.test_key", 0, 42, 100))
.WillOnce(Return(true));
EXPECT_CALL(*file_, write(_));
log->log(&request_headers_, &response_headers_, &response_trailers_, request_info_);

EXPECT_CALL(context_.random_, random()).WillOnce(Return(43));
EXPECT_CALL(runtime_.snapshot_, featureEnabled("access_log.test_key", 0, 43, 100))
EXPECT_CALL(runtime_.snapshot_, sampleFeatureEnabled("access_log.test_key", 0, 43, 100))
.WillOnce(Return(false));
EXPECT_CALL(*file_, write(_)).Times(0);
log->log(&request_headers_, &response_headers_, &response_trailers_, request_info_);

// Value is taken from x-request-id.
request_headers_.addCopy("x-request-id", "000000ff-0000-0000-0000-000000000000");
EXPECT_CALL(runtime_.snapshot_, featureEnabled("access_log.test_key", 0, 55, 100))
EXPECT_CALL(runtime_.snapshot_, sampleFeatureEnabled("access_log.test_key", 0, 55, 100))
.WillOnce(Return(true));
EXPECT_CALL(*file_, write(_));
log->log(&request_headers_, &response_headers_, &response_trailers_, request_info_);

EXPECT_CALL(runtime_.snapshot_, featureEnabled("access_log.test_key", 0, 55, 100))
EXPECT_CALL(runtime_.snapshot_, sampleFeatureEnabled("access_log.test_key", 0, 55, 100))
.WillOnce(Return(false));
EXPECT_CALL(*file_, write(_)).Times(0);
log->log(&request_headers_, &response_headers_, &response_trailers_, request_info_);
Expand All @@ -245,25 +245,25 @@ name: envoy.file_access_log

// Value is taken from random generator.
EXPECT_CALL(context_.random_, random()).WillOnce(Return(42));
EXPECT_CALL(runtime_.snapshot_, featureEnabled("access_log.test_key", 5, 42, 10000))
EXPECT_CALL(runtime_.snapshot_, sampleFeatureEnabled("access_log.test_key", 5, 42, 10000))
.WillOnce(Return(true));
EXPECT_CALL(*file_, write(_));
log->log(&request_headers_, &response_headers_, &response_trailers_, request_info_);

EXPECT_CALL(context_.random_, random()).WillOnce(Return(43));
EXPECT_CALL(runtime_.snapshot_, featureEnabled("access_log.test_key", 5, 43, 10000))
EXPECT_CALL(runtime_.snapshot_, sampleFeatureEnabled("access_log.test_key", 5, 43, 10000))
.WillOnce(Return(false));
EXPECT_CALL(*file_, write(_)).Times(0);
log->log(&request_headers_, &response_headers_, &response_trailers_, request_info_);

// Value is taken from x-request-id.
request_headers_.addCopy("x-request-id", "000000ff-0000-0000-0000-000000000000");
EXPECT_CALL(runtime_.snapshot_, featureEnabled("access_log.test_key", 5, 255, 10000))
EXPECT_CALL(runtime_.snapshot_, sampleFeatureEnabled("access_log.test_key", 5, 255, 10000))
.WillOnce(Return(true));
EXPECT_CALL(*file_, write(_));
log->log(&request_headers_, &response_headers_, &response_trailers_, request_info_);

EXPECT_CALL(runtime_.snapshot_, featureEnabled("access_log.test_key", 5, 255, 10000))
EXPECT_CALL(runtime_.snapshot_, sampleFeatureEnabled("access_log.test_key", 5, 255, 10000))
.WillOnce(Return(false));
EXPECT_CALL(*file_, write(_)).Times(0);
log->log(&request_headers_, &response_headers_, &response_trailers_, request_info_);
Expand All @@ -288,13 +288,13 @@ name: envoy.file_access_log
// Value should not be taken from x-request-id.
request_headers_.addCopy("x-request-id", "000000ff-0000-0000-0000-000000000000");
EXPECT_CALL(context_.random_, random()).WillOnce(Return(42));
EXPECT_CALL(runtime_.snapshot_, featureEnabled("access_log.test_key", 5, 42, 1000000))
EXPECT_CALL(runtime_.snapshot_, sampleFeatureEnabled("access_log.test_key", 5, 42, 1000000))
.WillOnce(Return(true));
EXPECT_CALL(*file_, write(_));
log->log(&request_headers_, &response_headers_, &response_trailers_, request_info_);

EXPECT_CALL(context_.random_, random()).WillOnce(Return(43));
EXPECT_CALL(runtime_.snapshot_, featureEnabled("access_log.test_key", 5, 43, 1000000))
EXPECT_CALL(runtime_.snapshot_, sampleFeatureEnabled("access_log.test_key", 5, 43, 1000000))
.WillOnce(Return(false));
EXPECT_CALL(*file_, write(_)).Times(0);
log->log(&request_headers_, &response_headers_, &response_trailers_, request_info_);
Expand Down
6 changes: 3 additions & 3 deletions test/common/http/conn_manager_utility_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -891,7 +891,7 @@ TEST_F(ConnectionManagerUtilityTest, NonTlsAlwaysForwardClientCert) {

// Sampling, global on.
TEST_F(ConnectionManagerUtilityTest, RandomSamplingWhenGlobalSet) {
EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.random_sampling", 10000, _, 10000))
EXPECT_CALL(runtime_.snapshot_, sampleFeatureEnabled("tracing.random_sampling", 10000, _, 10000))
.WillOnce(Return(true));
EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _))
.WillOnce(Return(true));
Expand All @@ -905,7 +905,7 @@ TEST_F(ConnectionManagerUtilityTest, RandomSamplingWhenGlobalSet) {

// Sampling must not be done on client traced.
TEST_F(ConnectionManagerUtilityTest, SamplingMustNotBeDoneOnClientTraced) {
EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.random_sampling", 10000, _, 10000))
EXPECT_CALL(runtime_.snapshot_, sampleFeatureEnabled("tracing.random_sampling", 10000, _, 10000))
.Times(0);
EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _))
.WillOnce(Return(true));
Expand All @@ -920,7 +920,7 @@ TEST_F(ConnectionManagerUtilityTest, SamplingMustNotBeDoneOnClientTraced) {

// Sampling, global off.
TEST_F(ConnectionManagerUtilityTest, NoTraceWhenSamplingSetButGlobalNotSet) {
EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.random_sampling", 10000, _, 10000))
EXPECT_CALL(runtime_.snapshot_, sampleFeatureEnabled("tracing.random_sampling", 10000, _, 10000))
.WillOnce(Return(true));
EXPECT_CALL(runtime_.snapshot_, featureEnabled("tracing.global_enabled", 100, _))
.WillOnce(Return(false));
Expand Down
Loading