Skip to content

Commit

Permalink
feat!: Implement consistency for DD_TRACE_RATE_LIMIT - Change field t…
Browse files Browse the repository at this point in the history
…o only accept ints instead of doubles, in alignment with the rest of the tracers

Issues: APMAPI-511
  • Loading branch information
zacharycmontoya committed Oct 3, 2024
1 parent f682d03 commit d059561
Show file tree
Hide file tree
Showing 10 changed files with 30 additions and 28 deletions.
2 changes: 1 addition & 1 deletion include/datadog/sampling_decision.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ struct SamplingDecision {
Optional<Rate> limiter_effective_rate;
// The per-second maximum allowed number of "keeps" configured for the limiter
// consulted in this decision, if any.
Optional<double> limiter_max_per_second;
Optional<int> limiter_max_per_second;
// The provenance of this decision.
Origin origin;
};
Expand Down
4 changes: 2 additions & 2 deletions include/datadog/span_sampler_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace tracing {
struct SpanSamplerConfig {
struct Rule : public SpanMatcher {
double sample_rate = 1.0;
Optional<double> max_per_second;
Optional<int> max_per_second;

Rule(const SpanMatcher&);
Rule() = default;
Expand All @@ -45,7 +45,7 @@ class FinalizedSpanSamplerConfig {
public:
struct Rule : public SpanMatcher {
Rate sample_rate;
Optional<double> max_per_second;
Optional<int> max_per_second;
};

std::vector<Rule> rules;
Expand Down
4 changes: 2 additions & 2 deletions include/datadog/trace_sampler_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ struct TraceSamplerConfig {

Optional<double> sample_rate;
std::vector<Rule> rules;
Optional<double> max_per_second;
Optional<int> max_per_second;
};

class FinalizedTraceSamplerConfig {
Expand All @@ -47,7 +47,7 @@ class FinalizedTraceSamplerConfig {
FinalizedTraceSamplerConfig() = default;

public:
double max_per_second;
int max_per_second;
std::vector<TraceSamplerRule> rules;
std::unordered_map<ConfigName, ConfigMetadata> metadata;
};
Expand Down
2 changes: 1 addition & 1 deletion src/datadog/span_sampler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace datadog {
namespace tracing {

SpanSampler::SynchronizedLimiter::SynchronizedLimiter(const Clock& clock,
double max_per_second)
int max_per_second)
: limiter(clock, max_per_second) {}

SpanSampler::Rule::Rule(const FinalizedSpanSamplerConfig::Rule& rule,
Expand Down
2 changes: 1 addition & 1 deletion src/datadog/span_sampler.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class SpanSampler {
std::mutex mutex;
Limiter limiter;

SynchronizedLimiter(const Clock&, double max_per_second);
SynchronizedLimiter(const Clock&, int max_per_second);
};

class Rule : public FinalizedSpanSamplerConfig::Rule {
Expand Down
2 changes: 1 addition & 1 deletion src/datadog/trace_sampler.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class TraceSampler {
std::unordered_map<std::string, Rate> collector_sample_rates_;
std::vector<TraceSamplerRule> rules_;
Limiter limiter_;
double limiter_max_per_second_;
int limiter_max_per_second_;

public:
TraceSampler(const FinalizedTraceSamplerConfig& config, const Clock& clock);
Expand Down
2 changes: 1 addition & 1 deletion src/datadog/trace_sampler_config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ Expected<TraceSamplerConfig> load_trace_sampler_env_config() {
}

if (auto limit_env = lookup(environment::DD_TRACE_RATE_LIMIT)) {
auto maybe_max_per_second = parse_double(*limit_env);
auto maybe_max_per_second = parse_int(*limit_env, 10);
if (auto *error = maybe_max_per_second.if_error()) {
std::string prefix;
prefix += "While parsing ";
Expand Down
4 changes: 2 additions & 2 deletions test/test_span_sampler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ namespace {
struct SpanSamplingTags {
Optional<double> mechanism;
Optional<double> rule_rate;
Optional<double> max_per_second;
Optional<int> max_per_second;
};

SpanSamplingTags span_sampling_tags(const SpanData& span) {
Expand Down Expand Up @@ -292,7 +292,7 @@ TEST_CASE("span rule limiter") {
struct TestCase {
std::string name;
std::size_t num_spans;
Optional<double> max_per_second;
Optional<int> max_per_second;
std::size_t expected_count;
};

Expand Down
8 changes: 4 additions & 4 deletions test/test_trace_sampler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,14 @@ TEST_CASE("trace sampling rate limiter") {
// second does not exceed that allowed by the configured limit.
struct TestCase {
std::string name;
double max_per_second;
int max_per_second;
std::size_t burst_size;
std::size_t expected_kept_count;
};

auto test_case = GENERATE(values<TestCase>({{"allow one", 1.0, 100, 1},
{"allow all", 100.0, 100, 100},
{"allow some", 10.0, 100, 10}}));
auto test_case = GENERATE(values<TestCase>({{"allow one", 1, 100, 1},
{"allow all", 100, 100, 100},
{"allow some", 10, 100, 10}}));

CAPTURE(test_case.name);
CAPTURE(test_case.max_per_second);
Expand Down
28 changes: 15 additions & 13 deletions test/test_tracer_config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -715,8 +715,8 @@ TEST_CASE("TracerConfig::trace_sampler") {

SECTION("must be >0 and a finite number") {
auto limit = GENERATE(0.0, -1.0, std::nan(""),
std::numeric_limits<double>::infinity(),
-std::numeric_limits<double>::infinity());
std::numeric_limits<int>::infinity(),
-std::numeric_limits<int>::infinity());

CAPTURE(limit);
CAPTURE(std::fpclassify(limit));
Expand Down Expand Up @@ -744,9 +744,11 @@ TEST_CASE("TracerConfig::trace_sampler") {
};

auto test_case = GENERATE(values<TestCase>({
{"nonsense", "nonsense", {Error::INVALID_DOUBLE}},
{"trailing space", "23 ", {Error::INVALID_DOUBLE}},
{"out of range of double", "123e9999999999", {Error::INVALID_DOUBLE}},
{"nonsense", "nonsense", {Error::INVALID_INTEGER}},
{"trailing space", "23 ", {Error::INVALID_INTEGER}},
{"out of range of double",
"123e9999999999",
{Error::INVALID_INTEGER}},
// Some C++ standard libraries parse "nan" and "inf" as the
// corresponding special floating point values. Other standard
// libraries consider "nan" and "inf" invalid.
Expand All @@ -755,17 +757,17 @@ TEST_CASE("TracerConfig::trace_sampler") {
// (0.0, Inf) allowed.
{"NaN",
"NaN",
{Error::INVALID_DOUBLE, Error::MAX_PER_SECOND_OUT_OF_RANGE}},
{Error::INVALID_INTEGER, Error::MAX_PER_SECOND_OUT_OF_RANGE}},
{"nan",
"nan",
{Error::INVALID_DOUBLE, Error::MAX_PER_SECOND_OUT_OF_RANGE}},
{Error::INVALID_INTEGER, Error::MAX_PER_SECOND_OUT_OF_RANGE}},
{"inf",
"inf",
{Error::INVALID_DOUBLE, Error::MAX_PER_SECOND_OUT_OF_RANGE}},
{Error::INVALID_INTEGER, Error::MAX_PER_SECOND_OUT_OF_RANGE}},
{"Inf",
"Inf",
{Error::INVALID_DOUBLE, Error::MAX_PER_SECOND_OUT_OF_RANGE}},
{"below range", "-0.1", {Error::MAX_PER_SECOND_OUT_OF_RANGE}},
{Error::INVALID_INTEGER, Error::MAX_PER_SECOND_OUT_OF_RANGE}},
{"below range", "-0.1", {Error::INVALID_INTEGER}},
{"zero (also below range)",
"0",
{Error::MAX_PER_SECOND_OUT_OF_RANGE}},
Expand Down Expand Up @@ -926,7 +928,7 @@ TEST_CASE("TracerConfig::span_sampler") {
"TracerConfig::span_sampler.rules") {
SpanSamplerConfig::Rule config_rule;
config_rule.service = "foosvc";
config_rule.max_per_second = 9.2;
config_rule.max_per_second = 9;
config.span_sampler.rules.push_back(config_rule);

auto rules_json = R"json([
Expand Down Expand Up @@ -982,7 +984,7 @@ TEST_CASE("TracerConfig::span_sampler") {
Error::RULE_WRONG_TYPE},
{"sample_rate must be a number", R"json([{"sample_rate": true}])json",
Error::SPAN_SAMPLING_RULES_SAMPLE_RATE_WRONG_TYPE},
{"max_per_second must be a number (or absent)",
{"max_per_second must be an integer (or absent)",
R"json([{"max_per_second": false}])json",
Error::SPAN_SAMPLING_RULES_MAX_PER_SECOND_WRONG_TYPE},
{"no unknown properties", R"json([{"extension": "denied!"}])json",
Expand All @@ -1005,7 +1007,7 @@ TEST_CASE("TracerConfig::span_sampler") {
// This rule will be overridden.
SpanSamplerConfig::Rule config_rule;
config_rule.service = "foosvc";
config_rule.max_per_second = 9.2;
config_rule.max_per_second = 9;
config.span_sampler.rules.push_back(config_rule);

auto rules_file_json = R"json([
Expand Down

0 comments on commit d059561

Please sign in to comment.