Skip to content

fix(tracing): mask dict metadata per key when the mask function fails - #1956

Merged
hassiebp merged 5 commits into
prepare-v5-releasefrom
fix/mask-metadata-fallback-per-key
Oct 9, 2026
Merged

hassiebp merged 5 commits into
prepare-v5-releasefrom
fix/mask-metadata-fallback-per-key

Conversation

@niklassemmler

@niklassemmler niklassemmler commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Problem

When the custom mask function raises, _mask_attribute returns the string "<fully masked due to failed mask function>". For dict metadata that string goes through the non-dict branch of _flatten_and_serialize_metadata and is written to the bare langfuse.observation.metadata attribute.

Repro: start an observation with metadata={"a": 1}, then update(metadata={"secret": "pw", "b": 2}) with a mask that raises on the second call. The span ends with:

{'langfuse.observation.metadata.a': 1,
 'langfuse.observation.metadata': '<fully masked due to failed mask function>'}

The server parses the bare attribute as a JSON object, so the fallback is lost (or counted as a parse failure). Nothing shows which keys were masked.

Separately, mask never ran on propagated trace metadata (propagate_attributes(metadata=...), written as langfuse.trace.metadata.<key>), so those values were exported unmasked.

Fix

_process_media_and_apply_mask now passes field to _mask_attribute. If the mask fails and field == "metadata" and the data is a dict, the fallback is {key: fallback for key in data}. Each key of the failed call is written as langfuse.observation.metadata.<key>, keys from earlier updates are kept, and no bare attribute is written. Non-dict metadata, input and output still get the plain-string fallback.

_process_media_and_apply_mask is the only caller of _mask_attribute, and every metadata path goes through it (start, update, and _set_processed_span_attributes), so they all get the same behavior.

The fallback string is now a shared constant, MASK_FALLBACK_VALUE in langfuse/_client/constants.py.

Masking propagated trace metadata

propagate_attributes now applies the client's mask to each propagated trace metadata value. The mask gets only the value, never the key, so keys are kept. If the mask raises, only that value becomes "<fully masked due to failed mask function>". Compared with langfuse-js #1003: keys are kept and a failing mask only replaces that value in both SDKs. The differences come from Python masking in propagate_attributes while JS masks at export; see the examples below.

  • Where: in _propagate_attributes, before values are written to the context. The masked values are what gets stored in the OTel context and in baggage, so the current span, every child span (copied by the span processor on start), and outgoing baggage to other services all carry them. The mask runs once per value, not once per span.
  • Raw value: the mask sees the real Python value (e.g. a dict), like observation masking does. The result is then serialized and checked against the 200 character limit, so a mask can shorten a too-long value, and a mask result over 200 characters is dropped.
  • Which client's mask: the same client get_client() would pick: the one for the public key in the execution context, otherwise the only initialized client. With no client, or with several clients and no public key in context, no mask is applied. Without a configured mask nothing changes.
  • Known limitation: with several clients and no public key in the execution context, or with no client yet when propagate_attributes runs, propagated metadata is not masked, even if a client has a mask. Workaround: pass langfuse_public_key via @observe, or use a single client. mask_otel_spans also sees the langfuse.trace.metadata.* attributes at export, but not outgoing baggage. This is documented on the mask parameter, the propagate_attributes metadata argument, and _get_current_mask(), and pinned by a test.
  • Only metadata is masked. Experiment metadata set internally by the SDK is not.
  • The mask docstring on Langfuse and the metadata docs on propagate_attributes now describe this.

Behavior change by example

Mask fails on dict observation metadata

def mask(*, data, **kwargs):
    if isinstance(data, dict) and "secret" in data:
        raise ValueError("mask failed")
    return data

span = langfuse.start_observation(name="checkout", metadata={"a": 1})
span.update(metadata={"secret": "pw", "b": 2})
Attribute Before After
langfuse.observation.metadata.a 1 1
langfuse.observation.metadata "<fully masked due to failed mask function>" not written
langfuse.observation.metadata.secret not written "<fully masked due to failed mask function>"
langfuse.observation.metadata.b not written "<fully masked due to failed mask function>"

The mask still gets the whole dict in one call, as before. So when it fails, every key from that call is masked. Non-dict metadata, input and output still get the plain string.

Propagated trace metadata is now masked, per value

def mask(*, data, **kwargs):
    if isinstance(data, dict):
        return {k: "***" if k == "token" else v for k, v in data.items()}
    if isinstance(data, str):
        return data.replace("secret", "***")
    return data

with propagate_attributes(metadata={"api_key": "secret-key", "auth": {"token": "secret"}}):
    ...
Attribute (current and child spans) Before After
langfuse.trace.metadata.api_key "secret-key" "***-key"
langfuse.trace.metadata.auth '{"token":"secret"}' '{"token":"***"}'

The mask gets the raw Python value (the dict for auth), never the key, and runs once per value, not once per span. In JS (#1003) the mask gets the serialized string and runs again for every span.

A throwing mask only replaces the value it failed on

A mask that raises for "secret", with metadata={"api_key": "secret", "env": "prod"}, exports api_key as "<fully masked due to failed mask function>" and env as "prod".

Baggage is masked too

With as_baggage=True, the langfuse_metadata_api_key baggage entry sent to downstream services carries the masked value. In JS, baggage is sent unmasked.

The 200 character limit is checked after masking

With a mask that returns data[:10], metadata={"long": "a" * 300} used to be dropped and is now exported as "aaaaaaaaaa". A mask result over 200 characters is dropped. JS checks the limit before masking. Masking first is deliberate: the limit then applies to the value that actually gets exported, and a value that is too long is never trimmed into a short version of something the mask would have redacted.

This is one of three small PRs replacing the abandoned single-JSON-blob approach in #1944 (LFE-17153).

Verification

  • New tests in tests/unit/test_otel.py::TestMetadataHandling: mask fails on update (earlier keys kept, each failed key masked, no bare attribute); mask fails at start with dict metadata; non-dict metadata, input and output keep the string fallback. The first two failed before the fix.
  • New tests in tests/unit/test_propagate_attributes.py::TestPropagateAttributesMask: masked values on the current span and on a child span, keys kept, the mask called once per raw value; masked values in baggage; a throwing mask only replaces that value; the 200 character limit applies to the masked value; no mask means no change; with two clients the mask applies only when the public key is in context. The first four failed before the change.
  • uv run --frozen ruff check .: passes
  • uv run --frozen ruff format --check .: flags only tests/unit/test_media.py, which this PR does not touch (already the case on main)
  • uv run --frozen mypy langfuse --no-error-summary: passes
  • uv run --frozen pytest -n auto --dist worksteal tests/unit: 773 passed, 2 skipped, 18 errors. All 18 errors are in tests/unit/test_prompt.py ("Langfuse client is not initialized"). They also happen locally on main and are unrelated to this change.
  • e2e and live-provider suites not run: the change is local to span attribute construction and context propagation, and is covered by the in-memory exporter tests.

🤖 Generated with Claude Code

@niklassemmler
niklassemmler force-pushed the fix/mask-metadata-fallback-per-key branch from 1e1e202 to 47d263b Compare October 8, 2026 08:41
@niklassemmler
niklassemmler changed the base branch from main to prepare-v5-release October 8, 2026 08:42
@niklassemmler
niklassemmler marked this pull request as ready for review October 8, 2026 08:42
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown

@claude review

@claude claude Bot 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.

Claude Code Review

No review was started: this request came from a bot account. Manual reviews can only be requested by someone with write access to this repository. Ask a maintainer to comment @claude review, or have your automation post the comment from a user account with write access.

Tip: disable this comment in your organization's Code Review settings.

@claude claude Bot 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.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Comment thread langfuse/_client/span.py Outdated
@niklassemmler niklassemmler changed the title fix(tracing): mask metadata per key when the mask function fails fix(tracing): mask metadata per key, including propagated trace metadata Oct 8, 2026
@niklassemmler

Copy link
Copy Markdown
Contributor Author

@claude review

🤖 Written by Claude (an AI agent) on behalf of Niklas

@claude claude Bot 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.

I reviewed this PR and didn't find any bugs. Because it adds new mask-resolution and OTel context/baggage propagation logic around sensitive metadata masking, a human look would still be worthwhile.

What was reviewed: _get_current_mask()'s client resolution (public-key context vs. single-instance fallback under LangfuseResourceManager._lock) — the multi-client-silently-skips-masking scenario matches the documented single-client/@ observe usage pattern, not a regression; the per-value masking in _propagate_attributes before serialization/200-char truncation — runs once per value, keys preserved, exception scoped to that value; the experiment/dataset metadata masking gap — intentional scope limit per the PR description, not a new regression.

Extended reasoning...

This PR (two commits since the diff base) adds per-key mask fallback for observation metadata in span.py and extends masking to propagate_attributes() in propagation.py, including a new lock-protected client/mask resolution helper and per-value masking before OTel context/baggage writes — code that directly affects whether sensitive data is exposed in traces. The second commit (mask propagation) is new since my last review of this PR and is non-trivial, so it merits a fresh pass. No new bugs were found this run; two candidate concerns about multi-client mask resolution and experiment-metadata masking scope were examined and ruled out as intentional/documented behavior rather than regressions. Given the security-sensitive surface and added complexity, a human review is still worthwhile even though no defects were found.

@niklassemmler

Copy link
Copy Markdown
Contributor Author

@claude review

🤖 Written by Claude (an AI agent) on behalf of Niklas

@claude claude Bot 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.

Code review found no issues

No high-confidence issues detected in this change.

@niklassemmler
niklassemmler requested a review from hassiebp October 8, 2026 12:50
@niklassemmler niklassemmler self-assigned this Oct 8, 2026
niklassemmler and others added 3 commits October 9, 2026 11:02
When a custom mask function raised on dict metadata, the fallback string
was written to the bare langfuse.observation.metadata attribute. The
server parses that attribute as a JSON object, so the fallback was lost,
and nothing showed which keys had been masked. For dict metadata, write
the fallback under each key of the failed call instead, so it merges with
keys from earlier updates like any other update. Non-dict metadata, input
and output keep the plain-string fallback.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
propagate_attributes now applies the client's mask to each propagated
trace metadata value (keys are kept), on the raw value before
serialization and the 200 character check. If the mask raises, only
that value becomes the fallback string. The masked values are stored in
the context and baggage, so the current span, child spans, and outgoing
baggage all carry them. Matches langfuse-js #1003.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…adata

The mask for propagated trace metadata comes from the client for the
public key in the execution context, otherwise the only initialized
client. With several clients and no public key in context, or with no
client yet, propagated metadata is not masked. Document this and the
workarounds, and pin the behavior with a test.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@niklassemmler
niklassemmler force-pushed the fix/mask-metadata-fallback-per-key branch from d852fda to 37a31a2 Compare October 9, 2026 09:04
Propagated metadata is passed explicitly by the developer. Masking it
adds client-resolution ambiguity and baggage/length-limit ordering
complexity, and diverges from the JS SDK.

Co-authored-by: Hassieb Pakzad <hassiebp@users.noreply.github.com>
@cursor

cursor Bot commented Oct 9, 2026

Copy link
Copy Markdown

Trimmed at Hassieb's request: masking of propagated trace metadata is removed (68f16e4), and only the per-key mask-failure fallback for dict observation metadata remains. Propagated metadata is passed explicitly by the developer, so it's already in their control; masking it added client-resolution ambiguity and baggage/length-limit ordering complexity, and diverged from the JS SDK.

@cursor cursor Bot changed the title fix(tracing): mask metadata per key, including propagated trace metadata fix(tracing): mask dict metadata per key when the mask function fails Oct 9, 2026
…metadata

Co-authored-by: Hassieb Pakzad <hassiebp@users.noreply.github.com>
@hassiebp
hassiebp merged commit e99cc02 into prepare-v5-release Oct 9, 2026
13 checks passed
@hassiebp
hassiebp deleted the fix/mask-metadata-fallback-per-key branch October 9, 2026 09:20
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