Skip to content

[Console] Fix autocomplete_entities API crash when response size is too big#140569

Merged
mibragimov merged 18 commits intoelastic:mainfrom
mibragimov:console/autocomplete_entities
Nov 11, 2022
Merged

[Console] Fix autocomplete_entities API crash when response size is too big#140569
mibragimov merged 18 commits intoelastic:mainfrom
mibragimov:console/autocomplete_entities

Conversation

@mibragimov
Copy link
Contributor

@mibragimov mibragimov commented Sep 13, 2022 •
edited by kibanamachine
Loading

Fixes #144310

Summary

This PR addresses the issue of the Kibana instance restarting when the response size is too big for the autocomplete_entities API. This happens when a cluster has a large number of mappings and we try to retrieve them all on the server side with esClient.asInternalUser.indices.getMapping(). esClient does not handle large responses well and throws an error that causes the Kibana instance to restart. As node's max string length is 2^28-1 (~512MB) if the response size is over 512MB, it will throw an error with the message "Invalid string length".

The fix is to use the raw http request to retrieve the mappings instead of esClient and check the response size before sending it to the client. If the response size is too big, we will return an empty object and log the error in the server logs.

Proposed changes

  • Remove ES JS client requests and use native Node.js HTTP client instead
  • Limit the response size to 10MB for the autocomplete_entities API

How to test this PR locally

To test this out, you will need to connect Kibana to a remote cluster with a large number of mappings. We created a patch file that you can apply to your local Kibana instance to test this PR. Since the patch file contains credentials, we can't share it publicly. Please reach out to me if you would like to test this PR. I will share the patch file and the instructions to apply it.

@mibragimov mibragimov added Feature:Console Dev Tools Console Feature Feature:Dev Tools release_note:fix Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// v8.5.0 labels Sep 13, 2022
@mibragimov mibragimov self-assigned this Sep 13, 2022
@mibragimov mibragimov force-pushed the console/autocomplete_entities branch from e5ae3dd to db5e2b5 Compare September 13, 2022 06:13
alexwizp
alexwizp reviewed Sep 19, 2022
View reviewed changes
@mibragimov mibragimov force-pushed the console/autocomplete_entities branch from db5e2b5 to 01ae4c0 Compare October 5, 2022 06:07
@mibragimov mibragimov force-pushed the console/autocomplete_entities branch from 08a9cb0 to 434ac82 Compare October 5, 2022 06:45
sabarasaba
sabarasaba reviewed Oct 5, 2022
View reviewed changes
Copy link
Member

@sabarasaba sabarasaba left a comment

Choose a reason for hiding this comment

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

Going through the changes and at first glance looks fine, I was wondering though how can I test this locally to make sure it works? 🤔

@mibragimov mibragimov added the ci:cloud-deploy Create or update a Cloud deployment label Oct 5, 2022
alexwizp
alexwizp reviewed Oct 5, 2022
View reviewed changes
alexwizp
alexwizp reviewed Oct 5, 2022
View reviewed changes
@mibragimov mibragimov force-pushed the console/autocomplete_entities branch from c4f19ef to db281e3 Compare October 7, 2022 07:31
alexwizp
alexwizp reviewed Oct 7, 2022
View reviewed changes
alexwizp
alexwizp reviewed Oct 7, 2022
View reviewed changes
@mibragimov mibragimov force-pushed the console/autocomplete_entities branch from 491bf97 to 3b75822 Compare October 11, 2022 07:30
@mibragimov
Copy link
Contributor Author

mibragimov commented Oct 21, 2022

Closing this PR as the issue was fixed by #143428

@mibragimov mibragimov closed this Oct 21, 2022
@mibragimov mibragimov deleted the console/autocomplete_entities branch October 21, 2022 09:30
@mibragimov mibragimov restored the console/autocomplete_entities branch November 2, 2022 09:24
@mibragimov mibragimov force-pushed the console/autocomplete_entities branch from 2de7634 to f94b90b Compare November 10, 2022 09:06
@mibragimov
Copy link
Contributor Author

mibragimov commented Nov 10, 2022 •
edited
Loading

Thanks for taking this one on @mibragimov! Verified with your testing instructions and everything worked as expected.

I left a couple minor comments in the code. Also, what is our testing strategy in general for the autocomplete request? Are there existing API integration tests for it?

Thanks for the review @alisonelizabeth! I've addressed your feedback with f94b90b and a31e406. We have existing API integration tests for autocomplete_entities, but they are not very extensive. I've added a few more tests to cover the response format.

@mibragimov
Copy link
Contributor Author

mibragimov commented Nov 10, 2022

Verified the api integration tests through the flaky test runner ~200 times https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/1539

alisonelizabeth
alisonelizabeth approved these changes Nov 10, 2022
View reviewed changes
Copy link
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

Thanks for addressing my feedback! I left a couple nits in the API integration tests. Let me know what you think. I'm going to go ahead and approve to unblock you.

@kibana-ci
Copy link

kibana-ci commented Nov 11, 2022

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
console 414.9KB 414.9KB +15.0B

Page load bundle

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

id before after diff
console 26.7KB 26.9KB +257.0B
Unknown metric groups

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 59 65 +6
osquery 108 113 +5
securitySolution 441 447 +6
total +19

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 67 73 +6
osquery 109 115 +6
securitySolution 518 524 +6
total +20

History

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

cc @mibragimov

@mibragimov mibragimov merged commit 0e01401 into elastic:main Nov 11, 2022
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 11, 2022
@mibragimov
[Console] Fix autocomplete_entities API crash when response size is t…
5659f40 
…oo big (elastic#140569)

Fixes elastic#144310

### Summary

This PR addresses the issue of the Kibana instance restarting when the
response size is too big for the `autocomplete_entities` API. This
happens when a cluster has a large number of mappings and we try to
retrieve them all on the server side with
`esClient.asInternalUser.indices.getMapping()`. esClient does not handle
large responses well and throws an error that causes the Kibana instance
to restart. As node's max [string
length](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/length#description)
is 2^28-1 (~512MB) if the response size is over 512MB, it will throw an
error with the message "Invalid string length".

The fix is to use the raw http request to retrieve the mappings instead
of esClient and check the response size before sending it to the client.
If the response size is too big, we will return an empty object and log
the error in the server logs.

#### Proposed changes

- Remove ES JS client requests and use native Node.js HTTP client
instead
- Limit the response size to 10MB for the `autocomplete_entities` API

#### How to test this PR locally
To test this out, you will need to connect Kibana to a remote cluster
with a large number of mappings. We created a patch file that you can
apply to your local Kibana instance to test this PR. Since the patch
file contains credentials, we can't share it publicly. Please reach out
to me if you would like to test this PR. I will share the patch file and
the instructions to apply it.

Co-authored-by: Muhammad Ibragimov <muhammad.ibragimov@elastic.co>
(cherry picked from commit 0e01401)
@kibanamachine kibanamachine mentioned this pull request Nov 11, 2022
Merged
@kibanamachine
Copy link
Contributor

kibanamachine commented Nov 11, 2022

💚 All backports created successfully

Status Branch Result
✅ 8.5

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

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Nov 11, 2022
@kibanamachine @mibragimov
[8.5] [Console] Fix autocomplete_entities API crash when response siz…
5ef1155 
…e is too big (#140569) (#145051)

# Backport

This will backport the following commits from `main` to `8.5`:
- [[Console] Fix autocomplete_entities API crash when response size is
too big (#140569)](#140569)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Muhammad
Ibragimov","email":"53621505+mibragimov@users.noreply.github.com"},"sourceCommit":{"committedDate":"2022-11-11T10:43:52Z","message":"[Console]
Fix autocomplete_entities API crash when response size is too big
(#140569)\n\nFixes
https://github.com/elastic/kibana/issues/144310\r\n\r\n###
Summary\r\n\r\nThis PR addresses the issue of the Kibana instance
restarting when the\r\nresponse size is too big for the
`autocomplete_entities` API. This\r\nhappens when a cluster has a large
number of mappings and we try to\r\nretrieve them all on the server side
with\r\n`esClient.asInternalUser.indices.getMapping()`. esClient does
not handle\r\nlarge responses well and throws an error that causes the
Kibana instance\r\nto restart. As node's max
[string\r\nlength](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/length#description)\r\nis
2^28-1 (~512MB) if the response size is over 512MB, it will throw
an\r\nerror with the message \"Invalid string length\".\r\n\r\nThe fix
is to use the raw http request to retrieve the mappings instead\r\nof
esClient and check the response size before sending it to the
client.\r\nIf the response size is too big, we will return an empty
object and log\r\nthe error in the server logs.\r\n\r\n#### Proposed
changes\r\n\r\n- Remove ES JS client requests and use native Node.js
HTTP client\r\ninstead\r\n- Limit the response size to 10MB for the
`autocomplete_entities` API\r\n\r\n#### How to test this PR
locally\r\nTo test this out, you will need to connect Kibana to a remote
cluster\r\nwith a large number of mappings. We created a patch file that
you can\r\napply to your local Kibana instance to test this PR. Since
the patch\r\nfile contains credentials, we can't share it publicly.
Please reach out\r\nto me if you would like to test this PR. I will
share the patch file and\r\nthe instructions to apply
it.\r\n\r\nCo-authored-by: Muhammad Ibragimov
<muhammad.ibragimov@elastic.co>","sha":"0e0140181faf65ad3041e22fc053e49c76f2ce47","branchLabelMapping":{"^v8.6.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Console","Feature:Dev
Tools","release_note:fix","Team:Deployment
Management","backport:prev-minor","v8.6.0"],"number":140569,"url":"https://github.com/elastic/kibana/pull/140569","mergeCommit":{"message":"[Console]
Fix autocomplete_entities API crash when response size is too big
(#140569)\n\nFixes
https://github.com/elastic/kibana/issues/144310\r\n\r\n###
Summary\r\n\r\nThis PR addresses the issue of the Kibana instance
restarting when the\r\nresponse size is too big for the
`autocomplete_entities` API. This\r\nhappens when a cluster has a large
number of mappings and we try to\r\nretrieve them all on the server side
with\r\n`esClient.asInternalUser.indices.getMapping()`. esClient does
not handle\r\nlarge responses well and throws an error that causes the
Kibana instance\r\nto restart. As node's max
[string\r\nlength](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/length#description)\r\nis
2^28-1 (~512MB) if the response size is over 512MB, it will throw
an\r\nerror with the message \"Invalid string length\".\r\n\r\nThe fix
is to use the raw http request to retrieve the mappings instead\r\nof
esClient and check the response size before sending it to the
client.\r\nIf the response size is too big, we will return an empty
object and log\r\nthe error in the server logs.\r\n\r\n#### Proposed
changes\r\n\r\n- Remove ES JS client requests and use native Node.js
HTTP client\r\ninstead\r\n- Limit the response size to 10MB for the
`autocomplete_entities` API\r\n\r\n#### How to test this PR
locally\r\nTo test this out, you will need to connect Kibana to a remote
cluster\r\nwith a large number of mappings. We created a patch file that
you can\r\napply to your local Kibana instance to test this PR. Since
the patch\r\nfile contains credentials, we can't share it publicly.
Please reach out\r\nto me if you would like to test this PR. I will
share the patch file and\r\nthe instructions to apply
it.\r\n\r\nCo-authored-by: Muhammad Ibragimov
<muhammad.ibragimov@elastic.co>","sha":"0e0140181faf65ad3041e22fc053e49c76f2ce47"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.6.0","labelRegex":"^v8.6.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/140569","number":140569,"mergeCommit":{"message":"[Console]
Fix autocomplete_entities API crash when response size is too big
(#140569)\n\nFixes
https://github.com/elastic/kibana/issues/144310\r\n\r\n###
Summary\r\n\r\nThis PR addresses the issue of the Kibana instance
restarting when the\r\nresponse size is too big for the
`autocomplete_entities` API. This\r\nhappens when a cluster has a large
number of mappings and we try to\r\nretrieve them all on the server side
with\r\n`esClient.asInternalUser.indices.getMapping()`. esClient does
not handle\r\nlarge responses well and throws an error that causes the
Kibana instance\r\nto restart. As node's max
[string\r\nlength](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/length#description)\r\nis
2^28-1 (~512MB) if the response size is over 512MB, it will throw
an\r\nerror with the message \"Invalid string length\".\r\n\r\nThe fix
is to use the raw http request to retrieve the mappings instead\r\nof
esClient and check the response size before sending it to the
client.\r\nIf the response size is too big, we will return an empty
object and log\r\nthe error in the server logs.\r\n\r\n#### Proposed
changes\r\n\r\n- Remove ES JS client requests and use native Node.js
HTTP client\r\ninstead\r\n- Limit the response size to 10MB for the
`autocomplete_entities` API\r\n\r\n#### How to test this PR
locally\r\nTo test this out, you will need to connect Kibana to a remote
cluster\r\nwith a large number of mappings. We created a patch file that
you can\r\napply to your local Kibana instance to test this PR. Since
the patch\r\nfile contains credentials, we can't share it publicly.
Please reach out\r\nto me if you would like to test this PR. I will
share the patch file and\r\nthe instructions to apply
it.\r\n\r\nCo-authored-by: Muhammad Ibragimov
<muhammad.ibragimov@elastic.co>","sha":"0e0140181faf65ad3041e22fc053e49c76f2ce47"}}]}]
BACKPORT-->

Co-authored-by: Muhammad Ibragimov <53621505+mibragimov@users.noreply.github.com>
@mistic mistic added v8.5.2 and removed v8.5.1 labels Nov 16, 2022
@mistic
Copy link
Contributor

mistic commented Nov 16, 2022

Changing v8.5.1 label into v8.5.2 as this PR didn't make into the build candidate.

@mibragimov mibragimov mentioned this pull request Nov 16, 2022
Closed
@rudolf rudolf mentioned this pull request Nov 17, 2022
Closed
@afharo afharo mentioned this pull request Dec 2, 2022
Closed
@yuliacech yuliacech mentioned this pull request Apr 3, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pgayvallet pgayvallet pgayvallet left review comments

@alisonelizabeth alisonelizabeth alisonelizabeth approved these changes

@alexwizp alexwizp Awaiting requested review from alexwizp

@sabarasaba sabarasaba Awaiting requested review from sabarasaba

Assignees

@mibragimov mibragimov

Labels

Feature:Console Dev Tools Console Feature Feature:Dev Tools release_note:fix Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more t// v8.5.2 v8.6.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Console] autocomplete_entities API crashes Kibana when response size is too big

9 participants

@mibragimov @elasticmachine @alisonelizabeth @kibana-ci @kibanamachine @mistic @sabarasaba @pgayvallet @alexwizp