From 9ae16e79495357f7690d142d15fce46e3cf9ac2f Mon Sep 17 00:00:00 2001 From: 82marbag <69267416+82marbag@users.noreply.github.com> Date: Wed, 30 Nov 2022 09:20:53 -0500 Subject: [PATCH] RFC: RequestId for services (#1942) * RFC 24 RequestId Signed-off-by: Daniele Ahmed --- design/src/SUMMARY.md | 1 + design/src/rfcs/overview.md | 1 + ...rfc0022_error_context_and_compatibility.md | 2 +- design/src/rfcs/rfc0024_request_id.md | 137 ++++++++++++++++++ 4 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 design/src/rfcs/rfc0024_request_id.md diff --git a/design/src/SUMMARY.md b/design/src/SUMMARY.md index 26b95d07d..4f745d310 100644 --- a/design/src/SUMMARY.md +++ b/design/src/SUMMARY.md @@ -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) diff --git a/design/src/rfcs/overview.md b/design/src/rfcs/overview.md index 6a458ad77..b4224e813 100644 --- a/design/src/rfcs/overview.md +++ b/design/src/rfcs/overview.md @@ -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) diff --git a/design/src/rfcs/rfc0022_error_context_and_compatibility.md b/design/src/rfcs/rfc0022_error_context_and_compatibility.md index 63f4c5da3..cf03b2ed8 100644 --- a/design/src/rfcs/rfc0022_error_context_and_compatibility.md +++ b/design/src/rfcs/rfc0022_error_context_and_compatibility.md @@ -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 diff --git a/design/src/rfcs/rfc0024_request_id.md b/design/src/rfcs/rfc0024_request_id.md new file mode 100644 index 000000000..5c150ca20 --- /dev/null +++ b/design/src/rfcs/rfc0024_request_id.md @@ -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. + + +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` gives customers the ability to write their handlers: + +```rust +pub async fn handler( + input: input::Input, + request_id: Extension, +) -> ... +``` +```rust +pub async fn handler( + input: input::Input, + request_id: Extension, +) -> ... +``` + +`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 Service> for ServerRequestIdProvider +where + S: Service>, +{ + type Response = S::Response; + type Error = S::Error; + type Future = S::Future; + + fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { + self.inner.poll_ready(cx) + } + + fn call(&mut self, mut req: http::Request) -> 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 Service> for ClientRequestIdProvider +where + S: Service>, +{ + type Response = S::Response; + type Error = S::Error; + type Future = S::Future; + + fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { + self.inner.poll_ready(cx) + } + + fn call(&mut self, mut req: http::Request) -> 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` +- [x] Implement `FromParts` for `Extension` -- GitLab