Improve the server instrumentation #2521
Labels
enhancement
New feature or request
good first issue
Good for newcomers
high-priority
High priority issue
server
Rust server SDK
Comments
hlbarber
added
enhancement
hlbarber
changed the title
This was referenced Mar 31, 2023
|
Something to keep in mind, from #2524 (comment)
This would be an easy thing to test if we had a plugin to generate application binaries using generated SDKs. |
david-perez
added a commit
that referenced
this issue
Apr 19, 2023
This commit logs server request rejections at the `DEBUG` level in an operation's `FromRequest` implementation. This commit is analogous to the one in PR #2524 for response rejections. However, request rejections are _not_ errors, so they shouldn't be logged at the `ERROR` level. Indeed, they happen every time the server rejects a malformed request. Prior to this commit, the `RuntimeError::NotAcceptable` variant was the only `RuntimeError` variant that was manually constructed. This commit makes it so that it now results from a conversion from a new `RequestRejection::NotAcceptable` variant. We now leverage `futures_util::future::FutureExt::map` to map a future that uses `RequestRejection` as its error into a future that uses `RuntimeError`, and centrally log the rejection there. `futures_util` is already a transitive dependency of server SDKs (via e.g. `hyper` and `tower`), so adding it is a direct dependency is not worse. This helps with #2521.
david-perez
added a commit
that referenced
this issue
Apr 19, 2023
This commit logs server request rejections at the `DEBUG` level in an operation's `FromRequest` implementation. This commit is analogous to the one in PR #2524 for response rejections. However, request rejections are _not_ errors, so they shouldn't be logged at the `ERROR` level. Indeed, they happen every time the server rejects a malformed request. Prior to this commit, the `RuntimeError::NotAcceptable` variant was the only `RuntimeError` variant that was manually constructed. This commit makes it so that it now results from a conversion from a new `RequestRejection::NotAcceptable` variant. We now leverage `futures_util::future::TryFutureExt::map` to map a future that uses `RequestRejection` as its error into a future that uses `RuntimeError`, and centrally log the rejection there. `futures_util` is already a transitive dependency of server SDKs (via e.g. `hyper` and `tower`), so adding it is a direct dependency is not worse. This helps with #2521. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
unexge
pushed a commit
that referenced
this issue
Apr 24, 2023
This commit logs server request rejections at the `DEBUG` level in an operation's `FromRequest` implementation. This commit is analogous to the one in PR #2524 for response rejections. However, request rejections are _not_ errors, so they shouldn't be logged at the `ERROR` level. Indeed, they happen every time the server rejects a malformed request. Prior to this commit, the `RuntimeError::NotAcceptable` variant was the only `RuntimeError` variant that was manually constructed. This commit makes it so that it now results from a conversion from a new `RequestRejection::NotAcceptable` variant. We now leverage `futures_util::future::TryFutureExt::map` to map a future that uses `RequestRejection` as its error into a future that uses `RuntimeError`, and centrally log the rejection there. `futures_util` is already a transitive dependency of server SDKs (via e.g. `hyper` and `tower`), so adding it is a direct dependency is not worse. This helps with #2521. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
rcoh
pushed a commit
that referenced
this issue
Apr 24, 2023
This commit logs server request rejections at the `DEBUG` level in an operation's `FromRequest` implementation. This commit is analogous to the one in PR #2524 for response rejections. However, request rejections are _not_ errors, so they shouldn't be logged at the `ERROR` level. Indeed, they happen every time the server rejects a malformed request. Prior to this commit, the `RuntimeError::NotAcceptable` variant was the only `RuntimeError` variant that was manually constructed. This commit makes it so that it now results from a conversion from a new `RequestRejection::NotAcceptable` variant. We now leverage `futures_util::future::TryFutureExt::map` to map a future that uses `RequestRejection` as its error into a future that uses `RuntimeError`, and centrally log the rejection there. `futures_util` is already a transitive dependency of server SDKs (via e.g. `hyper` and `tower`), so adding it is a direct dependency is not worse. This helps with #2521. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
At the moment
smithy-rsserver does not have great instrumentation coverage.We should identify key points in the request/response lifecycle to emit events/open spans.
Care MUST be taken to ensure that we do not log anything that could be sensitive.
The text was updated successfully, but these errors were encountered: