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.

Sign up for GitHub

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

Jump to bottom

[APM] Profiling #91818

Merged
merged 18 commits into from
Feb 22, 2021
Merged

[APM] Profiling #91818

merged 18 commits into from
Feb 22, 2021

Conversation

dgieselaar
Copy link
Member

@dgieselaar dgieselaar commented Feb 18, 2021 •
edited
Loading

Adds a (very experimental) profiling tab to a service. This won't ship to the bigger public any time soon, and we should consider it as experimental/internal only.

Currently only the go agent supports profiling. Enabling self-instrumentation for APM Server and setting profiling.cpu.enabled to true should work.

For more background, see:
elastic/apm-server#2839
elastic/apm-server#4017

This PR builds on the work by @axw and uses the new flamegraph layout from Kibana (thank you @monfera!)

To enable it in Kibana, add the following flag to kibana.yml:

xpack.apm.profilingEnabled: true

This will display a "Profiling" tab when the user navigates to a service.

The following value types are supported:

  • On-CPU
  • Wall time
  • Sample count
  • Allocated objects
  • Allocated space
  • In-use objects
  • In-use space

Currently, the Profiling feature has the following capabilities:

  • A timeline displaying counts of profiles containing data for supported value types
  • A flamegraph (provided by @elastic/charts) of the currently selected value type
  • A table, sorting the frames by self value
  • Collapsing irrelevant frames (e.g., one child, no self time)
  • Frame highlighting
  • Filtering profile data by using the query bar

Some things to keep in mind:

  • Up to 10k unique stacks are supported. If there's more than that, the data might not be accurate.
  • I have not yet verified whether the memory value types are correct. I hope to have time for that at some point later.
  • There are some performance issues around serializing/parsing JSON (both between Kibana and ES and browser and Kibana), and drawing the chart. Things might be slow or even crash the browser tab.
  • The screenshots below use a modified version of @elastic/charts that enables label clipping and fixes some text alignment issues. Work is ongoing to merge that back into the upstream repo: feat(partition): clip text in partition chart fill label elastic-charts#1033

Default view:

image

Filtered:

image

@dgieselaar dgieselaar added release_note:enhancement Team:APM All issues that need APM UI Team support v7.13.0 labels Feb 18, 2021
@dgieselaar dgieselaar added release_note:skip Skip the PR/issue when compiling release notes and removed release_note:enhancement labels Feb 19, 2021
@dgieselaar dgieselaar marked this pull request as ready for review February 19, 2021 10:05
@dgieselaar dgieselaar requested a review from a team as a code owner February 19, 2021 10:05
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

@dgieselaar dgieselaar added the auto-backport Deprecated - use backport:version if exact versions are needed label Feb 19, 2021
smith
smith approved these changes Feb 19, 2021
View reviewed changes
Copy link
Contributor

@smith smith left a comment

Choose a reason for hiding this comment

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

So hot

x-pack/plugins/apm/public/components/app/service_details/service_detail_tabs.tsx Outdated
}
)}
tooltipContent={i18n.translate(
'xpack.apm.serviceDetails.profilingTabExperimentalDedcription',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'xpack.apm.serviceDetails.profilingTabExperimentalDedcription',
'xpack.apm.serviceDetails.profilingTabExperimentalDescription',

x-pack/plugins/apm/server/lib/services/profiling/get_service_profiling_statistics.ts Outdated
filter: ESFilter[];
valueTypeField: string;
}) {
return withApmSpan('get_profile_stats', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we name these the same as the function that's calling them?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've changed the name slightly. In general that's a good approach, but we have to keep in mind that these span names should be unique and preferably recognisable across all custom spans, so it's easier for us to understand them at a glance.

x-pack/plugins/apm/server/routes/services.ts
query: t.intersection([
rangeRt,
uiFiltersRt,
t.partial({
Copy link
Contributor

Choose a reason for hiding this comment

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

There's an environmentRt that does the same thing.

Copy link
Member Author

Choose a reason for hiding this comment

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

nice 👍

@axw
Copy link
Member

axw commented Feb 20, 2021 •
edited
Loading

Doesn't need to be in this PR, if at all, but for filtering I think it might be better to highlight the whole stack for each stack that involves matching frames. Otherwise if there's a matching frame that is towards the end of a stack, and has a very small value, it may be difficult or impossible to see.

FWIW, that's how the pprof UI's "filter" function works. Well actually no, it's not: the pprof UI hides everything else and shows the stacks that involve the matching frames. Nearly the same I guess.

@dgieselaar
Copy link
Member Author

@axw I've added a comment as it's not a super easy fix (it's doable though): https://github.com/elastic/kibana/pull/91818/files#diff-52c51863858e4d9874706ef7ff7e775baec68dc9c67ecba0ad1b5a76c6a178d7R272.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
apm 1736 1751 +15

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
apm 5.2MB 5.3MB +33.8KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@dgieselaar dgieselaar merged commit e772488 into elastic:master Feb 22, 2021
@dgieselaar dgieselaar deleted the profiling-poc branch February 22, 2021 18:02
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Feb 22, 2021
@dgieselaar @kibanamachine
[APM] Profiling (elastic#91818)
e0eff5d
@kibanamachine kibanamachine mentioned this pull request Feb 22, 2021
Merged
@kibanamachine
Copy link
Contributor

💚 Backport successful

✅ 7.x / #92250

Successful backport PRs will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Feb 22, 2021
@kibanamachine @dgieselaar
[APM] Profiling (#91818) (#92250)
22b3f21 
Co-authored-by: Dario Gieselaar <[email protected]>
@trentm trentm mentioned this pull request May 27, 2021
Closed
@sorenlouv sorenlouv mentioned this pull request Aug 29, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smith smith smith approved these changes

Assignees
No one assigned
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support v7.13.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@dgieselaar @elasticmachine @axw @kibanamachine @smith