Skip to content

fix(BE): scope trace cost/usage aggregation to the resolved project on GET /traces/{id}#7561

Draft
kalra-mohit wants to merge 1 commit into
comet-ml:mainfrom
kalra-mohit:fix/trace-cost-cross-project-collision
Draft

fix(BE): scope trace cost/usage aggregation to the resolved project on GET /traces/{id}#7561
kalra-mohit wants to merge 1 commit into
comet-ml:mainfrom
kalra-mohit:fix/trace-cost-cross-project-collision

Conversation

@kalra-mohit

@kalra-mohit kalra-mohit commented Jul 22, 2026

Copy link
Copy Markdown

What

GET /v1/private/traces/{id} inflates total_estimated_cost, usage, span_count, llm_span_count, has_tool_spans and providers whenever the same trace_id exists in more than one project in the workspace.

This can legitimately happen in practice: the batch traces/spans endpoints (unlike the single-create endpoint) don't reject a trace_id that already exists under a different project_id, so deterministic/replayed client-side ID generation across independent imports can end up sharing an id across projects (see the reproducer and discussion on #7473).

Root cause

TraceDAO#findByIds first resolves every project_id that contains a matching trace_id via getTargetProjectIdsForTraces, and binds that whole set as target_project_ids.

The trace row itself is then correctly narrowed down to a single project (ORDER BY (workspace_id, project_id, id) DESC, last_updated_at DESC LIMIT 1 BY id). But the spans_agg CTE only grouped by trace_id (not project_id), and was LEFT JOINed onto the trace row using t.id = s.trace_id alone — so it summed total_estimated_cost/usage/span counts across every colliding project's spans, and attached that sum to the single resolved trace row, regardless of which project was actually requested.

Fix

Group spans_agg by (project_id, trace_id) and join it to the resolved trace row on both id and project_id, so the aggregate always matches the specific project the trace row belongs to.

This is a SQL-only, backward-compatible change — no API or schema changes, and it applies uniformly to both callers of findByIds (single GET /traces/{id} and the internal batch fetch path), since a trace_id collision across projects can affect either.

Note on why this one was silent: the codebase already has a defensive firstOrLogFanOut in TraceDAO#findById for CTEs (comments, feedback scores) that can fan a single trace id out into multiple joined rows across colliding projects — it logs a warning and returns the first row rather than throwing. spans_agg doesn't hit that path at all, because it pre-aggregates (sum/sumMap) down to one row before the join, so the cross-project blend happens invisibly inside the aggregate itself rather than as a detectable multi-row fan-out. That's why this surfaced as silently wrong data instead of a logged warning or a 500.

Testing

Added TracesResourceTest$GetTrace#getTrace__whenTraceIdCollidesAcrossProjects__thenCostAndUsageAreNotAggregatedAcrossProjects, which:

  • Creates the same trace id in two different projects via the batch trace-create endpoint (mirroring how the collision occurs in practice).
  • Creates one span per project with distinct, known totalEstimatedCost/usage values.
  • Calls GET /traces/{id} and asserts the response's cost/usage/span_count match only the resolved project's own span — not the cross-project sum.

Verified explicitly:

  • Against the unfixed query, the test fails with expected: 0.15 but was: 0.250000000000 (the exact cross-project sum of the two spans' costs) — reproducing the reported bug.
  • With the fix, the test passes.
  • The existing TracesResourceTest$GetTrace (6 tests) and TracesResourceTest$BatchInsert (22 tests) suites pass in full with the fix applied.
  • mvn spotless:check passes.

Fixes #7473

🤖 Generated with Claude Code

…n GET /traces/{id}

GET /v1/private/traces/{id} inflates total_estimated_cost, usage,
span_count, llm_span_count, has_tool_spans and providers whenever the
same trace_id exists in more than one project in the workspace (e.g.
data imported via the batch traces/spans endpoints, which unlike the
single-create endpoint do not reject a trace_id/project mismatch).

Root cause: TraceDAO#findByIds first resolves every project_id that
contains a matching trace_id (getTargetProjectIdsForTraces) and binds
that whole set as target_project_ids. The trace row itself is then
correctly narrowed to one project via `LIMIT 1 BY id`, but the
spans_agg CTE only grouped by trace_id (not project_id) and was
LEFT JOINed on trace_id alone - so it summed cost/usage/counts across
every colliding project's spans and attached that sum to the single
resolved trace row, regardless of which project was requested.

Fix: group spans_agg by (project_id, trace_id) and join it to the
resolved trace row on both id and project_id, so the aggregate always
matches the specific project the trace row belongs to. This is a
SQL-only, backward-compatible change - no API or schema changes.

Adds a regression test that creates the same trace id in two projects
via the batch endpoints (mirroring how this occurs in practice), and
asserts the by-id response's cost/usage/span_count match only the
resolved project's own span, not the cross-project sum. Verified the
test fails against the unfixed query (returns the summed cost) and
passes after the fix.

Fixes comet-ml#7473

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions github-actions Bot added java Pull requests that update Java code Backend tests Including test files, or tests related like configuration. labels Jul 22, 2026
Comment on lines +2407 to +2418
var spanA = factory.manufacturePojo(Span.class).toBuilder()
.projectName(projectNameA)
.traceId(sharedId)
.usage(spanUsageA)
.totalEstimatedCost(costA)
.build();
var spanB = factory.manufacturePojo(Span.class).toBuilder()
.projectName(projectNameB)
.traceId(sharedId)
.usage(spanUsageB)
.totalEstimatedCost(costB)
.build();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing aggregate regression coverage

According to the PR description, this fix should also prevent cross-project inflation of llm_span_count, has_tool_spans, and providers, but this test only pins totalEstimatedCost, usage, and spanCount, so a regression in the rest of spans_agg would still pass — should we set explicit type/provider values on spanA and spanB and assert llmSpanCount, hasToolSpans, and providers on actualTrace too?

Severity

Want Baz to fix this for you? Activate Fixer

Other fix methods

Fix in Cursor

Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/TracesResourceTest.java
around lines 2407-2449, inside
`getTrace__whenTraceIdCollidesAcrossProjects__thenCostAndUsageAreNotAggregatedAcrossProjects`,
make the test deterministic by explicitly setting `type` and `provider` on `spanA` and
`spanB` (instead of relying on factory-generated values). Then, after fetching
`actualTrace`, add assertions for the additional regression fields from the PR
description: `llmSpanCount`, `hasToolSpans`, and `providers`, with expected values
scoped to whichever `projectId` the by-id resolution picks (same conditional logic you
already use for `expectedCost`/`expectedUsage`). Finally, remove or extend only the
cost/usage/spanCount checks if needed, but ensure the test fails if spans aggregation
incorrectly inflates across both projects for those fields too.

LIMIT 1 BY id
) AS t
LEFT JOIN spans_agg s ON t.id = s.trace_id
LEFT JOIN spans_agg s ON t.id = s.trace_id AND t.project_id = s.project_id

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cross-project metadata leak persists

GET /v1/private/traces/{id} can still mix comments, feedback_scores, span_feedback_scores, and guardrails across projects because those joins only use t.id/trace_id, so a shared trace id can surface the wrong metadata in the single-trace response — should we carry project_id through them too?

Severity

Want Baz to fix this for you? Activate Fixer

Other fix methods

Fix in Cursor

Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
apps/opik-backend/src/main/java/com/comet/opik/domain/TraceDAO.java around lines 754-846
inside the SELECT_DETAILS_BY_ID query (the SQL behind GET /v1/private/traces/{id}), the
LEFT JOINs for comments (AS c), feedback_scores (AS fs), span_feedback_scores (AS sfs),
and guardrails (AS gr) join only on t.id / trace_id and not on project_id, so colliding
trace ids can mix metadata from other projects. Refactor each of those subqueries to
include project_id in the projected columns and to GROUP BY workspace_id, project_id,
and the entity key (entity_id/trace_id), then update the outer join predicates to match
both t.id = <entity_id/trace_id> and t.project_id = <project_id>. Keep the existing
spans_agg join change as the model, and confirm the final single-trace response uses
metadata exclusively from the same project as the selected trace row.

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

Labels

Backend baz: pending java Pull requests that update Java code 🟢 size/S tests Including test files, or tests related like configuration.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Cost overcounted when trace_id values shared between projects

1 participant