From 6335990189b0cb33af39d0962c9ad34e217c0da7 Mon Sep 17 00:00:00 2001 From: Pengyuan Bian Date: Tue, 23 Apr 2019 19:51:55 -0700 Subject: [PATCH 1/6] Retrieve tracing header at report time --- include/istio/control/http/report_data.h | 3 ++ include/istio/utils/attributes_builder.h | 13 +++++++++ src/envoy/http/mixer/filter.cc | 4 +-- src/envoy/http/mixer/report_data.h | 24 ++++++++++----- src/envoy/utils/BUILD | 1 + src/envoy/utils/trace_headers.h | 19 ++++++++++++ src/envoy/utils/utils.cc | 23 +++++++++++++++ src/envoy/utils/utils.h | 5 ++++ src/istio/control/http/attributes_builder.cc | 4 +++ .../control/http/attributes_builder_test.cc | 29 +++++++++++++++++-- src/istio/control/http/mock_report_data.h | 1 + 11 files changed, 114 insertions(+), 12 deletions(-) create mode 100644 src/envoy/utils/trace_headers.h diff --git a/include/istio/control/http/report_data.h b/include/istio/control/http/report_data.h index 8cf7326fe0f..2ecc843a570 100644 --- a/include/istio/control/http/report_data.h +++ b/include/istio/control/http/report_data.h @@ -34,6 +34,9 @@ class ReportData { // Get response HTTP headers. virtual std::map GetResponseHeaders() const = 0; + // Get tracing headers from HTTP request headers. + virtual void GetTracingHeaders(std::map &) const = 0; + // Get additional report info. struct ReportInfo { uint64_t response_total_size; diff --git a/include/istio/utils/attributes_builder.h b/include/istio/utils/attributes_builder.h index 4d1f0996284..43d56b52550 100644 --- a/include/istio/utils/attributes_builder.h +++ b/include/istio/utils/attributes_builder.h @@ -91,6 +91,19 @@ class AttributesBuilder { } } + void InsertStringMap(const std::string &key, + const std::map &string_map) { + if (string_map.size() == 0) { + return; + } + auto entries = (*attributes_->mutable_attributes())[key] + .mutable_string_map_value() + ->mutable_entries(); + for (const auto &map_it : string_map) { + (*entries)[map_it.first] = map_it.second; + } + } + void AddProtoStructStringMap(const std::string &key, const google::protobuf::Struct &struct_map) { if (struct_map.fields().empty()) { diff --git a/src/envoy/http/mixer/filter.cc b/src/envoy/http/mixer/filter.cc index 8c310fa27df..3daccaf1971 100644 --- a/src/envoy/http/mixer/filter.cc +++ b/src/envoy/http/mixer/filter.cc @@ -232,8 +232,8 @@ void Filter::log(const HeaderMap* request_headers, CheckData check_data(*request_headers, stream_info.dynamicMetadata(), decoder_callbacks_->connection()); // response trailer header is not counted to response total size. - ReportData report_data(response_headers, response_trailers, stream_info, - request_total_size_); + ReportData report_data(request_headers, response_headers, response_trailers, + stream_info, request_total_size_); handler_->Report(&check_data, &report_data); } diff --git a/src/envoy/http/mixer/report_data.h b/src/envoy/http/mixer/report_data.h index 33f4c6b0ed2..c267a179635 100644 --- a/src/envoy/http/mixer/report_data.h +++ b/src/envoy/http/mixer/report_data.h @@ -22,6 +22,7 @@ #include "extensions/filters/http/well_known_names.h" #include "google/protobuf/struct.pb.h" #include "include/istio/control/http/controller.h" +#include "src/envoy/utils/trace_headers.h" #include "src/envoy/utils/utils.h" namespace Envoy { @@ -52,22 +53,25 @@ bool ExtractGrpcStatus(const HeaderMap *headers, class ReportData : public ::istio::control::http::ReportData, public Logger::Loggable { - const HeaderMap *headers_; + const HeaderMap *request_headers_; + const HeaderMap *response_headers_; const HeaderMap *trailers_; const StreamInfo::StreamInfo &info_; uint64_t response_total_size_; uint64_t request_total_size_; public: - ReportData(const HeaderMap *headers, const HeaderMap *response_trailers, + ReportData(const HeaderMap *request_headers, const HeaderMap *response_headers, + const HeaderMap *response_trailers, const StreamInfo::StreamInfo &info, uint64_t request_total_size) - : headers_(headers), + : request_headers_(request_headers), + response_headers_(response_headers), trailers_(response_trailers), info_(info), response_total_size_(info.bytesSent()), request_total_size_(request_total_size) { - if (headers != nullptr) { - response_total_size_ += headers->byteSize(); + if (response_headers != nullptr) { + response_total_size_ += response_headers->byteSize(); } if (response_trailers != nullptr) { response_total_size_ += response_trailers->byteSize(); @@ -76,8 +80,8 @@ class ReportData : public ::istio::control::http::ReportData, std::map GetResponseHeaders() const override { std::map header_map; - if (headers_) { - Utils::ExtractHeaders(*headers_, ResponseHeaderExclusives, header_map); + if (response_headers_) { + Utils::ExtractHeaders(*response_headers_, ResponseHeaderExclusives, header_map); } if (trailers_) { Utils::ExtractHeaders(*trailers_, ResponseHeaderExclusives, header_map); @@ -85,6 +89,10 @@ class ReportData : public ::istio::control::http::ReportData, return header_map; } + void GetTracingHeaders(std::map& tracing_headers) const override { + Utils::FindHeaders(*request_headers_, Utils::TracingHeaderSet, tracing_headers); + } + void GetReportInfo( ::istio::control::http::ReportData::ReportInfo *data) const override { data->request_body_size = info_.bytesReceived(); @@ -119,7 +127,7 @@ class ReportData : public ::istio::control::http::ReportData, // Check trailer first. // If not response body, grpc-status is in response headers. return ExtractGrpcStatus(trailers_, status) || - ExtractGrpcStatus(headers_, status); + ExtractGrpcStatus(response_headers_, status); } // Get Rbac related attributes. diff --git a/src/envoy/utils/BUILD b/src/envoy/utils/BUILD index 3415ef32633..cf17b3f89b0 100644 --- a/src/envoy/utils/BUILD +++ b/src/envoy/utils/BUILD @@ -57,6 +57,7 @@ envoy_cc_library( "header_update.h", "mixer_control.h", "stats.h", + "trace_headers.h", "utils.h", ], repository = "@envoy", diff --git a/src/envoy/utils/trace_headers.h b/src/envoy/utils/trace_headers.h new file mode 100644 index 00000000000..392eae1565b --- /dev/null +++ b/src/envoy/utils/trace_headers.h @@ -0,0 +1,19 @@ +#pragma once + +#include + +namespace Envoy { +namespace Utils { + +// Zipkin B3 headers +const std::string kTraceID = "x-b3-traceid"; +const std::string kSpanID = "x-b3-spanid"; +const std::string kParentSpanID = "x-b3-parentspanid"; +const std::string kSampled = "x-b3-sampled"; + +const std::set TracingHeaderSet = { + kTraceID, kSpanID, kParentSpanID, kSampled, +}; + +} // namespace Utils +} // namespace Envoy diff --git a/src/envoy/utils/utils.cc b/src/envoy/utils/utils.cc index b45e042a1e7..681f455dc5b 100644 --- a/src/envoy/utils/utils.cc +++ b/src/envoy/utils/utils.cc @@ -59,6 +59,29 @@ void ExtractHeaders(const Http::HeaderMap& header_map, &ctx); } +void FindHeaders(const Http::HeaderMap& header_map, + const std::set& inclusives, + std::map& headers) { + struct Context { + Context(const std::set& inclusives, + std::map& headers) + : inclusives(inclusives), headers(headers) {} + const std::set& inclusives; + std::map& headers; + }; + Context ctx(inclusives, headers); + header_map.iterate( + [](const Http::HeaderEntry& header, + void* context) -> Http::HeaderMap::Iterate { + Context* ctx = static_cast(context); + if (ctx->inclusives.count(header.key().c_str()) != 0) { + ctx->headers[header.key().c_str()] = header.value().c_str(); + } + return Http::HeaderMap::Iterate::Continue; + }, + &ctx); +} + bool GetIpPort(const Network::Address::Ip* ip, std::string* str_ip, int* port) { if (ip) { *port = ip->port(); diff --git a/src/envoy/utils/utils.h b/src/envoy/utils/utils.h index 3fa4aac5b21..bce7af6eb7a 100644 --- a/src/envoy/utils/utils.h +++ b/src/envoy/utils/utils.h @@ -31,6 +31,11 @@ void ExtractHeaders(const Http::HeaderMap& header_map, const std::set& exclusives, std::map& headers); +// Find the given headers from the header map and extract them out to the string map. +void FindHeaders(const Http::HeaderMap& header_map, + const std::set& inclusives, + std::map& headers); + // Get ip and port from Envoy ip. bool GetIpPort(const Network::Address::Ip* ip, std::string* str_ip, int* port); diff --git a/src/istio/control/http/attributes_builder.cc b/src/istio/control/http/attributes_builder.cc index 32ffa5c8423..ab5e15a1f6f 100644 --- a/src/istio/control/http/attributes_builder.cc +++ b/src/istio/control/http/attributes_builder.cc @@ -226,6 +226,10 @@ void AttributesBuilder::ExtractReportAttributes( report_data->GetResponseHeaders(); builder.AddStringMap(utils::AttributeName::kResponseHeaders, headers); + std::map tracing_headers; + report_data->GetTracingHeaders(tracing_headers); + builder.InsertStringMap(utils::AttributeName::kRequestHeaders, tracing_headers); + builder.AddTimestamp(utils::AttributeName::kResponseTime, std::chrono::system_clock::now()); diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc index 09c84bc1567..19ed3e6db19 100644 --- a/src/istio/control/http/attributes_builder_test.cc +++ b/src/istio/control/http/attributes_builder_test.cc @@ -15,15 +15,15 @@ #include "src/istio/control/http/attributes_builder.h" -#include "gmock/gmock.h" #include "google/protobuf/stubs/status.h" #include "google/protobuf/text_format.h" #include "google/protobuf/util/message_differencer.h" -#include "gtest/gtest.h" #include "include/istio/utils/attribute_names.h" #include "include/istio/utils/attributes_builder.h" #include "src/istio/control/http/mock_check_data.h" #include "src/istio/control/http/mock_report_data.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" using ::google::protobuf::TextFormat; using ::google::protobuf::util::MessageDifferencer; @@ -350,6 +350,21 @@ attributes { int64_value: 8080 } } +attributes { + key: "request.headers" + value { + string_map_value { + entries { + key: "x-b3-traceid" + value: "abc" + } + entries { + key: "x-b3-spanid" + value: "def" + } + } + } +} attributes { key: "response.headers" value { @@ -765,6 +780,11 @@ TEST(AttributesBuilderTest, TestReportAttributes) { map["server"] = "my-server"; return map; })); + EXPECT_CALL(mock_data, GetTracingHeaders(_)) + .WillOnce(Invoke([](std::map &m) { + m["x-b3-traceid"] = "abc"; + m["x-b3-spanid"] = "def"; + })); EXPECT_CALL(mock_data, GetReportInfo(_)) .WillOnce(Invoke([](ReportData::ReportInfo *info) { info->request_body_size = 100; @@ -845,6 +865,11 @@ TEST(AttributesBuilderTest, TestReportAttributesWithDestIP) { map["server"] = "my-server"; return map; })); + EXPECT_CALL(mock_data, GetTracingHeaders(_)) + .WillOnce(Invoke([](std::map &m) { + m["x-b3-traceid"] = "abc"; + m["x-b3-spanid"] = "def"; + })); EXPECT_CALL(mock_data, GetReportInfo(_)) .WillOnce(Invoke([](ReportData::ReportInfo *info) { info->request_body_size = 100; diff --git a/src/istio/control/http/mock_report_data.h b/src/istio/control/http/mock_report_data.h index 64c0402ef9b..582f9d28270 100644 --- a/src/istio/control/http/mock_report_data.h +++ b/src/istio/control/http/mock_report_data.h @@ -27,6 +27,7 @@ namespace http { class MockReportData : public ReportData { public: MOCK_CONST_METHOD0(GetResponseHeaders, std::map()); + MOCK_CONST_METHOD1(GetTracingHeaders, void(std::map&)); MOCK_CONST_METHOD1(GetReportInfo, void(ReportInfo *info)); MOCK_CONST_METHOD2(GetDestinationIpPort, bool(std::string *ip, int *port)); MOCK_CONST_METHOD1(GetDestinationUID, bool(std::string *ip)); From 077a9b6b56c4e4c741b8486b3652091e4c840fc1 Mon Sep 17 00:00:00 2001 From: Pengyuan Bian Date: Tue, 23 Apr 2019 20:27:08 -0700 Subject: [PATCH 2/6] add license header --- src/envoy/utils/trace_headers.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/envoy/utils/trace_headers.h b/src/envoy/utils/trace_headers.h index 392eae1565b..f38f391e448 100644 --- a/src/envoy/utils/trace_headers.h +++ b/src/envoy/utils/trace_headers.h @@ -1,3 +1,18 @@ +/* Copyright 2019 Istio Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + #pragma once #include From 8b900e16d0dfbb77070e7de736f31ee18724b7a1 Mon Sep 17 00:00:00 2001 From: Pengyuan Bian Date: Tue, 23 Apr 2019 20:52:39 -0700 Subject: [PATCH 3/6] format --- include/istio/control/http/report_data.h | 3 ++- src/envoy/http/mixer/report_data.h | 12 ++++++++---- src/envoy/utils/trace_headers.h | 5 ++++- src/envoy/utils/utils.cc | 4 ++-- src/envoy/utils/utils.h | 7 ++++--- src/istio/control/http/attributes_builder.cc | 3 ++- src/istio/control/http/attributes_builder_test.cc | 4 ++-- src/istio/control/http/mock_report_data.h | 3 ++- 8 files changed, 26 insertions(+), 15 deletions(-) diff --git a/include/istio/control/http/report_data.h b/include/istio/control/http/report_data.h index 2ecc843a570..dac9998a77e 100644 --- a/include/istio/control/http/report_data.h +++ b/include/istio/control/http/report_data.h @@ -35,7 +35,8 @@ class ReportData { virtual std::map GetResponseHeaders() const = 0; // Get tracing headers from HTTP request headers. - virtual void GetTracingHeaders(std::map &) const = 0; + virtual void GetTracingHeaders( + std::map &) const = 0; // Get additional report info. struct ReportInfo { diff --git a/src/envoy/http/mixer/report_data.h b/src/envoy/http/mixer/report_data.h index c267a179635..5a8a70bc903 100644 --- a/src/envoy/http/mixer/report_data.h +++ b/src/envoy/http/mixer/report_data.h @@ -61,7 +61,8 @@ class ReportData : public ::istio::control::http::ReportData, uint64_t request_total_size_; public: - ReportData(const HeaderMap *request_headers, const HeaderMap *response_headers, + ReportData(const HeaderMap *request_headers, + const HeaderMap *response_headers, const HeaderMap *response_trailers, const StreamInfo::StreamInfo &info, uint64_t request_total_size) : request_headers_(request_headers), @@ -81,7 +82,8 @@ class ReportData : public ::istio::control::http::ReportData, std::map GetResponseHeaders() const override { std::map header_map; if (response_headers_) { - Utils::ExtractHeaders(*response_headers_, ResponseHeaderExclusives, header_map); + Utils::ExtractHeaders(*response_headers_, ResponseHeaderExclusives, + header_map); } if (trailers_) { Utils::ExtractHeaders(*trailers_, ResponseHeaderExclusives, header_map); @@ -89,8 +91,10 @@ class ReportData : public ::istio::control::http::ReportData, return header_map; } - void GetTracingHeaders(std::map& tracing_headers) const override { - Utils::FindHeaders(*request_headers_, Utils::TracingHeaderSet, tracing_headers); + void GetTracingHeaders( + std::map &tracing_headers) const override { + Utils::FindHeaders(*request_headers_, Utils::TracingHeaderSet, + tracing_headers); } void GetReportInfo( diff --git a/src/envoy/utils/trace_headers.h b/src/envoy/utils/trace_headers.h index f38f391e448..b73b983aaae 100644 --- a/src/envoy/utils/trace_headers.h +++ b/src/envoy/utils/trace_headers.h @@ -27,7 +27,10 @@ const std::string kParentSpanID = "x-b3-parentspanid"; const std::string kSampled = "x-b3-sampled"; const std::set TracingHeaderSet = { - kTraceID, kSpanID, kParentSpanID, kSampled, + kTraceID, + kSpanID, + kParentSpanID, + kSampled, }; } // namespace Utils diff --git a/src/envoy/utils/utils.cc b/src/envoy/utils/utils.cc index 681f455dc5b..74be36768cd 100644 --- a/src/envoy/utils/utils.cc +++ b/src/envoy/utils/utils.cc @@ -60,8 +60,8 @@ void ExtractHeaders(const Http::HeaderMap& header_map, } void FindHeaders(const Http::HeaderMap& header_map, - const std::set& inclusives, - std::map& headers) { + const std::set& inclusives, + std::map& headers) { struct Context { Context(const std::set& inclusives, std::map& headers) diff --git a/src/envoy/utils/utils.h b/src/envoy/utils/utils.h index bce7af6eb7a..7102b454a11 100644 --- a/src/envoy/utils/utils.h +++ b/src/envoy/utils/utils.h @@ -31,10 +31,11 @@ void ExtractHeaders(const Http::HeaderMap& header_map, const std::set& exclusives, std::map& headers); -// Find the given headers from the header map and extract them out to the string map. +// Find the given headers from the header map and extract them out to the string +// map. void FindHeaders(const Http::HeaderMap& header_map, - const std::set& inclusives, - std::map& headers); + const std::set& inclusives, + std::map& headers); // Get ip and port from Envoy ip. bool GetIpPort(const Network::Address::Ip* ip, std::string* str_ip, int* port); diff --git a/src/istio/control/http/attributes_builder.cc b/src/istio/control/http/attributes_builder.cc index ab5e15a1f6f..8c3fc539a06 100644 --- a/src/istio/control/http/attributes_builder.cc +++ b/src/istio/control/http/attributes_builder.cc @@ -228,7 +228,8 @@ void AttributesBuilder::ExtractReportAttributes( std::map tracing_headers; report_data->GetTracingHeaders(tracing_headers); - builder.InsertStringMap(utils::AttributeName::kRequestHeaders, tracing_headers); + builder.InsertStringMap(utils::AttributeName::kRequestHeaders, + tracing_headers); builder.AddTimestamp(utils::AttributeName::kResponseTime, std::chrono::system_clock::now()); diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc index 19ed3e6db19..d447542ce78 100644 --- a/src/istio/control/http/attributes_builder_test.cc +++ b/src/istio/control/http/attributes_builder_test.cc @@ -15,15 +15,15 @@ #include "src/istio/control/http/attributes_builder.h" +#include "gmock/gmock.h" #include "google/protobuf/stubs/status.h" #include "google/protobuf/text_format.h" #include "google/protobuf/util/message_differencer.h" +#include "gtest/gtest.h" #include "include/istio/utils/attribute_names.h" #include "include/istio/utils/attributes_builder.h" #include "src/istio/control/http/mock_check_data.h" #include "src/istio/control/http/mock_report_data.h" -#include "gmock/gmock.h" -#include "gtest/gtest.h" using ::google::protobuf::TextFormat; using ::google::protobuf::util::MessageDifferencer; diff --git a/src/istio/control/http/mock_report_data.h b/src/istio/control/http/mock_report_data.h index 582f9d28270..2b2b0fddc1d 100644 --- a/src/istio/control/http/mock_report_data.h +++ b/src/istio/control/http/mock_report_data.h @@ -27,7 +27,8 @@ namespace http { class MockReportData : public ReportData { public: MOCK_CONST_METHOD0(GetResponseHeaders, std::map()); - MOCK_CONST_METHOD1(GetTracingHeaders, void(std::map&)); + MOCK_CONST_METHOD1(GetTracingHeaders, + void(std::map &)); MOCK_CONST_METHOD1(GetReportInfo, void(ReportInfo *info)); MOCK_CONST_METHOD2(GetDestinationIpPort, bool(std::string *ip, int *port)); MOCK_CONST_METHOD1(GetDestinationUID, bool(std::string *ip)); From 05d9abe3b89c3f2f622ea002d61d844632541ddd Mon Sep 17 00:00:00 2001 From: Pengyuan Bian Date: Thu, 25 Apr 2019 23:08:04 -0700 Subject: [PATCH 4/6] add an integration test --- .../istio_http_integration_test.cc | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/test/integration/istio_http_integration_test.cc b/test/integration/istio_http_integration_test.cc index c3f7a25912a..9d209860055 100644 --- a/test/integration/istio_http_integration_test.cc +++ b/test/integration/istio_http_integration_test.cc @@ -24,6 +24,7 @@ #include "include/istio/utils/attribute_names.h" #include "mixer/v1/mixer.pb.h" #include "src/envoy/utils/filter_names.h" +#include "src/envoy/utils/trace_headers.h" #include "test/integration/http_protocol_integration.h" using ::google::protobuf::util::error::Code; @@ -102,6 +103,7 @@ constexpr char kDestinationUID[] = "kubernetes://dest.pod"; constexpr char kSourceUID[] = "kubernetes://src.pod"; constexpr char kTelemetryBackend[] = "telemetry-backend"; constexpr char kPolicyBackend[] = "policy-backend"; +constexpr char kZipkinBackend[] = "zipkin-backend"; // Generates basic test request header. Http::TestHeaderMapImpl BaseRequestHeaders() { @@ -227,6 +229,10 @@ class IstioHttpIntegrationTest : public HttpProtocolIntegrationTest { fake_upstreams_.emplace_back(new FakeUpstream( 0, FakeHttpConnection::Type::HTTP2, version_, timeSystem())); policy_upstream_ = fake_upstreams_.back().get(); + + fake_upstreams_.emplace_back(new FakeUpstream( + 0, FakeHttpConnection::Type::HTTP2, version_, timeSystem())); + zipkin_upstream_ = fake_upstreams_.back().get(); } void SetUp() override { @@ -239,6 +245,10 @@ class IstioHttpIntegrationTest : public HttpProtocolIntegrationTest { config_helper_.addConfigModifier(addCluster(kTelemetryBackend)); config_helper_.addConfigModifier(addCluster(kPolicyBackend)); + config_helper_.addConfigModifier(addCluster(kZipkinBackend)); + + config_helper_.addConfigModifier(addTracer()); + config_helper_.addConfigModifier(addTracingRate()); HttpProtocolIntegrationTest::initialize(); } @@ -247,6 +257,7 @@ class IstioHttpIntegrationTest : public HttpProtocolIntegrationTest { cleanupConnection(fake_upstream_connection_); cleanupConnection(telemetry_connection_); cleanupConnection(policy_connection_); + cleanupConnection(zipkin_connection_); } ConfigHelper::ConfigModifierFunction addNodeMetadata() { @@ -264,6 +275,27 @@ class IstioHttpIntegrationTest : public HttpProtocolIntegrationTest { }; } + ConfigHelper::ConfigModifierFunction addTracer() { + return [](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { + auto* http_tracing = bootstrap.mutable_tracing()->mutable_http(); + http_tracing->set_name("envoy.zipkin"); + auto* tracer_config_fields = http_tracing->mutable_config()->mutable_fields(); + (*tracer_config_fields)["collector_cluster"].set_string_value(kZipkinBackend); + (*tracer_config_fields)["collector_endpoint"].set_string_value("/api/v1/spans"); + }; + } + + ConfigHelper::HttpModifierFunction addTracingRate() { + return [](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& hcm) { + auto* tracing = hcm.mutable_tracing(); + tracing->set_operation_name( + envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager_Tracing_OperationName::HttpConnectionManager_Tracing_OperationName_EGRESS); + tracing->mutable_client_sampling()->set_value(100.0); + tracing->mutable_random_sampling()->set_value(100.0); + tracing->mutable_overall_sampling()->set_value(100.0); + }; + } + ConfigHelper::ConfigModifierFunction addCluster(const std::string& name) { return [name](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { auto* cluster = bootstrap.mutable_static_resources()->add_clusters(); @@ -329,6 +361,10 @@ class IstioHttpIntegrationTest : public HttpProtocolIntegrationTest { FakeUpstream* policy_upstream_{}; FakeHttpConnectionPtr policy_connection_{}; FakeStreamPtr policy_request_{}; + + FakeUpstream* zipkin_upstream_{}; + FakeHttpConnectionPtr zipkin_connection_{}; + FakeStreamPtr zipkin_request_{}; }; INSTANTIATE_TEST_CASE_P( @@ -435,5 +471,38 @@ TEST_P(IstioHttpIntegrationTest, GoodJwt) { EXPECT_STREQ("200", response->headers().Status()->value().c_str()); } +TEST_P(IstioHttpIntegrationTest, TracingHeader) { + codec_client_ = + makeHttpConnection(makeClientConnection((lookupPort("http")))); + auto response = + codec_client_->makeHeaderOnlyRequest(HeadersWithToken(kGoodToken)); + + ::istio::mixer::v1::CheckRequest check_request; + waitForPolicyRequest(&check_request); + sendPolicyResponse(); + + waitForNextUpstreamRequest(0); + // Send backend response. + upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, + true); + response->waitForEndStream(); + + ::istio::mixer::v1::ReportRequest report_request; + waitForTelemetryRequest(&report_request); + sendTelemetryResponse(); + + response->waitForEndStream(); + + EXPECT_TRUE(response->complete()); + Http::TestHeaderMapImpl upstream_headers(upstream_request_->headers()); + // Trace headers should be added into upstream request + EXPECT_TRUE(upstream_headers.has(Envoy::Utils::kTraceID)); + EXPECT_TRUE(upstream_headers.has(Envoy::Utils::kSpanID)); + EXPECT_TRUE(upstream_headers.has(Envoy::Utils::kSampled)); + + // trace id should be included in default words of report request + EXPECT_THAT(report_request.default_words(), ::testing::AllOf(Contains(upstream_headers.get_(Envoy::Utils::kSpanID)))); +} + } // namespace } // namespace Envoy From 2541782f6e95d94ed6ac48fe7cb2b7556ea9b0a4 Mon Sep 17 00:00:00 2001 From: Pengyuan Bian Date: Thu, 25 Apr 2019 23:15:34 -0700 Subject: [PATCH 5/6] format --- .../istio_http_integration_test.cc | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/test/integration/istio_http_integration_test.cc b/test/integration/istio_http_integration_test.cc index 9d209860055..cab6e496014 100644 --- a/test/integration/istio_http_integration_test.cc +++ b/test/integration/istio_http_integration_test.cc @@ -279,17 +279,23 @@ class IstioHttpIntegrationTest : public HttpProtocolIntegrationTest { return [](envoy::config::bootstrap::v2::Bootstrap& bootstrap) { auto* http_tracing = bootstrap.mutable_tracing()->mutable_http(); http_tracing->set_name("envoy.zipkin"); - auto* tracer_config_fields = http_tracing->mutable_config()->mutable_fields(); - (*tracer_config_fields)["collector_cluster"].set_string_value(kZipkinBackend); - (*tracer_config_fields)["collector_endpoint"].set_string_value("/api/v1/spans"); + auto* tracer_config_fields = + http_tracing->mutable_config()->mutable_fields(); + (*tracer_config_fields)["collector_cluster"].set_string_value( + kZipkinBackend); + (*tracer_config_fields)["collector_endpoint"].set_string_value( + "/api/v1/spans"); }; } ConfigHelper::HttpModifierFunction addTracingRate() { - return [](envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& hcm) { + return [](envoy::config::filter::network::http_connection_manager::v2:: + HttpConnectionManager& hcm) { auto* tracing = hcm.mutable_tracing(); tracing->set_operation_name( - envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager_Tracing_OperationName::HttpConnectionManager_Tracing_OperationName_EGRESS); + envoy::config::filter::network::http_connection_manager::v2:: + HttpConnectionManager_Tracing_OperationName:: + HttpConnectionManager_Tracing_OperationName_EGRESS); tracing->mutable_client_sampling()->set_value(100.0); tracing->mutable_random_sampling()->set_value(100.0); tracing->mutable_overall_sampling()->set_value(100.0); @@ -501,7 +507,9 @@ TEST_P(IstioHttpIntegrationTest, TracingHeader) { EXPECT_TRUE(upstream_headers.has(Envoy::Utils::kSampled)); // trace id should be included in default words of report request - EXPECT_THAT(report_request.default_words(), ::testing::AllOf(Contains(upstream_headers.get_(Envoy::Utils::kSpanID)))); + EXPECT_THAT( + report_request.default_words(), + ::testing::AllOf(Contains(upstream_headers.get_(Envoy::Utils::kSpanID)))); } } // namespace From 0b3d262fa815a9e2068d69bb404d2bf84e46a8f5 Mon Sep 17 00:00:00 2001 From: Pengyuan Bian Date: Thu, 25 Apr 2019 23:17:10 -0700 Subject: [PATCH 6/6] clean comment --- test/integration/istio_http_integration_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/istio_http_integration_test.cc b/test/integration/istio_http_integration_test.cc index cab6e496014..de97998fc72 100644 --- a/test/integration/istio_http_integration_test.cc +++ b/test/integration/istio_http_integration_test.cc @@ -506,7 +506,7 @@ TEST_P(IstioHttpIntegrationTest, TracingHeader) { EXPECT_TRUE(upstream_headers.has(Envoy::Utils::kSpanID)); EXPECT_TRUE(upstream_headers.has(Envoy::Utils::kSampled)); - // trace id should be included in default words of report request + // span id should be included in default words of report request EXPECT_THAT( report_request.default_words(), ::testing::AllOf(Contains(upstream_headers.get_(Envoy::Utils::kSpanID))));