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

Record request ID from Cloudfront #223

Merged
merged 2 commits into from
May 23, 2022
Merged

Record request ID from Cloudfront #223

merged 2 commits into from
May 23, 2022

Conversation

nlordell
Copy link
Contributor

Partially fulfills #215

This PR will use an externally set request ID with an info span from the X-Request-ID header (falling back to an internal counter if the aforementioned header is not available).

The motivation behind this change is to request logs in the orderbook API for specific Cloudfront HTTP requests.

Test Plan

You can test this locally by running the orderbook and using curl:

% curl -s -H "X-Request-ID: foobar" http://localhost:8080/api/v1/auction > /dev/null

And check that the log message include the request ID:

% cargo run -p orderbook
...
2022-05-23T12:16:54.163Z  INFO request{id="foobar"}: orderbook::api::request_summary: 127.0.0.1:57539 "GET /api/v1/auction HTTP/1.1" 200 "-" "curl/7.79.1" 294.041µs
...

Whether or not X-Request-ID is the correct header, we will see once this hits staging. That being said, some quick Googling reveals this is a fairly common header to use for identifying requests from reverse proxies.

@nlordell nlordell requested a review from a team as a code owner May 23, 2022 12:23
@@ -198,10 +201,19 @@ pub fn handle_all_routes(
.allow_headers(vec!["Origin", "Content-Type", "X-Auth-Token", "X-AppId"]);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that we intentionally don't add it to the list of allowed CORS headers.

Copy link
Contributor

@sunce86 sunce86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lg

@nlordell nlordell merged commit d68f457 into main May 23, 2022
@nlordell nlordell deleted the use-cloudfront-request-id branch May 23, 2022 12:59
@github-actions github-actions bot locked and limited conversation to collaborators May 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants