diff --git a/doc/resctrl-mon.md b/doc/resctrl-mon.md index 9014cfd..6e02860 100644 --- a/doc/resctrl-mon.md +++ b/doc/resctrl-mon.md @@ -105,6 +105,9 @@ mgr.AssignPID(podUID, pid) // Tie the group's lifetime to the pod, not to individual containers — a // container restart reuses the sandbox, and releasing the RMID early would // hand the replacement a fresh RMID whose counters carry residual values. +// Remove only when the pod's last sandbox is gone: kubelet can create a new +// sandbox for the same pod UID and garbage-collect the old one while the pod +// keeps running. mgr.Remove(podUID) // Crash recovery: remove orphaned groups not in the live set @@ -114,11 +117,53 @@ mgr.Reconcile(liveKeys) readings, _ := mgr.ReadCounters(podUID) for _, r := range readings { // r.Kind: monitor.Gauge (instantaneous) or monitor.Cumulative (monotonic counter) - // r.Unit: UCUM where available ("By", "J"), otherwise descriptive ("farads", "") + // r.Unit: UCUM unit of the raw kernel value where known ("By", "J", "nF"), otherwise "" fmt.Printf("%s/%s = %f (%v, %s)\n", r.Domain, r.Name, r.Value, r.Kind, r.Unit) } ``` +## OpenTelemetry Export + +`Manager.RegisterOTelInstruments` registers one observable instrument per +counter discovered under the resctrl root's `mon_data`. Instrument names are +derived mechanically from the counter files (see `InstrumentName`), cumulative +counters pass through a monotonic accumulator, and every data point carries +`domain.id` and `domain.name` attributes. + +| resctrl file | Instrument | Kind | Unit | Exported value | +|---|---|---|---|---| +| `mon_L3_*/llc_occupancy` | `l3.llc.occupancy` | gauge | `By` | raw | +| `mon_L3_*/mbm_local_bytes` | `l3.mbm.local.bytes` | counter | `By` | raw | +| `mon_L3_*/mbm_total_bytes` | `l3.mbm.total.bytes` | counter | `By` | raw | +| `mon_PERF_PKG_*/core_energy` | `perf.core.energy` | counter | `J` | raw | +| `mon_PERF_PKG_*/activity` | `perf.activity` | counter | `farads` | raw nanofarads × 1e-9 | +| other `mon_PERF_PKG_*` files | `perf.` | counter if known, else gauge | none | raw | + +`ReadCounters` always returns the raw kernel value; for `activity` its `Unit` +is `nF`. + +Prometheus names are produced by the consumer's exporter and depend on its +translation strategy: + +| Instrument | `UnderscoreEscapingWithSuffixes` | `NoUTF8EscapingWithSuffixes` | +|---|---|---| +| `l3.llc.occupancy` | `l3_llc_occupancy_bytes` | `l3.llc.occupancy_bytes` | +| `l3.mbm.local.bytes` | `l3_mbm_local_bytes_total` | `l3.mbm.local.bytes_total` | +| `l3.mbm.total.bytes` | `l3_mbm_bytes_total` | `l3.mbm.total.bytes_total` | +| `perf.core.energy` | `perf_core_energy_joules_total` | `perf.core.energy_joules_total` | +| `perf.activity` | `perf_activity_farads_total` | `perf.activity_farads_total` | + +The underscore strategy drops the word `total` from `l3.mbm.total.bytes`. +Older `go.opentelemetry.io/otel/exporters/prometheus` releases choose their +default strategy from the global name validation scheme, so select one +explicitly: + +```go +exp, err := prometheus.New( + prometheus.WithTranslationStrategy(otlptranslator.UnderscoreEscapingWithSuffixes), +) +``` + ## RMID Exhaustion Each mon_group consumes one RMID from a pool with a size that is platform diff --git a/pkg/monitor/monitor.go b/pkg/monitor/monitor.go index 2b678b1..3a7f336 100644 --- a/pkg/monitor/monitor.go +++ b/pkg/monitor/monitor.go @@ -294,7 +294,7 @@ func (m *Manager) EnsureGroup(key, rdtClass string) (*Group, error) { return nil, fmt.Errorf("failed to create mon_group %s: %w", monGroupDir, err) } } else { - log().Info("created mon_group", "key", key, "dir", monGroupDir) + log().Debug("created mon_group", "key", key, "dir", monGroupDir) } // Bump the monotonic generation on every (re)creation so a remove→recreate @@ -363,7 +363,7 @@ func (m *Manager) AssignPID(key string, pid int) error { if err := f.Close(); err != nil { return fmt.Errorf("failed to write pid %d for key %s: %w", pid, key, err) } - log().Info("assigned PID to mon_group", "key", key, "pid", pid) + log().Debug("assigned PID to mon_group", "key", key, "pid", pid) return nil } @@ -485,7 +485,7 @@ func (m *Manager) Remove(key string) error { } delete(m.entries, key) - log().Info("removed mon_group", "key", key, "dir", e.dir) + log().Debug("removed mon_group", "key", key, "dir", e.dir) return nil } diff --git a/pkg/monitor/otel.go b/pkg/monitor/otel.go index 8479d14..8378d0c 100644 --- a/pkg/monitor/otel.go +++ b/pkg/monitor/otel.go @@ -107,8 +107,8 @@ func (r *Registration) Close() error { // mon_L3_00/llc_occupancy → l3.llc.occupancy (unit: By) // mon_L3_00/mbm_local_bytes → l3.mbm.local.bytes (unit: By) // mon_L3_00/mbm_total_bytes → l3.mbm.total.bytes (unit: By) -// mon_PERF_PKG_00/core_energy → perf.core.energy (unit: J) -// mon_PERF_PKG_00/activity → perf.activity (unit: farads) +// mon_PERF_PKG_00/core_energy → perf.core.energy (unit: J) +// mon_PERF_PKG_00/activity → perf.activity (unit: farads; kernel nF × 1e-9) // mon_PERF_PKG_00/c1_res → perf.c1.res // mon_PERF_PKG_00/c6_res → perf.c6.res // mon_PERF_PKG_00/uops_retired → perf.uops.retired @@ -131,7 +131,8 @@ func (r *Registration) Close() error { // L3 instrument names are similar to pkg/rdt's RegisterOpenTelemetryInstruments // but preserve the _bytes counter suffix (e.g. l3.mbm.total.bytes vs // pkg/rdt's l3.mbm.total). This maintains backward compatibility with the -// kernel counter file names as a mechanical derivation. +// kernel counter file names as a mechanical derivation. See InstrumentName for +// how Prometheus exporters render these names. // // Each metric carries a "domain.id" attribute with the numeric instance // (e.g. "00") and a "domain.name" attribute with the full domain directory @@ -157,6 +158,7 @@ func (m *Manager) RegisterOTelInstruments(meter metric.Meter, opts ...OTelOption cfg: cfg, meter: meter, instrs: make(map[string]metric.Observable), + warned: make(map[string]bool), accum: newOTelAccumulator(), } @@ -177,6 +179,7 @@ type otelObserver struct { mu sync.Mutex instrs map[string]metric.Observable // instrName → instrument + warned map[string]bool // undiscovered instrument names already warned about accum *otelAccumulator reg metric.Registration // batch callback registration; nil if none } @@ -187,7 +190,8 @@ type otelObserver struct { // not read — so instruments are registered even when the kernel reports // temporary placeholder values like "Unavailable". func (o *otelObserver) discoverAndRegister() error { - counters, err := discoverCounters(filepath.Join(o.mgr.root, "mon_data")) + monDataPath := filepath.Join(o.mgr.root, "mon_data") + counters, err := discoverCounters(monDataPath) if err != nil { return err } @@ -202,7 +206,8 @@ func (o *otelObserver) discoverAndRegister() error { continue } seen[name] = struct{}{} - instr, err := o.createInstrument(name, metaKind(c.counter), metaUnit(c.counter)) + unit, _ := metaOTel(c.counter) + instr, err := o.createInstrument(name, metaKind(c.counter), unit) if err != nil { return err } @@ -211,6 +216,7 @@ func (o *otelObserver) discoverAndRegister() error { } if len(observables) == 0 { + log().Warn("otel: no resctrl counters discovered; nothing will be exported", "path", monDataPath) return nil } @@ -305,9 +311,17 @@ func (o *otelObserver) observe(ctx context.Context, obs metric.Observer) { instrName := InstrumentName(r.Domain, r.Name) o.mu.Lock() instr := o.instrs[instrName] + warned := o.warned[instrName] + if instr == nil && !warned { + o.warned[instrName] = true + } o.mu.Unlock() if instr == nil { - log().Warn("otel: unknown counter skipped (not discovered at registration)", + logUnknown := log().Debug + if !warned { + logUnknown = log().Warn + } + logUnknown("otel: unknown counter skipped (not discovered at registration)", "instrument", instrName, "domain", r.Domain, "counter", r.Name) continue } @@ -316,6 +330,9 @@ func (o *otelObserver) observe(ctx context.Context, obs metric.Observer) { if r.Kind == Cumulative { val = o.accum.monotonic(key, r.Domain, r.Name, val, g.Gen()) } + // Scale after accumulating so accumulator state stays in kernel units. + _, scale := metaOTel(r.Name) + val *= scale attrs := make([]attribute.KeyValue, 0, len(groupAttrs)+2) attrs = append(attrs, attribute.String("domain.id", DomainInstance(r.Domain))) @@ -343,21 +360,22 @@ func (o *otelObserver) observe(ctx context.Context, obs metric.Observer) { // --- Naming helpers (exported for use by callers building custom export) --- // InstrumentName derives the OTel instrument name from a resctrl domain -// directory name and counter file name. +// directory name and counter file name: the domain's resource prefix followed +// by the counter file name with "_" replaced by ".". The mapping is mechanical +// (the _bytes suffix is kept) so every instrument name identifies its resctrl +// file. +// +// Prometheus names are chosen by the consumer's exporter, not by this package, +// and depend on its translation strategy. For the counter l3.mbm.total.bytes +// (unit By): // -// The counter file name is converted to dot-separated segments and prepended -// with the domain's resource prefix. The _bytes suffix is preserved (not -// stripped) so that the OTel→Prometheus bridge's unit-suffix deduplication -// produces correct names without colliding with the counter _total suffix -// convention. +// UnderscoreEscapingWithSuffixes → l3_mbm_bytes_total +// NoUTF8EscapingWithSuffixes → l3.mbm.total.bytes_total // -// NOTE: This intentionally diverges from pkg/rdt's RegisterOpenTelemetryInstruments -// which uses names like "l3.mbm.total" (stripping _bytes). That approach -// produces incorrect Prometheus names via the OTel bridge: the bridge treats -// the trailing "total" as a counter suffix, yielding "l3_mbm_bytes_total" -// instead of the expected "l3_mbm_total_bytes_total". By preserving _bytes in -// the OTel name, the bridge sees the unit is already present and only appends -// _total for counters, producing the correct final name. +// The underscore strategy removes every "total" word from a counter name +// before appending _total, so pkg/rdt's l3.mbm.total renders the same way. +// Consumers should select a strategy explicitly, e.g. with +// prometheus.WithTranslationStrategy(otlptranslator.UnderscoreEscapingWithSuffixes). // // Examples: // diff --git a/pkg/monitor/reading.go b/pkg/monitor/reading.go index cca0158..36b596c 100644 --- a/pkg/monitor/reading.go +++ b/pkg/monitor/reading.go @@ -46,7 +46,7 @@ type Reading struct { Name string // counter file name, e.g. "llc_occupancy", "core_energy" Value float64 // parsed value (float to cover core_energy/activity) Kind ReadingKind // Gauge or Cumulative - Unit string // UCUM unit where available (e.g. "By", "J"), otherwise descriptive (e.g. "farads") + Unit string // UCUM unit of Value where known (e.g. "By", "J", "nF"), otherwise "" } // readingMeta maps known counter names to their kind and unit. @@ -58,9 +58,9 @@ var readingMeta = map[string]struct { "mbm_total_bytes": {Cumulative, "By"}, "mbm_local_bytes": {Cumulative, "By"}, "core_energy": {Cumulative, "J"}, - // activity accumulates dynamic capacitance; its rate of change is the - // workload's dynamic capacitance (Cdyn) - "activity": {Cumulative, "farads"}, + // activity accumulates dynamic capacitance in nanofarads; its rate of + // change is the workload's dynamic capacitance (Cdyn) + "activity": {Cumulative, "nF"}, "c1_res": {Cumulative, ""}, "c6_res": {Cumulative, ""}, "uops_retired": {Cumulative, ""}, @@ -163,3 +163,22 @@ func metaUnit(name string) string { } return "" } + +// otelExport overrides the exported OTel unit and scale for counters whose raw +// kernel unit is not a base unit. +var otelExport = map[string]struct { + unit string + scale float64 +}{ + // "farads" rather than UCUM "F": otlptranslator has no farad suffix mapping. + "activity": {"farads", 1e-9}, +} + +// metaOTel returns the OTel instrument unit for a counter and the factor that +// converts its raw value into that unit. +func metaOTel(name string) (unit string, scale float64) { + if o, ok := otelExport[name]; ok { + return o.unit, o.scale + } + return metaUnit(name), 1 +} diff --git a/pkg/monitor/reading_test.go b/pkg/monitor/reading_test.go index a0f2953..9a7a3e3 100644 --- a/pkg/monitor/reading_test.go +++ b/pkg/monitor/reading_test.go @@ -50,8 +50,10 @@ func TestReadCounters_MultiDomain(t *testing.T) { // Build a lookup map: domain/name -> value type key struct{ domain, name string } got := make(map[key]float64) + units := make(map[key]string) for _, r := range readings { got[key{r.Domain, r.Name}] = r.Value + units[key{r.Domain, r.Name}] = r.Unit } // L3 domain — integer values parse as float64. @@ -62,6 +64,11 @@ func TestReadCounters_MultiDomain(t *testing.T) { assert.InDelta(t, 54446119.644974, got[key{"mon_PERF_PKG_00", "core_energy"}], 0.001) assert.InDelta(t, 1042.371582, got[key{"mon_PERF_PKG_00", "activity"}], 0.001) + // Readings carry the raw kernel value and unit (activity is nanofarads). + assert.Equal(t, "J", units[key{"mon_PERF_PKG_00", "core_energy"}]) + assert.Equal(t, "nF", units[key{"mon_PERF_PKG_00", "activity"}]) + assert.Equal(t, "By", units[key{"mon_L3_00", "mbm_total_bytes"}]) + // mon_L3_01 has only "Unavailable" — no reading should be emitted for it. _, hasL301 := got[key{"mon_L3_01", "llc_occupancy"}] assert.False(t, hasL301, "non-numeric 'Unavailable' should be skipped") @@ -197,3 +204,16 @@ func TestMetaKind_UnknownDefaultsToGauge(t *testing.T) { // accumulator rather than risk corrupting a real gauge. assert.Equal(t, Gauge, metaKind("some_future_counter")) } + +func TestMetaOTel(t *testing.T) { + // Counters listed in otelExport are exported in its unit and scale. + for name, want := range otelExport { + unit, scale := metaOTel(name) + assert.Equal(t, want.unit, unit, name) + assert.Equal(t, want.scale, scale, name) + } + // Other counters are exported in their raw unit, unscaled. + unit, scale := metaOTel("core_energy") + assert.Equal(t, metaUnit("core_energy"), unit) + assert.Equal(t, 1.0, scale) +}