-
Notifications
You must be signed in to change notification settings - Fork 212
RFC: RequestId for services #1942
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
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
b12f7e1
RFC 23 RequestId
82marbag 1174a06
Add examples
82marbag 070f83d
Name the service
82marbag 3583a22
Applies to: server
6aeaf6e
Update RFC
82marbag 278e447
Update RFC
82marbag 6971c24
Update regex
82marbag a9852cf
typo
82marbag a6975cc
Address comments
82marbag 13f6b2e
Merge branch 'main' into requestid
82marbag da1ddb7
Remove lambda context
82marbag 2e1d8d3
Address comment
82marbag ea24518
Merge branch 'main' into requestid
82marbag File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,137 @@ | ||
RFC: RequestID in business logic handlers | ||
============= | ||
|
||
> Status: RFC | ||
> | ||
> Applies to: server | ||
|
||
For a summarized list of proposed changes, see the [Changes Checklist](#changes-checklist) section. | ||
|
||
Terminology | ||
----------- | ||
|
||
- **RequestID**: a service-wide request's unique identifier | ||
- **UUID**: a universally unique identifier | ||
|
||
RequestID is an element that uniquely identifies a client request. RequestID is used by services to map all logs, events and | ||
specific data to a single operation. This RFC discusses whether and how smithy-rs can make that value available to customers. | ||
|
||
Services use a RequestID to collect logs related to the same request and see its flow through the various operations, | ||
help clients debug requests by sharing this value and, in some cases, use this value to perform their business logic. RequestID is unique across a service at least within a certain timeframe. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, what cases? |
||
|
||
This value for the purposes above must be set by the service. | ||
|
||
Having the client send the value brings the following challenges: | ||
* The client could repeatedly send the same RequestID | ||
* The client could send no RequestID | ||
* The client could send a malformed or malicious RequestID (like in [1](https://en.wikipedia.org/wiki/Shellshock_(software_bug)) and | ||
[2](https://cwiki.apache.org/confluence/display/WW/S2-045)). | ||
|
||
To minimise the attack surface and provide a uniform experience to customers, servers should generate the value. | ||
However, services should be free to read the ID sent by clients in HTTP headers: it is common for services to | ||
read the request ID a client sends, record it and send it back upon success. A client may want to send the same value to multiple services. | ||
Services should still decide to have their own unique request ID per actual call. | ||
|
||
RequestIDs are not to be used by multiple services, but only within a single service. | ||
|
||
<!-- Explain how users will use this new feature and, if necessary, how this compares to the current user experience --> | ||
The user experience if this RFC is implemented | ||
---------------------------------------------- | ||
|
||
The proposal is to implement a `RequestId` type and make it available to middleware and business logic handlers, through [FromParts](../server/from-parts.md) and as a `Service`. | ||
To aid customers already relying on clients' request IDs, there will be two types: `ClientRequestId` and `ServerRequestId`. | ||
|
||
1. Implementing `FromParts` for `Extension<RequestId>` gives customers the ability to write their handlers: | ||
jjant marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
```rust | ||
pub async fn handler( | ||
input: input::Input, | ||
request_id: Extension<ServerRequestId>, | ||
) -> ... | ||
``` | ||
```rust | ||
pub async fn handler( | ||
input: input::Input, | ||
request_id: Extension<ClientRequestId>, | ||
) -> ... | ||
``` | ||
|
||
`ServerRequestId` and `ClientRequestId` will be injected into the extensions by a layer. | ||
This layer can also be used to open a span that will log the request ID: subsequent logs will be in the scope of that span. | ||
|
||
2. ServerRequestId format: | ||
|
||
Common formats for RequestIDs are: | ||
|
||
* UUID: a random string, represented in hex, of 128 bits from IETF RFC 4122: `7c038a43-e499-4162-8e70-2d4d38595930` | ||
* The hash of a sequence such as `date+thread+server`: `734678902ea938783a7200d7b2c0b487` | ||
* A verbose description: `current_ms+hostname+increasing_id` | ||
|
||
For privacy reasons, any format that provides service details should be avoided. A random string is preferred. | ||
The proposed format is to use UUID, version 4. | ||
|
||
A `Service` that inserts a RequestId in the extensions will be implemented as follows: | ||
```rust | ||
impl<R, S> Service<http::Request<R>> for ServerRequestIdProvider<S> | ||
where | ||
S: Service<http::Request<R>>, | ||
{ | ||
type Response = S::Response; | ||
type Error = S::Error; | ||
type Future = S::Future; | ||
|
||
fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> { | ||
self.inner.poll_ready(cx) | ||
} | ||
|
||
fn call(&mut self, mut req: http::Request<R>) -> Self::Future { | ||
request.extensions_mut().insert(ServerRequestId::new()); | ||
self.inner.call(req) | ||
} | ||
} | ||
``` | ||
|
||
For client request IDs, the process will be, in order: | ||
* If a header is found matching one of the possible ones, use it | ||
* Otherwise, None | ||
|
||
`Option` is used to distinguish whether a client had provided an ID or not. | ||
```rust | ||
impl<R, S> Service<http::Request<R>> for ClientRequestIdProvider<S> | ||
where | ||
S: Service<http::Request<R>>, | ||
{ | ||
type Response = S::Response; | ||
type Error = S::Error; | ||
type Future = S::Future; | ||
|
||
fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> { | ||
self.inner.poll_ready(cx) | ||
} | ||
|
||
fn call(&mut self, mut req: http::Request<R>) -> Self::Future { | ||
for possible_header in self.possible_headers { | ||
if let Some(id) = req.headers.get(possible_header) { | ||
req.extensions_mut().insert(Some(ClientRequestId::new(id))); | ||
return self.inner.call(req) | ||
} | ||
} | ||
req.extensions_mut().insert(None); | ||
self.inner.call(req) | ||
} | ||
} | ||
``` | ||
|
||
The string representation of a generated ID will be valid for this regex: | ||
* For `ServerRequestId`: `/^[A-Za-z0-9_-]{,48}$/` | ||
* For `ClientRequestId`: see [the spec](https://httpwg.org/specs/rfc9110.html#rfc.section.5.5) | ||
|
||
Although the generated ID is opaque, this will give guarantees to customers as to what they can expect, if the server ID is ever updated to a different format. | ||
|
||
Changes checklist | ||
----------------- | ||
|
||
- [ ] Implement `ServerRequestId`: a `new()` function that generates a UUID, with `Display`, `Debug` and `ToStr` implementations | ||
- [ ] Implement `ClientRequestId`: `new()` that wraps a string (the header value) and the header in which the value could be found, with `Display`, `Debug` and `ToStr` implementations | ||
- [x] Implement `FromParts` for `Extension<ServerRequestId>` | ||
- [x] Implement `FromParts` for `Extension<ClientRequestId>` |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.