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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Mark stacktrace as snapshot if captured at arbitrary moment #1231

Merged
merged 8 commits into from
Feb 15, 2021

Conversation

marandaneto
Copy link
Contributor

@marandaneto marandaneto commented Feb 4, 2021

馃摐 Description

Fix: Mark stacktrace as snapshot if captured at arbitrary moment

馃挕 Motivation and Context

ANR stack traces and the thread list attach a stack trace captured at an arbitrary moment and this could affect grouping, this flag will be used for better grouping, maybe using hierarchical grouping or something else on the server-side.

馃挌 How did you test it?

馃摑 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • No breaking changes

馃敭 Next steps

@marandaneto marandaneto marked this pull request as ready for review February 8, 2021 12:11
@marandaneto marandaneto requested a review from a team February 8, 2021 12:11
@marandaneto
Copy link
Contributor Author

marandaneto commented Feb 8, 2021

@jan-auer added you as a reviewer just to be aware that we are adding it to Android/Java

@bruno-garcia
Copy link
Member

@jan-auer is it correct to assume that snapshot stacktraces will be used for grouping based on the top-most frames?

This is what we're aiming for ANR and if that's not what snapshot is for, I'm afraid it won't be helpful to add this at all.

@jan-auer
Copy link
Member

is it correct to assume that snapshot stacktraces will be used for grouping based on the top-most frames?

@bruno-garcia Since snapshot stack traces are captured at an indeterministic time (based on external influence), it is likely that the top frame is random just like the rest of the stack trace. I'm afraid that grouping by the top frame wouldn't achieve better results in that case. If there are other indicators like the error type or a signal, we could group by that.

@marandaneto
Copy link
Contributor Author

is it correct to assume that snapshot stacktraces will be used for grouping based on the top-most frames?

@bruno-garcia Since snapshot stack traces are captured at an indeterministic time (based on external influence), it is likely that the top frame is random just like the rest of the stack trace. I'm afraid that grouping by the top frame wouldn't achieve better results in that case. If there are other indicators like the error type or a signal, we could group by that.

mechanism.type=ANR for ANR events.
@jan-auer when you say the top frame do you mean where it started or ended? I ask that because we revert the order of the frames before sending it right, so I am confused here.

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

LGTM, just made a few minor comments.

@jan-auer
Copy link
Member

@jan-auer when you say the top frame do you mean where it started or ended? I ask that because we revert the order of the frames before sending it right, so I am confused here.

@marandaneto Good point, this is ambiguous terminology. With "top" I always mean the inner-most frame.

@marandaneto marandaneto merged commit f205621 into main Feb 15, 2021
@marandaneto marandaneto deleted the fix/snapshot-stacktrace branch February 15, 2021 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants