diff --git a/docs/integrations.md b/docs/integrations.md index 0bfb68c..000de8d 100644 --- a/docs/integrations.md +++ b/docs/integrations.md @@ -52,3 +52,10 @@ outcome, exit code, and duration. Raw argv, configuration values, filesystem paths, and secrets are never attached. A missing API package, invalid provider, or failing exporter is logged at debug level and treated as a no-op; it cannot change the command's exit status or cleanup behavior. + +The span status is `OK` for a successful invocation and `ERROR` for every +non-success outcome. Unexpected exceptions are recorded on failed spans; +successful `SystemExit(0)` and Click exit-control flow are not recorded as +exceptions. Interrupts and other non-success outcomes are therefore visible as +errors while retaining the existing `base_cli.outcome` attribute for detailed +dashboard filtering. diff --git a/lib/python/base_cli/_app_core.py b/lib/python/base_cli/_app_core.py index 323f2d2..e5c2986 100644 --- a/lib/python/base_cli/_app_core.py +++ b/lib/python/base_cli/_app_core.py @@ -1038,6 +1038,7 @@ def wrapper(**kwargs: Any) -> Any: recorder: RunRecorder | None = None telemetry_session: TelemetrySession | None = None outcome = outcome_from_exit_code(ExitCode.SUCCESS) + exception: BaseException | None = None invocation_argv: list[str] = [] redaction_plan = self._redaction_plan if redaction_plan is None: @@ -1074,6 +1075,7 @@ def wrapper(**kwargs: Any) -> Any: outcome = outcome_from_exit_code(exit_code) return result except BaseException as exc: + exception = exc if context is not None: outcome = outcome_from_exception(click, exc) _record_lifecycle_diagnostic(context, outcome) @@ -1111,6 +1113,7 @@ def wrapper(**kwargs: Any) -> Any: context, outcome, ended_monotonic_ns=ended_monotonic_ns, + exception=exception, ) _finish_run_recorder( recorder, diff --git a/lib/python/base_cli/_attach.py b/lib/python/base_cli/_attach.py index cf55e6c..0733412 100644 --- a/lib/python/base_cli/_attach.py +++ b/lib/python/base_cli/_attach.py @@ -71,6 +71,7 @@ def __init__( self.context: Context[Any, Any, Any] | None = None self.invocation: _AttachedInvocation | None = None self.telemetry_session: TelemetrySession | None = None + self.exception: BaseException | None = None self.context_token: Any = None self.invocation_token: Any = None self.original_click_exit: Callable[..., Any] | None = None @@ -164,11 +165,16 @@ def record_result(self, _result: Any) -> None: state.attached_completion = True def record_exception(self, exc: BaseException) -> None: + outcome = outcome_from_exception(self.click, exc) + # Click represents a successful ``Context.exit(0)`` as an exception so + # it can unwind the context stack. It is control flow, not a failed + # command, and must not be exported as a span exception. + self.exception = None if str(outcome.status) == "ok" else exc state = _INVOCATION_STATE.get() if state is not None and state.owner_app is self.attachment.app: state.attached_completion = False if self.context is not None: - self.outcome = outcome_from_exception(self.click, exc) + self.outcome = outcome _record_lifecycle_diagnostic(self.context, self.outcome) def __exit__( @@ -239,6 +245,7 @@ def _finalize(self) -> None: context, self.outcome, ended_monotonic_ns=ended_monotonic_ns, + exception=self.exception, ) try: context.cleanup() diff --git a/lib/python/base_cli/integrations.py b/lib/python/base_cli/integrations.py index bfd9d1d..b920f05 100644 --- a/lib/python/base_cli/integrations.py +++ b/lib/python/base_cli/integrations.py @@ -130,6 +130,7 @@ def finish_telemetry( outcome: Any, *, ended_monotonic_ns: int | None = None, + exception: BaseException | None = None, ) -> None: """Finish a lifecycle span without allowing exporters to affect teardown.""" @@ -148,6 +149,9 @@ def finish_telemetry( } for key, value in attributes.items(): _safe_span_call(session.span, "set_attribute", key, value) + if exception is not None and str(getattr(outcome, "status", "error")) != "ok": + _safe_span_call(session.span, "record_exception", exception) + _set_span_status(session.span, outcome) _safe_span_call( session.span, "add_event", @@ -173,6 +177,19 @@ def _start_attributes(context: Any) -> dict[str, Any]: } +def _set_span_status(span: Any, outcome: Any) -> None: + """Set an OpenTelemetry status while remaining compatible with test spans.""" + + is_success = str(getattr(outcome, "status", "error")) == "ok" + try: + from opentelemetry.trace import Status, StatusCode + + status: Any = Status(StatusCode.OK if is_success else StatusCode.ERROR) + except Exception: # pragma: no cover - optional dependency boundary + status = "ok" if is_success else "error" + _safe_span_call(span, "set_status", status) + + def _safe_span_call(span: Any, method: str, *args: Any, **kwargs: Any) -> None: try: callback = getattr(span, method, None) diff --git a/tests/test_integrations.py b/tests/test_integrations.py index c8e4355..70529bf 100644 --- a/tests/test_integrations.py +++ b/tests/test_integrations.py @@ -17,6 +17,8 @@ def __init__(self) -> None: self.attributes: dict[str, object] = {} self.events: list[tuple[str, dict[str, object]]] = [] self.ended = False + self.status: object | None = None + self.exceptions: list[BaseException] = [] def set_attribute(self, key: str, value: object) -> None: self.attributes[key] = value @@ -27,6 +29,12 @@ def add_event(self, name: str, *, attributes: dict[str, object]) -> None: def end(self) -> None: self.ended = True + def set_status(self, status: object) -> None: + self.status = status + + def record_exception(self, exception: BaseException) -> None: + self.exceptions.append(exception) + class _Tracer: def __init__(self) -> None: @@ -118,6 +126,28 @@ def main(ctx: base_cli.Context) -> None: ["base_cli.run.started", "base_cli.run.finished"], ) self.assertIn("base_cli.duration_ms", tracer.span.attributes) + self.assertEqual(tracer.span.exceptions, []) + self.assertIsNotNone(tracer.span.status) + + def test_telemetry_marks_failures_and_records_exception(self) -> None: + tracer = _Tracer() + app = base_cli.App( + name="telemetry-error", + log_to_file=False, + telemetry=base_cli.TelemetryOptions(tracer=tracer), + ) + + @app.command() + def main(ctx: base_cli.Context) -> None: + del ctx + raise RuntimeError("boom") + + with tempfile.TemporaryDirectory() as tmpdir: + result = invoke(app, [], home=Path(tmpdir)) + + self.assertNotEqual(result.exit_code, 0) + self.assertEqual(len(tracer.span.exceptions), 1) + self.assertIsNotNone(tracer.span.status) def test_missing_or_broken_telemetry_never_changes_completion(self) -> None: app = base_cli.App(