Skip to content

Commit 66d5527

Browse files
JoshParkSJclaude
andcommitted
fix(governance): address review feedback
- Drop the None check on get_current_span — OTel returns INVALID_SPAN, never None, so the span-context validity check is sufficient. - Reword the behavior-matrix bullet: the no-op triggers on any valid current span context, including a remotely propagated one, not only a host-opened span. - Snapshot and restore the same private tracer-provider global in the test helper rather than mixing it with get_tracer_provider(). - Assert the agent span's parent off the exported ReadableSpan; the live Span protocol has no parent attribute, which failed mypy in CI. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 482e4c0 commit 66d5527

2 files changed

Lines changed: 25 additions & 20 deletions

File tree

‎src/uipath/runtime/governance/runtime.py‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,10 +79,11 @@ def _governance_root_span(agent_name: str, runtime_id: str) -> Iterator[None]:
7979
8080
Behavior matrix:
8181
82-
- **OTel installed + host opened a parent span**: no-op — the
83-
host's span already supplies the ``trace_id``, and a span
84-
inserted here is dropped by host-side export filters, orphaning
85-
everything below it.
82+
- **OTel installed + a valid span context is already current**
83+
(host-opened or remotely propagated): no-op — that context
84+
already supplies the ``trace_id``, and a span inserted here is
85+
dropped by host-side export filters, orphaning everything
86+
below it.
8687
- **OTel installed + no parent span**: this becomes the root
8788
span of a fresh trace; everything below it shares the new
8889
``trace_id``.
@@ -101,8 +102,7 @@ def _governance_root_span(agent_name: str, runtime_id: str) -> Iterator[None]:
101102
yield
102103
return
103104

104-
current = trace.get_current_span()
105-
if current is not None and current.get_span_context().is_valid:
105+
if trace.get_current_span().get_span_context().is_valid:
106106
yield
107107
return
108108

‎tests/test_governance_runtime.py‎

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -650,38 +650,42 @@ def _recording_tracer_provider() -> Iterator[Any]:
650650
provider = TracerProvider()
651651
provider.add_span_processor(SimpleSpanProcessor(exporter))
652652

653-
original = trace.get_tracer_provider()
654-
trace._TRACER_PROVIDER = provider # type: ignore[attr-defined]
653+
original = trace._TRACER_PROVIDER
654+
trace._TRACER_PROVIDER = provider
655655
try:
656656
yield exporter
657657
finally:
658-
trace._TRACER_PROVIDER = original # type: ignore[attr-defined]
658+
trace._TRACER_PROVIDER = original
659659

660660

661661
def _exported_names(exporter: Any) -> set[str]:
662662
"""Return the names of every span the exporter received."""
663663
return {span.name for span in exporter.get_finished_spans()}
664664

665665

666+
def _exported_span(exporter: Any, name: str) -> Any:
667+
"""Return the single exported span with ``name``."""
668+
spans = [span for span in exporter.get_finished_spans() if span.name == name]
669+
assert len(spans) == 1
670+
return spans[0]
671+
672+
666673
async def test_execute_opens_no_span_under_a_host_span() -> None:
667674
"""Under a host span the wrapper stays out of the tree entirely."""
668675
from opentelemetry import trace
669676

670677
class _SpanOpeningDelegate(_StubDelegate):
671-
"""Records the parent the agent's own span is given."""
672-
673-
def __init__(self) -> None:
674-
super().__init__()
675-
self.agent_span_parent: Any = None
678+
"""Opens a span the way a framework adapter would."""
676679

677680
async def execute(self, input: Any = None, options: Any = None) -> Any:
678681
tracer = trace.get_tracer("test.agent")
679-
with tracer.start_as_current_span("agent run") as agent_span:
680-
self.agent_span_parent = agent_span.parent
682+
with tracer.start_as_current_span("agent run"):
683+
pass
681684
return await super().execute(input, options)
682685

683-
delegate = _SpanOpeningDelegate()
684-
runtime = UiPathGovernedRuntime(delegate, PolicyIndex(), EnforcementMode.AUDIT)
686+
runtime = UiPathGovernedRuntime(
687+
_SpanOpeningDelegate(), PolicyIndex(), EnforcementMode.AUDIT
688+
)
685689

686690
with _recording_tracer_provider() as exporter:
687691
host_tracer = trace.get_tracer("test.host")
@@ -690,10 +694,11 @@ async def execute(self, input: Any = None, options: Any = None) -> Any:
690694
assert await runtime.execute({"x": 1}) == "result"
691695

692696
exported = _exported_names(exporter)
697+
agent_span = _exported_span(exporter, "agent run")
693698

694699
assert "uipath.governance.run" not in exported
695-
assert delegate.agent_span_parent is not None
696-
assert delegate.agent_span_parent.span_id == host_span_id
700+
assert agent_span.parent is not None
701+
assert agent_span.parent.span_id == host_span_id
697702

698703

699704
async def test_execute_opens_a_root_span_with_no_host_span() -> None:

0 commit comments

Comments
 (0)