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

[APM] Correlations GA UX polish (meta) #108332

Closed
13 tasks done
formgeist opened this issue Aug 12, 2021 · 11 comments
Closed
13 tasks done

[APM] Correlations GA UX polish (meta) #108332

formgeist opened this issue Aug 12, 2021 · 11 comments
Assignees
Labels
apm:correlations :ml polish Team:APM All issues that need APM UI Team support v7.15.0

Comments

@formgeist
Copy link
Contributor

formgeist commented Aug 12, 2021

Summary

This issue tracks the summarized list of UX improvements and changes before making the Latency Correlations feature GA within the Transaction group detail page.

Tasks

General

  • Rename "Latency correlations analysis" -> "Latency correlations" 5f8c045
  • Rename "Failed transaction rate" -> "Failed transactions correlations" 14c229d
  • Add beta badge to the "Failed transaction rate correlations" tab a2564f5 (added in the mentioned commit but there remains an issue where the badge vertically misaligns tab labels now) -> moved to content section inside tab in [ML] Replace APM error rate table with failed transactions correlations #108441
    failed-transaction-rate-correlations-beta
  • Remove the redundant transaction group name and log log description from the main Latency distribution titles, just name it Latency distribution
    03e450e
  • Add "transactions" to the tooltip on hover in the distribution to understand the number shown better
    CleanShot 2021-08-12 at 09 54 30@2x

Trace samples tab

  • Show tip to drag to select a range in the Trace samples distribution chart panel
    distribution-show-tip-to-select

  • Change the selected range label and clear selection option to EuiBadge with click action and tooltip on hover "Clear selection"
    range-selection-button

  • Use the same unit for the range selected as in the x-axis (right now it's ms)
    CleanShot 2021-08-12 at 09 48 28@2x

  • Move the "current sample" annotation to the bottom axis to not overlap with the 95p annotation marker
    CleanShot 2021-08-12 at 09 47 24@2x

Correlations tabs

  • Move the help popover to the top of the panel
    CleanShot 2021-08-12 at 10 23 32@2x

  • Move the refresh/cancel option to the right of the progress bar
    CleanShot 2021-08-12 at 10 25 11@2x

  • Always display the table for correlations - we'll also use it for displaying the "no significant correlations found" if that's returned
    CleanShot 2021-08-12 at 10 27 31@2x

  • Show improved empty state using the EuiEmptyPrompt for the correlations table when no significant correlations are found. Might need some help from our dear writers on the final copy.
    correlations-table-empty-state

@formgeist formgeist added Team:APM All issues that need APM UI Team support polish apm:correlations v7.15.0 labels Aug 12, 2021
@elasticmachine
Copy link
Contributor

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

@walterra walterra added the :ml label Aug 12, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@walterra walterra self-assigned this Aug 12, 2021
@sorenlouv
Copy link
Member

Rename "Failed transaction rate" -> "Failed transaction rate correlations"

Perhaps simply "Failed transaction correlation" is better?

@sorenlouv
Copy link
Member

sorenlouv commented Aug 12, 2021

Use the same unit for the range selected as in the x-axis (right now it's ms)

Comment for @walterra: In APM duration units are dynamic, meaning it can be ms, seconds, minutes etc. depending on the size. I think elastic charts work the same way, so we might have to call a utility function in elastic charts that return the unit used for the specific data (pseudo code):

import { getTimeUnit } from '@elastic/charts'
const unit = getTimeUnit(timeseries);

we could then use the same unit elsewhere in APM UI instead of calculating it like we do here:

const tickFormatter = getDurationFormatter(xMax);

@walterra
Copy link
Contributor

Rename "Failed transaction rate" -> "Failed transaction rate correlations"

I remember a discussion that the analysis we do for the errors/failed transactions is technically speaking not a correlation analysis, do you have any input on the wording here @sophiec20 @benwtrent ?

@formgeist
Copy link
Contributor Author

Rename "Failed transaction rate" -> "Failed transaction rate correlations"

Perhaps simply "Failed transaction correlation" is better?

Right, it's not the rate 👍

@sophiec20
Copy link
Contributor

We have three tabs and naming should work when seen together.

Currently:
Trace samples | Latency correlation analysis | Failing transactions

Tense for "failing" feels wrong as they have already failed.
Having "analysis" in one and not in the other seems odd. Suggest dropping.
Having "transactions" in one and not in the other seems odd too.

We have had discussions in the past about use of the term "correlations" with failed transactions. Linguistically they are correlations, but under the hood they use a different method (significant terms) from latency correlations. Personally, I don't think we need to name them differently on this basis and that it is fine to label both of these as correlations.

I suggest:
Trace samples | Latency correlations | Failure correlations

Perhaps @alex-fedotyev needs to make the final call.

@benwtrent
Copy link
Member

@walterra been thinking a bit more on the "correlation" for the "failed transactions". As a user, I don't care :D. We are calculating "Which term/value occurs significantly more with failed transactions than successful ones". So, we are indeed calculating the "interdependence" of two sets of data. Term+value <=> "failed". So, the word "correlation" is cool in my book.

But I am not particularly apt at naming things. It's difficult for me to get out of my technical brain :).

@alex-fedotyev
Copy link

I suggest we keep failed transactions and correlations, although it may looks somewhat complicated.
Trace samples | Latency correlations | Failed transactions correlations

My reasons:

  • We are trying to get away from "errors" and "failures" in transaction view which were causing misunderstanding in the past.
  • We will still have a chance to rename it again, when we make "failed transactions correlations" GA. I think that once we better understand what UX will look like, it may influence the naming so users have right expectations.

@formgeist
Copy link
Contributor Author

@alex-fedotyev thanks - I agree to keep failed transactions. I've updated the description task list above.

@walterra
Copy link
Contributor

Moved one item to the 7.16 meta issue #109220 (paging trace samples), the rest is done, closing 🥳 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:correlations :ml polish Team:APM All issues that need APM UI Team support v7.15.0
Projects
None yet
Development

No branches or pull requests

7 participants