fix: use release-health flag in sentry-actix and remove it from subcrates where unneeded
#787
Conversation
…ubcrates where not needed
sentry-actix/src/lib.rs
Outdated
| @@ -294,6 +296,7 @@ where | |||
| .map(|client| client.options().max_request_body_size) | |||
| .unwrap_or(MaxRequestBodySize::None); | |||
| if track_sessions { | |||
There was a problem hiding this comment.
you should probably move the #[cfg] up to where you define this field. I believe the client options auto_session_tracking is also not available with this feature, right?
There was a problem hiding this comment.
At the moment the option is available even without the flag, I've just added a doc line on it to make it clear that the feature flag needs to be enabled for that option to have any effect.
But, I think both auto_session_tracking and session_mode in ClientOptions should instead not be available if the feature is not enabled.
This way, if someone uses those options but their deps don't bring in sentry-core/release-health, it will fail to compile. Better than no-opping silently.
I will update the PR.
|
Now it should be good. |
sentry-core/src/session.rs
Outdated
| @@ -432,6 +432,7 @@ mod session_impl { | |||
| } | |||
|
|
|||
| #[test] | |||
| #[cfg(feature = "release-health")] | |||
There was a problem hiding this comment.
I think we are still running this test in CI, are we?
There was a problem hiding this comment.
Actually the whole session.rs is wrapped in:
#[cfg(feature = "release-health")]
mod session_impl {
So effectively these extra cfg macros inside it are useless. Removed them.
Removes the
release-healthfeature fromsentry-towerandsentry-tracing, where it was used unnecessarily.It's still a feature flag, enabled by default, in
sentry-actix, as it's used there to create a session per request.This was the default behavior previously, with no way of opting out, so it makes sense to keep it enabled by default there.
sentry-rust/sentry-actix/Cargo.toml
Line 16 in 3cc461a
I've fixed it to actually be used in the code because compilation would fail otherwise due to the
hub.start_session();call.There is no change to the
sentrycrate, where this flag is part of the default features as intended:sentry-rust/sentry/Cargo.toml
Line 30 in 3cc461a
Note that
sentryhas no way to bring insentry-actixvia feature flags.Is there a reason for that?
I'll create a separate PR to enable it, similarly to other integrations.