Skip to content

Commit 4a884b5

Browse files
authored
fix(config visibility): fix system test related to reporting of trace sampler and span sampler (#272)
* Report trace sampler * Report span sampler values * answer to comment, merge instead of passing by ref * Add more asserts in unit tests * answer to comments * rename telemetry_configs to metadata and remove old metadata
1 parent ef6d81e commit 4a884b5

13 files changed

Lines changed: 148 additions & 113 deletions

include/datadog/config.h

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,12 @@ struct ConfigMetadata {
6363
};
6464

6565
// Returns the final configuration value using the following
66-
// precedence order: environment > user code > default, and populates two maps:
67-
// 1. `telemetry_configs`: Records ALL configuration sources that were provided,
68-
// ordered from lowest to highest precedence.
69-
// 2. `metadata`: Records ONLY the winning configuration value (highest
70-
// precedence). Template Parameters:
66+
// precedence order: environment > user code > default, and populates metadata:
67+
// `metadata`: Records ALL configuration sources that were provided,
68+
// ordered from lowest to highest precedence. The last entry has the highest
69+
// precedence and is the winning value.
70+
//
71+
// Template Parameters:
7172
// Value: The type of the configuration value
7273
// Stringifier: Optional function type to convert Value to string
7374
// (defaults to std::nullptr_t, which uses string construction)
@@ -76,10 +77,8 @@ struct ConfigMetadata {
7677
// Parameters:
7778
// from_env: Optional value from environment variables (highest precedence)
7879
// from_user: Optional value from user code (middle precedence)
79-
// telemetry_configs: Output map that will be populated with all config
80-
// sources found for this config_name, in precedence order
81-
// metadata: Output map that will be populated with the winning config value
82-
// for this config_name
80+
// metadata: Output map that will be populated with all config sources found
81+
// for this config_name, in precedence order (last = highest)
8382
// config_name: The configuration parameter name identifier
8483
// fallback: Optional default value (lowest precedence). Pass nullptr to
8584
// indicate no default.
@@ -94,9 +93,7 @@ template <typename Value, typename Stringifier = std::nullptr_t,
9493
typename DefaultValue = std::nullptr_t>
9594
Value resolve_and_record_config(
9695
const Optional<Value>& from_env, const Optional<Value>& from_user,
97-
std::unordered_map<ConfigName, std::vector<ConfigMetadata>>*
98-
telemetry_configs,
99-
std::unordered_map<ConfigName, ConfigMetadata>* metadata,
96+
std::unordered_map<ConfigName, std::vector<ConfigMetadata>>* metadata,
10097
ConfigName config_name, DefaultValue fallback = nullptr,
10198
Stringifier to_string_fn = nullptr) {
10299
auto stringify = [&](const Value& v) -> std::string {
@@ -111,13 +108,12 @@ Value resolve_and_record_config(
111108
}
112109
};
113110

114-
std::vector<ConfigMetadata> telemetry_entries;
111+
std::vector<ConfigMetadata> metadata_entries;
115112
Optional<Value> chosen_value;
116113

117114
auto add_entry = [&](ConfigMetadata::Origin origin, const Value& val) {
118115
std::string val_str = stringify(val);
119-
telemetry_entries.emplace_back(
120-
ConfigMetadata{config_name, val_str, origin});
116+
metadata_entries.emplace_back(ConfigMetadata{config_name, val_str, origin});
121117
chosen_value = val;
122118
};
123119

@@ -134,9 +130,8 @@ Value resolve_and_record_config(
134130
add_entry(ConfigMetadata::Origin::ENVIRONMENT_VARIABLE, *from_env);
135131
}
136132

137-
(*telemetry_configs)[config_name] = std::move(telemetry_entries);
138-
if (!(*telemetry_configs)[config_name].empty()) {
139-
(*metadata)[config_name] = (*telemetry_configs)[config_name].back();
133+
if (!metadata_entries.empty()) {
134+
(*metadata)[config_name] = std::move(metadata_entries);
140135
}
141136

142137
return chosen_value.value_or(Value{});

include/datadog/datadog_agent_config.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ class FinalizedDatadogAgentConfig {
8585
std::chrono::steady_clock::duration request_timeout;
8686
std::chrono::steady_clock::duration shutdown_timeout;
8787
std::chrono::steady_clock::duration remote_configuration_poll_interval;
88-
std::unordered_map<ConfigName, ConfigMetadata> metadata;
88+
std::unordered_map<ConfigName, std::vector<ConfigMetadata>> metadata;
8989

9090
// Origin detection
9191
Optional<std::string> admission_controller_uid;

include/datadog/span_sampler_config.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ class FinalizedSpanSamplerConfig {
4949
};
5050

5151
std::vector<Rule> rules;
52-
std::unordered_map<ConfigName, ConfigMetadata> metadata;
52+
std::unordered_map<ConfigName, std::vector<ConfigMetadata>> metadata;
5353
};
5454

5555
Expected<FinalizedSpanSamplerConfig> finalize_config(const SpanSamplerConfig&,

include/datadog/trace_sampler_config.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ class FinalizedTraceSamplerConfig {
5050
public:
5151
double max_per_second;
5252
std::vector<TraceSamplerRule> rules;
53-
std::unordered_map<ConfigName, ConfigMetadata> metadata;
53+
std::unordered_map<ConfigName, std::vector<ConfigMetadata>> metadata;
5454

5555
public:
5656
/// Returns the trace sampler configuration when APM Tracing is disabled.

include/datadog/tracer_config.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ class FinalizedTracerConfig final {
229229
std::string integration_name;
230230
std::string integration_version;
231231
bool report_traces;
232-
std::unordered_map<ConfigName, ConfigMetadata> metadata;
232+
std::unordered_map<ConfigName, std::vector<ConfigMetadata>> metadata;
233233
Baggage::Options baggage_opts;
234234
HTTPClient::URL agent_url;
235235
std::shared_ptr<EventScheduler> event_scheduler;

src/datadog/config_manager.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,12 +287,18 @@ namespace rc = datadog::remote_config;
287287

288288
ConfigManager::ConfigManager(const FinalizedTracerConfig& config)
289289
: clock_(config.clock),
290-
default_metadata_(config.metadata),
291290
trace_sampler_(
292291
std::make_shared<TraceSampler>(config.trace_sampler, clock_)),
293292
rules_(config.trace_sampler.rules),
294293
span_defaults_(std::make_shared<SpanDefaults>(config.defaults)),
295-
report_traces_(config.report_traces) {}
294+
report_traces_(config.report_traces) {
295+
// Extract winning value (last entry) from each config's metadata history
296+
for (const auto& [name, metadata_vec] : config.metadata) {
297+
if (!metadata_vec.empty()) {
298+
default_metadata_[name] = metadata_vec.back();
299+
}
300+
}
301+
}
296302

297303
rc::Products ConfigManager::get_products() { return rc::product::APM_TRACING; }
298304

src/datadog/datadog_agent_config.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,8 @@ Expected<FinalizedDatadogAgentConfig> finalize_config(
142142
return std::move(*error);
143143
}
144144
result.url = *parsed_url;
145-
result.metadata[ConfigName::AGENT_URL] =
146-
ConfigMetadata(ConfigName::AGENT_URL, url, origin);
145+
result.metadata[ConfigName::AGENT_URL] = {
146+
ConfigMetadata(ConfigName::AGENT_URL, url, origin)};
147147

148148
// Starting Datadog Agent 7.62.0, the admission controller inject a unique
149149
// identifier through `DD_EXTERNAL_ENV`. This uid is used for origin

src/datadog/span_sampler_config.cpp

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -228,20 +228,21 @@ Expected<FinalizedSpanSamplerConfig> finalize_config(
228228
}
229229

230230
FinalizedSpanSamplerConfig result;
231-
232-
std::vector<SpanSamplerConfig::Rule> rules;
231+
Optional<std::vector<SpanSamplerConfig::Rule>> env_rules;
232+
Optional<std::vector<SpanSamplerConfig::Rule>> user_rules;
233233
if (!env_config->rules.empty()) {
234-
rules = env_config->rules;
235-
result.metadata[ConfigName::SPAN_SAMPLING_RULES] =
236-
ConfigMetadata(ConfigName::SPAN_SAMPLING_RULES, to_string(rules),
237-
ConfigMetadata::Origin::ENVIRONMENT_VARIABLE);
238-
} else if (!user_config.rules.empty()) {
239-
rules = user_config.rules;
240-
result.metadata[ConfigName::SPAN_SAMPLING_RULES] =
241-
ConfigMetadata(ConfigName::SPAN_SAMPLING_RULES, to_string(rules),
242-
ConfigMetadata::Origin::CODE);
234+
env_rules = env_config->rules;
235+
}
236+
if (!user_config.rules.empty()) {
237+
user_rules = user_config.rules;
243238
}
244239

240+
std::vector<SpanSamplerConfig::Rule> rules = resolve_and_record_config(
241+
env_rules, user_rules, &result.metadata, ConfigName::SPAN_SAMPLING_RULES,
242+
nullptr, [](const std::vector<SpanSamplerConfig::Rule> &r) {
243+
return to_string(r);
244+
});
245+
245246
for (const auto &rule : rules) {
246247
auto maybe_rate = Rate::from(rule.sample_rate);
247248
if (auto *error = maybe_rate.if_error()) {
@@ -276,7 +277,6 @@ Expected<FinalizedSpanSamplerConfig> finalize_config(
276277
finalized.max_per_second = rule.max_per_second;
277278
result.rules.push_back(std::move(finalized));
278279
}
279-
280280
return result;
281281
}
282282

src/datadog/trace_sampler_config.cpp

Lines changed: 12 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -162,14 +162,14 @@ Expected<FinalizedTraceSamplerConfig> finalize_config(
162162

163163
if (!env_config->rules.empty()) {
164164
rules = std::move(env_config->rules);
165-
result.metadata[ConfigName::TRACE_SAMPLING_RULES] =
165+
result.metadata[ConfigName::TRACE_SAMPLING_RULES] = {
166166
ConfigMetadata(ConfigName::TRACE_SAMPLING_RULES, to_string(rules),
167-
ConfigMetadata::Origin::ENVIRONMENT_VARIABLE);
167+
ConfigMetadata::Origin::ENVIRONMENT_VARIABLE)};
168168
} else if (!config.rules.empty()) {
169169
rules = std::move(config.rules);
170-
result.metadata[ConfigName::TRACE_SAMPLING_RULES] =
170+
result.metadata[ConfigName::TRACE_SAMPLING_RULES] = {
171171
ConfigMetadata(ConfigName::TRACE_SAMPLING_RULES, to_string(rules),
172-
ConfigMetadata::Origin::CODE);
172+
ConfigMetadata::Origin::CODE)};
173173
}
174174

175175
for (const auto &rule : rules) {
@@ -191,27 +191,16 @@ Expected<FinalizedTraceSamplerConfig> finalize_config(
191191
result.rules.emplace_back(std::move(finalized_rule));
192192
}
193193

194-
Optional<double> sample_rate;
195-
if (env_config->sample_rate) {
196-
sample_rate = env_config->sample_rate;
197-
result.metadata[ConfigName::TRACE_SAMPLING_RATE] = ConfigMetadata(
198-
ConfigName::TRACE_SAMPLING_RATE, to_string(*sample_rate, 1),
199-
ConfigMetadata::Origin::ENVIRONMENT_VARIABLE);
200-
} else if (config.sample_rate) {
201-
sample_rate = config.sample_rate;
202-
result.metadata[ConfigName::TRACE_SAMPLING_RATE] = ConfigMetadata(
203-
ConfigName::TRACE_SAMPLING_RATE, to_string(*sample_rate, 1),
204-
ConfigMetadata::Origin::CODE);
205-
} else {
206-
result.metadata[ConfigName::TRACE_SAMPLING_RATE] =
207-
ConfigMetadata(ConfigName::TRACE_SAMPLING_RATE, "1.0",
208-
ConfigMetadata::Origin::DEFAULT);
209-
}
194+
Optional<double> sample_rate = resolve_and_record_config(
195+
env_config->sample_rate, config.sample_rate, &result.metadata,
196+
ConfigName::TRACE_SAMPLING_RATE, 1.0,
197+
[](const double &d) { return to_string(d, 1); });
210198

199+
bool is_sample_rate_provided = env_config->sample_rate || config.sample_rate;
211200
// If `sample_rate` was specified, then it translates to a "catch-all" rule
212201
// appended to the end of `rules`. First, though, we have to make sure the
213202
// sample rate is valid.
214-
if (sample_rate) {
203+
if (sample_rate && is_sample_rate_provided) {
215204
auto maybe_rate = Rate::from(*sample_rate);
216205
if (auto *error = maybe_rate.if_error()) {
217206
return error->with_prefix(
@@ -225,11 +214,9 @@ Expected<FinalizedTraceSamplerConfig> finalize_config(
225214
result.rules.emplace_back(std::move(finalized_rule));
226215
}
227216

228-
std::unordered_map<ConfigName, std::vector<ConfigMetadata>>
229-
telemetry_configs_tmp;
230217
double max_per_second = resolve_and_record_config(
231-
env_config->max_per_second, config.max_per_second, &telemetry_configs_tmp,
232-
&result.metadata, ConfigName::TRACE_SAMPLING_LIMIT, 100.0,
218+
env_config->max_per_second, config.max_per_second, &result.metadata,
219+
ConfigName::TRACE_SAMPLING_LIMIT, 100.0,
233220
[](const double &d) { return std::to_string(d); });
234221

235222
const auto allowed_types = {FP_NORMAL, FP_SUBNORMAL};

0 commit comments

Comments
 (0)