Skip to content

Commit

Permalink
RFC: RequestId for services (#1942)
Browse files Browse the repository at this point in the history
* RFC 24 RequestId

Signed-off-by: Daniele Ahmed <[email protected]>
  • Loading branch information
82marbag authored Nov 30, 2022
1 parent b67f492 commit 9ae16e7
Show file tree
Hide file tree
Showing 4 changed files with 140 additions and 1 deletion.
1 change: 1 addition & 0 deletions design/src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
- [RFC-0021: Dependency Versions](./rfcs/rfc0021_dependency_versions.md)
- [RFC-0022: Error Context and Compatibility](./rfcs/rfc0022_error_context_and_compatibility.md)
- [RFC-0023: Evolving the new service builder API](./rfcs/rfc0023_refine_builder.md)
- [RFC-0024: RequestID](./rfcs/rfc0024_request_id.md)

- [Contributing](./contributing/overview.md)
- [Writing and debugging a low-level feature that relies on HTTP](./contributing/writing_and_debugging_a_low-level_feature_that_relies_on_HTTP.md)
1 change: 1 addition & 0 deletions design/src/rfcs/overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,4 @@
- [RFC-0021: Dependency Versions](./rfc0021_dependency_versions.md)
- [RFC-0022: Error Context and Compatibility](./rfc0022_error_context_and_compatibility.md)
- [RFC-0023: Evolving the new service builder API](./rfc0023_refine_builder.md)
- [RFC-0024: RequestID](./rfc0024_request_id.md)
2 changes: 1 addition & 1 deletion design/src/rfcs/rfc0022_error_context_and_compatibility.md
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ What could be improved:

### Hyper 1.0

Hyper is has outlined [some problems they want to address with errors](https://github.com/hyperium/hyper/blob/bd7928f3dd6a8461f0f0fdf7ee0fd95c2f156f88/docs/ROADMAP.md#errors)
Hyper has outlined [some problems they want to address with errors](https://github.com/hyperium/hyper/blob/bd7928f3dd6a8461f0f0fdf7ee0fd95c2f156f88/docs/ROADMAP.md#errors)
for the coming 1.0 release. To summarize:

- It's difficult to match on specific errors (Hyper 0.x's `Error` relies
Expand Down
137 changes: 137 additions & 0 deletions design/src/rfcs/rfc0024_request_id.md
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.

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:

```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>`

0 comments on commit 9ae16e7

Please sign in to comment.