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/docs/performance.md b/docs/performance.md index 8195656..96e0797 100644 --- a/docs/performance.md +++ b/docs/performance.md @@ -63,7 +63,7 @@ scheduler outlier block a change. | Cold no-op invocation, including startup and dispatch | 2,000 ms | 2,000 ms | 4,000 ms | 4,000 ms | | Base-cli lifecycle increment over Click warm dispatch | 5 ms | 5 ms | 15 ms | 15 ms | | Warm invocation and non-persistence feature scenarios | 50 ms | 50 ms | 100 ms | 100 ms | -| File-persistence-enabled scenario | 50 ms | 50 ms | 250 ms | 50 ms | +| File-persistence-enabled scenario | 125 ms | 125 ms | 250 ms | 50 ms | An initial 31-sample local calibration on macOS (Python 3.14.6, Apple Silicon) measured approximately 101 ms for base-cli cold import, 0.56 ms for warm @@ -76,6 +76,16 @@ budget instead of weakening other warm-scenario gates. These measurements are CI calibration evidence, not adoption claims or release comparisons; review subsequent retained artifacts before tightening platform budgets. +October 2026 hosted recalibration separates sustained persistence cost from +filesystem tails on Unix/macOS: median must remain at most **50 ms** and p95 +at most **125 ms**. The previous 50 ms p95 cap repeatedly rejected otherwise +unchanged runtime code, including the validation-only PR. Observed pairs were +14.66/118.04 ms (Unix median/p95) and 24.93/61.93 and 26.12/87.37 ms (macOS). +Evidence: [Unix run](https://github.com/basefoundry/base-cli/actions/runs/37048785893) +and [macOS validation-only run](https://github.com/basefoundry/base-cli/actions/runs/37052368353). +A sustained slowdown over 50 ms still fails; p95 over 125 ms also fails. +Windows, WSL, parser, import, and non-persistence limits are unchanged. + Each report is versioned as `base-cli.benchmark` schema version 1 and contains the package version, source revision, UTC timestamp, platform profile, Python version/ABI, OS release, architecture, CPU count, sample count, medians, p95, 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/scripts/benchmark_runtime.py b/scripts/benchmark_runtime.py index 24f3e78..f5ade65 100755 --- a/scripts/benchmark_runtime.py +++ b/scripts/benchmark_runtime.py @@ -48,8 +48,8 @@ "wsl": 100.0, } PERSISTENCE_ENABLED_P95_BUDGETS_MS = { - "unix": 50.0, - "macos": 50.0, + "unix": 125.0, + "macos": 125.0, "windows": 250.0, "wsl": 50.0, } @@ -331,6 +331,11 @@ def _check_results(results: dict[str, FrameworkMetrics]) -> list[str]: feature_budget = _feature_budget_for_platform(name, BENCHMARK_PLATFORM) if p95 is not None and p95 > feature_budget: failures.append(f"base-cli {name} p95 exceeded {feature_budget:.0f} ms") + if BENCHMARK_PLATFORM in {"unix", "macos"} and isinstance(features, dict): + persistence = features.get("persistence_enabled_ms", {}) + median = persistence.get("median") if isinstance(persistence, dict) else None + if not isinstance(median, (int, float)) or not 0 <= median <= 50.0: + failures.append("base-cli persistence_enabled_ms median is missing, invalid, or exceeded 50 ms") return failures diff --git a/tests/test_benchmark_runtime.py b/tests/test_benchmark_runtime.py index 8528e5e..7518e06 100644 --- a/tests/test_benchmark_runtime.py +++ b/tests/test_benchmark_runtime.py @@ -125,6 +125,18 @@ def test_windows_persistence_budget_rejects_material_regressions(self) -> None: self.assertTrue(any("persistence_enabled_ms p95 exceeded 250 ms" in failure for failure in failures)) + def test_persistence_budget_separates_sustained_cost_from_filesystem_tails(self) -> None: + for profile in ("unix", "macos"): + for median, p95, fails in ((26.0, 118.0, False), (51.0, 60.0, True), (26.0, 126.0, True)): + with self.subTest(profile=profile, median=median, p95=p95): + metrics = self._complete_results() + sample = self._summary(p95) + sample["median"] = median + metrics["base-cli"]["features"]["persistence_enabled_ms"] = sample + with mock.patch.object(benchmark_runtime, "BENCHMARK_PLATFORM", profile): + failures = benchmark_runtime._check_results(metrics) + self.assertEqual(any("persistence_enabled_ms" in failure for failure in failures), fails) + def test_github_summary_separates_lifecycle_overhead_from_parser(self) -> None: metrics = self._complete_results(lifecycle_p95=4.0, click_p95=1.5) report = { 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(