Merge pull request #5 from DataDog/david.goffredo/no-typeid

dgoffredo · web-flow · commit 8166499872a2 · 2022-11-02T03:20:50.000-04:00
More startup JSON logging:

- don't use mangled symbol names
- use return values instead of output parameters
diff --git a/src/datadog/collector.h b/src/datadog/collector.h
@@ -33,15 +33,14 @@ class Collector {
       std::vector<std::unique_ptr<SpanData>>&& spans,
       const std::shared_ptr<TraceSampler>& response_handler) = 0;
 
-  // Assign to the specified `destination` a JSON representation of this
-  // object's configuration. The JSON representation is an object with
-  // the following properties:
+  // Return a JSON representation of this object's configuration. The JSON
+  // representation is an object with the following properties:
   //
-  // - "type" is the unmangled, unqualified name of the most-derived class, e.g.
-  //   "DatadogAgent".
+  // - "type" is the unmangled, qualified name of the most-derived class, e.g.
+  //   "datadog::tracing::DatadogAgent".
   // - "config" is an object containing this object's configuration. "config"
   //   may be omitted if the derived class has no configuration.
-  virtual void config_json(nlohmann::json& destination) const = 0;
+  virtual nlohmann::json config_json() const = 0;
 
   virtual ~Collector() {}
 };
diff --git a/src/datadog/curl.cpp b/src/datadog/curl.cpp
@@ -19,6 +19,7 @@
 #include "dict_reader.h"
 #include "dict_writer.h"
 #include "http_client.h"
+#include "json.hpp"
 #include "logger.h"
 #include "parse_util.h"
 
@@ -125,6 +126,10 @@ void Curl::drain(std::chrono::steady_clock::time_point deadline) {
   impl_->drain(deadline);
 }
 
+nlohmann::json Curl::config_json() const {
+  return nlohmann::json::object({{"type", "datadog::tracing::Curl"}});
+}
+
 CurlImpl::CurlImpl(const std::shared_ptr<Logger> &logger)
     : logger_(logger), shutting_down_(false), num_active_handles_(0) {
   curl_global_init(CURL_GLOBAL_ALL);
diff --git a/src/datadog/curl.h b/src/datadog/curl.h
@@ -14,6 +14,7 @@
 #include <string>
 
 #include "http_client.h"
+#include "json_fwd.hpp"
 
 namespace datadog {
 namespace tracing {
@@ -35,6 +36,8 @@ class Curl : public HTTPClient {
                       ErrorHandler on_error) override;
 
   void drain(std::chrono::steady_clock::time_point deadline) override;
+
+  nlohmann::json config_json() const override;
 };
 
 }  // namespace tracing
diff --git a/src/datadog/datadog_agent.cpp b/src/datadog/datadog_agent.cpp
@@ -164,20 +164,20 @@ Expected<void> DatadogAgent::send(
   return std::nullopt;
 }
 
-void DatadogAgent::config_json(nlohmann::json& destination) const {
+nlohmann::json DatadogAgent::config_json() const {
   const auto& url = traces_endpoint_;  // brevity
   const auto flush_interval_milliseconds =
       std::chrono::duration_cast<std::chrono::milliseconds>(flush_interval_)
           .count();
 
   // clang-format off
-  destination = nlohmann::json::object({
-    {"type", "DatadogAgent"},
+  return nlohmann::json::object({
+    {"type", "datadog::tracing::DatadogAgent"},
     {"config", nlohmann::json::object({
       {"url", (url.scheme + "://" + url.authority + url.path)},
       {"flush_interval_milliseconds", flush_interval_milliseconds},
-      {"http_client_typeid", typeid(*http_client_).name()},
-      {"event_scheduler_typeid", typeid(*event_scheduler_).name()},
+      {"http_client", http_client_->config_json()},
+      {"event_scheduler", event_scheduler_->config_json()},
     })},
   });
   // clang-format on
diff --git a/src/datadog/datadog_agent.h b/src/datadog/datadog_agent.h
@@ -55,7 +55,7 @@ class DatadogAgent : public Collector {
       std::vector<std::unique_ptr<SpanData>>&& spans,
       const std::shared_ptr<TraceSampler>& response_handler) override;
 
-  void config_json(nlohmann::json& destination) const override;
+  nlohmann::json config_json() const override;
 };
 
 }  // namespace tracing
diff --git a/src/datadog/environment.cpp b/src/datadog/environment.cpp
@@ -19,13 +19,16 @@ std::optional<std::string_view> lookup(Variable variable) {
   return std::string_view{value};
 }
 
-void to_json(nlohmann::json &destination) {
-  destination = nlohmann::json::object({});
+nlohmann::json to_json() {
+  auto result = nlohmann::json::object({});
+
   for (const char *name : variable_names) {
     if (const char *value = std::getenv(name)) {
-      destination[name] = value;
+      result[name] = value;
     }
   }
+
+  return result;
 }
 
 }  // namespace environment
diff --git a/src/datadog/environment.h b/src/datadog/environment.h
@@ -72,7 +72,7 @@ std::string_view name(Variable variable);
 // `std::nullptr` if that variable is not set in the environment.
 std::optional<std::string_view> lookup(Variable variable);
 
-void to_json(nlohmann::json& destination);
+nlohmann::json to_json();
 
 }  // namespace environment
 }  // namespace tracing
diff --git a/src/datadog/event_scheduler.h b/src/datadog/event_scheduler.h
@@ -13,6 +13,7 @@
 #include <functional>
 
 #include "error.h"
+#include "json_fwd.hpp"
 
 namespace datadog {
 namespace tracing {
@@ -29,6 +30,15 @@ class EventScheduler {
       std::chrono::steady_clock::duration interval,
       std::function<void()> callback) = 0;
 
+  // Return a JSON representation of this object's configuration. The JSON
+  // representation is an object with the following properties:
+  //
+  // - "type" is the unmangled, qualified name of the most-derived class, e.g.
+  //   "datadog::tracing::ThreadedEventScheduler".
+  // - "config" is an object containing this object's configuration. "config"
+  //   may be omitted if the derived class has no configuration.
+  virtual nlohmann::json config_json() const = 0;
+
   virtual ~EventScheduler() = default;
 };
 
diff --git a/src/datadog/http_client.h b/src/datadog/http_client.h
@@ -14,6 +14,7 @@
 
 #include "error.h"
 #include "expected.h"
+#include "json_fwd.hpp"
 
 namespace datadog {
 namespace tracing {
@@ -51,6 +52,15 @@ class HTTPClient {
   // `deadline`.
   virtual void drain(std::chrono::steady_clock::time_point deadline) = 0;
 
+  // Return a JSON representation of this object's configuration. The JSON
+  // representation is an object with the following properties:
+  //
+  // - "type" is the unmangled, qualified name of the most-derived class, e.g.
+  //   "datadog::tracing::Curl".
+  // - "config" is an object containing this object's configuration. "config"
+  //   may be omitted if the derived class has no configuration.
+  virtual nlohmann::json config_json() const = 0;
+
   virtual ~HTTPClient() = default;
 };
 
diff --git a/src/datadog/null_collector.cpp b/src/datadog/null_collector.cpp
@@ -5,10 +5,10 @@
 namespace datadog {
 namespace tracing {
 
-void NullCollector::config_json(nlohmann::json& destination) const {
+nlohmann::json NullCollector::config_json() const {
   // clang-format off
-    destination = nlohmann::json::object({
-        {"type", "NullCollector"},
+    return nlohmann::json::object({
+        {"type", "datadog::tracing::NullCollector"},
         {"config", nlohmann::json::object({})},
     });
   // clang-format on
diff --git a/src/datadog/null_collector.h b/src/datadog/null_collector.h
@@ -16,7 +16,7 @@ class NullCollector : public Collector {
     return {};
   }
 
-  void config_json(nlohmann::json& destination) const override;
+  nlohmann::json config_json() const override;
 };
 
 }  // namespace tracing
diff --git a/src/datadog/propagation_styles.cpp b/src/datadog/propagation_styles.cpp
@@ -8,15 +8,15 @@
 namespace datadog {
 namespace tracing {
 
-void to_json(nlohmann::json& destination, const PropagationStyles& styles) {
+nlohmann::json to_json(const PropagationStyles& styles) {
   std::vector<std::string> selected_names;
   if (styles.datadog) {
     selected_names.emplace_back("datadog");
   }
   if (styles.b3) {
     selected_names.emplace_back("B3");
   }
-  destination = selected_names;
+  return selected_names;
 }
 
 }  // namespace tracing
diff --git a/src/datadog/propagation_styles.h b/src/datadog/propagation_styles.h
@@ -15,7 +15,7 @@ struct PropagationStyles {
   bool b3 = false;
 };
 
-void to_json(nlohmann::json& destination, const PropagationStyles&);
+nlohmann::json to_json(const PropagationStyles&);
 
 }  // namespace tracing
 }  // namespace datadog
diff --git a/src/datadog/span_defaults.cpp b/src/datadog/span_defaults.cpp
@@ -12,17 +12,18 @@ bool operator==(const SpanDefaults& left, const SpanDefaults& right) {
 #undef EQ
 }
 
-void to_json(nlohmann::json& destination, const SpanDefaults& defaults) {
-  destination = nlohmann::json::object({});
+nlohmann::json to_json(const SpanDefaults& defaults) {
+  auto result = nlohmann::json::object({});
 #define TO_JSON(FIELD) \
-  if (!defaults.FIELD.empty()) destination[#FIELD] = defaults.FIELD
+  if (!defaults.FIELD.empty()) result[#FIELD] = defaults.FIELD
   TO_JSON(service);
   TO_JSON(service_type);
   TO_JSON(environment);
   TO_JSON(version);
   TO_JSON(name);
   TO_JSON(tags);
 #undef TO_JSON
+  return result;
 }
 
 }  // namespace tracing
diff --git a/src/datadog/span_defaults.h b/src/datadog/span_defaults.h
@@ -21,7 +21,7 @@ struct SpanDefaults {
   std::unordered_map<std::string, std::string> tags;
 };
 
-void to_json(nlohmann::json& destination, const SpanDefaults&);
+nlohmann::json to_json(const SpanDefaults&);
 
 bool operator==(const SpanDefaults&, const SpanDefaults&);
 
diff --git a/src/datadog/span_matcher.cpp b/src/datadog/span_matcher.cpp
@@ -19,13 +19,13 @@ bool is_match(std::string_view pattern, std::string_view subject) {
 
 }  // namespace
 
-std::string SpanMatcher::to_json() const {
-  nlohmann::json object;
-  object["service"] = service;
-  object["name"] = name;
-  object["resource"] = resource;
-  object["tags"] = tags;
-  return object.dump();
+nlohmann::json SpanMatcher::to_json() const {
+  return nlohmann::json::object({
+      {"service", service},
+      {"name", name},
+      {"resource", resource},
+      {"tags", tags},
+  });
 }
 
 bool SpanMatcher::match(const SpanData& span) const {
diff --git a/src/datadog/span_matcher.h b/src/datadog/span_matcher.h
@@ -29,7 +29,7 @@ struct SpanMatcher {
   std::unordered_map<std::string, std::string> tags;
 
   bool match(const SpanData&) const;
-  std::string to_json() const;
+  nlohmann::json to_json() const;
 
   static Expected<SpanMatcher> from_json(const nlohmann::json&);
 };
diff --git a/src/datadog/span_sampler.cpp b/src/datadog/span_sampler.cpp
@@ -66,14 +66,13 @@ SpanSampler::Rule* SpanSampler::match(const SpanData& span) {
   return nullptr;
 }
 
-void SpanSampler::config_json(nlohmann::json& destination) const {
+nlohmann::json SpanSampler::config_json() const {
   std::vector<nlohmann::json> rules;
   for (const auto& rule : rules_) {
-    rules.emplace_back();
-    to_json(rules.back(), rule);
+    rules.push_back(to_json(rule));
   }
 
-  destination = nlohmann::json::object({
+  return nlohmann::json::object({
       {"rules", rules},
   });
 }
diff --git a/src/datadog/span_sampler.h b/src/datadog/span_sampler.h
@@ -60,7 +60,7 @@ class SpanSampler {
   // return null if there is no match.
   Rule* match(const SpanData&);
 
-  void config_json(nlohmann::json& destination) const;
+  nlohmann::json config_json() const;
 };
 
 }  // namespace tracing
diff --git a/src/datadog/span_sampler_config.cpp b/src/datadog/span_sampler_config.cpp
@@ -213,7 +213,7 @@ Expected<FinalizedSpanSamplerConfig> finalize_config(
       prefix +=
           "Unable to parse sample_rate in span sampling rule with span "
           "pattern ";
-      prefix += rule.to_json();
+      prefix += rule.to_json().dump();
       prefix += ": ";
       return error->with_prefix(prefix);
     }
@@ -226,7 +226,7 @@ Expected<FinalizedSpanSamplerConfig> finalize_config(
              std::end(allowed_types))) {
       std::string message;
       message += "Span sampling rule with pattern ";
-      message += rule.to_json();
+      message += rule.to_json().dump();
       message +=
           " should have a max_per_second value greater than zero, but the "
           "following value was given: ";
@@ -244,18 +244,19 @@ Expected<FinalizedSpanSamplerConfig> finalize_config(
   return result;
 }
 
-void to_json(nlohmann::json &destination,
-             const FinalizedSpanSamplerConfig::Rule &rule) {
-  destination = nlohmann::json::object({
+nlohmann::json to_json(const FinalizedSpanSamplerConfig::Rule &rule) {
+  auto result = nlohmann::json::object({
       {"service", rule.service},
       {"name", rule.name},
       {"resource", rule.resource},
       {"sample_rate", double(rule.sample_rate)},
   });
 
   if (rule.max_per_second) {
-    destination["max_per_second"] = *rule.max_per_second;
+    result["max_per_second"] = *rule.max_per_second;
   }
+
+  return result;
 }
 
 }  // namespace tracing
diff --git a/src/datadog/span_sampler_config.h b/src/datadog/span_sampler_config.h
@@ -54,8 +54,7 @@ class FinalizedSpanSamplerConfig {
 Expected<FinalizedSpanSamplerConfig> finalize_config(const SpanSamplerConfig&,
                                                      Logger&);
 
-void to_json(nlohmann::json& destination,
-             const FinalizedSpanSamplerConfig::Rule&);
+nlohmann::json to_json(const FinalizedSpanSamplerConfig::Rule&);
 
 }  // namespace tracing
 }  // namespace datadog
diff --git a/src/datadog/threaded_event_scheduler.cpp b/src/datadog/threaded_event_scheduler.cpp
@@ -2,6 +2,8 @@
 
 #include <thread>
 
+#include "json.hpp"
+
 namespace datadog {
 namespace tracing {
 
@@ -56,6 +58,11 @@ EventScheduler::Cancel ThreadedEventScheduler::schedule_recurring_event(
   };
 }
 
+nlohmann::json ThreadedEventScheduler::config_json() const {
+  return nlohmann::json::object(
+      {{"type", "datadog::tracing::ThreadedEventScheduler"}});
+}
+
 void ThreadedEventScheduler::run() {
   std::unique_lock<std::mutex> lock(mutex_);
 
diff --git a/src/datadog/threaded_event_scheduler.h b/src/datadog/threaded_event_scheduler.h
@@ -54,9 +54,10 @@ class ThreadedEventScheduler : public EventScheduler {
   ThreadedEventScheduler();
   ~ThreadedEventScheduler();
 
-  virtual Cancel schedule_recurring_event(
-      std::chrono::steady_clock::duration interval,
-      std::function<void()> callback) override;
+  Cancel schedule_recurring_event(std::chrono::steady_clock::duration interval,
+                                  std::function<void()> callback) override;
+
+  nlohmann::json config_json() const override;
 };
 
 }  // namespace tracing
diff --git a/src/datadog/trace_sampler.cpp b/src/datadog/trace_sampler.cpp
diff --git a/src/datadog/trace_sampler.h b/src/datadog/trace_sampler.h
diff --git a/src/datadog/trace_sampler_config.cpp b/src/datadog/trace_sampler_config.cpp
diff --git a/src/datadog/trace_sampler_config.h b/src/datadog/trace_sampler_config.h
diff --git a/src/datadog/tracer.cpp b/src/datadog/tracer.cpp
diff --git a/test/mocks/collectors.cpp b/test/mocks/collectors.cpp
diff --git a/test/mocks/collectors.h b/test/mocks/collectors.h
diff --git a/test/mocks/event_schedulers.h b/test/mocks/event_schedulers.h
diff --git a/test/mocks/http_clients.h b/test/mocks/http_clients.h

Original file line number	Diff line number	Diff line change
`@@ -19,13 +19,16 @@ std::optional<std::string_view> lookup(Variable variable) {`
`19`	`19`	`return std::string_view{value};`
`20`	`20`	`}`
`21`	`21`
`22`		`-void to_json(nlohmann::json &destination) {`
`23`		`- destination = nlohmann::json::object({});`
	`22`	`+nlohmann::json to_json() {`
	`23`	`+ auto result = nlohmann::json::object({});`
	`24`	`+`
`24`	`25`	`for (const char *name : variable_names) {`
`25`	`26`	`if (const char *value = std::getenv(name)) {`
`26`		`- destination[name] = value;`
	`27`	`+ result[name] = value;`
`27`	`28`	`}`
`28`	`29`	`}`
	`30`	`+`
	`31`	`+ return result;`
`29`	`32`	`}`
`30`	`33`
`31`	`34`	`} // namespace environment`