diff --git a/CHANGELOG.md b/CHANGELOG.md index 74d64f5a..ffb23913 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). The - **Adaptive call-level retry: per-attempt request override** (proposal 0095, llm-provider §7.1, spec v0.91.0). The LLM-completion call-level retry loop gains an opt-in per-attempt request override. A new `LlmRetryConfig` (the llm-provider-scoped superset of the generic `RetryConfig`, exported from `openarmature.llm`) carries a `per_attempt_override`: a schedule of `RuntimeConfig` partials applied to retries. Attempt 0 uses the caller's base `config` unchanged; retry `i` merges `per_attempt_override[i]` onto the base (the override's non-None fields replace; a None or unspecified field inherits the base, per the §6 null-skip semantics), and the last entry carries forward when the schedule is shorter than the retry count. The canonical use is an escalating temperature schedule that breaks the "temperature 0 replays the same output" determinism trap on a retried structured-output call. `complete()` never mutates the caller's `config` (each attempt config is a fresh copy), and a plain `RetryConfig` preserves the existing byte-identical replay. The per-attempt OTel span carries a new `openarmature.llm.retry_reason` attribute (`transient`) on retries, absent on the base attempt. This is the first half of proposal 0095; the structured-output reask half follows. Spec v0.91.0 is beyond the current v0.88.0 pin, so the behavior ships ahead of the pin (unit-tested); the conformance fixtures 061-066 ride the v0.17.0 pin bump. - **Adaptive call-level retry: structured-output reask** (proposal 0095, llm-provider §7.1, spec v0.91.0). The second half of 0095. `LlmRetryConfig` gains an opt-in `reask` builder (`Callable[[StructuredOutputInvalid], str]`). When present, a `structured_output_invalid` failure becomes retryable for that call (a call-level convenience, not a classifier change; without a builder it stays non-transient and raises on the first occurrence). On each such failure the loop appends two messages to a working transcript, the model's raw output as an `assistant` message and the builder's returned correction as a `user` message, so the retry is informed rather than a byte-identical replay. OA authors no prompt of its own (the caller owns every word beyond the model's output); the builder receives the raised `StructuredOutputInvalid` (its `raw_content` and `failure_description`). The transcript accumulates reask pairs across reask retries and consumes the `max_attempts` budget; a transient retry interleaved in a reask loop re-sends the accumulated transcript unchanged. `complete()` never mutates the caller's `messages` (each reask replaces the transcript with a fresh list rather than appending in place). The retry span's `openarmature.llm.retry_reason` is `reask` on a reask retry, `transient` otherwise. A reask always appends the model output as a fresh `assistant` message (never continues a trailing one): §3 requires the last message before a call to be `user`/`tool`, so the transcript never ends in `assistant`. Ships ahead of the pin (unit-tested); fixtures 062-066 ride the pin bump. - **Langfuse observer: credentials-in construction with tracer-provider isolation** (proposals 0114 + 0116 + 0117 + 0118, observability §6 / §8.9 / §8.4, spec v0.108.0 / v0.110.0 / v0.111.0 / v0.112.0). The Langfuse observer gains a second construction mode alongside today's caller-supplied client: `LangfuseObserver.from_credentials(public_key=..., secret_key=..., host=...)` (over the lower-level `LangfuseSDKAdapter.from_credentials(...)`) builds an OA-owned `Langfuse` client on a dedicated `TracerProvider` by default, so its observations no longer bind the global provider and leak onto the application's OTel backend. A Langfuse v4 client constructed with no `tracer_provider=` attaches its span processor to the globally-registered provider, so in any service that registers a global provider (the standard app-tracing setup) attaching the Langfuse observer silently exported every observation, prompts and completions included, to the app backend. Because the Langfuse SDK caches one client per `public_key`, a dedicated provider takes effect only when OA is the first constructor for that credential; OA reuses one isolated provider per credential and reads the actual binding back after construction. The invariant covers every payload OA harvests from the runtime -- the provider payload (`disable_provider_payload`), the Trace-level state input/output (`disable_state_payload` and the `trace_input_from_state` / `trace_output_from_state` hooks), and a failed Tool / Embedding / Retriever / LLM observation's `error_message` -- but not the dimensions the caller deliberately attaches (`correlation_id` / `session_id` / `userId` / trace name / caller metadata), which stay verbatim as cross-backend join keys. When any construction-determinable channel is live and OA establishes the client is bound to a provider it did not isolate, construction fails loud with a categorized `LangfuseProviderIsolationUnavailable` before any observation is emitted, rather than leaking payloads to a shared backend; where OA cannot establish the binding at all (a future SDK), it suppresses every channel and logs a warning. A failed observation's `error_message` is harvested exception text, so `disable_provider_payload` governs it for every failure category on all four provider observations: with payloads off it is not rendered, and the error category still rides as the status message where the event carries one. A Tool failure has no category, so its status message is null rather than falling back to the exception string. `error_type` is a classification token rather than harvested content and is never gated, which matters most for a Tool failure where it is the only remaining discriminator; it is optional, so it is emitted only where the failure event supplies one. A single `accept_shared_provider=True` opt-out turns the whole thing into a warn-and-proceed onto the shared provider. With no channel live (the default privacy posture), an un-isolatable client neither raises nor warns. The existing caller-supplied path (mode a) is unchanged and never mutated: a caller who builds their own client stays responsible for isolating its `tracer_provider`, and OA documents the remedy rather than reaching into the supplied client. The `secret_key` is accepted as a `pydantic.SecretStr`, masked in OA's own reprs and logs with the plaintext read only at the SDK call (`public_key` and `host` stay plain strings), and a blank credential is rejected at the boundary rather than falling through to the SDK's ambient `LANGFUSE_*` environment fallback. A `sample_rate` passed for the client is applied to the isolated provider, since the SDK only honors it on a provider it builds itself. `accept_shared_provider` binds the provider the application already registered rather than letting the SDK construct and globally register one of its own, which would capture OTel's single-assignment global slot. The new `LangfuseProviderIsolationUnavailable` derives from an `ObservabilityError` base, a fourth hierarchy alongside the graph-engine, llm-provider, and checkpoint ones. The behavior shipped ahead of the pin (unit-tested) and the pin has since advanced to v0.112.0. Fixture 159 now runs, and fixtures 098 / 137 / 138 are un-deferred and reconciled to the post-0118 shape. Fixtures 157 / 158 stay deferred: they need the conformance-adapter provider-faithful Langfuse fake and the `langfuse_client` construction directive, which are not yet built, and the source behavior is covered meanwhile by the unit suite. +- **Failed-observation `error_message` byte cap, and the `openarmature_` reserved namespace** (proposal 0119, observability §5.5.5 / §8.7 / §3.4, spec v0.116.0). Closes the two error-channel edges 0118 left open. **The cap:** a failed observation's `error_message` is now subject to the §5.5.5 per-value byte cap, on all four mapped provider observations (Generation, Embedding, Tool, Retriever). It takes the contract's *direct-application* arm rather than its *inheritance* arm: the OTel surface defines no `error_message` span attribute, so the value arrives untruncated and the observer that writes it applies its own `payload_byte_cap`, rather than inheriting a cap from an upstream OTel truncation that never happened. Re-applying a second cap to an already-truncated value would move the marker and misreport its byte total, which is why the two arms source the cap differently. A failed Tool observation renders the same harvested string twice, once in `metadata.error_message` and once as the observation's `statusMessage`, and **both** copies are capped: §5.5.5 governs payload-classified values rather than payload-classified fields, so capping one surface while the other still carried the whole exception would defeat the cap. The remaining `statusMessage` writes take the error *category*, a classification token, and stay uncapped. Under the default posture (`disable_provider_payload=True`) the field is absent entirely per 0118, so the cap is observable only where payloads are enabled, and the omission arm is unchanged: a withheld message still leaves `statusMessage` null on a Tool failure rather than substituting the message. **The reserved keys:** `openarmature_` joins `openarmature.` and `gen_ai.` as a reserved caller-metadata *namespace* prefix, and four exact names (`error_type`, `error_message`, `token_budget`, `token_budget_exceeded`) join the reserved set, which grows from 29 to 33. With `error_message` absent under the default posture, an unreserved caller key of that name would otherwise land unopposed in the very field 0118 requires to be absent, reintroducing through the metadata channel the leak the gate closes. Pre-1.0 behavioral change: a caller passing `invocation_metadata` with a key beginning `openarmature_`, or with any of those four names, is now rejected at the `invoke()` boundary with `ValueError` where it previously passed. Spec v0.116.0 is beyond the current v0.112.0 pin, so the behavior ships ahead of the pin (unit-tested); the `conformance.toml` entry and the fixtures ride the pin bump. No fixture drives the Tool arm today: the conformance adapter exposes no tool-calling node, so that arm is unit-tested only and the gap is reported to spec. **The maintenance check 0119 asks for lands with it:** a test that statically scans the Langfuse mapping for every top-level metadata key it writes and fails when one is neither reserved by name nor covered by a reserved namespace. It discovers the metadata bags from what is passed to a `metadata=` argument rather than hardcoding one name, because the observer builds five of them and a scan of the obvious one would miss four; 0119 asks for this specifically, since a sweep that looked at only a subset is what missed nine keys before. The check found three keys on the failure-isolation marker span (`error_category`, `failure_isolation_event_name`, `failure_isolation_node`) that a caller key still overwrites, because that handler merges caller metadata last. Those are held rather than reserved unilaterally: the marker is a graph-mechanism span that no mapping table covers, so whether the reserved set should reach an unmapped span is a spec question, and it is raised as one. **Truncation is also now surrogate-safe:** the cap runs on the failure path, where the value is harvested exception text and is the likeliest string in the observer to carry a lone surrogate (an `OSError` naming a surrogateescape-decoded path). Encoding one raises, an observer that raises is only warned about rather than logged, and the cap runs before the observation is created, so an unguarded encode would have deleted the very observation reporting the failure. A malformed message now degrades that one field and still respects the cap. ### Changed diff --git a/docs/agent/non-obvious-shapes.md b/docs/agent/non-obvious-shapes.md index cf7a97f2..118c8984 100644 --- a/docs/agent/non-obvious-shapes.md +++ b/docs/agent/non-obvious-shapes.md @@ -119,7 +119,7 @@ Different classes, same OTel-Logs export path. If both are attached against the The trace-side sibling of the `LoggerProvider` log-bridge gotcha above. When OA constructs the Langfuse client for you (`LangfuseObserver.from_credentials(...)` / `LangfuseSDKAdapter.from_credentials(...)`), it binds the client to a dedicated `TracerProvider` so OA's observations do not leak onto a provider shared with the application's OTel backend. But the Langfuse v4 SDK caches ONE client per `public_key` process-wide: if any client for that key was constructed first (the app called `Langfuse()`, used `langfuse.openai` / `@observe`, or an earlier OA call), the SDK returns the cached client and OA's dedicated provider is silently discarded. So isolation only holds when OA is the FIRST constructor for that credential. -Rather than leak silently, OA detects the binding and fails closed. When a harvested-payload channel is live and OA establishes the client landed on a provider it did not isolate, `from_credentials` raises `LangfuseProviderIsolationUnavailable` at construction, surfacing an init-ordering bug you would otherwise never see. The guarded channels are the provider payload (`disable_provider_payload`) and the Trace-level state input/output (`disable_state_payload` and the `trace_input_from_state` / `trace_output_from_state` hooks). A failed observation's `error_message` is harvested exception text, so `disable_provider_payload` governs it too, on all four mapped provider observations (LLM, Embedding, Tool, Retriever) and for every failure category: with payloads off it is simply not rendered. The error category still rides as the status message where the event carries one, so you can still see what kind of failure it was; a Tool failure has no category, so its status message is null rather than falling back to the exception string. `error_type` is a classification token, not harvested content, so it is never gated, which matters most for a Tool failure where it is the only discriminator left. A graph-mechanism span (a node span, a failure-isolation marker) never carries the exception message on any provider; that detail rides the OTel span instead (`openarmature.failure_isolation.message`), never Langfuse. Caller-attached dimensions (`correlation_id`, `session_id`, `userId`, trace name, your own metadata) are join keys by design and stay verbatim. +Rather than leak silently, OA detects the binding and fails closed. When a harvested-payload channel is live and OA establishes the client landed on a provider it did not isolate, `from_credentials` raises `LangfuseProviderIsolationUnavailable` at construction, surfacing an init-ordering bug you would otherwise never see. The guarded channels are the provider payload (`disable_provider_payload`) and the Trace-level state input/output (`disable_state_payload` and the `trace_input_from_state` / `trace_output_from_state` hooks). A failed observation's `error_message` is harvested exception text, so `disable_provider_payload` governs it too, on all four mapped provider observations (LLM, Embedding, Tool, Retriever) and for every failure category: with payloads off it is simply not rendered. The error category still rides as the status message where the event carries one, so you can still see what kind of failure it was; a Tool failure has no category, so with payloads off its status message is null rather than falling back to the exception string. Where payloads are on, the message is capped at the observer's own `payload_byte_cap` (nothing upstream has truncated it, because the OTel surface defines no `error_message` attribute), and a Tool failure's status message carries that same capped string rather than the full exception. `error_type` is a classification token, not harvested content, so it is never gated, which matters most for a Tool failure where it is the only discriminator left. A graph-mechanism span (a node span, a failure-isolation marker) never carries the exception message on any provider; that detail rides the OTel span instead (`openarmature.failure_isolation.message`), never Langfuse. Caller-attached dimensions (`correlation_id`, `session_id`, `userId`, trace name, your own metadata) are join keys by design and stay verbatim. Remedies: construct OA's Langfuse client before any other client for that `public_key`; or pass `accept_shared_provider=True` to acknowledge a shared provider (OA warns and proceeds); or build your own client with an isolated `tracer_provider=` and pass it in via the caller-supplied path (`LangfuseObserver(client=LangfuseSDKAdapter(your_client))`), which OA never mutates. Under OA's default privacy posture (no payload channel live), an un-isolatable client is harmless and neither raises nor warns. diff --git a/docs/concepts/observability.md b/docs/concepts/observability.md index 4e7410ca..ba6d9dc0 100644 --- a/docs/concepts/observability.md +++ b/docs/concepts/observability.md @@ -340,15 +340,29 @@ Every observability backend picks the entries up: work without any custom dashboard config. Validation runs at the `invoke()` boundary before any work begins. -Two rules: - -- **Keys** MUST NOT start with `openarmature.` or `gen_ai.` - (reserved for spec-normative attribute namespaces; collisions - would silently overwrite OA-emitted state). +Three rules: + +- **Keys** MUST NOT start with `openarmature.`, `gen_ai.`, or + `openarmature_` (reserved namespaces; collisions would silently + overwrite OA-emitted state). `openarmature_` is the underscore + form, for backends whose key syntax cannot carry a dot, and it is + a namespace rather than a list: any key under it is rejected, not + only the ones a mapping happens to write today. +- **Keys** MUST NOT exactly match a reserved name. These are the + top-level metadata keys OA itself writes alongside yours, so a + caller key of the same name would overwrite one. The list grows + with the spec mapping and currently includes `correlation_id`, + `entry_node`, `spec_version`, `namespace`, `step`, `error_type`, + `error_message`, `token_budget`, and `token_budget_exceeded`. The + authoritative set is `_RESERVED_KEY_NAMES` in + `openarmature.observability.metadata`. - **Values** MUST be OTel-attribute-compatible scalars (`str`, `int`, `float`, `bool`) or homogeneous arrays of those types. `None`, nested objects, and mixed-type arrays are rejected. +`userId` is deliberately not reserved: OA reads it to promote to +Langfuse's first-class `trace.userId`, so you can keep passing it. + Violations raise `ValueError` synchronously: no spans emitted, no work runs. @@ -1255,9 +1269,19 @@ What remains is enough to triage the failure. `error_type` is a classification token (an exception class name or vendor code), never gated, and the error category still rides as the observation's status message wherever the event carries one. A **tool** failure has no -category, so its status message is null rather than falling back to the -exception string. The full exception text is unaffected on the OTel -side. +category, so when the message is withheld its status message is null +rather than falling back to the exception string. The full exception +text is unaffected on the OTel side. + +Where the flag does permit the message, it is capped at +`payload_byte_cap` like any other payload value. The cap is applied +directly by this observer rather than inherited: the OTel surface +defines no `error_message` span attribute, so unlike +`generation.input` / `output` the value arrives untruncated and has +been capped by nobody upstream. A tool failure renders the same string +twice, in `metadata.error_message` and as the status message, and both +copies are capped, so a provider that echoes an HTML error page cannot +render in full through either one. ### Prompt linkage diff --git a/docs/patterns/caller-supplied-trace-identifiers.md b/docs/patterns/caller-supplied-trace-identifiers.md index bf21a120..7b23552e 100644 --- a/docs/patterns/caller-supplied-trace-identifiers.md +++ b/docs/patterns/caller-supplied-trace-identifiers.md @@ -87,8 +87,10 @@ Validation runs synchronously, before any node body fires. Both `invoke(metadata=...)` and `set_invocation_metadata(...)` enforce the same rules: -- Keys MUST NOT start with `openarmature.` or `gen_ai.` (reserved - namespaces per the spec). +- Keys MUST NOT start with `openarmature.`, `gen_ai.`, or + `openarmature_` (reserved namespaces per the spec). The + underscore form covers backends whose key syntax cannot carry a + dot. - Keys MUST NOT collide with the spec's reserved per-trace metadata keys (`correlation_id`, `entry_node`, `spec_version`, etc.). The set is enforced at the `invoke()` and `set_invocation_metadata` diff --git a/src/openarmature/AGENTS.md b/src/openarmature/AGENTS.md index 655d09ef..0bf9be5f 100644 --- a/src/openarmature/AGENTS.md +++ b/src/openarmature/AGENTS.md @@ -816,8 +816,10 @@ Validation runs synchronously, before any node body fires. Both `invoke(metadata=...)` and `set_invocation_metadata(...)` enforce the same rules: -- Keys MUST NOT start with `openarmature.` or `gen_ai.` (reserved - namespaces per the spec). +- Keys MUST NOT start with `openarmature.`, `gen_ai.`, or + `openarmature_` (reserved namespaces per the spec). The + underscore form covers backends whose key syntax cannot carry a + dot. - Keys MUST NOT collide with the spec's reserved per-trace metadata keys (`correlation_id`, `entry_node`, `spec_version`, etc.). The set is enforced at the `invoke()` and `set_invocation_metadata` @@ -1601,7 +1603,7 @@ Different classes, same OTel-Logs export path. If both are attached against the The trace-side sibling of the `LoggerProvider` log-bridge gotcha above. When OA constructs the Langfuse client for you (`LangfuseObserver.from_credentials(...)` / `LangfuseSDKAdapter.from_credentials(...)`), it binds the client to a dedicated `TracerProvider` so OA's observations do not leak onto a provider shared with the application's OTel backend. But the Langfuse v4 SDK caches ONE client per `public_key` process-wide: if any client for that key was constructed first (the app called `Langfuse()`, used `langfuse.openai` / `@observe`, or an earlier OA call), the SDK returns the cached client and OA's dedicated provider is silently discarded. So isolation only holds when OA is the FIRST constructor for that credential. -Rather than leak silently, OA detects the binding and fails closed. When a harvested-payload channel is live and OA establishes the client landed on a provider it did not isolate, `from_credentials` raises `LangfuseProviderIsolationUnavailable` at construction, surfacing an init-ordering bug you would otherwise never see. The guarded channels are the provider payload (`disable_provider_payload`) and the Trace-level state input/output (`disable_state_payload` and the `trace_input_from_state` / `trace_output_from_state` hooks). A failed observation's `error_message` is harvested exception text, so `disable_provider_payload` governs it too, on all four mapped provider observations (LLM, Embedding, Tool, Retriever) and for every failure category: with payloads off it is simply not rendered. The error category still rides as the status message where the event carries one, so you can still see what kind of failure it was; a Tool failure has no category, so its status message is null rather than falling back to the exception string. `error_type` is a classification token, not harvested content, so it is never gated, which matters most for a Tool failure where it is the only discriminator left. A graph-mechanism span (a node span, a failure-isolation marker) never carries the exception message on any provider; that detail rides the OTel span instead (`openarmature.failure_isolation.message`), never Langfuse. Caller-attached dimensions (`correlation_id`, `session_id`, `userId`, trace name, your own metadata) are join keys by design and stay verbatim. +Rather than leak silently, OA detects the binding and fails closed. When a harvested-payload channel is live and OA establishes the client landed on a provider it did not isolate, `from_credentials` raises `LangfuseProviderIsolationUnavailable` at construction, surfacing an init-ordering bug you would otherwise never see. The guarded channels are the provider payload (`disable_provider_payload`) and the Trace-level state input/output (`disable_state_payload` and the `trace_input_from_state` / `trace_output_from_state` hooks). A failed observation's `error_message` is harvested exception text, so `disable_provider_payload` governs it too, on all four mapped provider observations (LLM, Embedding, Tool, Retriever) and for every failure category: with payloads off it is simply not rendered. The error category still rides as the status message where the event carries one, so you can still see what kind of failure it was; a Tool failure has no category, so with payloads off its status message is null rather than falling back to the exception string. Where payloads are on, the message is capped at the observer's own `payload_byte_cap` (nothing upstream has truncated it, because the OTel surface defines no `error_message` attribute), and a Tool failure's status message carries that same capped string rather than the full exception. `error_type` is a classification token, not harvested content, so it is never gated, which matters most for a Tool failure where it is the only discriminator left. A graph-mechanism span (a node span, a failure-isolation marker) never carries the exception message on any provider; that detail rides the OTel span instead (`openarmature.failure_isolation.message`), never Langfuse. Caller-attached dimensions (`correlation_id`, `session_id`, `userId`, trace name, your own metadata) are join keys by design and stay verbatim. Remedies: construct OA's Langfuse client before any other client for that `public_key`; or pass `accept_shared_provider=True` to acknowledge a shared provider (OA warns and proceeds); or build your own client with an isolated `tracer_provider=` and pass it in via the caller-supplied path (`LangfuseObserver(client=LangfuseSDKAdapter(your_client))`), which OA never mutates. Under OA's default privacy posture (no payload channel live), an un-isolatable client is harmless and neither raises nor warns. diff --git a/src/openarmature/_patterns/caller-supplied-trace-identifiers.md b/src/openarmature/_patterns/caller-supplied-trace-identifiers.md index b25d1c6d..b84c67e3 100644 --- a/src/openarmature/_patterns/caller-supplied-trace-identifiers.md +++ b/src/openarmature/_patterns/caller-supplied-trace-identifiers.md @@ -87,8 +87,10 @@ Validation runs synchronously, before any node body fires. Both `invoke(metadata=...)` and `set_invocation_metadata(...)` enforce the same rules: -- Keys MUST NOT start with `openarmature.` or `gen_ai.` (reserved - namespaces per the spec). +- Keys MUST NOT start with `openarmature.`, `gen_ai.`, or + `openarmature_` (reserved namespaces per the spec). The + underscore form covers backends whose key syntax cannot carry a + dot. - Keys MUST NOT collide with the spec's reserved per-trace metadata keys (`correlation_id`, `entry_node`, `spec_version`, etc.). The set is enforced at the `invoke()` and `set_invocation_metadata` diff --git a/src/openarmature/observability/langfuse/observer.py b/src/openarmature/observability/langfuse/observer.py index 21be9fd0..173cbb77 100644 --- a/src/openarmature/observability/langfuse/observer.py +++ b/src/openarmature/observability/langfuse/observer.py @@ -189,9 +189,9 @@ def _apply_caller_metadata(metadata: dict[str, Any], caller_metadata: Mapping[st (``correlation_id``, ``entry_node``, ``spec_version``, ``namespace``, etc.) is not currently checked here: the rejection may happen at either boundary, and the ``invoke()`` API-boundary - validation already rejects ``openarmature.*`` / ``gen_ai.*`` - prefixed keys. Per-Langfuse-backend collision rejection is queued - as a follow-up. + validation already rejects every reserved namespace and reserved + name. Per-Langfuse-backend collision rejection is queued as a + follow-up. """ # None-tolerant, matching the OTel helper. An event kind whose # `caller_invocation_metadata` is optional (FailureIsolatedEvent) reaches @@ -562,13 +562,19 @@ def from_credentials( # observer built by handing a from_credentials adapter to the constructor. return cls(client=client, **observer_kwargs) - # NOTE: an emitted error_message is written verbatim, not through the - # payload_byte_cap truncation every other payload-classified field uses. The - # cap is not applied because 0118 classifies the field for GATING without - # saying it is subject to §5.5.5 truncation, and fixtures 150/151 exist to - # assert the message LITERALLY, which truncation would contradict. A provider - # that returns a very large exception string therefore renders it in full. - # Raised for the batched spec review rather than changed unilaterally. + # An emitted error_message IS capped, through `_capped_error_message` below. + # + # It was written verbatim until proposal 0119 (spec v0.116.0). 0118 had + # classified the field for GATING without saying it was subject to §5.5.5 + # truncation, so a provider returning a very large exception string rendered + # it in full. That reading was raised for the batched spec review rather than + # changed unilaterally, and 0119 answered it: §5.5.5 now governs every + # payload-classified VALUE, not only values written as span attributes, and + # §8.7 gives this one a direct-application arm because it has no span + # attribute to inherit a cap from. + # + # Do not re-derive the old behaviour from 0118 alone; 0119 is the governing + # text and it is later. def _emits_harvested_error_message(self) -> bool: # A failed observation's error_message is harvested exception text, so the # provider-payload flag governs it (0118) for every failure category and on @@ -2053,7 +2059,7 @@ def _handle_typed_llm_failed(self, event: LlmFailedEvent) -> None: if event.error_type is not None: metadata["error_type"] = event.error_type if self._emits_harvested_error_message(): - metadata["error_message"] = event.error_message + metadata["error_message"] = self._capped_error_message(event.error_message) model_parameters: dict[str, Any] = dict(event.request_params or {}) input_value: Any = None output_value: Any = None @@ -2160,8 +2166,19 @@ def _handle_tool_call(self, event: ToolCallEvent | ToolCallFailedEvent) -> None: # is withheld statusMessage stays null rather than taking the message # instead, which would smuggle the harvested string out. if self._emits_harvested_error_message(): - metadata["error_message"] = event.error_message - status_message = event.error_message + # BOTH surfaces are capped. §5.5.5 as 0119 restates it governs + # every payload-classified VALUE, not every payload-classified + # FIELD, and a Tool failure renders the harvested string twice: + # once in metadata and once as the status message. Capping one + # and not the other would leave the uncapped copy carrying the + # whole exception, which is the outcome the cap exists to stop. + # + # The other `status_message` writes in this file take + # `error_category`, a classification token rather than harvested + # content, and are correctly uncapped. + capped = self._capped_error_message(event.error_message) + metadata["error_message"] = capped + status_message = capped target_trace_id = self._trace_id_for(inv_state, event.namespace, event.fan_out_index) handle = self.client.tool( trace_id=target_trace_id, @@ -2271,7 +2288,7 @@ def _handle_embedding(self, event: EmbeddingEvent | EmbeddingFailedEvent) -> Non if event.error_type is not None: metadata["error_type"] = event.error_type if self._emits_harvested_error_message(): - metadata["error_message"] = event.error_message + metadata["error_message"] = self._capped_error_message(event.error_message) target_trace_id = self._trace_id_for(inv_state, event.namespace, event.fan_out_index) handle = self.client.embedding( trace_id=target_trace_id, @@ -2398,7 +2415,7 @@ def _handle_rerank(self, event: RerankEvent | RerankFailedEvent) -> None: if event.error_type is not None: metadata["error_type"] = event.error_type if self._emits_harvested_error_message(): - metadata["error_message"] = event.error_message + metadata["error_message"] = self._capped_error_message(event.error_message) target_trace_id = self._trace_id_for(inv_state, event.namespace, event.fan_out_index) handle = self.client.retriever( trace_id=target_trace_id, @@ -2516,10 +2533,11 @@ def _typed_event_metadata( # reflects the TERMINAL attempt's usage, so on a RETRIED call it can # differ from the OTel per-attempt attribute -- the parity is of the # FORMULA, not of per-attempt values, an intentional consequence of the - # terminal-only Generation; (b) like the token_budget bounds above, - # this unprefixed key shares the caller-metadata collision class -- a - # caller invocation-metadata key of the same name (applied later, - # unguarded) would shadow it until the reserved-key guard is extended. + # terminal-only Generation; (b) the caller-metadata collision + # this key once shared with the token_budget bounds is CLOSED: 0119 + # reserved `token_budget` / `token_budget_exceeded`, so a caller + # invocation-metadata key of either name is now rejected at the + # invoke() boundary rather than shadowing the emitted value. evaluations = _token_budget_evaluations(token_budget, event.usage) if evaluations: metadata["token_budget_exceeded"] = any(ev["actual"] > ev["max"] for ev in evaluations) @@ -2621,6 +2639,31 @@ def _maybe_truncate_for_input(self, value: Any) -> Any: return value # fits cap, native shape preserved return truncated + def _capped_error_message(self, message: str) -> str: + """A failed observation's ``error_message``, capped for emission.""" + # The cap is §8.7's. Applied DIRECTLY, under THIS observer's + # `payload_byte_cap`: the value has no span attribute to inherit a cap + # from, which is why §8.7 gives it + # a direct-application arm, and an observer MUST NOT take the OTel + # observer's cap for it. The two are configured independently and a + # deployment can set one and leave the other at its default. + # + # Encoding is guarded because this runs on the FAILURE path, where the + # value is harvested exception text and is the likeliest string in the + # observer to carry a lone surrogate: a `FileNotFoundError` naming a + # surrogateescape-decoded path, or a provider body decoded the same way. + # `"\udcff".encode("utf-8")` raises, and an observer that raises is only + # `warnings.warn`-ed by the engine, so the whole failed observation would + # vanish and the failure path would take out its own reporting. Degrade + # the one field instead. + try: + truncated = _truncate(message, self.payload_byte_cap) + except UnicodeEncodeError: + sanitized = message.encode("utf-8", errors="replace").decode("utf-8") + truncated = _truncate(sanitized, self.payload_byte_cap) + return sanitized if truncated is None else truncated + return message if truncated is None else truncated + def _maybe_truncate_for_output(self, value: str) -> str: # generation.output is a plain string in Langfuse's shape; # apply the cap directly to the source string. diff --git a/src/openarmature/observability/metadata.py b/src/openarmature/observability/metadata.py index 576e33f4..b18f4f7a 100644 --- a/src/openarmature/observability/metadata.py +++ b/src/openarmature/observability/metadata.py @@ -26,12 +26,12 @@ Validation rules (apply at every entry point): - Keys MUST be strings. -- Keys MUST NOT start with ``openarmature.`` or ``gen_ai.`` (reserved - attribute namespaces; collisions would silently - overwrite OA-emitted state at the observer layer). -- Keys MUST NOT exactly match a reserved OA-emitted top-level metadata - key name (the Langfuse set plus ``invocation_id``) for the same - collision reason. +- Keys MUST NOT start with any namespace in ``_RESERVED_PREFIXES``; + collisions would silently overwrite OA-emitted state at the observer + layer. +- Keys MUST NOT exactly match a name in ``_RESERVED_KEY_NAMES``, the + top-level metadata keys an OA-emitted backend mapping writes + alongside caller keys, for the same collision reason. - Values MUST be OTel-attribute-compatible scalars: ``str``, ``int``, ``float``, ``bool``, or a homogeneous list/tuple of those types. ``None``, nested objects, and mixed-type arrays are rejected. @@ -61,10 +61,21 @@ "openarmature.invocation_metadata", default=_EMPTY_METADATA ) +# The module docstring names these two sets rather than spelling them out. A +# hand-copied list there went stale the moment 0119 extended the tuple, and the +# copy is what a reader trusts. +# # Reserved key prefixes per §3.4. Keys with these prefixes are # off-limits to caller-supplied metadata; the engine rejects at the # boundary so observers never see a colliding key. -_RESERVED_PREFIXES: tuple[str, ...] = ("openarmature.", "gen_ai.") +# +# `openarmature_` is a NAMESPACE, not a set of exact names (proposal 0119, +# spec v0.116.0). Any caller key under it is rejected, not merely the ones +# an OA mapping happens to write today. It arrived in the same sentence as +# four new exact names, which makes "a few more exact matches" the natural +# misreading; it is the underscore form of the dotted prefix above, for +# backends whose key syntax cannot carry a dot. +_RESERVED_PREFIXES: tuple[str, ...] = ("openarmature.", "gen_ai.", "openarmature_") # Reserved exact key NAMES per §3.4 (proposals 0041, 0042): the # top-level metadata keys an OA-emitted §8 backend mapping writes @@ -94,6 +105,15 @@ "finish_reason", "system", "response_model", + # Proposal 0119 (spec v0.116.0). `error_type` / `error_message` became + # newly collidable when 0118 made `error_message` absent under the + # default posture: an unreserved caller key of that name would land + # unopposed in the very field 0118 requires to be absent, reintroducing + # the leak through the metadata channel. + "error_type", + "error_message", + "token_budget", + "token_budget_exceeded", "response_id", "prompt", "invocation_id", @@ -260,9 +280,16 @@ def _validate_metadata_key(key: Any) -> None: raise ValueError(f"invocation metadata key must be a string; got {type(key).__name__}") for reserved in _RESERVED_PREFIXES: if key.startswith(reserved): + # The list is rendered FROM the tuple, never hand-written. A + # hardcoded copy went stale the moment 0119 added a third prefix, + # leaving the message naming a prefix its own guidance excluded. + # "namespaces" rather than "attributes": these cover OTel span + # attributes AND the Langfuse top-level metadata keys. + known = ", ".join(f"{p}*" for p in _RESERVED_PREFIXES) raise ValueError( f"invocation metadata key {key!r} uses reserved namespace prefix {reserved!r}; " - f"reserved prefixes are for spec-normative attributes (openarmature.*, gen_ai.*)" + f"reserved namespaces are {known}, held for OA-emitted attributes and " + f"metadata keys. Rename the key." ) if key in _RESERVED_KEY_NAMES: raise ValueError( diff --git a/tests/unit/test_observability_langfuse.py b/tests/unit/test_observability_langfuse.py index 45591172..8cb447a6 100644 --- a/tests/unit/test_observability_langfuse.py +++ b/tests/unit/test_observability_langfuse.py @@ -2349,3 +2349,418 @@ def __str__(self) -> str: tools = [o for o in client.traces["inv-tool-opaque"].observations if o.type == "tool"] assert len(tools) == 1 + + +async def test_error_message_is_capped_under_this_observer_s_own_cap() -> None: + # Proposal 0119 (spec v0.116.0). §5.5.5 now governs every payload-classified + # VALUE rather than only values written as span attributes, and §8.7 gives a + # failed observation's `error_message` a DIRECT-application arm because it + # has no span attribute to inherit a cap from. + # + # It was written verbatim before 0119: 0118 classified the field for gating + # without saying it was subject to truncation, so a provider returning a very + # large exception string rendered it in full. + # + # The cap applied is THIS observer's `payload_byte_cap`. An observer MUST NOT + # take the OTel observer's cap for this value; the two are configured + # independently, under different names (`payload_max_bytes` there), and a + # deployment can set one and leave the other at its default. That asymmetry + # is what conformance fixture 160 exists to catch. This test pins the cap + # actually used by constructing the observer with `payload_byte_cap=256` and + # asserting the marker reports the cap applied at that value; it does not + # construct an OTel observer, so the cross-observer asymmetry itself rides + # fixture 160 rather than this unit test. + from openarmature.observability.correlation import ( + _reset_invocation_id, + _set_invocation_id, + ) + from tests._helpers.typed_event import make_failed_event + + client = InMemoryLangfuseClient() + # Payloads off is the §8.9 default, which withholds the harvested message + # entirely; that arm is fixture 160's THIRD case. These two are about the + # cap, so the channel is opened. + observer = LangfuseObserver(client=client, payload_byte_cap=256, disable_provider_payload=False) + long_message = "E" * 4000 + + token = _set_invocation_id("inv-cap") + try: + await observer( + make_failed_event( + invocation_id="inv-cap", + model="m-test", + error_category="provider_rate_limit", + error_type="ProviderRateLimit", + error_message=long_message, + call_id="cc-cap", + ) + ) + finally: + _reset_invocation_id(token) + + obs = next(o for o in client.traces["inv-cap"].observations if o.type == "generation") + rendered = obs.metadata["error_message"] + assert rendered != long_message, "the message was written verbatim; the cap was not applied" + assert len(rendered.encode("utf-8")) <= 256, ( + f"rendered {len(rendered.encode('utf-8'))} bytes against a 256-byte cap" + ) + # Pin the ALGORITHM, not just the length. Without the next three assertions a + # marker-less byte chop (`message.encode()[:cap].decode(errors="ignore")`) + # satisfies everything above while violating §5.5.5 outright. Verified by + # mutation: that exact chop left the whole suite green before these landed. + # + # The marker carries M, the PRE-truncation byte length, so asserting the + # exact tail also catches an implementation that reports the post-truncation + # length, or that serializes through JSON first (which would shift M by the + # two added quote bytes). + assert rendered.endswith("…[truncated, 4000 bytes total]"), ( + f"missing or malformed §5.5.5 truncation marker; tail was {rendered[-40:]!r}" + ) + kept = rendered[: -len("…[truncated, 4000 bytes total]")] + assert long_message.startswith(kept), "the kept bytes are not a prefix of the original message" + + +async def test_a_below_cap_error_message_is_left_alone() -> None: + # The control. Without it, an implementation that truncated unconditionally, + # or wrote a fixed marker, satisfies the cap test above. This is the case + # conformance fixture 160 adds beyond the three the proposal designed, and it + # is the one that catches that mistake. + # + # It is also why fixtures 150 / 151 are unaffected by the cap landing: their + # messages are far below any cap, so they still render literally. + # + # Provenance, corrected: this control is one of the THREE cases the proposal + # designed, not an addition beyond them. The two fixture 160 added beyond the + # proposal are the default-posture arm and the retriever arm. + from openarmature.observability.correlation import ( + _reset_invocation_id, + _set_invocation_id, + ) + from tests._helpers.typed_event import make_failed_event + + client = InMemoryLangfuseClient() + # Payloads off is the §8.9 default, which withholds the harvested message + # entirely; that arm is fixture 160's THIRD case. These two are about the + # cap, so the channel is opened. + observer = LangfuseObserver(client=client, payload_byte_cap=256, disable_provider_payload=False) + + token = _set_invocation_id("inv-small") + try: + await observer( + make_failed_event( + invocation_id="inv-small", + model="m-test", + error_category="provider_rate_limit", + error_type="ProviderRateLimit", + error_message="429 from upstream", + call_id="cc-small", + ) + ) + finally: + _reset_invocation_id(token) + + obs = next(o for o in client.traces["inv-small"].observations if o.type == "generation") + assert obs.metadata["error_message"] == "429 from upstream" + + +async def test_tool_failure_error_message_is_capped() -> None: + # §8.7's Tool arm, which is normative and has NO conformance fixture: a case + # would need a `mock_tool` primitive and a `calls_tool` block, and neither is + # defined in conformance-adapter §5. Spec recorded that in its + # open-questions and told us to read the arm as binding, so this unit test is + # the only cover the arm can have until the adapter grows those. + # + # The Tool observation is also the one where the message matters most. A + # failed Tool carries no error CATEGORY, so `error_type` is the only other + # discriminator on it; whatever the message does here cannot be inferred from + # the Generation arm, which has a category to fall back on. + from openarmature.graph.events import ToolCallFailedEvent + from openarmature.observability.correlation import ( + _reset_invocation_id, + _set_invocation_id, + ) + + client = InMemoryLangfuseClient() + observer = LangfuseObserver(client=client, payload_byte_cap=256, disable_provider_payload=False) + long_message = "T" * 4000 + + token = _set_invocation_id("inv-tool-cap") + try: + await observer( + ToolCallFailedEvent( + invocation_id="inv-tool-cap", + correlation_id=None, + node_name="run_tool", + namespace=("run_tool",), + attempt_index=0, + fan_out_index=None, + branch_name=None, + call_id="cc-cap", + tool_name="get_weather", + tool_call_id="call_cap", + arguments={"city": "Paris"}, + latency_ms=3.0, + error_type="TimeoutError", + error_message=long_message, + ) + ) + finally: + _reset_invocation_id(token) + + obs = next(o for o in client.traces["inv-tool-cap"].observations if o.type == "tool") + rendered = obs.metadata["error_message"] + assert rendered != long_message, "the Tool arm wrote the message verbatim" + assert len(rendered.encode("utf-8")) <= 256 + assert rendered.endswith("…[truncated, 4000 bytes total]"), ( + f"missing or malformed §5.5.5 truncation marker; tail was {rendered[-40:]!r}" + ) + # `error_type` is NOT payload-gated and NOT capped: it is a classification + # token, and on a Tool observation it is the only discriminator left. + assert obs.metadata["error_type"] == "TimeoutError" + # The status message is the SECOND surface carrying the same harvested + # string, and it must be capped too. It was not until following the Tool arm + # turned it up: capping only the metadata copy leaves the whole exception + # rendered on the observation, which is the outcome the cap exists to stop. + assert obs.status_message is not None + assert len(obs.status_message.encode("utf-8")) <= 256, ( + f"status message rendered {len(obs.status_message.encode('utf-8'))} bytes against a 256-byte cap" + ) + # Identical to the metadata copy, not merely short: the two surfaces render + # the same value and must not diverge. + assert obs.status_message == rendered + + +@pytest.mark.parametrize("arm", ["embedding", "rerank"]) +async def test_embedding_and_rerank_error_messages_are_capped(arm: str) -> None: + # The other two of §8.7's four mapped provider observations. Found by + # mutation: reverting the cap at each of the four sites individually left + # these two arms green, because the LLM and Tool tests above cover only their + # own handlers. A passthrough mutation of the shared helper, which breaks all + # four sites at once, produced exactly two failures rather than four, which is + # the same gap seen from the other side. + from openarmature.graph.events import EmbeddingFailedEvent, RerankFailedEvent + from openarmature.observability.correlation import ( + _reset_invocation_id, + _set_invocation_id, + ) + + client = InMemoryLangfuseClient() + observer = LangfuseObserver(client=client, payload_byte_cap=256, disable_provider_payload=False) + long_message = "E" * 4000 + inv = f"inv-{arm}-cap" + + event: Any + if arm == "embedding": + event = EmbeddingFailedEvent( + invocation_id=inv, + correlation_id=None, + node_name="embed", + namespace=("embed",), + attempt_index=0, + fan_out_index=None, + branch_name=None, + provider="openai", + model="embed-model", + latency_ms=1.0, + input_strings=["q"], + request_params={}, + request_extras={}, + active_prompt=None, + active_prompt_group=None, + call_id=f"cc-{arm}-cap", + error_category="provider_unavailable", + error_message=long_message, + ) + else: + event = RerankFailedEvent( + invocation_id=inv, + correlation_id=None, + node_name="rerank", + namespace=("rerank",), + attempt_index=0, + fan_out_index=None, + branch_name=None, + provider="cohere", + model="rerank-model", + latency_ms=1.0, + query="q", + documents=["d"], + document_count=1, + top_k=1, + request_params={}, + request_extras={}, + active_prompt=None, + active_prompt_group=None, + call_id=f"cc-{arm}-cap", + error_category="provider_unavailable", + error_message=long_message, + ) + + token = _set_invocation_id(inv) + try: + await observer(event) + finally: + _reset_invocation_id(token) + + # Selected by observation TYPE, not by error_message truthiness. Truthiness + # selection made both parametrizations assert byte-identical things, so + # misrouting the rerank event into the embedding handler still passed; it + # also breaks on an empty message, which a bare `raise SomeError()` produces. + expected_type = "embedding" if arm == "embedding" else "retriever" + obs = next(o for o in client.traces[inv].observations if o.type == expected_type) + rendered = obs.metadata["error_message"] + assert rendered != long_message, f"the {arm} arm wrote the message verbatim; the cap was not applied" + assert len(rendered.encode("utf-8")) <= 256, ( + f"{arm} rendered {len(rendered.encode('utf-8'))} bytes against a 256-byte cap" + ) + assert rendered.endswith("…[truncated, 4000 bytes total]"), ( + f"{arm}: missing or malformed §5.5.5 truncation marker; tail was {rendered[-40:]!r}" + ) + # The status message on these two arms takes the error CATEGORY, a + # classification token rather than harvested text, so it is correctly + # uncapped and must survive intact. Only the Tool arm renders the harvested + # string twice. + assert obs.status_message == "provider_unavailable" + + +async def test_a_multibyte_error_message_is_cut_on_a_code_point_boundary() -> None: + # `_truncate` backtracks off UTF-8 continuation bytes so the cut never lands + # mid-sequence. Every other cap test uses ASCII filler, where the backtrack + # is a no-op, so this is the only test that exercises it. Fixture 160's own + # header calls out the same hazard. + # + # A naive `encoded[:target].decode(errors="ignore")` silently drops the + # partial character and still looks plausible; a strict decode raises. This + # asserts the strict round-trip so either failure mode is caught. + from openarmature.observability.correlation import ( + _reset_invocation_id, + _set_invocation_id, + ) + from tests._helpers.typed_event import make_failed_event + + client = InMemoryLangfuseClient() + # The cap is 257, NOT 256, and the difference is the whole test. The marker + # for an 8000-byte message is 32 bytes, so a 256-byte cap leaves a 224-byte + # target, and 224 is an exact multiple of 4: the cut lands cleanly on a code + # point boundary and the backtracking loop never executes. Verified by + # mutation: at 256, deleting the loop entirely left the suite green. At 257 + # the target is 225, which is mid-sequence. (256 is also the §5.5.5 floor, so + # the cap cannot be lowered instead.) + observer = LangfuseObserver(client=client, payload_byte_cap=257, disable_provider_payload=False) + long_message = "\U0001f600" * 2000 + + token = _set_invocation_id("inv-multibyte") + try: + await observer( + make_failed_event( + invocation_id="inv-multibyte", + model="m-test", + error_category="provider_rate_limit", + error_type="ProviderRateLimit", + error_message=long_message, + call_id="cc-multibyte", + ) + ) + finally: + _reset_invocation_id(token) + + obs = next(o for o in client.traces["inv-multibyte"].observations if o.type == "generation") + rendered = obs.metadata["error_message"] + assert len(rendered.encode("utf-8")) <= 257 + # M counts BYTES, not code points: 2000 code points at 4 bytes each. + marker = "…[truncated, 8000 bytes total]" + assert rendered.endswith(marker) + # No partial sequence and no dropped character: every kept code point is whole. + kept = rendered[: -len(marker)] + assert kept == "\U0001f600" * len(kept), "the cut landed mid-sequence or dropped a partial character" + assert long_message.startswith(kept) + assert rendered.encode("utf-8").decode("utf-8", errors="strict") == rendered + # Prove the backtrack actually ran, rather than the cut happening to land on + # a boundary. The target is 257 - 32 = 225 bytes; a whole number of 4-byte + # code points below that is 224, so the kept run MUST be shorter than the + # target. Without this, a future cap change could silently return the test to + # the boundary-aligned case where the loop is dead code again. + target = 257 - len(marker.encode("utf-8")) + assert len(kept.encode("utf-8")) < target, ( + f"kept {len(kept.encode('utf-8'))} bytes against a {target}-byte target, so the cut " + "landed on a code point boundary and the backtracking loop was never exercised" + ) + + +async def test_a_surrogate_in_the_error_message_does_not_destroy_the_observation() -> None: + # Harvested exception text is the likeliest string in the observer to carry a + # lone surrogate: a FileNotFoundError naming a surrogateescape-decoded path, + # or a provider body decoded the same way. `"\udcff".encode("utf-8")` raises + # UnicodeEncodeError, and the cap is applied BEFORE the client call, so an + # unguarded encode kills the handler. + # + # The engine only `warnings.warn`s an observer exception, so the failed + # observation would disappear with no log record: the failure path would take + # out its own reporting. The observation must survive with the field degraded. + from openarmature.observability.correlation import ( + _reset_invocation_id, + _set_invocation_id, + ) + from tests._helpers.typed_event import make_failed_event + + client = InMemoryLangfuseClient() + observer = LangfuseObserver(client=client, payload_byte_cap=256, disable_provider_payload=False) + + token = _set_invocation_id("inv-surrogate") + try: + await observer( + make_failed_event( + invocation_id="inv-surrogate", + model="m-test", + error_category="provider_unavailable", + error_type="OSError", + error_message="cannot open /data/\udcff/report.json", + call_id="cc-surrogate", + ) + ) + finally: + _reset_invocation_id(token) + + generations = [o for o in client.traces["inv-surrogate"].observations if o.type == "generation"] + assert len(generations) == 1, "the observation was lost; the cap raised on the failure path" + rendered = generations[0].metadata["error_message"] + # Degraded, not dropped: the surrounding text survives and the value is + # encodable, so the backend can actually ingest it. + assert "cannot open" in rendered + assert "report.json" in rendered + rendered.encode("utf-8") + + +async def test_a_long_surrogate_bearing_message_is_still_capped() -> None: + # The sanitize path must not become a cap bypass: a message that is both + # malformed AND oversized still has to come back within the cap. + from openarmature.observability.correlation import ( + _reset_invocation_id, + _set_invocation_id, + ) + from tests._helpers.typed_event import make_failed_event + + client = InMemoryLangfuseClient() + observer = LangfuseObserver(client=client, payload_byte_cap=256, disable_provider_payload=False) + + token = _set_invocation_id("inv-surrogate-long") + try: + await observer( + make_failed_event( + invocation_id="inv-surrogate-long", + model="m-test", + error_category="provider_unavailable", + error_type="OSError", + error_message="\udcff" + "E" * 4000, + call_id="cc-surrogate-long", + ) + ) + finally: + _reset_invocation_id(token) + + obs = next(o for o in client.traces["inv-surrogate-long"].observations if o.type == "generation") + rendered = obs.metadata["error_message"] + assert len(rendered.encode("utf-8")) <= 256, ( + f"the sanitize path bypassed the cap: {len(rendered.encode('utf-8'))} bytes" + ) + assert "[truncated," in rendered diff --git a/tests/unit/test_observability_metadata.py b/tests/unit/test_observability_metadata.py index b30fc0c1..19e52e81 100644 --- a/tests/unit/test_observability_metadata.py +++ b/tests/unit/test_observability_metadata.py @@ -25,6 +25,8 @@ set_invocation_metadata, ) from openarmature.observability.metadata import ( + _RESERVED_KEY_NAMES, + _RESERVED_PREFIXES, validate_invocation_metadata, ) @@ -779,3 +781,183 @@ async def _writes_then_cancels(_state: Any) -> Mapping[str, Any]: assert dict(get_invocation_metadata()) == {"tenantId": "T1"} finally: _reset_invocation_metadata(baseline_token) + + +def test_validate_rejects_the_openarmature_underscore_namespace() -> None: + # Proposal 0119 (spec v0.116.0) reserves `openarmature_` as a NAMESPACE, not + # a set of exact names: any caller key under it is rejected, not merely the + # ones an OA mapping writes today. It is the underscore form of the dotted + # `openarmature.` prefix, for backends whose key syntax cannot carry a dot. + # + # The distinction is the whole point of this test. 0119 introduced the + # namespace in the same sentence as four new exact names, which makes "a few + # more exact matches" the natural misreading; a name no mapping emits, like + # the one below, is the case that tells the two readings apart. + from openarmature.observability.metadata import validate_invocation_metadata + + # Matched on the RULE, not the echoed key. The message interpolates the key, + # so `match="openarmature_"` succeeded for any ValueError naming it, + # including the exact-name rule and the value-type rule; it could not tell + # which branch fired. + with pytest.raises(ValueError, match="reserved namespace prefix"): + validate_invocation_metadata({"openarmature_not_a_key_we_emit": "x"}) + + +@pytest.mark.parametrize("key", ["error_type", "error_message", "token_budget", "token_budget_exceeded"]) +def test_validate_rejects_the_0119_reserved_names(key: str) -> None: + # `error_type` / `error_message` became newly collidable when 0118 made + # `error_message` absent under the default privacy posture: an unreserved + # caller key of that name lands unopposed in the very field 0118 requires to + # be absent, reintroducing the leak through the metadata channel. + from openarmature.observability.metadata import validate_invocation_metadata + + # The exact-name rule specifically, not merely "a rejection happened": these + # four are reserved by NAME, and matching the rule keeps the test honest if + # one of them ever gains a reserved prefix instead. + with pytest.raises(ValueError, match="is reserved: it exactly matches"): + validate_invocation_metadata({key: "x"}) + + +# --- Proposal 0119: the repository check that lands alongside the proposal --- +# +# 0119 requires a check that fails when a top-level metadata key written by the +# §8 mapping is neither in the §3.4 exact set nor covered by a reserved +# namespace. Its rationale, from the proposal: the maintenance rule has been in +# force since 0041 and has now been missed by THREE separate proposals. +# +# Two requirements the proposal states explicitly, both honored below: +# +# 1. It must scan every observation type rather than a subset. "A sweep that +# looked only at `observation.` / `trace.` / `generation.` / `span.metadata` +# is what missed the nine underscore keys in the first place." The analogue +# here is the metadata BAG NAME: the observer builds bags called `metadata`, +# `metadata_delta`, `link_metadata`, `dispatch_metadata` and +# `detached_metadata`, so a scan hardcoded to `metadata` would miss four of +# the five. The bag set is DISCOVERED from what is passed to a `metadata=` +# kwarg, so a sixth bag is picked up without editing this test. +# 2. It must encode the `userId` exclusion. `userId` is caller-SUPPLIED and read +# by OA for the §8.4.1 promotion to `trace.userId`; reserving it would break +# that promotion, so the check asserts it stays unreserved. + + +def _emitted_top_level_metadata_keys() -> dict[str, list[int]]: + # Static scan rather than a runtime sweep: a runtime sweep only sees the + # handlers a test happens to drive, which is the same subset problem one + # level up. + import ast + from pathlib import Path + + source = Path(__file__).resolve().parents[2] / "src/openarmature/observability/langfuse/observer.py" + tree = ast.parse(source.read_text(encoding="utf-8")) + + bags = { + kw.value.id + for node in ast.walk(tree) + if isinstance(node, ast.Call) + for kw in node.keywords + if kw.arg == "metadata" and isinstance(kw.value, ast.Name) + } + + found: dict[str, list[int]] = {} + + def note(key: str, lineno: int) -> None: + found.setdefault(key, []).append(lineno) + + def note_dict(node: ast.Dict, lineno: int) -> None: + for key_node in node.keys: + if isinstance(key_node, ast.Constant) and isinstance(key_node.value, str): + note(key_node.value, lineno) + + for node in ast.walk(tree): + if isinstance(node, ast.Assign): + for target in node.targets: + # bag["key"] = ... + if ( + isinstance(target, ast.Subscript) + and isinstance(target.value, ast.Name) + and target.value.id in bags + and isinstance(target.slice, ast.Constant) + and isinstance(target.slice.value, str) + ): + note(target.slice.value, target.lineno) + # bag = {...} + if isinstance(target, ast.Name) and target.id in bags and isinstance(node.value, ast.Dict): + note_dict(node.value, node.lineno) + # bag: dict[str, Any] = {...} + if ( + isinstance(node, ast.AnnAssign) + and isinstance(node.target, ast.Name) + and node.target.id in bags + and isinstance(node.value, ast.Dict) + ): + note_dict(node.value, node.lineno) + + return found + + +# The three keys on the `openarmature.failure_isolated` marker span. They are +# NOT reserved and a caller key of the same name overwrites them, because +# `_apply_caller_metadata` runs last on that handler with a bare merge. +# +# Held rather than fixed: that span is a graph-mechanism span which no §8.4.x +# table maps, so whether §3.4's reserved set should reach an UNMAPPED span is a +# spec question, not ours to settle unilaterally. Raised to spec in coord thread +# release-v0.17.0. When it rules, these move into `_RESERVED_KEY_NAMES` (or the +# handler's merge order changes) and this set shrinks to empty. +# +# Listing them by name is the point: a NEW unreserved key fails this test, which +# is the regression the proposal asks for. +_UNRESERVED_PENDING_SPEC_RULING = frozenset( + { + "error_category", + "failure_isolation_event_name", + "failure_isolation_node", + } +) + + +def test_every_emitted_metadata_key_is_reserved_or_namespaced() -> None: + emitted = _emitted_top_level_metadata_keys() + + # Guard the SCANNER, not just its verdict. If the observer is renamed or its + # metadata-building shape changes, a broken scan finds nothing and this test + # passes while checking nothing, which is the exact failure class the + # proposal is trying to close. The floor is far below today's count (44) so + # it does not need editing on every added key. + assert len(emitted) > 30, ( + f"the scan found only {len(emitted)} metadata keys, so it has probably " + "stopped matching the observer's shape; fix the scan before trusting a pass" + ) + + uncovered = { + key: lines + for key, lines in emitted.items() + if key not in _RESERVED_KEY_NAMES + and not any(key.startswith(prefix) for prefix in _RESERVED_PREFIXES) + and key not in _UNRESERVED_PENDING_SPEC_RULING + } + assert not uncovered, ( + "these top-level metadata keys are emitted by the Langfuse mapping but are " + "neither in _RESERVED_KEY_NAMES nor covered by a reserved prefix, so a " + f"caller metadata key of the same name silently overwrites them: {uncovered}. " + "Add each to _RESERVED_KEY_NAMES, or give it an `openarmature_` prefix." + ) + + +def test_the_scan_still_sees_the_keys_pending_a_spec_ruling() -> None: + # Pairs with the exclusion set above. Without this, a key that stops being + # emitted (or that the scan stops seeing) leaves a stale name in the + # exclusion set, quietly widening the hole the other test guards. + emitted = _emitted_top_level_metadata_keys() + assert _UNRESERVED_PENDING_SPEC_RULING <= set(emitted), ( + "a key held pending the spec ruling is no longer emitted; drop it from " + "_UNRESERVED_PENDING_SPEC_RULING rather than leaving the exclusion standing" + ) + + +def test_user_id_stays_unreserved_for_the_trace_promotion() -> None: + # The exclusion 0119 names. `userId` is caller-supplied and OA reads it to + # promote to the first-class `trace.userId`; reserving it would reject the + # very key the promotion exists to consume. + assert "userId" not in _RESERVED_KEY_NAMES + assert not any("userId".startswith(prefix) for prefix in _RESERVED_PREFIXES)