[data.search] Server-side background session service #81099

lukasolson · 2020-10-20T02:02:46Z

Summary

Adds the server-side background session service, allowing background search sessions to be created, deleted, listed, and restored. It also adds support to the client-side session service for accessing these methods.

The basic flow is as follows:

// Initialize the session:
const sessionId = data.search.session.start();

// Search using the given session (works the same pre/post-restore):
data.search.session.search(searchRequest, {
  sessionId,
  isStored: data.search.session.isStored(), // Has this session already been saved?
  isRestore: data.search.session.isRestore(), // Has this session been restored?
});

// When sending to background:
data.search.session.save('background session name', `/restore_url?sessionId=${sessionId}`);

// When restoring:
data.search.session.restore(sessionId);

(See the search example plugin for an example of how to use.)

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)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

Co-authored-by: Anton Dosov <dosantappdev@gmail.com>

…search-as-scoped

Dosant · 2020-10-27T10:51:48Z

src/plugins/data/common/search/session/utils.ts

+ * under the License.
+ */
+
+import { createHash } from 'crypto';


Seems there is already a trimmed down version of exactly what we need in core: src/core/public/utils/crypto/sha256.ts

Since this code is in /common and has a risk of getting into UI, should we consider using a trimmed down version from core now?
(Small caveat is that we will need to move that util to common first: src/core/public/utils/crypto/sha256.ts -> src/core/common/utils/crypto/sha256.ts)

Dosant

LGTM.

Using it in #83073 and in general seem to be working well.

Some remarks, don't think we need to fix it in this pr as can be done in parallel and unblock client side reviews:

Client side session service will be improved. (observables, missing methods, types, etc..)
In some places error handling needs to be improved. I often noticed "500 Internal service error" while testing. For example, when trying to restore searches and hash mismatches
Bumped couple times into race condition: ResponseError: [background-session:63112a17-1f82-4711-ab7d-6b2d2e974ae4]: version conflict, required seqNo [379], primary term [1]. current document has seqNo [380] and primary term [1]: version_conflict_engine_exception. I think this is happening when you get back to inactive tab.
Multi-tenant Kibana use-case
For some reason kibana complained about missing userId in background search SO mapping. Maybe I had some stale data?

Dosant · 2020-11-17T10:59:07Z

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

+  }
+
+  public delete(sessionId: string) {
+    return this.http!.delete(`/internal/session/${encodeURIComponent(sessionId)}}`);


Suggested change

return this.http!.delete(`/internal/session/${encodeURIComponent(sessionId)}}`);

return this.http!.delete(`/internal/session/${encodeURIComponent(sessionId)}`);

Dosant · 2020-11-17T10:59:23Z

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

+  }
+
+  public update(sessionId: string, attributes: Partial<BackgroundSessionSavedObjectAttributes>) {
+    return this.http!.put(`/internal/session/${encodeURIComponent(sessionId)}}`, {


Suggested change

return this.http!.put(`/internal/session/${encodeURIComponent(sessionId)}}`, {

return this.http!.put(`/internal/session/${encodeURIComponent(sessionId)}`, {

Dosant · 2020-11-17T10:59:40Z

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

+  }
+
+  public get(sessionId: string) {
+    return this.http!.get(`/internal/session/${encodeURIComponent(sessionId)}}`);


Suggested change

return this.http!.get(`/internal/session/${encodeURIComponent(sessionId)}}`);

return this.http!.get(`/internal/session/${encodeURIComponent(sessionId)}`);

Dosant · 2020-11-17T11:03:00Z

src/plugins/data/common/search/session/types.ts

+  update: (
+    sessionId: string,
+    attributes: Partial<BackgroundSessionSavedObjectAttributes>
+  ) => Promise<any>;


Would would be the proper return types for this CRUD methods?
I can add in follow up client side PR, just not sure what are those.

lizozom

This is an intermediate review, Ill add some more after testing

lizozom · 2020-11-17T12:49:44Z

src/plugins/data/common/search/session/types.ts

+
+export interface BackgroundSessionSavedObjectAttributes {
+  name: string;
+  url: string;


I thing we shouldn't have a URL, but an app name and two sets of parameters generated using the URL Generator (original and restore URLs)

Are you okay with leaving this to the PR that integrates with the URL generator? If you prefer, I can leave out the url portion altogether from this PR

Lets leave it out then

lizozom · 2020-11-17T12:53:36Z

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

  }

  public clear() {
+    this._isStored = false;
+    this._isRestore = true;


Why do you set this to true?

Should be false, not sure how that got through

lizozom · 2020-11-17T12:54:49Z

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

@@ -30,6 +34,18 @@ export class SessionService implements ISessionService {
  }
  private appChangeSubscription$?: Subscription;
  private curApp?: string;
+  private http?: HttpStart;


Maybe instead define private http!: HttpStart; and then we won't have to check all the time?

Didn't even know that was a thing

lizozom · 2020-11-17T13:00:47Z

src/plugins/data/server/saved_objects/background_session.ts

+      name: {
+        type: 'keyword',
+      },
+      url: {


I'd add a creation time, or all SOs have one?

We need the appId, and two sets of URL params (restore and original)

I think we also should save the status (we want to filter by it)

SOs will have updated_at but I'm not seeing a creation time. I'll add that and the status

lizozom · 2020-11-17T13:02:57Z

src/plugins/data/server/search/routes/session.ts

+    }
+  );
+
+  router.post(


// nit
Why use POST for find?
Sounds like a classic GET to me.

Yeah, that's what I thought initially too, but we need to send the FindOptions. I guess we could send them in as query parameters instead of inside the body. Thoughts?

I'd just do the same thing that core does with their client

lizozom · 2020-11-17T13:08:39Z

src/plugins/data/server/search/routes/session.ts

+          body: response,
+        });
+      } catch (err) {
+        return res.customError({


Maybe we could get a util for the error format? I see this block everywhere (npt necessarily in this PR)

lizozom · 2020-11-17T13:15:39Z

src/plugins/data/server/search/routes/search.ts

+        body: schema.object(
+          {
+            sessionId: schema.maybe(schema.string()),
+            isStored: schema.maybe(schema.boolean()),


Could we use a schema.conditional here?

lizozom · 2020-11-17T14:53:27Z

x-pack/plugins/data_enhanced/server/search/es_search_strategy.ts

@@ -57,6 +57,7 @@ export const enhancedEsSearchStrategyProvider = (
          utils.toSnakeCase({
            ...(await getDefaultSearchParams(uiSettingsClient)),
            batchedReduceSize: 64,
+            keepOnCompletion: true, // Always return an ID, even if the request completes quickly


Why did we add this? Sounds like a serious overhead to the system, that applies even if we dont save a single session

We need to ensure that all requests generate IDs and return them. Otherwise, how will we get back the original data when the session is restored?

src/plugins/data/server/search/search_service.ts

lukasolson · 2020-11-17T16:11:19Z

For some reason kibana complained about missing userId in background search SO mapping. Maybe I had some stale data?

Yeah, that is probably the case. I removed it for now and will re-add in a follow-up PR.

lizozom · 2020-11-18T08:54:20Z

@elasticmachine merge upstream

lizozom

Added a few more comments
Overall LGTM

lizozom · 2020-11-18T10:06:47Z

src/plugins/data/common/search/session/status.ts

+ */
+
+export enum BackgroundSessionStatus {
+  INCOMPLETE = 'incomplete',


//nit IN_PROGRESS?

lizozom · 2020-11-18T10:09:37Z

src/plugins/data/common/search/session/types.ts

+  /**
+   * Deletes a session
+   */
+  delete: (sessionId: string) => Promise<any>;


Returns Promise<boolean>?

Hmm, probably Promise<void> since an unsuccessful delete will throw.

lizozom · 2020-11-18T10:09:58Z

src/plugins/data/common/search/session/types.ts

+
+export interface BackgroundSessionSavedObjectAttributes {
+  name: string;
+  url: string;


Lets leave it out then

lizozom · 2020-11-18T11:52:51Z

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

+    sessionId: string,
+    name: string,
+    url: string,
+    expires: Date = new Date(Date.now() + DEFAULT_EXPIRATION),


If I'm not mistaken, the actual expiration is returned with each async search response, maybe we could use that?

There could be multiple requests/responses for a single session. What we should actually probably be doing here is actually calling the extend function (when it is implemented) to set the expires in each search to this value, rather than the other way around. Thoughts?

lizozom · 2020-11-18T11:54:12Z

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

+    return session;
+  };
+
+  // TODO: Throw an error if this session doesn't belong to this user


Doesn't savedObjectClient already do that, as it's scoped?

If two users are in the same space, they can read/write each other's objects without errors, which is expected. Here we need to enforce that even if a user is in the same space, they can't access another user's session.

kibanamachine · 2020-11-19T01:22:44Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: a3dc0d8

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`data`	597	599	+2

Distributable file count

id	before	after	diff
`default`	42853	42860	+7
`oss`	22474	22481	+7

Page load bundle

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

id	before	after	diff
`data`	965.9KB	970.5KB	+4.5KB
`dataEnhanced`	36.1KB	36.3KB	+206.0B
total			+4.8KB

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	+5

History

💔 Build #88798 failed 9bbfa3a
💚 Build #88754 succeeded 8d81f39
💔 Build #88724 failed dc6104c
💔 Build #88505 failed 645146b
💔 Build #88488 failed 967214d

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

* [Search] Add request context and asScoped pattern * Update docs * Unify interface for getting search client * [WIP] [data.search] Server-side background session service * Update examples/search_examples/server/my_strategy.ts Co-authored-by: Anton Dosov <dosantappdev@gmail.com> * Review feedback * Fix checks * Add tapFirst and additional props for session * Fix CI * Fix security search * Fix test * Fix test for reals * Add restore method * Add code to search examples * Add restore and search using restored ID * Fix handling of preference and order of params * Trim & cleanup * Fix types * Review feedback * Add tests and remove handling of username * Update docs * Move utils to server * Review feedback * More review feedback * Regenerate docs * Review feedback * Doc changes Co-authored-by: Anton Dosov <dosantappdev@gmail.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* master: (60 commits) Forward any registry cache-control header for files (elastic#83680) Revert "[Alerting] Add `alert.updatedAt` field to represent date of last user edit (elastic#83578)" [Security Solution][Detections] Fix adding an action to detection rules (elastic#83722) Make expectSnapshot available in all functional test runs (elastic#82932) Skip failing cypress test Increase bulk request timeout during esArchiver load (elastic#83657) [data.search] Server-side background session service (elastic#81099) [maps] convert VectorStyleEditor to TS (elastic#83582) Revert "[App Search] Engine overview layout stub (elastic#83504)" Adding documentation for global action configuration options (elastic#83557) [Metrics UI] Optimizations for Snapshot and Inventory Metadata (elastic#83596) chore(NA): update lmdb store to v0.8.15 (elastic#83726) [App Search] Engine overview layout stub (elastic#83504) [Workplace Search] Update SourceIcon to match latest changes in ent-search (elastic#83714) [Enterprise Search] Rename React Router helpers (elastic#83718) [Maps] Add 'crossed' & 'exited' events to tracking alert (elastic#82463) Updating code-owners to use new core/app-services team names (elastic#83731) Add Managed label to data streams and a view switch for the table (elastic#83049) [Maps] Add query bar inputs to geo threshold alerts tracked points & boundaries (elastic#80871) fix(NA): search examples kibana version declaration (elastic#83182) ...

…ode-details * 'master' of github.com:elastic/kibana: fixed pagination in connectors list (elastic#83638) Forward any registry cache-control header for files (elastic#83680) Revert "[Alerting] Add `alert.updatedAt` field to represent date of last user edit (elastic#83578)" [Security Solution][Detections] Fix adding an action to detection rules (elastic#83722) Make expectSnapshot available in all functional test runs (elastic#82932) Skip failing cypress test Increase bulk request timeout during esArchiver load (elastic#83657) [data.search] Server-side background session service (elastic#81099) [maps] convert VectorStyleEditor to TS (elastic#83582) Revert "[App Search] Engine overview layout stub (elastic#83504)" Adding documentation for global action configuration options (elastic#83557) [Metrics UI] Optimizations for Snapshot and Inventory Metadata (elastic#83596) chore(NA): update lmdb store to v0.8.15 (elastic#83726) [App Search] Engine overview layout stub (elastic#83504) [Workplace Search] Update SourceIcon to match latest changes in ent-search (elastic#83714) [Enterprise Search] Rename React Router helpers (elastic#83718)

* [Search] Add request context and asScoped pattern * Update docs * Unify interface for getting search client * [WIP] [data.search] Server-side background session service * Update examples/search_examples/server/my_strategy.ts Co-authored-by: Anton Dosov <dosantappdev@gmail.com> * Review feedback * Fix checks * Add tapFirst and additional props for session * Fix CI * Fix security search * Fix test * Fix test for reals * Add restore method * Add code to search examples * Add restore and search using restored ID * Fix handling of preference and order of params * Trim & cleanup * Fix types * Review feedback * Add tests and remove handling of username * Update docs * Move utils to server * Review feedback * More review feedback * Regenerate docs * Review feedback * Doc changes Co-authored-by: Anton Dosov <dosantappdev@gmail.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Anton Dosov <dosantappdev@gmail.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* [Search] Add request context and asScoped pattern * Update docs * Unify interface for getting search client * [WIP] [data.search] Server-side background session service * Update examples/search_examples/server/my_strategy.ts Co-authored-by: Anton Dosov <dosantappdev@gmail.com> * Review feedback * Fix checks * Add tapFirst and additional props for session * Fix CI * Fix security search * Fix test * Fix test for reals * Add restore method * Add code to search examples * Add restore and search using restored ID * Fix handling of preference and order of params * Trim & cleanup * Fix types * Review feedback * Add tests and remove handling of username * Update docs * Move utils to server * Review feedback * More review feedback * Regenerate docs * Review feedback * Doc changes Co-authored-by: Anton Dosov <dosantappdev@gmail.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

lukasolson added 5 commits October 15, 2020 15:16

[Search] Add request context and asScoped pattern

26c47bd

Update docs

36b8fcb

Merge branch 'master' into search-as-scoped

8274226

Unify interface for getting search client

c7296f0

[WIP] [data.search] Server-side background session service

d77c009

lukasolson requested a review from a team as a code owner October 20, 2020 02:02

lukasolson requested a review from a team October 20, 2020 02:02

lukasolson requested review from a team as code owners October 20, 2020 02:02

lukasolson self-assigned this Oct 20, 2020

lukasolson and others added 4 commits October 20, 2020 10:08

Update examples/search_examples/server/my_strategy.ts

9dadfb2

Co-authored-by: Anton Dosov <dosantappdev@gmail.com>

Merge branch 'master' into search-as-scoped

ab55bcb

Review feedback

2b3d2a0

Merge branch 'search-as-scoped' of github.com:lukasolson/kibana into …

7a048a0

…search-as-scoped

Dosant reviewed Oct 27, 2020

View reviewed changes

lukasolson added 14 commits October 27, 2020 09:34

Merge branch 'master' into search-as-scoped

2d0da67

Merge branch 'master' into search-as-scoped

f7cb5a1

Fix checks

6c9aec5

Merge branch 'search-as-scoped' into search-server-session

5b9b0c9

Add tapFirst and additional props for session

260e46c

Fix CI

1ae55ef

Fix security search

4634fb4

Merge branch 'master' into search-as-scoped

c35fb49

Merge branch 'master' into search-as-scoped

1567995

Fix test

5f7c9ab

Fix test for reals

f1b515d

Merge branch 'master' into search-server-session

a1d67ae

Merge branch 'search-as-scoped' into search-server-session

9dbd645

Add restore method

e32a435

lukasolson added the Feature:Search Querying infrastructure in Kibana label Nov 3, 2020

Fix types

0b10e22

lukasolson changed the title ~~[skip-ci] [WIP] [data.search] Server-side background session service~~ [data.search] Server-side background session service Nov 12, 2020

lukasolson added 3 commits November 12, 2020 12:11

Review feedback

700f2a0

Add tests and remove handling of username

0f34c4c

Update docs

c4e63c2

lukasolson added the review label Nov 12, 2020

lukasolson added 2 commits November 16, 2020 15:03

Merge branch 'master' into search-server-session

5c15656

Move utils to server

06e83fb

Dosant approved these changes Nov 17, 2020

View reviewed changes

lizozom reviewed Nov 17, 2020

View reviewed changes

Review feedback

967214d

Merge branch 'master' into search-server-session

645146b

lizozom approved these changes Nov 18, 2020

View reviewed changes

lukasolson added 2 commits November 18, 2020 13:24

More review feedback

dc6104c

Regenerate docs

8d81f39

botelastic bot added the Feature:Embedding Embedding content via iFrame label Nov 18, 2020

lukasolson added 2 commits November 18, 2020 16:14

Review feedback

9bbfa3a

Doc changes

a3dc0d8

lukasolson merged commit 9b30de4 into elastic:master Nov 19, 2020

lukasolson mentioned this pull request Nov 19, 2020

[7.x] [data.search] Server-side background session service (#81099) #83766

Merged

Dosant mentioned this pull request Nov 19, 2020

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

Merged

2 tasks

	return this.http!.delete(`/internal/session/${encodeURIComponent(sessionId)}}`);
	return this.http!.delete(`/internal/session/${encodeURIComponent(sessionId)}`);

	return this.http!.put(`/internal/session/${encodeURIComponent(sessionId)}}`, {
	return this.http!.put(`/internal/session/${encodeURIComponent(sessionId)}`, {

	return this.http!.get(`/internal/session/${encodeURIComponent(sessionId)}}`);
	return this.http!.get(`/internal/session/${encodeURIComponent(sessionId)}`);

[data.search] Server-side background session service #81099

[data.search] Server-side background session service #81099

Conversation

lukasolson commented Oct 20, 2020 • edited Loading

Summary

Checklist

For maintainers

Choose a reason for hiding this comment

Dosant left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lizozom left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukasolson commented Nov 17, 2020

lizozom commented Nov 18, 2020

lizozom left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lizozom Nov 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibanamachine commented Nov 19, 2020

💚 Build Succeeded

Metrics [docs]

Module Count

Distributable file count

Page load bundle

Saved Objects .kibana field count

History

lukasolson commented Oct 20, 2020 •

edited

Loading

Dosant left a comment •

edited

Loading

lizozom Nov 18, 2020 •

edited

Loading