replace RequestHeaderMap in tracers with general TraceContext (#17212)

Sub-PR of #16049. Check #16049 get more background information. This PR is last sub-PR of #16049. It simply replaces `Http::RequestHeaderMap` with general `Tracing::TraceContext` in all tracer drivers implementations. Check #16793 get more info about `Tracing::TraceContext`. After this PR, the main body of the entire general tracing system is completed. Next, we can try to use the new general tracing system in dubbo and thrift. Commit Message: replace RequestHeaderMap in tracers with general TraceContext Risk Level: Low. Testing: N/A. Docs Changes: N/A. Release Notes: N/A. Signed-off-by: wbpcode <[email protected]>
envoyproxy · Jul 8, 2021 · 00f5656 · 00f5656
1 parent 2b14b88
commit 00f5656
Show file tree

Hide file tree

Showing 27 changed files with 218 additions and 171 deletions.
diff --git a/envoy/tracing/BUILD b/envoy/tracing/BUILD
@@ -36,6 +36,7 @@ envoy_cc_library(
     name = "trace_driver_interface",
     hdrs = ["trace_driver.h"],
     deps = [
+        ":trace_context_interface",
         ":trace_reason_interface",
         "//envoy/stream_info:stream_info_interface",
     ],

diff --git a/envoy/tracing/trace_context.h b/envoy/tracing/trace_context.h
@@ -14,6 +14,10 @@ namespace Tracing {
  * Protocol-independent abstraction for traceable stream. It hides the differences between different
  * protocol and provides tracer driver with common methods for obtaining and setting the tracing
  * context.
+ *
+ * TODO(wbpcode): A new interface should be added to obtain general traceable stream information,
+ * such as host, RPC method, protocol identification, etc. At the same time, a new interface needs
+ * to be added to support traversal of all trace contexts.
  */
 class TraceContext {
 public:

diff --git a/envoy/tracing/trace_driver.h b/envoy/tracing/trace_driver.h
@@ -6,6 +6,7 @@
 
 #include "envoy/common/pure.h"
 #include "envoy/stream_info/stream_info.h"
+#include "envoy/tracing/trace_context.h"
 #include "envoy/tracing/trace_reason.h"
 
 namespace Envoy {
@@ -22,7 +23,7 @@ enum class OperationName { Ingress, Egress };
  * The context for the custom tag to obtain the tag value.
  */
 struct CustomTagContext {
-  const Http::RequestHeaderMap* request_headers;
+  const TraceContext* trace_context;
   const StreamInfo::StreamInfo& stream_info;
 };
 
@@ -117,7 +118,7 @@ class Span {
    * (implementation-specific) trace.
    * @param request_headers the headers to which propagation context will be added
    */
-  virtual void injectContext(Http::RequestHeaderMap& request_headers) PURE;
+  virtual void injectContext(TraceContext& trace_conext) PURE;
 
   /**
    * Create and start a child Span, with this Span as its parent in the trace.
@@ -171,7 +172,7 @@ class Driver {
   /**
    * Start driver specific span.
    */
-  virtual SpanPtr startSpan(const Config& config, Http::RequestHeaderMap& request_headers,
+  virtual SpanPtr startSpan(const Config& config, TraceContext& trace_conext,
                             const std::string& operation_name, SystemTime start_time,
                             const Tracing::Decision tracing_decision) PURE;
 };

diff --git a/source/common/tracing/http_tracer_impl.cc b/source/common/tracing/http_tracer_impl.cc
@@ -301,12 +301,12 @@ RequestHeaderCustomTag::RequestHeaderCustomTag(
       default_value_(request_header.default_value()) {}
 
 absl::string_view RequestHeaderCustomTag::value(const CustomTagContext& ctx) const {
-  if (!ctx.request_headers) {
+  if (ctx.trace_context == nullptr) {
     return default_value_;
   }
   // TODO(https://github.com/envoyproxy/envoy/issues/13454): Potentially populate all header values.
-  const auto entry = ctx.request_headers->get(name_);
-  return !entry.empty() ? entry[0]->value().getStringView() : default_value_;
+  const auto entry = ctx.trace_context->getTraceContext(name_);
+  return entry.value_or(default_value_);
 }
 
 MetadataCustomTag::MetadataCustomTag(const std::string& tag,

diff --git a/source/common/tracing/null_span_impl.h b/source/common/tracing/null_span_impl.h
@@ -22,7 +22,7 @@ class NullSpan : public Span {
   void setTag(absl::string_view, absl::string_view) override {}
   void log(SystemTime, const std::string&) override {}
   void finishSpan() override {}
-  void injectContext(Http::RequestHeaderMap&) override {}
+  void injectContext(Tracing::TraceContext&) override {}
   void setBaggage(absl::string_view, absl::string_view) override {}
   std::string getBaggage(absl::string_view) override { return EMPTY_STRING; }
   std::string getTraceIdAsHex() const override { return EMPTY_STRING; }

diff --git a/source/extensions/tracers/common/ot/opentracing_driver_impl.cc b/source/extensions/tracers/common/ot/opentracing_driver_impl.cc
@@ -21,52 +21,64 @@ Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::Request
     ot_span_context_handle(Http::CustomHeaders::get().OtSpanContext);
 
 namespace {
-class OpenTracingHTTPHeadersWriter : public opentracing::HTTPHeadersWriter {
+
+/**
+ * TODO(wbpcode): Use opentracing::TextMapWriter to replace opentracing::HTTPHeadersWriter.
+ */
+class OpenTracingHeadersWriter : public opentracing::HTTPHeadersWriter {
 public:
-  explicit OpenTracingHTTPHeadersWriter(Http::HeaderMap& request_headers)
-      : request_headers_(request_headers) {}
+  explicit OpenTracingHeadersWriter(Tracing::TraceContext& trace_context)
+      : trace_context_(trace_context) {}
 
   // opentracing::HTTPHeadersWriter
   opentracing::expected<void> Set(opentracing::string_view key,
                                   opentracing::string_view value) const override {
     Http::LowerCaseString lowercase_key{{key.data(), key.size()}};
-    request_headers_.remove(lowercase_key);
-    request_headers_.addCopy(std::move(lowercase_key), {value.data(), value.size()});
+    trace_context_.setTraceContext(lowercase_key, {value.data(), value.size()});
     return {};
   }
 
 private:
-  Http::HeaderMap& request_headers_;
+  Tracing::TraceContext& trace_context_;
 };
 
-class OpenTracingHTTPHeadersReader : public opentracing::HTTPHeadersReader {
+/**
+ * TODO(wbpcode): Use opentracing::TextMapReader to replace opentracing::HTTPHeadersReader.
+ */
+class OpenTracingHeadersReader : public opentracing::HTTPHeadersReader {
 public:
-  explicit OpenTracingHTTPHeadersReader(const Http::RequestHeaderMap& request_headers)
-      : request_headers_(request_headers) {}
+  explicit OpenTracingHeadersReader(const Tracing::TraceContext& trace_context)
+      : trace_context_(trace_context) {}
 
   using OpenTracingCb = std::function<opentracing::expected<void>(opentracing::string_view,
                                                                   opentracing::string_view)>;
 
   // opentracing::HTTPHeadersReader
   opentracing::expected<opentracing::string_view>
   LookupKey(opentracing::string_view key) const override {
-    const auto entry = request_headers_.get(Http::LowerCaseString{{key.data(), key.size()}});
-    if (!entry.empty()) {
-      // This is an implicitly untrusted header, so only the first value is used.
-      return opentracing::string_view{entry[0]->value().getStringView().data(),
-                                      entry[0]->value().getStringView().length()};
+    Http::LowerCaseString lowercase_key{{key.data(), key.size()}};
+    const auto entry = trace_context_.getTraceContext(lowercase_key);
+    if (entry.has_value()) {
+      return opentracing::string_view{entry.value().data(), entry.value().length()};
     } else {
       return opentracing::make_unexpected(opentracing::key_not_found_error);
     }
   }
 
   opentracing::expected<void> ForeachKey(OpenTracingCb f) const override {
-    request_headers_.iterate(headerMapCallback(f));
+    // TODO(wbpcode): TraceContext currently does not provide an API to traverse all entries. So
+    // dynamic_cast has to be used here. This is a temporary compromise to ensure that the existing
+    // functions are correct. After TraceContext provides the iterative API, this part of the code
+    // needs to be rewritten.
+    const auto headers = dynamic_cast<const Http::RequestHeaderMap*>(&trace_context_);
+    if (headers != nullptr) {
+      headers->iterate(headerMapCallback(f));
+    }
     return {};
   }
 
 private:
-  const Http::RequestHeaderMap& request_headers_;
+  const Tracing::TraceContext& trace_context_;
 
   static Http::HeaderMap::ConstIterateCb headerMapCallback(OpenTracingCb callback) {
     return [callback =
@@ -113,7 +125,7 @@ std::string OpenTracingSpan::getBaggage(absl::string_view key) {
   return span_->BaggageItem({key.data(), key.length()});
 }
 
-void OpenTracingSpan::injectContext(Http::RequestHeaderMap& request_headers) {
+void OpenTracingSpan::injectContext(Tracing::TraceContext& trace_context) {
   if (driver_.propagationMode() == OpenTracingDriver::PropagationMode::SingleHeader) {
     // Inject the span context using Envoy's single-header format.
     std::ostringstream oss;
@@ -125,12 +137,12 @@ void OpenTracingSpan::injectContext(Http::RequestHeaderMap& request_headers) {
       return;
     }
     const std::string current_span_context = oss.str();
-    request_headers.setInline(
-        ot_span_context_handle.handle(),
+    trace_context.setTraceContextReferenceKey(
+        Http::CustomHeaders::get().OtSpanContext,
         Base64::encode(current_span_context.c_str(), current_span_context.length()));
   } else {
-    // Inject the context using the tracer's standard HTTP header format.
-    const OpenTracingHTTPHeadersWriter writer{request_headers};
+    // Inject the context using the tracer's standard header format.
+    const OpenTracingHeadersWriter writer{trace_context};
     const opentracing::expected<void> was_successful =
         span_->tracer().Inject(span_->context(), writer);
     if (!was_successful) {
@@ -157,19 +169,19 @@ OpenTracingDriver::OpenTracingDriver(Stats::Scope& scope)
     : tracer_stats_{OPENTRACING_TRACER_STATS(POOL_COUNTER_PREFIX(scope, "tracing.opentracing."))} {}
 
 Tracing::SpanPtr OpenTracingDriver::startSpan(const Tracing::Config& config,
-                                              Http::RequestHeaderMap& request_headers,
+                                              Tracing::TraceContext& trace_context,
                                               const std::string& operation_name,
                                               SystemTime start_time,
                                               const Tracing::Decision tracing_decision) {
   const PropagationMode propagation_mode = this->propagationMode();
   const opentracing::Tracer& tracer = this->tracer();
   std::unique_ptr<opentracing::Span> active_span;
   std::unique_ptr<opentracing::SpanContext> parent_span_ctx;
-  if (propagation_mode == PropagationMode::SingleHeader &&
-      request_headers.getInline(ot_span_context_handle.handle())) {
+
+  const auto entry = trace_context.getTraceContext(Http::CustomHeaders::get().OtSpanContext);
+  if (propagation_mode == PropagationMode::SingleHeader && entry.has_value()) {
     opentracing::expected<std::unique_ptr<opentracing::SpanContext>> parent_span_ctx_maybe;
-    std::string parent_context = Base64::decode(
-        std::string(request_headers.getInlineValue(ot_span_context_handle.handle())));
+    std::string parent_context = Base64::decode(std::string(entry.value()));
 
     if (!parent_context.empty()) {
       InputConstMemoryStream istream{parent_context.data(), parent_context.size()};
@@ -187,7 +199,7 @@ Tracing::SpanPtr OpenTracingDriver::startSpan(const Tracing::Config& config,
       tracerStats().span_context_extraction_error_.inc();
     }
   } else if (propagation_mode == PropagationMode::TracerNative) {
-    const OpenTracingHTTPHeadersReader reader{request_headers};
+    const OpenTracingHeadersReader reader{trace_context};
     opentracing::expected<std::unique_ptr<opentracing::SpanContext>> parent_span_ctx_maybe =
         tracer.Extract(reader);
     if (parent_span_ctx_maybe) {

diff --git a/source/extensions/tracers/common/ot/opentracing_driver_impl.h b/source/extensions/tracers/common/ot/opentracing_driver_impl.h
@@ -37,7 +37,7 @@ class OpenTracingSpan : public Tracing::Span, Logger::Loggable<Logger::Id::traci
   void setOperation(absl::string_view operation) override;
   void setTag(absl::string_view name, const absl::string_view) override;
   void log(SystemTime timestamp, const std::string& event) override;
-  void injectContext(Http::RequestHeaderMap& request_headers) override;
+  void injectContext(Tracing::TraceContext& trace_context) override;
   Tracing::SpanPtr spawnChild(const Tracing::Config& config, const std::string& name,
                               SystemTime start_time) override;
   void setSampled(bool) override;
@@ -64,7 +64,7 @@ class OpenTracingDriver : public Tracing::Driver, protected Logger::Loggable<Log
   explicit OpenTracingDriver(Stats::Scope& scope);
 
   // Tracer::TracingDriver
-  Tracing::SpanPtr startSpan(const Tracing::Config& config, Http::RequestHeaderMap& request_headers,
+  Tracing::SpanPtr startSpan(const Tracing::Config& config, Tracing::TraceContext& trace_context,
                              const std::string& operation_name, SystemTime start_time,
                              const Tracing::Decision tracing_decision) override;