Skip to content

fix(langfuse): default base URL to localhost; reap stale per-turn trace state - #45326

Open
rdguidry wants to merge 1 commit into
NousResearch:mainfrom
rdguidry:fix/langfuse-plugin-localhost-default-and-reaper
Open

fix(langfuse): default base URL to localhost; reap stale per-turn trace state#45326
rdguidry wants to merge 1 commit into
NousResearch:mainfrom
rdguidry:fix/langfuse-plugin-localhost-default-and-reaper

Conversation

@rdguidry

Copy link
Copy Markdown

What does this PR do?

Two independent fixes to the Langfuse observability plugin:

1. Default HERMES_LANGFUSE_BASE_URL to http://localhost:3000

With credentials set but no base URL, the plugin currently defaults to https://cloud.langfuse.com — meaning a self-hosted deployment that forgets the URL silently ships trace payloads (user inputs, assistant outputs, tool results) to a third-party cloud. That is the wrong failure mode: a missing URL on a cloud setup fails loudly (401s on flush, no traces appear), while a missing URL on a self-hosted setup leaks data invisibly.

Defaulting to localhost inverts that: misconfiguration now fails closed (connection refused, traces dropped, fail-open semantics preserved) instead of leaking. Cloud users set the URL explicitly — which the README and hermes tools flow already instruct. Happy to flip this to requiring an explicit base URL (no default at all) if maintainers prefer that over a localhost default.

2. Reap orphaned TraceState entries

A turn that errors between pre_llm_request and the final post_llm_call never reaches _finish_trace, so its TraceState (root span, generation/tool observation handles) stays in the module-level _TRACE_STATE dict for the life of the process — a slow leak in long-running gateway deployments, and the spans are never ended.

TraceState already tracks last_updated_at; this PR adds _reap_stale_traces(), called at the start of each turn from both on_pre_llm_call and on_pre_llm_request, which flushes entries idle longer than 30 minutes through the existing _finish_trace path (ends all observations, ends the root span, flushes the client).

Note: this is complementary to #45048 / #43677, which close contexts at normal finish and interpreter exit — this handles turns that died mid-flight while the process keeps running.

Testing

scripts/run_tests.sh tests/plugins/test_langfuse_plugin.py — all 41 tests pass. Also running in production on a self-hosted Langfuse v3 deployment (gateway + Telegram platform) since this was written.

🤖 Generated with Claude Code

… state

Two fixes to the Langfuse observability plugin:

- Default HERMES_LANGFUSE_BASE_URL to http://localhost:3000 instead of
  https://cloud.langfuse.com. With keys set but no base URL, the old
  default silently shipped trace payloads (inputs, tool results) to
  Langfuse Cloud — the wrong failure mode for self-hosted deployments.
  Cloud users set the URL explicitly.

- Reap orphaned TraceState entries. A turn that errors between
  pre_llm_request and the final post_llm_call never reaches _finish_trace,
  leaking its TraceState (and unended spans) for the life of the process.
  _reap_stale_traces() now flushes entries idle longer than 30 minutes at
  the start of each turn.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins labels Jun 13, 2026

@tonydwb tonydwb 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 Summary

Verdict: Approved

Clean, well-scoped fix/feature with comprehensive tests. No issues found.

  • Logic is correct and focused
  • Tests cover the new behavior
  • No security concerns
  • Good error handling

Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused Langfuse work. The cloud-fallback premise remains on current main (plugins/observability/langfuse/__init__.py:201), but the stale-state portion needs adaptation because current main already bounds unfinished state with _MAX_TRACE_STATE and LRU eviction (plugins/observability/langfuse/__init__.py:713-735, commit f4fbaa6cd).

Problems

  • The Cloud setup path only collects the two credentials (hermes_cli/tools_config.py:551-557). PR 0fd6717e changes the fallback to localhost, so users configured through that supported Cloud flow would stop sending traces to Langfuse Cloud unless they manually add a URL.
  • The new reaper in PR 0fd6717e selects stale keys under _STATE_LOCK but finalizes them after releasing it. Current request setup refreshes last_updated_at under that lock (plugins/observability/langfuse/__init__.py:805-823), so a selected entry can become active before _finish_trace removes it.
  • The PR changes only README + implementation (gh pr diff 45326), leaving hermes_cli/config.py:4063-4069 and website docs still advertising the Cloud default; it also adds no regression coverage.

Suggested changes

  • Make the Cloud wizard write an explicit Cloud endpoint (or require an explicit endpoint consistently).
  • Atomically re-check and claim expired entries before finalization, with a refresh-race test.
  • Add fallback/reaper tests and update the config and website documentation.

Automated hermes-sweeper review.

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 sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants