request_id: Add option to always set request id in response

api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto

@@ -30,7 +30,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // HTTP connection manager :ref:`configuration overview <config_http_conn_man>`.
 // [#extension: envoy.filters.network.http_connection_manager]
 
-// [#next-free-field: 37]
+// [#next-free-field: 38]
 message HttpConnectionManager {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager";
@@ -433,6 +433,11 @@ message HttpConnectionManager {
   // is the current Envoy behaviour. This defaults to false.
   bool preserve_external_request_id = 32;
 
+  // If set, Envoy will always set :ref:`x-request-id <config_http_conn_man_headers_x-request-id>` header in response.
+  // If this is false or not set, the request ID is returned in responses only if tracing is forced using
+  // :ref:`x-envoy-force-trace <config_http_conn_man_headers_x-envoy-force-trace>` header.
+  bool always_set_request_id_in_response = 37;
+
   // How to handle the :ref:`config_http_conn_man_headers_x-forwarded-client-cert` (XFCC) HTTP
   // header.
   ForwardClientCertDetails forward_client_cert_details = 16

api/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto

@@ -30,7 +30,7 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO
 // HTTP connection manager :ref:`configuration overview <config_http_conn_man>`.
 // [#extension: envoy.filters.network.http_connection_manager]
 
-// [#next-free-field: 37]
+// [#next-free-field: 38]
 message HttpConnectionManager {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager";
@@ -433,6 +433,11 @@ message HttpConnectionManager {
   // is the current Envoy behaviour. This defaults to false.
   bool preserve_external_request_id = 32;
 
+  // If set, Envoy will always set :ref:`x-request-id <config_http_conn_man_headers_x-request-id>` header in response.
+  // If this is false or not set, the request ID is returned in responses only if tracing is forced using
+  // :ref:`x-envoy-force-trace <config_http_conn_man_headers_x-envoy-force-trace>` header.
+  bool always_set_request_id_in_response = 37;
+
   // How to handle the :ref:`config_http_conn_man_headers_x-forwarded-client-cert` (XFCC) HTTP
   // header.
   ForwardClientCertDetails forward_client_cert_details = 16

docs/root/version_history/current.rst

@@ -9,6 +9,9 @@ Changes
   are applied to using :ref:`HTTP headers <config_http_filters_fault_injection_http_header>` to the HTTP fault filter.
 * http: fixed a bug where the upgrade header was not cleared on responses to non-upgrade requests.
   Can be reverted temporarily by setting runtime feature `envoy.reloadable_features.fix_upgrade_response` to false.
+* request_id: added to :ref:`always_set_request_id_in_response setting <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.always_set_request_id_in_response>`
+  to set :ref:`x-request-id <config_http_conn_man_headers_x-request-id>` header in response even if
+  tracing is not forced.
 * tracing: tracing configuration has been made fully dynamic and every HTTP connection manager
   can now have a separate :ref:`tracing provider <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.Tracing.provider>`.
 

source/common/http/conn_manager_config.h

@@ -245,6 +245,11 @@ class ConnectionManagerConfig {
    */
   virtual bool preserveExternalRequestId() const PURE;
 
+  /**
+   * @return whether the x-request-id should always be set in the response.
+   */
+  virtual bool alwaysSetRequestIdInResponse() const PURE;
+
   /**
    * @return optional idle timeout for incoming connection manager connections.
    */

source/common/http/conn_manager_impl.cc

@@ -1528,8 +1528,7 @@ void ConnectionManagerImpl::ActiveStream::encode100ContinueHeaders(
   // Strip the T-E headers etc. Defer other header additions as well as drain-close logic to the
   // continuation headers.
   ConnectionManagerUtility::mutateResponseHeaders(headers, request_headers_.get(),
-                                                  connection_manager_.config_.requestIDExtension(),
-                                                  EMPTY_STRING);
+                                                  connection_manager_.config_, EMPTY_STRING);
 
   // Count both the 1xx and follow-up response code in stats.
   chargeStats(headers);
@@ -1612,7 +1611,7 @@ void ConnectionManagerImpl::ActiveStream::encodeHeadersInternal(ResponseHeaderMa
     headers.setReferenceServer(connection_manager_.config_.serverName());
   }
   ConnectionManagerUtility::mutateResponseHeaders(headers, request_headers_.get(),
-                                                  connection_manager_.config_.requestIDExtension(),
+                                                  connection_manager_.config_,
                                                   connection_manager_.config_.via());
 
   // See if we want to drain/close the connection. Send the go away frame prior to encoding the

source/common/http/conn_manager_utility.cc

@@ -363,9 +363,10 @@ void ConnectionManagerUtility::mutateXfccRequestHeader(RequestHeaderMap& request
   }
 }
 
-void ConnectionManagerUtility::mutateResponseHeaders(
-    ResponseHeaderMap& response_headers, const RequestHeaderMap* request_headers,
-    const RequestIDExtensionSharedPtr& rid_extension, const std::string& via) {
+void ConnectionManagerUtility::mutateResponseHeaders(ResponseHeaderMap& response_headers,
+                                                     const RequestHeaderMap* request_headers,
+                                                     ConnectionManagerConfig& config,
+                                                     const std::string& via) {
   if (request_headers != nullptr && Utility::isUpgrade(*request_headers) &&
       Utility::isUpgrade(response_headers)) {
     // As in mutateRequestHeaders, Upgrade responses have special handling.
@@ -386,8 +387,9 @@ void ConnectionManagerUtility::mutateResponseHeaders(
 
   response_headers.removeTransferEncoding();
 
-  if (request_headers != nullptr && request_headers->EnvoyForceTrace()) {
-    rid_extension->setInResponse(response_headers, *request_headers);
+  if (request_headers != nullptr &&
+      (config.alwaysSetRequestIdInResponse() || request_headers->EnvoyForceTrace())) {
+    config.requestIDExtension()->setInResponse(response_headers, *request_headers);
   }
   response_headers.removeKeepAlive();
   response_headers.removeProxyConnection();

source/common/http/conn_manager_utility.h

@@ -60,8 +60,7 @@ class ConnectionManagerUtility {
 
   static void mutateResponseHeaders(ResponseHeaderMap& response_headers,
                                     const RequestHeaderMap* request_headers,
-                                    const RequestIDExtensionSharedPtr& rid_extension,
-                                    const std::string& via);
+                                    ConnectionManagerConfig& config, const std::string& via);
 
   // Sanitize the path in the header map if forced by config.
   // Side affect: the string view of Path header is invalidated.

source/extensions/filters/network/http_connection_manager/config.cc

@@ -196,6 +196,7 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(
       drain_timeout_(PROTOBUF_GET_MS_OR_DEFAULT(config, drain_timeout, 5000)),
       generate_request_id_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, generate_request_id, true)),
       preserve_external_request_id_(config.preserve_external_request_id()),
+      always_set_request_id_in_response_(config.always_set_request_id_in_response()),
       date_provider_(date_provider),
       listener_stats_(Http::ConnectionManagerImpl::generateListenerStats(stats_prefix_,
                                                                          context_.listenerScope())),

source/extensions/filters/network/http_connection_manager/config.h

@@ -109,6 +109,7 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
   FilterChainFactory& filterFactory() override { return *this; }
   bool generateRequestId() const override { return generate_request_id_; }
   bool preserveExternalRequestId() const override { return preserve_external_request_id_; }
+  bool alwaysSetRequestIdInResponse() const override { return always_set_request_id_in_response_; }
   uint32_t maxRequestHeadersKb() const override { return max_request_headers_kb_; }
   uint32_t maxRequestHeadersCount() const override { return max_request_headers_count_; }
   absl::optional<std::chrono::milliseconds> idleTimeout() const override { return idle_timeout_; }
@@ -216,6 +217,7 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
   std::chrono::milliseconds drain_timeout_;
   bool generate_request_id_;
   const bool preserve_external_request_id_;
+  const bool always_set_request_id_in_response_;
   Http::DateProvider& date_provider_;
   Http::ConnectionManagerListenerStats listener_stats_;
   const bool proxy_100_continue_;

source/server/http/admin.h

@@ -116,6 +116,7 @@ class AdminImpl : public Admin,
   Http::FilterChainFactory& filterFactory() override { return *this; }
   bool generateRequestId() const override { return false; }
   bool preserveExternalRequestId() const override { return false; }
+  bool alwaysSetRequestIdInResponse() const override { return false; }
   absl::optional<std::chrono::milliseconds> idleTimeout() const override { return idle_timeout_; }
   bool isRoutable() const override { return false; }
   absl::optional<std::chrono::milliseconds> maxConnectionDuration() const override {

test/common/http/conn_manager_impl_fuzz_test.cc

@@ -103,6 +103,7 @@ class FuzzConfig : public ConnectionManagerConfig {
   FilterChainFactory& filterFactory() override { return filter_factory_; }
   bool generateRequestId() const override { return true; }
   bool preserveExternalRequestId() const override { return false; }
+  bool alwaysSetRequestIdInResponse() const override { return false; }
   uint32_t maxRequestHeadersKb() const override { return max_request_headers_kb_; }
   uint32_t maxRequestHeadersCount() const override { return max_request_headers_count_; }
   absl::optional<std::chrono::milliseconds> idleTimeout() const override { return idle_timeout_; }

test/common/http/conn_manager_impl_test.cc

@@ -293,6 +293,7 @@ class HttpConnectionManagerImplTest : public testing::Test, public ConnectionMan
   FilterChainFactory& filterFactory() override { return filter_factory_; }
   bool generateRequestId() const override { return true; }
   bool preserveExternalRequestId() const override { return false; }
+  bool alwaysSetRequestIdInResponse() const override { return false; }
   uint32_t maxRequestHeadersKb() const override { return max_request_headers_kb_; }
   uint32_t maxRequestHeadersCount() const override { return max_request_headers_count_; }
   absl::optional<std::chrono::milliseconds> idleTimeout() const override { return idle_timeout_; }