Skip to content

Add credentials-in Langfuse client construction with provider isolation - #266

Merged
chris-colinsky merged 4 commits into
mainfrom
feature/0114-langfuse-client-ownership
Aug 10, 2026
Merged

Add credentials-in Langfuse client construction with provider isolation#266
chris-colinsky merged 4 commits into
mainfrom
feature/0114-langfuse-client-ownership

Conversation

@chris-colinsky

Copy link
Copy Markdown
Member

Summary

Adds a second construction mode to the Langfuse observer: LangfuseObserver.from_credentials(...) (over LangfuseSDKAdapter.from_credentials(...)) builds an OA-owned Langfuse client on a dedicated TracerProvider, so OA's observations stop landing on the provider the application registered globally.

The problem it fixes was measured downstream: a Langfuse v4 client built without tracer_provider= attaches its span processor to the global provider, so in any service with app tracing (the standard setup) every OA observation, prompts and completions included, was also exported to the app's backend, inheriting that backend's storage and sensitivity profile.

Fail-closed behavior

The Langfuse SDK caches one client per public_key, so a dedicated provider only takes effect when OA constructs first. from_credentials reads the binding back after construction and picks an arm:

  • Isolated (OA constructed first): proceed normally.
  • Leaked (a client for that key already existed on another provider) and a payload channel is live: raise LangfuseProviderIsolationUnavailable before any observation is emitted.
  • Undetectable (a future SDK stops exposing the binding): suppress every channel and log a warning.
  • accept_shared_provider=True: warn and proceed onto the shared provider.

With no payload channel live (the default posture) an un-isolatable client neither raises nor warns.

Guarded channels

The provider payload (disable_provider_payload), the Trace state payload (disable_state_payload and the trace_input_from_state / trace_output_from_state hooks), and a failed provider observation's error_message / error_type, which is omitted per emission on a shared provider with the error category retained. Caller-attached dimensions (correlation_id, session_id, userId, trace name, caller metadata) stay verbatim; they are cross-backend join keys.

Separately, the openarmature.failure_isolated marker span no longer carries the caught exception's message. No mapping table covers that span, so writing harvested exception content onto it was over-emission that no privacy setting gated. It now matches the node span and carries only the category; the full exception is still recorded on the OTel side. A sweep of every bundled Langfuse handler found no other unmapped harvested-content emission.

Credentials are taken as SecretStr, masked in OA's own reprs and logs, with the plaintext read only at the SDK call.

Testing

Full suite: 1996 passed, 500 skipped. ruff + pyright clean. 22 unit tests cover the isolation classification, per-credential provider reuse, every construction arm, the state and hook triggers, and the per-emission error gate. Three rounds of adversarial review ran against earlier revisions.

Follow-ups (not in this PR)

Conformance fixtures 157 and 158 and the harness directives they need, plus the spec pin bump, ride a separate PR.

The Langfuse observer could only take a caller-built client, and a
Langfuse v4 client constructed without an explicit tracer_provider
binds the global one, so OA's observations were exported to every
processor on the application's provider. LangfuseObserver and
LangfuseSDKAdapter gain from_credentials, which builds an OA-owned
client on a dedicated TracerProvider reused per credential.

The SDK caches one client per public_key, so a dedicated provider only
takes effect when OA constructs first. from_credentials reads the
binding back and fails closed: when a payload channel is live and the
client landed on a provider OA did not isolate, construction raises
LangfuseProviderIsolationUnavailable before anything is emitted; when
the binding cannot be established it suppresses every channel and
warns; accept_shared_provider opts out with a warning instead. The
guarded channels are the provider payload, the Trace state payload and
its hooks, and a failed provider observation's error message, which is
omitted per emission with the error category retained.

The failure-isolation marker span no longer carries the caught
exception's message. No mapping table covers that span, so writing
harvested exception content onto it was over-emission that no privacy
setting gated; it now matches the node span and carries only the
category, with the full exception still on the OTel side.

Credentials are taken as SecretStr so they are masked in OA's reprs
and logs.
Copilot AI lite review requested due to automatic review settings August 9, 2026 20:39
Comment thread src/openarmature/observability/langfuse/client.py Fixed
Comment thread src/openarmature/observability/langfuse/client.py Fixed
Comment thread src/openarmature/observability/langfuse/client.py Fixed
Comment thread src/openarmature/observability/langfuse/client.py Fixed
Comment thread src/openarmature/observability/langfuse/client.py Fixed
Comment thread src/openarmature/observability/langfuse/client.py Fixed
Comment thread src/openarmature/observability/langfuse/client.py Fixed
Comment thread src/openarmature/observability/langfuse/client.py Fixed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a credentials-based construction path for the Langfuse observer/adapter that builds an OpenArmature-owned Langfuse client on a dedicated OpenTelemetry TracerProvider, preventing Langfuse span processors from attaching to (and exporting through) an application’s global provider. It also introduces fail-closed behavior and per-emission gating to avoid leaking harvested payloads (including harvested error messages) when provider isolation cannot be established.

Changes:

  • Add LangfuseObserver.from_credentials(...) and LangfuseSDKAdapter.from_credentials(...) to build an OA-owned Langfuse client with per-credential isolated TracerProvider reuse and post-construct isolation classification.
  • Enforce fail-closed / suppress / opt-out behavior based on isolation status, and gate harvested error_message / error_type emission when isolation is not established.
  • Add unit tests plus documentation and changelog entries describing the new construction mode and isolation guarantees.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/unit/test_langfuse_provider_isolation.py Adds unit tests covering isolation classification, provider reuse, and observer policy arms.
src/openarmature/observability/langfuse/observer.py Adds from_credentials, warning/raise/suppress policy, and per-emission harvested-error gating.
src/openarmature/observability/langfuse/errors.py Introduces a categorized exception for isolation-unavailable construction failures.
src/openarmature/observability/langfuse/client.py Defines SDK-free isolation status constants for observer-side gating.
src/openarmature/observability/langfuse/adapter.py Adds credentials-based Langfuse client construction with per-public-key isolated provider reuse and isolation classification.
src/openarmature/observability/langfuse/init.py Exports the new categorized isolation error.
src/openarmature/AGENTS.md Documents the new initialization-order gotcha and the fail-closed behavior.
docs/agent/non-obvious-shapes.md Mirrors the operational guidance for agent-facing documentation.
CHANGELOG.md Adds release notes for the new Langfuse construction mode and updated failure-isolated marker behavior.
Suppressed comments (3)

src/openarmature/observability/langfuse/observer.py:2211

  • The Embedding failure path now conditionally omits harvested error_type/error_message when _omit_harvested_error() is true, but the _handle_embedding docstring earlier still says those keys are always emitted for EmbeddingFailedEvent. Please update the docstring near the top of _handle_embedding to match this gate.
        metadata["openarmature_input_count"] = len(event.input_strings)
        if not self._omit_harvested_error():
            if event.error_type is not None:
                metadata["error_type"] = event.error_type
            metadata["error_message"] = event.error_message
        target_trace_id = self._trace_id_for(inv_state, event.namespace, event.fan_out_index)

src/openarmature/observability/langfuse/observer.py:2103

  • _handle_tool_call now conditionally omits harvested error_type/error_message (and the message-derived status_message) when _omit_harvested_error() is true, but the function docstring earlier still describes those fields as always present on ToolCallFailedEvent. Please update the docstring near the top of _handle_tool_call to reflect this privacy gate.
            if not self._omit_harvested_error():
                if event.error_type is not None:
                    metadata["error_type"] = event.error_type
                metadata["error_message"] = event.error_message
                status_message = event.error_message

src/openarmature/observability/langfuse/observer.py:2335

  • The Rerank/Retriever failure path now conditionally omits harvested error_type/error_message when _omit_harvested_error() is true, but the _handle_rerank docstring earlier still states those keys are always emitted for RerankFailedEvent. Please update the docstring near the top of _handle_rerank to reflect the privacy gate.
        if not self._omit_harvested_error():
            if event.error_type is not None:
                metadata["error_type"] = event.error_type
            metadata["error_message"] = event.error_message
        target_trace_id = self._trace_id_for(inv_state, event.namespace, event.fan_out_index)

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/openarmature/observability/langfuse/errors.py Outdated
Comment thread src/openarmature/observability/langfuse/observer.py Outdated
The isolation arms ran only in LangfuseObserver.from_credentials, so
building an observer over a LangfuseSDKAdapter.from_credentials client
reached emission on a shared provider with nothing raised, warned, or
suppressed. They move to __post_init__, which covers every path to an
OA-constructed client, and the payload sites now consult the client's
isolation status directly so a knob reopened after construction cannot
reopen the leak either.

Provider binding gets three corrections. The accept-a-shared-provider
opt-out resolves the provider the application registered instead of
passing None, which made the SDK build and globally register its own
and capture OTel's single-assignment slot. A tracing-disabled client is
classified before the binding is read, since it exports nothing whatever
provider it holds. The opt-out no longer short-circuits classification,
so a client the SDK resolved onto OA's own isolated provider is not
recorded as shared.

Blank credentials are rejected rather than falling through to the SDK's
ambient environment fallback, a sample_rate is applied to the isolated
provider, a construction that the SDK's per-credential cache will
discard says so, and the isolation warnings are no longer nested under
the payload check that left them silent by default. A
structured_output_invalid error message quotes the model's own output,
so it now follows the payload knob as well as the isolation gate.

Adds a canary test that plants a sentinel in every harvested input and
asserts none of them reach an un-isolated provider. It asserts the
invariant rather than the individual emission sites, so a channel nobody
enumerated fails the moment it is added; reintroducing either of the two
leaks found in review makes it fail.

Docs and both examples now lead with from_credentials rather than the
un-isolated construction they taught before, and the claim that the
failure-isolation message rides record_exception is corrected to the
span attribute it actually uses.
Export the ISOLATION_* constants from client.py's __all__. They are used
cross-module by the adapter and the observer, and live in client.py so
the observer can read a client's isolation status without importing the
SDK-gated adapter, but they were not declared as exports and so read as
module-private.

Annotate the error category as ClassVar[str], matching the sibling
prompt-management hierarchy; it was a bare assignment with no
annotation.

Correct the four failed-observation handlers' prose, which described
error_type and error_message as unconditionally surfaced. They are
omitted when isolation could not be established, and a tool failure
carries no category so its status message stays null.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

Suppressed comments (12)

docs/agent/non-obvious-shapes.md:124

  • This default-case statement is false for ISOLATION_UNDETECTABLE: _apply_isolation_policy logs a warning unconditionally even when every construction-time payload channel is off. Limit the no-warning claim to the known leaked-binding branch and mention the undetectable warning.
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.

docs/agent/non-obvious-shapes.md:128

  • The hierarchy count and list omit the already-public PromptError family, making ObservabilityError the fifth sibling hierarchy rather than the fourth. Add the prompt hierarchy and include both PromptError and ObservabilityError in the catch guidance.
### Four exception hierarchies; know which one your code catches

`openarmature` exceptions split across four sibling hierarchies:

src/openarmature/observability/langfuse/init.py:43

  • ObservabilityError is documented as the catchable package-level base, but this initializer exports only its subclass. Consequently from openarmature.observability.langfuse import ObservabilityError fails, unlike the public base classes in the LLM, prompt, and checkpoint packages. Re-export the base and add it to __all__.
from .errors import LangfuseProviderIsolationUnavailable

src/openarmature/observability/langfuse/errors.py:22

  • This list omits the existing checkpoint exception hierarchy, so the new base's documentation does not accurately distinguish it from all sibling public error families.
    Distinct from the graph-engine, llm-provider, and prompt-management
    hierarchies: these are raised while wiring or driving an observability
    backend, not while running a graph or calling a provider.

src/openarmature/observability/langfuse/observer.py:550

  • The factory can also raise when provider payloads remain disabled but state payloads or either state hook are enabled. This contract text also conflicts with the implementation's unconditional warning for an undetectable binding and its isolation fallback when no ambient provider is available. Document all construction-time channels and distinguish the leaked, undetectable, and no-ambient opt-out branches.
        When this observer emits provider payloads (``disable_provider_payload``
        is False) and OA cannot isolate the client -- the Langfuse SDK's
        per-``public_key`` singleton returned a client bound to a provider OA did
        not establish as isolated -- construction fails loud with
        :class:`LangfuseProviderIsolationUnavailable` before any observation is

src/openarmature/observability/langfuse/observer.py:954

  • The OTel failure-isolation handler does not call record_exception or use an OA-private provider; it writes the message to the openarmature.failure_isolation.message attribute on its configured OTel span (otel/observer.py:2218-2221). Correcting this comment avoids documenting a different telemetry shape than the implementation.
        # The caught exception's MESSAGE is deliberately absent: this marker is a
        # graph-mechanism span, which no §8.4.x table maps, so writing harvested
        # exception content onto it is non-conforming over-emission (0118). Like
        # the node Span, it carries only the error category; the full exception
        # reaches the OTel span via record_exception on OA's private provider.

tests/unit/test_langfuse_payload_leak_canary.py:60

  • Despite its name and invariant claim, this driver never emits embedding or rerank events, nor several success payload paths. In particular, removing either new error gate in _handle_embedding or _handle_rerank would still leave every canary test green. Add distinct sentinels and events for each mapped provider family, and include them in the isolated non-vacuity assertion.
async def _drive_every_channel(observer: LangfuseObserver) -> None:
    """Feed one event per harvested channel through the observer."""

docs/agent/non-obvious-shapes.md:120

  • An earlier OA from_credentials call is not a loss-of-isolation case: _reuse_isolated_provider deliberately returns the same provider for that key, and the classifier recognizes it as isolated. Only a client first constructed outside this OA-owned path causes the stated conflict.

This issue also appears in the following locations of the same file:

  • line 124
  • line 126
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.

examples/langfuse-observability/main.py:270

  • The relative clause currently says that passing tracer_provider= causes observations to reach the app backend, but a dedicated provider is precisely the documented remedy. State that the export occurs when no dedicated provider is supplied.
    # A Langfuse client you construct yourself binds the globally
    # registered TracerProvider unless you pass ``tracer_provider=``,
    # which also exports every observation to your app's tracing
    # backend. Validated against ``langfuse>=4.6,<5``.

docs/examples/langfuse-observability.md:145

  • This wording makes tracer_provider= sound like the cause of the duplicate export, although supplying a dedicated provider prevents it. Attribute the app-backend export to omitting a dedicated provider.
openarmature builds the client on a dedicated `TracerProvider` here, so
its observations stay off the provider your application registered
globally. Building the client yourself binds that global provider
unless you pass `tracer_provider=`, which exports every observation,
prompts and completions included, to your app's tracing backend too.

docs/concepts/observability.md:1142

  • This sentence incorrectly attaches the duplicate-export behavior to passing tracer_provider=. The manual construction example below passes a dedicated provider specifically to avoid exporting to the application's global backend.
Prefer `from_credentials`: openarmature builds the Langfuse client on a
dedicated `TracerProvider`, so its observations do not also land on the
provider your application registered globally. A client you build
yourself binds the global provider unless you pass `tracer_provider=`,
which exports every observation, prompts and completions included, to
your application's tracing backend as well.

CHANGELOG.md:23

  • This release-note entry has three factual mismatches with the implementation: an undetectable binding always warns even with default channels, accept_shared_provider=True falls back to isolation when no ambient provider exists, and ObservabilityError is the fifth public hierarchy because PromptError already exists. Align these claims with the implemented branches and public error families.
- **Langfuse observer: credentials-in construction with tracer-provider isolation** (proposals 0114 + 0116 + 0117, observability §6 / §8.9, spec v0.108.0 / v0.110.0 / v0.111.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` / `error_type` -- 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. The failed-observation error message is gated per-emission (not knowable at construction): on an un-isolatable provider it is omitted, retaining only the error category where one exists (a Tool failure has no category, so it carries no message-derived status either). 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. Spec v0.108.0 / v0.110.0 / v0.111.0 are beyond the current v0.107.0 pin, so this ships ahead of the pin (unit-tested); the conformance fixtures (157 / 158, proposals 0115 / 0116 / 0117) ride the pin bump. The LLM error-message arm ships ahead of its spec formalization (proposal 0118, in progress at time of writing).

The canary drove five events, which left three of the six gated
handlers unexercised: the LLM completion path, embeddings, and rerank.
For those the leak assertion passed vacuously, and the non-vacuity
check could not tell, because it only looked for the sentinels the
workload happened to plant.

The workload now covers every gated site with a distinct sentinel:
LLM input and output on the success path, the tool result, embedding
inputs and error message, and rerank query, documents and error
message. The non-vacuity check derives from the full sentinel set
rather than a hand-picked subset, so a channel that stops being driven
fails instead of quietly weakening the leak assertion.

Verified by mutation: ungating the embedding error rows or the rerank
provider payload now fails the canary, neither of which it could
detect before.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/openarmature/observability/langfuse/init.py:43

  • ObservabilityError is introduced as the public base of a new fourth exception hierarchy, and the new docs advertise it as living in openarmature.observability.langfuse, but this package re-exports only the concrete subclass. Unlike LlmProviderError and CheckpointError, callers therefore cannot import the documented base from the advertised package to catch the hierarchy. Re-export ObservabilityError here and include it in __all__.
from .errors import LangfuseProviderIsolationUnavailable

src/openarmature/observability/langfuse/observer.py:954

  • This comment misstates the OTel behavior. The OTel failure-isolation handler writes the message to the openarmature.failure_isolation.message attribute; it does not call record_exception, and it uses the OTel observer's configured provider rather than this Langfuse client's private provider. Please describe the actual attribute-based preservation so maintainers do not infer an exception event or provider coupling that does not exist.
        # the node Span, it carries only the error category; the full exception
        # reaches the OTel span via record_exception on OA's private provider.

src/openarmature/observability/langfuse/observer.py:557

  • This promises that the default posture never warns for any un-isolatable client, but _apply_isolation_policy() warns unconditionally for ISOLATION_UNDETECTABLE even when all construction-time channels have their defaults. Distinguish a known leaked binding from an undetectable binding so the public factory documentation matches its behavior.
        The default ``disable_provider_payload=True`` emits no payloads, so an
        un-isolatable client is harmless and neither raises nor warns.

src/openarmature/observability/langfuse/adapter.py:408

  • The isolation guarantee depends on private Langfuse SDK details (_resources.tracer_provider and the per-key resource-manager cache), but every new construction test patches Langfuse with a mock that reproduces those assumptions. Existing adapter tests already instantiate the supported real SDK without network access, so add equivalent real-client coverage for an isolated first construction and a pre-cached foreign-provider construction. Otherwise an SDK update within the supported <5 range can make classification always undetectable or misclassify a cached client while this suite remains green.
        client = Langfuse(

@chris-colinsky
chris-colinsky merged commit bcfc5a5 into main Aug 10, 2026
7 checks passed
@chris-colinsky
chris-colinsky deleted the feature/0114-langfuse-client-ownership branch August 10, 2026 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants