Skip to content

Commit

Permalink
Use const ptr to avoid copy, add test for default sampling values and…
Browse files Browse the repository at this point in the history 
… change route tracing sampling defaults to 100%

Signed-off-by: Gary Brown <gary@brownuk.com>
  • Loading branch information
@objectiser
objectiser committed May 26, 2019
1 parent 69a0441 commit 2e19997
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 29 deletions.
19 changes: 10 additions & 9 deletions source/common/http/conn_manager_utility.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,30 +234,31 @@ void ConnectionManagerUtility::mutateTracingRequestHeader(HeaderMap& request_hea
return;
}

envoy::type::FractionalPercent client_sampling = config.tracingConfig()->client_sampling_;
envoy::type::FractionalPercent random_sampling = config.tracingConfig()->random_sampling_;
envoy::type::FractionalPercent overall_sampling = config.tracingConfig()->overall_sampling_;
const envoy::type::FractionalPercent* client_sampling = &config.tracingConfig()->client_sampling_;
const envoy::type::FractionalPercent* random_sampling = &config.tracingConfig()->random_sampling_;
const envoy::type::FractionalPercent* overall_sampling =
&config.tracingConfig()->overall_sampling_;

if (route && route->tracingConfig()) {
client_sampling = route->tracingConfig()->getClientSampling();
random_sampling = route->tracingConfig()->getRandomSampling();
overall_sampling = route->tracingConfig()->getOverallSampling();
client_sampling = &route->tracingConfig()->getClientSampling();
random_sampling = &route->tracingConfig()->getRandomSampling();
overall_sampling = &route->tracingConfig()->getOverallSampling();
}

// Do not apply tracing transformations if we are currently tracing.
if (UuidTraceStatus::NoTrace == UuidUtils::isTraceableUuid(x_request_id)) {
if (request_headers.ClientTraceId() &&
runtime.snapshot().featureEnabled("tracing.client_enabled", client_sampling)) {
runtime.snapshot().featureEnabled("tracing.client_enabled", *client_sampling)) {
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", random_sampling,
} else if (runtime.snapshot().featureEnabled("tracing.random_sampling", *random_sampling,
result)) {
UuidUtils::setTraceableUuid(x_request_id, UuidTraceStatus::Sampled);
}
}

if (!runtime.snapshot().featureEnabled("tracing.global_enabled", overall_sampling, result)) {
if (!runtime.snapshot().featureEnabled("tracing.global_enabled", *overall_sampling, result)) {
UuidUtils::setTraceableUuid(x_request_id, UuidTraceStatus::NoTrace);
}

Expand Down
23 changes: 20 additions & 3 deletions source/common/router/config_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -324,9 +324,26 @@ void DecoratorImpl::apply(Tracing::Span& span) const {

const std::string& DecoratorImpl::getOperation() const { return operation_; }

RouteTracingImpl::RouteTracingImpl(const envoy::api::v2::route::Tracing& tracing)
: client_sampling_(tracing.client_sampling()), random_sampling_(tracing.random_sampling()),
overall_sampling_(tracing.overall_sampling()) {}
RouteTracingImpl::RouteTracingImpl(const envoy::api::v2::route::Tracing& tracing) {
if (!tracing.has_client_sampling()) {
client_sampling_.set_numerator(100);
client_sampling_.set_denominator(envoy::type::FractionalPercent::HUNDRED);
} else {
client_sampling_ = tracing.client_sampling();
}
if (!tracing.has_random_sampling()) {
random_sampling_.set_numerator(10000);
random_sampling_.set_denominator(envoy::type::FractionalPercent::TEN_THOUSAND);
} else {
random_sampling_ = tracing.random_sampling();
}
if (!tracing.has_overall_sampling()) {
overall_sampling_.set_numerator(100);
overall_sampling_.set_denominator(envoy::type::FractionalPercent::HUNDRED);
} else {
overall_sampling_ = tracing.overall_sampling();
}
}

const envoy::type::FractionalPercent& RouteTracingImpl::getClientSampling() const {
return client_sampling_;
Expand Down
6 changes: 3 additions & 3 deletions source/common/router/config_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,9 +364,9 @@ class RouteTracingImpl : public RouteTracing {
const envoy::type::FractionalPercent& getOverallSampling() const override;

private:
const envoy::type::FractionalPercent client_sampling_;
const envoy::type::FractionalPercent random_sampling_;
const envoy::type::FractionalPercent overall_sampling_;
envoy::type::FractionalPercent client_sampling_;
envoy::type::FractionalPercent random_sampling_;
envoy::type::FractionalPercent overall_sampling_;
};

/**
Expand Down
8 changes: 4 additions & 4 deletions source/extensions/filters/network/http_connection_manager/config.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,14 +233,14 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(

envoy::type::FractionalPercent client_sampling;
client_sampling.set_numerator(
PROTOBUF_PERCENT_TO_ROUNDED_INTEGER_OR_DEFAULT(tracing_config, client_sampling, 100, 100));
tracing_config.has_client_sampling() ? tracing_config.client_sampling().value() : 100);
envoy::type::FractionalPercent random_sampling;
random_sampling.set_numerator(PROTOBUF_PERCENT_TO_ROUNDED_INTEGER_OR_DEFAULT(
tracing_config, random_sampling, 10000, 10000));
random_sampling.set_numerator(
tracing_config.has_random_sampling() ? tracing_config.random_sampling().value() : 10000);
random_sampling.set_denominator(envoy::type::FractionalPercent::TEN_THOUSAND);
envoy::type::FractionalPercent overall_sampling;
overall_sampling.set_numerator(
PROTOBUF_PERCENT_TO_ROUNDED_INTEGER_OR_DEFAULT(tracing_config, overall_sampling, 100, 100));
tracing_config.has_overall_sampling() ? tracing_config.overall_sampling().value() : 100);

tracing_config_ =
std::make_unique<Http::TracingConnectionManagerConfig>(Http::TracingConnectionManagerConfig{
Expand Down
64 changes: 54 additions & 10 deletions test/common/router/config_impl_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3472,13 +3472,6 @@ TEST_F(RouteMatcherTest, WeightedClusters) {
metadata: { filter_metadata: { com.bar.foo: { baz: test_value }, baz: {name: meh} } }
decorator:
operation: hello
tracing:
client_sampling:
numerator: 1
random_sampling:
numerator: 2
overall_sampling:
numerator: 3
route:
weighted_clusters:
clusters:
Expand Down Expand Up @@ -3568,9 +3561,6 @@ TEST_F(RouteMatcherTest, WeightedClusters) {
EXPECT_EQ(nullptr, route_entry->typedMetadata().get<Foo>(baz_factory.name()));
EXPECT_EQ("meh", route_entry->typedMetadata().get<Baz>(baz_factory.name())->name);
EXPECT_EQ("hello", route->decorator()->getOperation());
EXPECT_EQ(1, route->tracingConfig()->getClientSampling().numerator());
EXPECT_EQ(2, route->tracingConfig()->getRandomSampling().numerator());
EXPECT_EQ(3, route->tracingConfig()->getOverallSampling().numerator());

Http::TestHeaderMapImpl response_headers;
StreamInfo::MockStreamInfo stream_info;
Expand Down Expand Up @@ -4828,6 +4818,60 @@ name: foo
EXPECT_EQ("foo", route_entry->virtualHost().routeConfig().name());
}

TEST_F(RouteConfigurationV2, RouteTracingConfig) {
const std::string yaml = R"EOF(
name: foo
virtual_hosts:
- name: bar
domains: ["*"]
routes:
- match: { regex: "/first" }
tracing:
client_sampling:
numerator: 1
route: { cluster: ww2 }
- match: { regex: "/second" }
tracing:
overall_sampling:
numerator: 1
route: { cluster: ww2 }
- match: { path: "/third" }
tracing:
client_sampling:
numerator: 1
random_sampling:
numerator: 200
denominator: 1
overall_sampling:
numerator: 3
route: { cluster: ww2 }
)EOF";
BazFactory baz_factory;
Registry::InjectFactory<HttpRouteTypedMetadataFactory> registered_factory(baz_factory);
const TestConfigImpl config(parseRouteConfigurationFromV2Yaml(yaml), factory_context_, true);

const auto route1 = config.route(genHeaders("www.foo.com", "/first", "GET"), 0);
const auto route2 = config.route(genHeaders("www.foo.com", "/second", "GET"), 0);
const auto route3 = config.route(genHeaders("www.foo.com", "/third", "GET"), 0);

// Check default values for random and overall sampling
EXPECT_EQ(10000, route1->tracingConfig()->getRandomSampling().numerator());
EXPECT_EQ(1, route1->tracingConfig()->getRandomSampling().denominator());
EXPECT_EQ(100, route1->tracingConfig()->getOverallSampling().numerator());
EXPECT_EQ(0, route1->tracingConfig()->getOverallSampling().denominator());

// Check default values for client sampling
EXPECT_EQ(100, route2->tracingConfig()->getClientSampling().numerator());
EXPECT_EQ(0, route2->tracingConfig()->getClientSampling().denominator());

EXPECT_EQ(1, route3->tracingConfig()->getClientSampling().numerator());
EXPECT_EQ(0, route3->tracingConfig()->getClientSampling().denominator());
EXPECT_EQ(200, route3->tracingConfig()->getRandomSampling().numerator());
EXPECT_EQ(1, route3->tracingConfig()->getRandomSampling().denominator());
EXPECT_EQ(3, route3->tracingConfig()->getOverallSampling().numerator());
EXPECT_EQ(0, route3->tracingConfig()->getOverallSampling().denominator());
}

// Test to check Prefix Rewrite for redirects
TEST_F(RouteConfigurationV2, RedirectPrefixRewrite) {
std::string RedirectPrefixRewrite = R"EOF(
Expand Down
60 changes: 60 additions & 0 deletions test/extensions/filters/network/http_connection_manager/config_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,66 @@ stat_prefix: router
EXPECT_EQ(5 * 60 * 1000, config.streamIdleTimeout().count());
}

TEST_F(HttpConnectionManagerConfigTest, SamplingDefault) {
const std::string yaml_string = R"EOF(
stat_prefix: ingress_http
internal_address_config:
unix_sockets: true
route_config:
name: local_route
tracing:
operation_name: ingress
http_filters:
- name: envoy.router
)EOF";

HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_,
date_provider_, route_config_provider_manager_);

EXPECT_EQ(100, config.tracingConfig()->client_sampling_.numerator());
EXPECT_EQ(envoy::type::FractionalPercent::HUNDRED,
config.tracingConfig()->client_sampling_.denominator());
EXPECT_EQ(10000, config.tracingConfig()->random_sampling_.numerator());
EXPECT_EQ(envoy::type::FractionalPercent::TEN_THOUSAND,
config.tracingConfig()->random_sampling_.denominator());
EXPECT_EQ(100, config.tracingConfig()->overall_sampling_.numerator());
EXPECT_EQ(envoy::type::FractionalPercent::HUNDRED,
config.tracingConfig()->overall_sampling_.denominator());
}

TEST_F(HttpConnectionManagerConfigTest, SamplingConfigured) {
const std::string yaml_string = R"EOF(
stat_prefix: ingress_http
internal_address_config:
unix_sockets: true
route_config:
name: local_route
tracing:
operation_name: ingress
client_sampling:
value: 1
random_sampling:
value: 2
overall_sampling:
value: 3
http_filters:
- name: envoy.router
)EOF";

HttpConnectionManagerConfig config(parseHttpConnectionManagerFromV2Yaml(yaml_string), context_,
date_provider_, route_config_provider_manager_);

EXPECT_EQ(1, config.tracingConfig()->client_sampling_.numerator());
EXPECT_EQ(envoy::type::FractionalPercent::HUNDRED,
config.tracingConfig()->client_sampling_.denominator());
EXPECT_EQ(2, config.tracingConfig()->random_sampling_.numerator());
EXPECT_EQ(envoy::type::FractionalPercent::TEN_THOUSAND,
config.tracingConfig()->random_sampling_.denominator());
EXPECT_EQ(3, config.tracingConfig()->overall_sampling_.numerator());
EXPECT_EQ(envoy::type::FractionalPercent::HUNDRED,
config.tracingConfig()->overall_sampling_.denominator());
}

TEST_F(HttpConnectionManagerConfigTest, UnixSocketInternalAddress) {
const std::string yaml_string = R"EOF(
stat_prefix: ingress_http
Expand Down

0 comments on commit 2e19997

Please sign in to comment.