[APM] Errors from suggestions endpoint in E2E tests

[dgieselaar]

The GET /internal/apm/suggestions endpoint is being called in the E2E tests with fieldName being an empty string, resulting in an error from ES, because the term query that is used ends up being invalid due to it missing a field name.

This results in a lot of noise in the E2E test log. We should:

• find out why it's being called with an empty string
• figure out whether and how we need to handle this case
• use nonEmptyStringRt instead of t.string for fieldName in the route validation if we decide the request should be invalid

At least one E2E test that is exhibiting this issue is power_user/settings/custom_links.cy.ts. See https://buildkite.com/elastic/kibana-pull-request/builds/82001#01840561-a6ab-4c53-86bb-c560ccc618a6 for an example.

proc [kibana] [2022-10-23T15:36:30.903+00:00][ERROR][plugins.apm] Error: x_content_parse_exception: [x_content_parse_exception] Reason: [1:193] [bool] failed to parse field [filter]
--
  | proc [kibana]     at /opt/local-ssd/buildkite/builds/kb-cigroup-6-1d0cfbc7457de76e/elastic/kibana-pull-request/kibana-build-xpack/x-pack/plugins/observability/common/utils/unwrap_es_response.js:59:11
  | proc [kibana]     at runMicrotasks (<anonymous>)
  | proc [kibana]     at processTicksAndRejections (node:internal/process/task_queues:96:5)
  | proc [kibana]     at getSuggestionsWithTermsAggregation (/opt/local-ssd/buildkite/builds/kb-cigroup-6-1d0cfbc7457de76e/elastic/kibana-pull-request/kibana-build-xpack/x-pack/plugins/apm/server/routes/suggestions/get_suggestions_with_terms_aggregation.js:37:20)
  | proc [kibana]     at /opt/local-ssd/buildkite/builds/kb-cigroup-6-1d0cfbc7457de76e/elastic/kibana-pull-request/kibana-build-xpack/x-pack/plugins/apm/server/routes/apm_routes/register_apm_server_routes.js:96:13
  | proc [kibana]     at Router.handle (/opt/local-ssd/buildkite/builds/kb-cigroup-6-1d0cfbc7457de76e/elastic/kibana-pull-request/kibana-build-xpack/node_modules/@kbn/core-http-router-server-internal/target_node/src/router.js:163:30)
  | proc [kibana]     at handler (/opt/local-ssd/buildkite/builds/kb-cigroup-6-1d0cfbc7457de76e/elastic/kibana-pull-request/kibana-build-xpack/node_modules/@kbn/core-http-router-server-internal/target_node/src/router.js:124:50)
  | proc [kibana]     at exports.Manager.execute (/opt/local-ssd/buildkite/builds/kb-cigroup-6-1d0cfbc7457de76e/elastic/kibana-pull-request/kibana-build-xpack/node_modules/@hapi/hapi/lib/toolkit.js:60:28)
  | proc [kibana]     at Object.internals.handler (/opt/local-ssd/buildkite/builds/kb-cigroup-6-1d0cfbc7457de76e/elastic/kibana-pull-request/kibana-build-xpack/node_modules/@hapi/hapi/lib/handler.js:46:20)
  | proc [kibana]     at exports.execute (/opt/local-ssd/buildkite/builds/kb-cigroup-6-1d0cfbc7457de76e/elastic/kibana-pull-request/kibana-build-xpack/node_modules/@hapi/hapi/lib/handler.js:31:20)
  | proc [kibana]     at Request._lifecycle (/opt/local-ssd/buildkite/builds/kb-cigroup-6-1d0cfbc7457de76e/elastic/kibana-pull-request/kibana-build-xpack/node_modules/@hapi/hapi/lib/request.js:371:32)
  | proc [kibana]     at Request._execute (/opt/local-ssd/buildkite/builds/kb-cigroup-6-1d0cfbc7457de76e/elastic/kibana-pull-request/kibana-build-xpack/node_modules/@hapi/hapi/lib/request.js:281:9)
  | proc [kibana] Response: {
  | proc [kibana]   error: {
  | proc [kibana]     root_cause: [
  | proc [kibana]       {
  | proc [kibana]         type: 'x_content_parse_exception',
  | proc [kibana]         reason: '[1:193] [bool] failed to parse field [filter]'
  | proc [kibana]       }
  | proc [kibana]     ],
  | proc [kibana]     type: 'x_content_parse_exception',
  | proc [kibana]     reason: '[1:193] [bool] failed to parse field [filter]',
  | proc [kibana]     caused_by: {
  | proc [kibana]       type: 'illegal_argument_exception',
  | proc [kibana]       reason: 'field name is null or empty'
  | proc [kibana]     }
  | proc [kibana]   },
  | proc [kibana]   status: 400
  | proc [kibana] }
  | proc [kibana]

[elasticmachine]

Pinging @elastic/apm-ui (Team:APM)

[sorenlouv]

If known, can you link to the test that is causing this?

[dgieselaar]

@sqren Updated the issue description.

[sorenlouv]

• use nonEmptyStringRt instead of t.string for fieldName in the route validation if we decide the request should be invalid

I agree that would be better but it looks like it wouldn't affect anything we do today. fieldName is already required in the two components calling that endpoint afaict:

https://github.com/elastic/kibana/blob/b31c1d6462fe1b893f0791100e257dc85121a8cf/x-pack/plugins/apm/public/components/shared/environment_select/index.tsx#L76-L86

https://github.com/elastic/kibana/blob/b31c1d6462fe1b893f0791100e257dc85121a8cf/x-pack/plugins/apm/public/components/shared/suggestions_select/index.tsx#L53-L69

Which begs the question: how does this then happen?

[dgieselaar]

@sqren TypeScript is not very helpful when dealing with empty strings (vs non-empty strings). nonEmptyStringRt would at least add a more strict API validation.

[sorenlouv]

Ahh, you are right. Required doesn't mean non-empty.

[sorenlouv]

I think the error is caused by the initial state that has an empty key: '':

https://github.com/elastic/kibana/blob/3f64d246607c8bf5fa14193cf6852874af860ee2/x-pack/plugins/apm/public/components/app/settings/custom_link/create_edit_custom_link_flyout/index.tsx#L39

We can fix this by only making the request when fieldName is non-empty: https://github.com/elastic/kibana/blob/3f64d246607c8bf5fa14193cf6852874af860ee2/x-pack/plugins/apm/public/components/shared/suggestions_select/index.tsx#L53-L69

[dgieselaar]

@gbamparop Still annoying, should be an easy fix, can we pull it in for 8.10?

[gbamparop]

@gbamparop Still annoying, should be an easy fix, can we pull it in for 8.10?

What about including it in the next test plan?