Update Local Scope Documentation #5136
Conversation
…a to include the new callback. align old code example and leave as alternative
|
@lbloder is attempting to deploy a commit to the Sentry Team on Vercel. A member of the Team first needs to authorize it. |
|
In order to keep the "old" version of creating a local Scope in the JavaSDK in the docs and not mess with the existing docs too much, I added 2 new Tabs to the code example. Not 100% sure if that is the way to go. What do you guys think? All in all I'm not really satisfied with the current state of that part of the doc, because we have different ways of creating a local scope depending on the SDK.
Or we add it as a function parameter to the captureMethods like in the following SDKs
The docs match the first approach, but are not 100% correct for the second one. Should we split them up and create two different doc sections based on the approach the SDK uses? |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
|
||
| <PlatformSection notSupported={["android"]}> | ||
|
|
||
| The JavaSDK provides two alternative ways of configuring a local scope. |
There was a problem hiding this comment.
| The JavaSDK provides two alternative ways of configuring a local scope. | |
| The Java SDK provides two alternative ways of configuring a local scope. |
| @@ -93,7 +93,7 @@ You can also apply this configuration when unsetting a user at logout: | |||
| To learn what useful information can be associated with scopes see | |||
| [the context documentation](../context/). | |||
|
|
|||
| <PlatformSection notSupported={["android"]}> | |||
| <PlatformSection> | |||
There was a problem hiding this comment.
This now shows the default "Local Scope" section for Android which mentions withScope a lot. Since withScope does not play nice with globalHubMode I'd say it would be better if we could add a custom text around it as well to not mention withScope.
There was a problem hiding this comment.
Not exactly, it shows the same doc (like all SDKs), but only the code example for the callback (like dotnet). Java on the other hand has code examples for both the old and the new version.
However, as mentioned above, we do have a discrepancy in the SDKs anyways. Some use an enclosure like the original java withScope and some use a callback-based approach on the capture methods directly.
In my opinion we should have 2 Sections for LocalScope something like
- Local Scope
- Local Scope Callback
and show them for the relevant SDKs.
Because the current doc doesn't really make sense for all the Callback based SDKs. For Java we could show both sections in the meantime.
There was a problem hiding this comment.
For Java we could also show an alert that the withScope variant doesn't play well with globalHubMode
There was a problem hiding this comment.
Sounds good, I'd move the current docs into a subheading and have a sibling subheading for the capture overload with callback param.
maybe like this:
## Local Scopes
### Using `withScope`
### Using Scope Callback Parameter
Warning sounds good as well.
| @@ -93,7 +93,7 @@ You can also apply this configuration when unsetting a user at logout: | |||
| To learn what useful information can be associated with scopes see | |||
| [the context documentation](../context/). | |||
|
|
|||
| <PlatformSection notSupported={["android"]}> | |||
| <PlatformSection> | |||
|
|
|||
| ## Local Scopes | |||
|
|
|||
There was a problem hiding this comment.
Further down is an <Alert> explaining that exceptions in withScope will be ignored. If I'm not mistaken, the new local scope overloads don't behave the same way. I think they should. Can we please update the implementation to catch Exceptions as withScope does?
There was a problem hiding this comment.
Well, we do have a bit of a discrepancy between the SDKs that use the callback parameter in this case: cocoa and dart don't capture exceptions that are thrown within the callback, while dotnet and java do because they invoke the callback within the try/catch of the respective capture methods
WDYT the correct behavior should be?
There was a problem hiding this comment.
I've created this getsentry/sentry-java#2123 to keep track of it. It's not blocking this PR just noticed this here and didn't wanna drop it.
…hScope and scope callback parameter of capture methods
lbloder
changed the title
| called `with-scope`, `push-scope` or implemented as a function parameter on the capture methods, depending on the SDK. It's very helpful if | ||
| you only want to send data for one specific event. | ||
|
|
||
| <PlatformSection supported={["go", "java", "javascript", "native", "php", "python", "ruby", "rust"]}> |
There was a problem hiding this comment.
How did we arrive at this list of supported platforms? Is it all that have a file in src/includes/enriching-events/scopes/with-scope?
There was a problem hiding this comment.
Native just has _This is not supported by the Native SDK._ shouldn't that simply be removed from the list?
There was a problem hiding this comment.
Yes, went through the platforms in the with-scope folder.
Removing native sounds good to me. Will have a look if that has any implications.
…move with-scope section from android
Adapt Documentation for LocalScopes (getsentry/sentry-java#2084)