diff --git a/docs/root/configuration/http_conn_man/stats.rst b/docs/root/configuration/http_conn_man/stats.rst index 0269fefc1d4b..4188dcea4a5c 100644 --- a/docs/root/configuration/http_conn_man/stats.rst +++ b/docs/root/configuration/http_conn_man/stats.rst @@ -98,7 +98,18 @@ following statistics: Per codec statistics ----------------------- -Each codec has the option of adding per-codec statistics. Currently only http2 has codec stats. +Each codec has the option of adding per-codec statistics. Both http1 and http2 have codec stats. + +Http1 codec statistics +~~~~~~~~~~~~~~~~~~~~~~ + +All http1 statistics are rooted at *http1.* + +.. csv-table:: + :header: Name, Type, Description + :widths: 1, 1, 2 + + metadata_not_supported_error, Counter, Total number of metadata dropped during HTTP/1 encoding Http2 codec statistics ~~~~~~~~~~~~~~~~~~~~~~ diff --git a/source/common/http/codec_client.cc b/source/common/http/codec_client.cc index 6c5488ef7883..7d9ad41f3af0 100644 --- a/source/common/http/codec_client.cc +++ b/source/common/http/codec_client.cc @@ -140,7 +140,8 @@ CodecClientProd::CodecClientProd(Type type, Network::ClientConnectionPtr&& conne : CodecClient(type, std::move(connection), host, dispatcher) { switch (type) { case Type::HTTP1: { - codec_ = std::make_unique(*connection_, *this); + codec_ = std::make_unique(*connection_, + host->cluster().statsScope(), *this); break; } case Type::HTTP2: { diff --git a/source/common/http/conn_manager_utility.cc b/source/common/http/conn_manager_utility.cc index e4ff80a349bd..5f9d541693b6 100644 --- a/source/common/http/conn_manager_utility.cc +++ b/source/common/http/conn_manager_utility.cc @@ -46,8 +46,8 @@ ServerConnectionPtr ConnectionManagerUtility::autoCreateCodec( return std::make_unique(connection, callbacks, scope, http2_settings, max_request_headers_kb); } else { - return std::make_unique(connection, callbacks, http1_settings, - max_request_headers_kb); + return std::make_unique(connection, scope, callbacks, + http1_settings, max_request_headers_kb); } } diff --git a/source/common/http/http1/BUILD b/source/common/http/http1/BUILD index 98ddb6edb484..9756586598c1 100644 --- a/source/common/http/http1/BUILD +++ b/source/common/http/http1/BUILD @@ -18,6 +18,8 @@ envoy_cc_library( "//include/envoy/http:codec_interface", "//include/envoy/http:header_map_interface", "//include/envoy/network:connection_interface", + "//include/envoy/stats:stats_interface", + "//include/envoy/stats:stats_macros", "//source/common/buffer:buffer_lib", "//source/common/buffer:watermark_buffer_lib", "//source/common/common:assert_lib", diff --git a/source/common/http/http1/codec_impl.cc b/source/common/http/http1/codec_impl.cc index 255de29a6a1e..704787276616 100644 --- a/source/common/http/http1/codec_impl.cc +++ b/source/common/http/http1/codec_impl.cc @@ -164,6 +164,10 @@ void StreamEncoderImpl::encodeData(Buffer::Instance& data, bool end_stream) { void StreamEncoderImpl::encodeTrailers(const HeaderMap&) { endEncode(); } +void StreamEncoderImpl::encodeMetadata(const MetadataMapVector&) { + connection_.stats().metadata_not_supported_error_.inc(); +} + void StreamEncoderImpl::endEncode() { if (chunk_encoding_) { connection_.buffer().add(LAST_CHUNK); @@ -321,10 +325,11 @@ const ToLowerTable& ConnectionImpl::toLowerTable() { return *table; } -ConnectionImpl::ConnectionImpl(Network::Connection& connection, http_parser_type type, - uint32_t max_headers_kb) - : connection_(connection), output_buffer_([&]() -> void { this->onBelowLowWatermark(); }, - [&]() -> void { this->onAboveHighWatermark(); }), +ConnectionImpl::ConnectionImpl(Network::Connection& connection, Stats::Scope& stats, + http_parser_type type, uint32_t max_headers_kb) + : connection_(connection), stats_{ALL_HTTP1_CODEC_STATS(POOL_COUNTER_PREFIX(stats, "http1."))}, + output_buffer_([&]() -> void { this->onBelowLowWatermark(); }, + [&]() -> void { this->onAboveHighWatermark(); }), max_headers_kb_(max_headers_kb), strict_header_validation_(Runtime::runtimeFeatureEnabled( "envoy.reloadable_features.strict_header_validation")) { output_buffer_.setWatermarks(connection.bufferLimit()); @@ -506,11 +511,11 @@ void ConnectionImpl::onResetStreamBase(StreamResetReason reason) { onResetStream(reason); } -ServerConnectionImpl::ServerConnectionImpl(Network::Connection& connection, +ServerConnectionImpl::ServerConnectionImpl(Network::Connection& connection, Stats::Scope& stats, ServerConnectionCallbacks& callbacks, Http1Settings settings, uint32_t max_request_headers_kb) - : ConnectionImpl(connection, HTTP_REQUEST, max_request_headers_kb), callbacks_(callbacks), - codec_settings_(settings) {} + : ConnectionImpl(connection, stats, HTTP_REQUEST, max_request_headers_kb), + callbacks_(callbacks), codec_settings_(settings) {} void ServerConnectionImpl::onEncodeComplete() { ASSERT(active_request_); @@ -681,8 +686,9 @@ void ServerConnectionImpl::onBelowLowWatermark() { } } -ClientConnectionImpl::ClientConnectionImpl(Network::Connection& connection, ConnectionCallbacks&) - : ConnectionImpl(connection, HTTP_RESPONSE, MAX_RESPONSE_HEADERS_KB) {} +ClientConnectionImpl::ClientConnectionImpl(Network::Connection& connection, Stats::Scope& stats, + ConnectionCallbacks&) + : ConnectionImpl(connection, stats, HTTP_RESPONSE, MAX_RESPONSE_HEADERS_KB) {} bool ClientConnectionImpl::cannotHaveBody() { if ((!pending_responses_.empty() && pending_responses_.front().head_request_) || diff --git a/source/common/http/http1/codec_impl.h b/source/common/http/http1/codec_impl.h index c1a9fa5d7e93..bf6708fa8f47 100644 --- a/source/common/http/http1/codec_impl.h +++ b/source/common/http/http1/codec_impl.h @@ -10,6 +10,7 @@ #include "envoy/http/codec.h" #include "envoy/network/connection.h" +#include "envoy/stats/scope.h" #include "common/buffer/watermark_buffer.h" #include "common/common/assert.h" @@ -22,6 +23,21 @@ namespace Envoy { namespace Http { namespace Http1 { +/** + * All stats for the HTTP/1 codec. @see stats_macros.h + */ +// clang-format off +#define ALL_HTTP1_CODEC_STATS(COUNTER) \ + COUNTER(metadata_not_supported_error) \ +// clang-format on + +/** + * Wrapper struct for the HTTP/1 codec stats. @see stats_macros.h + */ +struct CodecStats { + ALL_HTTP1_CODEC_STATS(GENERATE_COUNTER_STRUCT) +}; + class ConnectionImpl; /** @@ -37,7 +53,7 @@ class StreamEncoderImpl : public StreamEncoder, void encodeHeaders(const HeaderMap& headers, bool end_stream) override; void encodeData(Buffer::Instance& data, bool end_stream) override; void encodeTrailers(const HeaderMap& trailers) override; - void encodeMetadata(const MetadataMapVector&) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } + void encodeMetadata(const MetadataMapVector&) override; Stream& getStream() override { return *this; } // Http::Stream @@ -171,13 +187,16 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable( - connection, callbacks, http1_settings_, maxRequestHeadersKb()); + connection, context_.scope(), callbacks, http1_settings_, maxRequestHeadersKb()); case CodecType::HTTP2: return std::make_unique( connection, callbacks, context_.scope(), http2_settings_, maxRequestHeadersKb()); diff --git a/test/common/http/codec_impl_fuzz_test.cc b/test/common/http/codec_impl_fuzz_test.cc index 4a56733f710f..e25f56194efc 100644 --- a/test/common/http/codec_impl_fuzz_test.cc +++ b/test/common/http/codec_impl_fuzz_test.cc @@ -359,7 +359,8 @@ void codecFuzz(const test::common::http::CodecImplFuzzTestCase& input, HttpVersi stats_store, client_http2settings, max_request_headers_kb); } else { - client = std::make_unique(client_connection, client_callbacks); + client = std::make_unique(client_connection, stats_store, + client_callbacks); } NiceMock server_connection; @@ -371,8 +372,9 @@ void codecFuzz(const test::common::http::CodecImplFuzzTestCase& input, HttpVersi max_request_headers_kb); } else { const Http1Settings server_http1settings{fromHttp1Settings(input.h1_settings().server())}; - server = std::make_unique( - server_connection, server_callbacks, server_http1settings, max_request_headers_kb); + server = std::make_unique(server_connection, stats_store, + server_callbacks, server_http1settings, + max_request_headers_kb); } ReorderBuffer client_write_buf{*server}; diff --git a/test/common/http/http1/codec_impl_test.cc b/test/common/http/http1/codec_impl_test.cc index fec552d6f3f8..793a4b003f9c 100644 --- a/test/common/http/http1/codec_impl_test.cc +++ b/test/common/http/http1/codec_impl_test.cc @@ -49,8 +49,8 @@ class Http1ServerConnectionImplTest : public testing::Test { } void initialize() { - codec_ = std::make_unique(connection_, callbacks_, codec_settings_, - max_request_headers_kb_); + codec_ = std::make_unique(connection_, store_, callbacks_, + codec_settings_, max_request_headers_kb_); } NiceMock connection_; @@ -84,8 +84,8 @@ void Http1ServerConnectionImplTest::expect400(Protocol p, bool allow_absolute_ur if (allow_absolute_url) { codec_settings_.allow_absolute_url_ = allow_absolute_url; - codec_ = std::make_unique(connection_, callbacks_, codec_settings_, - max_request_headers_kb_); + codec_ = std::make_unique(connection_, store_, callbacks_, + codec_settings_, max_request_headers_kb_); } Http::MockStreamDecoder decoder; @@ -104,8 +104,8 @@ void Http1ServerConnectionImplTest::expectHeadersTest(Protocol p, bool allow_abs // Make a new 'codec' with the right settings if (allow_absolute_url) { codec_settings_.allow_absolute_url_ = allow_absolute_url; - codec_ = std::make_unique(connection_, callbacks_, codec_settings_, - max_request_headers_kb_); + codec_ = std::make_unique(connection_, store_, callbacks_, + codec_settings_, max_request_headers_kb_); } Http::MockStreamDecoder decoder; @@ -578,9 +578,7 @@ TEST_F(Http1ServerConnectionImplTest, HeaderOnlyResponseWith100Then200) { EXPECT_EQ("HTTP/1.1 200 OK\r\ncontent-length: 0\r\n\r\n", output); } -class Http1ServerConnectionImplDeathTest : public Http1ServerConnectionImplTest {}; - -TEST_F(Http1ServerConnectionImplDeathTest, MetadataTest) { +TEST_F(Http1ServerConnectionImplTest, MetadataTest) { initialize(); NiceMock decoder; @@ -598,7 +596,8 @@ TEST_F(Http1ServerConnectionImplDeathTest, MetadataTest) { MetadataMapPtr metadata_map_ptr = std::make_unique(metadata_map); MetadataMapVector metadata_map_vector; metadata_map_vector.push_back(std::move(metadata_map_ptr)); - EXPECT_DEATH_LOG_TO_STDERR(response_encoder->encodeMetadata(metadata_map_vector), ""); + response_encoder->encodeMetadata(metadata_map_vector); + EXPECT_EQ(1, store_.counter("http1.metadata_not_supported_error").value()); } TEST_F(Http1ServerConnectionImplTest, ChunkedResponse) { @@ -860,7 +859,9 @@ class Http1ClientConnectionImplTest : public testing::Test { generator_, validation_visitor_, *api_)}); } - void initialize() { codec_ = std::make_unique(connection_, callbacks_); } + void initialize() { + codec_ = std::make_unique(connection_, store_, callbacks_); + } NiceMock connection_; NiceMock callbacks_; diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index 8fb6c8f01b6c..fd970b6de8f1 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -221,7 +221,8 @@ FakeHttpConnection::FakeHttpConnection(SharedConnectionWrapper& shared_connectio : FakeConnectionBase(shared_connection, time_system) { if (type == Type::HTTP1) { codec_ = std::make_unique( - shared_connection_.connection(), *this, Http::Http1Settings(), max_request_headers_kb); + shared_connection_.connection(), store, *this, Http::Http1Settings(), + max_request_headers_kb); } else { auto settings = Http::Http2Settings(); settings.allow_connect_ = true;