Skip to content

feat(request-id): Add request-id constructor #552

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tottoto
Copy link
Contributor

@tottoto tottoto commented Mar 24, 2025

Motivation

Provides the same constructor of request-id as x-request-id.

Solution

Adds request-id constructor.

@tottoto tottoto force-pushed the add-request-id-constructor branch 2 times, most recently from f532cd1 to 6496831 Compare March 24, 2025 22:36
@tottoto tottoto changed the title feat(request-id): Add request-id constructor to SetRequestIdLayer feat(request-id): Add request-id constructor Mar 24, 2025
@@ -181,6 +181,7 @@ use tower_layer::Layer;
use tower_service::Service;
use uuid::Uuid;

pub(crate) const REQUEST_ID: HeaderName = HeaderName::from_static("x-request-id");
Copy link
Collaborator

Choose a reason for hiding this comment

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

This still has the x- 😄
But also, do you have a link to docs for the non-x--prefixed header?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, and rebased to resolve the conflict.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://www.rfc-editor.org/rfc/rfc6648 deprecates "X-" prefix header, and request-id is also commonly used same as x-request-id.

Copy link
Collaborator

@jplatte jplatte Apr 10, 2025

Choose a reason for hiding this comment

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

The RFC says "deprecates the convention for newly defined parameters" (emphasis mine) though. Do you have a source for the claim that it's commonly used?

@tottoto tottoto force-pushed the add-request-id-constructor branch from 6496831 to b8e0d1b Compare March 24, 2025 22:58
@tottoto tottoto force-pushed the add-request-id-constructor branch from b8e0d1b to a9ef9e7 Compare March 25, 2025 10:35
@GlenDC
Copy link
Contributor

GlenDC commented Mar 25, 2025

FWIW might want to add a unit test to also test the request_id one. It's dumb logic, but it's easy enough to copy one of the x-request-id ones :)

GlenDC added a commit to plabayo/rama that referenced this pull request Mar 25, 2025
originally contributed to tower-http by @tottoto in
<tower-rs/tower-http#552>

Co-authored-by: tottoto <tottotodev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants