elastic / kibana

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

Round date by default in date picker #94280

Open dgieselaar opened 3 years ago

dgieselaar commented 3 years ago

Some of the teams have starting rounding down the lower end of the time range to leverage ES caching better. This is especially useful with a large amount of concurrent users. I propose that the date picker rounds the lower end of the time range by default to 1m (or 30s or 15s, depending on the current time range), and solutions and/or users can opt-out. If we can't enable it by default, we can consider making the current "Round by the minute" more visible and more easily configurable by consumers of the date pickers, so teams don't have to round their date ranges themselves. IMO, it's also critical to make sure that whatever date range is displayed is also used for querying, so the user can trust that their data is based on the date range that is displayed.

elasticmachine commented 3 years ago

Pinging @elastic/kibana-app-services (Team:AppServices)

Dosant commented 3 years ago

@dgieselaar,

Some of the teams have starting rounding down the lower end of the time range to leverage ES caching better

To confirm, this rounding is happening on the client? And the flow is something like:

  1. time picker has a relative time range
  2. App code gets time from the time picker and converts it to an absolute time range rounding it down. Happens on the client.
  3. The result absolute time range pass to search requests.

Could you please point to some of the code examples for reference?


probably related and should be part of a longer term discussion: https://github.com/elastic/kibana/issues/88480

dgieselaar commented 3 years ago

@Dosant yes, that's pretty much it. This is where the rounding happens: https://github.com/elastic/kibana/blob/0337d7d93342449045dcf72f1ba31c85caa01ec3/x-pack/plugins/apm/public/context/url_params_context/helpers.ts#L45-L51

I' not sure if PIT makes sense here (for us), I vaguely remember it coming with a performance hit. See https://www.elastic.co/guide/en/elasticsearch/reference/master/point-in-time-api.html#point-in-time-keep-alive for instance.

dgieselaar commented 2 years ago

@dosant @ppisljar I'm not sure why this was tagged with impact:low. My perception is that it's low effort, high impact. With https://github.com/elastic/elasticsearch/issues/79610, the impact will be even bigger, especially if we enable this for Kibana as a whole. @jimczi any thoughts from the ES side about the impact of this?

Edit: actually https://github.com/elastic/elasticsearch/issues/79610 is not necessary if this means both the lower and upper boundary of the time range is rounded.

Dosant commented 2 years ago

@Dosant @ppisljar I'm not sure why this was tagged with impact:low

@dgieselaar, not at all, this was a jira integration misconfiguration

rayafratkina commented 2 years ago

@stratoula @lukasolson when we take on the Unified Search bar we should fix this too!

sixstringcode commented 2 years ago

@rayafratkina I am ok to include it as one of our dependencies for Unified Search. @lukasolson do you feel comfortable with this in our near term bucket?

dgieselaar commented 2 years ago

fwiw, I think this is more useful if both the lower and upper bound of the time range are rounded, to increase the chance of a cache hit on write indices.

elasticmachine commented 1 year ago

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

elasticmachine commented 1 year ago

Pinging @elastic/kibana-visualizations @elastic/kibana-visualizations-external (Team:Visualizations)

stratoula commented 1 year ago

@dgieselaar just a clarification about this. You would like this setting on the date picker to be true by default right?

image

or we just want the nowProvider to always round by default? Or do we want both?

drewdaemon commented 6 months ago

For the record: any changes to the NowProvider should be checked WRT the Lens formula context functions (now, time_range, interval).

These functions don't change the way queries are created so I would suggest we attempt to keep them as accurate as possible and leave rounding up to formula authors.

ppisljar commented 1 month ago

With https://github.com/elastic/kibana/pull/178721 we concluded that rounding dates to just seconds will not result in significant improvements as the chance of hitting the cache is pretty low. To increase the chance of hitting the cache we should round to considerably larger units.

Date picker has a rounding option, which already rounds to large units. For example when using a daterange now - 7d which we can consider to be a common one, it will round to a day. This gives every user hitting the dashboard in that same day a chance to hit the cache.

However there are a bunch of issues with date picker rounding which would become only more visible if we would turn this option on by default.

Issues with current date picker

Rounding setting losses state

State of the round-to-the checkbox is lost. When the user turns the setting on and closes the datepicker and reopens it the setting is not remembered. User then has to enable/disable to actually disable the setting.

It should be possible to programmatically pass in this setting and get the current configuration to EuiSuperDatePickler This is actually already possible by setting the correct times (now-7d/d vs now-7d), but 1. would need to be solved in a way that the state is retrieved from parsing the provided time string.

If user sets this setting on TO date (when not using NOW) there is no visual indicator in the time picker that this setting is turned on.

It should be possible to set rounding on from/to dates when setting the quick ranges.

In the quick selection time ranges it might be useful to indicate that some quick ranges are using the rounding.

Not rounding NOW

When using the NOW tab in the date picker its never rounded. This results in most user-picked time ranges (from quick selection) to result in a time range that will never hit the cache due to millisecond part.

It should be possible to round the now as well.

Not rounding absolute ranges

Absolute ranges most often result from user action on a chart, like selecting a range in a time series chart. Those will contain millisecond part even when the user was selecting a range on a chart with bars representing years. That was definitely not the user's intention, so those ranges should be rounded (to the nearest bucket?) as well. Confusing UI

The UX for this is a bit confusing, as we can set the rounding on FROM and TO dates independently, so one can be rounded and the second not. Its also not possible to set the rounding on NOW. It might be worth exploring moving this setting one layer up so its for both dates (FROM and TO)

Proposal:

Rather than fixing individual issues I propose moving rounding setting one layer up to global date picker settings (like refresh interval). Rather than having individual settings on from/to dates and another one on the now tab and possibly on the absolute tab we can have just one.

image5

image4

Pros:

Simplifies the UX Greatest chance of a cache hit Solves all above mentioned issues

Cons:

Less control for the user, as he can no longer apply different rounding to from and to part of the range

When will we hit a cache ?

Biggest advantage is in multi user systems. Lets say 1000 users come to the same dashboard within the hour. All but the first will hit the cache. When single user is navigating within the system: User opened a dashboard, then went to lens to change some colors or create some new charts. He comes back to dashboard within the time limit and hits the cache User enables/disables a filter …

markov00 commented 3 weeks ago

Related to rounding time when brushing charts https://github.com/elastic/elastic-charts/issues/851