Skip to content

fix(langfuse): repair silent trace-loss chain in observability plugin - #62882

Open
kfezer wants to merge 1 commit into
NousResearch:mainfrom
kfezer:fix/langfuse-cross-thread-trace
Open

kfezer wants to merge 1 commit into
NousResearch:mainfrom
kfezer:fix/langfuse-cross-thread-trace

Conversation

@kfezer

@kfezer kfezer commented Jul 11, 2026

Copy link
Copy Markdown

Summary

Traces silently never reach Langfuse in the bundled observability/langfuse plugin. Root cause: _start_root_trace enters client.start_as_current_observation(...) via a manual .__enter__() call, then ends the span later via .end() from _finish_trace — but Hermes fires its pre/post hooks across different worker threads. OTEL's contextvars.Token (pushed on enter) can only be popped in the same thread/context it was created in, so the mismatched enter/exit raises ValueError: <Token ...> was created in a different Context on essentially every turn. The exception is swallowed with no user-facing error or log line pointing at the real cause — the trace just never shows up.

  • Switches to the detached client.start_observation(...) API, which returns the same span type without mutating OTEL's "current span" contextvar, so start and end can safely happen on different threads.
  • Documents (README) that the langfuse>=3.0 SDK pin is load-bearing — an unpinned install can silently resolve to a 2.x SDK, which lacks propagate_attributes/OTEL support this plugin needs, making every hook a no-op with zero errors.
  • Documents that self-hosted Langfuse servers must also be v3+ (v3 SDKs speak OTLP exclusively; v2 servers don't implement that endpoint and 404 on every export).
  • Adds a known-working v3 self-hosted docker-compose reference plus a troubleshooting table for the failure modes hit while debugging this end to end.

Test plan

  • Verified locally: real Hermes turns (tui, cron, discord platforms) now produce complete traces in a self-hosted Langfuse v3 instance — nested LLM-call spans, token usage, cost fields, tool-call counts all present, confirmed via GET /api/public/traces/<id>.
  • python -c "import ast; ast.parse(...)" syntax check on the modified plugin file.
  • Would appreciate a maintainer sanity-check against Langfuse Cloud (only tested against self-hosted v3 here).

🤖 Generated with Claude Code

Traces were never reaching Langfuse due to a cross-context bug in the
plugin itself: start_as_current_observation() was entered in one hook
call and its span ended in another, and since Hermes fires pre/post
hooks across different worker threads, OTEL's contextvars Token could
not be popped in the thread it was created in — raising
"ValueError: <Token ...> was created in a different Context" on every
turn, silently, with the trace just never sent.

Switch to the detached start_observation() API, which returns the same
span object without mutating OTEL's "current span" contextvar, so
start/end can safely happen on different threads.

Also document (README) that the >=3.0 SDK pin is load-bearing (an
unpinned install can silently resolve to a 2.x SDK, which fails an
import this plugin needs and makes every hook a no-op with zero
errors), that self-hosted Langfuse servers must also be v3+ (v3 SDKs
only speak OTLP, which 2.x servers don't implement), and add a known-
working v3 self-hosted docker-compose reference plus a troubleshooting
table for the failure modes hit while debugging this end to end.
@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have labels Jul 11, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related cluster in plugins/observability/langfuse/__init__.py: #61166 (open, update_trace() API rename so turn Input/Output columns fill) and #59026 (open issue, silent trace loss after a venv refresh removes the SDK). Each is a distinct failure mode from this PR's cross-thread OTEL Token enter/exit mismatch, so linking as related rather than duplicate. Flagging for a maintainer since all three touch the same plugin file.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for isolating the root-observation lifecycle and documenting the observed Langfuse failure modes. The detached-observation direction is plausible, but this needs a small compatibility pass before it can be safely salvaged.

Problems

  • Removing root_ctx at plugins/observability/langfuse/__init__.py:45 leaves six current-main test constructions passing root_ctx=None (for example tests/plugins/test_langfuse_plugin.py:721); the PR does not update that file.
  • README plugins/observability/langfuse/README.md:15-24 calls SDK v3 mandatory, but the interactive path still treats any importable SDK as valid and installs unversioned langfuse (hermes_cli/tools_config.py:1389-1397).
  • The PR adds no automated coverage for the new start_observation() path or its claimed cross-context finalization behavior.

Suggested changes

  • Update the Langfuse fakes and all TraceState test constructors, then add a regression test for detached root creation and finalization.
  • Make the setup/version behavior and both user-facing setup documents agree, or narrow the README claim.

Automated hermes-sweeper review.

# trace silently never reaches Langfuse. start_observation() returns the
# same span object without touching the "current span" contextvar, so
# start/end can safely happen on different threads.
root_span = client.start_observation(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The existing Langfuse fake in tests/plugins/test_langfuse_plugin.py:265-304 implements only start_as_current_observation(), and the test file still constructs TraceState(..., root_ctx=None, ...) in six places. Please update that test contract and add coverage for this detached API before merging.


# Manual
pip install langfuse
pip install 'langfuse>=3.0'

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This version floor is not applied by the recommended interactive setup: hermes_cli/tools_config.py:1389-1397 accepts any importable SDK and otherwise installs unversioned langfuse. Please make the installer/version check and the primary built-in-plugin documentation enforce the same requirement, or narrow this claim.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 11, 2026
@jack-h-park

Copy link
Copy Markdown

Data point from Langfuse Cloud, which the test plan lists as untested.

  • langfuse 4.15.1, opentelemetry-* 1.44.0, Hermes v0.21.0 (29112bef),
    Python 3.11.16, macOS x86_64
  • The ValueError: <Token ...> was created in a different Context reproduces
    on every turn, so v4 does not fix it on its own.
  • But traces are not lost: five turns each produced a complete
    Hermes turn chain plus its generation, with model, token counts and
    session grouping intact, confirmed via GET /api/public/traces.

So on Cloud + v4 the two symptoms are separable — the detach error fires
without the trace loss this describes. Worth knowing before the fix is
evaluated only against the trace-loss claim.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants