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
6 times, most recently
from
August 26, 2026 01:05
9cc3ba3 to
afa27c1
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.
- Reject a non-positive otlp.interval in validateTelemetryConfig: an
interval of "0s" (or negative) is silently replaced by the OTel SDK with
the 60s default, contradicting the configured value, so fail configuration
instead of exporting at an unexpected cadence.
- Never let PID assignment change a container's resctrl control group.
Writing a PID into a mon_group's tasks file moves the task into the group's
parent ctrl_group and rewrites its CLOSID, so assigning a container whose
RDT class differs from the class the pod's mon_group was created under would
silently overwrite that container's own CAT/MBA allocation (the off-class
sidecar case). EnsureGroup reports exactly that mismatch, so gate every
assignment on it: Synchronize, StartContainer, and PostStartContainer now
call EnsureGroup and skip AssignPID on any error instead of assigning
unconditionally (PostCreateContainer swallows the mismatch so it must be
re-checked at the assignment sites).
- Emit a k8s.node.name resource attribute from the injected NODE_NAME so
every series carries a stable per-node label, and expose the configured
resourceAttributes (and the node name) as Prometheus constant labels via
WithResourceAsConstantLabels; the Prometheus exporter otherwise surfaces
resource attributes only through target_info, so they were absent from the
l3_*/perf_* samples on the pull path.
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, and assert that an off-class sidecar's PID is
never written into a pod mon_group created under a different RDT class.
Deployment assets:
- Correct the helm namespace documentation and values, and fix the pod
legend in the energy dashboard.
- Extract the trailing numeric port from telemetry.prometheus.listenAddress
via a helper so IPv6 forms (e.g. "[::]:9200") advertise the actual port in
the scrape annotation and containerPort instead of falling back to 9100.
- 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.
- Aggregate the per-pod dashboard panels by (pod, namespace) instead of pod
alone, and label the series "namespace/pod", so that when the namespace
variable selects All (or several namespaces) two pods with the same name
in different namespaces are not merged into one misattributed series.
- Group the package power/activity panels by (k8s_node_name, domain_id) and
label them "<node> / pkg<N>", so identically numbered CPU packages on
different nodes are not merged into one cluster-wide series.
- 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". Drop the
"share of observed" activity stat panel: the plugin exports only
pod-sourced series, so its numerator and denominator matched and it
always reported 100%.
- 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. Add an OTLP HTTP receiver on 4318 (plus container and
Service ports) so the reference collector also accepts
telemetry.otlp.protocol=http, not only gRPC on 4317.
- Inject the node name into the plugin DaemonSet (NODE_NAME from
spec.nodeName) so the per-node telemetry label is populated.
- otel-collector: annotate the collector pods for Prometheus scrape
discovery (prometheus.io/scrape on port 8889) so its exporter is actually
collected, and set the Service internalTrafficPolicy to Local so each
node's cumulative OTLP stream is pinned to that node's own collector
instead of being load-balanced across agents (which would scatter partial,
double-counted copies of the same node's counter series).
- README: mark the AET perf/energy counters (rdt=perf) as pending upstream
rather than available in a released kernel, and rename the "OTel Collector
sidecar" section to "OTel Collector agent" to match the DaemonSet manifest.
- README/values: warn that a non-empty telemetry.prometheus.namespace
prefixes every metric name and breaks the bundled dashboards (which query
the unprefixed l3_*/perf_* names), and add a kube-state-metrics row to the
runtime-requirements table since the dashboards depend on kube_pod_info.
- install-plugins: run under 'set -e', and take each plugin's static NRI
index from the "-idx" flag in its own Dockerfile ENTRYPOINT instead of
installing every plugin under one shared default index. Plugins that
declare no static index (the mutually exclusive resource-policy plugins)
are skipped rather than co-installed.
docs: rewrite the "How It Works" mon_group lifecycle to reflect the
sandbox-scoped lifetime (the group persists across container restarts and is
removed on RemovePodSandbox, with the reconciler reaping orphans) and add the
telemetry block to the plugin configuration example.
Concurrency:
- Add a dedicated stateMu lifecycle/config lock so a dynamic configuration
reload is synchronized with the NRI callbacks. The stub dispatches
Configure, Synchronize, and the container handlers without a shared lock,
so a setConfig that swaps config/mgr/telemetry/metrics could otherwise
race a concurrent handler. Handlers now snapshot config and mgr under the
lock (getConfig/getManager) and operate on the snapshot; setConfig,
Configure, and onClose mutate the fields under the write lock. The
existing mu stays scoped to pendingRemoval and liveKeys.
docs: drop the plugin-ignored scrapeInterval from the telemetry.prometheus
config example (it is a Helm-only annotation hint, silently ignored by the
plugin's config parser; the Helm README already documents it).
Round 11 review:
- telemetry: set ReadHeaderTimeout (5s) on the Prometheus metrics HTTP
server. The listener is exposed on all interfaces by default and had no
header timeout, so a slow-header client could hold connections open
indefinitely and exhaust the plugin's file descriptors/goroutines
(Slowloris). Response timing is left unrestricted for metric collection.
- dashboards (perf counters): align the four overview stat cards on a single
row (y:1) instead of a diagonal, and shift the panels below up so no empty
gap remains.
- dashboards (pod energy): carry namespace through the workload join in the
energy- and activity-breakdown pie charts (group_left(namespace, workload),
sum by (namespace, workload), legend "namespace/workload") so two
identically named workloads in different namespaces are not merged into one
misattributed slice when several namespaces are selected.
Round 12 review:
- Canonicalize pod UIDs in the reconciler live set. PodUIDValidator accepts
both the dashed containerd form and the compact form some CRI-O versions
report; setLiveKeys and dropLiveKey now key liveKeys by
monitor.CanonicalizePodUID so a Synchronize and a later RemovePodSandbox
that report different-but-equivalent forms drop the same entry, instead of
leaving a removed sandbox permanently protected from orphan reconciliation.
Add a regression test covering the compact-store / dashed-remove path.
- otel-collector-agent: document that the chart must be installed with
telemetry.prometheus.enabled=false when scraping the collector. Both the
collector and the plugin's own Prometheus exporter carry the same counters
and are annotated for prometheus.io/scrape, so leaving both enabled makes
pod discovery ingest two copies and the dashboards' unscoped sum() queries
double-count.
- README: use the namespace-qualified OTLP endpoint example
(otel-collector-resctrl.monitoring.svc:4317) since the optional collector
Service is always created in the monitoring namespace and the plain name
will not resolve when the chart is installed elsewhere.
Round 13 review:
- Preserve a concurrent removal across a Synchronize pass. The wholesale
setLiveKeys replacement rebuilt liveKeys from the pass's pod snapshot, so a
RemovePodSandbox racing the pass (dropLiveKey, then Remove returning
ErrNotTracked for a container-less sandbox) could be overwritten and the
orphan treated as live by every later Reconcile. Synchronize now opens a
removal-tombstone window (beginSync); dropLiveKey records removals into it
while it is open, and setLiveKeys drops any tombstoned key from the snapshot
before committing it. Add a regression test.
- otel-collector-rbac: drop the unused apps/replicasets ClusterRole rule. Pod
association uses k8s.pod.uid and the k8sattributes metadata requests no
owner/ReplicaSet/Deployment fields, so the reference manifest no longer
grants unnecessary cluster-wide get/list/watch access.
Signed-off-by: Christopher M. Cantalupo <christopher.m.cantalupo@intel.com>
cmcantalupo
force-pushed
the
resctrl-mon-goresctrl
branch
from
August 26, 2026 01:54
afa27c1 to
f4d857b
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.