pass elasticsearch response to browser as a stream #193060

ppisljar · 2024-09-16T17:14:55Z

Remove decoding and re-encoding of the ES-response from Kibana-server. Resolves #189640

Only works with bfetch disabled (bfetch:disable advanced setting set to true, which is the default)
only works with async esql and async dsl search strategies
In above cases it does not parse ES response server-side, but forwards the Readable stream from the ES JS client to the HTTP response
Reads ID (x-elasticsearch-async-id) and completion status (x-elasticsearch-async-is-running) from headers so that parsing the body can happen on the client rather than both on the client & server

How to test:
when using ese or async esql search strategies (discover, lens, maps, ....) you should see in the network tab of developer tools that the responses to your requests come directly from es and they should be gzipped

functionality wise nothing should change

ppisljar · 2024-09-24T09:38:53Z

/ci

ppisljar · 2024-09-24T11:16:49Z

/ci

ppisljar · 2024-09-24T14:21:47Z

/ci

ppisljar · 2024-09-25T06:48:54Z

/ci

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

yngrdyn

log_entry_search test LGTM

src/plugins/data/common/search/strategies/es_search/response_utils.test.ts

markov00 · 2024-10-29T08:39:41Z

src/plugins/data/common/search/strategies/es_search/response_utils.ts


 /**
 * Get the `total`/`loaded` for this response (see `IKibanaSearchResponse`). Note that `skipped` is
 * not included as it is already included in `successful`.
 * @internal
 */
 export function getTotalLoaded(response: estypes.SearchResponse<unknown>) {
+  if (!response._shards) return {};


Looking at the estypes.SearchResponse type this should never happen. Have you encountered a case where _shards property doesn't exist in response?

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

markov00 · 2024-10-29T08:45:21Z

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

+          }
+        )
+        .then((rawResponse) => {
+          const response = rawResponse.body!;


too much trust here, probably better to avoid non-null assertion since everything else here seems soo fragile

markov00 · 2024-10-29T15:47:21Z

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

+          const error = (response as unknown as Record<string, unknown>).error;
+          if (error) {


Does this error surfaced only with the stream configuration?
Because if yes we shouldn't solve it here, but probably in a more deeper substrate. Feels wrong to handle an error from something that should be a valid response

yes, this only comes up with streams

markov00 · 2024-10-29T15:50:59Z

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

+                'kbn-is-restored': response.isRestored ? '?1' : '?0',
+                'kbn-search-request-params': JSON.stringify(response.requestParams),


Can we improve the naming here, in particular, the kbn-is-restored doesn't describe much of what is restored.
Would be great if we could move these headers into some reused const.

markov00 · 2024-10-30T09:21:03Z

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

-            ...request,
-            ...searchOptions,
-          }),
+        .post<IKibanaSearchResponse>(


You can probably change this type to IKibanaSearchResponse | ErrorResponseBase ErrorResponseBase comes from elasticsearch client and seems to the the type of response that is returned by ES when you have an error. To test it you can created a custom DSL filter with a wrong syntax like

{ "match_ph": { "bytes": 21212 } }

it is very strange that we never considered using this type in our code. ES always return 200 even if there is an error in the query, it look like it just throw other status code if the call itself is wrong, not if the response is wrong.

no idea, but my goal was to keep the scope of this pr as limited as possible.

markov00

Changes looks good to me

elasticmachine · 2024-11-07T09:58:00Z

💚 Build Succeeded

Buildkite Build
Commit: a556138
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-193060-a556138439d8

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`data`	515	516	+1

Page load bundle

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

id	before	after	diff
`data`	419.9KB	421.5KB	+1.6KB

Unknown metric groups

API count

id	before	after	diff
`@kbn/search-types`	50	51	+1

ESLint disabled line counts

id	before	after	diff
`data`	56	57	+1

Total ESLint disabled count

id	before	after	diff
`data`	58	59	+1

History

💛 Build #248347 was flaky b52bb55
💔 Build #248111 failed 2c333b3
💔 Build #248109 failed 22d2ec4
💔 Build #247904 failed ac69404
💔 Build #247900 failed 8d1e3af

cc @ppisljar

Remove decoding and re-encoding of the ES-response from Kibana-server. Resolves elastic#189640

This reverts commit 7b81bcf.

lukasolson · 2025-01-07T21:09:20Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

Remove decoding and re-encoding of the ES-response from Kibana-server. Resolves elastic#189640 (cherry picked from commit 7b81bcf)

…205811) # Backport This will backport the following commits from `main` to `8.x`: - [pass elasticsearch response to browser as a stream (#193060)](#193060)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Peter Pisljar <[email protected]>

asstream

32774ba

ppisljar force-pushed the asstream branch from 12edebe to 32774ba Compare September 16, 2024 17:16

add support to async esql

b2e1904

ppisljar added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting labels Sep 17, 2024

ppisljar added 2 commits September 17, 2024 07:41

fix types

67b7d74

fix bfetch

8ee79d9

ppisljar force-pushed the asstream branch from fd00f11 to 8ee79d9 Compare September 17, 2024 08:10

kibanamachine and others added 13 commits September 17, 2024 08:25

[CI] Auto-commit changed files from 'node scripts/notice'

e57d86a

fix types

985571c

[CI] Auto-commit changed files from 'node scripts/notice'

3a645c9

Merge branch 'main' into asstream

04a63d4

fix types

66b431e

Merge branch 'asstream' of github.com:ppisljar/kibana into asstream

29f5455

fix tsvb

5dfca62

Merge branch 'main' into asstream

b5db788

fix ese

fa31ff9

fix timelion

cb1ef11

fix tsvb

3aa531e

add optional flag to enable streaming

772447f

fix flag types

342fac4

remove bfetch changes

4410b74

fix esql

2d2ba87

typo and type

f3f1262

ppisljar added 2 commits September 25, 2024 17:01

error handling

7158a1b

error handling ese

6b8a2a9

kertal assigned ppisljar Oct 28, 2024

ppisljar and others added 2 commits October 28, 2024 09:10

fix hits total

38e3c30

Add isRestored param to headers

620ba23

lukasolson reviewed Oct 28, 2024

View reviewed changes

src/plugins/data/server/search/routes/search.ts Outdated Show resolved Hide resolved

lukasolson and others added 2 commits October 28, 2024 21:20

Merge branch 'main' into asstream

9afbb68

rename headers

e98a822

yngrdyn approved these changes Oct 29, 2024

View reviewed changes

markov00 reviewed Oct 30, 2024

View reviewed changes

ppisljar added 2 commits November 4, 2024 08:05

Merge branch 'main' into asstream

8d1e3af

applying review feedback

ac69404

markov00 self-requested a review November 4, 2024 10:14

markov00 approved these changes Nov 4, 2024

View reviewed changes

ppisljar added 4 commits November 4, 2024 15:55

fix

22d2ec4

Merge branch 'main' into asstream

2c333b3

fix types

b52bb55

Merge branch 'main' into asstream

a556138

ppisljar merged commit 7b81bcf into elastic:main Nov 7, 2024
29 checks passed

kibanamachine added the v9.0.0 label Nov 7, 2024

thomasneirynck mentioned this pull request Nov 7, 2024

[data.plugin] Use CBOR instead of JSON as transfer format #199392

Open

mbondyra pushed a commit to mbondyra/kibana that referenced this pull request Nov 8, 2024

pass elasticsearch response to browser as a stream (elastic#193060)

743cbc1

Remove decoding and re-encoding of the ES-response from Kibana-server. Resolves elastic#189640

ppisljar added backport v8.18.0 labels Nov 11, 2024

ppisljar added a commit that referenced this pull request Nov 19, 2024

Revert "pass elasticsearch response to browser as a stream (#193060)"

c51ffc0

This reverts commit 7b81bcf.

ppisljar mentioned this pull request Nov 19, 2024

Revert "pass elasticsearch response to browser as a stream" #200690

Closed

lukasolson mentioned this pull request Jan 7, 2025

[8.x] pass elasticsearch response to browser as a stream (#193060) #205811

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pass elasticsearch response to browser as a stream #193060

pass elasticsearch response to browser as a stream #193060

ppisljar commented Sep 16, 2024 •

edited

Loading

ppisljar commented Sep 24, 2024

ppisljar commented Sep 24, 2024

ppisljar commented Sep 24, 2024

ppisljar commented Sep 25, 2024

yngrdyn left a comment

markov00 Oct 29, 2024

markov00 Oct 29, 2024

markov00 Oct 29, 2024

ppisljar Nov 4, 2024

markov00 Oct 29, 2024

markov00 Oct 30, 2024

markov00 Oct 30, 2024

ppisljar Nov 4, 2024

markov00 left a comment

elasticmachine commented Nov 7, 2024 •

edited

Loading

API count

ESLint disabled line counts

Total ESLint disabled count

lukasolson commented Jan 7, 2025

		const error = (response as unknown as Record<string, unknown>).error;
		if (error) {

		'kbn-is-restored': response.isRestored ? '?1' : '?0',
		'kbn-search-request-params': JSON.stringify(response.requestParams),

pass elasticsearch response to browser as a stream #193060

pass elasticsearch response to browser as a stream #193060

Conversation

ppisljar commented Sep 16, 2024 • edited Loading

ppisljar commented Sep 24, 2024

ppisljar commented Sep 24, 2024

ppisljar commented Sep 24, 2024

ppisljar commented Sep 25, 2024

yngrdyn 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

markov00 left a comment

Choose a reason for hiding this comment

elasticmachine commented Nov 7, 2024 • edited Loading

💚 Build Succeeded

Metrics [docs]

Module Count

Page load bundle

API count

ESLint disabled line counts

Total ESLint disabled count

History

lukasolson commented Jan 7, 2025

💚 All backports created successfully

Questions ?

ppisljar commented Sep 16, 2024 •

edited

Loading

elasticmachine commented Nov 7, 2024 •

edited

Loading