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

Round date by default in date picker #94280

Open
dgieselaar opened this issue Mar 10, 2021 · 17 comments
Open

Round date by default in date picker #94280

dgieselaar opened this issue Mar 10, 2021 · 17 comments
Assignees
Labels
enhancement New value added to drive a business result epic Feature:Search Querying infrastructure in Kibana Feature:Timepicker Timepicker Feature:Unified search Unified search related tasks impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Team:Visualizations Visualization editors, elastic-charts and infrastructure

Comments

@dgieselaar
Copy link
Member

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.

@dgieselaar dgieselaar added Feature:Search Querying infrastructure in Kibana Team:AppServices labels Mar 10, 2021
@elasticmachine
Copy link
Contributor

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

@Dosant Dosant added Feature:Timepicker Timepicker enhancement New value added to drive a business result labels Mar 10, 2021
@Dosant
Copy link
Contributor

Dosant commented Mar 10, 2021

@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: #88480

@dgieselaar
Copy link
Member Author

@Dosant yes, that's pretty much it. This is where the rounding happens:

// rounds down start to minute
const roundedStart = moment(start).startOf('minute');
return {
start: roundedStart.toISOString(),
end: end.toISOString(),
};

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.

@exalate-issue-sync exalate-issue-sync bot added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort labels Jun 2, 2021
@dgieselaar
Copy link
Member Author

dgieselaar commented Nov 6, 2021

@Dosant @ppisljar I'm not sure why this was tagged with impact:low. My perception is that it's low effort, high impact. With elastic/elasticsearch#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 elastic/elasticsearch#79610 is not necessary if this means both the lower and upper boundary of the time range is rounded.

@exalate-issue-sync exalate-issue-sync bot added impact:needs-assessment Product and/or Engineering needs to evaluate the impact of the change. and removed impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. labels Nov 8, 2021
@Dosant
Copy link
Contributor

Dosant commented Nov 8, 2021

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

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

@rayafratkina
Copy link
Contributor

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

@sixstringcode
Copy link

@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
Copy link
Member Author

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.

@exalate-issue-sync exalate-issue-sync bot added loe:medium Medium Level of Effort and removed loe:small Small Level of Effort labels Sep 12, 2022
@petrklapka petrklapka added Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. and removed Team:AppServicesSv labels Nov 23, 2022
@elasticmachine
Copy link
Contributor

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

@lukasolson lukasolson added Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Unified search Unified search related tasks labels Dec 13, 2022
@elasticmachine
Copy link
Contributor

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

@lukasolson lukasolson removed loe:medium Medium Level of Effort Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. labels Dec 13, 2022
@stratoula
Copy link
Contributor

stratoula commented Jun 2, 2023

@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?

@timductive timductive added impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. epic and removed impact:needs-assessment Product and/or Engineering needs to evaluate the impact of the change. labels Nov 29, 2023
@drewdaemon
Copy link
Contributor

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
Copy link
Member

ppisljar commented May 22, 2024

With #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
Copy link
Member

markov00 commented Jun 4, 2024

Related to rounding time when brushing charts elastic/elastic-charts#851

@thomasneirynck
Copy link
Contributor

Changing status to todo. Did initial research #94280 (comment), on hold for now.

@teresaalvarezsoler
Copy link

@thomasneirynck the research showed that we would need to round to a very big unit to hit the cache and that's a very bad user experience... can we close this as not planned? cc @ghudgins

@thomasneirynck
Copy link
Contributor

@teresaalvarezsoler do you have a link to the findings?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result epic Feature:Search Querying infrastructure in Kibana Feature:Timepicker Timepicker Feature:Unified search Unified search related tasks impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

No branches or pull requests