Skip to content

Add heap stats logging to Service#1238

Merged
elrayle merged 11 commits into
clearlydefined:masterfrom
harrider:users/harrider/add-heap-stats-logging
Dec 12, 2024
Merged

Add heap stats logging to Service#1238
elrayle merged 11 commits into
clearlydefined:masterfrom
harrider:users/harrider/add-heap-stats-logging

Conversation

@harrider
Copy link
Copy Markdown
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
Copy Markdown
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!

Comment thread app.js Outdated
Comment thread minimal.env.json
Comment thread package.json Outdated
Comment thread app.js Outdated
@qtomlinson
Copy link
Copy Markdown
Collaborator

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

Comment thread lib/heapLogger.js
Copy link
Copy Markdown
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
Copy Markdown
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.

Comment thread lib/heapLogger.js Outdated
@elrayle elrayle merged commit a569bab into clearlydefined:master Dec 12, 2024
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