tracing: Add hostname to Zipkin config. (#14186) (#14187)

Signed-off-by: John Esmet <john.esmet@gmail.com>

api/envoy/config/trace/v3/zipkin.proto

@@ -20,7 +20,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 
 // Configuration for the Zipkin tracer.
 // [#extension: envoy.tracers.zipkin]
-// [#next-free-field: 6]
+// [#next-free-field: 7]
 message ZipkinConfig {
   option (udpa.annotations.versioning).previous_message_type = "envoy.config.trace.v2.ZipkinConfig";
 
@@ -65,4 +65,8 @@ message ZipkinConfig {
   // Determines the selected collector endpoint version. By default, the ``HTTP_JSON_V1`` will be
   // used.
   CollectorEndpointVersion collector_endpoint_version = 5;
+
+  // Optional hostname to use when sending spans to the collector_cluster. Useful for collectors
+  // that require a specific hostname. Defaults to :ref:`collector_cluster <envoy_v3_api_field_config.trace.v3.ZipkinConfig.collector_cluster>` above.
+  string collector_hostname = 6;
 }

docs/root/version_history/current.rst

@@ -82,6 +82,7 @@ New Features
 * thrift_proxy: added a new :ref: `payload_passthrough <envoy_v3_api_field_extensions.filters.network.thrift_proxy.v3.ThriftProxy.payload_passthrough>` option to skip decoding body in the Thrift message.
 * tls: added support for RSA certificates with 4096-bit keys in FIPS mode.
 * tracing: added SkyWalking tracer.
+* tracing: added support for setting the hostname used when sending spans to a Zipkin collector using the :ref:`collector_hostname <envoy_v3_api_field_config.trace.v3.ZipkinConfig.collector_hostname>` field.
 * xds: added support for resource TTLs. A TTL is specified on the :ref:`Resource <envoy_api_msg_Resource>`. For SotW, a :ref:`Resource <envoy_api_msg_Resource>` can be embedded
   in the list of resources to specify the TTL.
 

source/extensions/tracers/zipkin/zipkin_tracer_impl.cc

@@ -80,6 +80,8 @@ Driver::Driver(const envoy::config::trace::v3::ZipkinConfig& zipkin_config,
   Config::Utility::checkCluster("envoy.tracers.zipkin", zipkin_config.collector_cluster(), cm_,
                                 /* allow_added_via_api */ true);
   cluster_ = zipkin_config.collector_cluster();
+  hostname_ = !zipkin_config.collector_hostname().empty() ? zipkin_config.collector_hostname()
+                                                          : zipkin_config.collector_cluster();
 
   CollectorInfo collector;
   if (!zipkin_config.collector_endpoint().empty()) {
@@ -180,7 +182,7 @@ void ReporterImpl::flushSpans() {
     Http::RequestMessagePtr message = std::make_unique<Http::RequestMessageImpl>();
     message->headers().setReferenceMethod(Http::Headers::get().MethodValues.Post);
     message->headers().setPath(collector_.endpoint_);
-    message->headers().setHost(driver_.cluster());
+    message->headers().setHost(driver_.hostname());
     message->headers().setReferenceContentType(
         collector_.version_ == envoy::config::trace::v3::ZipkinConfig::HTTP_PROTO
             ? Http::Headers::get().ContentTypeValues.Protobuf

source/extensions/tracers/zipkin/zipkin_tracer_impl.h

@@ -124,6 +124,7 @@ class Driver : public Tracing::Driver {
   // Getters to return the ZipkinDriver's key members.
   Upstream::ClusterManager& clusterManager() { return cm_; }
   const std::string& cluster() { return cluster_; }
+  const std::string& hostname() { return hostname_; }
   Runtime::Loader& runtime() { return runtime_; }
   ZipkinTracerStats& tracerStats() { return tracer_stats_; }
 
@@ -140,6 +141,7 @@ class Driver : public Tracing::Driver {
 
   Upstream::ClusterManager& cm_;
   std::string cluster_;
+  std::string hostname_;
   ZipkinTracerStats tracer_stats_;
   ThreadLocal::SlotPtr tls_;
   Runtime::Loader& runtime_;

test/extensions/tracers/zipkin/zipkin_tracer_impl_test.cc

@@ -62,23 +62,32 @@ class ZipkinDriverTest : public testing::Test {
                                        random_, time_source_);
   }
 
-  void setupValidDriver(const std::string& version) {
+  void setupValidDriverWithHostname(const std::string& version, const std::string& hostname) {
     cm_.initializeClusters({"fake_cluster"}, {});
 
-    const std::string yaml_string = fmt::format(R"EOF(
+    std::string yaml_string = fmt::format(R"EOF(
     collector_cluster: fake_cluster
     collector_endpoint: /api/v1/spans
     collector_endpoint_version: {}
     )EOF",
-                                                version);
+                                          version);
+    if (!hostname.empty()) {
+      yaml_string = yaml_string + fmt::format(R"EOF(
+    collector_hostname: {}
+    )EOF",
+                                              hostname);
+    }
+
     envoy::config::trace::v3::ZipkinConfig zipkin_config;
     TestUtility::loadFromYaml(yaml_string, zipkin_config);
 
     setup(zipkin_config, true);
   }
 
-  void expectValidFlushSeveralSpans(const std::string& version, const std::string& content_type) {
-    setupValidDriver(version);
+  void expectValidFlushSeveralSpansWithHostname(const std::string& version,
+                                                const std::string& content_type,
+                                                const std::string& hostname) {
+    setupValidDriverWithHostname(version, hostname);
 
     Http::MockAsyncClientRequest request(&cm_.async_client_);
     Http::AsyncClient::Callbacks* callback;
@@ -91,8 +100,9 @@ class ZipkinDriverTest : public testing::Test {
                        const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* {
               callback = &callbacks;
 
+              const std::string& expected_hostname = !hostname.empty() ? hostname : "fake_cluster";
               EXPECT_EQ("/api/v1/spans", message->headers().getPathValue());
-              EXPECT_EQ("fake_cluster", message->headers().getHostValue());
+              EXPECT_EQ(expected_hostname, message->headers().getHostValue());
               EXPECT_EQ(content_type, message->headers().getContentTypeValue());
 
               return &request;
@@ -128,6 +138,12 @@ class ZipkinDriverTest : public testing::Test {
     EXPECT_EQ(1U, stats_.counter("tracing.zipkin.reports_failed").value());
   }
 
+  void setupValidDriver(const std::string& version) { setupValidDriverWithHostname(version, ""); }
+
+  void expectValidFlushSeveralSpans(const std::string& version, const std::string& content_type) {
+    expectValidFlushSeveralSpansWithHostname(version, content_type, "");
+  }
+
   // TODO(#4160): Currently time_system_ is initialized from DangerousDeprecatedTestTime, which uses
   // real time, not mock-time. When that is switched to use mock-time instead, I think
   // generateRandom64() may not be as random as we want, and we'll need to inject entropy
@@ -206,6 +222,10 @@ TEST_F(ZipkinDriverTest, FlushSeveralSpansHttpJson) {
   expectValidFlushSeveralSpans("HTTP_JSON", "application/json");
 }
 
+TEST_F(ZipkinDriverTest, FlushSeveralSpansHttpJsonWithHostname) {
+  expectValidFlushSeveralSpansWithHostname("HTTP_JSON", "application/json", "zipkin.fakedomain.io");
+}
+
 TEST_F(ZipkinDriverTest, FlushSeveralSpansHttpProto) {
   expectValidFlushSeveralSpans("HTTP_PROTO", "application/x-protobuf");
 }