resctrl-mon: OTel telemetry via goresctrl pkg/monitor - #757
Draft
cmcantalupo wants to merge 2 commits into
Draft
Conversation
cmcantalupo
marked this pull request as draft
August 18, 2026 20:57
cmcantalupo
force-pushed
the
resctrl-mon-goresctrl
branch
from
August 18, 2026 21:00
03ebd56 to
1cd4fc7
Compare
There was a problem hiding this comment.
Pull request overview
Reworks the resctrl-mon NRI plugin to delegate resctrl monitoring-group lifecycle and counter export to github.com/intel/goresctrl/pkg/monitor, while embedding an OpenTelemetry SDK for Prometheus scraping (and optional OTLP push). This aligns metric naming with existing RDT conventions and adds Helm wiring + dashboards for the new OTel metric names.
Changes:
- Replace inline resctrl mon_group/state handling with
monitor.Manager, and register OTel instruments viaRegisterOTelInstruments(). - Add embedded telemetry stack (Prometheus endpoint + optional OTLP), plus unit tests for registration/reload and float64 counter fidelity.
- Update Helm chart values/templates and add optional OTel Collector + Grafana dashboards.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| sample-configs/nri-resctrl-mon.yaml | Adds telemetry configuration example block. |
| Makefile | Adds install-plugins target for static NRI plugin discovery. |
| go.mod | Adds temporary replace for unreleased goresctrl/pkg/monitor and updates OTel/Prometheus deps. |
| go.sum | Dependency checksum updates for new/updated modules. |
| deployment/helm/resctrl-mon/values.yaml | Adds telemetry configuration values (Prometheus/OTLP/perfCounters). |
| deployment/helm/resctrl-mon/templates/daemonset.yaml | Exposes metrics port + Prometheus scrape annotations when enabled. |
| deployment/helm/resctrl-mon/templates/configmap.yaml | Renders telemetry config into plugin config ConfigMap. |
| deployment/helm/resctrl-mon/README.md | Documents telemetry options and Prometheus integration guidance. |
| deployment/helm/resctrl-mon/optional/otel-collector-rbac.yaml | Optional RBAC for OTel Collector k8sattributes processor. |
| deployment/helm/resctrl-mon/optional/otel-collector-agent.yaml | Optional OTel Collector DaemonSet to receive/enrich/fan-out OTLP metrics. |
| deployment/helm/resctrl-mon/optional/grafana-resctrl-pod-energy.json | Grafana dashboard for per-pod energy/activity metrics using new names. |
| deployment/helm/resctrl-mon/optional/grafana-resctrl-perf-counters.json | Grafana dashboard for AET perf counters using new names. |
| cmd/plugins/resctrl-mon/telemetry.go | New embedded OTel SDK wiring (Prometheus exporter + optional OTLP). |
| cmd/plugins/resctrl-mon/telemetry_test.go | New tests for Prometheus output, float64 fidelity, perf-counter gating, naming. |
| cmd/plugins/resctrl-mon/metrics.go | New OTel instrument registration and filtering/attributes logic. |
| cmd/plugins/resctrl-mon/main.go | Starts telemetry during plugin startup. |
| cmd/plugins/resctrl-mon/plugin.go | Switches plugin lifecycle to monitor.Manager; adds telemetry/registration teardown & reconcile logic. |
| cmd/plugins/resctrl-mon/plugin_test.go | Updates tests to use monitor.Manager and adds config-reload regression test. |
| cmd/plugins/resctrl-mon/state.go | Removed legacy in-memory pod/container tracking. |
| cmd/plugins/resctrl-mon/resctrl.go | Removed legacy resctrl filesystem ops implementation. |
| cmd/plugins/resctrl-mon/resctrl_test.go | Removed legacy resctrl ops tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
363
to
365
| if !p.shouldMonitorPod(pod) { | ||
| return nil | ||
| } |
Comment on lines
+136
to
+158
| p.config = &cfg | ||
| p.rdt = newResctrlOps(cfg.ResctrlPath) | ||
|
|
||
| // Stop the old reconciler before swapping the manager; it holds a | ||
| // reference to the old manager and would otherwise leak. | ||
| if p.stopReconciler != nil { | ||
| close(p.stopReconciler) | ||
| p.stopReconciler = nil | ||
| } | ||
|
|
||
| // If telemetry is already running, its OTel batch callback is bound to the | ||
| // manager we are about to replace (which is pinned to the previous | ||
| // ResctrlPath). Tear it down so we neither leak the old manager nor keep a | ||
| // second registration reading the old root, then restart it against the new | ||
| // manager below. | ||
| restartTelemetry := p.telemetry != nil | ||
| if restartTelemetry { | ||
| if p.metrics != nil { | ||
| _ = p.metrics.Unregister() | ||
| p.metrics = nil | ||
| } | ||
| p.telemetry.shutdown(context.Background()) | ||
| p.telemetry = nil | ||
| } |
Comment on lines
+111
to
+113
| if p.telemetry != nil { | ||
| p.telemetry.shutdown(context.Background()) | ||
| } |
Comment on lines
+123
to
+132
| mux := http.NewServeMux() | ||
| mux.Handle("/metrics", promhttp.HandlerFor(reg, promhttp.HandlerOpts{})) | ||
| state.server = &http.Server{Addr: cfg.Prometheus.ListenAddress, Handler: mux} | ||
| go func() { | ||
| if err := state.server.ListenAndServe(); err != nil && err != http.ErrServerClosed { | ||
| log.Warnf("telemetry: prometheus server error: %v", err) | ||
| } | ||
| }() | ||
| log.Infof("telemetry: Prometheus endpoint listening on %s/metrics", cfg.Prometheus.ListenAddress) | ||
| } |
Comment on lines
+53
to
+55
| ScanAllGroups bool `json:"scanAllGroups"` | ||
| ResourceAttributes map[string]string `json:"resourceAttributes"` | ||
| } |
cmcantalupo
force-pushed
the
resctrl-mon-goresctrl
branch
from
August 21, 2026 18:51
cbd55d9 to
814be27
Compare
Rework the resctrl-mon NRI plugin to manage per-pod resctrl monitoring groups and export their counters through goresctrl/pkg/monitor and an embedded OpenTelemetry SDK, replacing the plugin's inline resctrl handling. Depends on goresctrl PR intel/goresctrl#192 (pkg/monitor), which is not yet merged or released. Until a goresctrl release containing pkg/monitor is available, go.mod carries a temporary replace pointing github.com/intel/ goresctrl at the PR containers#192 head commit (cmcantalupo/goresctrl@3705888). That replace must be dropped and the require bumped once the release ships; the plugin is not mergeable until then. Plugin: - Replace inline resctrl management (resctrl.go, state.go) with monitor.Manager; delegate instrument naming, monotonic accumulation, and counter discovery to monitor.RegisterOTelInstruments(). - Embed an OTel SDK (telemetry.go): Prometheus pull exporter on :9100 plus optional OTLP push. Tear down the OTel registration on onClose and on dynamic config reload so the previous Manager/registration is not leaked and telemetry is rebound to the new Manager. - perfCounterFilter gates perf-counter include/exclude; groupAttributes injects k8s.pod.uid, resctrl.control_group, source. Float64 counter fidelity is preserved from the kernel (no integer truncation). - mon_group lifetime is scoped to the pod sandbox: PostCreateContainer creates the group idempotently and RemovePodSandbox tears it down. StopContainer is intentionally not handled, so a container restart keeps the RMID stable and avoids residual-counter energy spikes; orphans from a missed teardown are reconciled in the background. Metric names use the domain-derived convention, aligning L3 counters with pkg/rdt's existing Prometheus names: l3_llc_occupancy_bytes, l3_mbm_local_bytes_total, l3_mbm_total_bytes_total, perf_core_energy_joules_total, perf_activity_farads_total Helm chart: - values.yaml telemetry block (prometheus/otlp/perfCounters); ConfigMap renders the full telemetry config; container port 9100 with prometheus.io/scrape annotations. - Optional OTel Collector DaemonSet + k8sattributes RBAC manifests. - Two Grafana dashboards using the OTel metric names, joined to kube_pod_info on the Pod UID. The join collapses kube_pod_info to one series per Pod before the many-to-one match, so a freshly created Pod (whose empty pod_ip yields a second series for the same uid) no longer fails the query with a duplicate-match-group error. Makefile: add an install-plugins target for static NRI discovery. Signed-off-by: Jedrzej Wasiukiewicz <jedrzej.wasiukiewicz@intel.com> Signed-off-by: Christopher M. Cantalupo <christopher.m.cantalupo@intel.com>
cmcantalupo
force-pushed
the
resctrl-mon-goresctrl
branch
2 times, most recently
from
August 24, 2026 16:04
475eda4 to
0775be7
Compare
Fold the review-round fixes for the resctrl-mon plugin, its tests, and the
reference deployment assets into a single change on top of the initial
OTel-telemetry feature.
Plugin lifecycle and config reload:
- Start telemetry from Configure() once a configuration is available rather
than before the plugin is configured, and shut the previous telemetry
down under a bounded timeout on reload.
- Treat the resctrl root as immutable once the plugin is running: reject a
setConfig that changes resctrlPath after telemetry/reconciliation start
instead of swapping in an empty manager (which would drop the in-memory
tracking and PIDs for every live pod, skip re-synchronization, and let a
pending removal bound to the old manager delete a same-UID group in the
new one). The initial configuration (from a --config file and/or the NRI
server, applied before the plugin runs) may still select a non-default
root and rebuild the manager; a restart is required to change it later.
- Fix the reconciler lifecycle: capture the stop channel and manager in
locals, and restart the reconciler only when the manager is (re)created.
- Publish the telemetry state only after instrument registration succeeds.
- Make a telemetry/filter reload transactional: if restarting telemetry
fails (e.g. the new Prometheus port is occupied), roll back to the
previous config, manager, telemetry, and reconciler instead of leaving
them permanently disabled.
- Retry leaked mon_groups: track keys whose Remove failed in a pending set
and retry them from the reconciler so a failed rmdir is not lost.
Synchronize and metrics:
- Seed the reconcile live set from the monitored pod sandboxes, not only
from containers, so a sandbox that is alive with no running container
(e.g. between container restarts) keeps its mon_group. Remember that live
set so the background reconciler keeps protecting a container-less
sandbox (which is never passed to EnsureGroup and so never appears in
Manager.List()); drop a key from it in RemovePodSandbox so a truly gone
sandbox can still be reaped.
- Replace the per-call regex glob with a simple wildcard matcher.
- Derive the resctrl control group relative to the configured root so a
non-default root (e.g. /mnt/rdt) is handled correctly.
- The manager validates and tracks pod UIDs only, so fix the
resctrl.group.source attribute to the constant "pod" and drop the
unreachable non-pod ("other") classification.
- Default otlp.insecure to true so the runtime matches the chart, sample
config, and README, which all document plaintext OTLP by default.
Tests:
- Harden the existing tests, bind the telemetry test to an ephemeral port,
table-drive controlGroupOf, and add coverage for orphan mon_group
removal, pending-removal retry, rejected resctrl root changes on a running
plugin, accepted initial root selection, and reconciler preservation of a
container-less live sandbox.
Deployment assets:
- Correct the helm namespace documentation and values, and fix the pod
legend in the energy dashboard.
- Translate the dashboard metric and label names to the names the OTel
Prometheus exporter actually emits (e.g. perf_core_energy_joules_total,
k8s_pod_uid), and wire the DS_PROMETHEUS and namespace template variables
through both dashboards so they import against any Prometheus datasource.
- Relabel the misleading "all workloads" package panels to reflect that
only per-pod mon_groups are observed, fix the domain legend, and filter
the package-total panels on resctrl_group_source="pod".
- otel-collector: promote the k8s.pod.uid data-point attribute to resource
scope with groupbyattrs so k8sattributes can associate it, add the Service
that exposes the documented OTLP endpoint, create the monitoring namespace
so the reference manifests apply on a clean cluster, and correct the
endpoint examples.
- README: mark the AET perf/energy counters (rdt=perf) as pending upstream
rather than available in a released kernel.
- Run install-plugins under 'set -e' so a failed install aborts the target.
Signed-off-by: Christopher M. Cantalupo <christopher.m.cantalupo@intel.com>
cmcantalupo
force-pushed
the
resctrl-mon-goresctrl
branch
from
August 24, 2026 18:57
0775be7 to
ce27891
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Rework the
resctrl-monNRI plugin to manage per-pod resctrl monitoring groups and export their counters throughgoresctrl/pkg/monitorand an embedded OpenTelemetry SDK, replacing the plugin's inline resctrl handling.Important
Depends on goresctrl PR intel/goresctrl#192 (
pkg/monitor), which is not yet merged or released. Until a goresctrl release containingpkg/monitoris available,go.modcarries a temporaryreplacepointinggithub.com/intel/goresctrlat the PR #192 head commit (cmcantalupo/goresctrl@3705888). Thatreplacemust be dropped and therequirebumped once the release ships. This PR is not mergeable until then.Motivation
The
resctrl-monplugin previously created and read monitoring groups with its own resctrl code. That logic — mon_group lifecycle, counter discovery/typing, instrument naming, monotonic accumulation — is now provided in reusable form bygoresctrl/pkg/monitor. Delegating to it removes duplicated resctrl handling from the plugin and lets the plugin focus on NRI lifecycle and telemetry wiring, while picking up support for allmon_datadomains including Intel AET (PERF_PKG) energy/perf counters.What changed
Plugin (
cmd/plugins/resctrl-mon/):resctrl.go,state.go) withmonitor.Manager; delegate instrument naming, monotonic accumulation, and counter discovery tomonitor.RegisterOTelInstruments().telemetry.go: embedded OTel SDK — Prometheus pull exporter on:9100plus optional OTLP push. The OTel registration is torn down ononCloseand on dynamic config reload, so the previousManager/registration is not leaked and telemetry is rebound to the newManager.metrics.go:perfCounterFiltergates perf-counter include/exclude; the group attributes injectk8s.pod.uid,resctrl.control_group, and aresctrl.group.sourceofpod— the manager validates and tracks pod UIDs only, so every exported mon_group is pod-sourced. Float64 counter fidelity is preserved from the kernel (no integer truncation).PostCreateContainercreates the group idempotently andRemovePodSandboxtears it down.StopContaineris intentionally not handled, so a container restart keeps the RMID stable and avoids residual-counter energy spikes; orphans from a missed teardown are reconciled in the background.Metric names use the domain-derived convention, aligning L3 counters with
pkg/rdt's existing Prometheus names:l3_llc_occupancy_bytes,l3_mbm_local_bytes_total,l3_mbm_bytes_total,perf_core_energy_joules_total,perf_activity_farads_total. (Thembm_total_bytesinstrument's semantictotalis folded into the counter_totalsuffix by the OTel Prometheus namer, so it is exported asl3_mbm_bytes_total.)Helm chart (
deployment/helm/resctrl-mon/):values.yamltelemetry block (prometheus / otlp / perfCounters); ConfigMap renders the full telemetry config; container port9100withprometheus.io/scrapeannotations.optional/.kube_pod_infoon the Pod UID. The join collapseskube_pod_infoto one series per Pod before the many-to-one match, so a freshly created Pod (whose emptypod_ipyields a second series for the same uid) no longer fails the query with a duplicate-match-group error.Makefile: add an
install-pluginstarget for static NRI discovery.Testing
go build ./...,go vet ./cmd/plugins/resctrl-mon/..., andgo test ./cmd/plugins/resctrl-mon/...pass.