Skip to content

Commit

Permalink
[quic]Export QUIC connection close source to transport error details (e…
Browse files Browse the repository at this point in the history
…nvoyproxy#35439)

Commit Message:Export QUIC connection close source to transport error
details
Risk Level: Low
Testing: existing tests
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

---------

Signed-off-by: Renjie Tang <[email protected]>
  • Loading branch information
RenjieTang authored Jul 27, 2024
1 parent 7873af8 commit 51e2534
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 27 deletions.
13 changes: 7 additions & 6 deletions source/common/quic/envoy_quic_client_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -383,12 +383,13 @@ 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), "|",
quic::ConnectionCloseSourceToString(source), "|",
frame.error_details));
}
quic::QuicSpdyClientStream::OnConnectionClosed(frame, source);
}
Expand Down
13 changes: 7 additions & 6 deletions source/common/quic/envoy_quic_server_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -375,12 +375,13 @@ 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), "|",
quic::ConnectionCloseSourceToString(source), "|",
frame.error_details));
}
quic::QuicSpdyServerStreamBase::OnConnectionClosed(frame, source);
}
Expand Down
2 changes: 1 addition & 1 deletion test/integration/http_integration.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_PEER|Closed_by_application}; UC\"");
} else {
EXPECT_EQ(response->headers().getProxyStatusValue(),
"envoy; error=connection_terminated; "
Expand Down
18 changes: 9 additions & 9 deletions test/integration/local_reply_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_PEER|Closed by application"
})";
} else {
expected_body = R"({
Expand Down Expand Up @@ -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("223", response->headers().ContentLength()->value().getStringView());
} else {
EXPECT_EQ("150", response->headers().ContentLength()->value().getStringView());
}
Expand All @@ -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_PEER|Closed_by_application}; UC\"");
} else {
EXPECT_EQ(response->headers().getProxyStatusValue(),
"envoy; error=connection_terminated; "
Expand Down Expand Up @@ -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_PEER|Closed by application"
})";
} else {
expected_grpc_message = R"({
Expand Down Expand Up @@ -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_PEER|Closed by application:503:path=/package.service/method%0A";
} else {
expected_grpc_message =
"upstream connect error or disconnect/reset before headers. reset reason:"
Expand Down Expand Up @@ -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_PEER|Closed by application"
})";
} else {
expected_body = R"({
Expand Down Expand Up @@ -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("227", response->headers().ContentLength()->value().getStringView());
} else {
EXPECT_EQ("154", response->headers().ContentLength()->value().getStringView());
}
Expand Down Expand Up @@ -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("168", response->headers().ContentLength()->value().getStringView());
} else {
EXPECT_EQ("95", response->headers().ContentLength()->value().getStringView());
}
Expand All @@ -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_PEER|Closed by application");
} else {
EXPECT_EQ(response->body(), "upstream connect error or disconnect/reset before headers. reset "
"reason: connection termination");
Expand Down
13 changes: 8 additions & 5 deletions test/integration/protocol_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4507,9 +4507,11 @@ 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_SELF|Write_failed_with_error:_9_(Bad_file_descriptor)}"));
} else {
EXPECT_THAT(waitForAccessLog(access_log_name_),
HasSubstr("upstream_reset_before_response_started{connection_termination}"));
Expand Down Expand Up @@ -4718,8 +4720,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_SELF|Invalid_headers}");
} else {
EXPECT_EQ(waitForAccessLog(access_log_name_),
"upstream_reset_before_response_started{protocol_error}");
Expand Down

0 comments on commit 51e2534

Please sign in to comment.