pass header map *

envoyproxy · Jun 9, 2017 · 5b91a78 · 5b91a78
1 parent 27b0875
commit 5b91a78
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 24 deletions.
diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc
@@ -339,7 +339,7 @@ ConnectionManagerImpl::ActiveStream::~ActiveStream() {
   if (request_info_.healthCheck()) {
     connection_manager_.config_.tracingStats().health_check_.inc();
   } else {
-    Tracing::HttpConnManFinalizerImpl finalizer{*request_headers_, request_info_, *this};
+    Tracing::HttpConnManFinalizerImpl finalizer(request_headers_.get(), request_info_, *this);
     active_span_->finishSpan(finalizer);
   }
 

diff --git a/source/common/tracing/http_tracer_impl.cc b/source/common/tracing/http_tracer_impl.cc
@@ -116,34 +116,36 @@ Decision HttpTracerUtility::isTracing(const Http::AccessLog::RequestInfo& reques
   NOT_REACHED;
 }
 
-HttpConnManFinalizerImpl::HttpConnManFinalizerImpl(Http::HeaderMap& request_headers,
+HttpConnManFinalizerImpl::HttpConnManFinalizerImpl(Http::HeaderMap* request_headers,
                                                    Http::AccessLog::RequestInfo& request_info,
                                                    Config& tracing_config)
     : request_headers_(request_headers), request_info_(request_info),
       tracing_config_(tracing_config) {}
 
 void HttpConnManFinalizerImpl::finalize(Span& span) {
   // Pre response data.
-  span.setTag("guid:x-request-id", std::string(request_headers_.RequestId()->value().c_str()));
-  span.setTag("request_line", buildRequestLine(request_headers_, request_info_));
-  span.setTag("request_size", std::to_string(request_info_.bytesReceived()));
-  span.setTag("host_header", valueOrDefault(request_headers_.Host(), "-"));
-  span.setTag("downstream_cluster",
-              valueOrDefault(request_headers_.EnvoyDownstreamServiceCluster(), "-"));
-  span.setTag("user_agent", valueOrDefault(request_headers_.UserAgent(), "-"));
-
-  if (request_headers_.ClientTraceId()) {
-    span.setTag("guid:x-client-trace-id",
-                std::string(request_headers_.ClientTraceId()->value().c_str()));
-  }
+  if (request_headers_) {
+    span.setTag("guid:x-request-id", std::string(request_headers_->RequestId()->value().c_str()));
+    span.setTag("request_line", buildRequestLine(*request_headers_, request_info_));
+    span.setTag("host_header", valueOrDefault(request_headers_->Host(), "-"));
+    span.setTag("downstream_cluster",
+                valueOrDefault(request_headers_->EnvoyDownstreamServiceCluster(), "-"));
+    span.setTag("user_agent", valueOrDefault(request_headers_->UserAgent(), "-"));
+
+    if (request_headers_->ClientTraceId()) {
+      span.setTag("guid:x-client-trace-id",
+                  std::string(request_headers_->ClientTraceId()->value().c_str()));
+    }
 
-  // Build tags based on the custom headers.
-  for (const Http::LowerCaseString& header : tracing_config_.requestHeadersForTags()) {
-    const Http::HeaderEntry* entry = request_headers_.get(header);
-    if (entry) {
-      span.setTag(header.get(), entry->value().c_str());
+    // Build tags based on the custom headers.
+    for (const Http::LowerCaseString& header : tracing_config_.requestHeadersForTags()) {
+      const Http::HeaderEntry* entry = request_headers_->get(header);
+      if (entry) {
+        span.setTag(header.get(), entry->value().c_str());
+      }
     }
   }
+  span.setTag("request_size", std::to_string(request_info_.bytesReceived()));
 
   // Post response data.
   span.setTag("response_code", buildResponseCode(request_info_));

diff --git a/source/common/tracing/http_tracer_impl.h b/source/common/tracing/http_tracer_impl.h
@@ -82,13 +82,13 @@ class NullFinalizer : public SpanFinalizer {
  */
 class HttpConnManFinalizerImpl : public SpanFinalizer {
 public:
-  HttpConnManFinalizerImpl(Http::HeaderMap& request_headers,
+  HttpConnManFinalizerImpl(Http::HeaderMap* request_headers,
                            Http::AccessLog::RequestInfo& request_info, Config& tracing_config);
 
   void finalize(Span& span) override;
 
 private:
-  Http::HeaderMap& request_headers_;
+  Http::HeaderMap* request_headers_;
   Http::AccessLog::RequestInfo& request_info_;
   Config& tracing_config_;
 };

diff --git a/test/common/tracing/http_tracer_impl_test.cc b/test/common/tracing/http_tracer_impl_test.cc
@@ -254,7 +254,26 @@ TEST(HttpConnManFinalizerImpl, OriginalAndLongPath) {
   EXPECT_CALL(*span, setTag("request_line", "GET " + expected_path + " HTTP/2"));
   NiceMock<MockConfig> config;
 
-  HttpConnManFinalizerImpl finalizer{request_headers, request_info, config};
+  HttpConnManFinalizerImpl finalizer(&request_headers, request_info, config);
+  finalizer.finalize(*span);
+}
+
+TEST(HttpConnManFinalizerImpl, NullRequestHeaders) {
+  std::unique_ptr<NiceMock<MockSpan>> span(new NiceMock<MockSpan>());
+  NiceMock<Http::AccessLog::MockRequestInfo> request_info;
+
+  EXPECT_CALL(request_info, bytesReceived()).WillOnce(Return(10));
+  EXPECT_CALL(request_info, bytesSent()).WillOnce(Return(11));
+  Optional<uint32_t> response_code;
+  EXPECT_CALL(request_info, responseCode()).WillRepeatedly(ReturnRef(response_code));
+
+  EXPECT_CALL(*span, setTag("response_code", "0"));
+  EXPECT_CALL(*span, setTag("response_size", "11"));
+  EXPECT_CALL(*span, setTag("response_flags", "-"));
+  EXPECT_CALL(*span, setTag("request_size", "10"));
+  NiceMock<MockConfig> config;
+
+  HttpConnManFinalizerImpl finalizer(nullptr, request_info, config);
   finalizer.finalize(*span);
 }
 
@@ -288,7 +307,7 @@ TEST(HttpConnManFinalizerImpl, SpanOptionalHeaders) {
 
   NiceMock<MockConfig> config;
 
-  HttpConnManFinalizerImpl finalizer{request_headers, request_info, config};
+  HttpConnManFinalizerImpl finalizer(&request_headers, request_info, config);
   finalizer.finalize(*span);
 }
 
@@ -340,7 +359,7 @@ TEST(HttpConnManFinalizerImpl, SpanPopulatedFailureResponse) {
   EXPECT_CALL(*span, setTag("response_size", "100"));
   EXPECT_CALL(*span, setTag("response_flags", "UT"));
 
-  HttpConnManFinalizerImpl finalizer{request_headers, request_info, config};
+  HttpConnManFinalizerImpl finalizer(&request_headers, request_info, config);
   finalizer.finalize(*span);
 }