Skip to content

feat(rust-plugins): rate/delta DSL primitive with persistent state - #6419

Open
julienmathis wants to merge 6 commits into
developfrom
feat/dsl-rate-delta
Open

feat(rust-plugins): rate/delta DSL primitive with persistent state#6419
julienmathis wants to merge 6 commits into
developfrom
feat/dsl-rate-delta

Conversation

@julienmathis

@julienmathis julienmathis commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Stacked on #6418 (tracing — state_read/state_write spans, reuses the SnmpConfig chain). SNMP counters (ifInOctets, ...) are monotonically increasing values; turning them into per-second rates requires the value and timestamp of the previous run. Adds a "rate": true field on a collect entry:

{ "name": "if", "oid": "1.3.6.1.2.1.2.2.1", "query": "Walk",
  "labels": { "1.2": "descr", "1.10": "in", "1.16": "out" },
  "rate": true }

Every numeric value of the entry becomes a per-second rate computed against the previous run; string columns (labels) are left untouched and vectors stay aligned. See examples/new-traffic-rate.json.

Design

  • Identity = full OID, never the table position — a new interface appearing between runs cannot shift its neighbors' rates.
  • State files (new src/state.rs module): mode 0600 + atomic write (temp file + rename + fsync) — never world-readable, never half-written. A read failure (missing/corrupt file) is never fatal (fresh start); a write failure IS fatal (a stale reference would silently produce wrong rates forever).
  • 32-bit counter wraparound corrected; a 64-bit decrease or a missing previous instance is treated as a reset and yields one aligned 0.0 sample rather than desynchronizing the vector.
  • First run: OK: Buffer creation, exit 0 (Perl parity).
  • New --statefile-dir CLI option (Perl parity, default /var/lib/centreon/centplugins).
  • Instrumented with state_read/state_write spans (visible via PLUGIN_LOG=debug or --trace-file, see feat(rust-plugins): tracing instrumentation — spans, span durations, Perfetto export #6418).

SnmpResult gains a samples: Vec<(item_key, full_oid, value)> field, populated only when a collect entry asks for rates (capture_samples threaded through the walk/get functions and process_response), so apply_rate can rebuild each numeric vector in original push order while keying state persistence by OID.

Test plan

  • cargo build --release / cargo test: 84 passed, 0 failed (9 new: wraparound, reset, dt≤0, state roundtrip + 0600 assert + corrupt-file recovery, two-run rate integration, new-instance alignment, sample capture)
  • --check-format validates the new example
  • Manual: state files created under a temp statefile-dir are mode 0600 and survive a corrupt-content injection without crashing (exercised by the state.rs unit tests directly against the real filesystem)

@julienmathis
fix(rust-plugins): SNMP protocol hardening — validate agent responses…
f926e08 
…, bound walks, timeouts & retries

The engine blindly trusted the agent it queried, which is precisely the
untrusted party of the dialogue:

- Agent errors are now detected: a response with error-status != 0
  produces a typed error naming the RFC 3416 status (e.g. "noSuchName").
- Request/response correlation: every request carries a fresh
  request-id, and received datagrams are validated (request-id,
  community echo, PDU type) before being accepted — unrelated
  datagrams (e.g. a late retransmission of a previous request) are
  discarded instead of being consumed as the current answer.
- Bounded walks: OIDs must be strictly increasing during a walk (the
  classic snmpwalk "OID not increasing" guard) and a single walk may
  collect at most 100 000 values — both protect against a buggy or
  malicious agent looping the walk or streaming endless data.
- Configurable timeouts and retries via three new CLI options:
  --timeout (per-attempt receive timeout, default 1s), --snmp-retries
  (default 2), and --collect-timeout (global budget for the whole
  collection, default 50s, so the plugin exits with a clean UNKNOWN
  before centengine's own kill timeout).
- The UDP receive buffer was 1024 bytes, silently truncating large
  bulk responses; raised to 65535.

Connection parameters (target, community, timeouts, retries) are now
threaded through a single SnmpConfig instead of loose string
arguments, and the two walk loops share one implementation.

Test plan:
- cargo build --release succeeds
- cargo test: 75 passed, 0 failed (10 new: RFC 3416 names, response
  validation, non-increasing OID, varbind cap, expired deadline)
- Manual check against an unroutable address: default settings exit
  UNKNOWN after retries*timeout with the attempt count in the message;
  --collect-timeout correctly preempts a longer per-attempt timeout
@julienmathis
julienmathis requested a review from a team as a code owner September 4, 2026 13:22
@julienmathis
julienmathis requested review from sdepassio and removed request for a team September 4, 2026 13:22
@julienmathis julienmathis mentioned this pull request Sep 4, 2026
Open
2 tasks
@julienmathis
fix(rust-plugins): update rust-error.robot to the hardened connection…
ae3cb96 
…-failure message

The protocol-hardening commit changed the no-connection error message
from "Could not connect to X is the hostname..." to "No valid SNMP
response from X after N attempts (timeout Ts per attempt)" (clearer:
it names the retry budget actually exhausted), but never updated the
Robot fixture that pins it. Both cgs-no-connection cases have been
failing CI since this branch was opened.
@julienmathis
julienmathis requested a review from a team as a code owner September 8, 2026 12:14
@julienmathis
julienmathis requested review from Evan-Adam and removed request for a team September 8, 2026 12:14
@julienmathis
fix(rust-plugins): use a plain GetRequest for scalar SNMP gets
4dd020c 
snmp_bulk_get built a GetBulkRequest PDU for every "get" query, relying on
a workaround that stripped a trailing .0 and depended on GetNext landing
exactly one leaf ahead. GetBulk (even with max-repetitions=1) can never
perform an exact match, so this silently returned the wrong value for any
non-.0-suffixed OID, such as a specific row of a multi-row table.

Every currently shipped definition only queries .0-suffixed scalars, so
the bug was latent. Switch to a real GetRequest and drop the trailing-zero
workaround, which is no longer needed.
@julienmathis
perf(rust-plugins): measured engine optimizations — 5x fewer round-tr…
52cdb76 
…ips on table walks

Every change here is backed by a measurement (synthetic 10 000-row
ifTable, 50 000 varbinds, snmpsim), not assumed:

- GetBulk max-repetitions raised from a hardcoded 10 to a default of
  50 (Perl parity), configurable globally via --maxrepetitions and
  per-query via "max-repetitions" in the JSON collect entry. On the
  synthetic table this cuts requests from 5001 to 1001 (a walk of N
  rows now costs ceil(N/max_repetitions) round-trips instead of
  ceil(N/10)).
- Nagios thresholds are now parsed once per metric (parse_threshold)
  instead of once per vector element — a 10 000-row table with
  warning+critical thresholds no longer reparses the same two strings
  10 000 times, and threshold syntax errors now name the metric and
  field.
- The UDP socket and receive buffer are allocated once per walk
  instead of once per request (open_socket / shared buffer threaded
  through send_request), removing a bind+connect+alloc per round trip
  on large walks.
- eval_str's macro-matching regex is compiled once (OnceLock) instead
  of on every template evaluation.

Test plan:
- cargo build --release succeeds
- cargo test: 75 passed, 0 failed
- Manual check against an unroutable address: retry/timeout behavior
  unchanged (UNKNOWN + exit 3 after the configured attempts)
@julienmathis
feat(rust-plugins): tracing instrumentation — spans, span durations, …
567d684 
…Perfetto export

Replaces log/env_logger with tracing/tracing-subscriber across the
crate, in three activation modes:

- default (production): silent on stderr, zero measurable overhead.
- PLUGIN_LOG=debug (or any level, mirroring the historical env var):
  structured logs plus **span durations on close**
  (`walk{oid=...} close time.busy=5.65ms`) — which stage was slow on
  this host, from one env var. PLUGIN_LOG's default moves from `info`
  to `warn`: a plugin must be silent on stderr nominally, and per-value
  info-level formatting was hot-path waste (deliberate behavior change).
- --trace-file <path>: full Chrome-trace recording, loadable in
  Perfetto — validated manually (JSON parses, span hierarchy present).

Spans: check -> collect -> walk{oid}/get -> request{id} per attempt,
metric{name}, aggregation{name}, output.

No OTLP/collector export: a process running at high check-per-minute
rates must not ship spans over the network; out of scope by design.

snmp_plugin() now returns the exit code instead of calling
process::exit mid-flight: destructors run on every path, so the trace
flush guard (held for the whole function) always writes the file
before the process exits. main() calls process::exit(code) once, at
the top level. The CLI argument loop is flattened to
`while let Some(arg) = parser.next()? { ... }`, so a lexopt parse
error now goes through the same UNKNOWN + exit(3) path as every other
error instead of a bare `Error: {err}` + exit(1).

Test plan:
- cargo build --release / cargo test: 75 passed, 0 failed
- Manual: nominal run has 0 bytes on stderr; PLUGIN_LOG=debug shows
  nested check/metric/aggregation spans with time.busy/time.idle on
  close; --trace-file produces a valid Chrome-trace JSON; unknown-flag,
  missing-JSON, --help and unroutable-target retry behavior unchanged
@julienmathis
feat(rust-plugins): rate/delta DSL primitive with persistent state
dff4638 
SNMP counters (ifInOctets, ...) are monotonically increasing values;
turning them into per-second rates requires the value and timestamp of
the previous run. Adds a "rate": true field on a collect entry:

  { "name": "if", "oid": "1.3.6.1.2.1.2.2.1", "query": "Walk",
    "labels": { "1.2": "descr", "1.10": "in", "1.16": "out" },
    "rate": true }

Every numeric value of the entry becomes a per-second rate computed
against the previous run; string columns (labels) are left untouched
and vectors stay aligned.

Design:
- Identity = full OID, never the table position — a new interface
  appearing between runs cannot shift its neighbors' rates.
- State files (new src/state.rs module): mode 0600 + atomic write
  (temp file + rename + fsync) — never world-readable, never
  half-written. A read failure (missing/corrupt file) is never fatal
  (fresh start); a write failure IS fatal (a stale reference would
  silently produce wrong rates forever).
- 32-bit counter wraparound corrected; a 64-bit decrease or a missing
  previous instance is treated as a reset and yields one aligned 0.0
  sample rather than desynchronizing the vector.
- First run: "OK: Buffer creation", exit 0 (Perl parity).
- New --statefile-dir CLI option (Perl parity, default
  /var/lib/centreon/centplugins).
- Instrumented with state_read/state_write spans (visible via
  PLUGIN_LOG=debug or --trace-file, see the tracing PR).

SnmpResult gains a `samples: Vec<(item_key, full_oid, value)>` field,
populated only when a collect entry asks for rates (capture_samples
threaded through the walk/get functions and process_response), so
apply_rate can rebuild each numeric vector in original push order
while keying state persistence by OID.

Test plan:
- cargo build --release / cargo test: 84 passed, 0 failed (9 new:
  wraparound, reset, dt<=0, state roundtrip + 0600 assert + corrupt-file
  recovery, two-run rate integration, new-instance alignment, sample
  capture)
- --check-format validates the new example (examples/new-traffic-rate.json)
- Manual: state files created under a temp statefile-dir are mode 0600
  and survive a corrupt-content injection without crashing (exercised
  by the state.rs unit tests directly against the real filesystem)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sdepassio sdepassio Awaiting requested review from sdepassio sdepassio is a code owner automatically assigned from centreon/owners-connectors
@Evan-Adam Evan-Adam Awaiting requested review from Evan-Adam Evan-Adam is a code owner automatically assigned from centreon/owners-robot-e2e

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

skip-workflow-Generic Plugins skip-workflow-plugins

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@julienmathis