Skip to content

Commit 0f35aae

Browse files
committed
Merge remote-tracking branch 'origin/main' into ci/393-20261003-ci-ruff-check-and-mypy-do-not-cover-the-compatibility-consum
2 parents 990cb66 + 30bcb1f commit 0f35aae

7 files changed

Lines changed: 176 additions & 5 deletions

File tree

‎docs/integrations.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,13 @@ paths, and secrets are never attached. A missing API package, invalid provider,
5353
or failing exporter is logged at debug level and treated as a no-op; it cannot
5454
change the command's exit status or cleanup behavior.
5555

56+
The span status is `OK` for a successful invocation and `ERROR` for every
57+
non-success outcome. Unexpected exceptions are recorded on failed spans;
58+
successful `SystemExit(0)` and Click exit-control flow are not recorded as
59+
exceptions. Interrupts and other non-success outcomes are therefore visible as
60+
errors while retaining the existing `base_cli.outcome` attribute for detailed
61+
dashboard filtering.
62+
5663
## Log timestamp environment variable
5764

5865
Set `BASE_CLI_LOG_UTC=1` to make the default text formatter use UTC

‎lib/python/base_cli/_app_core.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,6 +1038,7 @@ def wrapper(**kwargs: Any) -> Any:
10381038
recorder: RunRecorder | None = None
10391039
telemetry_session: TelemetrySession | None = None
10401040
outcome = outcome_from_exit_code(ExitCode.SUCCESS)
1041+
exception: BaseException | None = None
10411042
invocation_argv: list[str] = []
10421043
redaction_plan = self._redaction_plan
10431044
if redaction_plan is None:
@@ -1074,6 +1075,7 @@ def wrapper(**kwargs: Any) -> Any:
10741075
outcome = outcome_from_exit_code(exit_code)
10751076
return result
10761077
except BaseException as exc:
1078+
exception = exc
10771079
if context is not None:
10781080
outcome = outcome_from_exception(click, exc)
10791081
_record_lifecycle_diagnostic(context, outcome)
@@ -1111,6 +1113,7 @@ def wrapper(**kwargs: Any) -> Any:
11111113
context,
11121114
outcome,
11131115
ended_monotonic_ns=ended_monotonic_ns,
1116+
exception=exception,
11141117
)
11151118
_finish_run_recorder(
11161119
recorder,

‎lib/python/base_cli/_attach.py‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ def __init__(
7171
self.context: Context[Any, Any, Any] | None = None
7272
self.invocation: _AttachedInvocation | None = None
7373
self.telemetry_session: TelemetrySession | None = None
74+
self.exception: BaseException | None = None
7475
self.context_token: Any = None
7576
self.invocation_token: Any = None
7677
self.original_click_exit: Callable[..., Any] | None = None
@@ -164,11 +165,16 @@ def record_result(self, _result: Any) -> None:
164165
state.attached_completion = True
165166

166167
def record_exception(self, exc: BaseException) -> None:
168+
outcome = outcome_from_exception(self.click, exc)
169+
# Click represents a successful ``Context.exit(0)`` as an exception so
170+
# it can unwind the context stack. It is control flow, not a failed
171+
# command, and must not be exported as a span exception.
172+
self.exception = None if str(outcome.status) == "ok" else exc
167173
state = _INVOCATION_STATE.get()
168174
if state is not None and state.owner_app is self.attachment.app:
169175
state.attached_completion = False
170176
if self.context is not None:
171-
self.outcome = outcome_from_exception(self.click, exc)
177+
self.outcome = outcome
172178
_record_lifecycle_diagnostic(self.context, self.outcome)
173179

174180
def __exit__(
@@ -239,6 +245,7 @@ def _finalize(self) -> None:
239245
context,
240246
self.outcome,
241247
ended_monotonic_ns=ended_monotonic_ns,
248+
exception=self.exception,
242249
)
243250
try:
244251
context.cleanup()

‎lib/python/base_cli/_lifecycle_install.py‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,10 @@ def _capture_lifecycle_option(
9393
) -> Any:
9494
source = click_context.get_parameter_source(parameter.name)
9595
captures = click_context.meta.setdefault(_LIFECYCLE_CAPTURE_META_KEY, {})
96-
context_values = captures.setdefault(id(click_context), {})
96+
# Keep the context object itself as the key. ``id(context)`` values can be
97+
# reused after Click releases a context, which could associate a later
98+
# invocation with stale lifecycle values.
99+
context_values = captures.setdefault(click_context, {})
97100
context_values[key] = _RawLifecycleValue(
98101
value=value,
99102
source=source,
@@ -537,11 +540,15 @@ def _resolve_lifecycle_values(
537540
_LIFECYCLE_RESOLUTION_META_KEY,
538541
{},
539542
)
543+
# These maps live in Click's invocation-shared metadata and retain one
544+
# context-keyed entry per context until the root invocation closes. That
545+
# bounded, per-invocation retention is deliberate: it prevents id reuse
546+
# without retaining state across invocations.
540547
parent = getattr(click_context, "parent", None)
541-
parent_resolution = resolution_map.get(id(parent)) if parent is not None else None
548+
parent_resolution = resolution_map.get(parent) if parent is not None else None
542549
raw = dict(parent_resolution.raw) if isinstance(parent_resolution, _LifecycleResolution) else {}
543550
captures = click_context.meta.get(_LIFECYCLE_CAPTURE_META_KEY, {})
544-
context_captures = captures.get(id(click_context), {})
551+
context_captures = captures.get(click_context, {})
545552
depth = _context_depth(click_context)
546553

547554
for key, binding in bindings.items():
@@ -576,7 +583,7 @@ def _resolve_lifecycle_values(
576583
values=_normalize_lifecycle_values(click, raw),
577584
raw=raw,
578585
)
579-
resolution_map[id(click_context)] = resolution
586+
resolution_map[click_context] = resolution
580587
click_context.meta[LIFECYCLE_META_KEY] = resolution.values
581588
return resolution
582589

‎lib/python/base_cli/integrations.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ def finish_telemetry(
130130
outcome: Any,
131131
*,
132132
ended_monotonic_ns: int | None = None,
133+
exception: BaseException | None = None,
133134
) -> None:
134135
"""Finish a lifecycle span without allowing exporters to affect teardown."""
135136

@@ -148,6 +149,9 @@ def finish_telemetry(
148149
}
149150
for key, value in attributes.items():
150151
_safe_span_call(session.span, "set_attribute", key, value)
152+
if exception is not None and str(getattr(outcome, "status", "error")) != "ok":
153+
_safe_span_call(session.span, "record_exception", exception)
154+
_set_span_status(session.span, outcome)
151155
_safe_span_call(
152156
session.span,
153157
"add_event",
@@ -173,6 +177,19 @@ def _start_attributes(context: Any) -> dict[str, Any]:
173177
}
174178

175179

180+
def _set_span_status(span: Any, outcome: Any) -> None:
181+
"""Set an OpenTelemetry status while remaining compatible with test spans."""
182+
183+
is_success = str(getattr(outcome, "status", "error")) == "ok"
184+
try:
185+
from opentelemetry.trace import Status, StatusCode
186+
187+
status: Any = Status(StatusCode.OK if is_success else StatusCode.ERROR)
188+
except Exception: # pragma: no cover - optional dependency boundary
189+
status = "ok" if is_success else "error"
190+
_safe_span_call(span, "set_status", status)
191+
192+
176193
def _safe_span_call(span: Any, method: str, *args: Any, **kwargs: Any) -> None:
177194
try:
178195
callback = getattr(span, method, None)

‎tests/test_click_tree_attachment.py‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
from unittest import mock
1313

1414
import base_cli
15+
from base_cli import _lifecycle_install
1516
from base_cli._runtime import RuntimeDirectoryError
1617
from base_cli.testing import invoke
1718

@@ -63,6 +64,105 @@ def count_cleanup() -> None:
6364

6465
@unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed")
6566
class ClickTreeAttachmentTests(unittest.TestCase):
67+
def test_lifecycle_metadata_is_keyed_by_context_identity(self) -> None:
68+
class FakeContext:
69+
def __init__(self) -> None:
70+
self.meta: dict[object, Any] = {}
71+
72+
def get_parameter_source(self, name: str) -> None:
73+
del name
74+
return None
75+
76+
first = FakeContext()
77+
second = FakeContext()
78+
parameter = type("Parameter", (), {"name": "environment"})()
79+
80+
_lifecycle_install._capture_lifecycle_option(first, parameter, "first", key="environment")
81+
_lifecycle_install._capture_lifecycle_option(second, parameter, "second", key="environment")
82+
83+
captures = first.meta[next(key for key in first.meta if key is not None)]
84+
self.assertIn(first, captures)
85+
self.assertNotIn(id(first), captures)
86+
self.assertEqual(captures[first]["environment"].value, "first")
87+
second_captures = second.meta[next(key for key in second.meta if key is not None)]
88+
self.assertIn(second, second_captures)
89+
self.assertEqual(second_captures[second]["environment"].value, "second")
90+
91+
def test_chained_click_contexts_keep_lifecycle_values_through_teardown(self) -> None:
92+
import click
93+
94+
observed: list[tuple[str, str | None, int]] = []
95+
closed: list[str] = []
96+
97+
def capture_environment(click_context: Any, parameter: Any, value: Any) -> Any:
98+
return _lifecycle_install._capture_lifecycle_option(
99+
click_context,
100+
parameter,
101+
value,
102+
key="environment",
103+
)
104+
105+
@click.group(name="pipeline", chain=True)
106+
def pipeline() -> None:
107+
pass
108+
109+
@pipeline.command(name="first")
110+
@click.option("--environment", callback=capture_environment)
111+
def first(environment: str | None) -> None:
112+
del environment
113+
click_context = click.get_current_context()
114+
captures = click_context.meta[_lifecycle_install._LIFECYCLE_CAPTURE_META_KEY]
115+
observed.append(
116+
(
117+
"first",
118+
captures[click_context]["environment"].value,
119+
len(captures),
120+
)
121+
)
122+
click_context.call_on_close(lambda: closed.append("first"))
123+
124+
@pipeline.command(name="second")
125+
@click.option("--environment", callback=capture_environment)
126+
def second(environment: str | None) -> None:
127+
del environment
128+
click_context = click.get_current_context()
129+
captures = click_context.meta[_lifecycle_install._LIFECYCLE_CAPTURE_META_KEY]
130+
observed.append(
131+
(
132+
"second",
133+
captures[click_context]["environment"].value,
134+
len(captures),
135+
)
136+
)
137+
self.assertEqual(closed, ["first"])
138+
click_context.call_on_close(lambda: closed.append("second"))
139+
140+
app = base_cli.App(name="pipeline", log_to_file=False)
141+
app.attach(pipeline)
142+
143+
with tempfile.TemporaryDirectory() as tmpdir:
144+
result = invoke(
145+
app,
146+
[
147+
"first",
148+
"--environment",
149+
"first-env",
150+
"second",
151+
"--environment",
152+
"second-env",
153+
],
154+
home=Path(tmpdir),
155+
)
156+
157+
self.assertEqual(result.exit_code, 0, result.output)
158+
self.assertEqual(
159+
[(name, value) for name, value, _capture_count in observed],
160+
[("first", "first-env"), ("second", "second-env")],
161+
)
162+
self.assertGreaterEqual(observed[0][2], 2)
163+
self.assertEqual(observed[0][2], observed[1][2])
164+
self.assertEqual(closed, ["first", "second"])
165+
66166
def test_prebuilt_single_command_preserves_click_contract_and_lifecycle(self) -> None:
67167
import click
68168

‎tests/test_integrations.py‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ def __init__(self) -> None:
1717
self.attributes: dict[str, object] = {}
1818
self.events: list[tuple[str, dict[str, object]]] = []
1919
self.ended = False
20+
self.status: object | None = None
21+
self.exceptions: list[BaseException] = []
2022

2123
def set_attribute(self, key: str, value: object) -> None:
2224
self.attributes[key] = value
@@ -27,6 +29,12 @@ def add_event(self, name: str, *, attributes: dict[str, object]) -> None:
2729
def end(self) -> None:
2830
self.ended = True
2931

32+
def set_status(self, status: object) -> None:
33+
self.status = status
34+
35+
def record_exception(self, exception: BaseException) -> None:
36+
self.exceptions.append(exception)
37+
3038

3139
class _Tracer:
3240
def __init__(self) -> None:
@@ -118,6 +126,28 @@ def main(ctx: base_cli.Context) -> None:
118126
["base_cli.run.started", "base_cli.run.finished"],
119127
)
120128
self.assertIn("base_cli.duration_ms", tracer.span.attributes)
129+
self.assertEqual(tracer.span.exceptions, [])
130+
self.assertIsNotNone(tracer.span.status)
131+
132+
def test_telemetry_marks_failures_and_records_exception(self) -> None:
133+
tracer = _Tracer()
134+
app = base_cli.App(
135+
name="telemetry-error",
136+
log_to_file=False,
137+
telemetry=base_cli.TelemetryOptions(tracer=tracer),
138+
)
139+
140+
@app.command()
141+
def main(ctx: base_cli.Context) -> None:
142+
del ctx
143+
raise RuntimeError("boom")
144+
145+
with tempfile.TemporaryDirectory() as tmpdir:
146+
result = invoke(app, [], home=Path(tmpdir))
147+
148+
self.assertNotEqual(result.exit_code, 0)
149+
self.assertEqual(len(tracer.span.exceptions), 1)
150+
self.assertIsNotNone(tracer.span.status)
121151

122152
def test_missing_or_broken_telemetry_never_changes_completion(self) -> None:
123153
app = base_cli.App(

0 commit comments

Comments
 (0)