Redis proxy traffic mirroring runtime default

[kb000]

MirrorPolicyImpl::shouldMirror will override a default runtime fraction of 0, and instead mirror 100% of the time.

envoy/source/extensions/filters/network/redis_proxy/router_impl.cc

Lines 17 to 32 in 6ca8500

	bool MirrorPolicyImpl::shouldMirror(const std::string& command) const {
	if (!upstream_) {
	return false;
	}
	if (exclude_read_commands_ && Common::Redis::SupportedCommands::writeCommands().find(command) ==
	Common::Redis::SupportedCommands::writeCommands().end()) {
	return false;
	}
	if (default_value_.numerator() > 0) {
	return runtime_.snapshot().featureEnabled(runtime_key_, default_value_);
	}
	return true;
	}

This is at odds with documentation:

envoy/api/envoy/api/v2/route/route.proto

Lines 612 to 617 in 6ca8500

	// If not specified, all requests to the target cluster will be mirrored. If
	// specified, Envoy will lookup the runtime key to get the % of requests to
	// mirror. Valid values are from 0 to 10000, allowing for increments of
	// 0.01% of requests to be mirrored. If the runtime key is specified in the
	// configuration but not present in runtime, 0 is the default and thus 0% of
	// requests will be mirrored.

    // If not specified, all requests to the target cluster will be mirrored. If
    // specified, Envoy will lookup the runtime key to get the % of requests to
    // mirror. Valid values are from 0 to 10000, allowing for increments of
    // 0.01% of requests to be mirrored. If the runtime key is specified in the
    // configuration but not present in runtime, 0 is the default and thus 0% of
    // requests will be mirrored.

And also inconsistent with the behavior of http router traffic mirroring:

envoy/source/common/router/config_impl.cc

Lines 180 to 185 in eff0201

	if (config.request_mirror_policy().has_runtime_fraction()) {
	runtime_key_ = config.request_mirror_policy().runtime_fraction().runtime_key();
	default_value_ = config.request_mirror_policy().runtime_fraction().default_value();
	} else {
	runtime_key_ = config.request_mirror_policy().runtime_key();
	default_value_.set_numerator(0);

[kb000]

@HenryYYang

[zuercher]

Looks like it completely ignores the deprecated runtime_key field and only heeds runtime_fraction if the default numerator is > 0.