Skip to content

Commit c76dff8

Browse files
thiagohoraclaude
andauthored
[OPIK-7402] [BE] Add audit (shadow / log-only) mode to UUIDv7 ingestion validation (#7573)
* [OPIK-7402] [BE] Add audit (shadow / log-only) mode to UUIDv7 ingestion 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> * [OPIK-7402] [BE] Tag the reject path with mode=reject on the shared counter 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> * [OPIK-7402] [BE] Address review: async test coverage, log prefix, trimmed 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> * [OPIK-7402] [BE] Reconcile audit mode with merged OPIK-7352 foreign-id 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> * [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> * [OPIK-7402] [BE] Address review: centralize counter, simplify async, tidy nits - Centralize the opik.ingestion.uuid_v7.rejected counter and its label constants in UuidValidationMetrics; InvalidUUIDExceptionMapper now injects it and delegates via recordReject(reason, httpRoute), dropping its duplicate counter/constants. - Drop the redundant nested Mono.fromCallable inside deferContextual on the async validation paths (validate has no blocking work and there is no subscribeOn, so it ran on the subscribing thread either way; deferContextual already maps a thrown exception to onError). - Let resource fall back to unknown like workspaceId (relax @nonnull); the metric recorder defaults blank values. - Test: make the id generators static final and rename to constant case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * [OPIK-7402] [BE] Review: centralize IdGenerator + workspace overloads; factor metric assembly - IdGenerator: single core validateIdNotInFuture(id, resource, workspaceId); the 2-arg form and the async form delegate to it (no more duplicated version+window logic), and a workspace-parameterized overload is offered for callers that carry the request workspace. Same for validateIdNotInFutureIfPresent. The many config-entity callers keep the 2-arg form, defaulting to unknown by choice. - UuidValidationMetrics: factor the shared counter/mode/reason assembly into a private record(...) helper; recordAudit/recordReject only supply their path-specific tags. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1d6f99b commit c76dff8

12 files changed

Lines changed: 487 additions & 60 deletions

File tree

apps/opik-backend/config.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,13 @@ uuidValidation:
145145
# Description: Operational kill-switch. When false, ids are not checked against the window (the
146146
# window is still validated to be a sane value).
147147
enabled: ${UUID_VALIDATION_ENABLED:-false}
148+
# Default: false
149+
# Description: Audit (shadow / log-only) mode. Only takes effect when enabled is true. When true,
150+
# out-of-window ids are counted (opik.ingestion.uuid_v7.rejected, tagged by workspace) and logged
151+
# but NOT rejected, so offenders surface without breaking ingestion. When false, out-of-window ids
152+
# are rejected with HTTP 400. Effective mode: enabled=false -> disabled; enabled=true & auditOnly=true
153+
# -> audit; enabled=true & auditOnly=false -> reject.
154+
auditOnly: ${UUID_VALIDATION_AUDIT_ONLY:-false}
148155
# Default: 24h
149156
# Description: Validation window (between 12h and 45d). Writes whose `id` is a UUIDv7 with an
150157
# embedded timestamp more than this far in the past or future are rejected.

apps/opik-backend/src/main/java/com/comet/opik/api/error/InvalidUUIDExceptionMapper.java

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
package com.comet.opik.api.error;
22

3+
import com.comet.opik.infrastructure.metrics.UuidValidationMetrics;
34
import io.dropwizard.jersey.errors.ErrorMessage;
4-
import io.opentelemetry.api.GlobalOpenTelemetry;
5-
import io.opentelemetry.api.common.AttributeKey;
6-
import io.opentelemetry.api.common.Attributes;
7-
import io.opentelemetry.api.metrics.LongCounter;
85
import jakarta.inject.Inject;
96
import jakarta.inject.Provider;
107
import jakarta.ws.rs.Path;
@@ -21,8 +18,10 @@
2118
import static jakarta.ws.rs.core.Response.Status.BAD_REQUEST;
2219

2320
/**
24-
* Maps {@link InvalidUUIDException} to HTTP 400 and records the reject-rate metric.
25-
* Tags each rejection with the matched route.
21+
* Maps {@link InvalidUUIDException} to HTTP 400 and records the reject-rate metric, tagged with the
22+
* matched route and {@code mode=reject}. The counter itself lives in {@link UuidValidationMetrics}
23+
* (the central owner of {@code opik.ingestion.uuid_v7.rejected}); this mapper only supplies the route
24+
* and delegates.
2625
*
2726
* <p>{@link ResourceInfo} is request-scoped, so it is obtained lazily through a {@link Provider}
2827
* (injecting it directly would fail to construct this singleton outside a request). The lookup resolves
@@ -32,26 +31,17 @@
3231
@RequiredArgsConstructor(onConstructor_ = @Inject)
3332
public class InvalidUUIDExceptionMapper implements ExceptionMapper<InvalidUUIDException> {
3433

35-
private static final String METRIC_NAMESPACE = "opik.ingestion";
3634
private static final String UNKNOWN_ROUTE = "unknown";
3735

38-
private static final AttributeKey<String> HTTP_ROUTE_KEY = AttributeKey.stringKey("http_route");
39-
private static final AttributeKey<String> REASON_KEY = AttributeKey.stringKey("reason");
40-
41-
private static final LongCounter REJECTED_COUNTER = GlobalOpenTelemetry.get().getMeter(METRIC_NAMESPACE)
42-
.counterBuilder("%s.uuid_v7.rejected".formatted(METRIC_NAMESPACE))
43-
.setDescription("Number of writes rejected because the id failed UUIDv7 ingestion validation")
44-
.build();
45-
4636
private final Provider<ResourceInfo> resourceInfo;
37+
private final UuidValidationMetrics uuidValidationMetrics;
4738

4839
@Override
4940
public Response toResponse(@NonNull InvalidUUIDException exception) {
5041
var httpRoute = getHttpRoute();
5142
log.info("Rejected ingestion id, httpRoute: '{}', reason: '{}', error message: '{}'",
5243
httpRoute, exception.getReason().getValue(), exception.getMessage());
53-
REJECTED_COUNTER.add(1,
54-
Attributes.of(HTTP_ROUTE_KEY, httpRoute, REASON_KEY, exception.getReason().getValue()));
44+
uuidValidationMetrics.recordReject(exception.getReason().getValue(), httpRoute);
5545
// Force JSON: ingestion endpoints negotiate other content types (e.g. the OTel endpoint uses protobuf),
5646
// which have no writer for the error entity — without this the 400 fails to serialize and surfaces as a 500
5747
return Response.status(BAD_REQUEST)

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

Lines changed: 49 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22

33
import com.comet.opik.api.error.InvalidUUIDException;
44
import com.comet.opik.api.error.InvalidUUIDException.Reason;
5+
import com.comet.opik.infrastructure.auth.RequestContext;
56
import com.comet.opik.infrastructure.db.UuidV7TimestampValidator;
7+
import com.comet.opik.infrastructure.metrics.ErrorMetricsResolver;
68
import com.fasterxml.uuid.Generators;
79
import com.fasterxml.uuid.impl.TimeBasedEpochGenerator;
810
import com.google.inject.ImplementedBy;
@@ -11,6 +13,7 @@
1113
import lombok.NonNull;
1214
import lombok.RequiredArgsConstructor;
1315
import reactor.core.publisher.Mono;
16+
import reactor.util.context.ContextView;
1417

1518
import java.time.Instant;
1619
import java.util.UUID;
@@ -25,11 +28,12 @@ public interface IdGenerator {
2528
UUID getTimeOrderedEpoch(long epochMilli);
2629

2730
/**
28-
* Validates an ingested {@code id}: it must be a version 7 UUID ({@link #validateVersion(UUID, String)})
29-
* whose embedded timestamp is within the configured ingestion window.
30-
* Rejects with HTTP 400. Encapsulates both data-quality checks behind one call.
31+
* Validates an ingested {@code id}: it must be a version 7 UUID
32+
* ({@link #validateVersion(UUID, String)}) whose embedded timestamp is within the configured
33+
* ingestion window, throwing {@link InvalidUUIDException} otherwise. {@code workspaceId} attributes
34+
* the source workspace for observability; pass {@link ErrorMetricsResolver#UNKNOWN} when unavailable.
3135
*/
32-
void validateId(UUID id, String resource);
36+
void validateId(UUID id, String resource, String workspaceId);
3337

3438
Mono<UUID> validateIdAsync(UUID id, String resource);
3539

@@ -46,6 +50,13 @@ public interface IdGenerator {
4650
*/
4751
void validateIdNotInFuture(UUID id, String resource);
4852

53+
/**
54+
* Workspace-attributed overload of {@link #validateIdNotInFuture(UUID, String)}: callers that know the
55+
* request workspace pass it so the audit metric is attributed. The 2-arg form defaults to
56+
* {@link ErrorMetricsResolver#UNKNOWN} for callers that don't carry a workspace.
57+
*/
58+
void validateIdNotInFuture(UUID id, String resource, String workspaceId);
59+
4960
Mono<UUID> validateIdNotInFutureAsync(UUID id, String resource);
5061

5162
/**
@@ -54,6 +65,9 @@ public interface IdGenerator {
5465
*/
5566
void validateIdNotInFutureIfPresent(UUID id, String resource);
5667

68+
/** Workspace-attributed overload of {@link #validateIdNotInFutureIfPresent(UUID, String)}. */
69+
void validateIdNotInFutureIfPresent(UUID id, String resource, String workspaceId);
70+
5771
Mono<UUID> validateIdNotInFutureIfPresentAsync(UUID id, String resource);
5872

5973
static Mono<UUID> validateVersionAsync(@NonNull UUID id, String resource) {
@@ -93,40 +107,62 @@ public UUID getTimeOrderedEpoch(long epochMilli) {
93107
}
94108

95109
@Override
96-
public void validateId(@NonNull UUID id, String resource) {
110+
public void validateId(@NonNull UUID id, String resource, String workspaceId) {
97111
IdGenerator.validateVersion(id, resource);
98-
uuidV7TimestampValidator.validate(id);
112+
uuidV7TimestampValidator.validate(id, resource, workspaceId);
99113
}
100114

101115
@Override
102116
public Mono<UUID> validateIdAsync(@NonNull UUID id, String resource) {
103-
return Mono.fromCallable(() -> {
104-
validateId(id, resource);
105-
return id;
117+
return Mono.deferContextual(ctx -> {
118+
validateId(id, resource, workspaceId(ctx));
119+
return Mono.just(id);
106120
});
107121
}
108122

109123
@Override
110124
public void validateIdNotInFuture(@NonNull UUID id, String resource) {
125+
// Callers that don't carry a workspace (most config-entity references) default to UNKNOWN.
126+
validateIdNotInFuture(id, resource, ErrorMetricsResolver.UNKNOWN);
127+
}
128+
129+
@Override
130+
public void validateIdNotInFuture(@NonNull UUID id, String resource, String workspaceId) {
111131
IdGenerator.validateVersion(id, resource);
112-
uuidV7TimestampValidator.validateNotInFuture(id);
132+
uuidV7TimestampValidator.validateNotInFuture(id, resource, workspaceId);
113133
}
114134

115135
@Override
116136
public Mono<UUID> validateIdNotInFutureAsync(@NonNull UUID id, String resource) {
117-
return Mono.fromCallable(() -> {
118-
validateIdNotInFuture(id, resource);
119-
return id;
137+
return Mono.deferContextual(ctx -> {
138+
validateIdNotInFuture(id, resource, workspaceId(ctx));
139+
return Mono.just(id);
120140
});
121141
}
122142

143+
/**
144+
* Reads the {@code workspace_id} from the reactive context (the async ingestion paths carry it
145+
* there, not in a request-scoped thread-local), falling back to {@link ErrorMetricsResolver#UNKNOWN}
146+
* so the audit metric always has a value.
147+
*/
148+
private static String workspaceId(ContextView ctx) {
149+
return ctx.getOrDefault(RequestContext.WORKSPACE_ID, ErrorMetricsResolver.UNKNOWN);
150+
}
151+
123152
@Override
124153
public void validateIdNotInFutureIfPresent(UUID id, String resource) {
125154
if (id != null) {
126155
validateIdNotInFuture(id, resource);
127156
}
128157
}
129158

159+
@Override
160+
public void validateIdNotInFutureIfPresent(UUID id, String resource, String workspaceId) {
161+
if (id != null) {
162+
validateIdNotInFuture(id, resource, workspaceId);
163+
}
164+
}
165+
130166
@Override
131167
public Mono<UUID> validateIdNotInFutureIfPresentAsync(UUID id, String resource) {
132168
return id == null ? Mono.empty() : validateIdNotInFutureAsync(id, resource);

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

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -393,15 +393,6 @@ public Mono<Long> create(@NonNull SpanBatch batch) {
393393

394394
List<Span> dedupedSpans = dedupSpans(batch.spans());
395395

396-
// Fail fast on invalid ids BEFORE any side effect below (auto-stripped attachment deletion, project
397-
// creation), so a rejected batch never mutates state.
398-
dedupedSpans.forEach(span -> {
399-
if (span.id() != null) {
400-
idGenerator.validateId(span.id(), SPAN_KEY);
401-
}
402-
validateSpanReferences(span.traceId(), span.parentSpanId());
403-
});
404-
405396
List<String> projectNames = dedupedSpans
406397
.stream()
407398
.map(Span::projectName)
@@ -419,7 +410,19 @@ public Mono<Long> create(@NonNull SpanBatch batch) {
419410
.filter(Objects::nonNull)
420411
.collect(Collectors.toSet());
421412

422-
return attachmentService.deleteAutoStrippedAttachments(SPAN, spanIds)
413+
// Fail fast on invalid ids BEFORE any side effect below (auto-stripped attachment deletion, project
414+
// creation), so a rejected batch never mutates state. Runs inside deferContextual so the audit
415+
// metric can attribute the batch's own ids to the request workspace.
416+
return Mono.deferContextual(validationCtx -> {
417+
String validationWorkspaceId = validationCtx.get(RequestContext.WORKSPACE_ID);
418+
dedupedSpans.forEach(span -> {
419+
if (span.id() != null) {
420+
idGenerator.validateId(span.id(), SPAN_KEY, validationWorkspaceId);
421+
}
422+
validateSpanReferences(span.traceId(), span.parentSpanId());
423+
});
424+
return attachmentService.deleteAutoStrippedAttachments(SPAN, spanIds);
425+
})
423426
.then(Mono.deferContextual(ctx -> {
424427
String workspaceId = ctx.get(RequestContext.WORKSPACE_ID);
425428
String workspaceName = ctx.getOrDefault(RequestContext.WORKSPACE_NAME, "");
@@ -500,9 +503,8 @@ private List<Span> bindSpanToProjectAndId(List<Span> spans, List<Project> projec
500503
throw new IllegalStateException("Project not found: %s".formatted(span.projectName()));
501504
}
502505

506+
// Ids are already validated up-front in create(SpanBatch); generated ids are inherently valid.
503507
UUID id = span.id() == null ? idGenerator.generateId() : span.id();
504-
idGenerator.validateId(id, SPAN_KEY);
505-
// trace/parent references are validated up front in create(SpanBatch) before side effects.
506508

507509
return span.toBuilder().id(id).projectId(project.id()).build();
508510
})

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

Lines changed: 13 additions & 2 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, "");
@@ -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);
256267

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

apps/opik-backend/src/main/java/com/comet/opik/infrastructure/UuidValidationConfig.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,17 @@
1414
* <p>{@code enabled} is an operational kill-switch: when {@code false}, ids are not checked against
1515
* {@code window}. {@code window} bounds the embedded-timestamp distance from now, so a misbehaving
1616
* client can't land a row in a far-future partition.
17+
*
18+
* <p>{@code auditOnly} adds a third, shadow state on top of the {@code enabled} switch. It only takes
19+
* effect when {@code enabled} is {@code true}: instead of rejecting out-of-window ids, the validator
20+
* records the reject-rate metric (tagged by workspace) and logs, but lets the write through. This
21+
* surfaces offending clients in real time without breaking ingestion. The effective mode is:
22+
* {@code enabled=false} → disabled (no-op); {@code enabled=true, auditOnly=true} → audit (count + log,
23+
* no reject); {@code enabled=true, auditOnly=false} → reject (HTTP 400).
1724
*/
1825
@Builder(toBuilder = true)
1926
public record UuidValidationConfig(
2027
boolean enabled,
28+
boolean auditOnly,
2129
@NotNull @MinDuration(value = 12, unit = TimeUnit.HOURS) @MaxDuration(value = 45, unit = TimeUnit.DAYS) Duration window) {
2230
}

0 commit comments

Comments
 (0)