Skip to content

Commit 8e11e46

Browse files
dmehalacataphract
dmehala
authored and
cataphract
committed
move logic to config_manager
1 parent f25e15e commit 8e11e46

File tree

7 files changed

+51
-97
lines changed

7 files changed

+51
-97
lines changed

‎src/datadog/config_manager.cpp‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ ConfigManager::Update parse_dynamic_config(const nlohmann::json& j) {
109109
namespace rc = datadog::remote_config;
110110

111111
ConfigManager::ConfigManager(const FinalizedTracerConfig& config,
112+
const TracerSignature& tracer_signature,
112113
const std::shared_ptr<TracerTelemetry>& telemetry)
113114
: clock_(config.clock),
114115
default_metadata_(config.metadata),
@@ -117,6 +118,7 @@ ConfigManager::ConfigManager(const FinalizedTracerConfig& config,
117118
rules_(config.trace_sampler.rules),
118119
span_defaults_(std::make_shared<SpanDefaults>(config.defaults)),
119120
report_traces_(config.report_traces),
121+
tracer_signature_(tracer_signature),
120122
telemetry_(telemetry) {}
121123

122124
rc::Products ConfigManager::get_products() { return rc::product::APM_TRACING; }
@@ -128,7 +130,20 @@ rc::Capabilities ConfigManager::get_capabilities() {
128130
}
129131

130132
Optional<std::string> ConfigManager::on_update(const Configuration& config) {
133+
if (config.product != rc::product::Flag::APM_TRACING) {
134+
return nullopt;
135+
}
136+
131137
const auto config_json = nlohmann::json::parse(config.content);
138+
139+
const auto& targeted_service = config_json.at("service_target");
140+
if (targeted_service.at("service").get<StringView>() !=
141+
tracer_signature_.default_service ||
142+
targeted_service.at("env").get<StringView>() !=
143+
tracer_signature_.default_environment) {
144+
return "Wrong service targeted";
145+
}
146+
132147
auto config_update = parse_dynamic_config(config_json.at("lib_config"));
133148

134149
auto config_metadata = apply_update(config_update);

‎src/datadog/config_manager.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ class ConfigManager : public remote_config::Listener {
7676
DynamicConfig<std::shared_ptr<const SpanDefaults>> span_defaults_;
7777
DynamicConfig<bool> report_traces_;
7878

79+
const TracerSignature& tracer_signature_;
7980
std::shared_ptr<TracerTelemetry> telemetry_;
8081

8182
private:
@@ -85,6 +86,7 @@ class ConfigManager : public remote_config::Listener {
8586

8687
public:
8788
ConfigManager(const FinalizedTracerConfig& config,
89+
const TracerSignature& signature,
8890
const std::shared_ptr<TracerTelemetry>& telemetry);
8991
~ConfigManager() override{};
9092

‎src/datadog/remote_config/remote_config.cpp‎

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,12 @@ constexpr std::array<uint8_t, sizeof(uint64_t)> capabilities_byte_array(
3030
return res;
3131
}
3232

33-
struct ConfigKeyData {
33+
struct ConfigKeyMetadata final {
3434
product::Flag product;
3535
StringView config_id;
3636
};
3737

38-
Optional<ConfigKeyData> parse_config_path(StringView config_path) {
38+
Optional<ConfigKeyMetadata> parse_config_path(StringView config_path) {
3939
static const std::regex path_reg(
4040
"^(datadog/\\d+|employee)/([^/]+)/([^/]+)/[^/]+$");
4141

@@ -206,16 +206,16 @@ void Manager::process_response(const nlohmann::json& json) {
206206
auto config_path = client_config.get<StringView>();
207207
visited_config.emplace(config_path);
208208

209-
const auto config_key_data = parse_config_path(config_path);
210-
if (!config_key_data) {
209+
const auto config_key_metadata = parse_config_path(config_path);
210+
if (!config_key_metadata) {
211211
std::string reason{config_path};
212212
reason += " is an invalid configuration path";
213213

214214
error(reason);
215215
return;
216216
}
217217

218-
const auto product = config_key_data->product;
218+
const auto product = config_key_metadata->product;
219219

220220
const auto& config_metadata =
221221
targets.at("/signed/targets"_json_pointer).at(config_path);
@@ -244,30 +244,13 @@ void Manager::process_response(const nlohmann::json& json) {
244244
auto decoded_config = base64_decode(raw_data);
245245

246246
Configuration new_config;
247-
auto&& config_id = config_key_data->config_id;
248-
new_config.id = std::string{config_id.data(), config_id.size()};
247+
new_config.id = std::string{config_key_metadata->config_id};
249248
new_config.path = std::string{config_path};
250249
new_config.hash = config_metadata.at("/hashes/sha256"_json_pointer);
251250
new_config.content = std::move(decoded_config);
252251
new_config.version = config_metadata.at("/custom/v"_json_pointer);
253252
new_config.product = product;
254253

255-
// TODO: should be moved to the listener
256-
if (product == product::Flag::APM_TRACING) {
257-
const auto config_json = nlohmann::json::parse(new_config.content);
258-
new_config.version = config_json.at("revision");
259-
260-
const auto& targeted_service = config_json.at("service_target");
261-
if (targeted_service.at("service").get<StringView>() !=
262-
tracer_signature_.default_service ||
263-
targeted_service.at("env").get<StringView>() !=
264-
tracer_signature_.default_environment) {
265-
new_config.state = Configuration::State::error;
266-
new_config.error_message = "Wrong service targeted";
267-
goto skip_listeners;
268-
}
269-
}
270-
271254
for (const auto& listener : listeners_per_product_[product]) {
272255
// Q: Two listeners on the same product. What should be the behaviour
273256
// if one of the listeners report an error?
@@ -281,7 +264,6 @@ void Manager::process_response(const nlohmann::json& json) {
281264
}
282265
}
283266

284-
skip_listeners:
285267
applied_config_[std::string{config_path}] = new_config;
286268
}
287269

‎src/datadog/tracer.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@
3030
namespace datadog {
3131
namespace tracing {
3232

33+
void to_json(nlohmann::json& j, const PropagationStyle& style) {
34+
j = to_string_view(style);
35+
}
36+
3337
Tracer::Tracer(const FinalizedTracerConfig& config)
3438
: Tracer(config, default_id_generator(config.generate_128bit_trace_ids)) {}
3539

@@ -43,8 +47,8 @@ Tracer::Tracer(const FinalizedTracerConfig& config,
4347
tracer_telemetry_(std::make_shared<TracerTelemetry>(
4448
config.report_telemetry, config.clock, logger_, signature_,
4549
config.integration_name, config.integration_version)),
46-
config_manager_(
47-
std::make_shared<ConfigManager>(config, tracer_telemetry_)),
50+
config_manager_(std::make_shared<ConfigManager>(config, signature_,
51+
tracer_telemetry_)),
4852
collector_(/* see constructor body */),
4953
span_sampler_(
5054
std::make_shared<SpanSampler>(config.span_sampler, config.clock)),
@@ -83,10 +87,6 @@ Tracer::Tracer(const FinalizedTracerConfig& config,
8387
}
8488
}
8589

86-
void to_json(nlohmann::json& j, const PropagationStyle& style) {
87-
j = to_string_view(style);
88-
}
89-
9090
std::string Tracer::config() const {
9191
// clang-format off
9292
auto config = nlohmann::json::object({

‎test/remote_config/test_remote_config.cpp‎

Lines changed: 0 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -190,71 +190,6 @@ REMOTE_CONFIG_TEST("response processing") {
190190
CHECK(payload.contains("/client/state/error"_json_pointer) == true);
191191
}
192192

193-
SECTION(
194-
"configuration updates targetting the wrong tracer reports an error") {
195-
// clang-format off
196-
auto test_case = GENERATE(values<std::string>({
197-
// "service_target": { "service": "not-testsvc", "env": "test" }
198-
R"({
199-
"targets":
200-
"ewogICAgInNpZ25lZCI6IHsKICAgICAgICAiY3VzdG9tIjogewogICAgICAgICAgICAiYWdlbnRfcmVmcmVzaF9pbnRlcnZhbCI6IDUsCiAgICAgICAgICAgICJvcGFxdWVfYmFja2VuZF9zdGF0ZSI6ICJleUoyWlhKemFXOXVJam95TENKemRHRjBaU0k2ZXlKbWFXeGxYMmhoYzJobGN5STZleUprWVhSaFpHOW5MekV3TURBeE1qVTROREF2UVZCTlgxUlNRVU5KVGtjdk9ESTNaV0ZqWmpoa1ltTXpZV0l4TkRNMFpETXlNV05pT0RGa1ptSm1OMkZtWlRZMU5HRTBZall4TVRGalpqRTJOakJpTnpGalkyWTRPVGM0TVRrek9DOHlPVEE0Tm1Ka1ltVTFNRFpsTmpoaU5UQm1NekExTlRneU0yRXpaR0UxWTJVd05USTRaakUyTkRCa05USmpaamc0TmpFNE1UWmhZV0U1Wm1ObFlXWTBJanBiSW05WVpESnBlVU16ZUM5b1JXc3hlWFZoWTFoR04xbHFjWEpwVGs5QldVdHVaekZ0V0UwMU5WWktUSGM5SWwxOWZYMD0iCiAgICAgICAgfSwKICAgICAgICAic3BlY192ZXJzaW9uIjogIjEuMC4wIiwKICAgICAgICAidGFyZ2V0cyI6IHsKICAgICAgICAgICAgImVtcGxveWVlL0FQTV9UUkFDSU5HL3Rlc3RfcmNfd3JvbmdfdGFyZ2V0L3NlcnZpY2VfbmFtZSI6IHsKICAgICAgICAgICAgICAgICJoYXNoZXMiOiB7CiAgICAgICAgICAgICAgICAgICAgInNoYTI1NiI6ICJhMTc3NzY4YjIwYjdjN2Y4NDQ5MzVjYWU2OWM1YzVlZDg4ZWFhZTIzNGUwMTgyYTc4MzU5OTczMzllNTUyNGJjIgogICAgICAgICAgICAgICAgfSwKICAgICAgICAgICAgICAgICJsZW5ndGgiOiAzNzQsCgkJCQkiY3VzdG9tIjogeyAidiI6IDEyMyB9CiAgICAgICAgICAgIH0KICAgICAgICB9LAogICAgICAgICJ2ZXJzaW9uIjogNjYyMDQzMjAKICAgIH0KfQo=",
201-
"client_configs": ["employee/APM_TRACING/test_rc_wrong_target/service_name"],
202-
"target_files": [
203-
{
204-
"path": "employee/APM_TRACING/test_rc_wrong_target/service_name",
205-
"raw":
206-
"eyAiaWQiOiAiODI3ZWFjZjhkYmMzYWIxNDM0ZDMyMWNiODFkZmJmN2FmZTY1NGE0YjYxMTFjZjE2NjBiNzFjY2Y4OTc4MTkzOCIsICJyZXZpc2lvbiI6IDE2OTgxNjcxMjYwNjQsICJzY2hlbWFfdmVyc2lvbiI6ICJ2MS4wLjAiLCAiYWN0aW9uIjogImVuYWJsZSIsICJsaWJfY29uZmlnIjogeyAibGlicmFyeV9sYW5ndWFnZSI6ICJhbGwiLCAibGlicmFyeV92ZXJzaW9uIjogImxhdGVzdCIsICJzZXJ2aWNlX25hbWUiOiAidGVzdHN2YyIsICJlbnYiOiAidGVzdCIsICJ0cmFjaW5nX2VuYWJsZWQiOiB0cnVlLCAidHJhY2luZ19zYW1wbGluZ19yYXRlIjogMC42IH0sICJzZXJ2aWNlX3RhcmdldCI6IHsgInNlcnZpY2UiOiAibm90LXRlc3RzdmMiLCAiZW52IjogInRlc3QiIH0gfQ=="
207-
}
208-
]
209-
})",
210-
// "service_target": { "service": "testsvc", "env": "dev" }
211-
R"({
212-
"targets":
213-
"ewogICAgInNpZ25lZCI6IHsKICAgICAgICAiY3VzdG9tIjogewogICAgICAgICAgICAiYWdlbnRfcmVmcmVzaF9pbnRlcnZhbCI6IDUsCiAgICAgICAgICAgICJvcGFxdWVfYmFja2VuZF9zdGF0ZSI6ICJleUoyWlhKemFXOXVJam95TENKemRHRjBaU0k2ZXlKbWFXeGxYMmhoYzJobGN5STZleUprWVhSaFpHOW5MekV3TURBeE1qVTROREF2UVZCTlgxUlNRVU5KVGtjdk9ESTNaV0ZqWmpoa1ltTXpZV0l4TkRNMFpETXlNV05pT0RGa1ptSm1OMkZtWlRZMU5HRTBZall4TVRGalpqRTJOakJpTnpGalkyWTRPVGM0TVRrek9DOHlPVEE0Tm1Ka1ltVTFNRFpsTmpoaU5UQm1NekExTlRneU0yRXpaR0UxWTJVd05USTRaakUyTkRCa05USmpaamc0TmpFNE1UWmhZV0U1Wm1ObFlXWTBJanBiSW05WVpESnBlVU16ZUM5b1JXc3hlWFZoWTFoR04xbHFjWEpwVGs5QldVdHVaekZ0V0UwMU5WWktUSGM5SWwxOWZYMD0iCiAgICAgICAgfSwKICAgICAgICAic3BlY192ZXJzaW9uIjogIjEuMC4wIiwKICAgICAgICAidGFyZ2V0cyI6IHsKICAgICAgICAgICAgImVtcGxveWVlL0FQTV9UUkFDSU5HL3Rlc3RfcmNfd3JvbmdfdGFyZ2V0L2Vudl9uYW1lIjogewogICAgICAgICAgICAgICAgImhhc2hlcyI6IHsKICAgICAgICAgICAgICAgICAgICAic2hhMjU2IjogImExNzc3NjhiMjBiN2M3Zjg0NDkzNWNhZTY5YzVjNWVkODhlYWFlMjM0ZTAxODJhNzgzNTk5NzMzOWU1NTI0YmMiCiAgICAgICAgICAgICAgICB9LAogICAgICAgICAgICAgICAgImxlbmd0aCI6IDM3NCwKCQkJCSJjdXN0b20iOiB7ICJ2IjogMTI0IH0KICAgICAgICAgICAgfQogICAgICAgIH0sCiAgICAgICAgInZlcnNpb24iOiA2NjIwNDMyMAogICAgfQp9Cg==",
214-
"client_configs": ["employee/APM_TRACING/test_rc_wrong_target/env_name"],
215-
"target_files": [
216-
{
217-
"path": "employee/APM_TRACING/test_rc_wrong_target/env_name",
218-
"raw":
219-
"eyAiaWQiOiAiODI3ZWFjZjhkYmMzYWIxNDM0ZDMyMWNiODFkZmJmN2FmZTY1NGE0YjYxMTFjZjE2NjBiNzFjY2Y4OTc4MTkzOCIsICJyZXZpc2lvbiI6IDE2OTgxNjcxMjYwNjQsICJzY2hlbWFfdmVyc2lvbiI6ICJ2MS4wLjAiLCAiYWN0aW9uIjogImVuYWJsZSIsICJsaWJfY29uZmlnIjogeyAibGlicmFyeV9sYW5ndWFnZSI6ICJhbGwiLCAibGlicmFyeV92ZXJzaW9uIjogImxhdGVzdCIsICJzZXJ2aWNlX25hbWUiOiAidGVzdHN2YyIsICJlbnYiOiAidGVzdCIsICJ0cmFjaW5nX2VuYWJsZWQiOiB0cnVlLCAidHJhY2luZ19zYW1wbGluZ19yYXRlIjogMC42IH0sICJzZXJ2aWNlX3RhcmdldCI6IHsgInNlcnZpY2UiOiAidGVzdHN2YyIsICJlbnYiOiAiZGV2IiB9IH0="
220-
}
221-
]
222-
})"
223-
}));
224-
// clang-format on
225-
226-
CAPTURE(test_case);
227-
228-
const auto response_json =
229-
nlohmann::json::parse(/* input = */ test_case,
230-
/* parser_callback = */ nullptr,
231-
/* allow_exceptions = */ false);
232-
233-
REQUIRE(!response_json.is_discarded());
234-
235-
auto tracing_listener = std::make_shared<FakeListener>();
236-
tracing_listener->products = rc::product::APM_TRACING;
237-
238-
rc::Manager rc(tracer_signature, {tracing_listener}, logger);
239-
rc.process_response(response_json);
240-
241-
CHECK(tracing_listener->count_on_update == 0);
242-
CHECK(tracing_listener->count_on_revert == 0);
243-
CHECK(tracing_listener->count_on_post_process == 1);
244-
245-
// Verify next request set the config status
246-
const auto payload = rc.make_request_payload();
247-
REQUIRE(payload.contains("/client/state/config_states"_json_pointer) ==
248-
true);
249-
250-
constexpr auto error_state = 3;
251-
const auto& config_states =
252-
payload.at("/client/state/config_states"_json_pointer);
253-
REQUIRE(config_states.size() == 1);
254-
CHECK(config_states[0]["product"] == "APM_TRACING");
255-
CHECK(config_states[0]["apply_state"] == error_state);
256-
}
257-
258193
SECTION("update dispatch") {
259194
// Verify configuration updates are dispatched to the correct listener
260195
std::string_view rc_response = R"({

‎test/test_config_manager.cpp‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,33 @@ CONFIG_MANAGER_TEST("remote configuration handling") {
2929
auto tracer_telemetry = std::make_shared<TracerTelemetry>(
3030
false, default_clock, nullptr, tracer_signature, "", "");
3131

32-
ConfigManager config_manager(*finalize_config(config), tracer_telemetry);
32+
ConfigManager config_manager(*finalize_config(config), tracer_signature,
33+
tracer_telemetry);
3334

3435
rc::Listener::Configuration config_update{/* id = */ "id",
3536
/* path = */ "",
3637
/* content = */ "",
3738
/* version = */ 1,
3839
rc::product::Flag::APM_TRACING};
3940

41+
SECTION(
42+
"configuration updates targetting the wrong tracer reports an error") {
43+
// clang-format off
44+
auto test_case = GENERATE(values<std::string>({
45+
R"({ "service_target": { "service": "not-testsvc", "env": "test" } })",
46+
R"({ "service_target": { "service": "testsvc", "env": "not-test" } })"
47+
}));
48+
// clang-format on
49+
50+
CAPTURE(test_case);
51+
52+
config_update.content = test_case;
53+
54+
// TODO: targetting wrong procut -> error
55+
const auto err = config_manager.on_update(config_update);
56+
CHECK(err);
57+
}
58+
4059
SECTION("handling of `tracing_sampling_rate`") {
4160
// SECTION("invalid value") {
4261
// config_update.content = R"({

‎test/test_datadog_agent.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,8 @@ TEST_CASE("Remote Configuration", "[datadog_agent]") {
200200
finalized->report_telemetry, finalized->clock, finalized->logger,
201201
signature, "", "");
202202

203-
auto config_manager = std::make_shared<ConfigManager>(*finalized, telemetry);
203+
auto config_manager =
204+
std::make_shared<ConfigManager>(*finalized, signature, telemetry);
204205

205206
const auto& agent_config =
206207
std::get<FinalizedDatadogAgentConfig>(finalized->collector);

0 commit comments

Comments
 (0)