Skip to content

[Profiling] Don't group by address if executable is known but function is unknown #129498

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

Merged
merged 3 commits into from
Jun 18, 2025

Conversation

rockdaboot
Copy link
Contributor

This PR ignores the address or line number when grouping frames with unknown function name but known executable name.

Fully symbolized frames are not affected at all.
This change only affects how unsymbolized frames are grouped / aggregated.

For example, an unsymbolized lib.so.6 sill appear as a single entry in the TopN functions view, and no longer as hundreds of different entries.

In the flamegraph, this changed aggregation of frames leads to less complexity.
Here are screenshots that show the change as it is otherwise hard to phrase and understand.

Before
Screenshot_20250616_161518

After
Screenshot_20250616_161656

@rockdaboot rockdaboot self-assigned this Jun 16, 2025
@rockdaboot rockdaboot added >non-issue :UniversalProfiling/Application Elastic Universal Profiling REST APIs and infrastructure v9.1.0 labels Jun 16, 2025
@elasticsearchmachine elasticsearchmachine added the Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team label Jun 16, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

@elasticsearchmachine elasticsearchmachine added the external-contributor Pull request authored by a developer outside the Elasticsearch team label Jun 16, 2025
Copy link
Member

@christos68k christos68k left a comment

Choose a reason for hiding this comment

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

For an expert, this can also be seen as misleading as it hides the separation of different callers (even if the caller information is unknown, the fact that separate callers are shown can be useful in itself). But given our target audience and that it doesn't affect the relative order of CPU usage across executables and thread names, I think it's OK to introduce a simplification such as this and iterate.

…xecutable

This reduces the complexity of the flamegraph in unsymbolized areas.
Symbolized areas of the flamegraph are not affected.
@rockdaboot rockdaboot force-pushed the flamegraph-grouping branch from 0db2c1b to 596d993 Compare June 17, 2025 10:12
@rockdaboot rockdaboot requested a review from florianl June 17, 2025 10:12
@rockdaboot
Copy link
Contributor Author

rockdaboot commented Jun 17, 2025

For an expert, this can also be seen as misleading as it hides the separation of different callers

As you say, our target audience isn't the expert who takes the addresses and puts them in Ghidra or IDA to make use of it. But also, if enough customers ask for more details, we can add a setting to turn the details on again.

@rockdaboot rockdaboot merged commit 5e475f2 into elastic:main Jun 18, 2025
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external-contributor Pull request authored by a developer outside the Elasticsearch team >non-issue Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team :UniversalProfiling/Application Elastic Universal Profiling REST APIs and infrastructure v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants