feat(governance): audit pipeline — manager, console + traces sinks#122
feat(governance): audit pipeline — manager, console + traces sinks#122aditik0303 wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a governance audit pipeline with pluggable sinks, introducing OpenTelemetry trace spans for Orchestrator Traces and a developer-oriented console sink, plus tests that pin expected filtering and severity/status semantics.
Changes:
- Introduces
AuditManager+AuditSinkframework with async queueing, circuit-breaker behavior, and default sink configuration. - Adds built-in sinks:
TracesAuditSink(OTel spans) andConsoleAuditSink(stderr output), plus a simple sink factory. - Adds tests covering console formatting/filtering, sink re-registration counter reset semantics, and trace severity/status behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_traces_severity.py | Verifies span verbosityLevel and error-status semantics for rule vs hook spans across enforcement modes. |
| tests/test_audit_register_sink.py | Tests AuditManager.register_sink behavior around circuit-breaker counters and duplicate registration. |
| tests/test_audit_console.py | Tests ConsoleAuditSink filtering and per-event formatting to stderr. |
| src/uipath/runtime/governance/audit/traces.py | Implements OpenTelemetry span emission for hook summaries and rule evaluations. |
| src/uipath/runtime/governance/audit/factory.py | Adds a name-based sink factory (traces, console) with env-driven console verbosity. |
| src/uipath/runtime/governance/audit/console.py | Implements stderr console sink with verbose/non-verbose filtering. |
| src/uipath/runtime/governance/audit/base.py | Adds core audit model, sink base class, and AuditManager with async worker/queue + circuit-breaker. |
| src/uipath/runtime/governance/audit/init.py | Exposes audit framework public API and documents built-in sinks and env vars. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
91309f8 to
418fd8f
Compare
42e1f29 to
7111d27
Compare
418fd8f to
14bd3cc
Compare
7111d27 to
2db5f2d
Compare
14bd3cc to
acfa5b5
Compare
bd19932 to
add4e90
Compare
d500f99 to
7cdc791
Compare
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…eption) so flush() can't hang; use Status() object for set_status + update test Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-platform - traces.py: _get_uipath_trace_id / _get_uipath_context read trace/org/ tenant/folder/job ids from the environment via runtime-local ENV_* constants instead of importing UiPathConfig. - test_traces_severity: import the reset helper from tests._helpers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ntract traces
- privatize audit module (audit/ → _audit/) and drop re-export aggregator
- remove ConsoleAuditSink + 3 audit env vars (UIPATH_AUDIT_SYNC,
UIPATH_AUDIT_VERBOSE, UIPATH_GOVERNANCE_CONSOLE_LOG)
- traces sink drops redundant env reads (uipath OTel exporter already
stamps org/tenant/folder/job/trace ids onto outgoing UiPathSpan)
- adopt unified trace contract: uipath_governance.* namespace, snake_case
keys, UPPER_SNAKE enum values; rename rule_id→policy_id; add
evaluator_result + mode + version; derive action_applied from
(matched, configured action, mode) — single source of truth for both
the emitted attributes and the verbosityLevel/Status decision
- mode wire values AUDIT/ENFORCE (ENFORCE branch present but inert
until policy enforcement ships)
- version stamped from importlib.metadata("uipath-runtime")
- thread-safe lazy init in get_audit_manager() via double-checked locking
- drop unused stats property + counters
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses radu's recurring concerns (PR #121 patterns) applied to the audit pipeline; resolves the post-rebase ImportError that traces.py would have hit after PR #121 deleted governance/config.py. Architectural - AuditManager is no longer a process-singleton. _audit_manager / get_audit_manager / reset_audit_manager / _configure_default_sinks are deleted. Each GovernanceRuntime constructs its own manager. uipath eval parallel runtimes no longer share a worker thread or sink list. - Constructor auto-registers the platform-mandated `traces` sink. Tests pass `register_default_sinks=False` for bare-manager fixtures. Mode on event - emit_rule_evaluation / emit_hook_summary / emit_session_start / emit_session_end now require enforcement_mode: EnforcementMode. - traces.py drops `from uipath.runtime.governance.config import get_enforcement_mode` (that module is gone post-rebase) and instead reads mode from event.data via _resolve_mode(). - Inlined _mode_to_spec → mode.value.upper(); MODE_AUDIT/MODE_ENFORCE constants removed. Production-readiness fixes - Bounded atexit: replaced per-instance atexit.register(self.method) with a process-level handler walking weakref.WeakSet(AuditManager). N managers → 1 atexit slot, no strong ref pinning disposed managers. - Fork-rebuild safety: _ensure_alive_after_fork double-checks _pid under _sinks_lock so two threads in a fresh-fork child can't both rebuild queue/worker concurrently. - Removed dead `if TYPE_CHECKING: pass`. Tests - Deleted test_audit_manager_singleton.py (singleton it pinned no longer exists). - test_audit_register_sink uses register_default_sinks=False so assertions about registered sinks see only what the test put there. - test_traces_severity carries mode on the event; new test_two_events_carry_independent_modes pins cross-runtime isolation. - New test_audit_manager_lifecycle: 6 tests covering single atexit registration, weakref GC, no-double-close, fork-rebuild lock (8-thread barrier race), same-PID fast path. 211 passed, ruff/mypy/bandit clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
7cdc791 to
4da27ce
Compare
…from wiring Addresses radu's recurring PR #121 patterns applied to the guardrail compensation slice. Resolves the post-PR-#121 ImportError in the test file (it referenced the deleted ``uipath.runtime.governance.config`` / ``tests._helpers.reset_enforcement_mode``). Architectural — match the AuditManager / PolicyLoader shape - New GuardrailCompensator class. Each GovernanceRuntime instance gets one — owns its own ThreadPoolExecutor, BoundedSemaphore, and provider. uipath eval parallel runtimes no longer share workers, queue slots, or saturation state. - Module globals _pool / _inflight / _INFLIGHT_CAP / @atexit.register decorator removed. Process cleanup via a weakref.WeakSet of live compensators + one process-level atexit hook (same pattern PR #122 introduced for AuditManager): N runtimes → 1 atexit slot, no strong ref pinning disposed compensators. - close() is an instance method, idempotent, logs at debug on failure. - The free submit_compensation function is gone — callers use compensator.submit(...). Boundary — env reads move to the wiring layer - _resolve_trace_id signature changed to (supplied, fallback). It no longer reads UIPATH_TRACE_ID. The runtime layer is now env-free for this code path. - GovernanceRuntime accepts a trace_id: str | None constructor arg and exposes it via the .trace_id property. The wiring layer (uipath CLI) reads UIPATH_TRACE_ID and passes the value in; the evaluator slice forwards it into GuardrailCompensator(provider, trace_id=...). - GuardrailCompensator accepts trace_id at construction; it becomes the authoritative source. Per-submit trace_id is a per-call fallback. Polish - Replaced bare except Exception: pass in _resolve_trace_id with a logger.debug (bandit B110 cleared on this file). - Removed ENV_TRACE_ID constant + the os import that backed it. Tests - Full rewrite of test_guardrail_compensation to drop deleted imports (config, reset_enforcement_mode), use GuardrailCompensator(provider), and mirror AuditManager's lifecycle test set (one atexit registration, weakref GC, idempotent close, cross-instance isolation, semaphore release on provider error). - New test_resolve_trace_id_does_not_read_env pins the boundary rule: even with UIPATH_TRACE_ID set, the runtime layer ignores it. - New test_compensator_trace_id_overrides_caller_supplied_value pins the construction-supplied value winning over per-submit. - New test_governance_runtime_stashes_trace_id + test_governance_runtime_default_trace_id_is_none cover the new GovernanceRuntime kwarg + property. 238 passed, ruff/mypy clean; bandit clean on the touched files (one pre-existing B101 in _yaml_to_index.py is unchanged and out of scope). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…orts Closes radu's recurring boundary objection for the evaluator slice and makes the post-rebase stack actually import. The evaluator was the last place where everything PR #121-#123 instance-scoped collapsed back to process globals. Architectural - GovernanceEvaluator gains constructor injection: GovernanceEvaluator(policy_index, *, enforcement_mode=AUDIT, audit_manager=None, compensator=None) - Drop get_audit_manager() / get_enforcement_mode() / submit_compensation free-function lookups. The evaluator now consults zero process-globals on the hot path. - mode property is read-only (drop the setter); no two-writer race between the loader and evaluator. - audit_manager=None and compensator=None short-circuit cleanly so tests + minimal wirings work without injecting every dep. - Drop unused is_enforce_mode() public method (dead code; no caller in src/ or tests/). Post-rebase plumbing - _dispatch_compensation uses self._compensator.submit(...) instead of the deleted free function; reads r.validator (Pydantic attribute) instead of the old r["validator"] TypedDict access. - _emit_audit passes policy_id (PR #122 trace-contract field, was rule_id) and enforcement_mode=mode enum (PR #122 required arg). - Import EnforcementMode from uipath.core.governance (governance.config deleted in PR #121); import AuditManager from _audit.base (audit/ is _audit/ post-PR-#122). native/__init__.py - Drop the four module-level loader-function re-exports (get_policy_index / load_policy_index / prefetch_policy_index / reset_policy_index) — all deleted in PR #121's PolicyLoader refactor. - Export PolicyLoader instead. Tests - test_evaluator: full rewrite. Drop deleted-import paths (tests._helpers.reset_enforcement_mode, governance.config). Replace the global-manager fixture with a per-test AuditManager that uses register_default_sinks=False + a capturing sink. Every GovernanceEvaluator() call routes through a _build_evaluator helper with explicit mode + manager. New test_no_audit_manager_short_circuits replaces the previous test that mocked the global to raise. - test_evaluator_operators: drop the autouse mode-isolating fixture (no globals to isolate); DISABLED-mode test passes enforcement_mode=EnforcementMode.DISABLED via constructor. - test_guardrail_compensation: rebase-conflict resolution dropped the stale incoming-side imports (Action/LifecycleHook, backend_client, unguarded GovernanceEvaluator) since none of them are referenced in the rest of the file. 357 passed, 1 skipped (pre-existing wrapper skip). Ruff clean. Mypy clean (11 source files). Bandit shows only the pre-existing B101 in _yaml_to_index.py (out of scope). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…Runtime Closes architecture-review §2.1 + §2.2 — the UiPathWrappedRuntimeFactory bolted governance onto the generic runtime-factory registry (apply_wrappers=True turned every registered factory into a different type, breaking isinstance checks), and the second GovernanceRuntime in governance/wrapper.py reached into delegate._agent_definition / framework-specific private attrs through a 10-level walk to install framework-blind callbacks. Both patterns the doc unambiguously says to delete. Composition belongs in the host's decorator chain, FF-gated, where UiPathResumableRuntime already wraps the framework runtime; this PR's wrapper machinery was an end-run around that. Deletions - src/uipath/runtime/governance/wrapper.py (1002 LOC) — the second GovernanceRuntime with _AGENT_ATTRS / _replace_agent_in_delegate / model-context-var introspection. - src/uipath/runtime/wrapper.py (55 LOC) — the lazy-import dispatch shim that called the deleted governance_wrapper. - tests/test_dispose_isolation.py, tests/test_wrapper.py, tests/test_wrapper_internals.py (~650 LOC combined) — entire test suites for the deleted modules. Updates - src/uipath/runtime/registry.py — UiPathWrappedRuntimeFactory class and the apply_wrappers kwarg removed from get(). The registry returns the registered factory unchanged; cross-cutting concerns (governance, audit, …) are composed by the host into the decorator chain, not auto-applied here. - src/uipath/runtime/__init__.py — drop GOVERNANCE_FEATURE_FLAG / apply_governance_wrapper exports. - tests/test_registry.py — strip every apply_wrappers=False kwarg (the kwarg is gone) and drop the wrapping-behaviour section + its fixtures. Conflict resolution The rebase onto #125's tip replayed the upstream e186f5f commit (a cosmetic helper-import touch) into three test files that my PR #122/#123/#124 refactors had already rewritten end-to-end. HEAD-side resolution kept the refactored form in test_evaluator.py, test_evaluator_operators.py, test_guardrail_compensation.py — the incoming side referenced symbols (governance.audit, governance.config, tests._helpers.reset_enforcement_mode) that the post-rebase stack no longer ships. Verification - Monorepo grep for UiPathWrappedRuntimeFactory, apply_wrappers, apply_governance_wrapper, governance_wrapper, and the deleted module import paths: zero hits. - ruff clean, mypy clean (45 source files), 357 passed + 1 skipped. Net diff on top of #125's tip: −2005 / +38 LOC = −1967 net. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| span_name = f"governance.{hook.lower()}" | ||
|
|
||
| # Use the current OTel context if one is active; otherwise start a | ||
| # root span. A previous version fabricated a random parent |
There was a problem hiding this comment.
this comment references old implementation. remove it.
| # | ||
| # 2. **No strong reference to the manager.** ``WeakSet`` lets a disposed | ||
| # manager get garbage-collected; if it's already gone by exit time, | ||
| # we just skip it. The per-instance atexit hook held the manager |
There was a problem hiding this comment.
this comment references old implementation. remove it.
…ents
Two of the comments in this PR described how the implementation USED
to behave (per-instance atexit hooks, fabricated remote-parent
span_ids) rather than what it does now. Reviewer flagged both and
asked for them to be removed.
While in here, generalized a handful of doc strings that referenced
specific upstream/downstream names (``PolicyLoader``, "evaluator")
inside this module's docs. The audit module shouldn't be coupled to
those names — what it actually relies on is "the emitter passes its
own per-instance EnforcementMode on each event". Generalizing reads
the same on this branch and stays correct as the stack lands.
Reviewer-flagged
- ``_audit/base.py`` module-level comment block: dropped the
"per-instance atexit hook held the manager alive until process
exit" historical aside; kept the forward-looking statement of what
the WeakSet enables.
- ``_audit/traces.py`` ``_emit_hook_span``: dropped the "A previous
version fabricated a random parent span_id…" paragraph; kept the
current-behavior description.
Consistency mirrors
- ``_audit/traces.py`` ``_emit_rule_span`` cross-referenced the
removed paragraph ("rely on the current OTel context rather than
fabricating a remote-parent span_id"); cleaned up to match.
Generalizations
- ``_audit/base.py`` ``emit_rule_evaluation`` / ``emit_session_start``
docstrings: ``"per-loader mode"`` / ``"instance-scoped loaders"``
→ ``"emitter's per-instance mode"`` / ``"each emitter
(instance-scoped)"``.
- ``_audit/traces.py`` ``_resolve_mode`` docstring +
``_emit_hook_span`` / ``_emit_rule_span`` inline comments:
``"loader's per-instance mode"`` / ``"evaluator stamps it from the
per-loader instance"`` / ``"per-loader instance"`` → variants of
``"each emitter stamps its own per-instance mode"``.
- ``tests/test_traces_severity.py`` module docstring: explicit
``PolicyLoader.enforcement_mode`` reference → generic
``EnforcementMode``.
ruff + mypy clean (8 source files). 211 passed. No behavior change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| Double-checked under ``_atexit_lock`` so two concurrent first-time | ||
| constructions don't both register the process atexit handler. | ||
| """ | ||
| global _atexit_registered |
There was a problem hiding this comment.
Could we avoid the global _atexit_registered mutation by wrapping this cleanup state in a small private registry object?
class _AuditManagerCleanupRegistry:
def __init__(self) -> None:
self.live_managers: weakref.WeakSet[AuditManager] = weakref.WeakSet()
self.atexit_registered = False
self.lock = threading.Lock()
def register(self, manager: AuditManager) -> None:
self.live_managers.add(manager)
if self.atexit_registered:
return
with self.lock:
if not self.atexit_registered:
atexit.register(self.cleanup)
self.atexit_registered = True
def cleanup(self) -> None:
for manager in list(self.live_managers):
try:
manager.flush(timeout=2.0)
manager.close()
except Exception as exc: # noqa: BLE001
logger.debug("Audit manager process cleanup error: %s", exc)| """ | ||
|
|
||
| event_type: str | ||
| trace_id: str = "" |
There was a problem hiding this comment.
shouldn t we have a parent span id as well?
Stacked PR 3/7 — part of splitting
feat/governance-coreinto reviewable slices. Base:feat/governance-policy-loading. One logical slice (branch is cumulative so CI is green). Merge in order #1 → #7 and delete each branch on merge so the next PR auto-retargets ontofeat/agentic-governance.feat/governance-corekept untouched as backup.