diff --git a/api/envoy/api/v2/core/protocol.proto b/api/envoy/api/v2/core/protocol.proto index 5838ca7440759..9c47e388ee1af 100644 --- a/api/envoy/api/v2/core/protocol.proto +++ b/api/envoy/api/v2/core/protocol.proto @@ -85,8 +85,6 @@ message HttpProtocolOptions { // Total duration to keep alive an HTTP request/response stream. If the time limit is reached the stream will be // reset independent of any other timeouts. If not specified, this value is not set. - // The current implementation implements this timeout on downstream connections only. - // [#comment:TODO(shikugawa): add this functionality to upstream.] google.protobuf.Duration max_stream_duration = 4; // Action to take when a client request with a header name containing underscore characters is received. diff --git a/api/envoy/config/accesslog/v3/accesslog.proto b/api/envoy/config/accesslog/v3/accesslog.proto index f5732ba3f8e42..218ad5bda4b88 100644 --- a/api/envoy/config/accesslog/v3/accesslog.proto +++ b/api/envoy/config/accesslog/v3/accesslog.proto @@ -240,6 +240,7 @@ message ResponseFlagFilter { in: "SI" in: "IH" in: "DPE" + in: "UMSDR" } } }]; diff --git a/api/envoy/config/accesslog/v4alpha/accesslog.proto b/api/envoy/config/accesslog/v4alpha/accesslog.proto index 56911ca191855..5900f62f4ffee 100644 --- a/api/envoy/config/accesslog/v4alpha/accesslog.proto +++ b/api/envoy/config/accesslog/v4alpha/accesslog.proto @@ -239,6 +239,7 @@ message ResponseFlagFilter { in: "SI" in: "IH" in: "DPE" + in: "UMSDR" } } }]; diff --git a/api/envoy/config/core/v3/protocol.proto b/api/envoy/config/core/v3/protocol.proto index 400b0dd95a940..7866b87999e4e 100644 --- a/api/envoy/config/core/v3/protocol.proto +++ b/api/envoy/config/core/v3/protocol.proto @@ -92,8 +92,6 @@ message HttpProtocolOptions { // Total duration to keep alive an HTTP request/response stream. If the time limit is reached the stream will be // reset independent of any other timeouts. If not specified, this value is not set. - // The current implementation implements this timeout on downstream connections only. - // [#comment:TODO(shikugawa): add this functionality to upstream.] google.protobuf.Duration max_stream_duration = 4; // Action to take when a client request with a header name containing underscore characters is received. diff --git a/api/envoy/config/core/v4alpha/protocol.proto b/api/envoy/config/core/v4alpha/protocol.proto index dcb205444524f..773aa184bdba2 100644 --- a/api/envoy/config/core/v4alpha/protocol.proto +++ b/api/envoy/config/core/v4alpha/protocol.proto @@ -92,8 +92,6 @@ message HttpProtocolOptions { // Total duration to keep alive an HTTP request/response stream. If the time limit is reached the stream will be // reset independent of any other timeouts. If not specified, this value is not set. - // The current implementation implements this timeout on downstream connections only. - // [#comment:TODO(shikugawa): add this functionality to upstream.] google.protobuf.Duration max_stream_duration = 4; // Action to take when a client request with a header name containing underscore characters is received. diff --git a/api/envoy/data/accesslog/v3/accesslog.proto b/api/envoy/data/accesslog/v3/accesslog.proto index 374569d937f28..c97e2f4acef01 100644 --- a/api/envoy/data/accesslog/v3/accesslog.proto +++ b/api/envoy/data/accesslog/v3/accesslog.proto @@ -186,7 +186,7 @@ message AccessLogCommon { } // Flags indicating occurrences during request/response processing. -// [#next-free-field: 20] +// [#next-free-field: 21] message ResponseFlags { option (udpa.annotations.versioning).previous_message_type = "envoy.data.accesslog.v2.ResponseFlags"; @@ -263,6 +263,9 @@ message ResponseFlags { // Indicates there was an HTTP protocol error on the downstream request. bool downstream_protocol_error = 19; + + // Indicates there was a max stream duration reached on the upstream request. + bool upstream_max_stream_duration_reached = 20; } // Properties of a negotiated TLS connection. diff --git a/clang-tidy-fixes.yaml b/clang-tidy-fixes.yaml new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/docs/root/configuration/observability/access_log/usage.rst b/docs/root/configuration/observability/access_log/usage.rst index 46637c05ec4b5..1b8b6d51e915e 100644 --- a/docs/root/configuration/observability/access_log/usage.rst +++ b/docs/root/configuration/observability/access_log/usage.rst @@ -274,6 +274,7 @@ The following command operators are supported: :ref:`strictly-checked header ` in addition to 400 response code. * **SI**: Stream idle timeout in addition to 408 response code. * **DPE**: The downstream request had an HTTP protocol error. + * **UMSDR**: The upstream request reached to max stream duration. %ROUTE_NAME% Name of the route. diff --git a/docs/root/configuration/upstream/cluster_manager/cluster_stats.rst b/docs/root/configuration/upstream/cluster_manager/cluster_stats.rst index e58a1d32c90c5..96d385ffebca9 100644 --- a/docs/root/configuration/upstream/cluster_manager/cluster_stats.rst +++ b/docs/root/configuration/upstream/cluster_manager/cluster_stats.rst @@ -69,6 +69,7 @@ Every cluster has a statistics tree rooted at *cluster..* with the followi upstream_rq_cancelled, Counter, Total requests cancelled before obtaining a connection pool connection upstream_rq_maintenance_mode, Counter, Total requests that resulted in an immediate 503 due to :ref:`maintenance mode` upstream_rq_timeout, Counter, Total requests that timed out waiting for a response + upstream_rq_max_duration_reached, Counter, Total requests closed due to max duration reached upstream_rq_per_try_timeout, Counter, Total requests that hit the per try timeout upstream_rq_rx_reset, Counter, Total requests that were reset remotely upstream_rq_tx_reset, Counter, Total requests that were reset locally diff --git a/docs/root/faq/configuration/timeouts.rst b/docs/root/faq/configuration/timeouts.rst index 4cdca3a57167e..a1c92544e8f9c 100644 --- a/docs/root/faq/configuration/timeouts.rst +++ b/docs/root/faq/configuration/timeouts.rst @@ -59,10 +59,7 @@ context request/stream is interchangeable. HTTP request/response streams periodically. You can't use :ref:`request_timeout ` in this situation because this timer will be disarmed if a response header is received on the request/response streams. - - .. attention:: - - The current implementation implements this timeout on downstream connections only. + This timeout is available on both upstream and downstream connections. Route timeouts ^^^^^^^^^^^^^^ diff --git a/generated_api_shadow/envoy/api/v2/core/protocol.proto b/generated_api_shadow/envoy/api/v2/core/protocol.proto index 5838ca7440759..9c47e388ee1af 100644 --- a/generated_api_shadow/envoy/api/v2/core/protocol.proto +++ b/generated_api_shadow/envoy/api/v2/core/protocol.proto @@ -85,8 +85,6 @@ message HttpProtocolOptions { // Total duration to keep alive an HTTP request/response stream. If the time limit is reached the stream will be // reset independent of any other timeouts. If not specified, this value is not set. - // The current implementation implements this timeout on downstream connections only. - // [#comment:TODO(shikugawa): add this functionality to upstream.] google.protobuf.Duration max_stream_duration = 4; // Action to take when a client request with a header name containing underscore characters is received. diff --git a/generated_api_shadow/envoy/config/accesslog/v3/accesslog.proto b/generated_api_shadow/envoy/config/accesslog/v3/accesslog.proto index da29f198802f2..1edd344076359 100644 --- a/generated_api_shadow/envoy/config/accesslog/v3/accesslog.proto +++ b/generated_api_shadow/envoy/config/accesslog/v3/accesslog.proto @@ -238,6 +238,7 @@ message ResponseFlagFilter { in: "SI" in: "IH" in: "DPE" + in: "UMSDR" } } }]; diff --git a/generated_api_shadow/envoy/config/accesslog/v4alpha/accesslog.proto b/generated_api_shadow/envoy/config/accesslog/v4alpha/accesslog.proto index 56911ca191855..5900f62f4ffee 100644 --- a/generated_api_shadow/envoy/config/accesslog/v4alpha/accesslog.proto +++ b/generated_api_shadow/envoy/config/accesslog/v4alpha/accesslog.proto @@ -239,6 +239,7 @@ message ResponseFlagFilter { in: "SI" in: "IH" in: "DPE" + in: "UMSDR" } } }]; diff --git a/generated_api_shadow/envoy/config/core/v3/protocol.proto b/generated_api_shadow/envoy/config/core/v3/protocol.proto index 400b0dd95a940..7866b87999e4e 100644 --- a/generated_api_shadow/envoy/config/core/v3/protocol.proto +++ b/generated_api_shadow/envoy/config/core/v3/protocol.proto @@ -92,8 +92,6 @@ message HttpProtocolOptions { // Total duration to keep alive an HTTP request/response stream. If the time limit is reached the stream will be // reset independent of any other timeouts. If not specified, this value is not set. - // The current implementation implements this timeout on downstream connections only. - // [#comment:TODO(shikugawa): add this functionality to upstream.] google.protobuf.Duration max_stream_duration = 4; // Action to take when a client request with a header name containing underscore characters is received. diff --git a/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto b/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto index dcb205444524f..773aa184bdba2 100644 --- a/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto +++ b/generated_api_shadow/envoy/config/core/v4alpha/protocol.proto @@ -92,8 +92,6 @@ message HttpProtocolOptions { // Total duration to keep alive an HTTP request/response stream. If the time limit is reached the stream will be // reset independent of any other timeouts. If not specified, this value is not set. - // The current implementation implements this timeout on downstream connections only. - // [#comment:TODO(shikugawa): add this functionality to upstream.] google.protobuf.Duration max_stream_duration = 4; // Action to take when a client request with a header name containing underscore characters is received. diff --git a/generated_api_shadow/envoy/data/accesslog/v3/accesslog.proto b/generated_api_shadow/envoy/data/accesslog/v3/accesslog.proto index 374569d937f28..c97e2f4acef01 100644 --- a/generated_api_shadow/envoy/data/accesslog/v3/accesslog.proto +++ b/generated_api_shadow/envoy/data/accesslog/v3/accesslog.proto @@ -186,7 +186,7 @@ message AccessLogCommon { } // Flags indicating occurrences during request/response processing. -// [#next-free-field: 20] +// [#next-free-field: 21] message ResponseFlags { option (udpa.annotations.versioning).previous_message_type = "envoy.data.accesslog.v2.ResponseFlags"; @@ -263,6 +263,9 @@ message ResponseFlags { // Indicates there was an HTTP protocol error on the downstream request. bool downstream_protocol_error = 19; + + // Indicates there was a max stream duration reached on the upstream request. + bool upstream_max_stream_duration_reached = 20; } // Properties of a negotiated TLS connection. diff --git a/include/envoy/stream_info/stream_info.h b/include/envoy/stream_info/stream_info.h index 89824f4190f49..bb4a2e73382d2 100644 --- a/include/envoy/stream_info/stream_info.h +++ b/include/envoy/stream_info/stream_info.h @@ -72,8 +72,10 @@ enum ResponseFlag { InvalidEnvoyRequestHeaders = 0x20000, // Downstream request had an HTTP protocol error DownstreamProtocolError = 0x40000, + // Upstream request reached to user defined max stream duration. + UpstreamMaxStreamDurationReached = 0x80000, // ATTENTION: MAKE SURE THIS REMAINS EQUAL TO THE LAST FLAG. - LastFlag = DownstreamProtocolError + LastFlag = UpstreamMaxStreamDurationReached }; /** @@ -139,6 +141,8 @@ struct ResponseCodeDetailValues { const std::string UpstreamTimeout = "upstream_response_timeout"; // The final upstream try timed out const std::string UpstreamPerTryTimeout = "upstream_per_try_timeout"; + // The request was destroyed because of user defined max stream duration. + const std::string UpstreamMaxStreamDurationReached = "upstream_max_stream_duration_reached"; // 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"; diff --git a/include/envoy/upstream/upstream.h b/include/envoy/upstream/upstream.h index 7ed52ca584dab..5e8aa41e2ff53 100644 --- a/include/envoy/upstream/upstream.h +++ b/include/envoy/upstream/upstream.h @@ -571,6 +571,7 @@ class PrioritySet { COUNTER(upstream_rq_cancelled) \ COUNTER(upstream_rq_completed) \ COUNTER(upstream_rq_maintenance_mode) \ + COUNTER(upstream_rq_max_duration_reached) \ COUNTER(upstream_rq_pending_failure_eject) \ COUNTER(upstream_rq_pending_overflow) \ COUNTER(upstream_rq_pending_total) \ @@ -728,6 +729,12 @@ class ClusterInfo { */ virtual const envoy::config::core::v3::Http2ProtocolOptions& http2Options() const PURE; + /** + * @return const envoy::config::core::v3::HttpProtocolOptions for all of HTTP versions. + */ + virtual const envoy::config::core::v3::HttpProtocolOptions& + commonHttpProtocolOptions() const PURE; + /** * @param name std::string containing the well-known name of the extension for which protocol * options are desired diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 8d1ab9647bb84..fb676ed514e57 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -984,6 +984,29 @@ void Filter::onPerTryTimeout(UpstreamRequest& upstream_request) { StreamInfo::ResponseCodeDetails::get().UpstreamPerTryTimeout); } +void Filter::onStreamMaxDurationReached(UpstreamRequest& upstream_request) { + upstream_request.resetStream(); + + if (maybeRetryReset(Http::StreamResetReason::LocalReset, upstream_request)) { + return; + } + + upstream_request.removeFromList(upstream_requests_); + cleanup(); + + if (downstream_response_started_) { + callbacks_->streamInfo().setResponseCodeDetails( + StreamInfo::ResponseCodeDetails::get().UpstreamMaxStreamDurationReached); + callbacks_->resetStream(); + } else { + callbacks_->streamInfo().setResponseFlag( + StreamInfo::ResponseFlag::UpstreamMaxStreamDurationReached); + callbacks_->sendLocalReply( + Http::Code::RequestTimeout, "upstream max stream duration reached", modify_headers_, + absl::nullopt, StreamInfo::ResponseCodeDetails::get().UpstreamMaxStreamDurationReached); + } +} + void Filter::updateOutlierDetection(Upstream::Outlier::Result result, UpstreamRequest& upstream_request, absl::optional code) { diff --git a/source/common/router/router.h b/source/common/router/router.h index c74a333233773..058e82bdc540c 100644 --- a/source/common/router/router.h +++ b/source/common/router/router.h @@ -265,6 +265,7 @@ class RouterFilterInterface { UpstreamRequest& upstream_request) PURE; virtual void onUpstreamHostSelected(Upstream::HostDescriptionConstSharedPtr host) PURE; virtual void onPerTryTimeout(UpstreamRequest& upstream_request) PURE; + virtual void onStreamMaxDurationReached(UpstreamRequest& upstream_request) PURE; virtual Http::StreamDecoderFilterCallbacks* callbacks() PURE; virtual Upstream::ClusterInfoConstSharedPtr cluster() PURE; @@ -432,6 +433,7 @@ class Filter : Logger::Loggable, UpstreamRequest& upstream_request) override; void onUpstreamHostSelected(Upstream::HostDescriptionConstSharedPtr host) override; void onPerTryTimeout(UpstreamRequest& upstream_request) override; + void onStreamMaxDurationReached(UpstreamRequest& upstream_request) override; Http::StreamDecoderFilterCallbacks* callbacks() override { return callbacks_; } Upstream::ClusterInfoConstSharedPtr cluster() override { return cluster_; } FilterConfig& config() override { return config_; } diff --git a/source/common/router/upstream_request.cc b/source/common/router/upstream_request.cc index 6130d64de9df2..be15ec7a8e675 100644 --- a/source/common/router/upstream_request.cc +++ b/source/common/router/upstream_request.cc @@ -78,6 +78,9 @@ UpstreamRequest::~UpstreamRequest() { // Allows for testing. per_try_timeout_->disableTimer(); } + if (max_stream_duration_timer_ != nullptr) { + max_stream_duration_timer_->disableTimer(); + } clearRequestEncoder(); // If desired, fire the per-try histogram when the UpstreamRequest @@ -382,7 +385,18 @@ void UpstreamRequest::onPoolReady( paused_for_connect_ = true; } + if (upstream_host_->cluster().commonHttpProtocolOptions().has_max_stream_duration()) { + const auto max_stream_duration = std::chrono::milliseconds(DurationUtil::durationToMilliseconds( + upstream_host_->cluster().commonHttpProtocolOptions().max_stream_duration())); + if (max_stream_duration.count()) { + max_stream_duration_timer_ = parent_.callbacks()->dispatcher().createTimer( + [this]() -> void { onStreamMaxDurationReached(); }); + max_stream_duration_timer_->enableTimer(max_stream_duration); + } + } + upstream_->encodeHeaders(*parent_.downstreamHeaders(), shouldSendEndStream()); + calling_encode_headers_ = false; if (!paused_for_connect_) { @@ -426,6 +440,13 @@ void UpstreamRequest::encodeBodyAndTrailers() { } } +void UpstreamRequest::onStreamMaxDurationReached() { + upstream_host_->cluster().stats().upstream_rq_max_duration_reached_.inc(); + + // The upstream had closed then try to retry along with retry policy. + parent_.onStreamMaxDurationReached(*this); +} + void UpstreamRequest::clearRequestEncoder() { // Before clearing the encoder, unsubscribe from callbacks. if (upstream_) { diff --git a/source/common/router/upstream_request.h b/source/common/router/upstream_request.h index 3dd852fab2f4a..c215f9d45647c 100644 --- a/source/common/router/upstream_request.h +++ b/source/common/router/upstream_request.h @@ -107,6 +107,7 @@ class UpstreamRequest : public Logger::Loggable, UpstreamRequest* upstreamRequest() override { return this; } void clearRequestEncoder(); + void onStreamMaxDurationReached(); struct DownstreamWatermarkManager : public Http::DownstreamWatermarkCallbacks { DownstreamWatermarkManager(UpstreamRequest& parent) : parent_(parent) {} @@ -188,6 +189,8 @@ class UpstreamRequest : public Logger::Loggable, // Sentinel to indicate if timeout budget tracking is configured for the cluster, // and if so, if the per-try histogram should record a value. bool record_timeout_budget_ : 1; + + Event::TimerPtr max_stream_duration_timer_; }; class HttpConnPool : public GenericConnPool, public Http::ConnectionPool::Callbacks { diff --git a/source/common/stream_info/utility.cc b/source/common/stream_info/utility.cc index ccd24cb1acf71..2a173e9dd504d 100644 --- a/source/common/stream_info/utility.cc +++ b/source/common/stream_info/utility.cc @@ -25,6 +25,7 @@ const std::string ResponseFlagUtils::RATELIMIT_SERVICE_ERROR = "RLSE"; const std::string ResponseFlagUtils::STREAM_IDLE_TIMEOUT = "SI"; const std::string ResponseFlagUtils::INVALID_ENVOY_REQUEST_HEADERS = "IH"; const std::string ResponseFlagUtils::DOWNSTREAM_PROTOCOL_ERROR = "DPE"; +const std::string ResponseFlagUtils::UPSTREAM_MAX_STREAM_DURATION_REACHED = "UMSDR"; void ResponseFlagUtils::appendString(std::string& result, const std::string& append) { if (result.empty()) { @@ -37,7 +38,7 @@ void ResponseFlagUtils::appendString(std::string& result, const std::string& app const std::string ResponseFlagUtils::toShortString(const StreamInfo& stream_info) { std::string result; - static_assert(ResponseFlag::LastFlag == 0x40000, "A flag has been added. Fix this code."); + static_assert(ResponseFlag::LastFlag == 0x80000, "A flag has been added. Fix this code."); if (stream_info.hasResponseFlag(ResponseFlag::FailedLocalHealthCheck)) { appendString(result, FAILED_LOCAL_HEALTH_CHECK); @@ -114,6 +115,9 @@ const std::string ResponseFlagUtils::toShortString(const StreamInfo& stream_info appendString(result, DOWNSTREAM_PROTOCOL_ERROR); } + if (stream_info.hasResponseFlag(ResponseFlag::UpstreamMaxStreamDurationReached)) { + appendString(result, UPSTREAM_MAX_STREAM_DURATION_REACHED); + } return result.empty() ? NONE : result; } @@ -140,6 +144,8 @@ absl::optional ResponseFlagUtils::toResponseFlag(const std::string {ResponseFlagUtils::STREAM_IDLE_TIMEOUT, ResponseFlag::StreamIdleTimeout}, {ResponseFlagUtils::INVALID_ENVOY_REQUEST_HEADERS, ResponseFlag::InvalidEnvoyRequestHeaders}, {ResponseFlagUtils::DOWNSTREAM_PROTOCOL_ERROR, ResponseFlag::DownstreamProtocolError}, + {ResponseFlagUtils::UPSTREAM_MAX_STREAM_DURATION_REACHED, + ResponseFlag::UpstreamMaxStreamDurationReached}, }; const auto& it = map.find(flag); if (it != map.end()) { diff --git a/source/common/stream_info/utility.h b/source/common/stream_info/utility.h index fe8059b896439..85285d1d2f809 100644 --- a/source/common/stream_info/utility.h +++ b/source/common/stream_info/utility.h @@ -40,6 +40,7 @@ class ResponseFlagUtils { const static std::string STREAM_IDLE_TIMEOUT; const static std::string INVALID_ENVOY_REQUEST_HEADERS; const static std::string DOWNSTREAM_PROTOCOL_ERROR; + const static std::string UPSTREAM_MAX_STREAM_DURATION_REACHED; }; /** diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 0be39442ef834..bd88b40b53148 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -685,6 +685,7 @@ ClusterInfoImpl::ClusterInfoImpl( features_(parseFeatures(config)), http1_settings_(Http::Utility::parseHttp1Settings(config.http_protocol_options())), http2_options_(Http2::Utility::initializeAndValidateOptions(config.http2_protocol_options())), + common_http_protocol_options_(config.common_http_protocol_options()), extension_protocol_options_(parseExtensionProtocolOptions(config, validation_visitor)), resource_managers_(config, runtime, name_, *stats_scope_), maintenance_mode_runtime_key_(absl::StrCat("upstream.maintenance_mode.", name_)), diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index b9c871ccd94a8..382605591d9d6 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -539,6 +539,9 @@ class ClusterInfoImpl : public ClusterInfo, protected Logger::Loggable extension_protocol_options_; mutable ResourceManagers resource_managers_; const std::string maintenance_mode_runtime_key_; diff --git a/source/extensions/access_loggers/grpc/grpc_access_log_utils.cc b/source/extensions/access_loggers/grpc/grpc_access_log_utils.cc index 1d187bc299852..65ace2eb7edec 100644 --- a/source/extensions/access_loggers/grpc/grpc_access_log_utils.cc +++ b/source/extensions/access_loggers/grpc/grpc_access_log_utils.cc @@ -37,7 +37,7 @@ void Utility::responseFlagsToAccessLogResponseFlags( envoy::data::accesslog::v3::AccessLogCommon& common_access_log, const StreamInfo::StreamInfo& stream_info) { - static_assert(StreamInfo::ResponseFlag::LastFlag == 0x40000, + static_assert(StreamInfo::ResponseFlag::LastFlag == 0x80000, "A flag has been added. Fix this code."); if (stream_info.hasResponseFlag(StreamInfo::ResponseFlag::FailedLocalHealthCheck)) { @@ -116,6 +116,9 @@ void Utility::responseFlagsToAccessLogResponseFlags( if (stream_info.hasResponseFlag(StreamInfo::ResponseFlag::DownstreamProtocolError)) { common_access_log.mutable_response_flags()->set_downstream_protocol_error(true); } + if (stream_info.hasResponseFlag(StreamInfo::ResponseFlag::UpstreamMaxStreamDurationReached)) { + common_access_log.mutable_response_flags()->set_upstream_max_stream_duration_reached(true); + } } void Utility::extractCommonAccessLogProperties( diff --git a/test/common/access_log/access_log_impl_test.cc b/test/common/access_log/access_log_impl_test.cc index 7e5b54ab2bce8..74010eaef8ba3 100644 --- a/test/common/access_log/access_log_impl_test.cc +++ b/test/common/access_log/access_log_impl_test.cc @@ -945,12 +945,13 @@ name: accesslog - SI - IH - DPE + - UMSDR typed_config: "@type": type.googleapis.com/envoy.config.accesslog.v2.FileAccessLog path: /dev/null )EOF"; - static_assert(StreamInfo::ResponseFlag::LastFlag == 0x40000, + static_assert(StreamInfo::ResponseFlag::LastFlag == 0x80000, "A flag has been added. Fix this code."); const std::vector all_response_flags = { @@ -973,7 +974,7 @@ name: accesslog StreamInfo::ResponseFlag::StreamIdleTimeout, StreamInfo::ResponseFlag::InvalidEnvoyRequestHeaders, StreamInfo::ResponseFlag::DownstreamProtocolError, - }; + StreamInfo::ResponseFlag::UpstreamMaxStreamDurationReached}; InstanceSharedPtr log = AccessLogFactory::fromProto(parseAccessLogFromV2Yaml(yaml), context_); @@ -1005,7 +1006,7 @@ name: accesslog "[\"embedded message failed validation\"] | caused by " "ResponseFlagFilterValidationError.Flags[i]: [\"value must be in list \" [\"LH\" \"UH\" " "\"UT\" \"LR\" \"UR\" \"UF\" \"UC\" \"UO\" \"NR\" \"DI\" \"FI\" \"RL\" \"UAEX\" \"RLSE\" " - "\"DC\" \"URX\" \"SI\" \"IH\" \"DPE\"]]): name: \"accesslog\"\nfilter {\n " + "\"DC\" \"URX\" \"SI\" \"IH\" \"DPE\" \"UMSDR\"]]): name: \"accesslog\"\nfilter {\n " "response_flag_filter {\n flags: \"UnsupportedFlag\"\n }\n}\ntyped_config {\n " "[type.googleapis.com/envoy.config.accesslog.v2.FileAccessLog] {\n path: \"/dev/null\"\n " "}\n}\n"); @@ -1031,7 +1032,7 @@ name: accesslog "[\"embedded message failed validation\"] | caused by " "ResponseFlagFilterValidationError.Flags[i]: [\"value must be in list \" [\"LH\" \"UH\" " "\"UT\" \"LR\" \"UR\" \"UF\" \"UC\" \"UO\" \"NR\" \"DI\" \"FI\" \"RL\" \"UAEX\" \"RLSE\" " - "\"DC\" \"URX\" \"SI\" \"IH\" \"DPE\"]]): name: \"accesslog\"\nfilter {\n " + "\"DC\" \"URX\" \"SI\" \"IH\" \"DPE\" \"UMSDR\"]]): name: \"accesslog\"\nfilter {\n " "response_flag_filter {\n flags: \"UnsupportedFlag\"\n }\n}\ntyped_config {\n " "[type.googleapis.com/envoy.config.accesslog.v2.FileAccessLog] {\n path: \"/dev/null\"\n " "}\n}\n"); diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index 886004887a2ae..058f7a580d00d 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -125,6 +125,12 @@ class RouterTestBase : public testing::Test { EXPECT_CALL(*per_try_timeout_, disableTimer()); } + void expectMaxStreamDurationTimerCreate() { + max_stream_duration_timer_ = new Event::MockTimer(&callbacks_.dispatcher_); + EXPECT_CALL(*max_stream_duration_timer_, enableTimer(_, _)); + EXPECT_CALL(*max_stream_duration_timer_, disableTimer()); + } + AssertionResult verifyHostUpstreamStats(uint64_t success, uint64_t error) { if (success != cm_.conn_pool_.host_->stats_.rq_success_.value()) { return AssertionFailure() << fmt::format("rq_success {} does not match expected {}", @@ -317,6 +323,13 @@ class RouterTestBase : public testing::Test { .WillByDefault(Return(include)); } + void setUpstreamMaxStreamDuration(uint32_t seconds) { + common_http_protocol_options_.mutable_max_stream_duration()->MergeFrom( + ProtobufUtil::TimeUtil::MillisecondsToDuration(seconds)); + ON_CALL(cm_.conn_pool_.host_->cluster_, commonHttpProtocolOptions()) + .WillByDefault(ReturnRef(common_http_protocol_options_)); + } + void enableHedgeOnPerTryTimeout() { callbacks_.route_->route_entry_.hedge_policy_.hedge_on_per_try_timeout_ = true; callbacks_.route_->route_entry_.hedge_policy_.additional_request_chance_ = @@ -334,6 +347,7 @@ class RouterTestBase : public testing::Test { Event::SimulatedTimeSystem test_time_; std::string upstream_zone_{"to_az"}; envoy::config::core::v3::Locality upstream_locality_; + envoy::config::core::v3::HttpProtocolOptions common_http_protocol_options_; NiceMock stats_store_; NiceMock cm_; NiceMock runtime_; @@ -347,6 +361,7 @@ class RouterTestBase : public testing::Test { RouterTestFilter router_; Event::MockTimer* response_timeout_{}; Event::MockTimer* per_try_timeout_{}; + Event::MockTimer* max_stream_duration_timer_{}; Network::Address::InstanceConstSharedPtr host_address_{ Network::Utility::resolveUrl("tcp://10.0.0.5:9211")}; NiceMock original_encoder_; @@ -3888,6 +3903,145 @@ TEST_F(RouterTest, RetryTimeoutDuringRetryDelay) { EXPECT_TRUE(verifyHostUpstreamStats(0, 1)); } +TEST_F(RouterTest, MaxStreamDurationValidlyConfiguredWithoutRetryPolicy) { + NiceMock encoder1; + Http::ResponseDecoder* response_decoder = nullptr; + setUpstreamMaxStreamDuration(500); + EXPECT_CALL(cm_.conn_pool_, newStream(_, _)) + .WillOnce(Invoke( + [&](Http::ResponseDecoder& decoder, + Http::ConnectionPool::Callbacks& callbacks) -> Http::ConnectionPool::Cancellable* { + response_decoder = &decoder; + callbacks.onPoolReady(encoder1, cm_.conn_pool_.host_, upstream_stream_info_); + return nullptr; + })); + expectMaxStreamDurationTimerCreate(); + + Http::TestRequestHeaderMapImpl headers; + HttpTestUtility::addDefaultHeaders(headers); + router_.decodeHeaders(headers, false); + max_stream_duration_timer_->invokeCallback(); + + router_.onDestroy(); + EXPECT_TRUE(verifyHostUpstreamStats(0, 0)); +} + +TEST_F(RouterTest, MaxStreamDurationDisabledIfSetToZero) { + NiceMock encoder1; + Http::ResponseDecoder* response_decoder = nullptr; + setUpstreamMaxStreamDuration(0); + EXPECT_CALL(cm_.conn_pool_, newStream(_, _)) + .WillOnce(Invoke( + [&](Http::ResponseDecoder& decoder, + Http::ConnectionPool::Callbacks& callbacks) -> Http::ConnectionPool::Cancellable* { + response_decoder = &decoder; + callbacks.onPoolReady(encoder1, cm_.conn_pool_.host_, upstream_stream_info_); + return nullptr; + })); + + // not to be called timer creation. + EXPECT_CALL(callbacks_.dispatcher_, createTimer_).Times(0); + + Http::TestRequestHeaderMapImpl headers; + HttpTestUtility::addDefaultHeaders(headers); + router_.decodeHeaders(headers, false); + + router_.onDestroy(); + EXPECT_TRUE(verifyHostUpstreamStats(0, 0)); +} + +TEST_F(RouterTest, MaxStreamDurationCallbackNotCalled) { + NiceMock encoder1; + Http::ResponseDecoder* response_decoder = nullptr; + setUpstreamMaxStreamDuration(5000); + EXPECT_CALL(cm_.conn_pool_, newStream(_, _)) + .WillOnce(Invoke( + [&](Http::ResponseDecoder& decoder, + Http::ConnectionPool::Callbacks& callbacks) -> Http::ConnectionPool::Cancellable* { + response_decoder = &decoder; + callbacks.onPoolReady(encoder1, cm_.conn_pool_.host_, upstream_stream_info_); + return nullptr; + })); + expectMaxStreamDurationTimerCreate(); + + Http::TestRequestHeaderMapImpl headers; + HttpTestUtility::addDefaultHeaders(headers); + router_.decodeHeaders(headers, false); + + router_.onDestroy(); + EXPECT_TRUE(verifyHostUpstreamStats(0, 0)); +} + +TEST_F(RouterTest, MaxStreamDurationWhenDownstreamAlreadyStartedWithoutRetryPolicy) { + NiceMock encoder1; + Http::ResponseDecoder* response_decoder = nullptr; + setUpstreamMaxStreamDuration(500); + EXPECT_CALL(cm_.conn_pool_, newStream(_, _)) + .WillOnce(Invoke( + [&](Http::ResponseDecoder& decoder, + Http::ConnectionPool::Callbacks& callbacks) -> Http::ConnectionPool::Cancellable* { + response_decoder = &decoder; + callbacks.onPoolReady(encoder1, cm_.conn_pool_.host_, upstream_stream_info_); + return nullptr; + })); + expectMaxStreamDurationTimerCreate(); + + Http::TestRequestHeaderMapImpl headers; + HttpTestUtility::addDefaultHeaders(headers); + router_.decodeHeaders(headers, false); + Http::ResponseHeaderMapPtr response_headers( + new Http::TestResponseHeaderMapImpl{{":status", "200"}}); + response_decoder->decodeHeaders(std::move(response_headers), false); + max_stream_duration_timer_->invokeCallback(); + + router_.onDestroy(); + EXPECT_TRUE(verifyHostUpstreamStats(1, 0)); +} + +TEST_F(RouterTest, MaxStreamDurationWithRetryPolicy) { + // First upstream request + NiceMock encoder1; + Http::ResponseDecoder* response_decoder = nullptr; + setUpstreamMaxStreamDuration(500); + EXPECT_CALL(cm_.conn_pool_, newStream(_, _)) + .WillOnce(Invoke( + [&](Http::ResponseDecoder& decoder, + Http::ConnectionPool::Callbacks& callbacks) -> Http::ConnectionPool::Cancellable* { + response_decoder = &decoder; + callbacks.onPoolReady(encoder1, cm_.conn_pool_.host_, upstream_stream_info_); + return nullptr; + })); + expectMaxStreamDurationTimerCreate(); + + Http::TestRequestHeaderMapImpl headers{{"x-envoy-retry-on", "reset"}, + {"x-envoy-internal", "true"}}; + HttpTestUtility::addDefaultHeaders(headers); + router_.decodeHeaders(headers, false); + + router_.retry_state_->expectResetRetry(); + max_stream_duration_timer_->invokeCallback(); + + // Second upstream request + NiceMock encoder2; + setUpstreamMaxStreamDuration(500); + EXPECT_CALL(cm_.conn_pool_, newStream(_, _)) + .WillOnce(Invoke( + [&](Http::ResponseDecoder& decoder, + Http::ConnectionPool::Callbacks& callbacks) -> Http::ConnectionPool::Cancellable* { + response_decoder = &decoder; + callbacks.onPoolReady(encoder2, cm_.conn_pool_.host_, upstream_stream_info_); + return nullptr; + })); + expectMaxStreamDurationTimerCreate(); + router_.retry_state_->callback_(); + + EXPECT_CALL(*router_.retry_state_, shouldRetryHeaders(_, _)).WillOnce(Return(RetryStatus::No)); + Http::ResponseHeaderMapPtr response_headers( + new Http::TestResponseHeaderMapImpl{{":status", "200"}}); + response_decoder->decodeHeaders(std::move(response_headers), true); + EXPECT_TRUE(verifyHostUpstreamStats(1, 1)); +} + TEST_F(RouterTest, RetryTimeoutDuringRetryDelayWithUpstreamRequestNoHost) { NiceMock encoder1; Http::ResponseDecoder* response_decoder = nullptr; diff --git a/test/common/router/upstream_request_test.cc b/test/common/router/upstream_request_test.cc index 91c2a00cee76d..e228e6057ded3 100644 --- a/test/common/router/upstream_request_test.cc +++ b/test/common/router/upstream_request_test.cc @@ -77,6 +77,7 @@ class MockRouterFilterInterface : public RouterFilterInterface { UpstreamRequest& upstream_request)); MOCK_METHOD(void, onUpstreamHostSelected, (Upstream::HostDescriptionConstSharedPtr host)); MOCK_METHOD(void, onPerTryTimeout, (UpstreamRequest & upstream_request)); + MOCK_METHOD(void, onStreamMaxDurationReached, (UpstreamRequest & upstream_request)); MOCK_METHOD(Http::StreamDecoderFilterCallbacks*, callbacks, ()); MOCK_METHOD(Upstream::ClusterInfoConstSharedPtr, cluster, ()); diff --git a/test/common/stream_info/utility_test.cc b/test/common/stream_info/utility_test.cc index b3a02d18f1170..5b1b73760375f 100644 --- a/test/common/stream_info/utility_test.cc +++ b/test/common/stream_info/utility_test.cc @@ -15,7 +15,7 @@ namespace StreamInfo { namespace { TEST(ResponseFlagUtilsTest, toShortStringConversion) { - static_assert(ResponseFlag::LastFlag == 0x40000, "A flag has been added. Fix this code."); + static_assert(ResponseFlag::LastFlag == 0x80000, "A flag has been added. Fix this code."); std::vector> expected = { std::make_pair(ResponseFlag::FailedLocalHealthCheck, "LH"), @@ -37,7 +37,7 @@ TEST(ResponseFlagUtilsTest, toShortStringConversion) { std::make_pair(ResponseFlag::StreamIdleTimeout, "SI"), std::make_pair(ResponseFlag::InvalidEnvoyRequestHeaders, "IH"), std::make_pair(ResponseFlag::DownstreamProtocolError, "DPE"), - }; + std::make_pair(ResponseFlag::UpstreamMaxStreamDurationReached, "UMSDR")}; for (const auto& test_case : expected) { NiceMock stream_info; @@ -65,7 +65,7 @@ TEST(ResponseFlagUtilsTest, toShortStringConversion) { } TEST(ResponseFlagsUtilsTest, toResponseFlagConversion) { - static_assert(ResponseFlag::LastFlag == 0x40000, "A flag has been added. Fix this code."); + static_assert(ResponseFlag::LastFlag == 0x80000, "A flag has been added. Fix this code."); std::vector> expected = { std::make_pair("LH", ResponseFlag::FailedLocalHealthCheck), @@ -87,7 +87,7 @@ TEST(ResponseFlagsUtilsTest, toResponseFlagConversion) { std::make_pair("SI", ResponseFlag::StreamIdleTimeout), std::make_pair("IH", ResponseFlag::InvalidEnvoyRequestHeaders), std::make_pair("DPE", ResponseFlag::DownstreamProtocolError), - }; + std::make_pair("UMSDR", ResponseFlag::UpstreamMaxStreamDurationReached)}; EXPECT_FALSE(ResponseFlagUtils::toResponseFlag("NonExistentFlag").has_value()); diff --git a/test/extensions/access_loggers/grpc/grpc_access_log_utils_test.cc b/test/extensions/access_loggers/grpc/grpc_access_log_utils_test.cc index 90d18811c43ac..1519369ffbdbc 100644 --- a/test/extensions/access_loggers/grpc/grpc_access_log_utils_test.cc +++ b/test/extensions/access_loggers/grpc/grpc_access_log_utils_test.cc @@ -40,6 +40,8 @@ TEST(UtilityResponseFlagsToAccessLogResponseFlagsTest, All) { common_access_log_expected.mutable_response_flags()->set_stream_idle_timeout(true); common_access_log_expected.mutable_response_flags()->set_invalid_envoy_request_headers(true); common_access_log_expected.mutable_response_flags()->set_downstream_protocol_error(true); + common_access_log_expected.mutable_response_flags()->set_upstream_max_stream_duration_reached( + true); EXPECT_EQ(common_access_log_expected.DebugString(), common_access_log.DebugString()); } diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index a736718e5df58..8dc0dedf59bb8 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -1213,6 +1213,93 @@ void HttpIntegrationTest::testAdminDrain(Http::CodecClient::Type admin_request_t } } +void HttpIntegrationTest::testMaxStreamDuration() { + config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { + auto* static_resources = bootstrap.mutable_static_resources(); + auto* cluster = static_resources->mutable_clusters(0); + auto* http_protocol_options = cluster->mutable_common_http_protocol_options(); + http_protocol_options->mutable_max_stream_duration()->MergeFrom( + ProtobufUtil::TimeUtil::MillisecondsToDuration(200)); + }); + + initialize(); + fake_upstreams_[0]->set_allow_unexpected_disconnects(true); + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto encoder_decoder = codec_client_->startRequest(default_request_headers_); + request_encoder_ = &encoder_decoder.first; + auto response = std::move(encoder_decoder.second); + + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); + + test_server_->waitForCounterGe("cluster.cluster_0.upstream_rq_max_duration_reached", 1); + + if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) { + codec_client_->waitForDisconnect(); + } else { + response->waitForReset(); + codec_client_->close(); + } +} + +void HttpIntegrationTest::testMaxStreamDurationWithRetry(bool invoke_retry_upstream_disconnect) { + config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { + auto* static_resources = bootstrap.mutable_static_resources(); + auto* cluster = static_resources->mutable_clusters(0); + auto* http_protocol_options = cluster->mutable_common_http_protocol_options(); + http_protocol_options->mutable_max_stream_duration()->MergeFrom( + ProtobufUtil::TimeUtil::MillisecondsToDuration(1000)); + }); + + Http::TestRequestHeaderMapImpl retriable_header = Http::TestRequestHeaderMapImpl{ + {":method", "POST"}, {":path", "/test/long/url"}, {":scheme", "http"}, + {":authority", "host"}, {"x-forwarded-for", "10.0.0.1"}, {"x-envoy-retry-on", "5xx"}}; + initialize(); + fake_upstreams_[0]->set_allow_unexpected_disconnects(true); + codec_client_ = makeHttpConnection(lookupPort("http")); + + auto encoder_decoder = codec_client_->startRequest(retriable_header); + request_encoder_ = &encoder_decoder.first; + auto response = std::move(encoder_decoder.second); + + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); + ASSERT_TRUE(upstream_request_->waitForHeadersComplete()); + + if (fake_upstreams_[0]->httpType() == FakeHttpConnection::Type::HTTP1) { + ASSERT_TRUE(fake_upstream_connection_->waitForDisconnect()); + ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); + } else { + ASSERT_TRUE(upstream_request_->waitForReset()); + } + + test_server_->waitForCounterGe("cluster.cluster_0.upstream_rq_max_duration_reached", 1); + + ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); + + if (invoke_retry_upstream_disconnect) { + test_server_->waitForCounterGe("cluster.cluster_0.upstream_rq_max_duration_reached", 2); + if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) { + codec_client_->waitForDisconnect(); + } else { + response->waitForReset(); + codec_client_->close(); + } + + EXPECT_EQ("408", response->headers().Status()->value().getStringView()); + } else { + Http::TestHeaderMapImpl response_headers{{":status", "200"}}; + upstream_request_->encodeHeaders(response_headers, true); + + response->waitForHeaders(); + codec_client_->close(); + + EXPECT_TRUE(response->complete()); + EXPECT_EQ("200", response->headers().Status()->value().getStringView()); + } +} + 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 359fe1bc38d3a..85448db6d672d 100644 --- a/test/integration/http_integration.h +++ b/test/integration/http_integration.h @@ -220,7 +220,9 @@ class HttpIntegrationTest : public BaseIntegrationTest { bool response_trailers_present); // Test /drain_listener from admin portal. void testAdminDrain(Http::CodecClient::Type admin_request_type); - + // Test max stream duration. + void testMaxStreamDuration(); + void testMaxStreamDurationWithRetry(bool invoke_retry_upstream_disconnect); Http::CodecClient::Type downstreamProtocol() const { return downstream_protocol_; } // 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/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index 6b9590b70e6b5..b65c3a53312b5 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -782,6 +782,16 @@ TEST_P(ProtocolIntegrationTest, TwoRequests) { testTwoRequests(); } TEST_P(ProtocolIntegrationTest, TwoRequestsWithForcedBackup) { testTwoRequests(true); } +TEST_P(ProtocolIntegrationTest, BasicMaxStreamDuration) { testMaxStreamDuration(); } + +TEST_P(ProtocolIntegrationTest, MaxStreamDurationWithRetryPolicy) { + testMaxStreamDurationWithRetry(false); +} + +TEST_P(ProtocolIntegrationTest, MaxStreamDurationWithRetryPolicyWhenRetryUpstreamDisconnection) { + testMaxStreamDurationWithRetry(true); +} + // Verify that headers with underscores in their names are dropped from client requests // but remain in upstream responses. TEST_P(ProtocolIntegrationTest, HeadersWithUnderscoresDropped) { diff --git a/test/integration/stats_integration_test.cc b/test/integration/stats_integration_test.cc index 75012bd02116a..ea7fa20fd23b3 100644 --- a/test/integration/stats_integration_test.cc +++ b/test/integration/stats_integration_test.cc @@ -271,6 +271,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithFakeSymbolTable) { // 2020/03/24 10501 44261 44600 upstream: upstream_rq_retry_limit_exceeded. // 2020/04/02 10624 43356 44000 Use 100 clusters rather than 1000 to avoid timeouts // 2020/04/07 10661 43349 44000 fix clang tidy on master + // 2020/04/23 10531 44169 44600 http: max stream duration upstream support. // Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI // 'release' builds, where we control the platform and tool-chain. So you @@ -284,8 +285,8 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithFakeSymbolTable) { // If you encounter a failure here, please see // https://github.com/envoyproxy/envoy/blob/master/source/docs/stats.md#stats-memory-tests // for details on how to fix. - EXPECT_MEMORY_EQ(m_per_cluster, 43993); - EXPECT_MEMORY_LE(m_per_cluster, 44100); + EXPECT_MEMORY_EQ(m_per_cluster, 44169); + EXPECT_MEMORY_LE(m_per_cluster, 44600); } TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) { @@ -329,6 +330,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) { // 2020/03/24 10501 36300 36800 upstream: upstream_rq_retry_limit_exceeded. // 2020/04/02 10624 35564 36000 Use 100 clusters rather than 1000 to avoid timeouts // 2020/04/07 10661 35557 36000 fix clang tidy on master + // 2020/04/23 10531 36281 36800 http: max stream duration upstream support. // Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI // 'release' builds, where we control the platform and tool-chain. So you @@ -342,8 +344,8 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) { // If you encounter a failure here, please see // https://github.com/envoyproxy/envoy/blob/master/source/docs/stats.md#stats-memory-tests // for details on how to fix. - EXPECT_MEMORY_EQ(m_per_cluster, 36201); - EXPECT_MEMORY_LE(m_per_cluster, 36300); + EXPECT_MEMORY_EQ(m_per_cluster, 36281); + EXPECT_MEMORY_LE(m_per_cluster, 36800); } TEST_P(ClusterMemoryTestRunner, MemoryLargeHostSizeWithStats) { diff --git a/test/integration/tcp_tunneling_integration_test.cc b/test/integration/tcp_tunneling_integration_test.cc index 3cdaa3074770d..497fc5ee5b463 100644 --- a/test/integration/tcp_tunneling_integration_test.cc +++ b/test/integration/tcp_tunneling_integration_test.cc @@ -171,6 +171,30 @@ TEST_P(ConnectTerminationIntegrationTest, BuggyHeaders) { ASSERT_FALSE(response_->reset()); } +TEST_P(ConnectTerminationIntegrationTest, BasicMaxStreamDuration) { + config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { + auto* static_resources = bootstrap.mutable_static_resources(); + auto* cluster = static_resources->mutable_clusters(0); + auto* http_protocol_options = cluster->mutable_common_http_protocol_options(); + http_protocol_options->mutable_max_stream_duration()->MergeFrom( + ProtobufUtil::TimeUtil::MillisecondsToDuration(1000)); + }); + + initialize(); + fake_upstreams_[0]->set_allow_unexpected_disconnects(true); + setUpConnection(); + sendBidirectionalData(); + + test_server_->waitForCounterGe("cluster.cluster_0.upstream_rq_max_duration_reached", 1); + + if (downstream_protocol_ == Http::CodecClient::Type::HTTP1) { + codec_client_->waitForDisconnect(); + } else { + response_->waitForReset(); + codec_client_->close(); + } +} + // For this class, forward the CONNECT request upstream class ProxyingConnectIntegrationTest : public HttpProtocolIntegrationTest { public: diff --git a/test/mocks/upstream/cluster_info.cc b/test/mocks/upstream/cluster_info.cc index 55b0fffbb2b86..215368b58c473 100644 --- a/test/mocks/upstream/cluster_info.cc +++ b/test/mocks/upstream/cluster_info.cc @@ -55,6 +55,8 @@ MockClusterInfo::MockClusterInfo() ON_CALL(*this, eds_service_name()).WillByDefault(ReturnPointee(&eds_service_name_)); ON_CALL(*this, http1Settings()).WillByDefault(ReturnRef(http1_settings_)); ON_CALL(*this, http2Options()).WillByDefault(ReturnRef(http2_options_)); + ON_CALL(*this, commonHttpProtocolOptions()) + .WillByDefault(ReturnRef(common_http_protocol_options_)); ON_CALL(*this, extensionProtocolOptions(_)).WillByDefault(Return(extension_protocol_options_)); ON_CALL(*this, maxResponseHeadersCount()) .WillByDefault(ReturnPointee(&max_response_headers_count_)); diff --git a/test/mocks/upstream/cluster_info.h b/test/mocks/upstream/cluster_info.h index 6e2e8c3b113f5..9ef5fcc14618c 100644 --- a/test/mocks/upstream/cluster_info.h +++ b/test/mocks/upstream/cluster_info.h @@ -90,6 +90,8 @@ class MockClusterInfo : public ClusterInfo { MOCK_METHOD(uint64_t, features, (), (const)); MOCK_METHOD(const Http::Http1Settings&, http1Settings, (), (const)); MOCK_METHOD(const envoy::config::core::v3::Http2ProtocolOptions&, http2Options, (), (const)); + MOCK_METHOD(const envoy::config::core::v3::HttpProtocolOptions&, commonHttpProtocolOptions, (), + (const)); MOCK_METHOD(ProtocolOptionsConfigConstSharedPtr, extensionProtocolOptions, (const std::string&), (const)); MOCK_METHOD(const envoy::config::cluster::v3::Cluster::CommonLbConfig&, lbConfig, (), (const)); @@ -131,6 +133,7 @@ class MockClusterInfo : public ClusterInfo { absl::optional eds_service_name_; Http::Http1Settings http1_settings_; envoy::config::core::v3::Http2ProtocolOptions http2_options_; + envoy::config::core::v3::HttpProtocolOptions common_http_protocol_options_; ProtocolOptionsConfigConstSharedPtr extension_protocol_options_; uint64_t max_requests_per_connection_{}; uint32_t max_response_headers_count_{Http::DEFAULT_MAX_HEADERS_COUNT}; @@ -158,6 +161,7 @@ class MockClusterInfo : public ClusterInfo { envoy::config::cluster::v3::Cluster::CommonLbConfig lb_config_; envoy::config::core::v3::Metadata metadata_; std::unique_ptr typed_metadata_; + absl::optional max_stream_duration_; }; class MockIdleTimeEnabledClusterInfo : public MockClusterInfo {