[OPIK-7402] [BE] Add audit (shadow / log-only) mode to UUIDv7 ingestion validation#7573
Open
thiagohora wants to merge 7 commits into
Open
[OPIK-7402] [BE] Add audit (shadow / log-only) mode to UUIDv7 ingestion validation#7573thiagohora wants to merge 7 commits into
thiagohora wants to merge 7 commits into
Conversation
…on validation Adds a third `auditOnly` state to uuidValidation on top of the `enabled` kill-switch. When enabled=true and auditOnly=true, out-of-window UUIDv7 ids are counted and logged but NOT rejected, so clients emitting them surface in real time without breaking ingestion. Effective mode: enabled=false -> disabled; enabled=true & auditOnly=true -> audit; enabled=true & auditOnly=false -> reject (HTTP 400, unchanged). - New UuidValidationMetrics records opik.ingestion.uuid_v7.rejected in audit mode, tagged mode=audit + reason + resource + workspace_id (shares the instrument with InvalidUUIDExceptionMapper's reject-path counter). - Validator gains resource/workspaceId params; audit branch emits + logs instead of throwing. Scope is the window check; the always-on NOT_V7 version check is unchanged. - workspaceId is read from the reactive context on the async paths and threaded through the batch bind on the sync path. - Unit test covers disabled/reject/audit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
java
Contributor
⏱️ pre-commit per-hook timing
⏭️ 40 skipped (no matching files changed)
|
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
…ounter Address review: InvalidUUIDExceptionMapper recorded opik.ingestion.uuid_v7.rejected without a mode label, so the shared counter had series with and without mode and a `mode=reject` query would miss enforced rejections. Add mode=reject on the reject path (workspace_id stays audit-only, as the mapper has no threaded workspace), and align the UuidValidationMetrics contract javadoc: mode is always present; workspace_id is audit-only; the reject path additionally carries http_route. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…mmed javadoc - Add IdGeneratorAsyncValidationTest covering validateIdAsync / validateIdForUpdateAsync reject+audit behavior, with and without RequestContext.WORKSPACE_ID in the reactive context (the deferContextual workspace lookup + UNKNOWN fallback). - Keep a fixed, searchable prefix on the audit log line and move the variable fields to the end. - Trim the IdGenerator#validateId javadoc to validation semantics + InvalidUUIDException; keep workspaceId threading (the sync/batch path is the primary ingestion route where per-workspace attribution is the goal). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s://github.com/comet-ml/opik into thiaghora/OPIK-7402-uuid-validation-audit-mode
…d validation Resolve the semantic overlap after merging main (#7553): unify IdGenerator on the merged API — implement validateIdNotInFuture / validateIdNotInFutureAsync (and the IfPresent variants) and drop the superseded validateIdForUpdate*. Own-id validateId stays workspace-tagged for the audit metric; referenced-id validateIdNotInFuture falls back to UNKNOWN on the sync path and resolves the workspace from the reactive context on the async path. Keep #7553's fail-fast (validate before side effects) for span batches, now run inside deferContextual so the batch's own ids attribute to the request workspace; bindSpanToProjectAndId no longer re-validates. Trace batches keep validating in bindTraceToProjectAndId with the workspace threaded through. Update the async test to the renamed methods. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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>
| @@ -251,8 +262,8 @@ private List<Trace> bindTraceToProjectAndId(List<Trace> traces, List<Project> pr | |||
| String projectName = WorkspaceUtils.getProjectName(trace.projectName()); | |||
Contributor
There was a problem hiding this comment.
Duplicated project-binding logic
bindTraceToProjectAndId(...) repeats the same project-map lookup and id assignment flow as SpanService.bindSpanToProjectAndId(...), so the project-resolution and id-generation rules live in two places and any change needs duplicate edits — should we factor out a shared binder that takes the entity-specific copy step?
Want Baz to fix this for you? Activate Fixer
Comment on lines
+110
to
+114
| // Keep a fixed, searchable prefix ("UUIDv7 audit: would-reject id ...") and append the | ||
| // variable fields at the end, so log searches match on the message rather than the values. | ||
| log.info( | ||
| "UUIDv7 audit: would-reject id, embedded timestamp '{}' outside window '{}', reason '{}', resource '{}', workspace '{}'", | ||
| timestamp, window, reason.getValue(), resource, workspaceId); |
Contributor
There was a problem hiding this comment.
Missing offender correlation
This audit log records timestamp, window, reason, resource, and workspaceId but not the offending UUID, so repeated UUIDv7 audit: would-reject ... entries can't be tied back to a specific bad payload — should we add the actual id to the template/arguments while keeping the fixed prefix?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
apps/opik-backend/src/main/java/com/comet/opik/infrastructure/db/UuidV7TimestampValidator.java
around lines 110-114, in the audit log statement used when rejecting an invalid UUIDv7
(the block that starts with “UUIDv7 audit: would-reject id ...”), the message
template does not include the actual offending `id` value. Refactor the log
template/arguments to append the passed-in UUID `id` to the end (keeping the fixed
“UUIDv7 audit:” prefix unchanged) and add the corresponding placeholder argument so
the log can be tied back to a specific bad payload even when multiple invalid ids share
the same embedded timestamp.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Details
Adds a third audit (shadow / log-only) state to
uuidValidation, on top of the existingenabledkill-switch. Whenenabled=trueandauditOnly=true, ids whose embedded UUIDv7 timestamp is out of the window are counted and logged but not rejected, so clients emitting them surface in real time without breaking ingestion. This is the freshness mechanism for safely re-enabling rejection later.enabled=false→ disabled (no-op);enabled=true & auditOnly=true→ audit;enabled=true & auditOnly=false→ reject (HTTP 400, unchanged).opik.ingestion.uuid_v7.rejected(mode=audit) tagged byworkspace_id+reason+resource, so offenders can be broken down per workspace. It shares the counter/instrument with the reject-path mapper.enabled-gated one). The always-on NOT_V7 version check is unchanged.workspace_idis read from the reactive context on the async paths and threaded through the batch bind on the sync path;project_idis not available at this layer and is left as a follow-up. Reject-path workspace tagging is likewise a follow-up.Change checklist
Issues
AI-WATERMARK
AI-WATERMARK: yes
auditOnlyflag, scope limited to the window check, workspace-only tagging) chosen by the author;mvn test-compileand the new unit test run and green locally; diff reviewed.Testing
mvn test-compile— BUILD SUCCESS (main + tests).mvn surefire:test -Dtest=UuidV7TimestampValidatorTest— 5 tests, 0 failures. Covers all three modes: disabled (accepts out-of-window ids), reject (accepts in-window; rejects too-old/too-far-future on create, only too-far-future on update), audit (never rejects, tolerates a blank workspace id).Documentation
Config documented inline in
config.yml/config-test.yml(newauditOnlykey, effective-mode table) and in theUuidValidationConfig/UuidV7TimestampValidator/UuidValidationMetricsjavadoc. No user-facing docs change.