fix(observability): namespace Langfuse observation ids by trace to stop cross-trace overwrites#32
Conversation
…op cross-trace overwrites
Model generations and tool spans took their Langfuse observation id
straight from call_id (e.g. `agent_turn-model-1`). call_id is only
unique within a run, and the logical run id (`agent_turn`) is reused by
every interactive turn, so the id was identical across all turns and
threads. Langfuse upserts observations by id project-wide, so each new
turn's model generation silently overwrote the previous one and stole it
onto the newest trace — leaving every earlier trace with no model
usage/cost/content (observed: 75 traces, only 5 surviving `model`
generations).
Namespace the observation id with the globally-unique per-trace id
(`{trace_id}:{call_id}`): unique across turns/threads, stable for
idempotent re-ingestion. The raw call_id now rides in observation
metadata so in-run correlation (model.started <-> its generation) is
preserved. Event observations were already keyed by the unique event_id
and are unchanged.
Adds a regression test asserting two traces reusing the same call_id
produce distinct observation ids.
📝 WalkthroughWalkthroughThe Langfuse ingestion exporter now generates trace-scoped observation IDs instead of raw call_ids, preventing ID collisions across traces. Two new helper functions (scoped_observation_id, with_call_id) implement this, applied to both ModelCompleted and ToolCompleted event exports. Raw call_id is preserved in metadata. Tests updated and extended accordingly. ChangesTrace-scoped Observation IDs
Estimated code review effort: 2 (Simple) | ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1294c482c8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// is deterministic for a given (trace, call), keeping re-ingestion idempotent | ||
| /// while never colliding across turns or threads. | ||
| fn scoped_observation_id(trace_id: &str, call_id: &str) -> String { | ||
| format!("{trace_id}:{call_id}") |
There was a problem hiding this comment.
When either component can contain :, this raw delimiter join can map two different observations to the same Langfuse id, e.g. trace a:b + call c and trace a + call b:c both become a:b:c. The public trace/run/call ids are arbitrary strings, and generated model call ids include the run id, so colon-separated run or trace ids can still trigger the project-wide Langfuse upsert collision this change is meant to prevent. Please escape, length-prefix, hash, or otherwise encode the (trace_id, call_id) pair unambiguously.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/harness/observability/langfuse/mod.rs (2)
411-413: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDelimiter collision risk in
scoped_observation_id.
format!("{trace_id}:{call_id}")can produce identical strings for different(trace_id, call_id)pairs when either component contains a:. The test suite itself uses trace ids like"thread-A:turn-1"(colon-bearing), so this isn't a purely theoretical concern — e.g.trace_id="a", call_id="b:c"andtrace_id="a:b", call_id="c"both yield"a:b:c". Since this PR exists specifically to eliminate observation-id collisions, silently reintroducing a (rarer) collision vector under this scheme is worth guarding against, e.g. by escaping:in each component or using a length-prefixed encoding.🛡️ Example fix using length-prefixed encoding
-fn scoped_observation_id(trace_id: &str, call_id: &str) -> String { - format!("{trace_id}:{call_id}") -} +fn scoped_observation_id(trace_id: &str, call_id: &str) -> String { + format!("{}:{trace_id}:{call_id}", trace_id.len()) +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/harness/observability/langfuse/mod.rs` around lines 411 - 413, The scoped_observation_id helper still risks collisions because it concatenates trace_id and call_id with a raw ":" separator, so different pairs can map to the same string when either input contains a colon. Update scoped_observation_id in langfuse/mod.rs to use an unambiguous encoding such as escaping the delimiter in each component or switching to a length-prefixed format, and keep the change localized to this helper so all callers inherit the safer ID generation.
351-354: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueUnnecessary clone of
metadata.
metadata.clone()is taken here, butmetadataisn't reused afterward within theToolCompletedarm (unlike theRunFailed/default arms, which are separate match branches). This clone can be avoided by moving instead, mirroring how theModelCompletedarm at Line 306 movesmetadatadirectly intowith_call_id.♻️ Proposed fix
- let mut tool_metadata = with_call_id(metadata.clone(), call_id.as_str()); + let mut tool_metadata = with_call_id(metadata, call_id.as_str());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/harness/observability/langfuse/mod.rs` around lines 351 - 354, The ToolCompleted branch in the Langfuse observability handler is cloning `metadata` unnecessarily before calling `with_call_id`; since `metadata` is not used again in that arm, move it into `with_call_id` directly like the `ModelCompleted` arm does. Update the `tool_metadata` initialization in `src/harness/observability/langfuse/mod.rs` to avoid the extra clone while preserving the existing `output_bytes` insertion logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/harness/observability/langfuse/mod.rs`:
- Around line 411-413: The scoped_observation_id helper still risks collisions
because it concatenates trace_id and call_id with a raw ":" separator, so
different pairs can map to the same string when either input contains a colon.
Update scoped_observation_id in langfuse/mod.rs to use an unambiguous encoding
such as escaping the delimiter in each component or switching to a
length-prefixed format, and keep the change localized to this helper so all
callers inherit the safer ID generation.
- Around line 351-354: The ToolCompleted branch in the Langfuse observability
handler is cloning `metadata` unnecessarily before calling `with_call_id`; since
`metadata` is not used again in that arm, move it into `with_call_id` directly
like the `ModelCompleted` arm does. Update the `tool_metadata` initialization in
`src/harness/observability/langfuse/mod.rs` to avoid the extra clone while
preserving the existing `output_bytes` insertion logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6872afbd-9336-466c-99a9-b61ab86581be
📒 Files selected for processing (2)
src/harness/observability/langfuse/mod.rssrc/harness/observability/langfuse/test.rs
1 participant
Summary
Langfuse agent-turn traces were losing all per-turn model usage/cost/content because model generations (and tool spans) collided on their Langfuse observation
id.observation_eventset the observationidstraight fromcall_id(e.g.agent_turn-model-1).call_idis only unique within a run, and the logical run id (agent_turn) is reused by every interactive turn — so the id is identical across all turns and all threads. Langfuse upserts observations byidproject-wide, so each new turn'smodelgeneration silently overwrote the previous one and reattached it to the newest trace, leaving every earlier trace with no model usage/cost/content.Observed on a live project: 75 traces existed, but only 5
modelgenerations survived — the rest had been overwritten onto later threads.Fix
{trace_id}:{call_id}(globally unique per turn, stable for idempotent re-ingestion) for bothModelCompletedgenerations andToolCompletedspans.call_idin observation metadata so in-run correlation (model.started↔ itsmodelgeneration) is retained.event_idand are unchanged.Tests
call_scoped_observation_ids_are_unique_per_trace: two traces reusing the samecall_idnow produce distinct observation ids.builds_trace_and_generation_batchto assert the trace-scoped id +call_idin metadata.cargo test --lib observability::langfuse→ 17 passed;cargo fmt --checkclean.Commands run
Summary by CodeRabbit