-
[OPIK-7483] [BE] fix: eliminate project-less trace deletes (#7673)
frostbyte_neo 发布于
2026-07-31 09:57:41 +00:00 - [OPIK-7483] [BE] fix: eliminate project-less trace deletes
The null-project trace delete path (DELETE /traces/{id} and POST /traces/delete
without a project_id) resolved each id to any(project_id) over a partition-bounded
window, then fell back to a project-less DELETE (WHERE id, workspace_id) for ids it
could not resolve. Under the (workspace_id, project_id, id) ReplacingMergeTree dedup
key that fallback ignored two-thirds of the key: a reused id could be silently
over-deleted across every project of the workspace, and the deletion-events bridge
recorded empty-project_id events.Resolve all owning projects instead of any(): a bounded (partition-pruned) fast pass
plus an unbounded second pass on only the ids it leaves unresolved, so present rows
the bounded window can miss (malformed ids whose id_at is not monotonic in id, e.g. a
wrapped timestamp) are still found with their project. Delete each id under the full
key, once per project group, so a reused id is removed from all its projects and the
per-project span/feedback cascade is correctly scoped. Drop the project-less fallback;
ids that resolve to no project have no live row and are skipped. The DAO delete now
always filters the full (workspace_id, project_id, id) key, so no delete can be
project-less and none writes an empty-project_id bridge event.Tests: unit coverage for per-project grouping, the unbounded fallback, and skipping
absent ids; integration coverage that delete-by-id of a reused id clears it from all
projects and captures one deletion event per owning project (never empty).- test(traces): drop unit tests already covered by integration tests
The reused-id per-project delete and the skip-absent-ids behavior are both
verified end-to-end (TracesResourceTest.deleteByIdOfReusedIdRemovesItFromEveryProject- DeletionEventTest.deleteByIdOfReusedIdCapturesEventPerOwningProject, and
delete__whenTraceDoesNotExist / deleteTracesWithoutTraces). Keep only the
bounded-miss -> unbounded-fallback unit test, which integration cannot trigger
without raw-inserting a crafted wrapped-id_at row and which guards against the
fallback being dropped.
- fix(traces): reject null trace ids in batch delete at the API boundary
BatchDeleteByProject validated the ids set but not its elements, so a
POST /traces/delete with a null element (e.g. {"ids":[null]}) reached
Collections.min/max during project resolution and failed with an NPE (500).
Constrain the container element (Set<@NotNull UUID>) so the request is
rejected with 422 at the boundary. Addresses Baz PR review (OPIK-7483).- test(traces): dedupe delete client overloads and assert validation body
- Refactor TraceResourceClient.deleteTraces to delegate through a shared
callDeleteTraces(...) (mirrors the updateTrace/callUpdateTrace pattern)
instead of duplicating the request build across the void and
expected-status overloads. - Assert the 422 body on the null-id batch-delete test, not just the status.
- docs(traces): record delete-path decisions from review
- delete(): note that skipping ids with no live trace row is a no-op and that
orphaned child rows of a never-existent trace are the child entities' own
lifecycle concern, not this endpoint's. - resolveOwningProjects(): note the miss set is dominated by absent ids (the
unbounded pass finds nothing there); its real job is the rare malformed id,
and the fallback can be removed once traces is partitioned.
- perf(traces): index-pruned resolution, pair-batched deletes, project-scoped cascade
Follow-up refinements to the project-less trace-delete fix, on top of the base
commits:- Add an id bloom-filter skip index on traces (migration 000113, mirroring
traces_local_v2) so both owning-project resolver passes prune granules by id
instead of scanning the workspace. - Delete by bound (project_id, id) pairs in a single tuple-IN statement per batch
(arrayZip), collapsing the per-project delete loop into one statement; the DAO
contract becomes delete(Set, Connection). - Scope the delete cascade uniformly by project: comments and attachments now
filter by project_id/container_id like spans and feedback, so deleting a reused
id from one project no longer over-deletes another project's children. - Skip ids that resolve to no owning project instead of emitting a project-less
TracesDeleted - that cascade was an unscoped, workspace-wide child delete that
could over-delete a concurrently-ingested trace's children. - Document the optional project_id on the batch-delete OpenAPI schema.
- Null-safe CollectionUtils guards, immutable result sets, javadoc tightening,
rename to projectIdTraceIdPairs, and inline the single-use event helper. - Tests: two-pass unbounded-fallback unit test, mixed present/absent batch,
public-API cross-project attachment scoping, and the arch guard on the new
delete signature.
- refactor(traces): DRY attachment DAO binds, document delete 422
Addresses PR review:
- Extract AttachmentDAO.bindEntityIdsStatement to share the template render +
bind flow between getAttachmentsByEntityIds and deleteByEntityIds, so the two
paths can't drift on bind order or the optional-container wiring (also
normalizes the SELECT's entity_ids bind to a UUID[], matching the DELETE). - Document the 422 (Unprocessable Content) response on POST /traces/delete for a
null id element in the OpenAPI @Operation.
下载附件