[Search] Search Sessions with relative time range #84405

Dosant · 2020-11-26T12:29:54Z

Summary

In master background session restoration works only for absolute time ranges.
This pr attempts to make it work for relative time ranges, by "syncing" Date.now() between different searches when relative time range is converted into absolute time range.

Relative to absolute time conversion is not centralized even though we already had some forceNow logic for reporting. So some conversions could be missed and I am happy to follow on that

NowProvider is introduced in data services which keeps tracked of current now value. When session is started sessionService sets a new now into NowProvider so all subsequent searches using it for converting from relative to absolute time range:

high-level diagram

In future instead of using now for converting just time filter we would explore using point at time api

There was an edge case with auto refresh. Previously new session wasn't created on auto refresh fetches and with new approach it would cause the same now for all refreshes. Now new sessions are created on auto refresh fetches. For this autoRefresh subscription was moved on a dashboard level and removed from embeddable layer. #84405

How to test

xpack.data_enhanced.search.sendToBackground.enabled: true
data.search.aggs.shardDelay.enabled: true # helpful to create "slow" visualizations

Create a dashboard with search, visualize and lens embeddable.
Make a search with relative time range. Save it.
Get searchSessionId from any of inspector. Get absolute time range from any of inspectors.
build a dashboard URL to restore a session: ${dashboardURL}&searchSessionId=${searchSessionId} and replace relative time range with copied time range
Open it. Search session should be restored and you should see absolute time range

^^ this is what functional test doest

Demo

I took this pr + #81707 to record this demo:

demo_720.mov

Use this branch to try it yourself: https://github.com/Dosant/kibana/tree/feature/data/management-ui-plus

Checklist

Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

src/plugins/expressions/common/service/expressions_services.ts

Dosant · 2020-12-03T10:02:24Z

@elasticmachine merge upstream

lukasolson

Haven't tested yet, but the approach seems good to me. I would like to hear from @lizozom and/or @ppisljar since they've been closer to this code.

src/plugins/data/public/search/session/session_service.ts

ppisljar · 2020-12-09T11:59:18Z

src/plugins/data/public/now_provider/now_provider.ts

+/**
+ * Used to synchronize time between parallel searches with relative time range that rely on `now`.
+ */
+export class NowProvider {


i am wondering why is this not part of session service ? i would expect that when creating a session its 'fixed in time' by converting the relative time to absolute.

It is not part of session service because it is also includes functionality the previously was part of time filter about getting now from the URL (so not only session can control "now"):
https://github.com/elastic/kibana/pull/84405/files/c92374a20f65fe24447bdef8c5b1dbf8c94dfeec#diff-fe41d6d47fbb6465cde6d519ae701f58b0259c89ecf3779048a49651a43b6276R29

Dosant · 2020-12-09T13:58:39Z

src/plugins/data/public/now_provider/lib/get_force_now_from_url.ts

 import { parse } from 'query-string';

 /** @internal */
-export function parseQueryString() {
+export function getForceNowFromUrl(): Date | undefined {


this moved from timefilter to this new nowProvider
Long term we should make apps responsible for getting this from the URL and setting it into nowProvider

src/plugins/data/public/query/timefilter/timefilter.ts

elasticmachine · 2020-12-09T17:21:24Z

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

Dosant · 2020-12-10T08:16:30Z

@elasticmachine merge upstream

tsullivan

I'm not vastly familiar with the code that is changed in this PR. My takeaway is the motivation seems to add a new contract called NowProvider.

we already had some forceNow logic for reporting. So some conversions could be missed and I am happy to follow on that

I'm not sure what you mean by "conversions could be missed". [1]

Can you add more info about what NowProvider is and how an app should use it? Also talk about what conversions you worked on in this PR to help clarify my understanding about [1].

src/plugins/data/common/search/expressions/esaggs/esaggs_fn.ts

…lative-timerange

lizozom

Reviewed the code thoroughly and tested the things I could think.
LGTM.

…lative-timerange # Conflicts: # src/plugins/data/public/public.api.md # src/plugins/data/public/search/session/search_session_state.ts # src/plugins/data/public/search/session/session_service.test.ts

ThomThomson

Code review only: Presentation team changes LGTM. Can't wait to see these changes in by default!

ThomThomson · 2021-01-07T16:35:24Z

src/plugins/dashboard/public/application/dashboard_app.tsx

@@ -169,6 +169,11 @@ export function DashboardApp({
        setLastReloadTime(() => new Date().getTime());
      })
    );
+    subscriptions.add(
+      data.query.timefilter.timefilter.getAutoRefreshFetch$().subscribe(() => {


Nit: We already have a shortcut const for timefilter declared above. It also might be a bit cleaner to merge the two subs that do the same thing:

subscriptions.add( merge(data.search.session.onRefresh$, timeFilter.getAutoRefreshFetch$()).subscribe(() => setLastReloadTime(() => new Date().getTime()) ) );

Improved 👍

lizozom · 2021-01-11T08:56:33Z

@elasticmachine merge upstream

Dosant · 2021-01-12T08:45:33Z

@elasticmachine merge upstream

stratoula

Kibana app code review changes LGTM.

Dosant · 2021-01-12T11:08:38Z

@elasticmachine merge upstream

kibanamachine · 2021-01-12T13:21:40Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 5f2a1b6

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`data`	612	614	+2

Async chunks

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

id	before	after	diff
`dashboard`	199.9KB	200.1KB	+172.0B
`data`	271.0KB	271.0KB	+1.0B
`discover`	502.2KB	502.0KB	-175.0B
`lens`	1.1MB	1.1MB	-224.0B
`visualize`	131.7KB	132.0KB	+325.0B
total			+99.0B

Page load bundle

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

id	before	after	diff
`data`	1003.8KB	1006.0KB	+2.2KB
`visualizations`	171.2KB	171.0KB	-236.0B
total			+2.0KB

History

💔 Build #98149 failed 3014cbf
💚 Build #97841 succeeded ed491c5
💚 Build #97814 succeeded 10deea5
💚 Build #97593 succeeded 2a7f482
💚 Build #97339 succeeded d001f1b

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

Dosant force-pushed the dev/search/relative-timerange branch from 42fd29a to a13e3ce Compare December 2, 2020 14:15

Dosant changed the title ~~[WIP][Search] Send to background with relative time range~~ [Feedback wanted][Search] Send to background with relative time range Dec 2, 2020

Dosant added Feature:Search Querying infrastructure in Kibana Team:AppServices labels Dec 2, 2020

Dosant requested review from lukasolson, lizozom, tsullivan and ppisljar and removed request for lukasolson December 2, 2020 18:01

Dosant commented Dec 2, 2020

View reviewed changes

src/plugins/expressions/common/service/expressions_services.ts Outdated Show resolved Hide resolved

lukasolson reviewed Dec 3, 2020

View reviewed changes

src/plugins/data/public/search/session/session_service.ts Outdated Show resolved Hide resolved

ppisljar reviewed Dec 9, 2020

View reviewed changes

Dosant commented Dec 9, 2020

View reviewed changes

Dosant added release_note:skip Skip the PR/issue when compiling release notes v7.12.0 v8.0.0 labels Dec 9, 2020

Dosant commented Dec 9, 2020

View reviewed changes

src/plugins/data/public/query/timefilter/timefilter.ts Outdated Show resolved Hide resolved

Dosant changed the title ~~[Feedback wanted][Search] Send to background with relative time range~~ [Search] Send to background with relative time range Dec 9, 2020

Dosant marked this pull request as ready for review December 9, 2020 17:21

Dosant requested a review from a team December 9, 2020 17:21

Dosant requested review from a team as code owners December 9, 2020 17:21

tsullivan reviewed Dec 28, 2020

View reviewed changes

Dosant requested a review from a team as a code owner January 4, 2021 18:58

send to background with relative timerange

05b927f

Dosant force-pushed the dev/search/relative-timerange branch from 923baf9 to 05b927f Compare January 5, 2021 11:59

ppisljar reviewed Jan 6, 2021

View reviewed changes

src/plugins/data/common/search/expressions/esaggs/esaggs_fn.ts Show resolved Hide resolved

Dosant added 2 commits January 6, 2021 17:18

Merge branch 'master' of github.com:elastic/kibana into dev/search/re…

543203e

…lative-timerange

move conversion to handleRequest based on discussion with @ppisljar

d001f1b

Dosant requested review from ppisljar, lizozom, lukasolson and tsullivan January 6, 2021 16:33

lizozom changed the title ~~[Search] Send to background with relative time range~~ [Search] Search Sessions with relative time range Jan 6, 2021

lizozom approved these changes Jan 6, 2021

View reviewed changes

Dosant added 2 commits January 7, 2021 17:06

Merge branch 'master' of github.com:elastic/kibana into dev/search/re…

8da16d0

…lative-timerange # Conflicts: # src/plugins/data/public/public.api.md # src/plugins/data/public/search/session/search_session_state.ts # src/plugins/data/public/search/session/session_service.test.ts

fix

2a7f482

ThomThomson approved these changes Jan 7, 2021

View reviewed changes

kibanamachine and others added 2 commits January 11, 2021 03:56

Merge branch 'master' into dev/search/relative-timerange

10deea5

review

ed491c5

Merge branch 'master' into dev/search/relative-timerange

3014cbf

stratoula approved these changes Jan 12, 2021

View reviewed changes

Merge branch 'master' into dev/search/relative-timerange

5f2a1b6

Dosant mentioned this pull request Jan 12, 2021

[Search][Discover] Send to background integration improvements and fixes #87311

Merged

1 task

Dosant merged commit d3303f2 into elastic:master Jan 12, 2021

Dosant mentioned this pull request Jan 12, 2021

[7.x] [Search] Search Sessions with relative time range (#84405) #88013

Merged

Dosant added a commit to Dosant/kibana that referenced this pull request Jan 12, 2021

[Search] Search Sessions with relative time range (elastic#84405)

b782c0f

Dosant added a commit that referenced this pull request Jan 12, 2021

[Search] Search Sessions with relative time range (#84405) (#88013)

efaebd8

flash1293 mentioned this pull request Jan 18, 2021

[Lens] Restart session if fixed now becomes outdated #88575

Merged

lizozom mentioned this pull request Jan 18, 2021

[Search][Session] Make sure relative timeranges are handled correctly #82626

Closed

Dosant mentioned this pull request Jan 26, 2021

[Maps][Search Sessions] Pass down sessionId from embeddable to search calls #85449

Closed

Dosant mentioned this pull request Apr 8, 2021

Maps embeddable to respect containers' auto-refresh #96550

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Search] Search Sessions with relative time range #84405

[Search] Search Sessions with relative time range #84405

Dosant commented Nov 26, 2020 •

edited

Loading

Dosant commented Dec 3, 2020

lukasolson left a comment

ppisljar Dec 9, 2020

Dosant Dec 9, 2020

Dosant Dec 9, 2020

elasticmachine commented Dec 9, 2020

Dosant commented Dec 10, 2020

tsullivan left a comment

lizozom left a comment

ThomThomson left a comment •

edited

Loading

ThomThomson Jan 7, 2021

Dosant Jan 11, 2021

lizozom commented Jan 11, 2021

Dosant commented Jan 12, 2021

stratoula left a comment

Dosant commented Jan 12, 2021

kibanamachine commented Jan 12, 2021

[Search] Search Sessions with relative time range #84405

[Search] Search Sessions with relative time range #84405

Conversation

Dosant commented Nov 26, 2020 • edited Loading

Summary

How to test

Demo

Checklist

For maintainers

Dosant commented Dec 3, 2020

lukasolson left a comment

Choose a reason for hiding this comment

ppisljar Dec 9, 2020

Choose a reason for hiding this comment

Dosant Dec 9, 2020

Choose a reason for hiding this comment

Dosant Dec 9, 2020

Choose a reason for hiding this comment

elasticmachine commented Dec 9, 2020

Dosant commented Dec 10, 2020

tsullivan left a comment

Choose a reason for hiding this comment

lizozom left a comment

Choose a reason for hiding this comment

ThomThomson left a comment • edited Loading

Choose a reason for hiding this comment

ThomThomson Jan 7, 2021

Choose a reason for hiding this comment

Dosant Jan 11, 2021

Choose a reason for hiding this comment

lizozom commented Jan 11, 2021

Dosant commented Jan 12, 2021

stratoula left a comment

Choose a reason for hiding this comment

Dosant commented Jan 12, 2021

kibanamachine commented Jan 12, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

Dosant commented Nov 26, 2020 •

edited

Loading

ThomThomson left a comment •

edited

Loading