Update scopes documentation for Java and Android #4747

designedbyz · 2022-02-19T04:33:56Z

I was looking for information on the Sentry Kotlin extensions recently and as they're only significantly used for scopes found my way there. I found the documentation was a bit confusing, and decided to improve it. I also hid local scopes for Android since the documentation said that they are not supported there, and displayed the Kotlin Extensions information, as many Android apps use Kotlin and Coroutines (like the one I work on that uses Sentry!!). Finally, I moved the scope synchronization information out of the middle of the demonstration of setting and unsetting a user with with-config

I realize that the contributing docs say not to touch the common content, but that was very much needed, both to improve clarity and clean up the Android documentation.

vercel · 2022-02-19T04:34:01Z

@designedbyz is attempting to deploy a commit to the Sentry Team on Vercel.

A member of the Team first needs to authorize it.

I stumbled across the Local Scopes documentation while looking for information on the Kotlin extensions and thought I could make it a bit more clear. I also made mentioning that exceptions and errors thrown within `with-scope` an alert as I found that to be very surprising and unexpected, particularly as that seems like different behavior from `configure-scope`.

Not using a `SentryContext` when working with Sentry in a coroutine _may_ have been the cause of a bug I dealt with at work recently. Since lots of Android apps use Kotlin now, the Sentry Kotlin Extensions should be called out in the setup guides.

Local scopes are not supported on Android, but the documentation for them still shows up anyways. Conversely, information about the Kotlin extensions is missing, but many Android apps make heavy use of Kotlin and Coroutines, so this section should be displayed.

The android specific scope synchronization block was right after the code block that demonstrated setting a user, and before the code block that unsets a user. This made reading the documentation confusing, so I extracted a new included page and added a new platform section that only supports android to hold it.

vercel · 2022-02-22T13:53:28Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/sentry/sentry-docs/EkCz5J51j1LRy6CoCsHut9y875am
✅ Preview: https://sentry-docs-git-fork-designedbyz-update-scopes.sentry.dev

designedbyz · 2022-02-28T17:53:45Z

Hello! I just wanted to bump this and see if there's anything you need from me to get this merged. Happy to make changes as needed!

imatwawana · 2022-02-28T19:26:47Z

Hello! I just wanted to bump this and see if there's anything you need from me to get this merged. Happy to make changes as needed!

Hi @designedbyz, not yet. Once technical review is completed on this, we'll review it for writing/style and then there might be some feedback. Sorry about the delay.

src/includes/getting-started-install/android.mdx

src/platforms/common/enriching-events/scopes.mdx

imatwawana · 2022-02-28T20:34:18Z

@designedbyz, since there are a few concerns raised from technical review, I'll wait till those are resolved before I review for writing/style.

src/includes/enriching-events/scopes/local-scope/android.mdx

src/includes/enriching-events/scopes/scope-synchronization/android.mdx

designedbyz · 2022-03-01T05:41:20Z

Hi @designedbyz, not yet. Once technical review is completed on this, we'll review it for writing/style and then there might be some feedback. Sorry about the delay.

Not a problem! I just wanted to check in. Thanks for getting eyes on this!

designedbyz · 2022-03-17T00:05:56Z

Hey @marandaneto, @bruno-garcia, any chance of an updated review?

marandaneto · 2022-03-17T09:32:02Z

Hey @marandaneto, @bruno-garcia, any chance of an updated review?

src/platforms/common/enriching-events/attachments/index.mdx
error: bad link platforms/android/enriching-events/scopes#local-scopes at line 72

Please fix the linking error, so I can check the preview and its changes, thanks :)

designedbyz · 2022-03-31T18:38:04Z

Yay, looks like the license check passed and Vercel deployed the most recent changes!

marandaneto · 2022-04-01T07:29:22Z

Yay, looks like the license check passed and Vercel deployed the most recent changes!

Thanks, I will review on Monday :)

designedbyz · 2022-04-02T18:59:59Z

Thanks, I will review on Monday :)

Awesome, thank you!

marandaneto · 2022-04-04T13:07:57Z

src/platforms/common/enriching-events/scopes.mdx


-<PlatformSection supported={["java"]} notSupported={["android"]}>
+<PlatformSection supported={["java", "android"]}>


I guess we need to keep android as non supported due to https://github.com/getsentry/sentry-docs/pull/4747/files#r841714836

Oh! with this did you mean that you wanted to remove the mention of Kotlin as well as the local scopes stuff?

Yes, for Android, since there's a global hub/scope, it does not make sense to use that integration, so we can remove from the android page.

Got it, that makes sense. As a counter point though, I was confused that it was mentioned for Java and not Android, which is why I added it to begin with. What do you think about mentioning somewhere in the Android setup docs that the Kotlin extensions aren't needed for Android?

Not sure, the content is not for Android at all, it even says:

Sentry's SDK for Java stores the scope and the context in a thread-local variable

Instead of Sentry's SDK for Android, which is the case for all the other Android integrations, I guess explaining something from another platform would also make confusion.

@marandaneto done in 10b558e

Not sure, the content is not for Android at all, it even says:

Sentry's SDK for Java stores the scope and the context in a thread-local variable

Instead of Sentry's SDK for Android, which is the case for all the other Android integrations, I guess explaining something from another platform would also make confusion.

To go back to this thread though, I was confused and thought that the Kotlin extensions lib was needed for two reasons:

The first is the general tendency for java focused libs to have a lib-ktx library with extension functions focused on making the library more kotlin-like in the Kotlin world.

The platforms page for Kotlin explicitly says that it can be used for Android, and mentions the Kotlin Extensions, and links to the Java page There's nothing in either the Kotlin page or the using Sentry with coroutines part of the Java documents that says the extension functions library isn't needed for Android.

Thus, it's really easy for someone who's not familiar with the platform but using Kotlin on Android to poke around the docs and accidentally think that they need the Kotlin extensions library, when they do not.

Agree, The goal is that sentry-kotlin-extensions get expanded and add extension functions to be "Kotlin friendly", but right now it only has the SentryContext and this only makes sense for non-Android.

E.g. https://github.com/getsentry/sentry-java/pull/1886/files
This would make sense for Android too.

I'd leave this specific part for another PR, this will get addressed automatically when we land the 1st Kotlin function in the sentry-kotlin-extensions library for Android, which is gonna be very soon :)
Thanks for the feedback.

Agree, The goal is that sentry-kotlin-extensions get expanded and add extension functions to be "Kotlin friendly", but right now it only has the SentryContext and this only makes sense for non-Android.

Got it, that makes sense.

E.g. https://github.com/getsentry/sentry-java/pull/1886/files
This would make sense for Android too.

Oooh, that would be handy.

I'd leave this specific part for another PR, this will get addressed automatically when we land the 1st Kotlin function in the sentry-kotlin-extensions library for Android, which is gonna be very soon :)
Thanks for the feedback.

Awesome! That sounds good to me!

marandaneto · 2022-04-04T13:09:30Z

@imatwawana it's ready for wording review, the only issue left is #4747 (comment) from the technical point of view.

imatwawana

Made some wording, style, and punctuation edits, but otherwise, looks great!

src/includes/getting-started-install/android.mdx

src/platforms/common/enriching-events/scopes.mdx

Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>

designedbyz · 2022-04-26T16:05:13Z

Also, sorry for those force pushes earlier, I was trying to update this branch with master, but got things in a weird state, so I just put it all back to how it was before I started trying to rebase/ include the changes. (I'm usually in closed source land, so this is my first PR across repos 😅 )10b558e has the latest changes requested by @marandaneto

vercel · 2022-04-27T07:12:09Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
sentry-docs	✅ Ready (Inspect)	Visit Preview	Apr 27, 2022 at 7:29AM (UTC)

marandaneto · 2022-04-27T08:03:17Z

Thanks @designedbyz and sorry for the back and forth.

designedbyz · 2022-04-27T19:19:43Z

@marandaneto You're welcome, don't worry about it, I'm responsible for a bunch of it 😅 Thank you for reviewing!

designedbyz added 3 commits February 19, 2022 01:03

Cleanup sentry for android local scopes doc.

512c9e3

designedbyz force-pushed the update-scopes branch from bdb056f to 44654b2 Compare February 19, 2022 06:07

designedbyz and others added 2 commits February 19, 2022 01:18

designedbyz force-pushed the update-scopes branch from 44654b2 to 85df56d Compare February 19, 2022 06:18

imatwawana requested a review from a team February 22, 2022 13:53

trigger build

52945f7

vercel bot deployed to Preview February 22, 2022 14:39 View deployment

marandaneto requested review from bruno-garcia and imatwawana February 23, 2022 07:50

marandaneto reviewed Feb 28, 2022

View reviewed changes

src/includes/getting-started-install/android.mdx Outdated Show resolved Hide resolved

marandaneto reviewed Feb 28, 2022

View reviewed changes

src/platforms/common/enriching-events/scopes.mdx Outdated Show resolved Hide resolved

marandaneto reviewed Feb 28, 2022

View reviewed changes

src/platforms/common/enriching-events/scopes.mdx Show resolved Hide resolved

bruno-garcia reviewed Mar 1, 2022

View reviewed changes

src/includes/enriching-events/scopes/local-scope/android.mdx Outdated Show resolved Hide resolved

src/includes/enriching-events/scopes/scope-synchronization/android.mdx Show resolved Hide resolved

Fix accidentally excluded platform section

1d1fbe0

designedbyz added 2 commits March 7, 2022 22:38

Remove Local Scope information from Android docs

014eaa2

Only show Kotlin info for Java and Android

70231d3

designedbyz requested review from marandaneto and bruno-garcia March 8, 2022 03:45

vercel bot deployed to Preview March 8, 2022 14:45 View deployment

vercel bot deployed to Preview March 31, 2022 13:20 View deployment

marandaneto reviewed Apr 4, 2022

View reviewed changes

imatwawana approved these changes Apr 4, 2022

View reviewed changes

marandaneto and others added 7 commits April 5, 2022 10:03

Update src/platforms/common/enriching-events/scopes.mdx

fb07118

Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>

Update src/platforms/common/enriching-events/scopes.mdx

74f8aa8

Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>

Update src/platforms/common/enriching-events/scopes.mdx

ba85cae

Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>

Update src/platforms/common/enriching-events/scopes.mdx

e146473

Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>

Update src/platforms/common/enriching-events/scopes.mdx

fb551f7

Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>

Update src/platforms/common/enriching-events/scopes.mdx

a1d24cf

Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>

Update src/includes/getting-started-install/android.mdx

e2de2e3

Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>

vercel bot deployed to Preview April 5, 2022 14:49 View deployment

designedbyz force-pushed the update-scopes branch 3 times, most recently from 83e8bc6 to e2de2e3 Compare April 26, 2022 15:31

Remove mention of kotlin extensions dependency from Android scopes info

10b558e

designedbyz requested a review from marandaneto April 26, 2022 16:05

marandaneto approved these changes Apr 26, 2022

View reviewed changes

Merge branch 'master' into update-scopes

bc7165a

vercel bot deployed to Preview April 27, 2022 07:29 View deployment

marandaneto merged commit c00d621 into getsentry:master Apr 27, 2022

designedbyz deleted the update-scopes branch April 27, 2022 19:42

github-actions bot locked and limited conversation to collaborators May 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update scopes documentation for Java and Android #4747

Update scopes documentation for Java and Android #4747

designedbyz commented Feb 19, 2022 •

edited

Loading

vercel bot commented Feb 19, 2022

vercel bot commented Feb 22, 2022 •

edited

Loading

designedbyz commented Feb 28, 2022

imatwawana commented Feb 28, 2022

imatwawana commented Feb 28, 2022

designedbyz commented Mar 1, 2022

designedbyz commented Mar 17, 2022

marandaneto commented Mar 17, 2022

designedbyz commented Mar 31, 2022

marandaneto commented Apr 1, 2022

designedbyz commented Apr 2, 2022

marandaneto Apr 4, 2022

designedbyz Apr 5, 2022

marandaneto Apr 6, 2022

designedbyz Apr 6, 2022

marandaneto Apr 7, 2022

designedbyz Apr 26, 2022

designedbyz Apr 26, 2022

marandaneto Apr 26, 2022

marandaneto Apr 26, 2022

designedbyz Apr 26, 2022

marandaneto commented Apr 4, 2022

imatwawana left a comment

designedbyz commented Apr 26, 2022

vercel bot commented Apr 27, 2022 •

edited

Loading

marandaneto commented Apr 27, 2022

designedbyz commented Apr 27, 2022


		<PlatformSection supported={["java"]} notSupported={["android"]}>
		<PlatformSection supported={["java", "android"]}>

Update scopes documentation for Java and Android #4747

Update scopes documentation for Java and Android #4747

Conversation

designedbyz commented Feb 19, 2022 • edited Loading

vercel bot commented Feb 19, 2022

vercel bot commented Feb 22, 2022 • edited Loading

designedbyz commented Feb 28, 2022

imatwawana commented Feb 28, 2022

imatwawana commented Feb 28, 2022

designedbyz commented Mar 1, 2022

designedbyz commented Mar 17, 2022

marandaneto commented Mar 17, 2022

designedbyz commented Mar 31, 2022

marandaneto commented Apr 1, 2022

designedbyz commented Apr 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marandaneto commented Apr 4, 2022

imatwawana left a comment

Choose a reason for hiding this comment

designedbyz commented Apr 26, 2022

vercel bot commented Apr 27, 2022 • edited Loading

marandaneto commented Apr 27, 2022

designedbyz commented Apr 27, 2022

designedbyz commented Feb 19, 2022 •

edited

Loading

vercel bot commented Feb 22, 2022 •

edited

Loading

vercel bot commented Apr 27, 2022 •

edited

Loading