Skip to content

Conversation

@roaga
Copy link
Member

@roaga roaga commented Nov 19, 2025

Instead of just selecting the thread from the first sample, we now prioritize the thread with the most in-app frames. And if no in-app frames are present, we show system frames.

@roaga roaga requested a review from aliu39 November 19, 2025 17:34
@roaga roaga requested a review from a team as a code owner November 19, 2025 17:34
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 19, 2025
@codecov
Copy link

codecov bot commented Nov 19, 2025

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
29877 1 29876 244
View the top 1 failed test(s) by shortest run time
tests.sentry.seer.explorer.test_explorer_utils.TestConvertProfileToExecutionTree::test_convert_profile_selects_thread_with_most_in_app_frames
Stack Traces | 0.043s run time
#x1B[1m#x1B[.../seer/explorer/test_explorer_utils.py#x1B[0m:618: in test_convert_profile_selects_thread_with_most_in_app_frames
    assert len(root.children) == 2
#x1B[1m#x1B[31mE   AssertionError: assert 1 == 2#x1B[0m
#x1B[1m#x1B[31mE    +  where 1 = len([ExecutionTreeNode(function='worker_function_2', module='app', filename='worker.py', lineno=30, in_app=True, children=...ker.py:40/worker_function_2:worker.py:30', sample_count=1, first_seen_ns=1000000, last_seen_ns=1000000, duration_ns=0)])#x1B[0m
#x1B[1m#x1B[31mE    +    where [ExecutionTreeNode(function='worker_function_2', module='app', filename='worker.py', lineno=30, in_app=True, children=...ker.py:40/worker_function_2:worker.py:30', sample_count=1, first_seen_ns=1000000, last_seen_ns=1000000, duration_ns=0)] = ExecutionTreeNode(function='worker_function_3', module='app', filename='worker.py', lineno=40, in_app=True, children=[... node_id='/worker_function_3:worker.py:40', sample_count=1, first_seen_ns=1000000, last_seen_ns=1000000, duration_ns=0).children#x1B[0m

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

)
show_all_frames = (
True # Show all frames including system frames when no in_app frames exist
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: MainThread selection ignores whether thread has samples

When no threads have in_app frames, the code attempts to select MainThread from metadata without verifying it actually contains any samples. If MainThread exists in metadata but never appears in the samples data, selected_thread_id will be set to a thread that has no profiling data, causing all samples to be skipped and an empty execution tree to be returned instead of falling back to the first sample's thread.

Fix in Cursor Fix in Web

Copy link
Member

@JoshFerge JoshFerge left a comment

Choose a reason for hiding this comment

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

curious what the logic is on issue details for this, but makes sense to me

@roaga
Copy link
Member Author

roaga commented Nov 20, 2025

curious what the logic is on issue details for this, but makes sense to me

From dogfooding in the UI, it doesn't seem like there is any consistent logic besides selecting the first available thread, which isn't a great UX...

@roaga roaga merged commit 20324a5 into master Nov 20, 2025
66 checks passed
@roaga roaga deleted the explorer/fix-profile-thread-selection-2 branch November 20, 2025 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants