ext_proc: skip timeout timer on trailer in async mode. (#36524)

Commit Message: Like header and body, timer/timeout should be skipped on trailer as well in async. It is because there is no response/ or response is ignored. Risk Level: Low Testing: Integration test Docs Changes: N/A Release Notes: N/A Platform Specific Features: N/A --------- Signed-off-by: tyxia <[email protected]>
envoyproxy · Oct 10, 2024 · 2eebf0d · 2eebf0d
1 parent 601292a
commit 2eebf0d
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 3 deletions.
diff --git a/source/extensions/filters/http/ext_proc/ext_proc.cc b/source/extensions/filters/http/ext_proc/ext_proc.cc
@@ -976,9 +976,16 @@ void Filter::sendTrailers(ProcessorState& state, const Http::HeaderMap& trailers
   auto* trailers_req = state.mutableTrailers(req);
   MutationUtils::headersToProto(trailers, config_->allowedHeaders(), config_->disallowedHeaders(),
                                 *trailers_req->mutable_trailers());
-  state.onStartProcessorCall(std::bind(&Filter::onMessageTimeout, this), config_->messageTimeout(),
-                             ProcessorState::CallbackState::TrailersCallback);
-  ENVOY_LOG(debug, "Sending trailers message");
+
+  if (observability_mode) {
+    ENVOY_LOG(debug, "Sending trailers message in observability mode");
+  } else {
+    state.onStartProcessorCall(std::bind(&Filter::onMessageTimeout, this),
+                               config_->messageTimeout(),
+                               ProcessorState::CallbackState::TrailersCallback);
+    ENVOY_LOG(debug, "Sending trailers message");
+  }
+
   sendRequest(std::move(req), false);
   stats_.stream_msgs_sent_.inc();
 }

diff --git a/test/extensions/filters/http/ext_proc/ext_proc_integration_test.cc b/test/extensions/filters/http/ext_proc/ext_proc_integration_test.cc
@@ -4334,6 +4334,43 @@ TEST_P(ExtProcIntegrationTest, ObservabilityModeWithFullResponse) {
   timeSystem().advanceTimeWaitImpl(std::chrono::milliseconds(deferred_close_timeout_ms));
 }
 
+TEST_P(ExtProcIntegrationTest, ObservabilityModeWithFullRequestAndTimeout) {
+  proto_config_.set_observability_mode(true);
+  uint32_t deferred_close_timeout_ms = 2000;
+  proto_config_.mutable_deferred_close_timeout()->set_seconds(deferred_close_timeout_ms / 1000);
+
+  proto_config_.mutable_processing_mode()->set_request_body_mode(ProcessingMode::STREAMED);
+  proto_config_.mutable_processing_mode()->set_request_trailer_mode(ProcessingMode::SEND);
+  proto_config_.mutable_processing_mode()->set_response_header_mode(ProcessingMode::SKIP);
+
+  initializeConfig();
+  HttpIntegrationTest::initialize();
+  auto response = sendDownstreamRequestWithBodyAndTrailer("Hello");
+
+  processRequestHeadersMessage(*grpc_upstreams_[0], true,
+                               [this](const HttpHeaders&, HeadersResponse&) {
+                                 // Advance 400 ms. Default timeout is 200ms
+                                 timeSystem().advanceTimeWaitImpl(400ms);
+                                 return false;
+                               });
+  processRequestBodyMessage(*grpc_upstreams_[0], false, [this](const HttpBody&, BodyResponse&) {
+    // Advance 400 ms. Default timeout is 200ms
+    timeSystem().advanceTimeWaitImpl(400ms);
+    return false;
+  });
+  processRequestTrailersMessage(*grpc_upstreams_[0], false,
+                                [this](const HttpTrailers&, TrailersResponse&) {
+                                  // Advance 400 ms. Default timeout is 200ms
+                                  timeSystem().advanceTimeWaitImpl(400ms);
+                                  return false;
+                                });
+
+  handleUpstreamRequest();
+  verifyDownstreamResponse(*response, 200);
+
+  timeSystem().advanceTimeWaitImpl(std::chrono::milliseconds(deferred_close_timeout_ms - 1200));
+}
+
 TEST_P(ExtProcIntegrationTest, ObservabilityModeWithLogging) {
   proto_config_.set_observability_mode(true);