Skip to content

Conversation

@rockdaboot
Copy link
Contributor

@rockdaboot rockdaboot commented Dec 19, 2024

This PR adds aggregation by process name and by thread name to the Universal Profiling flamegraph view.

The profiling data model has been updated to fit for the new task, so some non-trivial amount of code had to be changed.

TBD: fix and improve tests

This PR works best together with this Kibana PR.

Before

Screenshot_20241219_180743

After

Screenshot_20241219_180530

@rockdaboot rockdaboot added >enhancement :UniversalProfiling/Application Elastic Universal Profiling REST APIs and infrastructure Team:obs-knowledge Meta label for Observability Knowledge team v9.0.0 labels Dec 19, 2024
@rockdaboot rockdaboot self-assigned this Dec 19, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/obs-knowledge-team (Team:obs-knowledge)

@elasticsearchmachine elasticsearchmachine added Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team external-contributor Pull request authored by a developer outside the Elasticsearch team labels Dec 19, 2024
@elasticsearchmachine
Copy link
Collaborator

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

@rockdaboot rockdaboot removed the external-contributor Pull request authored by a developer outside the Elasticsearch team label Dec 19, 2024
@rockdaboot rockdaboot force-pushed the flamegraph-executable-name branch 2 times, most recently from f9ecda3 to df54aca Compare January 2, 2025 17:29
@rockdaboot rockdaboot force-pushed the flamegraph-executable-name branch from df81709 to 52d909b Compare January 4, 2025 12:55
Copy link
Member

@danielmitterdorfer danielmitterdorfer left a comment

Choose a reason for hiding this comment

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

We had an offline conversation about the open questions, which are all resolved now. LGTM

viduni94 pushed a commit to viduni94/kibana that referenced this pull request Jan 23, 2025
…, root) (elastic#204977)

## Summary
This PR is a pre-requisite for adding aggregation by process name and by
thread name to the Universal Profiling flamegraph view.

It adds three artificial node types to the flamegraph including color
codes.

As a side-effect, the root node now has its own color code. Previously,
it (accidentally) used the color code of "unknown" type frames.

The PR is backwards compatible, so it doesn't change anything in the UI
when connecting with current Elasticsearch.
As soon as [the PR for
ES](elastic/elasticsearch#119115) is merged, the
new aggregations show up.
MiriamAparicio added a commit to elastic/kibana that referenced this pull request Feb 8, 2025
…pe is root (#209978)

Closes elastic/prodfiler#4386

### Summary

The flyout for the root block always contains the hint "Missing symbols
error" because there are no symbols for the root block. We should not
show it here.

### What was done

Added a condition to not render the callout when frameType is root

Before


![image](https://github.com/user-attachments/assets/4856bf40-6753-46bd-a20d-74eebf27533f)

After

Waiting for [this
PR](elastic/elasticsearch#119115) to be merged
for the condition to work
rockdaboot added a commit to elastic/kibana that referenced this pull request Mar 24, 2025
Adds a type and color for `Executable` flamegraph nodes.

The new type will be used by [this
change](elastic/elasticsearch#119115) in the ES
profiling plugin (the required code change from grouping by process name
to grouping by executable name needs tbd).
@rockdaboot rockdaboot merged commit 4fcce51 into elastic:main Mar 24, 2025
17 checks passed
JoseLuisGJ pushed a commit to JoseLuisGJ/kibana that referenced this pull request Mar 24, 2025
Adds a type and color for `Executable` flamegraph nodes.

The new type will be used by [this
change](elastic/elasticsearch#119115) in the ES
profiling plugin (the required code change from grouping by process name
to grouping by executable name needs tbd).
omricohenn pushed a commit to omricohenn/elasticsearch that referenced this pull request Mar 28, 2025
…stic#119115)

* Add field process.executable.name to profiling-events

* Amend query to aggregate events by executable name

* Send flamegraph row with grouping by executable name

* Flamegraph sub-aggregation by thread name

* Rework internal data model

* Cleanups

* Fix building tests

* Fix GetStackTracesResponseTests

* Fix unit tests

* Fix remaining unit tests

* [CI] Auto commit changes from spotless

* Fix flamegraph yaml tests

* Fix yaml REST tests

* Increase INDEX_TEMPLATE_VERSION for profiling.executable.name

* Fix yamlRestCompatTest

Co-authored-by: Jake Landis <[email protected]>

* Rename executable name to process name

* Remove warnings meant for testing

* Replace ChunkedToXContentHelper.wrapWithObject() with .object()

* Fix comment in ProfilingIndexTemplateRegistry.java

* Simplify sorting of unique stacktrace and host IDs

* [CI] Auto commit changes from spotless

* Add 'missing()' to aggregations

* Fix syntax error after resolving merge conflicts

* Revert "Rename executable name to process name"

This reverts commit e514874.

* Set FRAMETYPE_EXECUTABLE to 0x103

* Fix TransportGetFlamegraphActionTests

---------

Co-authored-by: elasticsearchmachine <[email protected]>
Co-authored-by: Jake Landis <[email protected]>
cqliu1 pushed a commit to cqliu1/kibana that referenced this pull request Mar 31, 2025
Adds a type and color for `Executable` flamegraph nodes.

The new type will be used by [this
change](elastic/elasticsearch#119115) in the ES
profiling plugin (the required code change from grouping by process name
to grouping by executable name needs tbd).
cauemarcondes pushed a commit to cauemarcondes/kibana that referenced this pull request May 2, 2025
…pe is root (elastic#209978)

Closes elastic/prodfiler#4386

### Summary

The flyout for the root block always contains the hint "Missing symbols
error" because there are no symbols for the root block. We should not
show it here.

### What was done

Added a condition to not render the callout when frameType is root

Before

![image](https://github.com/user-attachments/assets/4856bf40-6753-46bd-a20d-74eebf27533f)

After

Waiting for [this
PR](elastic/elasticsearch#119115) to be merged
for the condition to work

(cherry picked from commit 9e65d7a)
cauemarcondes pushed a commit to cauemarcondes/kibana that referenced this pull request May 2, 2025
…, root) (elastic#204977)

## Summary
This PR is a pre-requisite for adding aggregation by process name and by
thread name to the Universal Profiling flamegraph view.

It adds three artificial node types to the flamegraph including color
codes.

As a side-effect, the root node now has its own color code. Previously,
it (accidentally) used the color code of "unknown" type frames.

The PR is backwards compatible, so it doesn't change anything in the UI
when connecting with current Elasticsearch.
As soon as [the PR for
ES](elastic/elasticsearch#119115) is merged, the
new aggregations show up.

(cherry picked from commit bc5d8db)
cauemarcondes added a commit to elastic/kibana that referenced this pull request May 6, 2025
…ad name, root) (#204977) (#219979)

# Backport

This will backport the following commits from `main` to `8.19`:
- [[Profiling] Add colors for new frame types (process name, thread
name, root) (#204977)](#204977)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Tim
Rühsen","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-08T09:24:43Z","message":"[Profiling]
Add colors for new frame types (process name, thread name, root)
(#204977)\n\n## Summary\r\nThis PR is a pre-requisite for adding
aggregation by process name and by\r\nthread name to the Universal
Profiling flamegraph view.\r\n\r\nIt adds three artificial node types to
the flamegraph including color\r\ncodes.\r\n\r\nAs a side-effect, the
root node now has its own color code. Previously,\r\nit (accidentally)
used the color code of \"unknown\" type frames.\r\n\r\nThe PR is
backwards compatible, so it doesn't change anything in the UI\r\nwhen
connecting with current Elasticsearch.\r\nAs soon as [the PR
for\r\nES](elastic/elasticsearch#119115) is
merged, the\r\nnew aggregations show
up.","sha":"bc5d8db237bdef481de9e4ce37d9e460755ae418","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","release_note:skip","backport:skip","v9.0.0","ci:project-deploy-observability","Team:obs-ux-infra_services"],"title":"[Profiling]
Add colors for new frame types (process name, thread name,
root)","number":204977,"url":"https://github.com/elastic/kibana/pull/204977","mergeCommit":{"message":"[Profiling]
Add colors for new frame types (process name, thread name, root)
(#204977)\n\n## Summary\r\nThis PR is a pre-requisite for adding
aggregation by process name and by\r\nthread name to the Universal
Profiling flamegraph view.\r\n\r\nIt adds three artificial node types to
the flamegraph including color\r\ncodes.\r\n\r\nAs a side-effect, the
root node now has its own color code. Previously,\r\nit (accidentally)
used the color code of \"unknown\" type frames.\r\n\r\nThe PR is
backwards compatible, so it doesn't change anything in the UI\r\nwhen
connecting with current Elasticsearch.\r\nAs soon as [the PR
for\r\nES](elastic/elasticsearch#119115) is
merged, the\r\nnew aggregations show
up.","sha":"bc5d8db237bdef481de9e4ce37d9e460755ae418"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/204977","number":204977,"mergeCommit":{"message":"[Profiling]
Add colors for new frame types (process name, thread name, root)
(#204977)\n\n## Summary\r\nThis PR is a pre-requisite for adding
aggregation by process name and by\r\nthread name to the Universal
Profiling flamegraph view.\r\n\r\nIt adds three artificial node types to
the flamegraph including color\r\ncodes.\r\n\r\nAs a side-effect, the
root node now has its own color code. Previously,\r\nit (accidentally)
used the color code of \"unknown\" type frames.\r\n\r\nThe PR is
backwards compatible, so it doesn't change anything in the UI\r\nwhen
connecting with current Elasticsearch.\r\nAs soon as [the PR
for\r\nES](elastic/elasticsearch#119115) is
merged, the\r\nnew aggregations show
up.","sha":"bc5d8db237bdef481de9e4ce37d9e460755ae418"}}]}] BACKPORT-->

Co-authored-by: Tim Rühsen <[email protected]>
cauemarcondes added a commit to elastic/kibana that referenced this pull request May 8, 2025
…rame type is root (#209978) (#219964)

# Backport

This will backport the following commits from `main` to `8.19`:
- [[ObsUX][Profiling] Don't render missing symbols callout when frame
type is root (#209978)](#209978)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT
[{"author":{"name":"Miriam","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-02-08T08:31:25Z","message":"[ObsUX][Profiling]
Don't render missing symbols callout when frame type is root
(#209978)\n\nCloses
https://github.com/elastic/prodfiler/issues/4386\r\n\r\n###
Summary\r\n\r\nThe flyout for the root block always contains the hint
\"Missing symbols\r\nerror\" because there are no symbols for the root
block. We should not\r\nshow it here.\r\n\r\n### What was
done\r\n\r\nAdded a condition to not render the callout when frameType
is
root\r\n\r\nBefore\r\n\r\n\r\n![image](https://github.com/user-attachments/assets/4856bf40-6753-46bd-a20d-74eebf27533f)\r\n\r\nAfter\r\n\r\nWaiting
for [this\r\nPR](elastic/elasticsearch#119115)
to be merged\r\nfor the condition to
work","sha":"9e65d7a18398877f2721b9b86c69e5fd205e190b","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","backport:skip","Team:obs-ux-infra_services","v9.1.0"],"title":"[ObsUX][Profiling]
Don't render missing symbols callout when frame type is
root","number":209978,"url":"https://github.com/elastic/kibana/pull/209978","mergeCommit":{"message":"[ObsUX][Profiling]
Don't render missing symbols callout when frame type is root
(#209978)\n\nCloses
https://github.com/elastic/prodfiler/issues/4386\r\n\r\n###
Summary\r\n\r\nThe flyout for the root block always contains the hint
\"Missing symbols\r\nerror\" because there are no symbols for the root
block. We should not\r\nshow it here.\r\n\r\n### What was
done\r\n\r\nAdded a condition to not render the callout when frameType
is
root\r\n\r\nBefore\r\n\r\n\r\n![image](https://github.com/user-attachments/assets/4856bf40-6753-46bd-a20d-74eebf27533f)\r\n\r\nAfter\r\n\r\nWaiting
for [this\r\nPR](elastic/elasticsearch#119115)
to be merged\r\nfor the condition to
work","sha":"9e65d7a18398877f2721b9b86c69e5fd205e190b"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/209978","number":209978,"mergeCommit":{"message":"[ObsUX][Profiling]
Don't render missing symbols callout when frame type is root
(#209978)\n\nCloses
https://github.com/elastic/prodfiler/issues/4386\r\n\r\n###
Summary\r\n\r\nThe flyout for the root block always contains the hint
\"Missing symbols\r\nerror\" because there are no symbols for the root
block. We should not\r\nshow it here.\r\n\r\n### What was
done\r\n\r\nAdded a condition to not render the callout when frameType
is
root\r\n\r\nBefore\r\n\r\n\r\n![image](https://github.com/user-attachments/assets/4856bf40-6753-46bd-a20d-74eebf27533f)\r\n\r\nAfter\r\n\r\nWaiting
for [this\r\nPR](elastic/elasticsearch#119115)
to be merged\r\nfor the condition to
work","sha":"9e65d7a18398877f2721b9b86c69e5fd205e190b"}}]}] BACKPORT-->

Co-authored-by: Miriam <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>enhancement 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.

5 participants