[Search] Integrate "Send to background" UI with session service #83073

Dosant · 2020-11-10T16:02:23Z

Summary

This pr connects #81004 + #81099 making send to background indicator/popover more-or-less functional and we can see how the feature is shaping up.

Notes

Integrates only with discover and dashboard
Only absolute time ranges
Background session management is not implemented yet, so link goes to nowhere
There are already some known/existing bugs and follow-up enhancements, I am keeping a list here [Search] Send to background #83640.
- A notable bug: on a dashboard on some state changes (e.g. switching view/edit mode) background session indicator disappears until refresh is hit. I want to fix it separately as it is more of a dashboard change, then search layer change.
~~There is some TODOs in functional tests. Some units tests are missing. I will continue while it is being reviewed.~~ DONE
Client side session service split into two services. One is pure CRUD mostly for management screen. Another is tracking state of current session.

How to test

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

To try a restore flow you can build a restore session URL, by getting sessionId from inspector and appending to the URL: &searchSessionId=${your-id-from-inspector}. (See tests)

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

apmmachine · 2020-11-16T16:48:48Z

💚 Build Succeeded

pipeline
Commit: be48317

kibanamachine · 2020-11-17T17:50:12Z

⏳ Build in-progress, with failures

continuous-integration/kibana-ci/pull-request
Commit: 19d1915
This comment will update when the build is complete

Failed CI Steps

History

💔 Build #88306 failed f1275db
💔 Build #88059 failed be48317
💔 Build #87128 failed 19ab1ae

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

kibanamachine · 2020-11-18T13:29:11Z

⏳ Build in-progress, with failures

continuous-integration/kibana-ci/pull-request
Commit: b037743
This comment will update when the build is complete

Failed CI Steps

History

💔 Build #88424 failed 1fbfdaa
💔 Build #88386 failed 79c917b
💔 Build #88366 failed 19d1915
💔 Build #88306 failed f1275db
💔 Build #88059 failed be48317

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

wip docs

…rver-session-wip

elasticmachine · 2020-11-20T12:50:52Z

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

…rver-session-wip

kertal · 2020-11-26T12:17:40Z

@kertal, that setting indeed is not helpful for discover :(

you can at least test restore flow where you saved to background AFTER search is finished.

So when I saved the session, use the id in inspect in the URL, I'm getting an error message:

How can I see all the sessions that are available? thx!

Dosant · 2020-11-26T12:56:43Z

@kertal, Do you search with relative time range? It doesn't work for relative time range just yet (I noted that in pr description). This will be handled separately.
The error means that session service can't match older search request with your current search request (in this case because of difference in time due to relative time range)

How can I see all the sessions that are available? thx!

Management UI is not implemented yet, but if needed you could use API (session service) or look for saved objects of type background-session

kertal

Code LGTM, tested locally in Chrome, Firefox, works 👍 . Tested "restore flow where you saved to background AFTER search is finished". Thx for the refactoring

Dosant · 2020-11-26T18:58:22Z

@elasticmachine merge upstream

Dosant · 2020-11-30T07:37:35Z

@elasticmachine merge upstream

lizozom · 2020-11-25T13:10:14Z

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

+  public find(
+    options: SearchSessionFindOptions
+  ): Promise<SavedObjectsFindResponse<BackgroundSessionSavedObjectAttributes>> {
+    return this.http!.post(`/internal/session`, {


//nit
Shouldn't find use http.get?

I agree get makes more sense, especially since core.savedObjectClient also uses get
But this wasn't aded in this pr so out of scope here. cc @lukasolson

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

src/plugins/dashboard/public/url_generator.ts

src/plugins/data/public/search/search_interceptor.ts

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

src/plugins/discover/public/application/angular/discover.js

lizozom

I just realized that when a session is saved, we still cancel it upon navigating away.
Saved sessions should be allowed to run upon refresh / navigating away / etc.

In terms of the UI, I think we have too many spinners running

I also think the tooltip of the indicator needs a slight delay, and generally has some weird behavior about it

…rver-session-wip

Dosant · 2020-11-30T16:56:20Z

@lizozom, thanks for the review:

I just realized that when a session is saved, we still cancel it upon navigating away.
Saved sessions should be allowed to run upon refresh / navigating away / etc.

Great catch ❤️ I added logic to not delete async search in case this search was saved with background session. Unfortunately code to support this doesn't look slick, but it is the most reliable way I could think of.

In terms of the UI, I think we have too many spinners running

Something to think about... I'll add it here: #83640

I also think the tooltip of the indicator needs a slight delay, and generally has some weird behavior about it

Thanks! I caught it once and was thinking I went crazy. I didn't find a way to reliably reproduce it nor a way to fix it.
I am putting a note here and will follow up. #83640
Worst case will remove that tooltip all together.

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

lizozom · 2020-12-01T08:38:52Z

x-pack/plugins/data_enhanced/public/search/search_interceptor.ts

+      this.deps.session.state$
+        .pipe(
+          skip(1), // ignore any state, we are only interested in transition x -> BackgroundLoading
+          filter((state) => isCurrentSession() && state === SessionState.BackgroundLoading),


Same should apply if the session has finished loading.

Why not use sessionService.isStored instead?

Why not use sessionService.isStored instead?

This is what I initially thought of, but it didn't work for me:
sessionService tracks state of the CURRENT session and at the point when we cancel the search it might be already tracking different session.

Let's assume we have a search that will be aborted because of navigating away.
Consider this scenario:

Session is created, search is fired

before we start search, isStored is still false

Session is saved, isStored becomes true

User navigates away. session is cleared.

Search gets canceled, we are in catchError, sessionService.isStored is false, because session was cleared. So we don't know if the search that was cancelled was stored or not.

To support this use case using session service we should keep state of older sessions.
Other simpler alternative for this specific use case was to subscribe to state change into backgroundLoading state and keep that bool flag in closure.

Same should apply if the session has finished loading.

Don't think so, it seems that transition to BackgroundLoading is what we really interested in, if I am not missing something.

…rver-session-wip

…ana into dev/search-server-session-wip

kibanamachine · 2020-12-01T11:16:55Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 5660e56

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`dashboard`	185	186	+1
`data`	599	602	+3
`dataEnhanced`	38	37	-1
total			+3

Async chunks

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

id	before	after	diff
`dashboard`	193.2KB	197.2KB	+4.0KB
`dataEnhanced`	26.3KB	27.6KB	+1.3KB
`discover`	430.2KB	432.2KB	+2.0KB
total			+7.2KB

Page load bundle

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

id	before	after	diff
`dashboard`	321.3KB	322.2KB	+951.0B
`data`	974.2KB	983.3KB	+9.1KB
`dataEnhanced`	35.2KB	35.1KB	-121.0B
`discover`	81.8KB	82.1KB	+213.0B
total			+10.1KB

Saved Objects .kibana field count

Every field in each saved object type adds overhead to Elasticsearch. Kibana needs to keep the total field count below Elasticsearch's default limit of 1000 fields. Only specify field mappings for the fields you wish to search on or query. See https://www.elastic.co/guide/en/kibana/master/development-plugin-saved-objects.html#_mappings

id	before	after	diff
`background-session`	5	7	+2

History

💚 Build #90951 succeeded 05400d1
💚 Build #90795 succeeded b9bbbdb
💚 Build #90596 succeeded d128732
💚 Build #90496 succeeded 84de299
💚 Build #90279 succeeded 14f47c3

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

lizozom

LGTM

…tic#83073)

…) (#84635)

* master: (63 commits) Revert the Revert of "[Alerting] renames Resolved action group to Recovered (elastic#84123)" (elastic#84662) declare kbn/monaco dependency on kbn/i18n explicitly (elastic#84660) Remove unscripted fields from sample data index-pattern saved objects (elastic#84659) [ML] Fix unnecessary trigger of wildcard field type search for ML plugin routes. (elastic#84605) Update create.asciidoc (elastic#84046) [Security Solution][Detections] Fix labels and issue with mandatory fields (elastic#84525) Fix flaky test suite (elastic#84602) [Security Solution] [Detections] Create a 'partial failure' status for rules (elastic#84293) Revert "[Alerting] renames Resolved action group to Recovered (elastic#84123)" Update code-comments describing babel plugins (elastic#84622) [Security Solution] [Cases] Cypress for case connector selector options (elastic#80745) [Discover] Unskip doc table tests (elastic#84564) [Lens] (Accessibility) Improve landmarks in Lens (elastic#84511) [Lens] (Accessibility) Focus mistakenly stops on righthand form (elastic#84519) Return early when parallel install process detected (elastic#84190) [Security Solution][Detections] Support arrays in event fields for Severity/Risk overrides (elastic#83723) [Security Solution][Detections] Fix grammatical error in validation message for threshold field in "Create new rule" -> "Define rule" (elastic#84490) [Fleet] Update agent details page (elastic#84434) adding documentation of use of NODE_EXTRA_CA_CERTS env var (elastic#84578) [Search] Integrate "Send to background" UI with session service (elastic#83073) ...

Dosant requested a review from lukasolson November 10, 2020 16:02

Dosant mentioned this pull request Nov 18, 2020

[Search] Send to background #83640

Closed

38 tasks

wip

3e102cd

wip docs

Dosant force-pushed the dev/search-server-session-wip branch from b037743 to 3e102cd Compare November 19, 2020 13:59

fix

112d5e3

Dosant changed the title ~~[very wip] send to background UI + server session service~~ [Search] Integrate "Send to background" UI with session service Nov 19, 2020

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

bc6ffbd

…rver-session-wip

Dosant added Feature:Search Querying infrastructure in Kibana Team:AppServices v7.11.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Nov 19, 2020

Dosant marked this pull request as ready for review November 20, 2020 12:50

Dosant requested a review from a team as a code owner November 20, 2020 12:50

Dosant requested a review from a team November 20, 2020 12:50

Dosant requested review from a team as code owners November 20, 2020 12:50

Dosant requested a review from lizozom November 20, 2020 13:37

Dosant added 7 commits November 20, 2020 16:53

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

f698b4a

…rver-session-wip

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

3143c4d

…rver-session-wip

tests

95a1b83

tets

735a651

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

5848f6c

…rver-session-wip

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

0765798

…rver-session-wip

prettier bump

31187f5

kertal requested review from kertal and lukasolson November 26, 2020 18:40

kertal approved these changes Nov 26, 2020

View reviewed changes

Merge branch 'master' into dev/search-server-session-wip

84de299

Merge branch 'master' into dev/search-server-session-wip

d128732

lizozom reviewed Nov 30, 2020

View reviewed changes

Dosant added 3 commits November 30, 2020 14:27

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

400760b

…rver-session-wip

do not delete saved searches

150d90a

@lizozom review

b9bbbdb

Update search_interceptor.ts

05400d1

lizozom reviewed Dec 1, 2020

View reviewed changes

Dosant added 3 commits December 1, 2020 10:33

fix

70adabd

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

c16ba67

…rver-session-wip

Merge branch 'dev/search-server-session-wip' of github.com:Dosant/kib…

5660e56

…ana into dev/search-server-session-wip

lizozom approved these changes Dec 1, 2020

View reviewed changes

Dosant merged commit 4cb44d9 into elastic:master Dec 1, 2020

Dosant added a commit to Dosant/kibana that referenced this pull request Dec 1, 2020

[Search] Integrate "Send to background" UI with session service (elas…

462c59b

…tic#83073)

Dosant mentioned this pull request Dec 1, 2020

[7.x] [Search] Integrate "Send to background" UI with session service (#83073) #84635

Merged

Dosant added a commit that referenced this pull request Dec 1, 2020

[Search] Integrate "Send to background" UI with session service (#83073…

b59c652

…) (#84635)

Dosant mentioned this pull request Dec 3, 2020

[Lens] Encourage user to use smaller data set if searches take too long #84908

Closed

nreese mentioned this pull request Dec 16, 2020

[Maps] investigate search sessions #86109

Closed

liza-mae mentioned this pull request Jan 25, 2021

Setting for enhanced search send to background throws error definition for key is missing #89081

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Search] Integrate "Send to background" UI with session service #83073

[Search] Integrate "Send to background" UI with session service #83073

Dosant commented Nov 10, 2020 •

edited

Loading

apmmachine commented Nov 16, 2020

kibanamachine commented Nov 17, 2020

kibanamachine commented Nov 18, 2020

elasticmachine commented Nov 20, 2020

kertal commented Nov 26, 2020

Dosant commented Nov 26, 2020

kertal left a comment

Dosant commented Nov 26, 2020

Dosant commented Nov 30, 2020

lizozom Nov 25, 2020 •

edited

Loading

Dosant Nov 30, 2020

lizozom left a comment •

edited

Loading

Dosant commented Nov 30, 2020

lizozom Dec 1, 2020

lizozom Dec 1, 2020

Dosant Dec 1, 2020

kibanamachine commented Dec 1, 2020

lizozom left a comment

[Search] Integrate "Send to background" UI with session service #83073

[Search] Integrate "Send to background" UI with session service #83073

Conversation

Dosant commented Nov 10, 2020 • edited Loading

Summary

Notes

How to test

Checklist

For maintainers

apmmachine commented Nov 16, 2020

💚 Build Succeeded

kibanamachine commented Nov 17, 2020

⏳ Build in-progress, with failures

Failed CI Steps

History

kibanamachine commented Nov 18, 2020

⏳ Build in-progress, with failures

Failed CI Steps

History

elasticmachine commented Nov 20, 2020

kertal commented Nov 26, 2020

Dosant commented Nov 26, 2020

kertal left a comment

Choose a reason for hiding this comment

Dosant commented Nov 26, 2020

Dosant commented Nov 30, 2020

lizozom Nov 25, 2020 • edited Loading

Choose a reason for hiding this comment

Dosant Nov 30, 2020

Choose a reason for hiding this comment

lizozom left a comment • edited Loading

Choose a reason for hiding this comment

Dosant commented Nov 30, 2020

lizozom Dec 1, 2020

Choose a reason for hiding this comment

lizozom Dec 1, 2020

Choose a reason for hiding this comment

Dosant Dec 1, 2020

Choose a reason for hiding this comment

kibanamachine commented Dec 1, 2020

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

Saved Objects .kibana field count

History

lizozom left a comment

Choose a reason for hiding this comment

Dosant commented Nov 10, 2020 •

edited

Loading

lizozom Nov 25, 2020 •

edited

Loading

lizozom left a comment •

edited

Loading