Code review 2

Author: dmehala

BUILD.bazel

@@ -102,10 +102,10 @@ cc_library(
     "src/datadog/tags.h",
     "src/datadog/threaded_event_scheduler.h",
     "src/datadog/tracer_config.h",
+    "src/datadog/tracer_signature.h",
     "src/datadog/tracer_telemetry.h",
     "src/datadog/tracer.h",
     "src/datadog/trace_id.h",
-    "src/datadog/tracer_id.h",
     "src/datadog/trace_sampler_config.h",
     "src/datadog/trace_sampler.h",
     "src/datadog/trace_segment.h",

CMakeLists.txt

@@ -208,10 +208,10 @@ target_sources(dd_trace_cpp-objects PUBLIC
   src/datadog/tags.h
   src/datadog/threaded_event_scheduler.h
   src/datadog/tracer_config.h
+  src/datadog/tracer_signature.h
   src/datadog/tracer_telemetry.h
   src/datadog/tracer.h
   src/datadog/trace_id.h
-  src/datadog/tracer_id.h
   src/datadog/trace_sampler_config.h
   src/datadog/trace_sampler.h
   src/datadog/trace_segment.h

src/datadog/config_manager.cpp

@@ -1,5 +1,7 @@
 #include "config_manager.h"
 
+#include "trace_sampler.h"
+
 namespace datadog {
 namespace tracing {
 

src/datadog/config_manager.h

@@ -1,6 +1,9 @@
 #pragma once
 
-// TODO: Document
+// The `ConfigManager` class is designed to handle configuration update
+// and provide access to the current configuration.
+// It utilizes a mutex to ensure thread safety when updating or accessing
+// the configuration.
 
 #include <mutex>
 

src/datadog/config_update.h

@@ -1,10 +1,16 @@
+#pragma once
+
 #include "optional"
-#include "trace_sampler.h"
+#include "trace_sampler_config.h"
 
 namespace datadog {
 namespace tracing {
 
-// TODO: Document
+// The `ConfigUpdate` 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 ConfigUpdate {
   Optional<TraceSamplerConfig> trace_sampler;
 };

src/datadog/datadog_agent.cpp

@@ -149,8 +149,8 @@ std::variant<CollectorResponse, std::string> parse_agent_traces_response(
 DatadogAgent::DatadogAgent(
     const FinalizedDatadogAgentConfig& config,
     const std::shared_ptr<TracerTelemetry>& tracer_telemetry,
-    const std::shared_ptr<Logger>& logger, const TracerID& tracer_id,
-    ConfigManager& config_manager)
+    const std::shared_ptr<Logger>& logger,
+    const TracerSignature& tracer_signature, ConfigManager& config_manager)
     : tracer_telemetry_(tracer_telemetry),
       clock_(config.clock),
       logger_(logger),
@@ -164,7 +164,7 @@ DatadogAgent::DatadogAgent(
       flush_interval_(config.flush_interval),
       request_timeout_(config.request_timeout),
       shutdown_timeout_(config.shutdown_timeout),
-      remote_config_(tracer_id, config_manager) {
+      remote_config_(tracer_signature, config_manager) {
   assert(logger_);
   assert(tracer_telemetry_);
   if (tracer_telemetry_->enabled()) {

src/datadog/datadog_agent.h

@@ -26,7 +26,7 @@ class FinalizedDatadogAgentConfig;
 class Logger;
 struct SpanData;
 class TraceSampler;
-struct TracerID;
+struct TracerSignature;
 
 class DatadogAgent : public Collector {
  public:
@@ -65,7 +65,7 @@ class DatadogAgent : public Collector {
  public:
   DatadogAgent(const FinalizedDatadogAgentConfig&,
                const std::shared_ptr<TracerTelemetry>&,
-               const std::shared_ptr<Logger>&, const TracerID& id,
+               const std::shared_ptr<Logger>&, const TracerSignature& id,
                ConfigManager& config_manager);
   ~DatadogAgent();
 

src/datadog/remote_config.cpp

@@ -64,8 +64,8 @@ ConfigUpdate parse_dynamic_config(const nlohmann::json& j) {
 }  // namespace
 
 RemoteConfigurationManager::RemoteConfigurationManager(
-    const TracerID& tracer_id, ConfigManager& config_manager)
-    : tracer_id_(tracer_id),
+    const TracerSignature& tracer_signature, ConfigManager& config_manager)
+    : tracer_signature_(tracer_signature),
       config_manager_(config_manager),
       client_id_(uuid()) {}
 
@@ -87,11 +87,11 @@ nlohmann::json RemoteConfigurationManager::make_request_payload() {
       {"is_tracer", true},
       {"capabilities", k_apm_capabilities},
       {"client_tracer", {
-        {"runtime_id", tracer_id_.runtime_id.string()},
-        {"language", "cpp"},
-        {"tracer_version", tracer_version},
-        {"service", tracer_id_.service},
-        {"env", tracer_id_.environment}
+        {"runtime_id", tracer_signature_.runtime_id.string()},
+        {"language", tracer_signature_.library_language},
+        {"tracer_version", tracer_signature_.library_version},
+        {"service", tracer_signature_.default_service},
+        {"env", tracer_signature_.default_environment}
       }},
       {"state", {
         {"root_version", 1},
@@ -178,9 +178,9 @@ void RemoteConfigurationManager::process_response(const nlohmann::json& json) {
 
       const auto& targeted_service = config_json.at("service_target");
       if (targeted_service.at("service").get<StringView>() !=
-              tracer_id_.service ||
+              tracer_signature_.default_service ||
           targeted_service.at("env").get<StringView>() !=
-              tracer_id_.environment) {
+              tracer_signature_.default_environment) {
         continue;
       }
 

src/datadog/remote_config.h

@@ -1,6 +1,16 @@
 #pragma once
 
-// TODO: Document
+// Remote Configuration is a Datadog capability that allows a user to remotely
+// configure and change the behaviour of the tracing library.
+// The current implementation is restricted to Application Performance
+// Monitoring features.
+//
+// The `RemoteConfigurationManager` class implement the protocol to query,
+// process and verify configuration from a remote source. It is also
+// responsible for handling configuration updates received from a remote source
+// and maintains the state of applied configuration.
+// It interacts with the `ConfigManager` to seamlessly apply or revert
+// configurations based on responses received from the remote source.
 
 #include <string>
 
@@ -10,44 +20,56 @@
 #include "runtime_id.h"
 #include "string_view.h"
 #include "trace_sampler_config.h"
-#include "tracer_id.h"
+#include "tracer_signature.h"
 
 namespace datadog {
 namespace tracing {
 
 class RemoteConfigurationManager {
+  // Represents the *current* state of the RemoteConfigurationManager.
+  // It is also used to report errors to the remote source.
   struct State {
     uint64_t targets_version = 1;
     std::string opaque_backend_state;
     Optional<std::string> error_message;
   };
 
+  // Holds information about a specific configuration update,
+  // including its identifier, hash value, version number and the content.
   struct Configuration {
     std::string id;
     std::string hash;
     std::size_t version;
     ConfigUpdate content;
   };
 
-  TracerID tracer_id_;
+  TracerSignature tracer_signature_;
   ConfigManager& config_manager_;
-  std::string client_id_;
+  std::string client_id_;  ///< Identifier a `RemoteConfigurationManager`
 
   State state_;
   std::unordered_map<std::string, Configuration> applied_config_;
 
  public:
-  RemoteConfigurationManager(const TracerID& tracer_id,
+  RemoteConfigurationManager(const TracerSignature& tracer_signature,
                              ConfigManager& config_manager);
 
+  // Construct a JSON object representing the payload to be sent in a remote
+  // configuration request.
   nlohmann::json make_request_payload();
 
+  // Handles the response received from a remote source and udates the internal
+  // state accordingly.
   void process_response(const nlohmann::json& json);
 
  private:
+  // Tell if a `config_path` is a new configuration update.
   bool is_new_config(StringView config_path, const nlohmann::json& config_meta);
 
+  // Apply a remote configuration.
   void apply_config(Configuration config);
+
+  // Revert a remote configuration.
   void revert_config(Configuration config);
 };
 

src/datadog/tracer.cpp

@@ -23,7 +23,7 @@
 #include "tags.h"
 #include "trace_sampler.h"
 #include "trace_segment.h"
-#include "tracer_id.h"
+#include "tracer_signature.h"
 #include "version.h"
 #include "w3c_propagation.h"
 
@@ -40,9 +40,10 @@ Tracer::Tracer(const FinalizedTracerConfig& config,
       defaults_(std::make_shared<SpanDefaults>(config.defaults)),
       runtime_id_(config.runtime_id ? *config.runtime_id
                                     : RuntimeID::generate()),
-      id_{runtime_id_, config.defaults.service, config.defaults.environment},
+      signature_{runtime_id_, config.defaults.service,
+                 config.defaults.environment},
       tracer_telemetry_(std::make_shared<TracerTelemetry>(
-          config.report_telemetry, config.clock, logger_, id_)),
+          config.report_telemetry, config.clock, logger_, signature_)),
       span_sampler_(
           std::make_shared<SpanSampler>(config.span_sampler, config.clock)),
       generator_(generator),
@@ -59,8 +60,9 @@ Tracer::Tracer(const FinalizedTracerConfig& config,
     auto& agent_config =
         std::get<FinalizedDatadogAgentConfig>(config.collector);
 
-    auto agent = std::make_shared<DatadogAgent>(
-        agent_config, tracer_telemetry_, config.logger, id_, config_manager_);
+    auto agent = std::make_shared<DatadogAgent>(agent_config, tracer_telemetry_,
+                                                config.logger, signature_,
+                                                config_manager_);
     collector_ = agent;
 
     if (tracer_telemetry_->enabled()) {

src/datadog/tracer.h

@@ -19,6 +19,7 @@
 #include "optional.h"
 #include "span.h"
 #include "tracer_config.h"
+#include "tracer_signature.h"
 #include "tracer_telemetry.h"
 
 namespace datadog {
@@ -28,14 +29,13 @@ class DictReader;
 struct SpanConfig;
 class TraceSampler;
 class SpanSampler;
-struct TracerID;
 
 class Tracer {
   std::shared_ptr<Logger> logger_;
   std::shared_ptr<Collector> collector_;
   std::shared_ptr<const SpanDefaults> defaults_;
   RuntimeID runtime_id_;
-  TracerID id_;
+  TracerSignature signature_;
   std::shared_ptr<TracerTelemetry> tracer_telemetry_;
   std::shared_ptr<SpanSampler> span_sampler_;
   std::shared_ptr<const IDGenerator> generator_;

src/datadog/tracer_id.h

@@ -0,0 +1,53 @@
+#pragma once
+
+// This component provides a class, `TracerSignature`, that contains the parts
+// of a tracer's configuration that are used to refer to the tracer in Datadog's
+// telemetry and remote configuration APIs.
+//
+// `TracerSignature` is used in three contexts:
+//
+// 1. When telemetry is sent to the Datadog Agent, the tracer signature is
+//    included in the request payload. See
+//    `TracerTelemetry::generate_telemetry_body` in `tracer_telemetry.cpp`.
+// 2. When the Datadog Agent is polled for configuration updates, part of the
+//    tracer signature (all but the language version) is included in the request
+//    payload. See `RemoteConfigurationManager::make_request_payload` in
+//    `remote_config.h`.
+// 3. When the Datadog Agent responds with configuration updates, the service
+//    and environment of the tracer signature are used to determine whether the
+//    updates are relevant to the `Tracer` that created the collector that is
+//    polling the Datadog Agent. See
+//    `RemoteConfigurationManager::process_response` in `remote_config.h`.
+
+#include <string>
+
+#include "runtime_id.h"
+#include "string_view.h"
+#include "version.h"
+
+#define DD_TRACE_STRINGIFY(ARG) DD_TRACE_STRINGIFY_HELPER(ARG)
+#define DD_TRACE_STRINGIFY_HELPER(ARG) #ARG
+
+namespace datadog {
+namespace tracing {
+
+struct TracerSignature {
+  RuntimeID runtime_id;
+  std::string default_service;
+  std::string default_environment;
+  std::string library_version;
+  StringView library_language;
+  StringView library_language_version;
+
+  TracerSignature() = delete;
+  TracerSignature(RuntimeID id, std::string service, std::string environment)
+      : runtime_id(id),
+        default_service(std::move(service)),
+        default_environment(std::move(environment)),
+        library_version(tracer_version),
+        library_language("cpp"),
+        library_language_version(DD_TRACE_STRINGIFY(__cplusplus), 6) {}
+};
+
+}  // namespace tracing
+}  // namespace datadog

src/datadog/tracer_signature.h

@@ -10,11 +10,11 @@ namespace tracing {
 
 TracerTelemetry::TracerTelemetry(bool enabled, const Clock& clock,
                                  const std::shared_ptr<Logger>& logger,
-                                 const TracerID& tracer_id)
+                                 const TracerSignature& tracer_signature)
     : enabled_(enabled),
       clock_(clock),
       logger_(logger),
-      tracer_id_(tracer_id),
+      tracer_signature_(tracer_signature),
       hostname_(get_hostname().value_or("hostname-unavailable")) {
   if (enabled_) {
     // Register all the metrics that we're tracking by adding them to the
@@ -62,15 +62,16 @@ nlohmann::json TracerTelemetry::generate_telemetry_body(
       {"seq_id", seq_id_},
       {"request_type", request_type},
       {"tracer_time", tracer_time},
-      {"runtime_id", tracer_id_.runtime_id.string()},
+      {"runtime_id", tracer_signature_.runtime_id.string()},
       {"debug", debug_},
-      {"application", nlohmann::json::object({
-                          {"service_name", tracer_id_.service},
-                          {"env", tracer_id_.environment},
-                          {"tracer_version", tracer_version},
-                          {"language_name", "cpp"},
-                          {"language_version", std::to_string(__cplusplus)},
-                      })},
+      {"application",
+       nlohmann::json::object({
+           {"service_name", tracer_signature_.default_service},
+           {"env", tracer_signature_.default_environment},
+           {"tracer_version", tracer_signature_.library_version},
+           {"language_name", tracer_signature_.library_language},
+           {"language_version", tracer_signature_.library_language_version},
+       })},
       // TODO: host information (os, os_version, kernel, etc)
       {"host", nlohmann::json::object({
                    {"hostname", hostname_},