Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions docs/root/configuration/http/http_conn_man/stats.rst
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,24 @@ On the upstream side all http2 statistics are rooted at *cluster.<name>.http2.*
`downstream_rq_active` gauge due to differences in stream accounting between the codec and the
HTTP connection manager.

Http3 codec statistics
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @yanavlasov @danzh2010

I think that the difference in HTTP2 and HTTP3 stats point at missing functionality in the HTTP3 codec. For example, no stats for header_overflow, inbound_window_update_frames_flood, keepalive_timeout and streams_active hint at lower level of hardening options or lack of stats to observe the behavior of those protections.

Worth thinking which of these are relevant and how to track their absense.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

~~~~~~~~~~~~~~~~~~~~~~

On the downstream side all http3 statistics are rooted at *http3.*

On the upstream side all http3 statistics are rooted at *cluster.<name>.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 <envoy_v3_api_field_config.core.v3.HttpProtocolOptions.headers_with_underscores_action>`.
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 <envoy_v3_api_field_config.core.v3.HttpProtocolOptions.headers_with_underscores_action>`.
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
------------------

Expand Down
7 changes: 1 addition & 6 deletions source/common/http/http3/codec_stats.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have to remove these? I planned to populate them gradually.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think leaving them in the code base while they're not being used makes sense, but I think it'd be fine to add them back (and doc them up) as they're implemented.

*/
#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
Expand Down
4 changes: 4 additions & 0 deletions source/common/quic/envoy_quic_client_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions source/common/quic/envoy_quic_server_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion test/integration/fake_upstream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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() &&
Expand Down
7 changes: 4 additions & 3 deletions test/integration/fake_upstream.h
Original file line number Diff line number Diff line change
Expand Up @@ -653,15 +653,15 @@ class FakeUpstream : Logger::Loggable<Logger::Id::testing>,
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.
Expand Down Expand Up @@ -826,6 +826,7 @@ class FakeUpstream : Logger::Loggable<Logger::Id::testing>,
FakeListener listener_;
const Network::FilterChainSharedPtr filter_chain_;
std::list<Network::UdpRecvData> 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_;
Expand Down
24 changes: 24 additions & 0 deletions test/integration/http_integration.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions test/integration/http_integration.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
6 changes: 6 additions & 0 deletions test/integration/multiplexed_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
38 changes: 38 additions & 0 deletions test/integration/multiplexed_upstream_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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"));
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions test/integration/multiplexed_upstream_integration_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 3 additions & 4 deletions test/integration/protocol_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
17 changes: 7 additions & 10 deletions test/integration/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -442,27 +442,24 @@ class IntegrationTestServer : public Logger::Loggable<Logger::Id::testing>,
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));
}

Expand Down
9 changes: 5 additions & 4 deletions test/mocks/upstream/cluster_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint64_t>::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<std::chrono::milliseconds>()));
ON_CALL(*this, perUpstreamPreconnectRatio()).WillByDefault(Return(1.0));
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions test/mocks/upstream/cluster_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ class MockClusterInfo : public ClusterInfo {
envoy::config::core::v3::Metadata metadata_;
std::unique_ptr<Envoy::Config::TypedMetadata> typed_metadata_;
absl::optional<std::chrono::milliseconds> 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_;
Expand Down