Skip to content

feat(observability): add Relay client resource metrics - #68978

Merged
jquesnelle merged 15 commits into
NousResearch:mainfrom
afourniernv:feat/hermes-relay-client-dimensions
Aug 5, 2026
Merged

feat(observability): add Relay client resource metrics#68978
jquesnelle merged 15 commits into
NousResearch:mainfrom
afourniernv:feat/hermes-relay-client-dimensions

Conversation

@afourniernv

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds the Category 2 client-resource slice on top of the Relay skill-metrics stack. Shared-metrics packages now include bounded Hermes version, OS family, architecture, and install-method fields captured when each aggregate is recorded.

This is a stacked draft PR on #68883. Until the earlier PRs land, GitHub shows the cumulative stack against main; the client-resource-only diff is available here.

Stack Order

  1. feat(observability): integrate NeMo Relay runtime and shared metrics #67607 - core Relay runtime, task metrics, aggregation, and local export.
  2. feat(observability): report model and provider usage #68881 - bounded model metrics.
  3. feat(observability): aggregate bounded tool metrics #68882 - bounded tool lifecycle and approval metrics.
  4. feat(observability): aggregate bounded skill metrics #68883 - skill lifecycle, provenance, reuse, and reuse-after-patch metrics.
  5. This PR - bounded client OS, architecture, and install-method resources.

Merge and review in this order. This draft is cumulative until its predecessors land on main.

Related Issue

Depends on #68883.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • Classify OS family, CPU architecture, and Hermes install method through closed allowlists, mapping unrecognized values to unknown.
  • Persist the complete client-resource tuple with each SQLite aggregate so resource changes produce separate delta packages instead of combining counts.
  • Add a transactional store migration that preserves existing counters and install identity while assigning unknown to newly introduced dimensions.
  • Require the new bounded fields in the shared-metrics JSON schema and reject invalid resources before persistence.
  • Extend documentation and the real Relay smoke artifact to validate resource output without exporting raw platform strings, hostnames, or paths.

How to Test

  1. Run scripts/run_tests.sh tests/hermes_cli/test_relay_shared_metrics.py -q.
  2. Run scripts/run_tests.sh tests/hermes_cli/test_relay_shared_metrics_runtime.py tests/plugins/test_nemo_relay_plugin.py -q.
  3. Run .venv/bin/python scripts/smoke_nemo_relay_shared_metrics.py with the installed NeMo Relay binding.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS arm64

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

  • Focused store and schema suite: 186 passed.
  • Real Relay runtime and plugin suites: 81 passed.
  • End-to-end Relay smoke: passed with all existing aggregate families and schema-valid resources (macos, arm64, git on the test host); privacy canaries remained absent.

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard provider/nvidia NVIDIA NIM telemetry Touches outbound telemetry, usage attribution, or analytics — needs opt-in gating before merge labels Jul 21, 2026
@afourniernv
afourniernv force-pushed the feat/hermes-relay-client-dimensions branch from a5bf137 to ccfec97 Compare July 22, 2026 14:27
@afourniernv
afourniernv marked this pull request as ready for review July 23, 2026 15:09
@afourniernv
afourniernv requested a review from a team July 23, 2026 15:09
@afourniernv
afourniernv force-pushed the feat/hermes-relay-client-dimensions branch 3 times, most recently from 86bcafe to 034c811 Compare July 29, 2026 01:48
@afourniernv
afourniernv force-pushed the feat/hermes-relay-client-dimensions branch 4 times, most recently from 8587f5c to f118dab Compare July 29, 2026 18:51
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
@afourniernv
afourniernv force-pushed the feat/hermes-relay-client-dimensions branch from f118dab to 5607d09 Compare July 29, 2026 19:21

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the bounded-resource migration and compatibility coverage. I found two correctness gaps in the client-resource slice.

Problems

  • hermes_cli/observability/schemas/hermes.shared_metrics.v1.schema.json:52 adds the new properties but leaves resource.required as only hermes_version (lines 43-45). The accompanying test intentionally preserves that (tests/hermes_cli/test_relay_shared_metrics.py:254-272), so schema validation does not enforce the claimed complete resource tuple.
  • hermes_cli/observability/shared_metrics_subscriber.py:43 passes detect_install_method() directly into the new allowlist. The existing detector returns nix for /nix/store installs (hermes_cli/config.py:475-482), but the PR's allowlist has nixos, not nix, and normalizes unmatched values to unknown (hermes_cli/observability/shared_metrics_contract.py:188-231).

Suggested changes

  • Version the strict resource schema while retaining compatibility validation for already-queued v1 packages, then test that new packages cannot omit any resource field.
  • Normalize nix explicitly or add it to the contract and cover the subscriber path.

Automated hermes-sweeper review.

Comment thread hermes_cli/observability/schemas/hermes.shared_metrics.v1.schema.json Outdated
Comment thread hermes_cli/observability/shared_metrics_subscriber.py
@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 30, 2026
Signed-off-by: Alex Fournier <afournier@nvidia.com>

# Conflicts:
#	hermes_cli/observability/schemas/hermes.shared_metrics.v1.schema.json
#	scripts/smoke_nemo_relay_shared_metrics.py
#	tests/agent/test_skill_commands.py
#	tests/hermes_cli/test_relay_shared_metrics.py
#	tests/hermes_cli/test_relay_shared_metrics_runtime.py
#	tests/tools/test_skill_manager_tool.py
#	tests/tools/test_skill_usage.py
#	tests/tools/test_skills_tool.py
Signed-off-by: Alex Fournier <afournier@nvidia.com>

# Conflicts:
#	docs/observability/relay-shared-metrics.md
#	hermes_cli/observability/shared_metrics.py
#	tests/hermes_cli/test_relay_shared_metrics.py
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>

# Conflicts:
#	tests/tools/test_skills_hub.py
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>

# Conflicts:
#	tests/run_agent/test_run_agent.py
Signed-off-by: Alex Fournier <afournier@nvidia.com>
Signed-off-by: Alex Fournier <afournier@nvidia.com>
@jquesnelle
jquesnelle merged commit edf0a7e into NousResearch:main Aug 5, 2026
38 checks passed
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…lay-client-dimensions

feat(observability): add Relay client resource metrics
33hodl pushed a commit to 33hodl/hermes-agent that referenced this pull request Aug 12, 2026
…lay-client-dimensions

feat(observability): add Relay client resource metrics
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have provider/nvidia NVIDIA NIM sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data telemetry Touches outbound telemetry, usage attribution, or analytics — needs opt-in gating before merge type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants