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.

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

Grouping code should take stack trace on Threads into account #23559

Open
bruno-garcia opened this issue Feb 2, 2021 · 5 comments
Open

Grouping code should take stack trace on Threads into account #23559

bruno-garcia opened this issue Feb 2, 2021 · 5 comments
Assignees

Comments

@bruno-garcia
Copy link
Member

bruno-garcia commented Feb 2, 2021

Currently, grouping doesn't consider stack trace of threads, falling back to message. An example, in Cocoa.

This behavior was seeing at least when no thread was marked at crashed.

sentry-cocoa to mark a frame as crashed.

@marandaneto
Copy link
Contributor

marandaneto commented Feb 3, 2021

it's not current, it's the crashed field.

current=the main/UI thread - shows was active: yes/no) in the UI
crashed=the crashed thread (calling thread, it does not mean there's an exception) - shows errored: yes/no in the UI

I've updated the issues desc and title

@bruno-garcia
Copy link
Member Author

This is for captureMessage with attachStacktrace. If there's a crashed thread, we have an exception too which is already used for grouping.

@bruno-garcia
Copy link
Member Author

bruno-garcia commented Feb 3, 2021

current:

Optional. A flag indicating whether the thread was in the foreground. Defaults to false.

crashed:

Optional. A flag indicating whether the thread crashed. Defaults to false.

https://develop.sentry.dev/sdk/event-payloads/threads/#attributes

The UI shows was active for current=true.

@marandaneto
Copy link
Contributor

marandaneto commented Mar 15, 2021

after a discussion and talking to some other folks, that's how it should be:

current: if the thread was scheduled/running or not, as the docs say: A flag indicating whether the thread was in the foreground

crashed: if it was the thread that captured the event, either a message or exception, it's just a bad/legacy naming, it does not mean that this thread threw an exception or crashed the App.

there's no JSON field for UI/Main thread, we could consider adding a tag for that or introducing a new JSON field, a new JSON field would require a UI label though.

if that's the correct assumption, current should be fixed for the Android SDK.

@lobsterkatie
Copy link
Member

@marandaneto @bruno-garcia Is this still an issue? And if so, is it a problem with our grouping logic or with how the data is getting annotated by the SDK? (Sounds like the latter?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants