Skip to content

ROX-35448: add VM fleet telemetry via ClusterMetrics - #22311

Draft
vikin91 wants to merge 7 commits into
masterfrom
piotr/ROX-35448-vm-fleet-telemetry
Draft

ROX-35448: add VM fleet telemetry via ClusterMetrics#22311
vikin91 wants to merge 7 commits into
masterfrom
piotr/ROX-35448-vm-fleet-telemetry

Conversation

@vikin91

@vikin91 vikin91 commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Description

ACS had no per-secured-cluster signal for VM scanning: whether it is on, how many VMs Sensor is tracking, how many have been scraped, or which roxagent versions are in the fleet. That belongs on the existing cluster-metrics / Segment identity path, next to node count and CPU capacity.

Sensor's VM scraper now exposes a point-in-time Stats() snapshot (tracked VMs, VMs with at least one successful scrape, and a version histogram capped at the top 20 versions plus an other bucket). When VM scanning is enabled and the scraper is running, the cluster-metrics component copies that snapshot into a new optional VirtualMachineMetrics field on ClusterMetrics.

Every current Sensor advertises a VirtualMachineTelemetry capability on Hello, including when scanning is off. Central uses that to tell three cases apart:

  • capable Sensor with metrics present: scanning is on; set VM Scanning Enabled=true and the counts
  • capable Sensor with no metrics: scanning is confirmed off; zero the VM traits (including Roxagent Version Counts as []) so stale values do not linger
  • Sensor without the capability: leave existing VM traits untouched (predates this field)

Roxagent version counts are sent as a sorted JSON array trait so the payload is stable.

The proto also declares backoff and trailing scan-duration fields so the message shape is complete on the wire. This PR leaves them at zero; populating them needs the per-VM retry-backoff work (ROX-35445).

The cluster-metrics pipeline treats a nil message injector as "no capability" so tests (and any nil-injector path) do not panic on HasCapability.

User-facing documentation

Internal Segment traits only; no operator-facing behavior change.

Testing and quality

  • the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Stats are only attached when ROX_VIRTUAL_MACHINES is on and the scraper exists. The Hello capability is unconditional so Central can zero traits when scanning is off on a current Sensor.

Automated testing

  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

  • CI
  • Manually verified on a cluster

AI-Assisted: cursor, generated scraper stats, ClusterMetrics wiring, Segment trait mapping, and the nil-injector fix; user reviewed.

vikin91 and others added 7 commits August 14, 2026 12:04
Define the full VirtualMachineMetrics and ScanDurationStats shape up
front. Fields 3-6 (backoff, scan_duration) are declared here but will
be populated in a follow-up part that depends on the retry-backoff PR.

ROX-35448

Partially generated by AI.

Co-authored-by: Cursor <cursoragent@cursor.com>
Capture lastAgentVersion from ResponseMeta on each successful scrape
(already read but discarded). Stats() iterates vmState under s.mu to
produce TrackedVMs, VMsScanned, and VersionCounts (top-20 + "other" cap,
empty version bucketed as "unknown").

VMsInBackoff, AvgBackoff, MaxBackoff, ScanDuration are declared on the
Stats struct but left zero-valued until Part 3, which depends on the
per-VM retry backoff PR (ROX-35445).

ROX-35448

Partially generated by AI.

Co-authored-by: Cursor <cursoragent@cursor.com>
Add VMStatsSource interface to clustermetrics package. NewWithInterval
now accepts an optional VMStatsSource; when non-nil, collectMetrics
populates VirtualMachineMetrics with tracked_vms, vms_scanned, and
roxagent_version_counts. sensor.go passes the VMScraper instance when
VM scanning is enabled, nil otherwise.

ROX-35448

Partially generated by AI.

Co-authored-by: Cursor <cursoragent@cursor.com>
Declared on every Hello regardless of whether VM scanning is currently
enabled, so Central can distinguish "feature off on a new Sensor" from
"old Sensor that predates VM telemetry" and zero stale traits only in
the former case.

ROX-35448

Partially generated by AI.

Co-authored-by: Cursor <cursoragent@cursor.com>
Three-way case split on (HasCapability, GetVirtualMachineMetrics):
- capable + non-nil: set VM Scanning Enabled=true with counts
- capable + nil: zero all VM traits (feature confirmed off)
- no capability: leave existing traits untouched (old Sensor)

Roxagent version counts are serialized as a sorted JSON array.
UpdateSecuredClusterIdentity gains a hasVMTelemetryCap parameter;
the pipeline reads it from the connection's capability set.

ROX-35448

Partially generated by AI.

Co-authored-by: Cursor <cursoragent@cursor.com>
Verify that VMScraper Stats() flows through collectMetrics and populates
the VirtualMachineMetrics proto end-to-end.

Partially generated by AI (ROX-35448).

Co-authored-by: Cursor <cursoragent@cursor.com>
The cluster-metrics pipeline called HasCapability on the injector
unconditionally, which panics when tests (and any nil-injector path)
pass a nil MessageInjector. Treat nil as lacking the capability, and
assert the full Segment trait map including empty/sorted version JSON.

User request: fix the nil-injector panic and incomplete buildVMTraits
assertions, then commit.

Partially generated by AI.
@openshift-ci

openshift-ci Bot commented Aug 14, 2026

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@coderabbitai

coderabbitai Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: dfcd8c7b-45cc-41e3-a310-6833e4d96db8

📥 Commits

Reviewing files that changed from the base of the PR and between 89f9442 and 1645e70.

⛔ Files ignored due to path filters (2)
  • generated/internalapi/central/cluster_metrics.pb.go is excluded by !**/*.pb.go, !**/generated/**
  • generated/internalapi/central/cluster_metrics_vtproto.pb.go is excluded by !**/*.pb.go, !**/generated/**
📒 Files selected for processing (12)
  • central/cluster/datastore/telemetry.go
  • central/cluster/datastore/telemetry_test.go
  • central/sensor/service/pipeline/clustermetrics/pipeline.go
  • central/sensor/service/pipeline/clustermetrics/pipeline_test.go
  • pkg/centralsensor/caps_list.go
  • proto/internalapi/central/cluster_metrics.proto
  • sensor/common/sensor/central_communication_impl.go
  • sensor/common/virtualmachine/vmscraper/scraper.go
  • sensor/common/virtualmachine/vmscraper/scraper_test.go
  • sensor/kubernetes/clustermetrics/cluster_metrics.go
  • sensor/kubernetes/clustermetrics/cluster_metrics_test.go
  • sensor/kubernetes/sensor/sensor.go

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added virtual machine telemetry to cluster metrics, including tracked and scanned VM counts, scan activity, backoff statistics, scan durations, and Rox Agent version counts.
    • Central now displays VM telemetry in secured-cluster identity data when supported.
    • Added capability detection so newer Sensors can report VM scanning status accurately.
    • Version reporting is limited to the top entries with remaining versions grouped together.
  • Bug Fixes

    • Preserved existing telemetry behavior for Sensors that do not provide VM metrics.

Walkthrough

The change adds VM telemetry metrics to cluster metrics, tracks VM agent versions, advertises Sensor capability support, and merges capability-aware VM traits into Central secured-cluster identity telemetry.

Changes

Virtual machine telemetry

Layer / File(s) Summary
Telemetry contracts and capability advertisement
proto/internalapi/central/cluster_metrics.proto, pkg/centralsensor/caps_list.go, sensor/common/sensor/central_communication_impl.go
Cluster metrics define VM statistics messages. Sensors advertise VirtualMachineTelemetryCap.
VM scraper statistics
sensor/common/virtualmachine/vmscraper/scraper.go, sensor/common/virtualmachine/vmscraper/scraper_test.go
VMScraper records agent versions and reports VM counts, scan statistics, and capped deterministic version-count buckets.
Cluster metrics integration
sensor/kubernetes/clustermetrics/*, sensor/kubernetes/sensor/sensor.go
Cluster metrics accepts an optional VM statistics source. Sensor initialization supplies the VM scraper when VM support is enabled.
Central identity telemetry traits
central/cluster/datastore/telemetry.go, central/cluster/datastore/telemetry_test.go, central/sensor/service/pipeline/clustermetrics/*
Central generates capability-aware VM traits. The cluster-metrics pipeline queries the capability and passes it to secured-cluster identity updates.

Estimated code review effort: 3 (Moderate) | ~30 minutes

Merge Risk: ⚪ Minimal · up to 1645e

This PR adds VM fleet telemetry and compatibility handling without any identified current-head correctness, availability, security, or deployment risk; no actionable merge-blocking risk remains beyond normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant VMScraper
  participant ClusterMetrics
  participant Pipeline
  participant Central
  VMScraper->>ClusterMetrics: Provide VM statistics
  ClusterMetrics->>Pipeline: Emit cluster metrics
  Pipeline->>Central: Update secured-cluster identity with capability
  Central->>Central: Merge VM telemetry traits
Loading

Suggested reviewers: guzalv

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding VM fleet telemetry through ClusterMetrics.
Description check ✅ Passed The description explains the implementation, behavior, testing, documentation status, and validation; only CI inspection remains unchecked.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch piotr/ROX-35448-vm-fleet-telemetry

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

🚀 Build Images Ready

Images are ready for commit 1645e70. To use with deploy scripts:

export MAIN_IMAGE_TAG=4.12.x-776-g1645e7062c

@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.30189% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.21%. Comparing base (23b5732) to head (1645e70).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
sensor/kubernetes/sensor/sensor.go 0.00% 18 Missing ⚠️
central/cluster/datastore/telemetry.go 89.65% 3 Missing ⚠️
sensor/common/virtualmachine/vmscraper/scraper.go 95.12% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #22311      +/-   ##
==========================================
- Coverage   51.23%   51.21%   -0.02%     
==========================================
  Files        2855     2855              
  Lines      178712   178796      +84     
==========================================
+ Hits        91568    91579      +11     
- Misses      79082    79136      +54     
- Partials     8062     8081      +19     
Flag Coverage Δ
go-unit-tests 51.21% <78.30%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant