refactor: Remote Configuration (#130)

Refactor the Remote Configuration (RC) as a standalone module, enabling
its use independently of the tracer. Additionally, ensure that external
listeners can register and receive configuration updates when RC is
embedded in the tracer.

Changes:
  - Move APM_TRACING updates handling to the configuration manager.
  - Expose the `remote_configuration_listeners` field in datadog agent config
    to allow registration of external listeners.
  - Improve the robustness of config path parsing and ensure an error is reported
    if parsing fails.
  - Add `to_upper` utility function.
  - Increase test coverage for RC and the new configuration manager
    behaviour.

Author: dmehala

BUILD.bazel

@@ -27,7 +27,7 @@ cc_library(
     "src/datadog/propagation_style.cpp",
     "src/datadog/random.cpp",
     "src/datadog/rate.cpp",
-    "src/datadog/remote_config.cpp",
+    "src/datadog/remote_config/remote_config.cpp",
     "src/datadog/runtime_id.cpp",
     "src/datadog/span.cpp",
     "src/datadog/span_data.cpp",
@@ -55,7 +55,6 @@ cc_library(
     "src/datadog/config.h",
     "src/datadog/clock.h",
     "src/datadog/config_manager.h",
-    "src/datadog/config_update.h",
     "src/datadog/collector.h",
     "src/datadog/collector_response.h",
 #     "src/datadog/curl.h", no libcurl
@@ -88,7 +87,10 @@ cc_library(
     "src/datadog/propagation_style.h",
     "src/datadog/random.h",
     "src/datadog/rate.h",
-    "src/datadog/remote_config.h",
+    "src/datadog/remote_config/capability.h",
+    "src/datadog/remote_config/listener.h",
+    "src/datadog/remote_config/product.h",
+    "src/datadog/remote_config/remote_config.h",
     "src/datadog/runtime_id.h",
     "src/datadog/sampling_decision.h",
     "src/datadog/sampling_mechanism.h",
@@ -118,6 +120,7 @@ cc_library(
     "src/datadog/w3c_propagation.h",
     ],
     strip_include_prefix = "src/",
+    copts = ["-Isrc/datadog"],
     visibility = ["//visibility:public"],
     deps = [
         "@com_google_absl//absl/strings",

CMakeLists.txt

@@ -103,7 +103,7 @@ target_sources(dd_trace_cpp-objects
     src/datadog/propagation_style.cpp
     src/datadog/random.cpp
     src/datadog/rate.cpp
-    src/datadog/remote_config.cpp
+    src/datadog/remote_config/remote_config.cpp
     src/datadog/runtime_id.cpp
     src/datadog/span.cpp
     src/datadog/span_data.cpp
@@ -137,7 +137,6 @@ target_sources(dd_trace_cpp-objects PUBLIC
   src/datadog/cerr_logger.h
   src/datadog/clock.h
   src/datadog/config_manager.h
-  src/datadog/config_update.h
   src/datadog/collector.h
   src/datadog/collector_response.h
   # src/datadog/curl.h except for curl.h
@@ -170,7 +169,9 @@ target_sources(dd_trace_cpp-objects PUBLIC
   src/datadog/propagation_style.h
   src/datadog/random.h
   src/datadog/rate.h
-  src/datadog/remote_config.h
+  src/datadog/remote_config/remote_config.h
+  src/datadog/remote_config/capability.h
+  src/datadog/remote_config/listener.h
   src/datadog/runtime_id.h
   src/datadog/sampling_decision.h
   src/datadog/sampling_mechanism.h
@@ -204,12 +205,11 @@ target_sources(dd_trace_cpp-objects PUBLIC
 # or installing the library.
 target_include_directories(dd_trace_cpp-objects
   PUBLIC
-    $<INSTALL_INTERFACE:src>
+    $<INSTALL_INTERFACE:src/datadog>
     $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/src>
+    $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/src/datadog>
 )
 
-add_dependencies(dd_trace_cpp-objects Threads::Threads)
-
 target_link_libraries(dd_trace_cpp-objects
   PUBLIC
     Threads::Threads

src/datadog/config_manager.cpp

@@ -77,16 +77,71 @@ Expected<Rules> parse_trace_sampling_rules(const nlohmann::json& json_rules) {
   return parsed_rules;
 }
 
+ConfigManager::Update parse_dynamic_config(const nlohmann::json& j) {
+  ConfigManager::Update config_update;
+
+  if (auto sampling_rate_it = j.find("tracing_sampling_rate");
+      sampling_rate_it != j.cend() && sampling_rate_it->is_number()) {
+    config_update.trace_sampling_rate = sampling_rate_it->get<double>();
+  }
+
+  if (auto tags_it = j.find("tracing_tags");
+      tags_it != j.cend() && tags_it->is_array()) {
+    config_update.tags = tags_it->get<std::vector<StringView>>();
+  }
+
+  if (auto tracing_enabled_it = j.find("tracing_enabled");
+      tracing_enabled_it != j.cend() && tracing_enabled_it->is_boolean()) {
+    config_update.report_traces = tracing_enabled_it->get<bool>();
+  }
+
+  if (auto tracing_sampling_rules_it = j.find("tracing_sampling_rules");
+      tracing_sampling_rules_it != j.cend() &&
+      tracing_sampling_rules_it->is_array()) {
+    config_update.trace_sampling_rules = &(*tracing_sampling_rules_it);
+  }
+
+  return config_update;
+}
+
 }  // namespace
 
-ConfigManager::ConfigManager(const FinalizedTracerConfig& config)
+namespace rc = datadog::remote_config;
+
+ConfigManager::ConfigManager(const FinalizedTracerConfig& config,
+                             const std::shared_ptr<TracerTelemetry>& telemetry)
     : clock_(config.clock),
       default_metadata_(config.metadata),
       trace_sampler_(
           std::make_shared<TraceSampler>(config.trace_sampler, clock_)),
       rules_(config.trace_sampler.rules),
       span_defaults_(std::make_shared<SpanDefaults>(config.defaults)),
-      report_traces_(config.report_traces) {}
+      report_traces_(config.report_traces),
+      telemetry_(telemetry) {}
+
+rc::Products ConfigManager::get_products() { return rc::product::APM_TRACING; }
+
+rc::Capabilities ConfigManager::get_capabilities() {
+  using namespace rc::capability;
+  return APM_TRACING_SAMPLE_RATE | APM_TRACING_TAGS | APM_TRACING_ENABLED |
+         APM_TRACING_SAMPLE_RULES;
+}
+
+Optional<std::string> ConfigManager::on_update(const Configuration& config) {
+  const auto config_json = nlohmann::json::parse(config.content);
+  auto config_update = parse_dynamic_config(config_json.at("lib_config"));
+
+  auto config_metadata = apply_update(config_update);
+  telemetry_->capture_configuration_change(config_metadata);
+
+  // TODO:
+  return nullopt;
+}
+
+void ConfigManager::on_revert(const Configuration&) {
+  auto config_metadata = apply_update({});
+  telemetry_->capture_configuration_change(config_metadata);
+}
 
 std::shared_ptr<TraceSampler> ConfigManager::trace_sampler() {
   std::lock_guard<std::mutex> lock(mutex_);
@@ -103,7 +158,8 @@ bool ConfigManager::report_traces() {
   return report_traces_.value();
 }
 
-std::vector<ConfigMetadata> ConfigManager::update(const ConfigUpdate& conf) {
+std::vector<ConfigMetadata> ConfigManager::apply_update(
+    const ConfigManager::Update& conf) {
   std::vector<ConfigMetadata> metadata;
 
   std::lock_guard<std::mutex> lock(mutex_);
@@ -210,8 +266,6 @@ void ConfigManager::reset_config(ConfigName name, T& conf,
   metadata.emplace_back(default_metadata_[name]);
 }
 
-std::vector<ConfigMetadata> ConfigManager::reset() { return update({}); }
-
 nlohmann::json ConfigManager::config_json() const {
   std::lock_guard<std::mutex> lock(mutex_);
   return nlohmann::json{{"defaults", to_json(*span_defaults_.value())},

src/datadog/config_manager.h

@@ -8,16 +8,31 @@
 #include <mutex>
 
 #include "clock.h"
-#include "config_update.h"
 #include "json.hpp"
 #include "optional.h"
+#include "remote_config/listener.h"
 #include "span_defaults.h"
 #include "tracer_config.h"
+#include "tracer_telemetry.h"
 
 namespace datadog {
 namespace tracing {
 
-class ConfigManager {
+class ConfigManager : public remote_config::Listener {
+ public:
+  // The `Update` struct serves as a container for configuration that can
+  // exclusively be changed remotely.
+  //
+  // Configurations can be `nullopt` to signal the absence of a value from the
+  // remote configuration value.
+  struct Update {
+    Optional<bool> report_traces;
+    Optional<double> trace_sampling_rate;
+    Optional<std::vector<StringView>> tags;
+    const nlohmann::json* trace_sampling_rules = nullptr;
+  };
+
+ private:
   // A class template for managing dynamic configuration values.
   //
   // This class allows storing and managing dynamic configuration values. It
@@ -60,13 +75,25 @@ class ConfigManager {
   DynamicConfig<std::shared_ptr<const SpanDefaults>> span_defaults_;
   DynamicConfig<bool> report_traces_;
 
+  std::shared_ptr<TracerTelemetry> telemetry_;
+
  private:
   template <typename T>
   void reset_config(ConfigName name, T& conf,
                     std::vector<ConfigMetadata>& metadata);
 
  public:
-  ConfigManager(const FinalizedTracerConfig& config);
+  ConfigManager(const FinalizedTracerConfig& config,
+                const std::shared_ptr<TracerTelemetry>& telemetry);
+  ~ConfigManager() override{};
+
+  remote_config::Products get_products() override;
+  remote_config::Capabilities get_capabilities() override;
+
+  Optional<std::string> on_update(
+      const Listener::Configuration& config) override;
+  void on_revert(const Listener::Configuration& config) override;
+  void on_post_process() override{};
 
   // Return the `TraceSampler` consistent with the most recent configuration.
   std::shared_ptr<TraceSampler> trace_sampler();
@@ -77,16 +104,11 @@ class ConfigManager {
   // Return whether traces should be sent to the collector.
   bool report_traces();
 
-  // Apply the specified `conf` update.
-  std::vector<ConfigMetadata> update(const ConfigUpdate& conf);
-
-  // Restore the configuration that was passed to this object's constructor,
-  // overriding any previous calls to `update`.
-  std::vector<ConfigMetadata> reset();
-
   // Return a JSON representation of the current configuration managed by this
   // object.
   nlohmann::json config_json() const;
+
+  std::vector<ConfigMetadata> apply_update(const ConfigManager::Update& conf);
 };
 
 }  // namespace tracing

src/datadog/config_update.h

@@ -145,12 +145,14 @@ std::variant<CollectorResponse, std::string> parse_agent_traces_response(
 
 }  // namespace
 
+namespace rc = datadog::remote_config;
+
 DatadogAgent::DatadogAgent(
     const FinalizedDatadogAgentConfig& config,
     const std::shared_ptr<TracerTelemetry>& tracer_telemetry,
     const std::shared_ptr<Logger>& logger,
     const TracerSignature& tracer_signature,
-    const std::shared_ptr<ConfigManager>& config_manager)
+    const std::vector<std::shared_ptr<rc::Listener>>& rc_listeners)
     : tracer_telemetry_(tracer_telemetry),
       clock_(config.clock),
       logger_(logger),
@@ -162,7 +164,7 @@ DatadogAgent::DatadogAgent(
       flush_interval_(config.flush_interval),
       request_timeout_(config.request_timeout),
       shutdown_timeout_(config.shutdown_timeout),
-      remote_config_(tracer_signature, config_manager),
+      remote_config_(tracer_signature, rc_listeners, logger),
       tracer_signature_(tracer_signature) {
   assert(logger_);
   assert(tracer_telemetry_);
@@ -410,14 +412,13 @@ void DatadogAgent::send_heartbeat_and_telemetry() {
   send_telemetry("app-heartbeat", tracer_telemetry_->heartbeat_and_telemetry());
 }
 
-void DatadogAgent::send_app_closing() {
-  send_telemetry("app-closing", tracer_telemetry_->app_closing());
+void DatadogAgent::send_configuration_change() {
+  send_telemetry("app-client-configuration-change",
+                 tracer_telemetry_->configuration_change());
 }
 
-void DatadogAgent::send_configuration_change(
-    const std::vector<ConfigMetadata>& config) {
-  send_telemetry("app-client-configuration-change",
-                 tracer_telemetry_->configuration_change(config));
+void DatadogAgent::send_app_closing() {
+  send_telemetry("app-closing", tracer_telemetry_->app_closing());
 }
 
 void DatadogAgent::get_and_apply_remote_configuration_updates() {
@@ -457,11 +458,13 @@ void DatadogAgent::get_and_apply_remote_configuration_updates() {
         }
 
         if (!response_json.empty()) {
-          auto updated_configuration =
-              remote_config_.process_response(response_json);
-          if (!updated_configuration.empty()) {
-            send_configuration_change(updated_configuration);
-          }
+          remote_config_.process_response(response_json);
+          // NOTE(@dmehala): Not ideal but it mimics the old behavior.
+          // In the future, I would prefer telemetry pushing to the agent
+          // and not the agent pulling from telemetry. That way telemetry will
+          // be more flexible and could support env var to customize how often
+          // it captures metrics.
+          send_configuration_change();
         }
       };
 

src/datadog/datadog_agent.cpp

@@ -16,7 +16,7 @@
 #include "event_scheduler.h"
 #include "http_client.h"
 #include "metrics.h"
-#include "remote_config.h"
+#include "remote_config/remote_config.h"
 #include "tracer_signature.h"
 #include "tracer_telemetry.h"
 
@@ -55,7 +55,7 @@ class DatadogAgent : public Collector {
   std::chrono::steady_clock::duration request_timeout_;
   std::chrono::steady_clock::duration shutdown_timeout_;
 
-  RemoteConfigurationManager remote_config_;
+  remote_config::Manager remote_config_;
   TracerSignature tracer_signature_;
 
   void flush();
@@ -67,7 +67,8 @@ class DatadogAgent : public Collector {
   DatadogAgent(const FinalizedDatadogAgentConfig&,
                const std::shared_ptr<TracerTelemetry>&,
                const std::shared_ptr<Logger>&, const TracerSignature& id,
-               const std::shared_ptr<ConfigManager>& config_manager);
+               const std::vector<std::shared_ptr<remote_config::Listener>>&
+                   rc_listeners);
   ~DatadogAgent();
 
   Expected<void> send(
@@ -77,7 +78,7 @@ class DatadogAgent : public Collector {
   void send_app_started(
       const std::unordered_map<ConfigName, ConfigMetadata>& config_metadata);
 
-  void send_configuration_change(const std::vector<ConfigMetadata>& config);
+  void send_configuration_change();
 
   void get_and_apply_remote_configuration_updates();
 

src/datadog/datadog_agent.h

@@ -78,6 +78,9 @@ Expected<FinalizedDatadogAgentConfig> finalize_config(
     result.event_scheduler = user_config.event_scheduler;
   }
 
+  result.remote_configuration_listeners =
+      user_config.remote_configuration_listeners;
+
   if (auto flush_interval_milliseconds =
           value_or(env_config->flush_interval_milliseconds,
                    user_config.flush_interval_milliseconds, 2000);