diff --git a/docs/root/configuration/http/http_conn_man/stats.rst b/docs/root/configuration/http/http_conn_man/stats.rst index 7fd66553222f9..dd6891ce2d220 100644 --- a/docs/root/configuration/http/http_conn_man/stats.rst +++ b/docs/root/configuration/http/http_conn_man/stats.rst @@ -160,6 +160,24 @@ On the upstream side all http2 statistics are rooted at *cluster..http2.* `downstream_rq_active` gauge due to differences in stream accounting between the codec and the HTTP connection manager. +Http3 codec statistics +~~~~~~~~~~~~~~~~~~~~~~ + +On the downstream side all http3 statistics are rooted at *http3.* + +On the upstream side all http3 statistics are rooted at *cluster..http3.* + +.. csv-table:: + :header: Name, Type, Description + :widths: 1, 1, 2 + + dropped_headers_with_underscores, Counter, Total number of dropped headers with names containing underscores. This action is configured by setting the :ref:`headers_with_underscores_action config setting `. + requests_rejected_with_underscores_in_headers, Counter, Total numbers of rejected requests due to header names containing underscores. This action is configured by setting the :ref:`headers_with_underscores_action config setting `. + rx_reset, Counter, Total number of reset stream frames received by Envoy + tx_reset, Counter, Total number of reset stream frames transmitted by Envoy + metadata_not_supported_error, Counter, Total number of metadata dropped during HTTP/3 encoding + + Tracing statistics ------------------ diff --git a/source/common/http/http3/codec_stats.h b/source/common/http/http3/codec_stats.h index 327b345f1d468..c9dcf6f395a29 100644 --- a/source/common/http/http3/codec_stats.h +++ b/source/common/http/http3/codec_stats.h @@ -11,18 +11,13 @@ namespace Http3 { /** * All stats for the HTTP/3 codec. @see stats_macros.h - * TODO(danzh) populate all of them in codec. */ #define ALL_HTTP3_CODEC_STATS(COUNTER, GAUGE) \ COUNTER(dropped_headers_with_underscores) \ - COUNTER(header_overflow) \ COUNTER(requests_rejected_with_underscores_in_headers) \ - COUNTER(rx_messaging_error) \ COUNTER(rx_reset) \ - COUNTER(trailers) \ COUNTER(tx_reset) \ - COUNTER(metadata_not_supported_error) \ - GAUGE(streams_active, Accumulate) + COUNTER(metadata_not_supported_error) /** * Wrapper struct for the HTTP/3 codec stats. @see stats_macros.h diff --git a/source/common/quic/envoy_quic_client_stream.cc b/source/common/quic/envoy_quic_client_stream.cc index 6cb99fbc02d6d..1098a39b79fd5 100644 --- a/source/common/quic/envoy_quic_client_stream.cc +++ b/source/common/quic/envoy_quic_client_stream.cc @@ -256,11 +256,15 @@ void EnvoyQuicClientStream::maybeDecodeTrailers() { } void EnvoyQuicClientStream::OnStreamReset(const quic::QuicRstStreamFrame& frame) { + ENVOY_STREAM_LOG(debug, "received reset code={}", *this, frame.error_code); + stats_.rx_reset_.inc(); quic::QuicSpdyClientStream::OnStreamReset(frame); runResetCallbacks(quicRstErrorToEnvoyRemoteResetReason(frame.error_code)); } void EnvoyQuicClientStream::Reset(quic::QuicRstStreamErrorCode error) { + ENVOY_STREAM_LOG(debug, "sending reset code={}", *this, error); + stats_.tx_reset_.inc(); // Upper layers expect calling resetStream() to immediately raise reset callbacks. runResetCallbacks(quicRstErrorToEnvoyLocalResetReason(error)); quic::QuicSpdyClientStream::Reset(error); diff --git a/source/common/quic/envoy_quic_server_stream.cc b/source/common/quic/envoy_quic_server_stream.cc index 0c90cf66f6c97..74b439e246253 100644 --- a/source/common/quic/envoy_quic_server_stream.cc +++ b/source/common/quic/envoy_quic_server_stream.cc @@ -266,11 +266,15 @@ bool EnvoyQuicServerStream::OnStopSending(quic::QuicRstStreamErrorCode error) { } void EnvoyQuicServerStream::OnStreamReset(const quic::QuicRstStreamFrame& frame) { + ENVOY_STREAM_LOG(debug, "received reset code={}", *this, frame.error_code); + stats_.rx_reset_.inc(); quic::QuicSpdyServerStreamBase::OnStreamReset(frame); runResetCallbacks(quicRstErrorToEnvoyRemoteResetReason(frame.error_code)); } void EnvoyQuicServerStream::Reset(quic::QuicRstStreamErrorCode error) { + ENVOY_STREAM_LOG(debug, "sending reset code={}", *this, error); + stats_.tx_reset_.inc(); // Upper layers expect calling resetStream() to immediately raise reset callbacks. runResetCallbacks(quicRstErrorToEnvoyLocalResetReason(error)); quic::QuicSpdyServerStreamBase::Reset(error); diff --git a/test/integration/fake_upstream.cc b/test/integration/fake_upstream.cc index be91ed958e96e..9e24e0f55d171 100644 --- a/test/integration/fake_upstream.cc +++ b/test/integration/fake_upstream.cc @@ -506,7 +506,8 @@ FakeUpstream::FakeUpstream(Network::TransportSocketFactoryPtr&& transport_socket handler_(new Server::ConnectionHandlerImpl(*dispatcher_, 0)), config_(config), read_disable_on_new_connection_(true), enable_half_close_(config.enable_half_close_), listener_(*this, http_type_ == FakeHttpConnection::Type::HTTP3), - filter_chain_(Network::Test::createEmptyFilterChain(std::move(transport_socket_factory))) { + filter_chain_(Network::Test::createEmptyFilterChain(std::move(transport_socket_factory))), + stats_scope_(stats_store_.createScope("test_server_scope")) { ENVOY_LOG(info, "starting fake server at {}. UDP={} codec={}", localAddress()->asString(), config.udp_fake_upstream_.has_value(), FakeHttpConnection::typeToString(http_type_)); if (config.udp_fake_upstream_.has_value() && diff --git a/test/integration/fake_upstream.h b/test/integration/fake_upstream.h index eb3e9ac757095..06682a0795dcb 100644 --- a/test/integration/fake_upstream.h +++ b/test/integration/fake_upstream.h @@ -653,15 +653,15 @@ class FakeUpstream : Logger::Loggable, void cleanUp(); Http::Http1::CodecStats& http1CodecStats() { - return Http::Http1::CodecStats::atomicGet(http1_codec_stats_, stats_store_); + return Http::Http1::CodecStats::atomicGet(http1_codec_stats_, *stats_scope_); } Http::Http2::CodecStats& http2CodecStats() { - return Http::Http2::CodecStats::atomicGet(http2_codec_stats_, stats_store_); + return Http::Http2::CodecStats::atomicGet(http2_codec_stats_, *stats_scope_); } Http::Http3::CodecStats& http3CodecStats() { - return Http::Http3::CodecStats::atomicGet(http3_codec_stats_, stats_store_); + return Http::Http3::CodecStats::atomicGet(http3_codec_stats_, *stats_scope_); } // Write into the outbound buffer of the network connection at the specified index. @@ -826,6 +826,7 @@ class FakeUpstream : Logger::Loggable, FakeListener listener_; const Network::FilterChainSharedPtr filter_chain_; std::list received_datagrams_ ABSL_GUARDED_BY(lock_); + Stats::ScopePtr stats_scope_; Http::Http1::CodecStats::AtomicPtr http1_codec_stats_; Http::Http2::CodecStats::AtomicPtr http2_codec_stats_; Http::Http3::CodecStats::AtomicPtr http3_codec_stats_; diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index 2c225369cb8f4..508b0bcc4aac6 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -1505,6 +1505,30 @@ void HttpIntegrationTest::testMaxStreamDurationWithRetry(bool invoke_retry_upstr } } +std::string HttpIntegrationTest::downstreamProtocolStatsRoot() const { + switch (downstreamProtocol()) { + case Http::CodecClient::Type::HTTP1: + return "http1"; + case Http::CodecClient::Type::HTTP2: + return "http2"; + case Http::CodecClient::Type::HTTP3: + return "http3"; + } + return "invalid"; +} + +std::string HttpIntegrationTest::upstreamProtocolStatsRoot() const { + switch (upstreamProtocol()) { + case FakeHttpConnection::Type::HTTP1: + return "http1"; + case FakeHttpConnection::Type::HTTP2: + return "http2"; + case FakeHttpConnection::Type::HTTP3: + return "http3"; + } + return "invalid"; +} + std::string HttpIntegrationTest::listenerStatPrefix(const std::string& stat_name) { if (version_ == Network::Address::IpVersion::v4) { return "listener.127.0.0.1_0." + stat_name; diff --git a/test/integration/http_integration.h b/test/integration/http_integration.h index 23227ce7f2fbe..f1b8b89aa1086 100644 --- a/test/integration/http_integration.h +++ b/test/integration/http_integration.h @@ -243,6 +243,10 @@ class HttpIntegrationTest : public BaseIntegrationTest { void testMaxStreamDuration(); void testMaxStreamDurationWithRetry(bool invoke_retry_upstream_disconnect); Http::CodecClient::Type downstreamProtocol() const { return downstream_protocol_; } + // Return the stats root for the downstream protocol. + std::string downstreamProtocolStatsRoot() const; + // Return the upstream protocol part of the stats root. + std::string upstreamProtocolStatsRoot() const; // Prefix listener stat with IP:port, including IP version dependent loopback address. std::string listenerStatPrefix(const std::string& stat_name); diff --git a/test/integration/multiplexed_integration_test.cc b/test/integration/multiplexed_integration_test.cc index 63a976a738e03..1d34e1e51533a 100644 --- a/test/integration/multiplexed_integration_test.cc +++ b/test/integration/multiplexed_integration_test.cc @@ -309,6 +309,12 @@ TEST_P(Http2MetadataIntegrationTest, ProxyMetadataInResponse) { // Verifies stream is reset. ASSERT_TRUE(response->waitForReset()); ASSERT_FALSE(response->complete()); + + // The cluster should have received the reset. + // The downstream codec should send one. + std::string counter = + absl::StrCat("cluster.cluster_0.", upstreamProtocolStatsRoot(), ".rx_reset"); + test_server_->waitForCounterEq(counter, 1); } TEST_P(Http2MetadataIntegrationTest, ProxyMultipleMetadata) { diff --git a/test/integration/multiplexed_upstream_integration_test.cc b/test/integration/multiplexed_upstream_integration_test.cc index 2c86cba32fb0a..2c2eb1717cf4b 100644 --- a/test/integration/multiplexed_upstream_integration_test.cc +++ b/test/integration/multiplexed_upstream_integration_test.cc @@ -50,6 +50,10 @@ TEST_P(Http2UpstreamIntegrationTest, RouterUpstreamDisconnectBeforeResponseCompl TEST_P(Http2UpstreamIntegrationTest, RouterDownstreamDisconnectBeforeRequestComplete) { testRouterDownstreamDisconnectBeforeRequestComplete(); + + // Given the downstream disconnect, Envoy will reset the upstream stream. + EXPECT_EQ(1, upstreamTxResetCounterValue()); + EXPECT_EQ(0, upstreamRxResetCounterValue()); } TEST_P(Http2UpstreamIntegrationTest, RouterDownstreamDisconnectBeforeResponseComplete) { @@ -136,6 +140,24 @@ TEST_P(Http2UpstreamIntegrationTest, LargeBidirectionalStreamingWithBufferLimits bidirectionalStreaming(1024 * 32); } +uint64_t Http2UpstreamIntegrationTest::upstreamRxResetCounterValue() { + return test_server_ + ->counter(absl::StrCat("cluster.cluster_0.", upstreamProtocolStatsRoot(), ".rx_reset")) + ->value(); +} + +uint64_t Http2UpstreamIntegrationTest::upstreamTxResetCounterValue() { + return test_server_ + ->counter(absl::StrCat("cluster.cluster_0.", upstreamProtocolStatsRoot(), ".tx_reset")) + ->value(); +} +uint64_t Http2UpstreamIntegrationTest::downstreamRxResetCounterValue() { + return test_server_->counter(absl::StrCat(downstreamProtocolStatsRoot(), ".rx_reset"))->value(); +} +uint64_t Http2UpstreamIntegrationTest::downstreamTxResetCounterValue() { + return test_server_->counter(absl::StrCat(downstreamProtocolStatsRoot(), ".tx_reset"))->value(); +} + TEST_P(Http2UpstreamIntegrationTest, BidirectionalStreamingReset) { initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); @@ -169,6 +191,13 @@ TEST_P(Http2UpstreamIntegrationTest, BidirectionalStreamingReset) { upstream_request_->encodeResetStream(); ASSERT_TRUE(response->waitForReset()); EXPECT_FALSE(response->complete()); + + // The upstream stats should reflect receiving the reset, and downstream + // reflect sending it on. + EXPECT_EQ(1, upstreamRxResetCounterValue()); + EXPECT_EQ(0, upstreamTxResetCounterValue()); + EXPECT_EQ(0, downstreamRxResetCounterValue()); + EXPECT_EQ(1, downstreamTxResetCounterValue()); } void Http2UpstreamIntegrationTest::simultaneousRequest(uint32_t request1_bytes, @@ -371,6 +400,8 @@ TEST_P(Http2UpstreamIntegrationTest, UpstreamConnectionCloseWithManyStreams) { for (uint32_t i = 1; i < num_requests; ++i) { ASSERT_TRUE(responses[i]->waitForReset()); } + + EXPECT_NE(0, downstreamRxResetCounterValue()); } // Regression test for https://github.com/envoyproxy/envoy/issues/6744 @@ -431,6 +462,13 @@ name: router ASSERT_TRUE(response->waitForEndStream()); EXPECT_TRUE(response->complete()); + + // As the error was internal, Envoy should reset the upstream connection. + // Downstream gets an error, so no resets there. + EXPECT_EQ(1, upstreamTxResetCounterValue()); + EXPECT_EQ(0, downstreamTxResetCounterValue()); + EXPECT_EQ(0, upstreamRxResetCounterValue()); + EXPECT_EQ(0, downstreamRxResetCounterValue()); } // Tests the default limit for the number of response headers is 100. Results in a stream reset if diff --git a/test/integration/multiplexed_upstream_integration_test.h b/test/integration/multiplexed_upstream_integration_test.h index 45e35a83b42e6..961954a3db150 100644 --- a/test/integration/multiplexed_upstream_integration_test.h +++ b/test/integration/multiplexed_upstream_integration_test.h @@ -20,5 +20,10 @@ class Http2UpstreamIntegrationTest : public HttpProtocolIntegrationTest { void manySimultaneousRequests(uint32_t request_bytes, uint32_t response_bytes); bool use_alpn_{false}; + + uint64_t upstreamRxResetCounterValue(); + uint64_t upstreamTxResetCounterValue(); + uint64_t downstreamRxResetCounterValue(); + uint64_t downstreamTxResetCounterValue(); }; } // namespace Envoy diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index b8ab19fda6c9e..6cd2b034020a4 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -537,11 +537,10 @@ TEST_P(ProtocolIntegrationTest, Retry) { TestUtility::findCounter(stats, "cluster.cluster_0.upstream_rq_tx_reset"); ASSERT_NE(nullptr, counter); EXPECT_EQ(1L, counter->value()); - } else { - Stats::CounterSharedPtr counter = - TestUtility::findCounter(stats, "cluster.cluster_0.http1.dropped_headers_with_underscores"); - EXPECT_NE(nullptr, counter); } + EXPECT_NE(nullptr, + test_server_->counter(absl::StrCat("cluster.cluster_0.", upstreamProtocolStatsRoot(), + ".dropped_headers_with_underscores"))); } TEST_P(ProtocolIntegrationTest, RetryStreaming) { diff --git a/test/integration/server.h b/test/integration/server.h index d62e3f7dd5f48..b28b08aaac0e8 100644 --- a/test/integration/server.h +++ b/test/integration/server.h @@ -442,27 +442,24 @@ class IntegrationTestServer : public Logger::Loggable, Buffer::WatermarkFactorySharedPtr watermark_factory, bool v2_bootstrap); void waitForCounterEq(const std::string& name, uint64_t value, - std::chrono::milliseconds timeout = std::chrono::milliseconds::zero(), + std::chrono::milliseconds timeout = TestUtility::DefaultTimeout, Event::Dispatcher* dispatcher = nullptr) override { ASSERT_TRUE( TestUtility::waitForCounterEq(statStore(), name, value, time_system_, timeout, dispatcher)); } - void - waitForCounterGe(const std::string& name, uint64_t value, - std::chrono::milliseconds timeout = std::chrono::milliseconds::zero()) override { + void waitForCounterGe(const std::string& name, uint64_t value, + std::chrono::milliseconds timeout = TestUtility::DefaultTimeout) override { ASSERT_TRUE(TestUtility::waitForCounterGe(statStore(), name, value, time_system_, timeout)); } - void - waitForGaugeEq(const std::string& name, uint64_t value, - std::chrono::milliseconds timeout = std::chrono::milliseconds::zero()) override { + void waitForGaugeEq(const std::string& name, uint64_t value, + std::chrono::milliseconds timeout = TestUtility::DefaultTimeout) override { ASSERT_TRUE(TestUtility::waitForGaugeEq(statStore(), name, value, time_system_, timeout)); } - void - waitForGaugeGe(const std::string& name, uint64_t value, - std::chrono::milliseconds timeout = std::chrono::milliseconds::zero()) override { + void waitForGaugeGe(const std::string& name, uint64_t value, + std::chrono::milliseconds timeout = TestUtility::DefaultTimeout) override { ASSERT_TRUE(TestUtility::waitForGaugeGe(statStore(), name, value, time_system_, timeout)); } diff --git a/test/mocks/upstream/cluster_info.cc b/test/mocks/upstream/cluster_info.cc index 9c698a61ccd2a..5e755e1851df0 100644 --- a/test/mocks/upstream/cluster_info.cc +++ b/test/mocks/upstream/cluster_info.cc @@ -59,7 +59,8 @@ MockClusterInfo::MockClusterInfo() cluster_circuit_breakers_stat_names_)), resource_manager_(new Upstream::ResourceManagerImpl( runtime_, "fake_key", 1, 1024, 1024, 1, std::numeric_limits::max(), - circuit_breakers_stats_, absl::nullopt, absl::nullopt)) { + circuit_breakers_stats_, absl::nullopt, absl::nullopt)), + stats_scope_(stats_store_.createScope("test_scope")) { ON_CALL(*this, connectTimeout()).WillByDefault(Return(std::chrono::milliseconds(1))); ON_CALL(*this, idleTimeout()).WillByDefault(Return(absl::optional())); ON_CALL(*this, perUpstreamPreconnectRatio()).WillByDefault(Return(1.0)); @@ -126,15 +127,15 @@ MockClusterInfo::MockClusterInfo() MockClusterInfo::~MockClusterInfo() = default; Http::Http1::CodecStats& MockClusterInfo::http1CodecStats() const { - return Http::Http1::CodecStats::atomicGet(http1_codec_stats_, statsScope()); + return Http::Http1::CodecStats::atomicGet(http1_codec_stats_, *stats_scope_); } Http::Http2::CodecStats& MockClusterInfo::http2CodecStats() const { - return Http::Http2::CodecStats::atomicGet(http2_codec_stats_, statsScope()); + return Http::Http2::CodecStats::atomicGet(http2_codec_stats_, *stats_scope_); } Http::Http3::CodecStats& MockClusterInfo::http3CodecStats() const { - return Http::Http3::CodecStats::atomicGet(http3_codec_stats_, statsScope()); + return Http::Http3::CodecStats::atomicGet(http3_codec_stats_, *stats_scope_); } } // namespace Upstream diff --git a/test/mocks/upstream/cluster_info.h b/test/mocks/upstream/cluster_info.h index 5a3322536313c..ab597fade7fa4 100644 --- a/test/mocks/upstream/cluster_info.h +++ b/test/mocks/upstream/cluster_info.h @@ -200,6 +200,7 @@ class MockClusterInfo : public ClusterInfo { envoy::config::core::v3::Metadata metadata_; std::unique_ptr typed_metadata_; absl::optional max_stream_duration_; + Stats::ScopePtr stats_scope_; mutable Http::Http1::CodecStats::AtomicPtr http1_codec_stats_; mutable Http::Http2::CodecStats::AtomicPtr http2_codec_stats_; mutable Http::Http3::CodecStats::AtomicPtr http3_codec_stats_;