Skip to content

Commit e18bf10

Browse files
thiagohoraclaude
andcommitted
[OPIK-7402] [BE] Make trace batch validation symmetric with spans
bindTraceToProjectAndId validated own ids only after deleteAutoStrippedAttachments and project getOrCreate, so a bad trace id in a batch still mutated state before failing (unlike spans, which #7553 validates fail-fast). Hoist trace own-id validation into a leading deferContextual that runs before any side effect and attributes the audit metric to the request workspace, and stop re-validating in bindTraceToProjectAndId — mirroring create(SpanBatch). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 26042fc commit e18bf10

1 file changed

Lines changed: 15 additions & 4 deletions

File tree

apps/opik-backend/src/main/java/com/comet/opik/domain/TraceService.java

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,18 @@ public Mono<Long> create(TraceBatch batch) {
191191
.filter(Objects::nonNull)
192192
.collect(Collectors.toSet());
193193

194-
return attachmentService.deleteAutoStrippedAttachments(EntityType.TRACE, traceIds)
194+
// Fail fast on invalid ids BEFORE any side effect below (auto-stripped attachment deletion, project
195+
// creation), so a rejected batch never mutates state. Runs inside deferContextual so the audit
196+
// metric can attribute the batch's own ids to the request workspace.
197+
return Mono.deferContextual(validationCtx -> {
198+
String validationWorkspaceId = validationCtx.get(RequestContext.WORKSPACE_ID);
199+
dedupedTraces.forEach(trace -> {
200+
if (trace.id() != null) {
201+
idGenerator.validateId(trace.id(), TRACE_KEY, validationWorkspaceId);
202+
}
203+
});
204+
return attachmentService.deleteAutoStrippedAttachments(EntityType.TRACE, traceIds);
205+
})
195206
.then(Mono.deferContextual(ctx -> {
196207
String workspaceId = ctx.get(RequestContext.WORKSPACE_ID);
197208
String workspaceName = ctx.getOrDefault(RequestContext.WORKSPACE_NAME, "");
@@ -200,7 +211,7 @@ public Mono<Long> create(TraceBatch batch) {
200211
Mono<List<Trace>> resolveProjects = Flux.fromIterable(projectNames)
201212
.flatMap(projectService::getOrCreate)
202213
.collectList()
203-
.map(projects -> bindTraceToProjectAndId(dedupedTraces, projects, workspaceId))
214+
.map(projects -> bindTraceToProjectAndId(dedupedTraces, projects))
204215
.flatMapMany(Flux::fromIterable)
205216
.flatMap(trace -> attachmentStripperService.stripAttachments(trace, workspaceId,
206217
userName,
@@ -237,7 +248,7 @@ private List<Trace> dedupTraces(List<Trace> initialTraces) {
237248
return result;
238249
}
239250

240-
private List<Trace> bindTraceToProjectAndId(List<Trace> traces, List<Project> projects, String workspaceId) {
251+
private List<Trace> bindTraceToProjectAndId(List<Trace> traces, List<Project> projects) {
241252
Map<String, Project> projectPerName = projects.stream()
242253
.collect(Collectors.toMap(
243254
WorkspaceUtils::stripProjectName,
@@ -251,8 +262,8 @@ private List<Trace> bindTraceToProjectAndId(List<Trace> traces, List<Project> pr
251262
String projectName = WorkspaceUtils.getProjectName(trace.projectName());
252263
Project project = projectPerName.get(projectName);
253264

265+
// Ids are already validated up-front in create(TraceBatch); generated ids are inherently valid.
254266
UUID id = trace.id() == null ? idGenerator.generateId() : trace.id();
255-
idGenerator.validateId(id, TRACE_KEY, workspaceId);
256267

257268
return trace.toBuilder().id(id).projectId(project.id()).projectName(project.name()).build();
258269
})

0 commit comments

Comments
 (0)