feat(dhcp): add reconcile observability metrics and structured logs

[dmathur0]

Summary

Adds enough state to diagnose future DHCP upgrade failures without relying on container restart counts alone — the signal that was missing during the incident. Instruments the sync/refresh loops with 3 metrics and a structured log-state taxonomy, with strict secret redaction.

Metrics (exposed via existing /metrics)

Name	Type	Labels
nv_config_manager_dhcp_sync_failures_total	Counter	operation, ip_version
nv_config_manager_dhcp_config_hash_mismatches_total	Counter	ip_version
nv_config_manager_dhcp_last_successful_sync_timestamp_seconds	Gauge	ip_version

• operation values: redis_read, config_generation, config_set, hash_get, config_test, postgres — so operators can tell where it broke.
• The gauge is a Unix timestamp; alert on its age (time() - gauge).

Log-state taxonomy (sync_state field)

waiting-for-initial-redis-config, in-sync, drift-detected (logs desired_hash+running_hash), applying (logs desired_hash), dependency-error (logs operation+escaped error).

Redaction is strict: only truncated SHA-256 hashes are logged — never credentials or full config-get responses; structured field values are newline-escaped to prevent log forging.

Changes

• metrics.py — metric definitions + SyncOperation/SyncState taxonomy classes
• cli.py — instrumented _sync_kea_configuration_async and _refresh_kea_configuration_async
• test_cli.py — 8 new unit tests

Test plan

• failure-counter labels; config-set failure path
• mismatch counter + hash logging; gauge updates on success
• waiting-state log; secret/full-config redaction
• 8/8 new tests pass; 104/105 dhcp suite (1 pre-existing Docker-only failure)
• ruff + mypy clean

Part 4 of 5 from the DHCP config-sync recovery proposal, opened independently against main.

Merge note

The hash_get operation label and the drift/verified-sync points carry MERGE HOOK (Part 1/2) comments — they slot into #168 (Part 1's get_config_hash/expected_hash) and #171 (Part 2's last-successful-reconciliation state) without reimplementing that logic. cli.py (_sync_kea_configuration_async) overlaps with #168/#170/#171, so whichever merges later needs a small rebase.

Summary by CodeRabbit

• New Features

  • Added DHCP synchronization metrics for failures, configuration drift, and the last successful sync.
  • Added structured reconciliation status logging, including waiting, in-sync, drift detected, applying, and dependency-error states.
  • Configuration drift is now detected using secure fingerprints, with running and desired states clearly distinguished.

• Bug Fixes

  • Improved failure tracking for Redis, Kea, PostgreSQL, and configuration operations.
  • Enhanced log safety by preventing secrets, full configurations, and forged multiline messages from being exposed.

[coderabbitai]

📝 Walkthrough

Walkthrough

The DHCP reconcile loop now uses hashed configuration drift detection, structured sync-state logging, labeled dependency-failure metrics, verified-sync timestamps, and log-safety protections. New tests cover failure accounting, drift handling, waiting states, successful syncs, and secret redaction.

Changes

DHCP reconciliation observability

Layer / File(s)	Summary
Reconciliation metric contracts src/nv_config_manager/dhcp/metrics.py	Adds canonical sync operation/state labels and Prometheus metrics for failures, configuration mismatches, and last successful sync timestamps.
Tracked reconciliation flow src/nv_config_manager/dhcp/cli.py	Adds tracked Redis, PostgreSQL, configuration generation, testing, and Kea configuration operations; compares configuration hashes and logs reconciliation states.
Reconciliation observability validation src/tests/dhcp/test_cli.py	Tests labeled failures, drift metrics and logs, verified-sync timestamps, initial-config waiting, newline escaping, and secret-safe logging.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Redis
  participant DHCPReconcileLoop
  participant Kea
  participant Prometheus
  Redis->>DHCPReconcileLoop: load desired configuration
  DHCPReconcileLoop->>DHCPReconcileLoop: compare desired and running hashes
  DHCPReconcileLoop->>Prometheus: record state or mismatch
  DHCPReconcileLoop->>Kea: apply configuration when drift is detected
  Kea-->>DHCPReconcileLoop: return application result
  DHCPReconcileLoop->>Prometheus: update successful sync timestamp

Loading

Possibly related PRs

• NVIDIA/nv-config-manager#168: Updates the DHCP reconcile loop with KEA SHA-256 hash comparison and configuration reapplication.

Suggested reviewers: zsblevins

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the PR’s main change: DHCP reconcile observability metrics and structured logging.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch davesh/dhcp-sync-observability

---

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

[coderabbitai]

🧹 Nitpick comments (1)

src/nv_config_manager/dhcp/cli.py (1)

435-443: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Avoid computing the config fingerprint on every "still in sync" tick when it's discarded.

_config_fingerprint(previous_config) (JSON-serializing + SHA-256 hashing the whole config) runs on every loop iteration here, but _mark_verified_sync immediately discards running_hash whenever log is falsy (if not log: return, metrics.py/cli.py _mark_verified_sync). Since log=debug and debug is False in production, this is wasted work on every tick of an indefinitely-running loop.

⚡ Proposed fix: only compute the fingerprint when it will actually be logged

                     else:
                         # Still in sync: keep the last-verified-sync gauge fresh
                         # so its age only climbs when reconciliation stalls.
                         _mark_verified_sync(
                             ip_version,
-                            _config_fingerprint(previous_config),
+                            _config_fingerprint(previous_config) if debug else "",
                             recovered=False,
                             log=debug,
                         )

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/nv_config_manager/dhcp/cli.py` around lines 435 - 443, In the
still-in-sync branch, update the call to _mark_verified_sync so
_config_fingerprint(previous_config) is computed only when debug is enabled;
otherwise pass the appropriate absent-value accepted by _mark_verified_sync.
Preserve the existing recovered=False behavior and keep fingerprint logging
unchanged when debug is true.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/nv_config_manager/dhcp/cli.py`:
- Around line 435-443: In the still-in-sync branch, update the call to
_mark_verified_sync so _config_fingerprint(previous_config) is computed only
when debug is enabled; otherwise pass the appropriate absent-value accepted by
_mark_verified_sync. Preserve the existing recovered=False behavior and keep
fingerprint logging unchanged when debug is true.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b2259520-cfef-46cd-8f45-8a2fb74fb285

📥 Commits

Reviewing files that changed from the base of the PR and between 784510c and f59faeb.

📒 Files selected for processing (3)

• src/nv_config_manager/dhcp/cli.py
• src/nv_config_manager/dhcp/metrics.py
• src/tests/dhcp/test_cli.py