Hubs/Scopes Merge 1 - Introduce IScopes interface.#3297
Conversation
|
lbloder
left a comment
lbloder
left a comment
There was a problem hiding this comment.
Commented on three instances where the HubScopesWrapper could be used to make it compile when -Werror is disabled.
Otherwise LGTM 👍
| void reportFullyDisplayed(); | ||
|
|
||
| /** | ||
| * @deprecated See {@link IHub#reportFullyDisplayed()}. |
| import org.jetbrains.annotations.NotNull; | ||
| import org.jetbrains.annotations.Nullable; | ||
|
|
||
| public interface IScopes { |
There was a problem hiding this comment.
Is there a specific reason we name it plural IScopes instead of IScope?
There was a problem hiding this comment.
IScope is already present (i.e. the scope) and IScopes is a set of multiple scopes (isolation scope and current scope) which (sometimes) fork together or separately. IScopes (and Scopes) is meant to be a drop-in replacement for hub.
| * @deprecated See {@link IHub#reportFullyDisplayed()}. | ||
| */ | ||
| @Deprecated | ||
| default void reportFullDisplayed() { |
There was a problem hiding this comment.
Given that we'll do a major bump I guess it's worth considering removing deprecated methods.
There was a problem hiding this comment.
Created #3364 to track and added it to 8.0.0 milestone.
4 participants
#skip-changelog
📜 Description
Tests on this PR will likely fail, there's more PRs migrating
IHubreferences toIScopesand fixing tests.TODO: this should target a new branch for a new major release.
IScopesIHubextendsIScopesso anyHubcan be passed in asIScopes. All methods onIHubhave been moved toIScopes.IHuband methods tied to it likegetCurrentHub,cloneMainHubandclonehave been deprecated.IHubwe're wrappingIScopesusingHubScopesWrapperScopesas a replacement forHubScopesAdapteras a replacement forHubAdapterSentry.getCurrentScopes()replacesSentry.getCurrentHub()Sentry.cloneMainHub()will be replaced in a follow up PR by something likeSentry.forkRootScopes()or similar.TODOcomments in this PR that have to be cleaned up in follow up PRs.Follow up PRs for changing from
IHubtoIScopes:IHubwithIScopesin core #3298IHubwithIScopesin Android core #3299IHubwithIScopesin Android integrations #3300IHubwithIScopesin apollo integrations #3301IHubwithIScopesin OkHttp integration #3302IHubwithIScopesin GraphQL integration #3303IHubwithIScopesin logging integrations #3304IHubwithIScopesin more integrations #3305IHubwithIScopesin OTel integration #3306IHubwithIScopesin Spring 5 / Spring Boot 2 integrations #3308IHubwithIScopesin Spring 6 / Spring Boot 3 integrations #3309IHubwithIScopesin samples #3310💡 Motivation and Context
This is the first PR of many to merge hubs and scopes which is a prerequisite for Performance powered by OpenTelemetry.
💚 How did you test it?
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps