Add "Anatomy of a Service" documentation #1782
Conversation
hlbarber
added
documentation
design/src/docs/anatomy.md
Outdated
|
|
||
| As mentioned, `L` is applied _after_ the `Operation<S, L>` has been "upgraded" to a HTTP service. The procedure of upgrading a model service to a HTTP service is described in the [Upgrading a Model Service](#upgrading-a-model-service) section below. | ||
|
|
||
| ## Serialization and Deserialization |
There was a problem hiding this comment.
At the moment this document is dominated by the builder API and the mechanisms directly under it. However this makes up a small percentage of the server side code.
I think it'd make sense to write a section here on how various shapes structure/enum/map etc to map to Rust types. Not in terms of the actual imperative steps that need to take place in the Kotlin but a kind of declarative description on the mapping. Maybe a table would be nice here?
This section on "Serialization and Deserialization" can then be a subsection of that.
There was a problem hiding this comment.
I think that is already fairly well covered in section 5 here: https://awslabs.github.io/smithy-rs/design/smithy/simple_shapes.html
design/src/docs/anatomy.md
Outdated
|
|
||
| To summarize, the `S`, in `Operation<S, L>`, is a **model service** constructed from a `Handler` or a `OperationService` subject to the constraints of a `OperationShape`. | ||
|
|
||
| Now, what about the `L` in `Operation<S, L>`? |
There was a problem hiding this comment.
The exact reason behind L is a mystery until after we describe its role in the upgrade procedure in a later section. If this is too confusing then reviewers should mention it and I can try remedy it.
design/src/docs/anatomy.md
Outdated
| The following nomenclature will aid us in our survey. We describe a `tower::Service` as a "model service" if it's request and response are Smithy structures, as defined by the `OperationShape` trait - the `GetPokemonSpeciesInput`, `GetPokemonSpeciesOutput`, and `GetPokemonSpeciesError` described above. Similarly, we describe a `tower::Service` as a "HTTP service" if it's request and response are [`http`](https://github.com/hyperium/http) structures - `http::Request` and `http::Response`. | ||
|
|
||
| <!-- TODO(hidden_docs): Link to `Operation` and `OperationShapeExt` documentation --> | ||
| In contrast to the marker ZSTs above, the `Operation<S, L>` structure holds the actual runtime behavior of an operation, which is specified, during construction, by the customer. The `S` here is a model service, this is specified during construction of the `Operation<S, L>`. The constructors exist on the marker ZSTs as an extension trait to `OperationShape`, namely `OperationShapeExt`: |
There was a problem hiding this comment.
In constrast...
No reason is given for why the implementation makes a distinction here. If the immediate reaction is to ask "Why is there a distinction between Op: OperationShape and Operation<S, L>?" then the documentation should provide a satisfactory explanation.
design/src/docs/anatomy.md
Outdated
|
|
||
| Notice that we can "upgrade" a model service to a HTTP service using `FromRequest` and `IntoResponse` described in the prior section: | ||
|
|
||
|  |
There was a problem hiding this comment.
I think all of these diagrams look bad. It's supposed to be a data flow diagram. If anyone has any better ideas for these schematics please comment. UML sequence diagram perhaps? What about the diagrams which indicate composition?
There was a problem hiding this comment.
There's also the question of where should we host them? Is checking them into the commit OK? LFS? S3?
There was a problem hiding this comment.
If you use mermaid, GitHub will automatically render the diagrams (see here as an example when you switch to preview mode). That can save us the hosting headache.
There was a problem hiding this comment.
mermaid looks incredible - this is a great idea.
There was a problem hiding this comment.
mdbook-mermaid is already used in the CI pipeline as well, so they should render correctly on GitHub Pages as well (should we choose to index these in the book later on).
There was a problem hiding this comment.
How're these?
design/src/docs/anatomy.md
Outdated
|
|
||
| As seen in the `Pluggable` documentation, third-parties can use extension traits over `Pluggable` to extend the API of builders. | ||
|
|
||
| ## Accessing Unmodelled Data |
There was a problem hiding this comment.
This section is quite brief. The full story amounts to pepper the code with Exts: FromParts<P> in a predictable way and can be read quite easily from the source code.
Should I go into more detail? Should I include the paragraph above?
design/src/docs/anatomy.md
Outdated
| } | ||
| ``` | ||
|
|
||
| ## Plugins |
There was a problem hiding this comment.
There should be a standalone tutorial on actually writing a Plugin. And when there is we should link to it here.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
design/src/docs/anatomy.md
Outdated
| ```rust | ||
| /// Provides a protocol aware extraction from a [`Request`]. This consumes the | ||
| /// [`Request`], in contrast to [`FromParts`]. | ||
| pub trait FromRequest<Protocol>: Sized { |
There was a problem hiding this comment.
These traits and others all have a loose counterparts in axum e.g. https://docs.rs/axum/latest/axum/extract/trait.FromRequest.html because
- We used to use
axumextensively and inherited some of the machinery. - These traits are occur as common solutions across all frameworks (Rocket, Actix).
Should we mention these facts?
There was a problem hiding this comment.
I don't think it's necessary, it's a fairly common approach in the Rust web ecosystem (as you stated).
design/src/docs/anatomy.md
Outdated
| } | ||
| ``` | ||
|
|
||
| Here `PluginStack` works in a similar way to [`tower::layer::util::Stack`](https://docs.rs/tower/latest/tower/layer/util/struct.Stack.html) - allowing users to append a new plugin rather than replacing the currently set one. |
There was a problem hiding this comment.
It's not immediately clear to me when a user should opt for a Plugin instead of attaching a Layer to the overall Router.
A concrete example of common functionality that could only be accomplished with a Plugin instead of a Layer would ⭐
There was a problem hiding this comment.
That's a very good observation! Plugins allow us to modify the "upgrade procedure" from model services to HTTP services. There we can "squish in" middlewares that work at the model service layer, whereas the ones applied to the overall Router need to operate at the HTTP service layer, before routing has taken place. You'll notice this difference if you look closely at the diagrams.
You can pull the thread for the primary motivation for introducing plugins in item 2 in the Service Builder Improvements RFC, to perform operation-specific logging behavior, but there are other use cases where we want to modify behavior at the model service layer.
There was a problem hiding this comment.
How's this?
8ff688e
There's an unfortunate conflict, which exists for now, between the Router struct, which is essentially an enum of the protocol specific routers and the Router trait which is an interface for routing requests to services. This makes things more confusing for people today.
design/src/docs/anatomy.md
Outdated
|
|
||
| Observe that there are two constructors provided: `from_handler` which takes a `H: Handler` and `from_service` which takes a `S: OperationService`. In both cases `Self` is passed as a parameter to the traits - this constrains `handler: H` and `svc: S` to the signature given by the implementation of `OperationShape` on `Self`. | ||
|
|
||
| The [`Handler`](https://github.com/awslabs/smithy-rs/blob/4c5cbc39384f0d949d7693eb87b5853fe72629cd/rust-runtime/aws-smithy-http-server/src/operation/handler.rs#L21-L29) and [`OperationService`](https://github.com/awslabs/smithy-rs/blob/4c5cbc39384f0d949d7693eb87b5853fe72629cd/rust-runtime/aws-smithy-http-server/src/operation/operation_service.rs#L15-L29) both serve a similar purpose - they provide a common interface for converting to a model service `S`. The `Handler<GetPokemonSpecies>` trait covers all closures taking `GetPokemonSpeciesInput` and asynchronously returning a `Result<GetPokemonSpeciesOutput, GetPokemonSpeciesError>` - they are converted to a model service by a `IntoService`. The `OperationService<GetPokemonSpecies>` trait covers all `tower::Service`s with request `GetPokemonSpeciesInput`, response `GetPokemonSpeciesOutput` and error `GetPokemonSpeciesOutput` - they are converted to a model service by a `Normalize` (this is a very small conversion which flattens request tuples). |
There was a problem hiding this comment.
closures and functions.
There was a problem hiding this comment.
It's the first time IntoService is mentioned, but it's not a link and the definition does not appear inline.
There was a problem hiding this comment.
I think readers will be confused by the mention of Normalize unless you go into more detail here. There are things you left out in the definition of OperationShapeExt above: PollError, Exts... I think it's fair game to omit IntoService here too and maybe cover it later in the text.
Perhaps a suitable educational narrative would be to start with a minimal definition of OperationShape and OperationShapeExt, and keep expanding them as requirements and problems are highlighted.
There was a problem hiding this comment.
There's some detailed documentation in the operation.rs module. I'll link to that so they can see the details if they want.
design/src/docs/anatomy.md
Outdated
| } | ||
| ``` | ||
|
|
||
| The `builder` constructor provides a `PokemonServiceBuilder` where `build` cannot be called until all operations are set because `MissingOperation` purposefully doesn't implement `Upgradable`. In contrast, the `unchecked_builder` which sets all `Op{N}` to `FailOnMissingOperation` can be immediately built, however any unset operations are upgraded into a service which always returns status code 500, as noted in [Upgrading a Model Service](#upgrading-a-model-service). |
There was a problem hiding this comment.
I'd omit unchecked_builder entirely. It's just going to make the reader raise their eyebrows as to its utility if we don't mention here the multi-Lambda based set up use case, which is not relevant to this text.
There was a problem hiding this comment.
I'm on the fence here - if we don't have unchecked_builder and FailOnMissingOperation then we don't have an example of an alternative Upgradable::upgrade path which weakens the motivation for the Upgradable trait entirely - which might raise eyebrows.
design/src/docs/anatomy.md
Outdated
| } | ||
| ``` | ||
|
|
||
| Here `PluginStack` works in a similar way to [`tower::layer::util::Stack`](https://docs.rs/tower/latest/tower/layer/util/struct.Stack.html) - allowing users to append a new plugin rather than replacing the currently set one. |
There was a problem hiding this comment.
That's a very good observation! Plugins allow us to modify the "upgrade procedure" from model services to HTTP services. There we can "squish in" middlewares that work at the model service layer, whereas the ones applied to the overall Router need to operate at the HTTP service layer, before routing has taken place. You'll notice this difference if you look closely at the diagrams.
You can pull the thread for the primary motivation for introducing plugins in item 2 in the Service Builder Improvements RFC, to perform operation-specific logging behavior, but there are other use cases where we want to modify behavior at the model service layer.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
|
||
| ## Accessing Unmodelled Data | ||
|
|
||
| An additional omitted detail is that we provide an "escape hatch" allowing `Handler`s and `OperationService`s to accept data that isn't modelled. In addition to accepting `Op::Input` they can accept additional arguments which implement the [`FromParts`](https://github.com/awslabs/smithy-rs/blob/4c5cbc39384f0d949d7693eb87b5853fe72629cd/rust-runtime/aws-smithy-http-server/src/request.rs#L114-L121) trait: |
There was a problem hiding this comment.
Do we have any example of using FromParts? If so, it would probably be useful to link it here.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
hlbarber
force-pushed
the
harryb/anatomy-service
branch
from
a47ba8b
to
06b604d
Compare
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
Motivation and Context
The new service builder API, described in Service Builder Improvements, is built on a variety of interfaces. Proper documentation should be provided to onboard new contributors on how they stack.
Description
Rendered
anatomy.mdand various diagrams.Notes
#[doc(hidden)]while it awaits customer feedback.#[doc(hidden)]Rust documentation for contributors #1778 if this is acted upon we should switch the GitHub links accordingly.