tracing: Fix duplicated x-b3-* header (#5019)

Signed-off-by: tianqian.zyf <[email protected]>
istio · Nov 13, 2018 · fcc68f1 · fcc68f1
1 parent 6b56612
commit fcc68f1
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 9 deletions.
diff --git a/source/extensions/tracers/zipkin/zipkin_core_constants.h b/source/extensions/tracers/zipkin/zipkin_core_constants.h
@@ -32,11 +32,11 @@ class ZipkinCoreConstantValues {
   const std::string SERVER_ADDR = "sa";
 
   // Zipkin B3 headers
-  const Http::LowerCaseString X_B3_TRACE_ID{"X-B3-TraceId"};
-  const Http::LowerCaseString X_B3_SPAN_ID{"X-B3-SpanId"};
-  const Http::LowerCaseString X_B3_PARENT_SPAN_ID{"X-B3-ParentSpanId"};
-  const Http::LowerCaseString X_B3_SAMPLED{"X-B3-Sampled"};
-  const Http::LowerCaseString X_B3_FLAGS{"X-B3-Flags"};
+  const Http::LowerCaseString X_B3_TRACE_ID{"x-b3-traceid"};
+  const Http::LowerCaseString X_B3_SPAN_ID{"x-b3-spanid"};
+  const Http::LowerCaseString X_B3_PARENT_SPAN_ID{"x-b3-parentspanid"};
+  const Http::LowerCaseString X_B3_SAMPLED{"x-b3-sampled"};
+  const Http::LowerCaseString X_B3_FLAGS{"x-b3-flags"};
 
   // Zipkin b3 single header
   const Http::LowerCaseString B3{"b3"};

diff --git a/source/extensions/tracers/zipkin/zipkin_tracer_impl.cc b/source/extensions/tracers/zipkin/zipkin_tracer_impl.cc
@@ -30,18 +30,18 @@ void ZipkinSpan::setTag(const std::string& name, const std::string& value) {
 
 void ZipkinSpan::injectContext(Http::HeaderMap& request_headers) {
   // Set the trace-id and span-id headers properly, based on the newly-created span structure.
-  request_headers.addReferenceKey(ZipkinCoreConstants::get().X_B3_TRACE_ID,
+  request_headers.setReferenceKey(ZipkinCoreConstants::get().X_B3_TRACE_ID,
                                   span_.traceIdAsHexString());
-  request_headers.addReferenceKey(ZipkinCoreConstants::get().X_B3_SPAN_ID, span_.idAsHexString());
+  request_headers.setReferenceKey(ZipkinCoreConstants::get().X_B3_SPAN_ID, span_.idAsHexString());
 
   // Set the parent-span header properly, based on the newly-created span structure.
   if (span_.isSetParentId()) {
-    request_headers.addReferenceKey(ZipkinCoreConstants::get().X_B3_PARENT_SPAN_ID,
+    request_headers.setReferenceKey(ZipkinCoreConstants::get().X_B3_PARENT_SPAN_ID,
                                     span_.parentIdAsHexString());
   }
 
   // Set the sampled header.
-  request_headers.addReferenceKey(ZipkinCoreConstants::get().X_B3_SAMPLED,
+  request_headers.setReferenceKey(ZipkinCoreConstants::get().X_B3_SAMPLED,
                                   span_.sampled() ? ZipkinCoreConstants::get().SAMPLED
                                                   : ZipkinCoreConstants::get().NOT_SAMPLED);
 }

diff --git a/test/extensions/tracers/zipkin/zipkin_tracer_impl_test.cc b/test/extensions/tracers/zipkin/zipkin_tracer_impl_test.cc
@@ -1,6 +1,8 @@
 #include <chrono>
+#include <functional>
 #include <memory>
 #include <string>
+#include <unordered_map>
 
 #include "common/http/header_map_impl.h"
 #include "common/http/headers.h"
@@ -577,6 +579,37 @@ TEST_F(ZipkinDriverTest, ExplicitlySetSampledTrue) {
   // Check B3 sampled flag is set to sample
   EXPECT_EQ(ZipkinCoreConstants::get().SAMPLED, sampled_entry->value().getStringView());
 }
+
+TEST_F(ZipkinDriverTest, DuplicatedHeader) {
+  setupValidDriver();
+  request_headers_.addReferenceKey(ZipkinCoreConstants::get().X_B3_TRACE_ID,
+                                   Hex::uint64ToHex(generateRandom64()));
+  request_headers_.addReferenceKey(ZipkinCoreConstants::get().X_B3_SPAN_ID,
+                                   Hex::uint64ToHex(generateRandom64()));
+  request_headers_.addReferenceKey(ZipkinCoreConstants::get().X_B3_PARENT_SPAN_ID,
+                                   Hex::uint64ToHex(generateRandom64()));
+  Tracing::SpanPtr span = driver_->startSpan(config_, request_headers_, operation_name_,
+                                             start_time_, {Tracing::Reason::Sampling, false});
+
+  typedef std::function<bool(const std::string& key)> DupCallback;
+  DupCallback dup_callback = [](const std::string& key) -> bool {
+    static std::unordered_map<std::string, bool> dup;
+    if (dup.find(key) == dup.end()) {
+      dup[key] = true;
+      return false;
+    }
+    return true;
+  };
+
+  span->setSampled(true);
+  span->injectContext(request_headers_);
+  request_headers_.iterate(
+      [](const Http::HeaderEntry& header, void* cb) -> Http::HeaderMap::Iterate {
+        EXPECT_FALSE(static_cast<DupCallback*>(cb)->operator()(header.key().c_str()));
+        return Http::HeaderMap::Iterate::Continue;
+      },
+      &dup_callback);
+}
 } // namespace Zipkin
 } // namespace Tracers
 } // namespace Extensions