From 5b91a78d09862654e8c52fb04660b55fab25c3d4 Mon Sep 17 00:00:00 2001 From: Roman Dzhabarov Date: Fri, 9 Jun 2017 12:55:40 -0700 Subject: [PATCH] pass header map * --- source/common/http/conn_manager_impl.cc | 2 +- source/common/tracing/http_tracer_impl.cc | 38 ++++++++++---------- source/common/tracing/http_tracer_impl.h | 4 +-- test/common/tracing/http_tracer_impl_test.cc | 25 +++++++++++-- 4 files changed, 45 insertions(+), 24 deletions(-) diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index f358732f695a..d5c5f3aff66f 100644 --- 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 index b050db77ae3d..59a47307ac7b 100644 --- a/source/common/tracing/http_tracer_impl.cc +++ b/source/common/tracing/http_tracer_impl.cc @@ -116,7 +116,7 @@ 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), @@ -124,26 +124,28 @@ HttpConnManFinalizerImpl::HttpConnManFinalizerImpl(Http::HeaderMap& request_head 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 index 28584248fe3d..61ebcb09cf39 100644 --- 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 index 6d2a1daf1ed3..6bfa704b0dea 100644 --- 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 config; - HttpConnManFinalizerImpl finalizer{request_headers, request_info, config}; + HttpConnManFinalizerImpl finalizer(&request_headers, request_info, config); + finalizer.finalize(*span); +} + +TEST(HttpConnManFinalizerImpl, NullRequestHeaders) { + std::unique_ptr> span(new NiceMock()); + NiceMock request_info; + + EXPECT_CALL(request_info, bytesReceived()).WillOnce(Return(10)); + EXPECT_CALL(request_info, bytesSent()).WillOnce(Return(11)); + Optional 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 config; + + HttpConnManFinalizerImpl finalizer(nullptr, request_info, config); finalizer.finalize(*span); } @@ -288,7 +307,7 @@ TEST(HttpConnManFinalizerImpl, SpanOptionalHeaders) { NiceMock 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); }