Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: ability to use x-request-id in response_headers_to_add #8683

Closed
ascheglov opened this issue Oct 20, 2019 · 1 comment · Fixed by #8782
Closed

Feature request: ability to use x-request-id in response_headers_to_add #8683

ascheglov opened this issue Oct 20, 2019 · 1 comment · Fixed by #8782
Labels
enhancement Feature requests. Not bugs or questions. help wanted Needs help!

Comments

@ascheglov
Copy link
Contributor

Currently there is no way to use request headers in response_headers_to_add (it doesn't support %REQ(header-name)%).

However it would be nice to have an access to certain headers, namely x-request-id.

Normally a backend would echo the x-request-id header, so that it would be proxied to a client.
When there is a request timeout, envoy sends the "504 upstream request timeout" reply, and the client doesn't get the x-request-id.

Although it might be useful to echo other envoy-generated request headers (e.g. uber-trace-id for traces), for now it would be sufficient to add the %REQUEST_ID% in StreamInfoHeaderFormatter, requestId in StreamInfo, and set it in the http connection manager together with setDownstreamRemoteAddress, after the ConnectionManagerUtility::mutateRequestHeaders call.

@lizan lizan added enhancement Feature requests. Not bugs or questions. help wanted Needs help! labels Oct 21, 2019
@ascheglov
Copy link
Contributor Author

And there is another thing, on request timeout, the Filter::onUpstreamAbort function calls sendLocalReply and it doesn't call finalizeResponseHeaders in its headers callback, so response_headers_to_add are not used anyways.

So there are two things to do:

  1. add %REQ(x-request-id)% to response_headers_to_add
  2. add a finalizeResponseHeaders call to onUpstreamAbort

ascheglov pushed a commit to ascheglov/envoy that referenced this issue Oct 28, 2019
Risk Level: low
Testing: unit tests
Doc Changes: yes
Release Notes: yes
Fixes envoyproxy#8683

Signed-off-by: Anatoly Scheglov <[email protected]>
zuercher pushed a commit that referenced this issue Nov 7, 2019
request_headers_to_add and response_headers_to_add can now use "%REQ(header-name)%"

Risk Level: low
Testing: unit tests
Doc Changes: yes
Release Notes: yes
Fixes: #8683

Signed-off-by: Anatoly Scheglov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests. Not bugs or questions. help wanted Needs help!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants