[Lens] Absolute time shift support in formula

[dej611]

Summary

Closes #102493

This PR introduces the ability to define an absolute time shift as a new type of time shift in Lens.
The idea is to avoid to push down the absolute logic to the agg level, rather translate as soon as possible the absolute shift into a relative one (in seconds, the minimal time unit allowed) and perform all checks in this format.

Note:
The feature is currently enabled as formula-only and using it in a Quick function context will validate it as an invalid shift.

Details:
The used format for anchoring right now is:

• startAt(2022-11-03T18:30:27.278Z) for start anchoring (from the given date used as start anchor forward to a dynamic end date), and

• endAt(2022-11-03T18:30:27.278Z) for end anchoring (from the given date backward to a start date dynamically computed)

• when an absolute time shift is detected in formula then the shifts suggestions are translated into absolute shifts

Basics

• Add a Absolute time shift validation function
• Add a strict shift parser to duration (in seconds)
• Add a function to extract tokens from raw string
• Enable the feature in Lens
  • formula
    • parsing
    • formula validation
    • suggestion adapted to work with the Absolute shift
  • errors
  • warnings
    • interval-based validations are skipped for absolute shifts as issues are resolved at expression translation time anyway
  • via API (formula helper)

Implementation details

While the range is computed correctly based on the current interval, the actual shift is rounded to prevent issues on data fetching. The interval is computed with an algorithm similar to the one used by ES and the shift is offset to the closest multiple of that interval (i.e. if the current shift is 1.78 times the interval, then the shift is rounded to 2 intervals in a way to include the given date).

Checklist

Delete any items that are not applicable to this PR.

• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Documentation was added for features that require explanation or tutorials
• Unit or functional tests were updated or added to match the most common scenarios
• Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
• Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
• If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
• This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
• This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

• This was checked for breaking API changes and was labeled appropriately

[elasticmachine]

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

[flash1293]

discussion: should we have a specific error message for this?

[flash1293]

This is probably debatable, but is it anchoring the right way?

start - <date> seems to anchor to the end of the time range. Maybe I'm confusing something

[flash1293]

Another considerations about the negative shift case - maybe we should allow it because with static anchor it's easy to change the time range and run into the error. For example you look at the last week with a reference time frame from last month (that works fine). However when trying to check a week from last year, you would get an error because it's a negative time shift now.

Not sure whether relevant, we can also split that out

[dej611]

This is probably debatable, but is it anchoring the right way?

start - <date> seems to anchor to the end of the time range. Maybe I'm confusing something

That's a good point. I've assumed that start - <date> meant something like | ------- <date> (and end - <date> something like <date> ------ |) but I see how you've interpreted it.

I've updated the PR to work the other way around.

[dej611]

Another considerations about the negative shift case - maybe we should allow it because with static anchor it's easy to change the time range and run into the error. For example you look at the last week with a reference time frame from last month (that works fine). However when trying to check a week from last year, you would get an error because it's a negative time shift now.

Not sure whether relevant, we can also split that out

I've introduced a stricter check now on these cases. I see how it can be useful if the reference time range is in the past, but there's no easy way to make a proper distinction of cases.

[dej611]

This is probably debatable, but is it anchoring the right way?
start - <date> seems to anchor to the end of the time range. Maybe I'm confusing something

That's a good point. I've assumed that start - <date> meant something like | ------- <date> (and end - <date> something like <date> ------ |) but I see how you've interpreted it.

I've updated the PR to work the other way around.

On the matter I've had a chat offline with @markov00 and the notation format is something confusing/not fully clear (is the - a math operation or just a divider?).

At first something like start - <date> and <date> - end might make sense, but that would make all the validation logic to be more complex.
So at the end something like startAt(date) and endAt(date) has been proposed, which sounds to me more clear and less prone to misunderstandings. WDYT @flash1293 @stratoula @ninoslavmiskovic ?

[stratoula]

This is a great feature Marco!! I am playing a bit with it and I found a confusing behavior.
I play with the timepicker and although the formula errors out, the chart isn't. When I click the formula then the chart errors out.

[stratoula]

Also I see this error

(I have the aggregate by this field switch off)

[dej611]

This is a great feature Marco!! I am playing a bit with it and I found a confusing behavior. I play with the timepicker and although the formula errors out, the chart isn't. When I click the formula then the chart errors out.

Thanks @stratoula for the feedback, managed to reproduce this. I've forgotten to pass the new interval in this case and the error is not propagated to the chart. Will fix this ASAP.

Also I see this error

(I have the aggregate by this field switch off)

Good catch! Managed to reproduce it. On it!

[dej611]

Managed to partially solve this problem.
The only case which remains to be addressed is the following, which is a general case that affects formula also outside of the absolute time shift use case:

• Add a date histogram
• Create a formula with an invalid time shift for the given range (i.e. pick a date range X -> Y and use an absolute date in the formula shift where ABS > X)
• Now change the date range to make the absolute time shift become valid ( so X > ABS )
• The chart will not errors, rather show a No results screen

That is due to the formula transitioning from an initial invalid state to a valid one, hence there are no referenced columns yet for the formula. Opening the formula dimension panel will trigger the generation of all the structure underneath required and render the data.
This is a more general problem to solve.
Note that if the formula has been at least once valid, then the transition to invalid and back to valid will trigger correctly the rendering.

I can reproduce the same bug with relative time shifts (i.e. count(shift='7d') and a Top values breakdown dimension with that flag disabled). I'll create a distinct issue for that.

[stratoula]

Thanx for the fix Marco. This looks much better now. Do you think we could document it?
If I type shift it suggests me relative shifts and not the startAt/endAt (which I like tbh but how the users can know about this addition?) If we dont' add this on autocomplete but add a note on our documentation? Wdyt?

[dej611]

If I type shift it suggests me relative shifts and not the startAt/endAt (which I like tbh but how the users can know about this addition?) If we dont' add this on autocomplete but add a note on our documentation? Wdyt?

The autosuggests for the absolute time shift kicks in after the startAt( or endAt( prefix, replacing the relative one. Perhaps I could reduce the trigger char to just s and e given the distinct letters from the relative one. What do you think?

[dej611]

@stratoula I've changed the autocomplete logic to be more user friendly:

• when the user types shift='' now two abs suggestions will be appended to the list (one startAt and one endAt using the first relative shift value):
• when the user types the ( in startAt( or endAt( the old autosuggestion kicks in to show all relative suggestions translated into absolute ones now

Would that work for you?

[stratoula]

Yes Marco! Much better now! Thanx

[davismcphee]

I saw Data Discovery was requested on this, so a quick code-only review. I don't really have the context needed to review properly, but the code, comments, and tests for parse_time_shift LGTM.

We share ownership of the data plugin now anyway, so I think @stratoula's approval is most important here anyway 👍

[lukasolson]

Minor nit but otherwise LGTM! Code review

[lukasolson]

Nit: Prefer upper case for constants (e.g. `const PREVIOUS_DATE = 'previous';)

[kibana-ci]

💛 Build succeeded, but was flaky

• Buildkite Build
• Commit: 9227530

Failed CI Steps

• FTR Configs #31
• Rules, Alerts and Exceptions ResponseOps Cypress Tests on Security Solution

Test Failures

• [job] [logs] Rules, Alerts and Exceptions ResponseOps Cypress Tests on Security Solution / Alerts timeline Add a non-empty property to default timeline

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
data	2557	2561	+4
lens	597	598	+1
total			+5

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
lens	1.3MB	1.3MB	+2.0KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
data	401.4KB	402.9KB	+1.5KB
lens	29.8KB	29.8KB	+46.0B
total			+1.6KB

Unknown metric groups

API count

id	before	after	diff
data	3269	3279	+10
lens	693	694	+1
total			+11

ESLint disabled in files

id	before	after	diff
osquery	1	2	+1

ESLint disabled line counts

id	before	after	diff
enterpriseSearch	19	21	+2
fleet	59	65	+6
osquery	109	115	+6
securitySolution	445	451	+6
total			+20

Total ESLint disabled count

id	before	after	diff
enterpriseSearch	20	22	+2
fleet	68	74	+6
osquery	110	117	+7
securitySolution	521	527	+6
total			+21

History

• 💔 Build #93226 failed d5ac890
• 💔 Build #93220 failed 4058308
• 💚 Build #92631 succeeded 232ff5b
• 💚 Build #90879 succeeded a30befe
• 💚 Build #90749 succeeded 8e06754

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream