diff --git a/include/envoy/http/filter.h b/include/envoy/http/filter.h index eec28670591ed..44fa734bc3be9 100644 --- a/include/envoy/http/filter.h +++ b/include/envoy/http/filter.h @@ -283,6 +283,18 @@ class StreamDecoderFilterCallbacks : public virtual StreamFilterCallbacks { */ virtual HeaderMap& addDecodedTrailers() PURE; + /** + * A wrapper for legacy sendLocalReply replies without the details parameter. + * See sendLocalReply below for usage + */ + // TODO(alyssawilk) send an email to envoy-dev for API change, add for all other filters, and + // delete this placeholder. + void sendLocalReply(Code response_code, absl::string_view body_text, + std::function modify_headers, + const absl::optional grpc_status) { + sendLocalReply(response_code, body_text, modify_headers, grpc_status, ""); + } + /** * Create a locally generated response using the provided response_code and body_text parameters. * If the request was a gRPC request the local reply will be encoded as a gRPC response with a 200 @@ -295,10 +307,12 @@ class StreamDecoderFilterCallbacks : public virtual StreamFilterCallbacks { * @param modify_headers supplies an optional callback function that can modify the * response headers. * @param grpc_status the gRPC status code to override the httpToGrpcStatus mapping with. + * @param details a string detailing why this local reply was sent. */ virtual void sendLocalReply(Code response_code, absl::string_view body_text, std::function modify_headers, - const absl::optional grpc_status) PURE; + const absl::optional grpc_status, + absl::string_view details) PURE; /** * Called with 100-Continue headers to be encoded. diff --git a/include/envoy/stream_info/stream_info.h b/include/envoy/stream_info/stream_info.h index 3bdb8a6e69e75..aed53c9506c4a 100644 --- a/include/envoy/stream_info/stream_info.h +++ b/include/envoy/stream_info/stream_info.h @@ -76,8 +76,57 @@ enum ResponseFlag { struct ResponseCodeDetailValues { // Response code was set by the upstream. const std::string ViaUpstream = "via_upstream"; - - // TODO(#6542): add values for sendLocalReply use-cases + // Envoy is doing non-streaming proxying, and the request payload exceeded + // configured limits. + const std::string RequestPayloadTooLarge = "request_payload_too_large"; + // Envoy is doing non-streaming proxying, and the response payload exceeded + // configured limits. + const std::string ResponsePayloadTooLArge = "response_payload_too_large"; + // The per-stream keepalive timeout was exceeded. + const std::string StreamIdleTimeout = "stream_idle_timeout"; + // The per-stream total request timeout was exceeded + const std::string RequestOverallTimeout = "request_overall_timeout"; + // The request was rejected due to the Overload Manager reaching configured resource limits. + const std::string Overload = "overload"; + // The HTTP/1.0 or HTTP/0.9 request was rejected due to HTTP/1.0 support not being configured. + const std::string LowVersion = "low_version"; + // The request was rejected due to the Host: or :authority field missing + const std::string MissingHost = "missing_host_header"; + // The request was rejected due to the request headers being larger than the configured limit. + const std::string RequestHeadersTooLarge = "request_headers_too_large"; + // The request was rejected due to the Path or :path header field missing. + const std::string MissingPath = "missing_path_rejected"; + // The request was rejected due to using an absolute path on a route not supporting them. + const std::string AbsolutePath = "absolute_path_rejected"; + // The request was rejected because path normalization was configured on and failed, probably due + // to an invalid path. + const std::string PathNormalizationFailed = "path_normalization_failed"; + // The request was rejected because it attempted an unsupported upgrade. + const std::string UpgradeFailed = "upgrade_failed"; + // The request was rejected by the router filter because there was no route found. + const std::string RouteNotFound = "route_not_found"; + // A direct response was generated by the router filter. + const std::string DirectResponse = "direct_response"; + // The request was rejected by the router filter because there was no cluster found for the + // selected route. + const std::string ClusterNotFound = "cluster_not_found"; + // The request was rejected by the router filter because the cluster was in maintenance mode. + const std::string MaintenanceMode = "maintenance_mode"; + // The request was rejected by the router filter because there was no healthy upstream found. + const std::string NoHealthyUpstream = "no_healthy_upstream"; + // The upstream response timed out + const std::string UpstreamTimeout = "upstream_response_timeout"; + // The final upstream try timed out + const std::string UpstreamPerTryTimeout = "upstream_per_try_timeout"; + // The upstream connection was reset before a response was started. This + // will generally be accompanied by details about why the reset occurred. + const std::string EarlyUpstreamReset = "upstream_reset_before_response_started"; + // The upstream connection was reset after a response was started. This + // will generally be accompanied by details about why the reset occurred but + // indicates that original "success" headers may have been sent downstream + // despite the subsequent failure. + const std::string LateUpstreamReset = "upstream_reset_after_response_started"; + // TODO(#6542): continue addding values for sendLocalReply use-cases }; typedef ConstSingleton ResponseCodeDetails; diff --git a/source/common/http/async_client_impl.h b/source/common/http/async_client_impl.h index b288664db86a9..eb319aa9c6bdb 100644 --- a/source/common/http/async_client_impl.h +++ b/source/common/http/async_client_impl.h @@ -327,9 +327,9 @@ class AsyncStreamImpl : public AsyncClient::Stream, } void sendLocalReply(Code code, absl::string_view body, std::function modify_headers, - const absl::optional grpc_status) override { - // TODO(#6542): add an extra parameter for setting rc details - stream_info_.setResponseCodeDetails(""); + const absl::optional grpc_status, + absl::string_view details) override { + stream_info_.setResponseCodeDetails(details); Utility::sendLocalReply( is_grpc_request_, [this, modify_headers](HeaderMapPtr&& headers, bool end_stream) -> void { diff --git a/source/common/http/conn_manager_impl.cc b/source/common/http/conn_manager_impl.cc index 286ee7855f67b..e179668d169ce 100644 --- a/source/common/http/conn_manager_impl.cc +++ b/source/common/http/conn_manager_impl.cc @@ -494,9 +494,10 @@ void ConnectionManagerImpl::ActiveStream::onIdleTimeout() { connection_manager_.doEndStream(*this); } else { stream_info_.setResponseFlag(StreamInfo::ResponseFlag::StreamIdleTimeout); - sendLocalReply( - request_headers_ != nullptr && Grpc::Common::hasGrpcContentType(*request_headers_), - Http::Code::RequestTimeout, "stream timeout", nullptr, is_head_request_, absl::nullopt); + sendLocalReply(request_headers_ != nullptr && + Grpc::Common::hasGrpcContentType(*request_headers_), + Http::Code::RequestTimeout, "stream timeout", nullptr, is_head_request_, + absl::nullopt, StreamInfo::ResponseCodeDetails::get().StreamIdleTimeout); } } @@ -504,7 +505,7 @@ void ConnectionManagerImpl::ActiveStream::onRequestTimeout() { connection_manager_.stats_.named_.downstream_rq_timeout_.inc(); sendLocalReply(request_headers_ != nullptr && Grpc::Common::hasGrpcContentType(*request_headers_), Http::Code::RequestTimeout, "request timeout", nullptr, is_head_request_, - absl::nullopt); + absl::nullopt, StreamInfo::ResponseCodeDetails::get().RequestOverallTimeout); } void ConnectionManagerImpl::ActiveStream::addStreamDecoderFilterWorker( @@ -594,7 +595,7 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers, connection_manager_.stats_.named_.downstream_rq_overload_close_.inc(); sendLocalReply(Grpc::Common::hasGrpcContentType(*request_headers_), Http::Code::ServiceUnavailable, "envoy overloaded", nullptr, is_head_request_, - absl::nullopt); + absl::nullopt, StreamInfo::ResponseCodeDetails::get().Overload); return; } @@ -622,7 +623,8 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers, stream_info_.protocol(protocol); if (!connection_manager_.config_.http1Settings().accept_http_10_) { // Send "Upgrade Required" if HTTP/1.0 support is not explicitly configured on. - sendLocalReply(false, Code::UpgradeRequired, "", nullptr, is_head_request_, absl::nullopt); + sendLocalReply(false, Code::UpgradeRequired, "", nullptr, is_head_request_, absl::nullopt, + StreamInfo::ResponseCodeDetails::get().LowVersion); return; } else { // HTTP/1.0 defaults to single-use connections. Make sure the connection @@ -645,7 +647,8 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers, } else { // Require host header. For HTTP/1.1 Host has already been translated to :authority. sendLocalReply(Grpc::Common::hasGrpcContentType(*request_headers_), Code::BadRequest, "", - nullptr, is_head_request_, absl::nullopt); + nullptr, is_head_request_, absl::nullopt, + StreamInfo::ResponseCodeDetails::get().MissingHost); return; } } @@ -653,7 +656,8 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers, ASSERT(connection_manager_.config_.maxRequestHeadersKb() > 0); if (request_headers_->byteSize() > (connection_manager_.config_.maxRequestHeadersKb() * 1024)) { sendLocalReply(Grpc::Common::hasGrpcContentType(*request_headers_), - Code::RequestHeaderFieldsTooLarge, "", nullptr, is_head_request_, absl::nullopt); + Code::RequestHeaderFieldsTooLarge, "", nullptr, is_head_request_, absl::nullopt, + StreamInfo::ResponseCodeDetails::get().RequestHeadersTooLarge); return; } @@ -664,9 +668,13 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers, // :path because CONNECT does not have a path, and we don't support that currently. if (!request_headers_->Path() || request_headers_->Path()->value().getStringView().empty() || request_headers_->Path()->value().getStringView()[0] != '/') { + const bool has_path = + request_headers_->Path() && !request_headers_->Path()->value().getStringView().empty(); connection_manager_.stats_.named_.downstream_rq_non_relative_path_.inc(); sendLocalReply(Grpc::Common::hasGrpcContentType(*request_headers_), Code::NotFound, "", nullptr, - is_head_request_, absl::nullopt); + is_head_request_, absl::nullopt, + has_path ? StreamInfo::ResponseCodeDetails::get().AbsolutePath + : StreamInfo::ResponseCodeDetails::get().MissingPath); return; } @@ -674,7 +682,8 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers, if (!ConnectionManagerUtility::maybeNormalizePath(*request_headers_, connection_manager_.config_)) { sendLocalReply(Grpc::Common::hasGrpcContentType(*request_headers_), Code::BadRequest, "", - nullptr, is_head_request_, absl::nullopt); + nullptr, is_head_request_, absl::nullopt, + StreamInfo::ResponseCodeDetails::get().PathNormalizationFailed); return; } @@ -704,7 +713,8 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers, // Do not allow upgrades if the route does not support it. connection_manager_.stats_.named_.downstream_rq_ws_on_non_ws_route_.inc(); sendLocalReply(Grpc::Common::hasGrpcContentType(*request_headers_), Code::Forbidden, "", - nullptr, is_head_request_, absl::nullopt); + nullptr, is_head_request_, absl::nullopt, + StreamInfo::ResponseCodeDetails::get().UpgradeFailed); return; } // Allow non websocket requests to go through websocket enabled routes. @@ -1114,13 +1124,15 @@ void ConnectionManagerImpl::ActiveStream::refreshCachedRoute() { void ConnectionManagerImpl::ActiveStream::sendLocalReply( bool is_grpc_request, Code code, absl::string_view body, const std::function& modify_headers, bool is_head_request, - const absl::optional grpc_status) { + const absl::optional grpc_status, absl::string_view details) { + ENVOY_STREAM_LOG(debug, "Sending local reply with details {}", *this, details); ASSERT(response_headers_ == nullptr); // For early error handling, do a best-effort attempt to create a filter chain // to ensure access logging. if (!state_.created_filter_chain_) { createFilterChain(); } + stream_info_.setResponseCodeDetails(details); Utility::sendLocalReply( is_grpc_request, [this, modify_headers](HeaderMapPtr&& headers, bool end_stream) -> void { @@ -1875,7 +1887,7 @@ void ConnectionManagerImpl::ActiveStreamDecoderFilter::requestDataTooLarge() { } else { parent_.connection_manager_.stats_.named_.downstream_rq_too_large_.inc(); sendLocalReply(Code::PayloadTooLarge, CodeUtility::toString(Code::PayloadTooLarge), nullptr, - absl::nullopt); + absl::nullopt, StreamInfo::ResponseCodeDetails::get().RequestPayloadTooLarge); } } @@ -1980,6 +1992,8 @@ void ConnectionManagerImpl::ActiveStreamEncoderFilter::responseDataTooLarge() { parent_.state_.encoder_filters_streaming_ = true; allowIteration(); + parent_.stream_info_.setResponseCodeDetails( + StreamInfo::ResponseCodeDetails::get().RequestHeadersTooLarge); Http::Utility::sendLocalReply( Grpc::Common::hasGrpcContentType(*parent_.request_headers_), [&](HeaderMapPtr&& response_headers, bool end_stream) -> void { diff --git a/source/common/http/conn_manager_impl.h b/source/common/http/conn_manager_impl.h index 3d61de6622912..a932e920c2cd2 100644 --- a/source/common/http/conn_manager_impl.h +++ b/source/common/http/conn_manager_impl.h @@ -221,11 +221,11 @@ class ConnectionManagerImpl : Logger::Loggable, void sendLocalReply(Code code, absl::string_view body, std::function modify_headers, - const absl::optional grpc_status) override { - // TODO(#6542): add an extra parameter for setting rc details - parent_.stream_info_.setResponseCodeDetails(""); + const absl::optional grpc_status, + absl::string_view details) override { + parent_.stream_info_.setResponseCodeDetails(details); parent_.sendLocalReply(is_grpc_request_, code, body, modify_headers, parent_.is_head_request_, - grpc_status); + grpc_status, details); } void encode100ContinueHeaders(HeaderMapPtr&& headers) override; void encodeHeaders(HeaderMapPtr&& headers, bool end_stream) override; @@ -359,7 +359,8 @@ class ConnectionManagerImpl : Logger::Loggable, void sendLocalReply(bool is_grpc_request, Code code, absl::string_view body, const std::function& modify_headers, bool is_head_request, - const absl::optional grpc_status); + const absl::optional grpc_status, + absl::string_view details); void encode100ContinueHeaders(ActiveStreamEncoderFilter* filter, HeaderMap& headers); void encodeHeaders(ActiveStreamEncoderFilter* filter, HeaderMap& headers, bool end_stream); // Sends data through encoding filter chains. filter_iteration_start_state indicates which diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 47f40754ba81e..bdfcd87e0869a 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -291,7 +291,8 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e headers.Path()->value().getStringView()); callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::NoRouteFound); - callbacks_->sendLocalReply(Http::Code::NotFound, "", nullptr, absl::nullopt); + callbacks_->sendLocalReply(Http::Code::NotFound, "", nullptr, absl::nullopt, + StreamInfo::ResponseCodeDetails::get().RouteNotFound); return Http::FilterHeadersStatus::StopIteration; } @@ -310,7 +311,7 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e } direct_response->finalizeResponseHeaders(response_headers, callbacks_->streamInfo()); }, - absl::nullopt); + absl::nullopt, StreamInfo::ResponseCodeDetails::get().DirectResponse); return Http::FilterHeadersStatus::StopIteration; } @@ -323,7 +324,8 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::NoRouteFound); callbacks_->sendLocalReply(route_entry_->clusterNotFoundResponseCode(), "", nullptr, - absl::nullopt); + absl::nullopt, + StreamInfo::ResponseCodeDetails::get().ClusterNotFound); return Http::FilterHeadersStatus::StopIteration; } cluster_ = cluster->info(); @@ -350,7 +352,7 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e headers.insertEnvoyOverloaded().value(Http::Headers::get().EnvoyOverloadedValues.True); } }, - absl::nullopt); + absl::nullopt, StreamInfo::ResponseCodeDetails::get().MaintenanceMode); cluster_->stats().upstream_rq_maintenance_mode_.inc(); return Http::FilterHeadersStatus::StopIteration; } @@ -424,7 +426,8 @@ void Filter::sendNoHealthyUpstreamResponse() { callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::NoHealthyUpstream); chargeUpstreamCode(Http::Code::ServiceUnavailable, nullptr, false); callbacks_->sendLocalReply(Http::Code::ServiceUnavailable, "no healthy upstream", nullptr, - absl::nullopt); + absl::nullopt, + StreamInfo::ResponseCodeDetails::get().NoHealthyUpstream); } Http::FilterDataStatus Filter::decodeData(Buffer::Instance& data, bool end_stream) { @@ -562,7 +565,8 @@ void Filter::onResponseTimeout() { upstream_requests_.front()->resetStream(); } - onUpstreamTimeoutAbort(StreamInfo::ResponseFlag::UpstreamRequestTimeout); + onUpstreamTimeoutAbort(StreamInfo::ResponseFlag::UpstreamRequestTimeout, + StreamInfo::ResponseCodeDetails::get().UpstreamTimeout); } void Filter::onPerTryTimeout(UpstreamRequest& upstream_request) { @@ -572,7 +576,8 @@ void Filter::onPerTryTimeout(UpstreamRequest& upstream_request) { return; } - onUpstreamTimeoutAbort(StreamInfo::ResponseFlag::UpstreamRequestTimeout); + onUpstreamTimeoutAbort(StreamInfo::ResponseFlag::UpstreamRequestTimeout, + StreamInfo::ResponseCodeDetails::get().UpstreamPerTryTimeout); } void Filter::updateOutlierDetection(Http::Code code, UpstreamRequest& upstream_request) { @@ -581,14 +586,15 @@ void Filter::updateOutlierDetection(Http::Code code, UpstreamRequest& upstream_r } } -void Filter::onUpstreamTimeoutAbort(StreamInfo::ResponseFlag response_flags) { +void Filter::onUpstreamTimeoutAbort(StreamInfo::ResponseFlag response_flags, + absl::string_view details) { const absl::string_view body = timeout_response_code_ == Http::Code::GatewayTimeout ? "upstream request timeout" : ""; - onUpstreamAbort(timeout_response_code_, response_flags, body, false); + onUpstreamAbort(timeout_response_code_, response_flags, body, false, details); } void Filter::onUpstreamAbort(Http::Code code, StreamInfo::ResponseFlag response_flags, - absl::string_view body, bool dropped) { + absl::string_view body, bool dropped, absl::string_view details) { ASSERT(upstream_requests_.size() <= 1); // If we have not yet sent anything downstream, send a response with an appropriate status code. // Otherwise just reset the ongoing response. @@ -598,6 +604,7 @@ void Filter::onUpstreamAbort(Http::Code code, StreamInfo::ResponseFlag response_ config_.stats_.rq_reset_after_downstream_response_started_.inc(); } // This will destroy any created retry timers. + callbacks_->streamInfo().setResponseCodeDetails(details); cleanup(); callbacks_->resetStream(); } else { @@ -626,7 +633,7 @@ void Filter::onUpstreamAbort(Http::Code code, StreamInfo::ResponseFlag response_ headers.insertEnvoyOverloaded().value(Http::Headers::get().EnvoyOverloadedValues.True); } }, - absl::nullopt); + absl::nullopt, details); } } @@ -680,7 +687,13 @@ void Filter::onUpstreamReset(Http::StreamResetReason reset_reason, const bool dropped = reset_reason == Http::StreamResetReason::Overflow; callbacks_->streamInfo().setUpstreamTransportFailureReason(transport_failure_reason); - onUpstreamAbort(Http::Code::ServiceUnavailable, response_flags, body, dropped); + const std::string& basic_details = + downstream_response_started_ ? StreamInfo::ResponseCodeDetails::get().LateUpstreamReset + : StreamInfo::ResponseCodeDetails::get().EarlyUpstreamReset; + const std::string details = absl::StrCat( + basic_details, "{", Http::Utility::resetReasonToString(reset_reason), + transport_failure_reason.empty() ? "" : absl::StrCat(",", transport_failure_reason), "}"); + onUpstreamAbort(Http::Code::ServiceUnavailable, response_flags, body, dropped, details); } StreamInfo::ResponseFlag diff --git a/source/common/router/router.h b/source/common/router/router.h index 0c560ba8fe80c..04cb299cab525 100644 --- a/source/common/router/router.h +++ b/source/common/router/router.h @@ -384,12 +384,12 @@ class Filter : Logger::Loggable, void onResponseTimeout(); void onUpstream100ContinueHeaders(Http::HeaderMapPtr&& headers); // Handle an upstream request aborted due to a local timeout. - void onUpstreamTimeoutAbort(StreamInfo::ResponseFlag response_flag); + void onUpstreamTimeoutAbort(StreamInfo::ResponseFlag response_flag, absl::string_view details); // Handle an "aborted" upstream request, meaning we didn't see response // headers (e.g. due to a reset). Handles recording stats and responding // downstream if appropriate. void onUpstreamAbort(Http::Code code, StreamInfo::ResponseFlag response_flag, - absl::string_view body, bool dropped); + absl::string_view body, bool dropped, absl::string_view details); void onUpstreamHeaders(uint64_t response_code, Http::HeaderMapPtr&& headers, UpstreamRequest& upstream_request, bool end_stream); void onUpstreamData(Buffer::Instance& data, UpstreamRequest& upstream_request, bool end_stream); diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc index dd6e61bcccf34..201c8a20e2b41 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -551,8 +551,10 @@ TEST_F(HttpConnectionManagerImplTest, InvalidPathWithDualFilter) { EXPECT_CALL(*filter, encodeHeaders(_, true)); EXPECT_CALL(response_encoder_, encodeHeaders(_, true)) - .WillOnce(Invoke([](const HeaderMap& headers, bool) -> void { + .WillOnce(Invoke([&](const HeaderMap& headers, bool) -> void { EXPECT_EQ("404", headers.Status()->value().getStringView()); + EXPECT_EQ("absolute_path_rejected", + filter->decoder_callbacks_->streamInfo().responseCodeDetails().value()); })); EXPECT_CALL(*filter, onDestroy()); @@ -588,8 +590,10 @@ TEST_F(HttpConnectionManagerImplTest, PathFailedtoSanitize) { EXPECT_CALL(*filter, encodeHeaders(_, true)); EXPECT_CALL(response_encoder_, encodeHeaders(_, true)) - .WillOnce(Invoke([](const HeaderMap& headers, bool) -> void { + .WillOnce(Invoke([&](const HeaderMap& headers, bool) -> void { EXPECT_EQ("400", headers.Status()->value().getStringView()); + EXPECT_EQ("path_normalization_failed", + filter->decoder_callbacks_->streamInfo().responseCodeDetails().value()); })); EXPECT_CALL(*filter, onDestroy()); @@ -1191,6 +1195,7 @@ TEST_F(HttpConnectionManagerImplTest, TestAccessLogWithInvalidRequest) { const StreamInfo::StreamInfo& stream_info) { EXPECT_TRUE(stream_info.responseCode()); EXPECT_EQ(stream_info.responseCode().value(), uint32_t(400)); + EXPECT_EQ("missing_host_header", stream_info.responseCodeDetails().value()); EXPECT_NE(nullptr, stream_info.downstreamLocalAddress()); EXPECT_NE(nullptr, stream_info.downstreamRemoteAddress()); EXPECT_NE(nullptr, stream_info.downstreamDirectRemoteAddress()); @@ -3130,8 +3135,7 @@ TEST_F(HttpConnectionManagerImplTest, HitRequestBufferLimits) { Buffer::OwnedImpl data("A longer string"); decoder_filters_[0]->callbacks_->addDecodedData(data, false); const auto rc_details = encoder_filters_[1]->callbacks_->streamInfo().responseCodeDetails(); - EXPECT_TRUE(rc_details.has_value()); - EXPECT_EQ("", rc_details.value()); + EXPECT_EQ("request_payload_too_large", rc_details.value()); } // Return 413 from an intermediate filter and make sure we don't continue the filter chain. diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index e083c5e587ade..56949106c6131 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -259,6 +259,7 @@ TEST_F(RouterTest, RouteNotFound) { router_.decodeHeaders(headers, true); EXPECT_EQ(1UL, stats_store_.counter("test.no_route").value()); EXPECT_TRUE(verifyHostUpstreamStats(0, 0)); + EXPECT_EQ(callbacks_.details_, "route_not_found"); } TEST_F(RouterTest, ClusterNotFound) { @@ -271,6 +272,7 @@ TEST_F(RouterTest, ClusterNotFound) { router_.decodeHeaders(headers, true); EXPECT_EQ(1UL, stats_store_.counter("test.no_cluster").value()); EXPECT_TRUE(verifyHostUpstreamStats(0, 0)); + EXPECT_EQ(callbacks_.details_, "cluster_not_found"); } TEST_F(RouterTest, PoolFailureWithPriority) { @@ -301,6 +303,7 @@ TEST_F(RouterTest, PoolFailureWithPriority) { HttpTestUtility::addDefaultHeaders(headers); router_.decodeHeaders(headers, true); EXPECT_TRUE(verifyHostUpstreamStats(0, 1)); + EXPECT_EQ(callbacks_.details_, "upstream_reset_before_response_started{connection failure}"); } TEST_F(RouterTest, Http1Upstream) { @@ -500,6 +503,8 @@ TEST_F(RouterTest, AddCookie) { HttpTestUtility::addDefaultHeaders(headers); router_.decodeHeaders(headers, true); + absl::string_view rc_details2 = "via_upstream"; + EXPECT_CALL(callbacks_.stream_info_, setResponseCodeDetails(rc_details2)); Http::HeaderMapPtr response_headers(new Http::TestHeaderMapImpl{{":status", "200"}}); response_decoder->decodeHeaders(std::move(response_headers), true); // When the router filter gets reset we should cancel the pool request. @@ -696,6 +701,7 @@ TEST_F(RouterTest, NoHost) { .counter("upstream_rq_maintenance_mode") .value()); EXPECT_TRUE(verifyHostUpstreamStats(0, 0)); + EXPECT_EQ(callbacks_.details_, "no_healthy_upstream"); } TEST_F(RouterTest, MaintenanceMode) { @@ -720,6 +726,7 @@ TEST_F(RouterTest, MaintenanceMode) { EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->load_report_stats_store_ .counter("upstream_rq_dropped") .value()); + EXPECT_EQ(callbacks_.details_, "maintenance_mode"); } // Validate that we don't set x-envoy-overloaded when Envoy header suppression @@ -1555,6 +1562,8 @@ TEST_F(RouterTest, RetryUpstreamResetResponseStarted) { Http::HeaderMapPtr response_headers(new Http::TestHeaderMapImpl{{":status", "200"}}); EXPECT_CALL(cm_.conn_pool_.host_->outlier_detector_, putHttpResponseCode(200)); response_decoder->decodeHeaders(std::move(response_headers), false); + absl::string_view rc_details2 = "upstream_reset_after_response_started{remote reset}"; + EXPECT_CALL(callbacks_.stream_info_, setResponseCodeDetails(rc_details2)); EXPECT_CALL(cm_.conn_pool_.host_->outlier_detector_, putHttpResponseCode(503)); encoder1.stream_.resetStream(Http::StreamResetReason::RemoteReset); // For normal HTTP, once we have a 200 we consider this a success, even if a @@ -3108,6 +3117,7 @@ TEST_F(WatermarkTest, RetryRequestNotComplete) { // This should not trigger a retry as the retry state has been deleted. EXPECT_CALL(cm_.conn_pool_.host_->outlier_detector_, putHttpResponseCode(503)); encoder1.stream_.resetStream(Http::StreamResetReason::RemoteReset); + EXPECT_EQ(callbacks_.details_, "upstream_reset_before_response_started{remote reset}"); } class RouterTestChildSpan : public RouterTestBase { diff --git a/test/extensions/access_loggers/http_grpc/grpc_access_log_integration_test.cc b/test/extensions/access_loggers/http_grpc/grpc_access_log_integration_test.cc index 0e3188e776839..11ef6b03cf299 100644 --- a/test/extensions/access_loggers/http_grpc/grpc_access_log_integration_test.cc +++ b/test/extensions/access_loggers/http_grpc/grpc_access_log_integration_test.cc @@ -134,6 +134,7 @@ TEST_P(AccessLogIntegrationTest, BasicAccessLogFlow) { response: response_code: value: 404 + response_code_details: "route_not_found" response_headers_bytes: 54 )EOF", VersionInfo::version()))); @@ -157,6 +158,7 @@ TEST_P(AccessLogIntegrationTest, BasicAccessLogFlow) { response: response_code: value: 404 + response_code_details: "route_not_found" response_headers_bytes: 54 )EOF")); @@ -204,6 +206,7 @@ TEST_P(AccessLogIntegrationTest, BasicAccessLogFlow) { response: response_code: value: 404 + response_code_details: "route_not_found" response_headers_bytes: 54 )EOF", VersionInfo::version()))); diff --git a/test/mocks/http/mocks.h b/test/mocks/http/mocks.h index a5f2971743c2b..774672196a95e 100644 --- a/test/mocks/http/mocks.h +++ b/test/mocks/http/mocks.h @@ -152,7 +152,9 @@ class MockStreamDecoderFilterCallbacks : public StreamDecoderFilterCallbacks, // Http::StreamDecoderFilterCallbacks void sendLocalReply(Code code, absl::string_view body, std::function modify_headers, - const absl::optional grpc_status) override { + const absl::optional grpc_status, + absl::string_view details) override { + details_ = std::string(details); Utility::sendLocalReply( is_grpc_request_, [this, modify_headers](HeaderMapPtr&& headers, bool end_stream) -> void { @@ -191,6 +193,7 @@ class MockStreamDecoderFilterCallbacks : public StreamDecoderFilterCallbacks, std::list callbacks_{}; testing::NiceMock active_span_; testing::NiceMock tracing_config_; + std::string details_; bool is_grpc_request_{}; bool is_head_request_{false}; bool stream_destroyed_{};