startSession and endSession available thru the Static class
Java does have Sentry.startSession and Sentry.endSession so if one decides to do its own instrumentation, that's possible.
https://develop.sentry.dev/sdk/sessions/#exposed-api
here the exposed API only states available thru the Hub.
Java hides getCurrentHub, it's package-private.
Cocoa has SentrySDK.currentHub() public so one could SentrySDK.currentHub().startSession() but it doesn't have SentrySDK.startSession directly on the Static API.
Is there a reason why? should we unify this?
@bruno-garcia @philipphofmann tagging @mitsuhiko as the initial API design.
On Cocoa, you have both SentrySDK.currentHub, which we use internally quite a lot, and SentrySdk.setCurrentHub(). I don't think that you have many use cases on mobile where you want to change the hub. I don't really know why these two methods are public. Maybe @HazAT or @bruno-garcia you know why these methods are public?
I think it would make sense to offer both Hub.startSession() and SentrySDK.startSession(), but I'm not sure if the above methods should be public.
I wish that wasn't public. We have as part of the documented API a way to bind a client to the scope. but swapping out Hubs from the static API isn't something users should be aware as I understand there are no use cases and just makes things harder for us to reason about the code base.
I think it would make sense to offer both Hub.startSession() and SentrySDK.startSession(), but I'm not sure if the above methods should be public.
Makes sense to me. The static SentrySDK.startSession() is just a pass through to the Hub instance set to the static field, so we need that to call into hub.startSession().
I think the main reason why this is public in Cocoa is that RN, for example, uses it. Still, we could replace the usages in RN with calls to a new method in SentrySDK directly.
I propose the following:
- Mark
SentrySDK.currentHubandSentrySDK.setCurrentHubas deprecated and remove them in 7.0.0. - Add
captureEnvelopeandstoreEnvelopetoSentrySDK.mbut not in the header. The hybrid SDKs can create a category to make these two methods accessible. We use this pattern for the test initializers already. - Add both
SentrySDK.startSession()andSentrySDK.endSession()to the Static API.
Sound good to me. @HazAT you might have thoughts or we're good to go?
@mitsuhiko any reasons not to add startSession on the static Sentry entrypoint?
If not, we'll add it to develop.sentry.dev
I don’t have strong opinions. It didn’t sound like a super common use case which is why it was on the hub to start.
This is quite similar in spirit to the set_fingerprint we discussed yesterday, the big difference being Java not exposing a way to get the current hub, thus leaving no way to have methods on the Hub that are not also exposed in the global/static API.
I wonder if instead of documenting startSession as a global API as proposed in https://github.com/getsentry/develop/issues/248#issuecomment-805887468, we could consider making Java more similar to the rest of the SDKs and make getCurrentHub public? What were the downsides that led to getCurrentHub being private?
@rhcarvalho pretty much because getCurrentHub isn't in the unified API -> https://develop.sentry.dev/sdk/unified-api/#static-api
only the Hub exposes it, but nobody should deal with the Hub directly.
I agree with @marandaneto that we shouldn't expose getCurrentHub. It was public in Cocoa until 7.0.0. For me, it would be weird to do almost all interactions through the static API, and then for more advanced things, use getCurrentHub. I think all interactions should be possible through the static class to keep it consistent.
@lbloder FYI
This was fixed with https://github.com/getsentry/sentry-cocoa/pull/1021 in 2021. We can close this.