Skip to content

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
kderusso pushed a commit to kderusso/elasticsearch that referenced this pull request Jun 23, 2025
…n is unknown (elastic#129498)

* [Profiling] Omit address in grouping for unknown function but known executable

This reduces the complexity of the flamegraph in unsymbolized areas.
Symbolized areas of the flamegraph are not affected.

* Fix tests

* Fix IT tests
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