From 9b270000d130af829d1c87daeace5e2fd335c250 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Tue, 31 Jan 2017 15:10:48 -0800 Subject: [PATCH 1/2] use access_log handler. --- src/envoy/prototype/api_manager_filter.cc | 56 ++++++++++++++--------- src/envoy/prototype/envoy-esp.conf | 3 +- src/envoy/repositories.bzl | 2 +- 3 files changed, 37 insertions(+), 24 deletions(-) diff --git a/src/envoy/prototype/api_manager_filter.cc b/src/envoy/prototype/api_manager_filter.cc index 87a3beeb48a..6cf63e90d6b 100644 --- a/src/envoy/prototype/api_manager_filter.cc +++ b/src/envoy/prototype/api_manager_filter.cc @@ -163,16 +163,23 @@ class Request : public google::api_manager::Request { }; class Response : public google::api_manager::Response { + const AccessLog::RequestInfo& request_info_; + + public: + Response(const AccessLog::RequestInfo& request_info) + : request_info_(request_info) {} + google::api_manager::utils::Status GetResponseStatus() { return google::api_manager::utils::Status::OK; } - std::size_t GetRequestSize() { return 0; } + std::size_t GetRequestSize() { return request_info_.bytesReceived(); } - std::size_t GetResponseSize() { return 0; } + std::size_t GetResponseSize() { return request_info_.bytesSent(); } google::api_manager::utils::Status GetLatencyInfo( google::api_manager::service_control::LatencyInfo* info) { + info->request_time_ms = request_info_.duration().count(); return google::api_manager::utils::Status::OK; } }; @@ -180,6 +187,7 @@ class Response : public google::api_manager::Response { const Http::HeaderMapImpl BadRequest{{Http::Headers::get().Status, "400"}}; class Instance : public Http::StreamFilter, + public Http::AccessLog::Instance, public Logger::Loggable { private: std::shared_ptr api_manager_; @@ -207,12 +215,12 @@ class Instance : public Http::StreamFilter, : api_manager_(config->api_manager()), state_(NotStarted), initiating_call_(false) { - log().debug("Called ApiManager::Instance : {}", __func__); + Log().debug("Called ApiManager::Instance : {}", __func__); } FilterHeadersStatus decodeHeaders(HeaderMap& headers, bool end_stream) override { - log().debug("Called ApiManager::Instance : {}", __func__); + Log().debug("Called ApiManager::Instance : {}", __func__); std::unique_ptr request( new Request(headers, decoder_callbacks_->downstreamAddress(), getRouteVirtualHost(headers))); @@ -227,13 +235,13 @@ class Instance : public Http::StreamFilter, if (state_ == Complete) { return FilterHeadersStatus::Continue; } - log().debug("Called ApiManager::Instance : {} Stop", __func__); + Log().debug("Called ApiManager::Instance : {} Stop", __func__); return FilterHeadersStatus::StopIteration; } FilterDataStatus decodeData(Buffer::Instance& data, bool end_stream) override { - log().debug("Called ApiManager::Instance : {} ({}, {})", __func__, + Log().debug("Called ApiManager::Instance : {} ({}, {})", __func__, data.length(), end_stream); if (state_ == Calling) { return FilterDataStatus::StopIterationAndBuffer; @@ -242,7 +250,7 @@ class Instance : public Http::StreamFilter, } FilterTrailersStatus decodeTrailers(HeaderMap& trailers) override { - log().debug("Called ApiManager::Instance : {}", __func__); + Log().debug("Called ApiManager::Instance : {}", __func__); if (state_ == Calling) { return FilterTrailersStatus::StopIteration; } @@ -250,13 +258,13 @@ class Instance : public Http::StreamFilter, } void setDecoderFilterCallbacks( StreamDecoderFilterCallbacks& callbacks) override { - log().debug("Called ApiManager::Instance : {}", __func__); + Log().debug("Called ApiManager::Instance : {}", __func__); decoder_callbacks_ = &callbacks; decoder_callbacks_->addResetStreamCallback( [this]() { state_ = Responded; }); } void completeCheck(const google::api_manager::utils::Status& status) { - log().debug("Called ApiManager::Instance : check complete {}", + Log().debug("Called ApiManager::Instance : check complete {}", status.ToJson()); if (!status.ok() && state_ != Responded) { state_ = Responded; @@ -275,31 +283,35 @@ class Instance : public Http::StreamFilter, virtual FilterHeadersStatus encodeHeaders(HeaderMap& headers, bool end_stream) override { - log().debug("Called ApiManager::Instance : {}", __func__); + Log().debug("Called ApiManager::Instance : {}", __func__); return FilterHeadersStatus::Continue; } virtual FilterDataStatus encodeData(Buffer::Instance& data, bool end_stream) override { - log().debug("Called ApiManager::Instance : {}", __func__); + Log().debug("Called ApiManager::Instance : {}", __func__); return FilterDataStatus::Continue; } virtual FilterTrailersStatus encodeTrailers(HeaderMap& trailers) override { - log().debug("Called ApiManager::Instance : {}", __func__); + Log().debug("Called ApiManager::Instance : {}", __func__); return FilterTrailersStatus::Continue; } virtual void setEncoderFilterCallbacks( StreamEncoderFilterCallbacks& callbacks) override { - log().debug("Called ApiManager::Instance : {}", __func__); + Log().debug("Called ApiManager::Instance : {}", __func__); encoder_callbacks_ = &callbacks; } - // note: cannot extend ~ActiveStream for access log, placing it here - ~Instance() { - log().debug("Called ApiManager::Instance : {}", __func__); - std::unique_ptr response(new Response()); - request_handler_->Report(std::move(response), - [this]() { log().debug("Report returns"); }); + virtual void log(const HeaderMap* request_headers, + const HeaderMap* response_headers, + const AccessLog::RequestInfo& request_info) override { + Log().debug("Called ApiManager::Instance : {}", __func__); + std::unique_ptr response( + new Response(request_info)); + request_handler_->Report(std::move(response), []() {}); } + // There are two log() functions. Use this Log() to redirect to base class + // log(). + spdlog::logger& Log() { return Logger::Loggable::log(); } }; } } @@ -320,8 +332,10 @@ class ApiManagerConfig : public HttpFilterConfigFactory { new Http::ApiManager::Config(config, server)); return [api_manager_config]( Http::FilterChainFactoryCallbacks& callbacks) -> void { - auto instance = new Http::ApiManager::Instance(api_manager_config); - callbacks.addStreamFilter(Http::StreamFilterPtr{instance}); + std::shared_ptr instance( + new Http::ApiManager::Instance(api_manager_config)); + callbacks.addStreamFilter(Http::StreamFilterPtr(instance)); + callbacks.addAccessLogHandler(Http::AccessLog::InstancePtr(instance)); }; } }; diff --git a/src/envoy/prototype/envoy-esp.conf b/src/envoy/prototype/envoy-esp.conf index d3ec54f405d..27ea9f1b397 100644 --- a/src/envoy/prototype/envoy-esp.conf +++ b/src/envoy/prototype/envoy-esp.conf @@ -85,6 +85,5 @@ ] } ] - }, - "tracing_enabled": "true" + } } diff --git a/src/envoy/repositories.bzl b/src/envoy/repositories.bzl index e19efc6dce1..d14053308e1 100644 --- a/src/envoy/repositories.bzl +++ b/src/envoy/repositories.bzl @@ -629,6 +629,6 @@ cc_test( native.new_git_repository( name = "envoy_git", remote = "https://github.com/lyft/envoy.git", - commit = "39f42378fa41c10996d4c3ffba534951de30ceb8", + commit = "0bac7508c6803ec315c2228672728281b99149bd", build_file_content = BUILD, ) From 62f29282a3003b9d41a20d5e1c0fe9798cf6bba4 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Wed, 1 Feb 2017 11:43:56 -0800 Subject: [PATCH 2/2] Not to use Loggable base class. --- src/envoy/prototype/api_manager_filter.cc | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/envoy/prototype/api_manager_filter.cc b/src/envoy/prototype/api_manager_filter.cc index 6cf63e90d6b..3983a4d8b41 100644 --- a/src/envoy/prototype/api_manager_filter.cc +++ b/src/envoy/prototype/api_manager_filter.cc @@ -186,9 +186,7 @@ class Response : public google::api_manager::Response { const Http::HeaderMapImpl BadRequest{{Http::Headers::get().Status, "400"}}; -class Instance : public Http::StreamFilter, - public Http::AccessLog::Instance, - public Logger::Loggable { +class Instance : public Http::StreamFilter, public Http::AccessLog::Instance { private: std::shared_ptr api_manager_; std::unique_ptr @@ -309,9 +307,12 @@ class Instance : public Http::StreamFilter, new Response(request_info)); request_handler_->Report(std::move(response), []() {}); } - // There are two log() functions. Use this Log() to redirect to base class - // log(). - spdlog::logger& Log() { return Logger::Loggable::log(); } + + spdlog::logger& Log() { + static spdlog::logger& instance = + Logger::Registry::getLog(Logger::Id::http); + return instance; + } }; } }