From 9b7b5a66554ff6dab2d22efd76b2044356c2000c Mon Sep 17 00:00:00 2001 From: Renjie Tang Date: Thu, 25 Jul 2024 10:01:05 -0700 Subject: [PATCH 1/3] expose connection close source Signed-off-by: Renjie Tang --- source/common/quic/envoy_quic_client_stream.cc | 12 ++++++------ source/common/quic/envoy_quic_server_stream.cc | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/source/common/quic/envoy_quic_client_stream.cc b/source/common/quic/envoy_quic_client_stream.cc index a9aace12849a..00fa0a1001b4 100644 --- a/source/common/quic/envoy_quic_client_stream.cc +++ b/source/common/quic/envoy_quic_client_stream.cc @@ -383,12 +383,12 @@ void EnvoyQuicClientStream::ResetWithError(quic::QuicResetStreamError error) { void EnvoyQuicClientStream::OnConnectionClosed(const quic::QuicConnectionCloseFrame& frame, quic::ConnectionCloseSource source) { if (!end_stream_decoded_) { - runResetCallbacks( - source == quic::ConnectionCloseSource::FROM_SELF - ? quicErrorCodeToEnvoyLocalResetReason(frame.quic_error_code, - session()->OneRttKeysAvailable()) - : quicErrorCodeToEnvoyRemoteResetReason(frame.quic_error_code), - absl::StrCat(quic::QuicErrorCodeToString(frame.quic_error_code), "|", frame.error_details)); + runResetCallbacks(source == quic::ConnectionCloseSource::FROM_SELF + ? quicErrorCodeToEnvoyLocalResetReason(frame.quic_error_code, + session()->OneRttKeysAvailable()) + : quicErrorCodeToEnvoyRemoteResetReason(frame.quic_error_code), + absl::StrCat(quic::QuicErrorCodeToString(frame.quic_error_code), + "|from:", source, "|", frame.error_details)); } quic::QuicSpdyClientStream::OnConnectionClosed(frame, source); } diff --git a/source/common/quic/envoy_quic_server_stream.cc b/source/common/quic/envoy_quic_server_stream.cc index 78cf02270d40..b23df7092726 100644 --- a/source/common/quic/envoy_quic_server_stream.cc +++ b/source/common/quic/envoy_quic_server_stream.cc @@ -375,12 +375,12 @@ void EnvoyQuicServerStream::OnConnectionClosed(const quic::QuicConnectionCloseFr // Run reset callback before closing the stream so that the watermark change will not trigger // callbacks. if (!local_end_stream_) { - runResetCallbacks( - source == quic::ConnectionCloseSource::FROM_SELF - ? quicErrorCodeToEnvoyLocalResetReason(frame.quic_error_code, - session()->OneRttKeysAvailable()) - : quicErrorCodeToEnvoyRemoteResetReason(frame.quic_error_code), - absl::StrCat(quic::QuicErrorCodeToString(frame.quic_error_code), "|", frame.error_details)); + runResetCallbacks(source == quic::ConnectionCloseSource::FROM_SELF + ? quicErrorCodeToEnvoyLocalResetReason(frame.quic_error_code, + session()->OneRttKeysAvailable()) + : quicErrorCodeToEnvoyRemoteResetReason(frame.quic_error_code), + absl::StrCat(quic::QuicErrorCodeToString(frame.quic_error_code), + "|from:", source, "|", frame.error_details)); } quic::QuicSpdyServerStreamBase::OnConnectionClosed(frame, source); } From 4af0279d6f5dba10933ee9682fe57e0eeb4cbf1d Mon Sep 17 00:00:00 2001 From: Renjie Tang Date: Thu, 25 Jul 2024 11:17:21 -0700 Subject: [PATCH 2/3] fix test Signed-off-by: Renjie Tang --- test/integration/http_integration.cc | 2 +- .../local_reply_integration_test.cc | 18 +++++++++--------- test/integration/protocol_integration_test.cc | 12 +++++++----- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index 922e2dd29657..217d97515b39 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -842,7 +842,7 @@ void HttpIntegrationTest::testRouterUpstreamDisconnectBeforeRequestComplete() { EXPECT_EQ(response->headers().getProxyStatusValue(), "envoy; error=connection_terminated; " "details=\"upstream_reset_before_response_started{connection_termination|QUIC_NO_" - "ERROR|Closed_by_application}; UC\""); + "ERROR|from:0|Closed_by_application}; UC\""); } else { EXPECT_EQ(response->headers().getProxyStatusValue(), "envoy; error=connection_terminated; " diff --git a/test/integration/local_reply_integration_test.cc b/test/integration/local_reply_integration_test.cc index 79c2631a5f4e..0662191098b7 100644 --- a/test/integration/local_reply_integration_test.cc +++ b/test/integration/local_reply_integration_test.cc @@ -53,7 +53,7 @@ TEST_P(LocalReplyIntegrationTest, MapStatusCodeAndFormatToJson) { expected_body = R"({ "level": "TRACE", "user_agent": null, - "response_body": "upstream connect error or disconnect/reset before headers. reset reason: connection termination, transport failure reason: QUIC_NO_ERROR|Closed by application" + "response_body": "upstream connect error or disconnect/reset before headers. reset reason: connection termination, transport failure reason: QUIC_NO_ERROR|from:0|Closed by application" })"; } else { expected_body = R"({ @@ -93,7 +93,7 @@ TEST_P(LocalReplyIntegrationTest, MapStatusCodeAndFormatToJson) { EXPECT_TRUE(response->complete()); EXPECT_EQ("application/json-custom", response->headers().ContentType()->value().getStringView()); if (GetParam().upstream_protocol == Http::CodecType::HTTP3) { - EXPECT_EQ("213", response->headers().ContentLength()->value().getStringView()); + EXPECT_EQ("220", response->headers().ContentLength()->value().getStringView()); } else { EXPECT_EQ("150", response->headers().ContentLength()->value().getStringView()); } @@ -102,7 +102,7 @@ TEST_P(LocalReplyIntegrationTest, MapStatusCodeAndFormatToJson) { EXPECT_EQ(response->headers().getProxyStatusValue(), "envoy; error=connection_terminated; " "details=\"upstream_reset_before_response_started{connection_termination|QUIC_NO_" - "ERROR|Closed_by_application}; UC\""); + "ERROR|from:0|Closed_by_application}; UC\""); } else { EXPECT_EQ(response->headers().getProxyStatusValue(), "envoy; error=connection_terminated; " @@ -189,7 +189,7 @@ TEST_P(LocalReplyIntegrationTest, MapStatusCodeAndFormatToJson4Grpc) { if (GetParam().upstream_protocol == Http::CodecType::HTTP3) { expected_grpc_message = R"({ "code": 503, - "message":"upstream connect error or disconnect/reset before headers. reset reason: connection termination, transport failure reason: QUIC_NO_ERROR|Closed by application" + "message":"upstream connect error or disconnect/reset before headers. reset reason: connection termination, transport failure reason: QUIC_NO_ERROR|from:0|Closed by application" })"; } else { expected_grpc_message = R"({ @@ -252,7 +252,7 @@ TEST_P(LocalReplyIntegrationTest, MapStatusCodeAndFormat2Text4Grpc) { expected_grpc_message = "upstream connect error or disconnect/reset before headers. reset reason:" " connection termination, transport failure reason: " - "QUIC_NO_ERROR|Closed by application:503:path=/package.service/method%0A"; + "QUIC_NO_ERROR|from:0|Closed by application:503:path=/package.service/method%0A"; } else { expected_grpc_message = "upstream connect error or disconnect/reset before headers. reset reason:" @@ -416,7 +416,7 @@ TEST_P(LocalReplyIntegrationTest, ShouldNotMatchAnyFilter) { expected_body = R"({ "level": "TRACE", "response_flags": "UC", - "response_body": "upstream connect error or disconnect/reset before headers. reset reason: connection termination, transport failure reason: QUIC_NO_ERROR|Closed by application" + "response_body": "upstream connect error or disconnect/reset before headers. reset reason: connection termination, transport failure reason: QUIC_NO_ERROR|from:0|Closed by application" })"; } else { expected_body = R"({ @@ -456,7 +456,7 @@ TEST_P(LocalReplyIntegrationTest, ShouldNotMatchAnyFilter) { EXPECT_TRUE(response->complete()); EXPECT_EQ("application/json", response->headers().ContentType()->value().getStringView()); if (GetParam().upstream_protocol == Http::CodecType::HTTP3) { - EXPECT_EQ("217", response->headers().ContentLength()->value().getStringView()); + EXPECT_EQ("224", response->headers().ContentLength()->value().getStringView()); } else { EXPECT_EQ("154", response->headers().ContentLength()->value().getStringView()); } @@ -524,7 +524,7 @@ TEST_P(LocalReplyIntegrationTest, ShouldMapResponseCodeAndMapToDefaultTextRespon EXPECT_TRUE(response->complete()); EXPECT_EQ("text/plain", response->headers().ContentType()->value().getStringView()); if (GetParam().upstream_protocol == Http::CodecType::HTTP3) { - EXPECT_EQ("158", response->headers().ContentLength()->value().getStringView()); + EXPECT_EQ("165", response->headers().ContentLength()->value().getStringView()); } else { EXPECT_EQ("95", response->headers().ContentLength()->value().getStringView()); } @@ -534,7 +534,7 @@ TEST_P(LocalReplyIntegrationTest, ShouldMapResponseCodeAndMapToDefaultTextRespon if (GetParam().upstream_protocol == Http::CodecType::HTTP3) { EXPECT_EQ(response->body(), "upstream connect error or disconnect/reset before headers. reset " "reason: connection termination, transport failure reason: " - "QUIC_NO_ERROR|Closed by application"); + "QUIC_NO_ERROR|from:0|Closed by application"); } else { EXPECT_EQ(response->body(), "upstream connect error or disconnect/reset before headers. reset " "reason: connection termination"); diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index ad1a04a26ae8..419271c7cb3e 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -4507,9 +4507,10 @@ TEST_P(ProtocolIntegrationTest, HandleUpstreamSocketFail) { ASSERT_TRUE(response->waitForEndStream()); if (upstreamProtocol() == Http::CodecType::HTTP3) { - EXPECT_THAT(waitForAccessLog(access_log_name_), - HasSubstr("upstream_reset_before_response_started{connection_termination|QUIC_" - "PACKET_WRITE_ERROR|Write_failed_with_error:_9_(Bad_file_descriptor)}")); + EXPECT_THAT( + waitForAccessLog(access_log_name_), + HasSubstr("upstream_reset_before_response_started{connection_termination|QUIC_" + "PACKET_WRITE_ERROR|from:1|Write_failed_with_error:_9_(Bad_file_descriptor)}")); } else { EXPECT_THAT(waitForAccessLog(access_log_name_), HasSubstr("upstream_reset_before_response_started{connection_termination}")); @@ -4718,8 +4719,9 @@ TEST_P(ProtocolIntegrationTest, InvalidResponseHeaderName) { EXPECT_EQ("502", response->headers().getStatusValue()); test_server_->waitForCounterGe("http.config_test.downstream_rq_5xx", 1); if (upstreamProtocol() == Http::CodecType::HTTP3) { - EXPECT_EQ(waitForAccessLog(access_log_name_), "upstream_reset_before_response_started{protocol_" - "error|QUIC_HTTP_FRAME_ERROR|Invalid_headers}"); + EXPECT_EQ(waitForAccessLog(access_log_name_), + "upstream_reset_before_response_started{protocol_" + "error|QUIC_HTTP_FRAME_ERROR|from:1|Invalid_headers}"); } else { EXPECT_EQ(waitForAccessLog(access_log_name_), "upstream_reset_before_response_started{protocol_error}"); From e2a835e2bd877e6091dabf8b9627522cd79a7770 Mon Sep 17 00:00:00 2001 From: Renjie Tang Date: Fri, 26 Jul 2024 14:43:30 -0700 Subject: [PATCH 3/3] to string Signed-off-by: Renjie Tang --- source/common/quic/envoy_quic_client_stream.cc | 5 +++-- source/common/quic/envoy_quic_server_stream.cc | 5 +++-- test/integration/http_integration.cc | 2 +- .../local_reply_integration_test.cc | 18 +++++++++--------- test/integration/protocol_integration_test.cc | 7 ++++--- 5 files changed, 20 insertions(+), 17 deletions(-) diff --git a/source/common/quic/envoy_quic_client_stream.cc b/source/common/quic/envoy_quic_client_stream.cc index 00fa0a1001b4..e81683eddced 100644 --- a/source/common/quic/envoy_quic_client_stream.cc +++ b/source/common/quic/envoy_quic_client_stream.cc @@ -387,8 +387,9 @@ void EnvoyQuicClientStream::OnConnectionClosed(const quic::QuicConnectionCloseFr ? quicErrorCodeToEnvoyLocalResetReason(frame.quic_error_code, session()->OneRttKeysAvailable()) : quicErrorCodeToEnvoyRemoteResetReason(frame.quic_error_code), - absl::StrCat(quic::QuicErrorCodeToString(frame.quic_error_code), - "|from:", source, "|", frame.error_details)); + absl::StrCat(quic::QuicErrorCodeToString(frame.quic_error_code), "|", + quic::ConnectionCloseSourceToString(source), "|", + frame.error_details)); } quic::QuicSpdyClientStream::OnConnectionClosed(frame, source); } diff --git a/source/common/quic/envoy_quic_server_stream.cc b/source/common/quic/envoy_quic_server_stream.cc index b23df7092726..de9cb028f09a 100644 --- a/source/common/quic/envoy_quic_server_stream.cc +++ b/source/common/quic/envoy_quic_server_stream.cc @@ -379,8 +379,9 @@ void EnvoyQuicServerStream::OnConnectionClosed(const quic::QuicConnectionCloseFr ? quicErrorCodeToEnvoyLocalResetReason(frame.quic_error_code, session()->OneRttKeysAvailable()) : quicErrorCodeToEnvoyRemoteResetReason(frame.quic_error_code), - absl::StrCat(quic::QuicErrorCodeToString(frame.quic_error_code), - "|from:", source, "|", frame.error_details)); + absl::StrCat(quic::QuicErrorCodeToString(frame.quic_error_code), "|", + quic::ConnectionCloseSourceToString(source), "|", + frame.error_details)); } quic::QuicSpdyServerStreamBase::OnConnectionClosed(frame, source); } diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index 217d97515b39..8d23789e5e39 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -842,7 +842,7 @@ void HttpIntegrationTest::testRouterUpstreamDisconnectBeforeRequestComplete() { EXPECT_EQ(response->headers().getProxyStatusValue(), "envoy; error=connection_terminated; " "details=\"upstream_reset_before_response_started{connection_termination|QUIC_NO_" - "ERROR|from:0|Closed_by_application}; UC\""); + "ERROR|FROM_PEER|Closed_by_application}; UC\""); } else { EXPECT_EQ(response->headers().getProxyStatusValue(), "envoy; error=connection_terminated; " diff --git a/test/integration/local_reply_integration_test.cc b/test/integration/local_reply_integration_test.cc index 0662191098b7..95b7234dab05 100644 --- a/test/integration/local_reply_integration_test.cc +++ b/test/integration/local_reply_integration_test.cc @@ -53,7 +53,7 @@ TEST_P(LocalReplyIntegrationTest, MapStatusCodeAndFormatToJson) { expected_body = R"({ "level": "TRACE", "user_agent": null, - "response_body": "upstream connect error or disconnect/reset before headers. reset reason: connection termination, transport failure reason: QUIC_NO_ERROR|from:0|Closed by application" + "response_body": "upstream connect error or disconnect/reset before headers. reset reason: connection termination, transport failure reason: QUIC_NO_ERROR|FROM_PEER|Closed by application" })"; } else { expected_body = R"({ @@ -93,7 +93,7 @@ TEST_P(LocalReplyIntegrationTest, MapStatusCodeAndFormatToJson) { EXPECT_TRUE(response->complete()); EXPECT_EQ("application/json-custom", response->headers().ContentType()->value().getStringView()); if (GetParam().upstream_protocol == Http::CodecType::HTTP3) { - EXPECT_EQ("220", response->headers().ContentLength()->value().getStringView()); + EXPECT_EQ("223", response->headers().ContentLength()->value().getStringView()); } else { EXPECT_EQ("150", response->headers().ContentLength()->value().getStringView()); } @@ -102,7 +102,7 @@ TEST_P(LocalReplyIntegrationTest, MapStatusCodeAndFormatToJson) { EXPECT_EQ(response->headers().getProxyStatusValue(), "envoy; error=connection_terminated; " "details=\"upstream_reset_before_response_started{connection_termination|QUIC_NO_" - "ERROR|from:0|Closed_by_application}; UC\""); + "ERROR|FROM_PEER|Closed_by_application}; UC\""); } else { EXPECT_EQ(response->headers().getProxyStatusValue(), "envoy; error=connection_terminated; " @@ -189,7 +189,7 @@ TEST_P(LocalReplyIntegrationTest, MapStatusCodeAndFormatToJson4Grpc) { if (GetParam().upstream_protocol == Http::CodecType::HTTP3) { expected_grpc_message = R"({ "code": 503, - "message":"upstream connect error or disconnect/reset before headers. reset reason: connection termination, transport failure reason: QUIC_NO_ERROR|from:0|Closed by application" + "message":"upstream connect error or disconnect/reset before headers. reset reason: connection termination, transport failure reason: QUIC_NO_ERROR|FROM_PEER|Closed by application" })"; } else { expected_grpc_message = R"({ @@ -252,7 +252,7 @@ TEST_P(LocalReplyIntegrationTest, MapStatusCodeAndFormat2Text4Grpc) { expected_grpc_message = "upstream connect error or disconnect/reset before headers. reset reason:" " connection termination, transport failure reason: " - "QUIC_NO_ERROR|from:0|Closed by application:503:path=/package.service/method%0A"; + "QUIC_NO_ERROR|FROM_PEER|Closed by application:503:path=/package.service/method%0A"; } else { expected_grpc_message = "upstream connect error or disconnect/reset before headers. reset reason:" @@ -416,7 +416,7 @@ TEST_P(LocalReplyIntegrationTest, ShouldNotMatchAnyFilter) { expected_body = R"({ "level": "TRACE", "response_flags": "UC", - "response_body": "upstream connect error or disconnect/reset before headers. reset reason: connection termination, transport failure reason: QUIC_NO_ERROR|from:0|Closed by application" + "response_body": "upstream connect error or disconnect/reset before headers. reset reason: connection termination, transport failure reason: QUIC_NO_ERROR|FROM_PEER|Closed by application" })"; } else { expected_body = R"({ @@ -456,7 +456,7 @@ TEST_P(LocalReplyIntegrationTest, ShouldNotMatchAnyFilter) { EXPECT_TRUE(response->complete()); EXPECT_EQ("application/json", response->headers().ContentType()->value().getStringView()); if (GetParam().upstream_protocol == Http::CodecType::HTTP3) { - EXPECT_EQ("224", response->headers().ContentLength()->value().getStringView()); + EXPECT_EQ("227", response->headers().ContentLength()->value().getStringView()); } else { EXPECT_EQ("154", response->headers().ContentLength()->value().getStringView()); } @@ -524,7 +524,7 @@ TEST_P(LocalReplyIntegrationTest, ShouldMapResponseCodeAndMapToDefaultTextRespon EXPECT_TRUE(response->complete()); EXPECT_EQ("text/plain", response->headers().ContentType()->value().getStringView()); if (GetParam().upstream_protocol == Http::CodecType::HTTP3) { - EXPECT_EQ("165", response->headers().ContentLength()->value().getStringView()); + EXPECT_EQ("168", response->headers().ContentLength()->value().getStringView()); } else { EXPECT_EQ("95", response->headers().ContentLength()->value().getStringView()); } @@ -534,7 +534,7 @@ TEST_P(LocalReplyIntegrationTest, ShouldMapResponseCodeAndMapToDefaultTextRespon if (GetParam().upstream_protocol == Http::CodecType::HTTP3) { EXPECT_EQ(response->body(), "upstream connect error or disconnect/reset before headers. reset " "reason: connection termination, transport failure reason: " - "QUIC_NO_ERROR|from:0|Closed by application"); + "QUIC_NO_ERROR|FROM_PEER|Closed by application"); } else { EXPECT_EQ(response->body(), "upstream connect error or disconnect/reset before headers. reset " "reason: connection termination"); diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index 419271c7cb3e..261ddd3c430e 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -4509,8 +4509,9 @@ TEST_P(ProtocolIntegrationTest, HandleUpstreamSocketFail) { if (upstreamProtocol() == Http::CodecType::HTTP3) { EXPECT_THAT( waitForAccessLog(access_log_name_), - HasSubstr("upstream_reset_before_response_started{connection_termination|QUIC_" - "PACKET_WRITE_ERROR|from:1|Write_failed_with_error:_9_(Bad_file_descriptor)}")); + HasSubstr( + "upstream_reset_before_response_started{connection_termination|QUIC_" + "PACKET_WRITE_ERROR|FROM_SELF|Write_failed_with_error:_9_(Bad_file_descriptor)}")); } else { EXPECT_THAT(waitForAccessLog(access_log_name_), HasSubstr("upstream_reset_before_response_started{connection_termination}")); @@ -4721,7 +4722,7 @@ TEST_P(ProtocolIntegrationTest, InvalidResponseHeaderName) { if (upstreamProtocol() == Http::CodecType::HTTP3) { EXPECT_EQ(waitForAccessLog(access_log_name_), "upstream_reset_before_response_started{protocol_" - "error|QUIC_HTTP_FRAME_ERROR|from:1|Invalid_headers}"); + "error|QUIC_HTTP_FRAME_ERROR|FROM_SELF|Invalid_headers}"); } else { EXPECT_EQ(waitForAccessLog(access_log_name_), "upstream_reset_before_response_started{protocol_error}");