Add minimal tracing to server framework #1314
Conversation
This commit adds minimal tracing at the `DEBUG` level to the code-generated `OperationHandler` trait implementations for each operation, logging the deserialized operation input as well as any errors. The router also has some minimal tracing to detect requests that are not routed successfully.
|
A new generated diff is ready to view.
A new doc preview is ready to view. Rust Wrk benchmark report:Duration: 90 sec, Connections: 32, Threads: 2
|
| let extension = $serverCrate::extension::RuntimeErrorExtension::new(extension_not_found_rejection.to_string()); | ||
| let runtime_error = $serverCrate::runtime_error::RuntimeError { | ||
| protocol: #{SmithyHttpServer}::protocols::Protocol::${protocol.name.toPascalCase()}, | ||
| kind: extension_not_found_rejection.into(), | ||
| }; | ||
| let mut response = runtime_error.into_response(); | ||
| response.extensions_mut().insert(extension); | ||
| return response.map($serverCrate::body::boxed); | ||
| let response = response.map($serverCrate::body::boxed); | ||
| #{Tracing}::debug!(?response, "returning HTTP response"); |
There was a problem hiding this comment.
Response bodies should probably only be logged at TRACE level, since they can be very big.
Same for request bodies, which I'm not logging anywhere.
There was a problem hiding this comment.
Most boxed types don't have a real debug impl https://docs.rs/http-body/latest/src/http_body/combinators/box_body.rs.html#32-36 so is this actually useful to print the body?
| @@ -157,6 +181,7 @@ class ServerOperationHandlerGenerator( | |||
| Fut: std::future::Future<Output = $outputType> + Send, | |||
| B: $serverCrate::body::HttpBody + Send + 'static, $streamingBodyTraitBounds | |||
| B::Data: Send, | |||
| B: std::fmt::Debug, | |||
There was a problem hiding this comment.
I don't really like adding debug bounds like this since sometimes it can be painful. I think we should avoid maybe logging bodies?
There was a problem hiding this comment.
Unless its an error then I would try to convert the body into a string to print that maybe?
There was a problem hiding this comment.
I was thinking of logging them at TRACE level, but they can contain sensitive data (https://awslabs.github.io/smithy/1.0/spec/core/documentation-traits.html#sensitive-trait), so it's best that we don't log anything by default. Perhaps we can do it in a separate Tower middleware users can opt into if they absolutely must log bodies.
|
The Pokémon Service should change its usage of |
|
Closing in favor of #1536. |
This commit adds minimal tracing at the
DEBUGlevel to thecode-generated
OperationHandlertrait implementations for eachoperation, logging the deserialized operation input as well as any
errors. The router also has some minimal tracing to detect requests that
are not routed successfully.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Examples
Successful
pokemon-speciesoperation:Unsuccessful
pokemon-speciesoperation:Unsuccessful routing: