Skip to content
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.

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

Add heap stats logging to Service #1238

Merged

Conversation

harrider
Copy link
Contributor

Description

It would be useful to gain insight into memory usage in helping to debug runtime issues.

Work Done

  • Added the v8 npm package to gather heap usage statistics
  • Added conditional logging to log these heap usage stats
  • Added 2 environment variables to control heap stats logging and interval
    • LOG_NODE_HEAPSTATS
      • string value of true or false (case insensitive) to enable the heap stats logging
    • LOG_NODE_HEAPSTATS_INTERVAL_MS
      • string value of a number (in milliseconds) representing the interval to log heap stats at
  • Updated <type>.env.json files to add new env vars for visibility
  • Updated config.js to look for new env var values

@mpcen mpcen self-requested a review November 20, 2024 16:43
Copy link
Collaborator

@qtomlinson qtomlinson 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 adding another tool to our troubleshooting toolkit!

app.js Outdated Show resolved Hide resolved
minimal.env.json Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
app.js Outdated Show resolved Hide resolved
@qtomlinson
Copy link
Collaborator

@mpcen @elrayle Our docker run command to run the service container may have to be adjusted (--memory) to reflect this change. Any thoughts?

Copy link
Collaborator

@qtomlinson qtomlinson 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 the detailed documentation!

Copy link
Collaborator

@elrayle elrayle left a comment

Choose a reason for hiding this comment

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

Made the Copyright change a suggestion so we can get this merged.

lib/heapLogger.js Outdated Show resolved Hide resolved
@elrayle elrayle merged commit a569bab into clearlydefined:master Dec 12, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants