Skip to content

Convert transport.request() to typed ES client methods#256456

Merged
rudolf merged 15 commits intomainfrom
ralph/typed-es-client-methods
Mar 11, 2026
Merged

Convert transport.request() to typed ES client methods#256456
rudolf merged 15 commits intomainfrom
ralph/typed-es-client-methods

Conversation

@rudolf
Copy link
Copy Markdown
Contributor

@rudolf rudolf commented Mar 6, 2026

Summary

Partially addresses https://github.com/elastic/kibana-team/issues/2886

Further iteration on #256430 — converts transport.request() calls to typed elasticsearch-js client methods where feasible, improving type safety and ensuring CPS project_routing is automatically handled by the client library.

Converted (4 call sites):

  • elasticsearch_retriever.tsesClient.search() — Search Playground RAG retriever
  • esql_request.tsesql.asyncQuery() / asyncQueryGet() / asyncQueryDelete() — Security detection engine ES|QL executor
  • esql_search_strategy.tsesql.query() — Synchronous ES|QL search (Discover/Lens)
  • esql_async_search_strategy.tsesql.asyncQuery() / asyncQueryGet() / asyncQueryStop() / asyncQueryDelete() — Async ES|QL search. Also fixed incorrect SqlGetAsyncResponse type → EsqlAsyncQueryResponse

Remaining as transport.request (2 call sites):

  • mvt_routes.ts — Maps MVT tiles. Requires restructuring getTile() helper and tile request builders to pass index/field/zoom/x/y as separate params instead of a pre-built path string
  • observability_ai_assistant elasticsearch.ts — Dynamic caller by design, method/path determined at runtime by AI

Why typed methods are better for CPS:
Typed client methods already include meta.acceptedParams in their implementation, so the CPS handler automatically injects project_routing — no explicit injection or annotation needed. The two remaining transport.request callers use explicit project_routing: '_alias:_origin' injection (stripped automatically when CPS is disabled).

Test plan

  • conversational_chain.test.ts — 12/12 pass (updated mocks from transport.request to search())
  • esql_request.test.ts — all pass (updated mocks to typed esql methods)
  • esql.test.ts — 5/5 pass (updated mocks)
  • esql_async_search_strategy.test.ts — 15/15 pass (updated mocks + assertions)
  • async_utils.test.ts — 16/16 pass
  • es_search_strategy.test.ts + ese_search_strategy.test.ts — 35/35 pass (regression check)
  • cps_request_handler.test.ts — 38/38 pass (CPS handler sanity check)
  • mvt_routes.test.ts — pass
  • Typecheck passes for all affected plugins

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com

rudolf and others added 6 commits March 6, 2026 12:39
Replace transport.request() with typed esClient.search() in the Search
Playground's ElasticsearchRetriever. Remove explicit project_routing
(CPS handler injects it automatically for typed methods). Remove unused
AggregationsAggregate and SearchResponse imports.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace transport.request() calls with typed elasticsearch-js client
methods in the Security Solution ES|QL request executor:
- POST /_query/async -> esClient.esql.asyncQuery()
- GET /_query/async/${id} -> esClient.esql.asyncQueryGet()
- DELETE /_query/async/${id} -> esClient.esql.asyncQueryDelete()

Remove explicit project_routing from submit body (CPS handler injects
it automatically for typed methods). Cast response to AsyncEsqlResponse
via unknown since the local type uses narrower column/value types than
the ES client's EsqlAsyncQueryResponse.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@rudolf rudolf requested review from a team as code owners March 6, 2026 13:32
@rudolf rudolf requested a review from denar50 March 6, 2026 13:32
@botelastic botelastic Bot added ci:project-deploy-observability Create an Observability project Team:obs-ai Observability AI team labels Mar 6, 2026
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/obs-ai-team (Team:obs-ai)

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 6, 2026

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

path: `/_query/async`,
body: params,
querystring: dropNullColumns ? 'drop_null_columns' : '',
...params,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As noted in https://github.com/elastic/kibana-team/issues/2886 ES|QL search strategy often uses transport.request as a way to quickly adopt new ES features before the specs are available.

While I converted to the typed API method now, if we ever get blocked by specs we can safely convert it back to a transport.request using project_routing: '_alias:_origin' as the default for when project_routing isn't specified by the caller.

Copy link
Copy Markdown
Contributor

@viduni94 viduni94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obs AI changes LGTM (CR only)

@rudolf rudolf added backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes labels Mar 6, 2026
Copy link
Copy Markdown
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kibana-presentation changes LGTM
code review only

Use the ES client return types directly instead of double-casting
through `unknown`. Adds a runtime guard for the optional `id` field
and removes the redundant local `AsyncEsqlResponse` type.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@TattdCodeMonkey TattdCodeMonkey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

search changes LGTM, but left some comments related to typescript

Copy link
Copy Markdown
Contributor

@lukasolson lukasolson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened a PR against this branch to clean up a few things: #256523

{
method: 'GET',
path: `/_query/async/${id}`,
id: id!,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in #256523.

Copy link
Copy Markdown
Contributor

@rylnd rylnd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Detection Engine changes LGTM.

body: requestBody,
querystring: requestQueryParams,
const asyncEsqlResponse = await esClient.esql.asyncQuery({
...requestBody,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For posterity: while requestQueryParams is only ever { drop_null_columns?: boolean }, which I see handled similarly elsewhere, requestBody contains both query and filter keys, both of which are top-level params to asyncQuery 👍

Copy link
Copy Markdown
Contributor

@markov00 markov00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM

lukasolson and others added 3 commits March 9, 2026 12:37
## Summary

Fixing some type issues in
#256456.
This is PR against the branch from that PR (not main)
…_async_search/esql_async_search_strategy.test.ts

Co-authored-by: Marco Vettorello <marco.vettorello@elastic.co>
Copy link
Copy Markdown
Contributor

@mbondyra mbondyra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for doing this! Checked the results for data plugin/Discover and when mocking projectRouting == undefined, the results are the same as with _alias:_origin.

Copy link
Copy Markdown
Contributor

@denar50 denar50 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@rudolf rudolf enabled auto-merge (squash) March 10, 2026 11:05
gsoldevila added a commit to gsoldevila/kibana that referenced this pull request Mar 10, 2026
Replace transport.request() calls with typed elasticsearch-js client
methods:
- POST /_query/async       -> esClient.esql.asyncQuery()
- GET /_query/async/${id}  -> esClient.esql.asyncQueryGet()
- POST /_query/async/${id}/stop -> esClient.esql.asyncQueryStop()
- DELETE /_query/async/${id}    -> esClient.esql.asyncQueryDelete()

Typed methods include meta.acceptedParams so the CPS OnRequestHandler
automatically injects project_routing — no explicit injection needed.
Also fixes the incorrect SqlGetAsyncResponse type to EsqlAsyncEsqlResult.

Inspired by elastic#256456.

Made-with: Cursor
rudolf and others added 4 commits March 10, 2026 16:37
- Remove stray `);` syntax error in esql_async_search_strategy.test.ts
- Set queryId before early return in esql_request.ts so completed queries are cleaned up
- Update FTR test assertions for `drop_null_columns=true` query param serialization

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Mar 11, 2026

⏳ Build in-progress, with failures

  • Buildkite Build
  • Commit: c4f51f7
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-256456-c4f51f7c18c7

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #25 / Cloud Security Posture Vulnerability Dashboard API responds with a 200 status code and matching data mock

History

@rudolf rudolf merged commit f148f02 into main Mar 11, 2026
18 checks passed
@rudolf rudolf deleted the ralph/typed-es-client-methods branch March 11, 2026 16:33
gsoldevila added a commit to gsoldevila/kibana that referenced this pull request Mar 12, 2026
Replace transport.request() calls with typed elasticsearch-js client
methods:
- POST /_query/async       -> esClient.esql.asyncQuery()
- GET /_query/async/${id}  -> esClient.esql.asyncQueryGet()
- POST /_query/async/${id}/stop -> esClient.esql.asyncQueryStop()
- DELETE /_query/async/${id}    -> esClient.esql.asyncQueryDelete()

Typed methods include meta.acceptedParams so the CPS OnRequestHandler
automatically injects project_routing — no explicit injection needed.
Also fixes the incorrect SqlGetAsyncResponse type to EsqlAsyncEsqlResult.

Inspired by elastic#256456.

Made-with: Cursor
gsoldevila added a commit to gsoldevila/kibana that referenced this pull request Mar 12, 2026
Replace transport.request() calls with typed elasticsearch-js client
methods:
- POST /_query/async       -> esClient.esql.asyncQuery()
- GET /_query/async/${id}  -> esClient.esql.asyncQueryGet()
- POST /_query/async/${id}/stop -> esClient.esql.asyncQueryStop()
- DELETE /_query/async/${id}    -> esClient.esql.asyncQueryDelete()

Typed methods include meta.acceptedParams so the CPS OnRequestHandler
automatically injects project_routing — no explicit injection needed.
Also fixes the incorrect SqlGetAsyncResponse type to EsqlAsyncEsqlResult.

Inspired by elastic#256456.

Made-with: Cursor
gsoldevila added a commit to gsoldevila/kibana that referenced this pull request Mar 13, 2026
Replace transport.request() calls with typed elasticsearch-js client
methods:
- POST /_query/async       -> esClient.esql.asyncQuery()
- GET /_query/async/${id}  -> esClient.esql.asyncQueryGet()
- POST /_query/async/${id}/stop -> esClient.esql.asyncQueryStop()
- DELETE /_query/async/${id}    -> esClient.esql.asyncQueryDelete()

Typed methods include meta.acceptedParams so the CPS OnRequestHandler
automatically injects project_routing — no explicit injection needed.
Also fixes the incorrect SqlGetAsyncResponse type to EsqlAsyncEsqlResult.

Inspired by elastic#256456.

Made-with: Cursor
mbondyra added a commit that referenced this pull request Mar 16, 2026
## Summary

1. In this one: #256456 we
accidentally patched Maps mvt routes with only supporting local project
routing, but we shouldn't have, as CPS is supported on Maps (the setting
should be taken from the project picker)

2. In this PR I also removed a setting from the cps picker, since it
cannot be linked for tech preview
<img width="932" height="263" alt="Screenshot 2026-03-13 at 16 41 35"
src="https://github.com/user-attachments/assets/ec12dcf5-3a9d-4597-bda6-1573b4f9247d"
/>
sorenlouv pushed a commit that referenced this pull request Mar 17, 2026
## Summary

Partially addresses elastic/kibana-team#2886

Further iteration on #256430 —
converts `transport.request()` calls to typed elasticsearch-js client
methods where feasible, improving type safety and ensuring CPS
`project_routing` is automatically handled by the client library.

**Converted (4 call sites):**
- `elasticsearch_retriever.ts` → `esClient.search()` — Search Playground
RAG retriever
- `esql_request.ts` → `esql.asyncQuery()` / `asyncQueryGet()` /
`asyncQueryDelete()` — Security detection engine ES|QL executor
- `esql_search_strategy.ts` → `esql.query()` — Synchronous ES|QL search
(Discover/Lens)
- `esql_async_search_strategy.ts` → `esql.asyncQuery()` /
`asyncQueryGet()` / `asyncQueryStop()` / `asyncQueryDelete()` — Async
ES|QL search. Also fixed incorrect `SqlGetAsyncResponse` type →
`EsqlAsyncQueryResponse`

**Remaining as transport.request (2 call sites):**
- `mvt_routes.ts` — Maps MVT tiles. Requires restructuring `getTile()`
helper and tile request builders to pass `index`/`field`/`zoom`/`x`/`y`
as separate params instead of a pre-built path string
- `observability_ai_assistant elasticsearch.ts` — Dynamic caller by
design, method/path determined at runtime by AI

**Why typed methods are better for CPS:**
Typed client methods already include `meta.acceptedParams` in their
implementation, so the CPS handler automatically injects
`project_routing` — no explicit injection or annotation needed. The two
remaining `transport.request` callers use explicit `project_routing:
'_alias:_origin'` injection (stripped automatically when CPS is
disabled).

## Test plan

- [x] `conversational_chain.test.ts` — 12/12 pass (updated mocks from
`transport.request` to `search()`)
- [x] `esql_request.test.ts` — all pass (updated mocks to typed esql
methods)
- [x] `esql.test.ts` — 5/5 pass (updated mocks)
- [x] `esql_async_search_strategy.test.ts` — 15/15 pass (updated mocks +
assertions)
- [x] `async_utils.test.ts` — 16/16 pass
- [x] `es_search_strategy.test.ts` + `ese_search_strategy.test.ts` —
35/35 pass (regression check)
- [x] `cps_request_handler.test.ts` — 38/38 pass (CPS handler sanity
check)
- [x] `mvt_routes.test.ts` — pass
- [x] Typecheck passes for all affected plugins

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Lukas Olson <lukas@elastic.co>
Co-authored-by: Marco Vettorello <marco.vettorello@elastic.co>
sorenlouv pushed a commit that referenced this pull request Mar 17, 2026
## Summary

1. In this one: #256456 we
accidentally patched Maps mvt routes with only supporting local project
routing, but we shouldn't have, as CPS is supported on Maps (the setting
should be taken from the project picker)

2. In this PR I also removed a setting from the cps picker, since it
cannot be linked for tech preview
<img width="932" height="263" alt="Screenshot 2026-03-13 at 16 41 35"
src="https://github.com/user-attachments/assets/ec12dcf5-3a9d-4597-bda6-1573b4f9247d"
/>
jeramysoucy pushed a commit to jeramysoucy/kibana that referenced this pull request Mar 26, 2026
## Summary

Partially addresses elastic/kibana-team#2886

Further iteration on elastic#256430 —
converts `transport.request()` calls to typed elasticsearch-js client
methods where feasible, improving type safety and ensuring CPS
`project_routing` is automatically handled by the client library.

**Converted (4 call sites):**
- `elasticsearch_retriever.ts` → `esClient.search()` — Search Playground
RAG retriever
- `esql_request.ts` → `esql.asyncQuery()` / `asyncQueryGet()` /
`asyncQueryDelete()` — Security detection engine ES|QL executor
- `esql_search_strategy.ts` → `esql.query()` — Synchronous ES|QL search
(Discover/Lens)
- `esql_async_search_strategy.ts` → `esql.asyncQuery()` /
`asyncQueryGet()` / `asyncQueryStop()` / `asyncQueryDelete()` — Async
ES|QL search. Also fixed incorrect `SqlGetAsyncResponse` type →
`EsqlAsyncQueryResponse`

**Remaining as transport.request (2 call sites):**
- `mvt_routes.ts` — Maps MVT tiles. Requires restructuring `getTile()`
helper and tile request builders to pass `index`/`field`/`zoom`/`x`/`y`
as separate params instead of a pre-built path string
- `observability_ai_assistant elasticsearch.ts` — Dynamic caller by
design, method/path determined at runtime by AI

**Why typed methods are better for CPS:**
Typed client methods already include `meta.acceptedParams` in their
implementation, so the CPS handler automatically injects
`project_routing` — no explicit injection or annotation needed. The two
remaining `transport.request` callers use explicit `project_routing:
'_alias:_origin'` injection (stripped automatically when CPS is
disabled).

## Test plan

- [x] `conversational_chain.test.ts` — 12/12 pass (updated mocks from
`transport.request` to `search()`)
- [x] `esql_request.test.ts` — all pass (updated mocks to typed esql
methods)
- [x] `esql.test.ts` — 5/5 pass (updated mocks)
- [x] `esql_async_search_strategy.test.ts` — 15/15 pass (updated mocks +
assertions)
- [x] `async_utils.test.ts` — 16/16 pass
- [x] `es_search_strategy.test.ts` + `ese_search_strategy.test.ts` —
35/35 pass (regression check)
- [x] `cps_request_handler.test.ts` — 38/38 pass (CPS handler sanity
check)
- [x] `mvt_routes.test.ts` — pass
- [x] Typecheck passes for all affected plugins

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Lukas Olson <lukas@elastic.co>
Co-authored-by: Marco Vettorello <marco.vettorello@elastic.co>
jeramysoucy pushed a commit to jeramysoucy/kibana that referenced this pull request Mar 26, 2026
…7706)

## Summary

1. In this one: elastic#256456 we
accidentally patched Maps mvt routes with only supporting local project
routing, but we shouldn't have, as CPS is supported on Maps (the setting
should be taken from the project picker)

2. In this PR I also removed a setting from the cps picker, since it
cannot be linked for tech preview
<img width="932" height="263" alt="Screenshot 2026-03-13 at 16 41 35"
src="https://github.com/user-attachments/assets/ec12dcf5-3a9d-4597-bda6-1573b4f9247d"
/>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting ci:project-deploy-observability Create an Observability project release_note:skip Skip the PR/issue when compiling release notes Team:obs-ai Observability AI team v9.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.