Add metrics for VM states #2649
Conversation
- Update Heartbeat.to_hash to include process_length attribute when present - Extract process_length from heartbeat payload in Riemann plugin - Attach process_length to each Riemann event (if present) - Support both symbol and string keys for payload compatibility - Add unit tests verifying process_length inclusion/omission in events - All unit tests pass (Riemann: 5/5, Heartbeat: 10/10)
cf-foundation-community-automation
bot
moved this to Inbox
in Foundational Infrastructure Working Group
rkoster
requested review from
a team,
ramonskie and
s4heid
and removed request for
a team
rkoster
moved this from Inbox
to Pending Review | Discussion
in Foundational Infrastructure Working Group
|
Cross linking agent changes cloudfoundry/bosh-agent#390 |
There was a problem hiding this comment.
Pull request overview
This PR adds new metrics for tracking VM states in BOSH, including unhealthy, failing, stopped, and unknown instances. The implementation introduces five new Prometheus metrics and corresponding API endpoints to expose these state counts per deployment, with agents reporting the number of running processes to detect unhealthy states.
Key changes:
- Adds
number_of_processesandjob_statetracking to the Agent class - Introduces five new metrics methods in InstanceManager for counting agents by state
- Creates corresponding API endpoints and Prometheus gauge metrics for each state
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| src/bosh-monitor/lib/bosh/monitor/agent.rb | Adds job_state and number_of_processes attributes to track agent health state |
| src/bosh-monitor/lib/bosh/monitor/instance_manager.rb | Implements five new metric calculation methods (unhealthy, failing, stopped, unknown, total_available agents) |
| src/bosh-monitor/lib/bosh/monitor/api_controller.rb | Adds five new GET endpoints to expose the VM state metrics |
| src/bosh-monitor/lib/bosh/monitor/events/heartbeat.rb | Updates heartbeat to include number_of_processes in the serialized hash |
| src/bosh-director/lib/bosh/director/metrics_collector.rb | Registers five new Prometheus gauges and fetches/populates metrics from health monitor endpoints |
| src/bosh-monitor/spec/unit/bosh/monitor/instance_manager_spec.rb | Adds comprehensive test coverage for all five new metric methods |
| src/bosh-monitor/spec/unit/bosh/monitor/api_controller_spec.rb | Adds tests for all five new API endpoints including 503 responses |
| src/bosh-monitor/spec/unit/bosh/monitor/agent_spec.rb | Tests that update_instance preserves job_state and number_of_processes |
| src/bosh-director/spec/unit/bosh/director/metrics_collector_spec.rb | Tests Prometheus metric registration and population for all new VM state metrics |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Fetch and populate total_available_agents metrics | ||
| response_total = Net::HTTP.get_response('127.0.0.1', '/total_available_agents', @config.health_monitor_port) | ||
| if response_total.is_a?(Net::HTTPSuccess) | ||
| total_agent_counts = JSON.parse(response_total.body) rescue nil |
There was a problem hiding this comment.
The rescue nil pattern silently swallows all exceptions during JSON parsing. If the parsing fails for unexpected reasons (not just malformed JSON), this could hide bugs. Consider using a more specific rescue clause like rescue JSON::ParserError or at least logging the error when it occurs.
| total_agent_counts = JSON.parse(response_total.body) rescue nil | |
| total_agent_counts = begin | |
| JSON.parse(response_total.body) | |
| rescue JSON::ParserError => e | |
| @logger.warn("Failed to parse total_available_agents response as JSON: #{e.message}") if @logger | |
| nil | |
| end |
| # Fetch and populate stopped_instances metrics | ||
| response_stopped = Net::HTTP.get_response('127.0.0.1', '/stopped_instances', @config.health_monitor_port) | ||
| if response_stopped.is_a?(Net::HTTPSuccess) | ||
| stopped_counts = JSON.parse(response_stopped.body) rescue nil |
There was a problem hiding this comment.
The rescue nil pattern silently swallows all exceptions during JSON parsing. Consider using a more specific rescue clause like rescue JSON::ParserError for consistency with error handling best practices.
| # Fetch and populate unknown_instances metrics | ||
| response_unknown = Net::HTTP.get_response('127.0.0.1', '/unknown_instances', @config.health_monitor_port) | ||
| if response_unknown.is_a?(Net::HTTPSuccess) | ||
| unknown_counts = JSON.parse(response_unknown.body) rescue nil |
There was a problem hiding this comment.
The rescue nil pattern silently swallows all exceptions during JSON parsing. Consider using a more specific rescue clause like rescue JSON::ParserError for consistency with error handling best practices.
| # Fetch and populate failing_instances metrics | ||
| response_failing = Net::HTTP.get_response('127.0.0.1', '/failing_instances', @config.health_monitor_port) | ||
| if response_failing.is_a?(Net::HTTPSuccess) | ||
| failing_counts = JSON.parse(response_failing.body) rescue nil |
There was a problem hiding this comment.
The rescue nil pattern silently swallows all exceptions during JSON parsing. Consider using a more specific rescue clause like rescue JSON::ParserError for consistency with error handling best practices.
src/bosh-director/spec/unit/bosh/director/metrics_collector_spec.rb
Outdated
Show resolved
Hide resolved
3 participants
What is this change about?
This adds new bosh metrics regarding the VM states. Initially the idea was proposed in this PR that which was reverted here due to breaking the system design.
Please provide contextual information.
Initial PR which was reverted defined the state "unhealthy" if the VM state is not "running". In this PR, each state has its own metric, and there is additional metric "unhealthy" when the VM state is running but doesn't have any processes. This requires modification in the agent heartbeat to include the number of processes. The corresponding PR for the agent heartbeat is here.
The new metrics are:
What tests have you run against this PR?
Ran the unit tests and also tested a bosh dev release on a development environment with the modified bosh-agent.
How should this change be described in bosh release notes?
Bosh now emits different states of VM as metrics, which can be used to monitor the state of the instances per deployment over time
Does this PR introduce a breaking change?
No
Tag your pair, your PM, and/or team!
@DennisAhausSAP