Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Local Scope Documentation #5136

Merged
merged 5 commits into from
Jun 27, 2022
Merged

Update Local Scope Documentation #5136

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
29f4570
add local scope doc to Android Platform, adapt LocalScope doc for Jav…
lbloder Jun 10, 2022
5f8f0d2
Update local scope documentation. Differentiate between withScope/pus…
lbloder Jun 16, 2022
715fa08
Merge branch 'master' into feat/java-local-scope
lbloder Jun 19, 2022
12e870a
CR remove local scope section from native, as it is not supported. Re…
lbloder Jun 24, 2022
e85f78b
Merge branch 'getsentry:master' into feat/java-local-scope
lbloder Jun 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 79 additions & 8 deletions src/includes/enriching-events/scopes/with-scope/java.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,96 @@
<PlatformSection supported={["android"]}>

```java {tabTitle: Java}
import io.sentry.Sentry;
import io.sentry.SentryLevel;

// will be tagged with my-tag="my value"
Sentry.captureException(new Exception("my error"), scope -> {
scope.setTag("my-tag", "my value");
scope.setLevel(SentryLevel.WARNING);
});

// will not be tagged with my-tag
Sentry.captureException(new Exception("my error"));
```

```kotlin {tabTitle: Kotlin}
import io.sentry.Sentry
import io.sentry.SentryLevel

// will be tagged with my-tag="my value"
Sentry.captureException(Exception("my error")) { scope ->
scope.setTag("my-tag", "my value")
scope.level = SentryLevel.WARNING
}

// will not be tagged with my-tag
Sentry.captureException(Exception("my error"))
```

</PlatformSection>

<PlatformSection notSupported={["android"]}>

The JavaSDK provides two alternative ways of configuring a local scope.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The JavaSDK provides two alternative ways of configuring a local scope.
The Java SDK provides two alternative ways of configuring a local scope.


```java {tabTitle: Java}
import io.sentry.Sentry;
import io.sentry.SentryLevel;

// will be tagged with my-tag="my value"
Sentry.captureException(new Exception("my error"), scope -> {
scope.setTag("my-tag", "my value");
scope.setLevel(SentryLevel.WARNING);
});

// will not be tagged with my-tag
Sentry.captureException(new Exception("my error"));
```

```java {tabTitle: Java-Alternative}
import io.sentry.Sentry;
import io.sentry.SentryLevel;

Sentry.withScope(scope -> {
scope.setLevel(SentryLevel.FATAL);
scope.setTransaction("main");
scope.setTag("my-tag", "my value");
scope.setLevel(SentryLevel.WARNING);

// This message includes the dataset to the scope in this block:
Sentry.captureMessage("Fatal message!");
// will be tagged with my-tag="my value"
Sentry.captureException(new Exception("my error"));
});

// will not be tagged with my-tag
Sentry.captureException(new Exception("my error"));
```

```kotlin {tabTitle: Kotlin}
import io.sentry.Sentry
import io.sentry.SentryLevel

// will be tagged with my-tag="my value"
Sentry.captureException(Exception("my error")) { scope ->
scope.setTag("my-tag", "my value")
scope.level = SentryLevel.WARNING
}

// will not be tagged with my-tag
Sentry.captureException(Exception("my error"))
```

```kotlin {tabTitle: Kotlin-Alternative}
import io.sentry.Sentry
import io.sentry.SentryLevel

Sentry.withScope { scope ->
scope.level = SentryLevel.FATAL
scope.setTransaction("main")
scope.setTag("my-tag", "my value")
scope.level = SentryLevel.WARNING

// This message includes the dataset to the scope in this block:
Sentry.captureMessage("Fatal message!")
// will be tagged with my-tag="my value"
Sentry.captureException(Exception("my error"))
}

// will not be tagged with my-tag
Sentry.captureException(Exception("my error"))
```
</PlatformSection>
2 changes: 1 addition & 1 deletion src/platforms/common/enriching-events/scopes.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Java we could also show an alert that the withScope variant doesn't play well with globalHubMode

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


## Local Scopes

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

https://github.com/getsentry/sentry-docs/pull/5136/files#diff-1b74f2288f1f938f1b7595fe4986f66d21eb90be88c3670334e42ca2520746d1R117-R122

Copy link
Collaborator Author

@lbloder lbloder Jun 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try to clarify

Copy link
Member

@adinauer adinauer Jun 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Expand Down