Repository navigation
feat(OMN-13847): per-hook wall-clock latency instrumentation (emit side) - #1834
Conversation
…t side) The OMN-13244 measurement baseline had no way to measure per-hook wall-clock latency. Add a reusable emit-side timing primitive that wraps a hook body with a monotonic start/stop timer and emits a structured hook.executed event (hook_name, event, duration_ms, blocked, correlation_id, session_id) via the existing emit-daemon socket path (emit_client_wrapper.emit_event) -- canonical event bus only, no bespoke REST. Inert when hooks are disabled: nothing runs at import, timing only happens while a hook body actually runs; does not re-enable any hook. Fast-follow (OMN-13847): register hook.executed in the daemon topic registry (omnimarket) + mirrored omniclaude EVENT_REGISTRY/TopicBase as one coordinated cross-repo change (event_registry_drift gate), then land the hook_executions projection (migration+consumer, mirroring skill_executions) and the /api/hook-health/summary endpoint hook_health_alert expects.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 35 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
| Verdict | Meaning | Blocks merge? |
|---|---|---|
passed |
No critical/error findings | No |
blocked |
CRITICAL or ERROR findings found | Yes |
degraded |
All models unavailable (infra) | No (pilot) |
Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
2 participants
Summary
Closes OMN-13847.
The OMN-13244 measurement baseline had no way to measure per-hook wall-clock latency (gap identified in the hook-usefulness analysis). This PR lands the foundational emit-side piece: a reusable monotonic-timer instrumentation primitive that wraps a hook body and emits a structured
hook.executedtelemetry event so hook usefulness becomes data-driven (p50/p95 latency per hook) instead of guessed.What changed
plugins/onex/hooks/lib/hook_latency_instrumentation.py(new)instrument_hook(hook_name, event, ...)context manager —time.monotonic()start/stop timer; emits on exit (from afinallyblock, so duration is recorded even when the hook raises orsys.exit()s). The yielded handle lets the hook body flagblocked = True.instrumented_hook(...)decorator form for a hook entrypoint callable.build_hook_execution_payload(...)/emit_hook_execution(...)— payload carrieshook_name,event(PreToolUse/PostToolUse/…),duration_ms,blocked,correlation_id, andsession_id(injected byemit_eventfromCLAUDE_CODE_SESSION_IDwhen not supplied).emit_client_wrapper.emit_event) — canonical event bus only, no bespoke REST/HTTP. Fire-and-forget: never raises, never blocks the hook.tests/unit/hooks/lib/test_hook_latency_instrumentation.py(new) — 14 unit tests assertingduration_msis emitted,blockedpropagates, timing is recorded even when the body raises, the emitter is fail-open (never raises), and the decorator preserves metadata.Design constraints honored
skill.started/hook.health.error.Fast-follow (tracked on OMN-13847)
The daemon-side routing + projection is deliberately deferred because it is a coordinated cross-repo change (the
event_registry_driftgate requires the omniclaudeEVENT_REGISTRY/TopicBasesurfaces and the omnimarket daemon topic registry to move together, so registeringhook.executedin only one repo would red-gate the other):hook.executedinomnimarket/node_emit_daemon/registries/topics.yaml+ the mirrored omniclaudeSUPPORTED_EVENT_TYPES/EVENT_REGISTRY/TopicBasein one coordinated PR.hook_executionstime-series projection (migration + consumer, mirroringskill_executions) keyed for p50/p95 latency per hook./api/hook-health/summaryendpoint thehook_health_alertskill already expects.Until (1) lands,
emit_eventtreatshook.executedas an unregistered type and drops it client-side (graceful no-op), so this emit-side primitive is safe to ship ahead of the projection.dod_evidence
jonah/omn-13847-hook-latency-instrumentation(based onorigin/dev)tests/unit/hooks/lib/test_hook_latency_instrumentation.py— 14 passed.uv run ruff format— 2 files left unchangeduv run ruff check <both files>— All checks passeduv run mypy plugins/onex/hooks/lib/hook_latency_instrumentation.py— Success: no issues founduv run pytest tests/unit/hooks/lib/test_hook_latency_instrumentation.py— 14 passedpytest --collect-only— 10,846 tests collected, no import/syntax errors introducedpre-commit(pre-commit + pre-push stages) — passing on the changed files under the canonical 3.13 interpreter (CLAUDE.md rule Add Claude Code GitHub Workflow #11)Evidence-Source: OCC#3487
Evidence-Ticket: OMN-13847