Skip to content

Enable heap snapshots for POSIX distributables #182082

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
jloleysens merged 5 commits into from
May 2, 2024
Merged

Enable heap snapshots for POSIX distributables #182082

jloleysens merged 5 commits into from
May 2, 2024

Conversation

@jloleysens
Copy link
Contributor

@jloleysens jloleysens commented Apr 30, 2024 •
edited
Loading

Summary

See #181363

Changed the approach to only enable this by default for POSIX platforms (added defaults to the bin/kibana.sh script). Can be tested in the same way as the previous PR but only available on Linux/macOS.

@jloleysens jloleysens added Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// release_note:skip Skip the PR/issue when compiling release notes v8.15.0 labels Apr 30, 2024
@jloleysens jloleysens self-assigned this Apr 30, 2024
@jloleysens jloleysens requested a review from a team as a code owner April 30, 2024 09:10
@elasticmachine
Copy link
Contributor

elasticmachine commented Apr 30, 2024

Pinging @elastic/kibana-core (Team:Core)

@jloleysens jloleysens changed the title Enable heap snapshots for UNIX distributables Enable heap snapshots for POSIX distributables Apr 30, 2024
@jloleysens jloleysens requested a review from a team April 30, 2024 09:23
jbudz
jbudz previously approved these changes Apr 30, 2024
View reviewed changes
@jbudz jbudz self-requested a review April 30, 2024 13:47
jbudz
jbudz reviewed Apr 30, 2024
View reviewed changes
src/dev/build/tasks/bin/scripts/kibana Outdated
NODE_OPTIONS="--no-warnings --max-http-header-size=65536 $KBN_NODE_OPTS $NODE_OPTIONS" NODE_ENV=production exec "${NODE}" "${DIR}/src/cli/dist" "${@}"
# Enable capturing heap snapshots. See https://nodejs.org/api/cli.html#--heapsnapshot-signalsignal
# For now only on POSIX platforms, term signals work differently on Windows.
NODE_OPTIONS_HEAPSNAPSHOT_DEFAULT="--heapsnapshot-signal=SIGUSR2 --diagnostic-dir=./data"
Copy link
Contributor

@jbudz jbudz Apr 30, 2024

Choose a reason for hiding this comment

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

On deb and rpm package this isn't going to work. The kibana user will not have write access to anything in /usr/share/kibana. The data directory is at /var/lib/kibana.

Copy link
Contributor

@jbudz jbudz Apr 30, 2024

Choose a reason for hiding this comment

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

For tar.gz it should be something like this to support starting Kibana from outside the root directory:

Suggested change
NODE_OPTIONS_HEAPSNAPSHOT_DEFAULT="--heapsnapshot-signal=SIGUSR2 --diagnostic-dir=./data"
NODE_OPTIONS_HEAPSNAPSHOT_DEFAULT="--heapsnapshot-signal=SIGUSR2 --diagnostic-dir=${DIR}/data"

Copy link
Contributor

@jbudz jbudz Apr 30, 2024

Choose a reason for hiding this comment

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

https://github.com/elastic/kibana/blob/4b4c573904b35650701a695767d560ee4f3917f3/src/dev/build/tasks/os_packages/service_templates/env/kibana:

NODE_OPTIONS_HEAPSNAPSHOT_DEFAULT="--heapsnapshot-signal=SIGUSR2 --diagnostic-dir=/var/lib/kibana"

and then in bin/kibana:

NODE_OPTIONS_HEAPSNAPSHOT_DEFAULT="${NODE_OPTIONS_HEAPSNAPSHOT_DEFAULT:---heapsnapshot-signal=SIGUSR2 --diagnostic-dir=${DIR}/data}"

Mind if we add the ci:build-all-platforms and ci:build-os-packages labels after to test if we go this route?

@jbudz jbudz dismissed their stale review April 30, 2024 13:58

comment

jloleysens
jloleysens commented May 1, 2024
View reviewed changes
Copy link
Contributor Author

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

Also tested submitting a heap snapshot workflow on serverless

@kibana-ci
Copy link

kibana-ci commented May 1, 2024 •
edited
Loading

💚 Build Succeeded

The CI Stats report is too large to be displayed here, check out the CI build annotation for this information.

History

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

cc @jloleysens

@jloleysens jloleysens merged commit 687e30b into elastic:main May 2, 2024
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label May 2, 2024
@jloleysens jloleysens deleted the heap-snapshots branch May 2, 2024 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pgayvallet pgayvallet pgayvallet approved these changes

@jbudz jbudz jbudz approved these changes

Assignees

@jloleysens jloleysens

Labels

backport:skip This PR does not require backporting ci:build-all-platforms ci:build-os-packages ci:project-deploy-observability Create an Observability project release_note:skip Skip the PR/issue when compiling release notes Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v8.15.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@jloleysens @elasticmachine @kibana-ci @pgayvallet @jbudz @kibanamachine