Skip to content

feat(tracing)!: JSON-encode observation and trace metadata values - #1958

Open
niklassemmler wants to merge 10 commits into
prepare-v5-releasefrom
fix/json-encode-metadata-values
Open

niklassemmler wants to merge 10 commits into
prepare-v5-releasefrom
fix/json-encode-metadata-values

Conversation

@niklassemmler

@niklassemmler niklassemmler commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Per-key metadata attributes (langfuse.observation.metadata.<key>) are built in the dict branch of _flatten_and_serialize_metadata. Today str and int values are passed through as-is and everything else goes through _serialize (JSON). That causes two problems:

  1. Ints beyond int64 make the OTLP encoder fail for that key ("Failed to encode key ...: Value out of range"), so the key is silently dropped. Ints beyond 2^53 lose precision on the (JS) server.
  2. The server will start JSON-decoding per-key metadata values for newer SDK versions. With raw strings, the string "123" or "true" would decode to a number/bool, so strings and numbers would be conflated.

Fix

Every value in the dict branch is now sent as json.dumps(value, cls=EventSerializer), strings included. Non-dict metadata on the bare langfuse.observation.metadata attribute gets the same encoding.

  • Values are written with separators=(",", ":"), ensure_ascii=False, so the output is byte-identical with JS JSON.stringify (compact separators, non-ASCII kept as is), same as feat(tracing)!: JSON-serialize propagated metadata values #1932.
  • EventSerializer already turns ints outside the JS-safe range into JSON strings and NaN/Infinity into strings, so nothing extra is needed for those.
  • EventSerializer.encode catches every exception and returns a placeholder JSON string, so a value that fails to serialize can't drop the other keys. No extra per-value handling added.
  • None values are still skipped (logged at debug level, same message as langfuse-js fix(media): allow setting IO media via decorator update #1005), so update(metadata={"k": None}) doesn't overwrite an earlier value.
  • The non-dict branch (bare langfuse.observation.metadata attribute) is now JSON-encoded too, strings included, matching langfuse-js fix(media): allow setting IO media via decorator update #1005. Before, it went through _serialize, which passes str through raw, so metadata="foo" sent foo from Python and "foo" from JS. None is still not written. Input/output and the other _serialize callers are unchanged.
  • The type="trace" prefix of the same function gets the same encoding. On prepare-v5-release no production code calls it with "trace" (create_trace_attributes takes no metadata); only a unit test does.
  • _flatten_and_serialize_metadata_values (propagated trace metadata via propagate_attributes) is not touched here. feat(tracing)!: JSON-serialize propagated metadata values #1932 (already in prepare-v5-release) covers it.
  • OpenAI, LangChain and run_experiment only pass metadata dicts in. Nothing in the SDK reads these attributes back, so no integration code changed.

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

Wire values

Python value Before After
"hello" hello (str) "hello"
"123" 123 (str) "123"
"true" true (str) "true"
5 5 (OTel int) 5 (str)
True true (OTel bool) true (str)
1.5 1.5 (str) 1.5 (str)
2**70 key dropped by the OTLP encoder "1180591620717411303424"
float("nan") "NaN" "NaN"
[1, "a", None] [1, "a", null] [1,"a",null]
{"a": {"b": [1]}} {"a": {"b": [1]}} {"a":{"b":[1]}}
datetime(..., tz=UTC) "2024-01-02T03:04:05Z" "2024-01-02T03:04:05Z"
None skipped skipped

Non-dict metadata on the bare langfuse.observation.metadata key, matching langfuse-js #1005:

Python value Before After
"foo" foo "foo"
5 5 5
[1, "a", None] [1, "a", null] [1,"a",null]
None not written not written

Wire format change

  • All per-key metadata attributes, and the bare langfuse.observation.metadata attribute for non-dict metadata, are now JSON strings (same as langfuse-js fix(media): allow setting IO media via decorator update #1005). Custom span exporters and mask_otel_spans callbacks now see e.g. '"hello"' instead of 'hello', and "5" / "true" strings instead of native int/bool attributes.
  • This relies on the Langfuse server JSON-decoding per-key metadata values for newer SDK versions. Without that, the server would store string values with their quotes ("hello" including the quotes) and numbers/bools as strings. This PR must ship together with or after that server change.

E2E / live-provider tests that depend on server-side decoding

I did not change these expectations. They read observation metadata back through the API and will pass only once the server decodes the JSON values:

  • tests/e2e/test_core_sdk.py: test_concurrency (metadata["count"] == i, int), test_create_generation_complex (metadata["tags"] == ["yo"]; JSON before and after, unaffected), test_update_generation, test_update_span, test_end_generation_with_data, test_end_span_with_data, test_kwargs (string values such as "value", "whatsapp")
  • tests/e2e/test_decorators.py: test_nested_observations, test_nested_observations_with_non_parentheses_decorator, test_concurrent_decorator_executions, test_decorators_langchain, test_decorated_class_and_instance_methods, test_async_nested_openai_chat_stream (observation metadata["key"] / someKey), plus the multiproject tests that read obs.metadata.get("level" / "async" / "type")
  • tests/e2e/test_media.py::test_replace_media_reference_string_in_object and tests/e2e/test_decorators.py::test_media (dict value; JSON before and after, unaffected)
  • tests/live_provider/test_langchain.py (metadata["is_langchain_root"] is True, bool) and tests/live_provider/test_openai.py (metadata["someKey"] == "someResponse")

tests/e2e/test_core_sdk.py: test_create_numeric_score, test_create_boolean_score, test_create_categorical_score and test_create_text_score set non-dict metadata (metadata="test") on a generation but never read it back, so they don't depend on the decoding.

Trace-level metadata assertions (trace.metadata[...]) in e2e go through propagate_attributes and are out of scope here (see #1932).

CI follow-up (2f2596f)

The server decodes per-key metadata only for Python SDK major >= 5 (langfuse/langfuse#18436, not released yet), so on this 4.x branch nothing is decoded in CI. Three e2e tests failed because of that:

  • test_concurrency wrote str(i) and asserted an int. It now writes the int, which reads back the same way with or without decoding.
  • test_run_experiment_on_local_dataset / test_run_experiment_on_langfuse_dataset: the experiment items API doesn't parse metadata values on read, so values came back as '"Euro capitals"'. They now compare against raw_metadata_value(...) (tests/support/utils.py). That helper expects the JSON string below SDK major 5 and the plain value from 5 on, so the strict assertions return automatically with the version bump.

Verification

  • uv run --frozen ruff check .: passed
  • uv run --frozen ruff format --check .: only tests/unit/test_media.py would be reformatted. This PR doesn't touch that file and it's the same on main.
  • uv run --frozen mypy langfuse --no-error-summary: passed
  • uv run --frozen pytest -n auto --dist worksteal tests/unit (after rebasing onto prepare-v5-release): 773 passed, 2 skipped, 18 errors. All 18 errors are in tests/unit/test_prompt.py ("Langfuse client is not initialized"), which also happens locally on main.
  • New tests in tests/unit/test_otel.py::TestMetadataHandling were written first and failed before the fix: per-type encoding, big int surviving real OTLP encode_spans, None keeping the earlier value on update, and the trace prefix. test_non_dict_metadata_is_json_encoded covers string, int and list as non-dict metadata.
  • Skipped: e2e and live-provider suites. They need a Langfuse server with the decoding change (see above).

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

🤖 Generated with Claude Code

RetriggerConfidence Score: 2/5

Fix the lost run scores, repeated evaluations, and stale-output scoring before merging, and satisfy the required import layout.

What we checked:

  • Can one bad value drop other keys?: EventSerializer.encode catches encoding errors and returns a JSON string. Each metadata key is encoded separately.
Summary

The diff JSON-encodes each observation metadata value and preserves existing values when updates contain None. It also changes experiment tracking, batch evaluation, exporter defaults, supported Python versions, and integration tests.

  • Experiment run scores can disappear with custom samplers.
  • Cursor-based resume can score an observation twice.
  • Batch reads omit the update times needed to choose the newest duplicate row.
  • New imports must move to module level to satisfy the repository rule.

niklassemmler acknowledged the metadata wire-format change and its server-decoding dependency; those were not reported again.

Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[Observation metadata] --> B[Encode each value as JSON]
    B --> C[OpenTelemetry attributes]
    C --> D[Langfuse exporter]
    E[Experiment items] --> F[Item spans]
    F --> D
    E --> G[Run evaluators]
    G --> H[Sampling check]
    H --> I[Run scores]
    J[Existing observations] --> K[Cursor pages]
    K --> L[Choose duplicate rows]
    L --> M[Evaluators and scores]
    K --> N[Resume token]
    N --> K
Loading

Reviews (1) · Last reviewed commit: "fix(tracing): JSON-encode metadata attri..." · Reviewed by Greptile

Per-key `langfuse.observation.metadata.<key>` attributes now carry
`json.dumps(value, cls=EventSerializer)` for every value, strings
included. Previously str and int were passed raw/natively, so ints
beyond int64 made the OTLP encoder drop the key, ints beyond 2^53 lost
precision on the server, and a server that JSON-decodes values could
not tell the string "123" from the number 123.

None values are still skipped so updates keep earlier keys. The bare
non-dict metadata attribute is unchanged.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@niklassemmler
niklassemmler force-pushed the fix/json-encode-metadata-values branch from 871e170 to 1090a1e Compare October 8, 2026 08:42
@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.

The bare `langfuse.observation.metadata` attribute (non-dict metadata) is
now sent as `json.dumps(metadata, cls=EventSerializer)`, strings
included, matching the per-key values and langfuse-js #1005. Previously
strings were passed through raw, so `metadata="foo"` sent `foo` in
Python but `"foo"` in JS. None is still not written. Input/output and
other `_serialize` callers are unchanged.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@niklassemmler niklassemmler changed the title fix(tracing): JSON-encode metadata attribute values feat(tracing)!: JSON-encode metadata attribute values Oct 8, 2026

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

Beyond the inline finding, I also checked the lack of a version/feature gate for the new JSON-quoted wire format against self-hosted servers that haven't added the corresponding decoding change — the PR description explicitly discloses this as an intentional, coordinated-rollout constraint ("must ship together with or after that server change"), not an overlooked gap, so I'm not raising it separately.

Extended reasoning...

The diff changes _flatten_and_serialize_metadata in langfuse/_client/attributes.py to json.dumps every per-key metadata dict value and to skip None values, with matching test updates across four unit test files; it touches span/trace attribute export (no auth/crypto) but does affect how exported string attributes are later scanned for base64 media by span_exporter.py. I verified the confirmed inline finding by tracing _is_base64_data_uri in span_exporter.py, which requires value.startswith("data:") — after json.dumps a base64 data-URI string gets wrapped in literal quotes, so the startswith check fails and the payload ships inline instead of being extracted as media, matching the reported bug. Given that confirmed regression, a human should review before merging.

Comment thread langfuse/_client/attributes.py Outdated
@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 new issues

No new issues were found in this update; 1 finding from earlier reviews is still open above.

niklassemmler and others added 3 commits October 8, 2026 12:58
The server JSON-decodes per-key metadata values only for Python SDK
major >= 5 (langfuse/langfuse#18436). On 4.x the experiment items API
returns the raw JSON string, so compare against the encoded value until
the version bump. test_concurrency now writes an int so it round-trips
the same way with and without server-side decoding.

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

Encode observation and trace metadata values with separators=(",", ":")
and ensure_ascii=False so the output is byte-identical to JS
JSON.stringify, matching propagated metadata (#1932).

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

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@niklassemmler niklassemmler self-assigned this Oct 8, 2026
@niklassemmler
niklassemmler requested a review from hassiebp October 8, 2026 12:49
Update the span-attribute-limit and failed-mask tests to expect JSON-encoded
metadata values.

Co-authored-by: Hassieb Pakzad <hassiebp@users.noreply.github.com>
cursoragent and others added 3 commits October 9, 2026 10:13
…n metadata

propagate_attributes(metadata=...) now uses the same encoder as observation
metadata, so a string such as "123" stays a string once the server decodes
langfuse.trace.metadata.<key>. Ints outside the JS-safe range and NaN/Infinity
become JSON strings instead of exact digits or being dropped. The 200 character
limit applies to the encoded value.

Plain str values skip EventSerializer and go straight to json.dumps; the output
is byte-identical and pinned against EventSerializer and JSON.stringify.

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

Metadata values are JSON-encoded, so a top-level data URI arrives quoted and the
exporter no longer recognized it, shipping the base64 inline. Decode the JSON
string for Langfuse metadata attributes, process the media, and re-encode the
reference string.

Co-authored-by: Hassieb Pakzad <hassiebp@users.noreply.github.com>
Co-authored-by: Hassieb Pakzad <hassiebp@users.noreply.github.com>
@cursor cursor Bot changed the title feat(tracing)!: JSON-encode metadata attribute values feat(tracing)!: JSON-encode observation and trace metadata values Oct 9, 2026
@cursor

cursor Bot commented Oct 9, 2026

Copy link
Copy Markdown

At Hassieb's request, I added these on top of Niklas's changes:

The PR description doesn't mention the propagated-metadata, media and fast-path changes yet.

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

cursor Bot commented Oct 9, 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.

This branch has not been deployed

No deployments
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.

2 participants