Skip to content
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.

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

feat!: Change type of DD_TRACE_RATE_LIMIT to int #160

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
6 changes: 1 addition & 5 deletions src/datadog/span_sampler_config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -254,12 +254,8 @@ Expected<FinalizedSpanSamplerConfig> finalize_config(
return error->with_prefix(prefix);
}

const auto allowed_types = {FP_NORMAL, FP_SUBNORMAL};
if (rule.max_per_second &&
(!(*rule.max_per_second > 0) ||
std::find(std::begin(allowed_types), std::end(allowed_types),
std::fpclassify(*rule.max_per_second)) ==
std::end(allowed_types))) {
(!(*rule.max_per_second > 0))) {
std::string message;
message += "Span sampling rule with pattern ";
message += nlohmann::json(static_cast<SpanMatcher>(rule)).dump();
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
9 changes: 3 additions & 6 deletions 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 Expand Up @@ -224,14 +224,11 @@ Expected<FinalizedTraceSamplerConfig> finalize_config(
}

const auto [origin, max_per_second] =
pick(env_config->max_per_second, config.max_per_second, 200);
pick(env_config->max_per_second, config.max_per_second, 100);
result.metadata[ConfigName::TRACE_SAMPLING_LIMIT] = ConfigMetadata(
ConfigName::TRACE_SAMPLING_LIMIT, std::to_string(max_per_second), origin);

const auto allowed_types = {FP_NORMAL, FP_SUBNORMAL};
if (!(max_per_second > 0) ||
std::find(std::begin(allowed_types), std::end(allowed_types),
std::fpclassify(max_per_second)) == std::end(allowed_types)) {
if (!(max_per_second > 0)) {
std::string message;
message +=
"Trace sampling max_per_second must be greater than zero, but the "
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
32 changes: 17 additions & 15 deletions test/test_tracer_config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -707,16 +707,16 @@ TEST_CASE("TracerConfig::trace_sampler") {
}

SECTION("max_per_second") {
SECTION("defaults to 200") {
SECTION("defaults to 100") {
auto finalized = finalize_config(config);
REQUIRE(finalized);
REQUIRE(finalized->trace_sampler.max_per_second == 200);
REQUIRE(finalized->trace_sampler.max_per_second == 100);
}

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