remote config: more code review comments

BUILD.bazel

@@ -38,6 +38,7 @@ cc_library(
     "src/datadog/tags.cpp",
     "src/datadog/threaded_event_scheduler.cpp",
     "src/datadog/tracer_config.cpp",
+    "src/datadog/tracer_signature.cpp",
     "src/datadog/tracer_telemetry.cpp",
     "src/datadog/tracer.cpp",
     "src/datadog/trace_id.cpp",
@@ -102,10 +103,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

@@ -138,6 +138,7 @@ target_sources(dd_trace_cpp-objects PRIVATE
     src/datadog/tags.cpp
     src/datadog/threaded_event_scheduler.cpp
     src/datadog/tracer_config.cpp
+    src/datadog/tracer_signature.cpp
     src/datadog/tracer_telemetry.cpp
     src/datadog/tracer.cpp
     src/datadog/trace_id.cpp
@@ -208,10 +209,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_update.h

@@ -1,10 +1,13 @@
+#pragma once
+
+// TODO: Document
+
 #include "optional"
-#include "trace_sampler.h"
+#include "trace_sampler_config.h"
 
 namespace datadog {
 namespace tracing {
 
-// TODO: Document
 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()) {
@@ -401,12 +401,11 @@ void DatadogAgent::get_and_apply_remote_configuration_updates() {
              std::string response_body) {
         if (response_status < 200 || response_status >= 300) {
           if (response_status == 404) {
-            /*
-             * 404 is not considered as an error as the agent use it to
-             * signal remote configuration is disabled. At any point, the
-             * feature could be enabled, so the tracer must continuously check
-             * for new remote configuration.
-             */
+            // If the Datadog Agent doesn't understand remote configuration,
+            // or if remote configuration is disabled in the agent, then it
+            // will return 404. This is not an error.
+            // We'll keep polling, though, because the agent's configuration
+            // might change.
             return;
           }
 
@@ -431,6 +430,7 @@ void DatadogAgent::get_and_apply_remote_configuration_updates() {
           return;
         }
 
+        // TODO(@dgoffredo): When is the parsed JSON object empty?
         if (!response_json.empty()) {
           // TODO (during Active Configuration): `process_response` should
           // return a list of configuration update and should be consumed by

src/datadog/datadog_agent.h

@@ -26,7 +26,7 @@ class FinalizedDatadogAgentConfig;
 class Logger;
 struct SpanData;
 class TraceSampler;
-struct TracerID;
+class 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

@@ -10,44 +10,53 @@
 #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 {
+  // TODO: document
   struct State {
     uint64_t targets_version = 1;
     std::string opaque_backend_state;
     Optional<std::string> error_message;
   };
 
+  // TODO: document
   struct Configuration {
     std::string id;
     std::string hash;
     std::size_t version;
     ConfigUpdate content;
   };
 
-  TracerID tracer_id_;
+  TracerSignature tracer_signature_;
   ConfigManager& config_manager_;
+  // TODO: document
   std::string client_id_;
 
   State state_;
+  // TODO: document
   std::unordered_map<std::string, Configuration> applied_config_;
 
  public:
-  RemoteConfigurationManager(const TracerID& tracer_id,
+  RemoteConfigurationManager(const TracerSignature& tracer_signature,
                              ConfigManager& config_manager);
 
+  // TODO: document
   nlohmann::json make_request_payload();
 
+  // TODO: document
   void process_response(const nlohmann::json& json);
 
  private:
+  // TODO: document
   bool is_new_config(StringView config_path, const nlohmann::json& config_meta);
 
+  // TODO: document
   void apply_config(Configuration config);
+  // TODO: document
   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_signature.cpp

@@ -0,0 +1,33 @@
+#include "tracer_signature.h"
+
+#include "version.h"
+
+namespace datadog {
+namespace tracing {
+
+TracerSignature::TracerSignature(const RuntimeID& runtime_id,
+                                 const std::string& default_service,
+                                 const std::string& default_environment)
+    : runtime_id_(runtime_id),
+      default_service_(default_service),
+      default_environment_(default_environment) {}
+
+const RuntimeID& TracerSignature::runtime_id() const { return runtime_id_; }
+
+StringView TracerSignature::default_service() const { return default_service_; }
+
+StringView TracerSignature::default_environment() const {
+  return default_environment_;
+}
+
+StringView TracerSignature::library_version() const { return tracer_version; }
+
+StringView TracerSignature::library_language() const { return "cpp"; }
+
+StringView TracerSignature::library_language_version() const {
+  static const std::string value = std::to_string(__cplusplus);
+  return value;
+}
+
+}  // namespace tracing
+}  // namespace datadog

src/datadog/tracer_signature.h

@@ -0,0 +1,63 @@
+#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"
+
+namespace datadog {
+namespace tracing {
+
+class TracerSignature {
+  RuntimeID runtime_id_;
+  std::string default_service_;
+  std::string default_environment_;
+
+ public:
+  // Create a tracer signature having the specified `runtime_id`,
+  // `default_service`, and `default_environment`. `default_service` and
+  // `default_environment` refer to the corresponding fields from
+  // `SpanDefaults`.
+  TracerSignature(const RuntimeID& runtime_id,
+                  const std::string& default_service,
+                  const std::string& default_environment);
+
+  // Return the runtime ID with which the tracer was configured.
+  const RuntimeID& runtime_id() const;
+  // Return the `SpanDefaults::service` with which the tracer was configured.
+  StringView default_service() const;
+  // Return the `SpanDefaults::environment` with which the tracer was
+  // configured.
+  StringView default_environment() const;
+  // Return the version of this tracing library (`tracer_version` from
+  // `version.h`).
+  StringView library_version() const;
+  // Return the name of the programming language in which this library is
+  // written: "cpp".
+  StringView library_language() const;
+  // Return the version of C++ standard used to compile this library. It should
+  // be "201703".
+  StringView library_language_version() const;
+};
+
+}  // namespace tracing
+}  // namespace datadog