Use a single ares channel per dns resolution. (#140)

* Use a single ares channel per dns resolution. * Remove extra include.
lightstep · Apr 9, 2019 · 5d97290 · 5d97290
1 parent 864369e
commit 5d97290
Show file tree

Hide file tree

Showing 6 changed files with 16 additions and 19 deletions.
diff --git a/example/stream/main.cpp b/example/stream/main.cpp
@@ -7,6 +7,7 @@
 #include <thread>
 
 static void MakeSpan(opentracing::Tracer& tracer, int span_index) {
+  std::cout << "Make Span: " << span_index << "\n";
   auto span = tracer.StartSpan("span_" + std::to_string(span_index));
   assert(span != nullptr);
   for (int tag_index = 0; tag_index < 25; ++tag_index) {
@@ -34,7 +35,7 @@ int main() {
   for (int i = 0; i < 1000; ++i) {
     MakeSpan(*tracer, i);
   }
-  std::cout << "Closing tracer" << std::endl;
+  std::cout << "Closing tracer\n";
   tracer->Close();
   return 0;
 }
diff --git a/src/recorder/stream_recorder/satellite_dns_resolution_manager.cpp b/src/recorder/stream_recorder/satellite_dns_resolution_manager.cpp
@@ -8,12 +8,13 @@ namespace lightstep {
 // constructor
 //--------------------------------------------------------------------------------------------------
 SatelliteDnsResolutionManager::SatelliteDnsResolutionManager(
-    Logger& logger, EventBase& event_base, DnsResolver& dns_resolver,
+    Logger& logger, EventBase& event_base,
     const StreamRecorderOptions& recorder_options, int family, const char* name,
     std::function<void()> on_ready_callback)
     : logger_{logger},
       event_base_{event_base},
-      dns_resolver_{dns_resolver},
+      dns_resolver_{MakeDnsResolver(logger, event_base,
+                                    recorder_options.dns_resolver_options)},
       recorder_options_{recorder_options},
       family_{family},
       name_{name},
@@ -23,7 +24,8 @@ SatelliteDnsResolutionManager::SatelliteDnsResolutionManager(
 // Start
 //--------------------------------------------------------------------------------------------------
 void SatelliteDnsResolutionManager::Start() noexcept {
-  dns_resolver_.Resolve(name_, family_, *this);
+  logger_.Info("Resolving ", name_, " for ", family_ == AF_INET ? "ipv4" : "ipv6");
+  dns_resolver_->Resolve(name_, family_, *this);
 }
 
 //--------------------------------------------------------------------------------------------------
@@ -39,6 +41,7 @@ void SatelliteDnsResolutionManager::OnDnsResolution(
   std::vector<IpAddress> ip_addresses;
   ip_addresses.reserve(ip_addresses_.size());
   dns_resolution.ForeachIpAddress([&](const IpAddress& ip_address) {
+    logger_.Info("Resolved ", name_, " to ", ip_address);
     ip_addresses.push_back(ip_address);
     return true;
   });
@@ -61,7 +64,7 @@ void SatelliteDnsResolutionManager::OnDnsResolution(
 // OnRefresh
 //--------------------------------------------------------------------------------------------------
 void SatelliteDnsResolutionManager::OnRefresh() noexcept {
-  dns_resolver_.Resolve(name_, family_, *this);
+  dns_resolver_->Resolve(name_, family_, *this);
 }
 
 //--------------------------------------------------------------------------------------------------

diff --git a/src/recorder/stream_recorder/satellite_dns_resolution_manager.h b/src/recorder/stream_recorder/satellite_dns_resolution_manager.h
@@ -20,7 +20,6 @@ class SatelliteDnsResolutionManager final : public DnsResolutionCallback,
                                             private Noncopyable {
  public:
   SatelliteDnsResolutionManager(Logger& logger, EventBase& event_base,
-                                DnsResolver& dns_resolver,
                                 const StreamRecorderOptions& recorder_options,
                                 int family, const char* name,
                                 std::function<void()> on_ready_callback);
@@ -50,7 +49,7 @@ class SatelliteDnsResolutionManager final : public DnsResolutionCallback,
  private:
   Logger& logger_;
   EventBase& event_base_;
-  DnsResolver& dns_resolver_;
+  std::unique_ptr<DnsResolver> dns_resolver_;
   const StreamRecorderOptions& recorder_options_;
   int family_;
   const char* name_;

diff --git a/src/recorder/stream_recorder/satellite_endpoint_manager.cpp b/src/recorder/stream_recorder/satellite_endpoint_manager.cpp
@@ -13,9 +13,7 @@ SatelliteEndpointManager::SatelliteEndpointManager(
     const LightStepTracerOptions& tracer_options,
     const StreamRecorderOptions& recorder_options,
     std::function<void()> on_ready_callback)
-    : on_ready_callback_{std::move(on_ready_callback)},
-      dns_resolver_{MakeDnsResolver(logger, event_base,
-                                    recorder_options.dns_resolver_options)} {
+    : on_ready_callback_{std::move(on_ready_callback)} {
   if (tracer_options.satellite_endpoints.empty()) {
     throw std::runtime_error{"no satellite endpoints provided"};
   }
@@ -28,10 +26,10 @@ SatelliteEndpointManager::SatelliteEndpointManager(
   for (auto& name : hosts) {
     SatelliteHostManager host_manager;
     host_manager.ipv4_resolutions.reset(new SatelliteDnsResolutionManager{
-        logger, event_base, *dns_resolver_, recorder_options, AF_INET, name,
+        logger, event_base, recorder_options, AF_INET, name,
         on_resolution_ready});
     host_manager.ipv6_resolutions.reset(new SatelliteDnsResolutionManager{
-        logger, event_base, *dns_resolver_, recorder_options, AF_INET6, name,
+        logger, event_base, recorder_options, AF_INET6, name,
         on_resolution_ready});
     host_managers_.emplace_back(std::move(host_manager));
   }

diff --git a/src/recorder/stream_recorder/satellite_endpoint_manager.h b/src/recorder/stream_recorder/satellite_endpoint_manager.h
@@ -40,7 +40,6 @@ class SatelliteEndpointManager : private Noncopyable {
  private:
   std::function<void()> on_ready_callback_;
   std::vector<SatelliteHostManager> host_managers_;
-  std::unique_ptr<DnsResolver> dns_resolver_;
   std::vector<std::pair<int, uint16_t>> endpoints_;
   uint32_t endpoint_index_{0};
   int num_resolutions_ready_{0};

diff --git a/test/recorder/stream_recorder/satellite_dns_resolution_manager_test.cpp b/test/recorder/stream_recorder/satellite_dns_resolution_manager_test.cpp
@@ -33,14 +33,11 @@ TEST_CASE("SatelliteDnsResolutionManager") {
   Logger logger;
   EventBase event_base;
 
-  auto resolver = MakeDnsResolver(logger, event_base,
-                                  recorder_options.dns_resolver_options);
-
   std::function<void()> on_ready_callback = [&] { event_base.LoopBreak(); };
 
   SECTION("Hosts get resolved to ip addresses.") {
     SatelliteDnsResolutionManager resolution_manager{
-        logger,  event_base,     *resolver,        recorder_options,
+        logger,  event_base,            recorder_options,
         AF_INET, "test.service", on_ready_callback};
     resolution_manager.Start();
     event_base.Dispatch();
@@ -50,7 +47,7 @@ TEST_CASE("SatelliteDnsResolutionManager") {
 
   SECTION("Dns resolutions are periodically refreshed.") {
     SatelliteDnsResolutionManager resolution_manager{
-        logger,  event_base,     *resolver,        recorder_options,
+        logger,  event_base,            recorder_options,
         AF_INET, "flip.service", on_ready_callback};
     resolution_manager.Start();
     event_base.Dispatch();
@@ -69,7 +66,7 @@ TEST_CASE("SatelliteDnsResolutionManager") {
 
   SECTION("Dns resolutions are retried when if there's an error.") {
     SatelliteDnsResolutionManager resolution_manager{
-        logger,  event_base,      *resolver,        recorder_options,
+        logger,  event_base,             recorder_options,
         AF_INET, "flaky.service", on_ready_callback};
     resolution_manager.Start();
     event_base.Dispatch();