feat(langfuse): attribute traces to the messaging end-user (userId)

[kamonspecial]

Summary

Set the Langfuse trace userId to the messaging end-user (sender_id) so gateway traffic can be attributed per person.

Problem

When Hermes runs as a gateway for a shared, multi-user chat (e.g. a team Slack/Discord), the bundled observability/langfuse plugin groups every turn under the internal Hermes session_id only. The trace userId is never set, so for gateway traffic there's no way to tell which human drove a given turn — the exact thing userId exists for (per-user cost/latency, "who asked for this?" when debugging).

Solution

No core change — the data is already available via the documented hook contract:

• The turn-scoped pre_llm_call hook emits sender_id (from agent._user_id ← source.user_id, populated for every gateway platform).
• The plugin's on_pre_llm_call previously discarded it: the turn-scoped variant early-returns to avoid an orphan trace, and the root trace is created later in _start_root_trace (driven by pre_api_request), which had no user identity.

Contained to plugins/observability/langfuse/__init__.py:

• on_pre_llm_call accepts sender_id and stashes it by session_id (_SENDER_BY_SESSION) before its early return.
• _start_root_trace accepts user_id and sets it via propagate_attributes(user_id=...), mirrored into trace metadata. The legacy messages-list path passes sender_id directly; the pre_api_request path reads it back from the stash.
• _finish_trace clears the per-session entry. Cron/bot turns have no end-user, so userId stays unset (intended).

This stays platform-agnostic — it only uses the generic sender_id already provided by the hook contract, so it works for every gateway with no per-platform code. Older Langfuse SDKs whose propagate_attributes predates the user_id kwarg degrade gracefully (the call is retried without it, keeping session grouping/tags), with the user still recorded in trace metadata.

Testing

• uv run --extra dev --extra messaging pytest tests/plugins/test_langfuse_plugin.py -q → 43 passed (4 new in TestEndUserAttribution).
• uv run --extra dev ruff check ... → clean.

[liuhao1024]

Verification review — clean ✅

Read the full diff. The approach is well-designed:

1. Thread safety: _SENDER_BY_SESSION is properly guarded by the existing _STATE_LOCK. The stash (in on_pre_llm_call) and read (in on_pre_llm_request) + pop (in _finish_trace) are all under the lock.

2. Backward compatibility: The nested try/except TypeError around propagate_attributes(**attr_kwargs) gracefully handles older Langfuse SDKs that don't accept user_id. The fallback preserves session grouping / trace_name / tags — only user_id is dropped from the context manager call, while it still lands in trace metadata.

3. Lifecycle correctness: The mapping is stashed on the turn-scoped pre_llm_call (which carries sender identity), consumed on pre_api_request (which creates the root trace but has no sender info), and popped on _finish_trace. This matches the documented two-phase trace creation pattern in references/langfuse-plugin-hook-parameter-asymmetry.md.

4. Test coverage: 4 tests cover the happy path (sender sets user_id), the stash+consume+clear lifecycle, the no-sender case (user_id omitted entirely, not passed as None), and the legacy SDK fallback. The _fresh_plugin() helper avoids cross-test pollution.

No issues found.

[austinpickett]

Good technical implementation, deferring to maintainer for product sign-off on whether this feature is wanted.

Technical notes:

✅ Correct plumbing: sender_id is captured on the turn-scoped pre_llm_call (where it is available in the hook payload) into a _SENDER_BY_SESSION dict, then consumed on on_pre_llm_request where the root trace is actually created — a necessary two-phase approach since the request-scoped hook carries no user identity.
✅ TraceState gets a session_id field so _finish_trace can clean up the _SENDER_BY_SESSION entry, preventing a long-running / persistent-session memory leak.
✅ propagate_attributes user_id kwarg is passed with a graceful TypeError fallback for older Langfuse SDK versions that do not accept it.
✅ user_id is also stashed in trace metadata as a secondary record, which is useful for querying even when propagate_attributes fails.
✅ Well-tested: three new test cases cover the happy path, the two-phase stash/read path, and the no-sender case.

Minor concerns:

1. PII consideration: sender_id is typically a messaging platform user ID (Slack user ID, Discord snowflake, etc.) — an opaque identifier, not a name or email. As long as the Langfuse instance is self-hosted or the operator has a DPA in place with Langfuse Cloud, this is fine. Worth a note in the plugin README or config docs that enabling sender_id attribution sends user identifiers to the configured Langfuse endpoint.

2. _SENDER_BY_SESSION grows unbounded on session crash: _finish_trace cleans up the entry on normal completion, but if the process crashes or _finish_trace is never called (e.g. unhandled exception before turn end), the entry leaks for the lifetime of the process. A TTL-based eviction or size cap would be defensive.

3. Thread safety on _SENDER_BY_SESSION: Reads in on_pre_llm_request (_SENDER_BY_SESSION.get(session_id, "")) are outside the _STATE_LOCK. CPython dict reads are effectively atomic for single-key .get(), but this is an implicit assumption worth documenting or protecting explicitly.

[tonydwb]

Code Review Summary

Verdict: Approved (with notes)

Overview

Feature adding Langfuse userId attribution by capturing sender_id from the turn-scoped pre_llm_call hook and propagating it to root traces.

Looks Good

• Clean two-phase approach: stash sender_id on pre_llm_call (where available), read on pre_api_request (where trace is created)
• Proper TraceState.session_id field for cleanup
• Graceful fallback for older Langfuse SDK versions without propagate_attributes
• user_id also stored in metadata as secondary record
• Well-tested with three test cases

Notes (non-blocking)

• sender_id (messaging platform user ID, e.g. Slack snowflake) is sent to the configured Langfuse endpoint. Ensure Langfuse is self-hosted or a DPA is in place for cloud usage.
• _SENDER_BY_SESSION dict is cleaned on normal trace completion but can leak on process crash. TTL/size cap would be defensive but not required.
• Thread safety is an implicit assumption (CPython single-key dict.get is atomic). Worth documenting or adding explicit locking for belt-and-suspenders.

---

Reviewed by Hermes Agent

[kamonspecial]

CI test (5) failure is unrelated to this PR. it's tests/test_tui_gateway_server.py::test_notification_poller_emits_distinct_watch_matches_once in the
process-registry watch-match dedup logic. This PR touches only plugins/observability/langfuse/. The same job has gone red on at least one unrelated branch in recent runs, suggesting a flake.

[kamonspecial]

Rebased onto current main to resolve the v0.17.0 conflict, and made end-user attribution opt-in: it's now gated behind HERMES_LANGFUSE_END_USER_ATTRIBUTION (default off), so the change is purely additive — when unset the trace carries no user and nothing changes vs current behavior or #26455. The token/cost fix landed separately in #40560. Tests were updated to the v0.17.0 scaffolding (full plugin suite: 56 passing).

[teknium1]

Thanks for the careful two-phase tracing implementation and the cleanup/compatibility coverage.

This automated hermes-sweeper review is closing this under the standing configuration policy:

• The PR introduces HERMES_LANGFUSE_END_USER_ATTRIBUTION as a non-secret, user-facing behavior flag (plugins/observability/langfuse/__init__.py, PR commit 58c73c7285f36b90a3de28276998f7fb26a0645d).
• AGENTS.md:102-107 requires behavioral configuration to use config.yaml, not a new HERMES_* environment variable.
• More specifically, AGENTS.md:118-121 requires a generic user-facing opt-in — config gate, setup prompt, and hermes tools toggle — before exporting third-party user identifiers or attribution tags.

The current-main plumbing does expose sender_id to the turn hook (agent/turn_context.py:482-493), so the technical direction is understood. Please keep this capability parked or re-scope it once the generic telemetry-consent surface exists, rather than adding a plugin-specific .env switch.

---

Closed as not-planned per standing maintainer policy (env-var-for-config). This is a design-direction decision, not a code-quality judgment — see the Contribution Rubric in AGENTS.md for what the project is looking for. If you believe this policy was misapplied to your change, comment here and a maintainer will take a look.