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

flamegraph_arrow: Inject artifical label node #3516

Merged
merged 3 commits into from
Aug 9, 2023

Conversation

metalmatze
Copy link
Member

@metalmatze metalmatze commented Jul 27, 2023

On the backend, we check every sample for its pprof labels.
If it has pprof labels we either inject a artifical node into the stack containing the pprof labels as JSON for the function name, or we merge the stack into the already existing same pprof label node.

In the frontend, instead of showing {"thread_name":"foo"} we're going to parse the JSON and display it as thread_name="foo" in a Prometheus fashion. Given that each injected pprof label node is unique the drilling down into the icicle graph by function name now works flawlessly again. 👍

Screenshot from 2023-07-27 20-12-51

@metalmatze metalmatze requested review from a team as code owners July 27, 2023 18:24
@alwaysmeticulous
Copy link

alwaysmeticulous bot commented Jul 27, 2023

🤖 Meticulous replayed 50 user sessions and took 214 screenshots. Meticulous has not yet run on 77528a2 of the main branch and so there was nothing to compare against.

If you recently setup Meticulous, this is expected. Meticulous will start reporting comparisons for new pull requests after the next commit to the main branch.

Last updated for commit f4ff76b. This comment will update as new commits are pushed.

// Add this label row to the root row's children.
fb.children[0] = append(fb.children[0], row)
//// Add the next row as child of this label row.
//fb.children[row] = append(fb.children[row], row+1)
Copy link
Member

Choose a reason for hiding this comment

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

leftover?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think this has been resolved?

When grouping by pprof labels we want to inject a node beneath the root into the flamegraph that contains the name of that label. This will make finding the correct stacks by pprof labels easier.
Instead of showing ``{"thread_name":"foo"}` we're going to parse the JSON and display it as `thread_name="foo"` in a Prometheus fashion.
@metalmatze metalmatze force-pushed the flamegraph-arrow-labels-grouping branch from df84982 to f4ff76b Compare August 9, 2023 10:40
@metalmatze metalmatze merged commit bde1efe into main Aug 9, 2023
@metalmatze metalmatze deleted the flamegraph-arrow-labels-grouping branch August 9, 2023 11:49
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.

2 participants