search

elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.69k stars 8.11k forks source link

[APM] Expected bounds comparison: Add default comparison option and enabling/disabling time comparison the advanced settings #133116

Open formgeist opened 2 years ago

formgeist commented 2 years ago

Follow-up to: https://github.com/elastic/kibana/pull/132456

We need to define the advanced setting options for this feature as we'll now introduce additional options in the comparison feature for expected bounds. Basically, we want to support turning the historical comparison on or off since the feature might result in a performance hit on the page loads for larger clusters.

If you have ML jobs enabled, expected bounds comparison can be seen as a default feature. That means if the historical comparison is turned off, the expected bounds comparison will be the default. This also means that the comparison feature as a whole cannot be turned off, only the historical option. E.g. if the user has turned off the historical comparison and then enables the ML integration, they will immediately see the expected bounds in the UI without having to know to turn on the comparison feature in Advanced settings.

So I propose the following changes to the advanced settings options for more granular control;

Existing feature flag option (for reference)

CleanShot 2022-01-18 at 08 33 46@2x

Proposed options

image

We may have to rename the environment variables to be more specific for the historical comparison and the suggestion for the default comparison type is only suggested, feel free to figure out a better name.

Originally posted by @formgeist in https://github.com/elastic/kibana/issues/121520#issuecomment-1015150564

elasticmachine commented 2 years ago

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

elasticmachine commented 2 years ago

Pinging @elastic/ml-ui (:ml)

gbamparop commented 2 years ago

If we aim to remove the ability to turn off the comparison feature, can this be simplified by just having a dropdown for the default comparison option? Or we could have three options (Time, Expected bounds, Off) cc @boriskirov @sqren

gbamparop commented 2 years ago

One other thing to keep in mind is that we only support expected bounds on service overview and transaction pages, and when there are ML jobs for the selected environment. This means that even if the user sets the default comparison option to be Expected bounds there will be fallbacks in certain cases. For instance, if a user starts from service inventory, the offset (e.g. 1d) is pushed in the url. When they navigate to a service overview page, we wouldn't know if the selection was made deliberately or not so we can default to expected bounds.

sorenlouv commented 2 years ago

If we aim to remove the ability to turn off the comparison feature, can this be simplified by just having a dropdown for the default comparison option?

We need to keep the option to turn off comparisons (it was requested by customers due to perf concerns).

Or we could have three options (Time, Expected bounds, Off)

That sounds good to me.

One other thing to keep in mind is that we only support expected bounds on service overview and transaction pages, and when there are ML jobs for the selected environment. This means that even if the user sets the default comparison option to be Expected bounds there will be fallbacks in certain cases.

We can add support for expected bounds comparisons (on spark lines) on the service inventory page. That will mostly fix this problem afaict.

sorenlouv commented 2 years ago

One more thing: we should change the comparisons dropdown to be EUI Selectable (or similar). This will make it possible to add some help text to each option where needed. For instance, if no ML jobs exist, the ML option should be disabled with a text below to let the user know that they need to add an ML job for the selected environment.