-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Lens] Implement time scaling function #82104
Conversation
Jenkins, test this. |
Pinging @elastic/kibana-app-arch (Team:AppArch) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
App arch code LGTM, just one testing nit
from: '2020-10-05T00:00:00.000Z', | ||
to: '2020-10-10T00:00:00.000Z', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should also add a test for the new case that is introduced in this PR where we fall back to interval
if the appliedTimeRange
is undefined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point - we always had this case but the types didn't reflect it. Actually it's an error case, I extended the condition to throw an error and added unit tests.
@elasticmachine merge upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, didn't test (I think tests are sufficient)
💚 Build SucceededMetrics [docs]@kbn/optimizer bundle module count
async chunks size
page load bundle size
History
To update your PR or re-run it, just comment with: |
Fixes #77692
Adds
lens_time_scale
function to scale numbers by time bucket.The time scale function takes the following arguments:
This function fails if there is no meta data on the date column to determine the used time zone, time range and interval.
It builds on top of the
getDateMetaByDatatableColumn
added in a previous PR, but one change was necessary.Right now, the applied time range is added to the column meta data using the input date math range (e.g. from
now-24h
tonow
). This is problematic because due to async search the time scale function could get triggered much later than the actual query hitting Elasticsearch - this would skew the size of the first and last bucket in the time range and produce wrong scaling for those buckets. By resolving the date range directly before sending the request to Elasticsearch and converting it into ISO strings for from and to, the time scaling will be very close to exact even for long running searches.