Skip to content

Commit fd712d3

Browse files
therealak12therealak12
committed
replace double with fractional percent
Signed-off-by: therealak12 <[email protected]>
1 parent 8bd8607 commit fd712d3

File tree

11 files changed

+87
-60
lines changed

11 files changed

+87
-60
lines changed

‎api/envoy/extensions/tracers/opentelemetry/samplers/v3/BUILD

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ licenses(["notice"]) # Apache 2
77
api_proto_package(
88
deps = [
99
"//envoy/config/core/v3:pkg",
10+
"//envoy/type/v3:pkg",
1011
"@com_github_cncf_xds//udpa/annotations:pkg",
1112
"@com_github_cncf_xds//xds/annotations/v3:pkg",
1213
"@com_github_cncf_xds//xds/type/v3:pkg",

‎api/envoy/extensions/tracers/opentelemetry/samplers/v3/trace_id_ratio_based_sampler.proto

+5-4
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ syntax = "proto3";
22

33
package envoy.extensions.tracers.opentelemetry.samplers.v3;
44

5+
import "envoy/type/v3/percent.proto";
6+
57
import "udpa/annotations/status.proto";
68

79
option java_package = "io.envoyproxy.envoy.extensions.tracers.opentelemetry.samplers.v3";
@@ -20,10 +22,9 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
2022
// [#extension: envoy.tracers.opentelemetry.samplers.trace_id_ratio_based]
2123

2224
message TraceIdRatioBasedSamplerConfig {
23-
// This is a required value in the [0, 1] interval, If the given trace_id
24-
// falls into a given ratio of all possible trace_id values, ShouldSample will
25-
// return RECORD_AND_SAMPLE.
25+
// If the given trace_id falls into a given percentage of all possible
26+
// trace_id values, ShouldSample will return RECORD_AND_SAMPLE.
2627
// required
2728
// [#extension-category: envoy.tracers.opentelemetry.samplers]
28-
double ratio = 1;
29+
type.v3.FractionalPercent sampling_percentage = 2;
2930
}

‎source/extensions/tracers/opentelemetry/samplers/trace_id_ratio_based/trace_id_ratio_based_sampler.cc

+13-31
Original file line numberDiff line numberDiff line change
@@ -13,30 +13,14 @@
1313
#include "source/extensions/tracers/opentelemetry/span_context.h"
1414

1515
namespace {
16-
/**
17-
* Converts a ratio in [0, 1] to a threshold in [0, MILLION].
18-
*/
19-
uint64_t ratioToThreshold(double ratio) noexcept {
20-
const uint64_t MAX_VALUE = Envoy::ProtobufPercentHelper::fractionalPercentDenominatorToInt(
21-
envoy::type::v3::FractionalPercent::MILLION);
22-
23-
if (ratio <= 0.0) {
24-
return 0;
25-
}
26-
if (ratio >= 1.0) {
27-
return MAX_VALUE;
28-
}
29-
30-
return static_cast<uint64_t>(ratio * static_cast<double>(MAX_VALUE));
31-
}
3216

3317
/**
3418
* @param trace_id a required value to be converted to uint64_t. trace_id must
3519
* at least 8 bytes long. trace_id is expected to be a valid hex string.
36-
* @return Returns the uint64 value associated with first 8 bytes of the trace_id modulo MILLION.
20+
* @return Returns the uint64 value associated with first 8 bytes of the trace_id.
3721
*
3822
*/
39-
uint64_t calculateThresholdFromBuffer(const std::string& trace_id) noexcept {
23+
uint64_t traceIdToUint64(const std::string& trace_id) noexcept {
4024
uint8_t buffer[8] = {0};
4125
for (size_t i = 0; i < 8; ++i) {
4226
std::string byte_string = trace_id.substr(i * 2, 2);
@@ -46,8 +30,7 @@ uint64_t calculateThresholdFromBuffer(const std::string& trace_id) noexcept {
4630
uint64_t first_8_bytes = 0;
4731
Envoy::safeMemcpyUnsafeSrc(&first_8_bytes, buffer);
4832

49-
return first_8_bytes % Envoy::ProtobufPercentHelper::fractionalPercentDenominatorToInt(
50-
envoy::type::v3::FractionalPercent::MILLION);
33+
return first_8_bytes;
5134
}
5235
} // namespace
5336

@@ -60,15 +43,13 @@ TraceIdRatioBasedSampler::TraceIdRatioBasedSampler(
6043
const envoy::extensions::tracers::opentelemetry::samplers::v3::TraceIdRatioBasedSamplerConfig&
6144
config,
6245
Server::Configuration::TracerFactoryContext& /*context*/)
63-
: threshold_(ratioToThreshold(config.ratio())) {
64-
double ratio = config.ratio();
65-
if (ratio > 1.0) {
66-
ratio = 1.0;
67-
}
68-
if (ratio < 0.0) {
69-
ratio = 0.0;
70-
}
71-
description_ = "TraceIdRatioBasedSampler{" + std::to_string(ratio) + "}";
46+
: sampling_percentage_(config.sampling_percentage()) {
47+
const envoy::type::v3::FractionalPercent& sampling_percentage = config.sampling_percentage();
48+
description_ = "TraceIdRatioBasedSampler{" + std::to_string(sampling_percentage.numerator()) +
49+
"/" +
50+
std::to_string(ProtobufPercentHelper::fractionalPercentDenominatorToInt(
51+
sampling_percentage.denominator())) +
52+
"}";
7253
}
7354

7455
SamplingResult TraceIdRatioBasedSampler::shouldSample(
@@ -82,11 +63,12 @@ SamplingResult TraceIdRatioBasedSampler::shouldSample(
8263
result.tracestate = parent_context.value().tracestate();
8364
}
8465

85-
if (threshold_ == 0) {
66+
if (sampling_percentage_.numerator() == 0) {
8667
return result;
8768
}
8869

89-
if (calculateThresholdFromBuffer(trace_id) <= threshold_) {
70+
if (ProtobufPercentHelper::evaluateFractionalPercent(sampling_percentage_,
71+
traceIdToUint64(trace_id))) {
9072
result.decision = Decision::RecordAndSample;
9173
return result;
9274
}

‎source/extensions/tracers/opentelemetry/samplers/trace_id_ratio_based/trace_id_ratio_based_sampler.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
#include "envoy/extensions/tracers/opentelemetry/samplers/v3/trace_id_ratio_based_sampler.pb.h"
66
#include "envoy/server/factory_context.h"
7+
#include "envoy/type/v3/percent.pb.h"
78

89
#include "source/common/common/logger.h"
910
#include "source/extensions/tracers/opentelemetry/samplers/sampler.h"
@@ -33,7 +34,7 @@ class TraceIdRatioBasedSampler : public Sampler, Logger::Loggable<Logger::Id::tr
3334

3435
private:
3536
std::string description_;
36-
const uint64_t threshold_;
37+
const envoy::type::v3::FractionalPercent sampling_percentage_;
3738
};
3839

3940
} // namespace OpenTelemetry

‎test/extensions/tracers/opentelemetry/samplers/parent_based/config_test.cc

+3-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@ TEST(ParentBasedSamplerFactoryTest, Test) {
2929
name: envoy.tracers.opentelemetry.samplers.trace_id_ratio_based
3030
typed_config:
3131
"@type": type.googleapis.com/envoy.extensions.tracers.opentelemetry.samplers.v3.TraceIdRatioBasedSamplerConfig
32-
ratio: "0.002"
32+
sampling_percentage:
33+
numerator: 20
34+
denominator: TEN_TOUSAND
3335
)EOF";
3436
TestUtility::loadFromYaml(yaml, typed_config);
3537
NiceMock<Server::Configuration::MockTracerFactoryContext> context;

‎test/extensions/tracers/opentelemetry/samplers/parent_based/parent_based_sampler_integration_test.cc

+3-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,9 @@ class ParentBasedSamplerIntegrationTest
4343
name: envoy.tracers.opentelemetry.samplers.trace_id_ratio_based
4444
typed_config:
4545
"@type": type.googleapis.com/envoy.extensions.tracers.opentelemetry.samplers.v3.TraceIdRatioBasedSamplerConfig
46-
ratio: 0.5
46+
sampling_percentage:
47+
numerator: 50
48+
denominator: HUNDRED
4749
)EOF";
4850

4951
auto tracing_config =

‎test/extensions/tracers/opentelemetry/samplers/parent_based/parent_based_sampler_test.cc

+22-4
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ namespace Extensions {
1818
namespace Tracers {
1919
namespace OpenTelemetry {
2020

21+
const auto percentage_denominator = envoy::type::v3::FractionalPercent::MILLION;
22+
2123
// // Verify the ShouldSample function
2224
TEST(ParentBasedSamplerTest, TestShouldSample) {
2325
// Case 1: Parent doesn't exist -> Return result of delegateSampler()
@@ -30,7 +32,11 @@ TEST(ParentBasedSamplerTest, TestShouldSample) {
3032
std::srand(std::time(nullptr));
3133

3234
for (int i = 0; i < 100; ++i) {
33-
trace_id_ratio_based_config.set_ratio(static_cast<double>(std::rand()) / RAND_MAX);
35+
uint64_t numerator = std::rand() % ProtobufPercentHelper::fractionalPercentDenominatorToInt(
36+
percentage_denominator);
37+
trace_id_ratio_based_config.mutable_sampling_percentage()->set_denominator(
38+
percentage_denominator);
39+
trace_id_ratio_based_config.mutable_sampling_percentage()->set_numerator(numerator);
3440
auto wrapped_sampler =
3541
std::make_shared<TraceIdRatioBasedSampler>(trace_id_ratio_based_config, context);
3642

@@ -56,7 +62,11 @@ TEST(ParentBasedSamplerTest, TestShouldSample) {
5662

5763
// Case 2: Parent exists and SampledFlag is true -> Return RecordAndSample.
5864
for (int i = 0; i < 10; ++i) {
59-
trace_id_ratio_based_config.set_ratio(static_cast<double>(std::rand()) / RAND_MAX);
65+
uint64_t numerator = std::rand() % ProtobufPercentHelper::fractionalPercentDenominatorToInt(
66+
percentage_denominator);
67+
trace_id_ratio_based_config.mutable_sampling_percentage()->set_denominator(
68+
percentage_denominator);
69+
trace_id_ratio_based_config.mutable_sampling_percentage()->set_numerator(numerator);
6070
auto wrapped_sampler =
6171
std::make_shared<TraceIdRatioBasedSampler>(trace_id_ratio_based_config, context);
6272

@@ -79,7 +89,11 @@ TEST(ParentBasedSamplerTest, TestShouldSample) {
7989

8090
// Case 3: Parent exists and SampledFlag is false -> Return Drop.
8191
for (int i = 0; i < 10; ++i) {
82-
trace_id_ratio_based_config.set_ratio(static_cast<double>(std::rand()) / RAND_MAX);
92+
uint64_t numerator = std::rand() % ProtobufPercentHelper::fractionalPercentDenominatorToInt(
93+
percentage_denominator);
94+
trace_id_ratio_based_config.mutable_sampling_percentage()->set_denominator(
95+
percentage_denominator);
96+
trace_id_ratio_based_config.mutable_sampling_percentage()->set_numerator(numerator);
8397
auto wrapped_sampler =
8498
std::make_shared<TraceIdRatioBasedSampler>(trace_id_ratio_based_config, context);
8599

@@ -108,7 +122,11 @@ TEST(ParentBasedSamplerTest, TestGetDescriptionAndContext) {
108122
envoy::extensions::tracers::opentelemetry::samplers::v3::TraceIdRatioBasedSamplerConfig
109123
trace_id_ratio_based_config;
110124
std::srand(std::time(nullptr));
111-
trace_id_ratio_based_config.set_ratio(static_cast<double>(std::rand()) / RAND_MAX);
125+
uint64_t numerator = std::rand() % ProtobufPercentHelper::fractionalPercentDenominatorToInt(
126+
percentage_denominator);
127+
trace_id_ratio_based_config.mutable_sampling_percentage()->set_denominator(
128+
percentage_denominator);
129+
trace_id_ratio_based_config.mutable_sampling_percentage()->set_numerator(numerator);
112130
auto wrapped_sampler =
113131
std::make_shared<TraceIdRatioBasedSampler>(trace_id_ratio_based_config, context);
114132

‎test/extensions/tracers/opentelemetry/samplers/trace_id_ratio_based/BUILD

+1
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ envoy_extension_cc_test(
3737
"//test/mocks/server:tracer_factory_context_mocks",
3838
"//test/test_common:utility_lib",
3939
"@envoy_api//envoy/extensions/tracers/opentelemetry/samplers/v3:pkg_cc_proto",
40+
"@envoy_api//envoy/type/v3:pkg_cc_proto",
4041
],
4142
)
4243

‎test/extensions/tracers/opentelemetry/samplers/trace_id_ratio_based/config_test.cc

+3-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@ TEST(TraceIdRatioBasedSamplerFactoryTest, Test) {
2626
name: envoy.tracers.opentelemetry.samplers.trace_id_ratio_based
2727
typed_config:
2828
"@type": type.googleapis.com/envoy.extensions.tracers.opentelemetry.samplers.v3.TraceIdRatioBasedSamplerConfig
29-
ratio: 0.0002
29+
sampling_percentage:
30+
numerator: 2
31+
denominator: TEN_THOUSAND
3032
)EOF";
3133
TestUtility::loadFromYaml(yaml, typed_config);
3234
NiceMock<Server::Configuration::MockTracerFactoryContext> context;

‎test/extensions/tracers/opentelemetry/samplers/trace_id_ratio_based/trace_id_ratio_based_sampler_integration_test.cc

+3-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@ class TraceIdRatioBasedSamplerIntegrationTest
4040
name: envoy.tracers.opentelemetry.samplers.trace_id_ratio_based
4141
typed_config:
4242
"@type": type.googleapis.com/envoy.extensions.tracers.opentelemetry.samplers.v3.TraceIdRatioBasedSamplerConfig
43-
ratio: 0.002
43+
sampling_percentage:
44+
numerator: 20
45+
denominator: TEN_THOUSAND
4446
)EOF";
4547

4648
auto tracing_config =

‎test/extensions/tracers/opentelemetry/samplers/trace_id_ratio_based/trace_id_ratio_based_sampler_test.cc

+31-16
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#include <string>
33

44
#include "envoy/extensions/tracers/opentelemetry/samplers/v3/trace_id_ratio_based_sampler.pb.h"
5+
#include "envoy/type/v3/percent.pb.h"
56

67
#include "source/common/common/random_generator.h"
78
#include "source/extensions/tracers/opentelemetry/samplers/trace_id_ratio_based/trace_id_ratio_based_sampler.h"
@@ -17,6 +18,8 @@ namespace Extensions {
1718
namespace Tracers {
1819
namespace OpenTelemetry {
1920

21+
const auto percentage_denominator = envoy::type::v3::FractionalPercent::MILLION;
22+
2023
// As per the docs: https://opentelemetry.io/docs/specs/otel/trace/sdk/#traceidratiobased
2124
// > A TraceIDRatioBased sampler with a given sampling rate MUST also sample
2225
// all traces that any TraceIDRatioBased sampler with a lower sampling rate
@@ -27,19 +30,24 @@ TEST(TraceIdRatioBasedSamplerTest, TestTraceIdRatioSamplesInclusively) {
2730

2831
std::srand(std::time(nullptr));
2932
for (int i = 0; i < 100; ++i) {
30-
double ratio_low = static_cast<double>(std::rand()) / RAND_MAX;
31-
double ratio_high = static_cast<double>(std::rand()) / RAND_MAX;
32-
if (ratio_low > ratio_high) {
33-
double holder = ratio_low;
34-
ratio_low = ratio_high;
35-
ratio_high = holder;
33+
uint64_t numerator_low = std::rand() % ProtobufPercentHelper::fractionalPercentDenominatorToInt(
34+
percentage_denominator);
35+
uint64_t numerator_high =
36+
std::rand() %
37+
ProtobufPercentHelper::fractionalPercentDenominatorToInt(percentage_denominator);
38+
if (numerator_low > numerator_high) {
39+
double holder = numerator_low;
40+
numerator_low = numerator_high;
41+
numerator_high = holder;
3642
}
3743
envoy::extensions::tracers::opentelemetry::samplers::v3::TraceIdRatioBasedSamplerConfig
3844
config_low;
3945
envoy::extensions::tracers::opentelemetry::samplers::v3::TraceIdRatioBasedSamplerConfig
4046
config_high;
41-
config_low.set_ratio(ratio_low);
42-
config_high.set_ratio(ratio_high);
47+
config_low.mutable_sampling_percentage()->set_denominator(percentage_denominator);
48+
config_low.mutable_sampling_percentage()->set_numerator(numerator_low);
49+
config_high.mutable_sampling_percentage()->set_denominator(percentage_denominator);
50+
config_high.mutable_sampling_percentage()->set_numerator(numerator_high);
4351
auto sampler_low = std::make_shared<TraceIdRatioBasedSampler>(config_low, context);
4452
auto sampler_high = std::make_shared<TraceIdRatioBasedSampler>(config_high, context);
4553

@@ -72,7 +80,8 @@ TEST(TraceIdRatioBasedSamplerTest, TestSpecialRatios) {
7280
std::srand(std::time(nullptr));
7381

7482
// ratio = 0, should never sample
75-
config.set_ratio(0);
83+
config.mutable_sampling_percentage()->set_denominator(percentage_denominator);
84+
config.mutable_sampling_percentage()->set_numerator(0);
7685
auto sampler = std::make_shared<TraceIdRatioBasedSampler>(config, context);
7786

7887
for (int i = 0; i < 10; ++i) {
@@ -86,7 +95,7 @@ TEST(TraceIdRatioBasedSamplerTest, TestSpecialRatios) {
8695
}
8796

8897
// ratio < 0, should never sample
89-
config.set_ratio(-5);
98+
config.mutable_sampling_percentage()->set_numerator(-5);
9099
sampler = std::make_shared<TraceIdRatioBasedSampler>(config, context);
91100

92101
for (int i = 0; i < 10; ++i) {
@@ -100,7 +109,8 @@ TEST(TraceIdRatioBasedSamplerTest, TestSpecialRatios) {
100109
}
101110

102111
// ratio = 1, should always sample
103-
config.set_ratio(1);
112+
config.mutable_sampling_percentage()->set_numerator(
113+
ProtobufPercentHelper::fractionalPercentDenominatorToInt(percentage_denominator));
104114
sampler = std::make_shared<TraceIdRatioBasedSampler>(config, context);
105115

106116
for (int i = 0; i < 10; ++i) {
@@ -113,8 +123,9 @@ TEST(TraceIdRatioBasedSamplerTest, TestSpecialRatios) {
113123
EXPECT_EQ(sampling_result.decision, Decision::RecordAndSample);
114124
}
115125

116-
// ratio < 0, should never sample
117-
config.set_ratio(7);
126+
// ratio > 1, should always sample
127+
config.mutable_sampling_percentage()->set_numerator(
128+
7 * ProtobufPercentHelper::fractionalPercentDenominatorToInt(percentage_denominator));
118129
sampler = std::make_shared<TraceIdRatioBasedSampler>(config, context);
119130

120131
for (int i = 0; i < 10; ++i) {
@@ -132,17 +143,21 @@ TEST(TraceIdRatioBasedSamplerTest, TestTraceIdRatioDescription) {
132143
envoy::extensions::tracers::opentelemetry::samplers::v3::TraceIdRatioBasedSamplerConfig config;
133144
NiceMock<Server::Configuration::MockTracerFactoryContext> context;
134145
NiceMock<StreamInfo::MockStreamInfo> info;
135-
config.set_ratio(0.0157);
146+
config.mutable_sampling_percentage()->set_denominator(percentage_denominator);
147+
config.mutable_sampling_percentage()->set_numerator(157);
136148
auto sampler = std::make_shared<TraceIdRatioBasedSampler>(config, context);
137-
EXPECT_STREQ(sampler->getDescription().c_str(), "TraceIdRatioBasedSampler{0.015700}");
149+
EXPECT_STREQ(sampler->getDescription().c_str(), "TraceIdRatioBasedSampler{157/1000000}");
138150
}
139151

140152
TEST(TraceIdRatioBasedSamplerTest, TestTraceIdRatioAttrs) {
141153
envoy::extensions::tracers::opentelemetry::samplers::v3::TraceIdRatioBasedSamplerConfig config;
142154
NiceMock<Server::Configuration::MockTracerFactoryContext> context;
143155
NiceMock<StreamInfo::MockStreamInfo> info;
144156
std::srand(std::time(nullptr));
145-
config.set_ratio(static_cast<double>(std::rand()) / RAND_MAX);
157+
uint64_t numerator = std::rand() % ProtobufPercentHelper::fractionalPercentDenominatorToInt(
158+
percentage_denominator);
159+
config.mutable_sampling_percentage()->set_denominator(percentage_denominator);
160+
config.mutable_sampling_percentage()->set_numerator(numerator);
146161
auto sampler = std::make_shared<TraceIdRatioBasedSampler>(config, context);
147162
SpanContext parent_context("0", "12345", "45678", true, "random_key=random_value");
148163
auto sampling_result = sampler->shouldSample(

0 commit comments

Comments
 (0)