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.

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

[ML] APM Latency Correlations UX improvements/fixes #108860

Merged
merged 85 commits into from
Aug 18, 2021

Conversation

walterra
Copy link
Contributor

@walterra walterra commented Aug 17, 2021

Summary

Part of #108332.
Follow up to #107266.

  • Tweak label for log log chart data series for all transactions. f53d428
  • Show tip to drag to select a range in the Trace samples distribution chart panel. 086c5a0
  • Change the selected range label and clear selection option to EuiBadge with click action and tooltip on hover 'Clear selection'. 64937ff
  • Move the 'current sample' annotation to the bottom axis to not overlap with the 95p annotation marker. f50e96f
  • Move the help popover to the top of the panel. f24a5cc
  • Move the refresh/cancel option to the right of the progress bar. f425339
  • Always display the table for correlations. 212b77f
  • Show improved empty state using the EuiEmptyPrompt for the correlations table when no significant correlations are found. 212b77f
  • Fix Filter behaviour: when clicking on the filter item, jump to show top of page where the filter bar is evident, and reset the correlation tabs and show the Trace Samples tab ebc05ae
  • Fix updating the log log chart and correlation service when the user changes the kquery or time span. 5804216

Checklist

Delete any items that are not applicable to this PR.

For maintainers

walterra added 30 commits August 4, 2021 17:44
@formgeist
Copy link
Contributor

formgeist commented Aug 17, 2021

@walterra I noticed two more things just going through it again (in dark mode).

Default charts configuration is not used
APM uses a default configuration and theme provider (through EUI) for supporting dark theme and I believe configurations around the e.g. gridlines in the chart canvas. I could see in the distribution chart the appropriate theming isn't added for dark theme and the gridlines are not present (hard to tell on the light theme).

CleanShot 2021-08-17 at 21 12 46@2x

Filters added using special chars are not formatted correctly in the query bar
I added a filter for a url.path and it didn't return any results in the data because the formatting from clicking the filter value option in the table wasn't correct.

CleanShot 2021-08-17 at 20 29 58@2x

CleanShot 2021-08-17 at 20 30 15@2x

CleanShot 2021-08-17 at 20 30 30@2x

Fixed the formatting of the URL yielded results in the end

@walterra walterra marked this pull request as ready for review August 17, 2021 19:44
@walterra walterra requested a review from a team as a code owner August 17, 2021 19:44
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@walterra
Copy link
Contributor Author

@formgeist Thanks for the hint with the theming in other charts, I was able to fix it in 14c6896

Fixed Light mode:

image

Fixed Dark mode:

image

@qn895
Copy link
Member

qn895 commented Aug 17, 2021

Tested latest changes and LGTM 🎉

@walterra walterra enabled auto-merge (squash) August 17, 2021 20:54
@walterra walterra added the auto-backport Deprecated - use backport:version if exact versions are needed label Aug 17, 2021
@walterra
Copy link
Contributor Author

@elasticmachine merge upstream

@walterra
Copy link
Contributor Author

@elasticmachine merge upstream

@qn895
Copy link
Member

qn895 commented Aug 17, 2021

@elasticmachine merge upstream

Copy link
Member

@sorenlouv sorenlouv left a comment

Choose a reason for hiding this comment

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

lgtm

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
apm 4.4MB 4.4MB +2.4KB

History

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

cc @walterra

@walterra walterra merged commit 2908ece into elastic:master Aug 18, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Aug 18, 2021
- Tweak label for log log chart data series for all transactions.
- Show tip to drag to select a range in the Trace samples distribution chart panel.
- Change the selected range label and clear selection option to `EuiBadge` with click action and tooltip on hover 'Clear selection'.
- Move the 'current sample' annotation to the bottom axis to not overlap with the 95p annotation marker.
- Move the help popover to the top of the panel.
- Move the refresh/cancel option to the right of the progress bar.
- Always display the table for correlations.
- Show improved empty state using the `EuiEmptyPrompt` for the correlations table when no significant correlations are found.
- Fix Filter behaviour: when clicking on the filter item, jump to show top of page where the filter bar is evident, and reset the correlation tabs and show the Trace Samples tab
- Fix updating the log log chart and correlation service when the user changes the kquery or time span.
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Aug 18, 2021
- Tweak label for log log chart data series for all transactions.
- Show tip to drag to select a range in the Trace samples distribution chart panel.
- Change the selected range label and clear selection option to `EuiBadge` with click action and tooltip on hover 'Clear selection'.
- Move the 'current sample' annotation to the bottom axis to not overlap with the 95p annotation marker.
- Move the help popover to the top of the panel.
- Move the refresh/cancel option to the right of the progress bar.
- Always display the table for correlations.
- Show improved empty state using the `EuiEmptyPrompt` for the correlations table when no significant correlations are found.
- Fix Filter behaviour: when clicking on the filter item, jump to show top of page where the filter bar is evident, and reset the correlation tabs and show the Trace Samples tab
- Fix updating the log log chart and correlation service when the user changes the kquery or time span.

Co-authored-by: Walter Rafelsberger <[email protected]>
@walterra walterra deleted the ml-apm-correlations-page-ux branch August 18, 2021 08:00
@walterra
Copy link
Contributor Author

I added an item about the character encoding problem when filtering to the meta issue, it wasn't fixed as part of this PR, see #106381.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:correlations auto-backport Deprecated - use backport:version if exact versions are needed bug Fixes for quality problems that affect the customer experience :ml release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support v7.15.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants