[ObsUX] Add Profiling tab to Asset Details by mykolaharmash · Pull Request #171764 · elastic/kibana

mykolaharmash · 2023-11-22T16:00:50Z

Summary

This adds a new "Universal Profiling" tab to asset details with a flamegrapth for a selected host. The tab is behind a feature flag and is disabled by default. It will be enabled by default for clound/onprem once we implement Profiling empty state, serverless is tbd.

Added two new endpoints for fetching profiling status (not used by the FE yet) and flamegraph data.
Added a profilingEnabled feature flag
Added a new tab in the UI and integrated the Flamegraph embeddable

How to test

Connect local kibana to oblt cluster that has Profiling configured (e.g. edge)
Add this to your dev kibana.yml

xpack.profiling.enabled: true
xpack.infra.profilingEnabled: true

# Direct ES URL on the oblt cluster that you're using, in case of edge it's https://edge-oblt.es.us-west2.gcp.elastic-cloud.com:443
xpack.profiling.elasticsearch.hosts: REMOTE_CLUSTER_ES_URL

# If needed create a new user on the remote oblt cluster
xpack.profiling.elasticsearch.username: REMOTE_CLUSTER_USER
xpack.profiling.elasticsearch.password: REMOTE_CLUSTER_PASWORD

Open kibana, go to Hosts
Open a flyout for one of the hosts and make sure you see the Profiling tab with a flamegraph
Open Host details as a full page and also make sure you see the new tab

ghost · 2023-11-22T16:01:06Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
/oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

elasticmachine · 2023-11-23T10:10:15Z

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

mykolaharmash · 2023-11-23T10:15:26Z

I left out API tests for now as I could not generate a good profiling data archive to test against. Will try to figure it out in the upcoming changes.

…fix'

crespocarlos · 2023-11-23T10:40:32Z

x-pack/plugins/infra/public/components/asset_details/tabs/profiling/profiling.tsx

+  const { loading, response, makeRequest } = useHTTPRequest<BaseFlameGraph>(
+    '/api/infra/profiling/flamegraph',
+    'POST',
+    JSON.stringify(InfraProfilingRequestParamsRT.encode(body)),
+    undefined,
+    undefined,
+    undefined,
+    true
+  );
+
+  useEffect(() => {
+    makeRequest();
+  }, [makeRequest]);


Disclaimer: I haven't tested the PR yet, so everything I'm going to write here might not make sense because your implementation could be working as expected.

That said, here are just some initial thoughts regarding this component and suggestions:

1 - In the asset details flyout/full page, to prevent changes in the date picker from triggering requests in tabs that are not currently visible, we use the useIntersectingState hook. With this hook, you can delay component re-rendering to happen only once the component becomes visible.

2 - If Auto-Refresh is enabled, two things could happen: a never-ending loading loop and performance issues. If ongoing requests are not properly canceled between refresh cycles, the server will be busy with unnecessary requests.

To fix this, you could use the useRequestObservable() hook. This observable is responsible for determining when a new request can be made if auto-refresh is enabled. Here is an example of usage.

Feel free to reach out if you need any help, and sorry for the long comment :)

Thank you @crespocarlos! Those are very good suggestions 🙌 Tbh I have not put much work into the FE component yet and was planning to do this with my next change while adding the Top Functions embeddable, at that point this component will become more complex and I'll have a better idea how to structure the code.

hey @mykolaharmash , I've got some examples of what I said above.

Inventory UI: Auto refresh on (loading state and many cancelled requests)

auto_refresh_prof.mov

flamegraph request happen when the visible tab is the overview tab

hidden_prof_request.mov

For the auto-reload, it might be a good idea in the future if the profiler loader kept the rendered component and just showed a loading indicator above the content. Similar to the charts and table in the hosts view. I know this is not part of this ticket.

Thank you for making the videos @crespocarlos! I'll add useIntersectingState and useRequestObservable as you suggested.

@crespocarlos please take another look when have time. I've refactored a bit the request logic, the issues should be fixed now. I've used useRequestObservable to fix the canceled requests thingy, but for the unintended requests I went for a simple active flag based on the currently active tab (see here). It seemed like it's enough in this case, but let me know if I'm missing something.

@crespocarlos, I've merged this one, but let me know if you see issues with the code, I'll address it with my next change 🙌

…-fix'

mykolaharmash · 2023-11-23T11:15:59Z

@elasticmachine merge upstream

mykolaharmash · 2023-11-23T11:27:17Z

x-pack/plugins/serverless_search/public/application/components/connectors_ingestion.tsx

        <EuiFlexItem grow={false}>
-          <EuiLink onClick={() => createConnector()}>
+          <EuiLink
+            data-test-subj="serverlessSearchConnectorIngestionPanelSetUpAConnectorLink"


This seems to be some automated change, not related to the feature

sphilipse

Approving for CI auto-commit

cauemarcondes

did a first look, but haven't run it yet though.

cauemarcondes · 2023-11-23T14:04:46Z

x-pack/plugins/infra/public/components/asset_details/tabs/profiling/profiling.tsx

+    makeRequest();
+  }, [makeRequest]);
+
+  if (loading || response === null) {


I think you can remove this if-clause, am I right? The Flamegraph component will render a loading spinner while loading = true.

True, it was a quick patch to implicitly cast the response to the BaseFlameGraph and I forgot to get rid of it.

x-pack/plugins/infra/server/routes/profiling/index.ts

thomheymann

security changes LGTM

…a-profiling

shahzad31

Obs UX MG Changes LGTM !!

kibana-ci · 2023-11-24T15:50:14Z

💛 Build succeeded, but was flaky

Failed CI Steps

FTR Configs #66

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`infra`	1371	1374	+3

Async chunks

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

id	before	after	diff
`infra`	1.9MB	1.9MB	+984.0B

History

💚 Build #178593 succeeded d2b4ca5
💔 Build #178436 failed 4b76b25
💔 Build #178392 failed ddf9cc2
💔 Build #178135 failed 1487730596ad15d8be1ebb06fc80262a567532f8

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

cauemarcondes

LGTM, great work 🚀

mykolaharmash force-pushed the prodfiler-4268-infra-profiling branch from 273f886 to 885c8d4 Compare November 22, 2023 16:02

mykolaharmash added v8.12.0 release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting Team:obs-ux-infra_services - DEPRECATED DEPRECATED - Use Team:obs-presentation. labels Nov 22, 2023

mykolaharmash force-pushed the prodfiler-4268-infra-profiling branch 2 times, most recently from 285093f to 163ce8d Compare November 23, 2023 10:09

mykolaharmash marked this pull request as ready for review November 23, 2023 10:10

mykolaharmash requested a review from a team as a code owner November 23, 2023 10:10

mykolaharmash requested a review from a team November 23, 2023 10:10

mykolaharmash requested review from a team as code owners November 23, 2023 10:10

[ObsUX] Add Profiling tab to Asset Details

f6b1119

mykolaharmash force-pushed the prodfiler-4268-infra-profiling branch from 163ce8d to f6b1119 Compare November 23, 2023 10:13

mykolaharmash requested a review from a team as a code owner November 23, 2023 10:13

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

ddf9cc2

…fix'

crespocarlos reviewed Nov 23, 2023

View reviewed changes

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

21f1d4d

…-fix'

kibanamachine requested a review from a team November 23, 2023 10:56

pgayvallet approved these changes Nov 23, 2023

View reviewed changes

Merge branch 'main' into prodfiler-4268-infra-profiling

4b76b25

mykolaharmash commented Nov 23, 2023

View reviewed changes

sphilipse approved these changes Nov 23, 2023

View reviewed changes

cauemarcondes reviewed Nov 23, 2023

View reviewed changes

thomheymann approved these changes Nov 23, 2023

View reviewed changes

Adress PR comments

c746666

Merge remote-tracking branch 'upstream/main' into prodfiler-4268-infr…

a8f9881

…a-profiling

mykolaharmash requested a review from a team as a code owner November 23, 2023 15:44

Toggle the feature flag

d2b4ca5

mykolaharmash requested a review from cauemarcondes November 24, 2023 08:57

mykolaharmash added 2 commits November 24, 2023 15:32

Fix flamegraph requests issues

f4f9db5

Merge remote-tracking branch 'upstream/main' into prodfiler-4268-infr…

38de32b

…a-profiling

mykolaharmash requested a review from crespocarlos November 24, 2023 14:38

shahzad31 approved these changes Nov 24, 2023

View reviewed changes

cauemarcondes approved these changes Nov 27, 2023

View reviewed changes

mykolaharmash merged commit 3f9b2ce into elastic:main Nov 27, 2023

mykolaharmash deleted the prodfiler-4268-infra-profiling branch November 27, 2023 09:29

Conversation

mykolaharmash commented Nov 22, 2023 • edited by cauemarcondes Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

How to test

Uh oh!

ghost commented Nov 22, 2023

🤖 GitHub comments

Uh oh!

elasticmachine commented Nov 23, 2023

Uh oh!

mykolaharmash commented Nov 23, 2023

Uh oh!

crespocarlos Nov 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mykolaharmash Nov 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crespocarlos Nov 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mykolaharmash Nov 24, 2023

Choose a reason for hiding this comment

Uh oh!

mykolaharmash Nov 24, 2023

Choose a reason for hiding this comment

Uh oh!

mykolaharmash Nov 27, 2023

Choose a reason for hiding this comment

Uh oh!

mykolaharmash commented Nov 23, 2023

Uh oh!

mykolaharmash Nov 23, 2023

Choose a reason for hiding this comment

Uh oh!

sphilipse left a comment

Choose a reason for hiding this comment

Uh oh!

cauemarcondes left a comment

Choose a reason for hiding this comment

Uh oh!

cauemarcondes Nov 23, 2023

Choose a reason for hiding this comment

Uh oh!

mykolaharmash Nov 23, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thomheymann left a comment

Choose a reason for hiding this comment

Uh oh!

shahzad31 left a comment

Choose a reason for hiding this comment

Uh oh!

kibana-ci commented Nov 24, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Async chunks

History

Uh oh!

cauemarcondes left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

mykolaharmash commented Nov 22, 2023 •

edited by cauemarcondes

Loading

crespocarlos Nov 23, 2023 •

edited

Loading

mykolaharmash Nov 23, 2023 •

edited

Loading

crespocarlos Nov 23, 2023 •

edited

Loading