[Profiling] Replacing TopN functions table for EuiGrid

[cauemarcondes]

TopN functions

Diff TopN functions

[apmmachine]

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

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

[jbcrail]

I did a first pass to review the code and it looks good. I will follow up on Wednesday to try it out with live data.

[jbcrail]

Here are a few miscellaneous items that I noticed after setting up a cloud instance with live data:

1. This is a very general observation with no concrete data, but it seemed that the grid rendered slower than the prior implementation. Perhaps this is a function of running Kibana locally and pulling the data from the cloud. Or maybe it's inherent to the EuiGrid. I'm not exactly sure without further investigation.

2. I saw at least one React warning in the console:

Warning: Cannot update during an existing state transition (such as within `render`). Render methods should be a pure function of props and state.

3. For non-differential TopN functions, the data in the function row does not match with the data in the frame information window. For instance, see the values for total CPU, self CPU, and both samples. In the following screenshots, I clicked on the row for rank 21.

Before

After

4. When the button for fullscreen mode is clicked and then closed, the grid does not return to the original size (fit to the available width). You can see the grid overflow past the right side of the page in the second screenshot. I'm not sure if this is a EuiGrid bug or controllable on our end.

Before

After

5. Would it be clearer from a UI perspective nd/or easier for internalization to swap the abbreviation in the Samples column (estd.) with the same question icon used in the CPU columns?

6. I noticed that the "Annualized CO2" column is sorted using A-Z vs Z-A, not High-Low vs Low-High. Since the annualized CO2 is ultimately a numeric value but is represented by a string, this creates an odd sorted list that may not match a user's expectation.

[cauemarcondes]

1. This is a very general observation with no concrete data, but it seemed that the grid rendered slower than the prior implementation. Perhaps this is a function of running Kibana locally and pulling the data from the cloud. Or maybe it's inherent to the EuiGrid. I'm not exactly sure without further investigation.
2. I saw at least one React warning in the console:

I noticed this too. I was rendering 500 items but that must be too much. I changed to 100 items.

For non-differential TopN functions, the data in the function row does not match with the data in the frame information window. For instance, see the values for total CPU, self CPU, and both samples. In the following screenshots, I clicked on the row for rank 21.

This is actually a regression. I fixed it here, but I will open a new PR fixing only this and merging it on 8.9 too.

[cauemarcondes]

Would it be clearer from a UI perspective nd/or easier for internalization to swap the abbreviation in the Samples column (estd.) with the same question icon used in the CPU columns?

Looks good to me. I added an Estimated values in the hint.
cc: @iogbole

[cauemarcondes]

6. I noticed that the "Annualized CO2" column is sorted using A-Z vs Z-A, not High-Low vs Low-High. Since the annualized CO2 is ultimately a numeric value but is represented by a string, this creates an odd sorted list that may not match a user's expectation.

Thanks, I fixed it. I also noticed that the Samples column wasn't being sorted correctly. I also fixed it.

[jbcrail]

Thanks for fixing those items. I retested and only saw one small issue with two consecutive tildes in the frame information window, specifically for the CO2 emission and Dollar cost estimates:

[jbcrail]

Looks good 👍 🚀

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: bd76760
• Storybooks Preview

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
profiling	169	174	+5

Async chunks

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

id	before	after	diff
profiling	290.6KB	293.9KB	+3.3KB

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
enterpriseSearch	14	16	+2
securitySolution	411	415	+4
total			+6

Total ESLint disabled count

id	before	after	diff
enterpriseSearch	15	17	+2
securitySolution	490	494	+4
total			+6

History

• 💚 Build #142377 succeeded 4596ef7
• 💚 Build #142346 succeeded 6f25d4f
• 💚 Build #141215 succeeded a41c8bb
• 💚 Build #140297 succeeded a05de07
• 💔 Build #140284 failed f195633
• 💔 Build #140107 failed 03bffce

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