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

feat(anr): Improve ANR root cause analysis #49673

Merged
merged 5 commits into from
Jun 2, 2023
Merged

Conversation

romtsn
Copy link
Member

@romtsn romtsn commented May 24, 2023

The PR includes 3 main changes:

  • Improves resources (makes them more informative and user-friendly) and adds new functions and thread states to the ANR root cause detection
  • The frames are iterated in reverse order to ensure that we surface the most relevant root cause (the topmost frame is probably the most relevant one)
  • Introduces a new UI change to highlight the Suspect Frames using the Tag component. When the Tag is clicked it automatically scrolls to the Suspect Root Cause section.

@romtsn romtsn requested review from wmak and shruthilayaj May 24, 2023 09:31
@romtsn romtsn requested a review from a team as a code owner May 24, 2023 09:31
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 24, 2023
@romtsn romtsn mentioned this pull request May 24, 2023
13 tasks

for (let index = 0; index < exceptionFrames.length; index++) {
// iterating the frames in reverse order, because the topmost frames most like the root cause
for (let index = exceptionFrames.length - 1; index > 0; index--) {
Copy link
Member

Choose a reason for hiding this comment

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

What's the max number of frames here? If it's really low we don't really need an early exit by reverse walking this array. Up to you though.

Copy link
Member Author

Choose a reason for hiding this comment

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

we need the early exit here in case there are multiple offending frames. The UI is only capable of rendering a single root cause and we're picking the one that is the closest to the exception root cause, that's why we reverse it. Unless I misunderstood your comment :)

Copy link
Member

Choose a reason for hiding this comment

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

Ah there are multiple, nevermind 👍

static/app/components/events/interfaces/analyzeFrames.tsx Outdated Show resolved Hide resolved
static/app/components/events/interfaces/analyzeFrames.tsx Outdated Show resolved Hide resolved
@romtsn romtsn enabled auto-merge (squash) June 2, 2023 09:47
@romtsn romtsn merged commit 6b2f7ed into master Jun 2, 2023
@romtsn romtsn deleted the rz/feat/anr-root-cause-enh branch June 2, 2023 10:05
@romtsn
Copy link
Member Author

romtsn commented Jun 2, 2023

@k-fish I've merged this, lemme know if I misunderstood and should follow up with some changes. Thank you

@github-actions github-actions bot locked and limited conversation to collaborators Jul 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants