feat(observability): add Relay setup and first-use metrics - #69437
feat(observability): add Relay setup and first-use metrics#69437afourniernv wants to merge 11 commits into
Conversation
42cb040 to
927b26b
Compare
|
One scope clarification from review: this PR measures setup only for profiles that already have shared metrics enabled. Fresh first-time setup is not captured because Hermes does not currently ask for telemetry consent during onboarding, and this path deliberately creates no identity or persisted telemetry before consent. I am keeping that privacy boundary in this PR. Until onboarding consent is decided, these metrics should be described as consented setup activity rather than complete first-time setup coverage. I opened #69526 to track the product decision and implementation. If first-run coverage is required, the proposed shape is to hold the setup-start fact only in memory, emit it after an explicit opt-in, and discard it on opt-out or exit before consent. |
ff58384 to
ab53c0b
Compare
5053e87 to
03eda10
Compare
Signed-off-by: Alex Fournier <afournier@nvidia.com>
03eda10 to
b955f15
Compare
Signed-off-by: Alex Fournier <afournier@nvidia.com>
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the detailed consent boundary and the focused setup instrumentation.
Problems
hermes_cli/observability/relay_shared_metrics.py:1271returns whenenabled()becomes false. The setup wrapper opens a synthetic scope before executing the wizard (hermes_cli/setup.py:2737), and the telemetry section can turn that setting off. That path therefore skipsrecord_setup_finished(), which is also the only path toclose_owned_session(), leaving the opened Relay session unclosed.
Suggested changes
- Make scope closure unconditional for a non-null attempt; gate only terminal metric emission on the final consent state. Add a test for enabled → disabled during
hermes setup telemetrythat asserts no terminal metric is persisted and the owned session is closed.
Automated hermes-sweeper review.
Signed-off-by: Alex Fournier <afournier@nvidia.com> # Conflicts: # hermes_cli/observability/schemas/hermes.shared_metrics.v1.schema.json # hermes_cli/observability/shared_metrics.py # hermes_cli/observability/shared_metrics_contract.py # hermes_cli/observability/shared_metrics_subscriber.py # scripts/smoke_nemo_relay_shared_metrics.py # tests/hermes_cli/test_relay_shared_metrics_runtime.py # tests/hermes_cli/test_setup_model_provider.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: # docs/observability/relay-shared-metrics.md # scripts/smoke_nemo_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>
|
@teknium1 This is caught up with current |
Signed-off-by: Alex Fournier <afournier@nvidia.com>
|
Caught up with current Local validation after the merge:
Fresh CI is running on the updated head. |
|
@teknium1 Could you take another look when CI completes? The branch is caught up and the prior cleanup finding remains covered. |
What does this PR do?
Adds consented setup and first-use metrics to Hermes' existing Relay shared-metrics pipeline. Setup runs for profiles that already opted in emit bounded start and terminal lifecycle marks, while the first usable runtime boundary and first successful task are persisted as one-time profile milestones.
All prerequisite observability PRs have merged. The diff against
mainnow contains only the setup and first-use changes in this PR.Predecessors
All six predecessors are merged.
Related Issue
Builds on merged #69416. Relates to #69526, which tracks the separate product decision about telemetry consent during first-run onboarding.
Type of Change
Changes Made
hermes.setup.startedandhermes.setup.finishedRelay marks for opted-in setup commands, using only bounded mode, outcome, and failure-stage values.hermes.client.first_usableonce at the first consented session or task runtime boundary.hermes.client.first_successful_tasktransactionally with the first accepted successful task terminal.main.How to Test
./.venv/bin/pytest -q tests/hermes_cli/test_relay_shared_metrics.py tests/hermes_cli/test_relay_shared_metrics_runtime.py tests/hermes_cli/test_setup_shared_metrics.py tests/hermes_cli/test_setup_blank_slate.py tests/hermes_cli/test_setup_model_provider.py../.venv/bin/pytest -q tests/hermes_cli -k setup../.venv/bin/python scripts/smoke_nemo_relay_shared_metrics.pywith the installed NeMo Relay 0.7.1 binding.uv lock --check.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AScreenshots / Logs
Validated after merging current
main:tests/hermes_cli -k setupselection: 130 passed, 1 skipped, with no failures.uv lock --checkpassed.