[inspector] show request method, path, and querystring (#169970)

Closes #45931 PR updates data plugin `search` and `bsearch` endpoints to return method, path, and querystring request params from elasticsearch-js client requests. This provides inspector with the exact details used to fetch data from elasticsearch, ensuring inspector displays request exactly as used by elasticsearch-js client. **ESQL** This PR makes it possible to open ESQL searches in console. <img width="500" alt="Screen Shot 2023-09-16 at 4 19 58 PM" src="https://github.com/elastic/kibana/assets/373691/56019fb5-ca88-46cf-a42f-86f5f51edfcc"> ### background If you are thinking to yourself, "haven't I reviewed this before?", you are right. This functionality has been through several iterations. 1) Original PR #166565 was reverted for exposing `headers`. 2) [Fix to only expose method, path, and querystring keys from request parameters](#167544) was rejected because it applied changes to `kibana_utils/server/report_server_error.ts`, which is used extensively throughout Kibana. 3) This iteration moves logic into the data plugin to be as narrow as possible. --------- Co-authored-by: kibanamachine <[email protected]>
elastic · Nov 3, 2023 · f284454 · f284454
1 parent 45d0e32
commit f284454
Show file tree

Hide file tree

Showing 29 changed files with 627 additions and 43 deletions.
diff --git a/src/plugins/data/common/search/expressions/esdsl.ts b/src/plugins/data/common/search/expressions/esdsl.ts
@@ -126,7 +126,7 @@ export const getEsdslFn = ({
       });
 
       try {
-        const { rawResponse } = await lastValueFrom(
+        const { rawResponse, requestParams } = await lastValueFrom(
           search(
             {
               params: {
@@ -180,7 +180,7 @@ export const getEsdslFn = ({
           };
         }
 
-        request.stats(stats).ok({ json: rawResponse });
+        request.stats(stats).ok({ json: rawResponse, requestParams });
         request.json(dsl);
 
         return {

diff --git a/src/plugins/data/common/search/expressions/esql.ts b/src/plugins/data/common/search/expressions/esql.ts
@@ -220,7 +220,7 @@ export const getEsqlFn = ({ getStartDependencies }: EsqlFnArguments) => {
               return throwError(() => error);
             }),
             tap({
-              next({ rawResponse }) {
+              next({ rawResponse, requestParams }) {
                 logInspectorRequest()
                   .stats({
                     hits: {
@@ -234,12 +234,14 @@ export const getEsqlFn = ({ getStartDependencies }: EsqlFnArguments) => {
                     },
                   })
                   .json(params)
-                  .ok({ json: rawResponse });
+                  .ok({ json: rawResponse, requestParams });
               },
               error(error) {
-                logInspectorRequest().error({
-                  json: 'attributes' in error ? error.attributes : { message: error.message },
-                });
+                logInspectorRequest()
+                  .json(params)
+                  .error({
+                    json: 'attributes' in error ? error.attributes : { message: error.message },
+                  });
               },
             })
           );

diff --git a/src/plugins/data/common/search/expressions/essql.ts b/src/plugins/data/common/search/expressions/essql.ts
@@ -217,7 +217,7 @@ export const getEssqlFn = ({ getStartDependencies }: EssqlFnArguments) => {
               return throwError(() => error);
             }),
             tap({
-              next({ rawResponse, took }) {
+              next({ rawResponse, requestParams, took }) {
                 logInspectorRequest()
                   .stats({
                     hits: {
@@ -245,7 +245,7 @@ export const getEssqlFn = ({ getStartDependencies }: EssqlFnArguments) => {
                     },
                   })
                   .json(params)
-                  .ok({ json: rawResponse });
+                  .ok({ json: rawResponse, requestParams });
               },
               error(error) {
                 logInspectorRequest().error({

diff --git a/src/plugins/data/common/search/types.ts b/src/plugins/data/common/search/types.ts
@@ -6,6 +6,8 @@
  * Side Public License, v 1.
  */
 
+import { estypes } from '@elastic/elasticsearch';
+import type { ConnectionRequestParams } from '@elastic/transport';
 import type { TransportRequestOptions } from '@elastic/elasticsearch';
 import type { KibanaExecutionContext } from '@kbn/core/public';
 import type { DataView } from '@kbn/data-views-plugin/common';
@@ -39,6 +41,11 @@ export interface ISearchClient {
   extend: ISearchExtendGeneric;
 }
 
+export type SanitizedConnectionRequestParams = Pick<
+  ConnectionRequestParams,
+  'method' | 'path' | 'querystring'
+>;
+
 export interface IKibanaSearchResponse<RawResponse = any> {
   /**
    * Some responses may contain a unique id to identify the request this response came from.
@@ -86,6 +93,17 @@ export interface IKibanaSearchResponse<RawResponse = any> {
    * The raw response returned by the internal search method (usually the raw ES response)
    */
   rawResponse: RawResponse;
+
+  /**
+   * HTTP request parameters from elasticsearch transport client t
+   */
+  requestParams?: SanitizedConnectionRequestParams;
+}
+
+export interface IEsErrorAttributes {
+  error?: estypes.ErrorCause;
+  rawResponse?: estypes.SearchResponseBody;
+  requestParams?: SanitizedConnectionRequestParams;
 }
 
 export interface IKibanaSearchRequest<Params = any> {

diff --git a/src/plugins/data/public/search/errors/types.ts b/src/plugins/data/public/search/errors/types.ts
@@ -6,13 +6,8 @@
  * Side Public License, v 1.
  */
 
-import { estypes } from '@elastic/elasticsearch';
 import { KibanaServerError } from '@kbn/kibana-utils-plugin/common';
-
-interface IEsErrorAttributes {
-  rawResponse?: estypes.SearchResponseBody;
-  error?: estypes.ErrorCause;
-}
+import { IEsErrorAttributes } from '../../../common';
 
 export type IEsError = KibanaServerError<IEsErrorAttributes>;
 

diff --git a/src/plugins/data/public/search/search_interceptor/search_interceptor.ts b/src/plugins/data/public/search/search_interceptor/search_interceptor.ts
@@ -56,6 +56,7 @@ import {
   ISearchOptionsSerializable,
   pollSearch,
   UI_SETTINGS,
+  type SanitizedConnectionRequestParams,
 } from '../../../common';
 import { SearchUsageCollector } from '../collectors';
 import {
@@ -304,18 +305,38 @@ export class SearchInterceptor {
 
     const cancel = () => id && !isSavedToBackground && sendCancelRequest();
 
+    // Async search requires a series of requests
+    // 1) POST /<index pattern>/_async_search/
+    // 2..n) GET /_async_search/<async search identifier>
+    //
+    // First request contains useful request params for tools like Inspector.
+    // Preserve and project first request params into responses.
+    let firstRequestParams: SanitizedConnectionRequestParams;
+
     return pollSearch(search, cancel, {
       pollInterval: this.deps.searchConfig.asyncSearch.pollInterval,
       ...options,
       abortSignal: searchAbortController.getSignal(),
     }).pipe(
       tap((response) => {
+        if (!firstRequestParams && response.requestParams) {
+          firstRequestParams = response.requestParams;
+        }
+
         id = response.id;
 
         if (!isRunningResponse(response)) {
           searchTracker?.complete();
         }
       }),
+      map((response) => {
+        return firstRequestParams
+          ? {
+              ...response,
+              requestParams: firstRequestParams,
+            }
+          : response;
+      }),
       catchError((e: Error) => {
         searchTracker?.error();
         cancel();

diff --git a/src/plugins/data/server/search/report_search_error.ts b/src/plugins/data/server/search/report_search_error.ts
@@ -6,9 +6,12 @@
  * Side Public License, v 1.
  */
 
+import type { ConnectionRequestParams } from '@elastic/transport';
 import { errors } from '@elastic/elasticsearch';
 import { KibanaResponseFactory } from '@kbn/core/server';
 import { KbnError } from '@kbn/kibana-utils-plugin/common';
+import type { SanitizedConnectionRequestParams } from '../../common';
+import { sanitizeRequestParams } from './sanitize_request_params';
 
 // Why not use just use kibana-utils-plugin KbnServerError and reportServerError?
 //
@@ -19,9 +22,17 @@ import { KbnError } from '@kbn/kibana-utils-plugin/common';
 // non-search usages of KbnServerError and reportServerError with extra information.
 export class KbnSearchError extends KbnError {
   public errBody?: Record<string, any>;
-  constructor(message: string, public readonly statusCode: number, errBody?: Record<string, any>) {
+  public requestParams?: SanitizedConnectionRequestParams;
+
+  constructor(
+    message: string,
+    public readonly statusCode: number,
+    errBody?: Record<string, any>,
+    requestParams?: ConnectionRequestParams
+  ) {
     super(message);
     this.errBody = errBody;
+    this.requestParams = requestParams ? sanitizeRequestParams(requestParams) : undefined;
   }
 }
 
@@ -35,7 +46,8 @@ export function getKbnSearchError(e: Error) {
   return new KbnSearchError(
     e.message ?? 'Unknown error',
     e instanceof errors.ResponseError ? e.statusCode! : 500,
-    e instanceof errors.ResponseError ? e.body : undefined
+    e instanceof errors.ResponseError ? e.body : undefined,
+    e instanceof errors.ResponseError ? e.meta?.meta?.request?.params : undefined
   );
 }
 
@@ -53,6 +65,7 @@ export function reportSearchError(res: KibanaResponseFactory, err: KbnSearchErro
         ? {
             error: err.errBody.error,
             rawResponse: err.errBody.response,
+            ...(err.requestParams ? { requestParams: err.requestParams } : {}),
           }
         : undefined,
     },

diff --git a/src/plugins/data/server/search/routes/bsearch.ts b/src/plugins/data/server/search/routes/bsearch.ts
@@ -50,6 +50,7 @@ export function registerBsearchRoute(
                     ? {
                         error: err.errBody.error,
                         rawResponse: err.errBody.response,
+                        ...(err.requestParams ? { requestParams: err.requestParams } : {}),
                       }
                     : undefined,
                 };

diff --git a/src/plugins/data/server/search/sanitize_request_params.test.ts b/src/plugins/data/server/search/sanitize_request_params.test.ts
@@ -0,0 +1,41 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import { sanitizeRequestParams } from './sanitize_request_params';
+
+describe('sanitizeRequestParams', () => {
+  test('should remove headers and body', () => {
+    expect(
+      sanitizeRequestParams({
+        method: 'POST',
+        path: '/endpoint',
+        querystring: 'param1=value',
+        headers: {
+          Connection: 'Keep-Alive',
+        },
+        body: 'response',
+      })
+    ).toEqual({
+      method: 'POST',
+      path: '/endpoint',
+      querystring: 'param1=value',
+    });
+  });
+
+  test('should not include querystring key when its not provided', () => {
+    expect(
+      sanitizeRequestParams({
+        method: 'POST',
+        path: '/endpoint',
+      })
+    ).toEqual({
+      method: 'POST',
+      path: '/endpoint',
+    });
+  });
+});
diff --git a/src/plugins/data/server/search/sanitize_request_params.ts b/src/plugins/data/server/search/sanitize_request_params.ts
@@ -0,0 +1,20 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import type { ConnectionRequestParams } from '@elastic/transport';
+import type { SanitizedConnectionRequestParams } from '../../common';
+
+export function sanitizeRequestParams(
+  requestParams: ConnectionRequestParams
+): SanitizedConnectionRequestParams {
+  return {
+    method: requestParams.method,
+    path: requestParams.path,
+    ...(requestParams.querystring ? { querystring: requestParams.querystring } : {}),
+  };
+}
diff --git a/src/plugins/data/server/search/strategies/eql_search/eql_search_strategy.ts b/src/plugins/data/server/search/strategies/eql_search/eql_search_strategy.ts
@@ -77,7 +77,11 @@ export const eqlSearchStrategyProvider = (
               meta: true,
             });
 
-        return toEqlKibanaSearchResponse(response as TransportResult<EqlSearchResponse>);
+        return toEqlKibanaSearchResponse(
+          response as TransportResult<EqlSearchResponse>,
+          // do not return requestParams on polling calls
+          id ? undefined : (response as TransportResult<EqlSearchResponse>).meta?.request?.params
+        );
       };
 
       const cancel = async () => {

diff --git a/src/plugins/data/server/search/strategies/eql_search/response_utils.ts b/src/plugins/data/server/search/strategies/eql_search/response_utils.ts
@@ -6,21 +6,25 @@
  * Side Public License, v 1.
  */
 
+import type { ConnectionRequestParams } from '@elastic/transport';
 import type { TransportResult } from '@elastic/elasticsearch';
 import { EqlSearchResponse } from './types';
 import { EqlSearchStrategyResponse } from '../../../../common';
+import { sanitizeRequestParams } from '../../sanitize_request_params';
 
 /**
  * Get the Kibana representation of an EQL search response (see `IKibanaSearchResponse`).
  * (EQL does not provide _shard info, so total/loaded cannot be calculated.)
  */
 export function toEqlKibanaSearchResponse(
-  response: TransportResult<EqlSearchResponse>
+  response: TransportResult<EqlSearchResponse>,
+  requestParams?: ConnectionRequestParams
 ): EqlSearchStrategyResponse {
   return {
     id: response.body.id,
     rawResponse: response.body,
     isPartial: response.body.is_partial,
     isRunning: response.body.is_running,
+    ...(requestParams ? { requestParams: sanitizeRequestParams(requestParams) } : {}),
   };
 }
diff --git a/src/plugins/data/server/search/strategies/es_search/es_search_strategy.test.ts b/src/plugins/data/server/search/strategies/es_search/es_search_strategy.test.ts
@@ -113,7 +113,7 @@ describe('ES search strategy', () => {
       )
     );
     const [, searchOptions] = esClient.search.mock.calls[0];
-    expect(searchOptions).toEqual({ signal: undefined, maxRetries: 5 });
+    expect(searchOptions).toEqual({ signal: undefined, maxRetries: 5, meta: true });
   });
 
   it('can be aborted', async () => {
@@ -131,7 +131,10 @@ describe('ES search strategy', () => {
       ...params,
       track_total_hits: true,
     });
-    expect(esClient.search.mock.calls[0][1]).toEqual({ signal: expect.any(AbortSignal) });
+    expect(esClient.search.mock.calls[0][1]).toEqual({
+      signal: expect.any(AbortSignal),
+      meta: true,
+    });
   });
 
   it('throws normalized error if ResponseError is thrown', async () => {

diff --git a/src/plugins/data/server/search/strategies/es_search/es_search_strategy.ts b/src/plugins/data/server/search/strategies/es_search/es_search_strategy.ts
@@ -50,12 +50,13 @@ export const esSearchStrategyProvider = (
           ...(terminateAfter ? { terminate_after: terminateAfter } : {}),
           ...requestParams,
         };
-        const body = await esClient.asCurrentUser.search(params, {
+        const { body, meta } = await esClient.asCurrentUser.search(params, {
           signal: abortSignal,
           ...transport,
+          meta: true,
         });
         const response = shimHitsTotal(body, options);
-        return toKibanaSearchResponse(response);
+        return toKibanaSearchResponse(response, meta?.request?.params);
       } catch (e) {
         throw getKbnSearchError(e);
       }

diff --git a/src/plugins/data/server/search/strategies/es_search/response_utils.ts b/src/plugins/data/server/search/strategies/es_search/response_utils.ts
@@ -6,8 +6,10 @@
  * Side Public License, v 1.
  */
 
+import type { ConnectionRequestParams } from '@elastic/transport';
 import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
 import { ISearchOptions } from '../../../../common';
+import { sanitizeRequestParams } from '../../sanitize_request_params';
 
 /**
  * Get the `total`/`loaded` for this response (see `IKibanaSearchResponse`). Note that `skipped` is
@@ -24,11 +26,15 @@ export function getTotalLoaded(response: estypes.SearchResponse<unknown>) {
  * Get the Kibana representation of this response (see `IKibanaSearchResponse`).
  * @internal
  */
-export function toKibanaSearchResponse(rawResponse: estypes.SearchResponse<unknown>) {
+export function toKibanaSearchResponse(
+  rawResponse: estypes.SearchResponse<unknown>,
+  requestParams?: ConnectionRequestParams
+) {
   return {
     rawResponse,
     isPartial: false,
     isRunning: false,
+    ...(requestParams ? { requestParams: sanitizeRequestParams(requestParams) } : {}),
     ...getTotalLoaded(rawResponse),
   };
 }