Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add tower integration #356

Merged
merged 5 commits into from
Aug 12, 2021
Merged

Conversation

Tuetuopay
Copy link
Contributor

This MR adds a new sentry-tower subcrate which provides tower layers intended to bind Sentry hub to each request. This avoids mixing up breadcrumbs in each sentry event, to only keep those in the logical path that led to the captured event. It is best used in conjunction with sentry-log or sentry-tracing to automatically collect logs to breadcrumbs.

It provides a ready to use NewSentryLayer, that creates a new hub from the currently active one and binds it for the duration of the service handler. It is based upon the SentryLayer which is much more flexible, as it can take a hub directly or a closure producing hub. See the lib.rs and test for usage example.

Note that this could be expanded to automatically capture errors returned by the service, should it fail, though more design thought should be put into it.

@Tuetuopay
Copy link
Contributor Author

Note about the clippy lint: it is bogus, the suggested fix does not compiles:

error: implementation of `std::ops::FnOnce` is not general enough
   --> sentry-tower/src/lib.rs:143:9
    |
143 |         Hub::with(Hub::new_from_top).into()
    |         ^^^^^^^^^ implementation of `std::ops::FnOnce` is not general enough
    |
    = note: `fn(&'2 std::sync::Arc<sentry_core::Hub>) -> sentry_core::Hub {sentry_core::Hub::new_from_top::<&'2 std::sync::Arc<sentry_core::Hub>>}` must implement `std::ops::FnOnce<(&'1 std::sync::Arc<sentry_core::Hub>,)>`, for any lifetime `'1`...
    = note: ...but it actually implements `std::ops::FnOnce<(&'2 std::sync::Arc<sentry_core::Hub>,)>`, for some specific lifetime `'2`

Copy link
Member

@Swatinem Swatinem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks very good, thank you! Just some minor comments.

sentry-tower/Cargo.toml Outdated Show resolved Hide resolved
sentry-tower/src/lib.rs Outdated Show resolved Hide resolved
sentry-tower/src/lib.rs Show resolved Hide resolved
sentry-tower/src/helloworld.rs Show resolved Hide resolved
Somehow this works locally and not in CI...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants