Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
kalra-mohit wants to merge 1 commit into
base: main
Choose a base branch
from
+79 −2
Draft

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions apps/opik-backend/src/main/java/com/comet/opik/domain/TraceDAO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,7 @@ feedback_scores_grouped AS (
FROM span_feedback_scores_grouped
), spans_agg AS (
SELECT
project_id,
trace_id,
sumMap(usage) as usage,
sum(total_estimated_cost) as total_estimated_cost,
Expand All @@ -682,7 +683,13 @@ feedback_scores_grouped AS (
WHERE workspace_id = :workspace_id
<if(has_target_projects)>AND project_id IN :target_project_ids<endif>
AND trace_id IN :ids
GROUP BY trace_id
-- grouped per (project_id, trace_id): a trace_id may collide across
-- projects (e.g. batch-imported data), and target_project_ids can
-- legitimately contain more than one project id in that case. Without
-- grouping by project_id here, the join below would match every
-- colliding project's spans onto a single trace row and sum their
-- cost/usage together (see OPIK-7473).
GROUP BY project_id, trace_id
), experiments_agg AS (
SELECT
ei.trace_id,
Expand Down Expand Up @@ -744,7 +751,7 @@ ORDER BY (workspace_id, dataset_id, id) DESC, last_updated_at DESC
ORDER BY (workspace_id, project_id, id) DESC, last_updated_at DESC
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

@baz-reviewer baz-reviewer Bot Jul 22, 2026

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.

LEFT JOIN experiments_agg eaag ON eaag.trace_id = t.id
LEFT JOIN (
SELECT
Expand Down
70 changes: 70 additions & 0 deletions apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/TracesResourceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2378,6 +2378,76 @@ void getTrace__whenTraceDoesNotExist__thenReturnNotFound() {
var id = generator.generate();
getAndAssertTraceNotFound(id, API_KEY, TEST_WORKSPACE);
}

@Test
@DisplayName("when the same trace id exists in two different projects, then cost/usage/span "
+ "counts of the by-id response are scoped to the trace's own project, not summed across "
+ "both projects")
void getTrace__whenTraceIdCollidesAcrossProjects__thenCostAndUsageAreNotAggregatedAcrossProjects() {
// Batch endpoints don't enforce the single-create project/id consistency check, so two
// traces sharing the same id can legitimately end up in different projects (e.g. via
// deterministic client-side id generation across independent imports). See OPIK-7473.
var sharedId = generator.generate();
var projectNameA = RandomStringUtils.secure().nextAlphanumeric(10);
var projectNameB = RandomStringUtils.secure().nextAlphanumeric(10);

var traceA = createTrace().toBuilder().id(sharedId).projectName(projectNameA).build();
var traceB = createTrace().toBuilder().id(sharedId).projectName(projectNameB).build();
traceResourceClient.batchCreateTraces(List.of(traceA), API_KEY, TEST_WORKSPACE);
traceResourceClient.batchCreateTraces(List.of(traceB), API_KEY, TEST_WORKSPACE);

BigDecimal costA = new BigDecimal("0.10");
BigDecimal costB = new BigDecimal("0.15");
// Span.usage() values are Integer; the trace-level aggregate (Trace.usage()) sums them as Long.
Map<String, Integer> spanUsageA = Map.of("prompt_tokens", 3);
Map<String, Integer> spanUsageB = Map.of("prompt_tokens", 7);
Map<String, Long> traceUsageA = Map.of("prompt_tokens", 3L);
Map<String, Long> traceUsageB = Map.of("prompt_tokens", 7L);

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();
Comment on lines +2407 to +2418

@baz-reviewer baz-reviewer Bot Jul 22, 2026

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.

batchCreateSpansAndAssert(List.of(spanA), API_KEY, TEST_WORKSPACE);
batchCreateSpansAndAssert(List.of(spanB), API_KEY, TEST_WORKSPACE);

var projectIdA = getProjectId(projectNameA, TEST_WORKSPACE, API_KEY);
var projectIdB = getProjectId(projectNameB, TEST_WORKSPACE, API_KEY);

var actualTrace = traceResourceClient.getById(sharedId, TEST_WORKSPACE, API_KEY);

// Whichever project the by-id lookup resolves the trace to, its cost/usage/span count must
// match *that project's* single span only. Regardless of which project wins, the buggy
// behaviour (summing both projects' spans) is excluded by the "not equal to combined" checks
// below, and the "must equal the resolved project's own values" checks pin down correctness.
assertThat(actualTrace.projectId()).isIn(projectIdA, projectIdB);

BigDecimal expectedCost = actualTrace.projectId().equals(projectIdA) ? costA : costB;
Map<String, Long> expectedUsage = actualTrace.projectId().equals(projectIdA) ? traceUsageA : traceUsageB;

assertThat(actualTrace.totalEstimatedCost())
.usingComparator(BigDecimal::compareTo)
.isEqualTo(expectedCost);
assertThat(actualTrace.usage()).isEqualTo(expectedUsage);
assertThat(actualTrace.spanCount()).isEqualTo(1);

// The regression this guards against: before the fix, spans_agg grouped only by trace_id
// (not project_id) and was joined on trace_id alone, so a colliding trace_id summed cost,
// usage and span_count across every project sharing that id.
BigDecimal buggyAggregatedCost = costA.add(costB);
assertThat(actualTrace.totalEstimatedCost())
.usingComparator(BigDecimal::compareTo)
.isNotEqualTo(buggyAggregatedCost);
assertThat(actualTrace.spanCount()).isNotEqualTo(2);
}
}

private Trace createTrace() {
Expand Down
Loading