feat: PreserveExternalRequestID on ClientTrafficPolicy

[ardikabs]

What type of PR is this?
Adds configuration to ClientTrafficPolicy for Headers to support preserving External Request ID (x-request-id).

What this PR does / why we need it:
When it is enabled on Envoy, it will improve the observability experience. This allows users to set their request ID from front-facing applications without being overwritten by Envoy, which is the default behavior.

More reference about the field and its behavior: https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto, keyword preserve_external_request_id.

[zirain]

When it is enabled on Envoy, it will improve the observability experience.

what will happen it will enable this by default?

[ardikabs]

what will happen it will enable this by default?

@zirain
I have no idea yet the implication if we enable it by default. One thing for sure, user able to preserve the x-request-id from app or during load balancer stacking.

Reference from Envoy, envoyproxy/envoy#7140.

[zirain]

Whether the connection manager will keep the x-request-id header if passed for a request that is edge (Edge request is the request from external clients to front Envoy) and not reset it, which is the current Envoy behaviour.

so in what user case, we should set it to true?

[ardikabs]

@zirain when users intend to manage their own x-request-id from the front (either apps or edge LB). Envoy's current behavior of always overwriting it can be problematic, especially when users intend to utilize the x-request-id header for correlation tracking purposes.

Please also check similar intentions on Istio, about preserving external request id on the Istio gateway, issue istio/istio#10875.

[zirain]

I assume current behavior worked as excepted? What I missed?

curl -v 172.18.255.201  -H "x-request-id: 7fb3693f-4061-4070-95d4-3889aa8a22a5"
*   Trying 172.18.255.201:80...
* Connected to 172.18.255.201 (172.18.255.201) port 80
> GET / HTTP/1.1
> Host: 172.18.255.201
> User-Agent: curl/8.4.0
> Accept: */*
> x-request-id: 7fb3693f-4061-4070-95d4-3889aa8a22a5
>
< HTTP/1.1 200 OK
< content-type: application/json
< x-content-type-options: nosniff
< date: Sun, 21 Apr 2024 03:15:37 GMT
< content-length: 448
<
{
 "path": "/",
 "host": "172.18.255.201",
 "method": "GET",
 "proto": "HTTP/1.1",
 "headers": {
  "Accept": [
   "*/*"
  ],
  "User-Agent": [
   "curl/8.4.0"
  ],
  "X-Envoy-Internal": [
   "true"
  ],
  "X-Forwarded-For": [
   "172.18.0.1"
  ],
  "X-Forwarded-Proto": [
   "http"
  ],
  "X-Request-Id": [
   "7fb3693f-4061-4070-95d4-3889aa8a22a5"
  ]
 },
 "namespace": "default",
 "ingress": "",
 "service": "",
 "pod": "backend-96f75bbf-rzb79"
* Connection #0 to host 172.18.255.201 left intact
}

[ardikabs]

@zirain i assume you were accessing directly to the envoy proxy, but that's not exactly how it looks, suppose you add an edge LB and then an arbitrary number of proxies in front of your envoy proxy, it might look like the diagram below:

User -> Edge LB (cloudflare) --> (an arbitrary number of proxies before envoy) -> Envoy Proxy

Envoy proxy will overwrite x-request-id if any, check this also envoyproxy/envoy#6050.

[zirain]

@zirain i assume you were accessing directly to the envoy proxy, but that's not exactly how it looks, suppose you add an edge LB and then an arbitrary number of proxies in front of your envoy proxy, it might look like the diagram below:

User -> Edge LB (cloudflare) --> (an arbitrary number of proxies before envoy) -> Envoy Proxy

Envoy proxy will overwrite x-request-id if any, check this also envoyproxy/envoy#6050.

Oh, I got your point.

@zirain i assume you were accessing directly to the envoy proxy, but that's not exactly how it looks, suppose you add an edge LB and then an arbitrary number of proxies in front of your envoy proxy, it might look like the diagram below:

User -> Edge LB (cloudflare) --> (an arbitrary number of proxies before envoy) -> Envoy Proxy

Envoy proxy will overwrite x-request-id if any, check this also envoyproxy/envoy#6050.

Oh, I got your point, seems XffNumTrustedHops didn't help.

[zirain]

I'm thinking about should EG make it default true?

[ardikabs]

I'm thinking about should EG make it default true?

I voted yes, although I have no idea yet about the implication if it is enabled by default.

[zirain]

cc @envoyproxy/gateway-maintainers PTAL

[guydc]

I'm thinking about should EG make it default true?

I'm not familiar with an accepted standard for this header. However, some projects make the following distinctions:

• X-Request-Id: unique identifier for a each request/response. In this interpretation, sanitizing the header can be seen as the correct behavior.
• X-Correlation-Id: unique identifier that is stable throughout the entire transactions (chain of requests and responses).

Currently, X-Request-Id can always be used for correlating envoy and backend logs for a specific request/response sequence. If this header is preserved by default and clients generate bad x-request-id (e.g., always have the same value for all requests), this header will not be as useful for troubleshooting envoy <> backend issues.

So, I think that preservation should be an opt-in option for applications that trust their clients to generate meaningful and well-formed x-request-id values that are reliable for troubleshooting issues that occur on envoy <> backend side of things.

[ardikabs]

/retest

[arkodg]

thoughts on PreserveXRequestID ?

the comments highlight the fact that this is opt in and needed when client is external / envoy is edge
cc @envoyproxy/gateway-maintainers

[ardikabs]

actually, my initial plan was to use that naming, but I'm trying to mimic Envoy's naming conventions. i have changed just now to PreserveXRequestID

[arkodg]

LGTM thanks !

[arkodg]

other reviewers, please weigh in on PreserveExternalRequestID vs PreserveXRequestID , is it valuable to keep the envoy naming to reduce confusion ?

[zhaohuabing]

Prefer PreserveXRequestID as it's clearer. "External" seems somewhat vague to me.

[zhaohuabing]

LGTM thanks!

[zirain]

+1 on PreserveXRequestID

[Alice-Lilith]

👍