发布

RSS 订阅源

2.2.7-6146 5445108f95

比较
[OPIK-7333] [BE] server-side ingestion size guards (request 413 + document 4xx) (#7483)

frostbyte_neo 发布于 2026-07-26 06:31:11 +00:00
- feat(backend): server-side ingestion size guards - request 413 + document 4xx (OPIK-7333, OPIK-7334)
Server-side half of the unbounded-payload OOM fix (parent OPIK-7312; client-side
per-span cap is OPIK-7335 / PR #7469).

OPIK-7333 - RequestSizeLimitFilter: reject a request whose Content-Length exceeds
maxRequestSizeBytes with 413 before the body is read. Guicey auto-installs it as a JAX-RS
provider and injects JacksonConfig via the Guicey @Config sub-config binding (no explicit
module bind) - same style as TraceThreadConfig / OnlineScoringConfig injection. No
Content-Length (chunked) falls through to maxDocumentLength. Header parsed as long so

2GB can't slip past an int overflow; malformed/negative -> 400.

OPIK-7334 - maxDocumentLength: cap the whole decompressed document on both the HTTP and
JsonUtils ObjectMappers via StreamReadConstraints (set directly alongside maxStringLength;
the builder normalizes <=0 to unlimited), so an oversized batch aborts mid-parse with 4xx
(via JsonProcessingExceptionMapper) before a multi-GB node tree is built.

Limits (config-driven, env-overridable):
- maxStringLength = 100MB (per-field cap; unchanged; the documented "100MB per field")
- maxDocumentLength = 512MB self-hosted default; Comet cloud overrides to 256MB via
  JACKSON_MAX_DOCUMENT_LENGTH (deployment-side, OPIK-7353)
- maxRequestSizeBytes = 512MB self-hosted default; cloud overrides to 256MB via
  JACKSON_MAX_REQUEST_SIZE_BYTES. Kept == maxDocumentLength so the
  request filter never rejects a request the document guard would accept.
  config-test.yml uses lower, payload-calibrated test caps (independent of shipped defaults)
  so the guard tests trip at modest sizes; JacksonConfigTest verifies the real 512MB defaults.
NOTE (deployment): the cloud 256MB override lives in comet-helm/values-production-opik.yaml
(component.backend.env), NOT in this repo - tracked as OPIK-7353. Without it, cloud inherits 512MB.

Tests (all green via mvn):
- Unit (11): RequestSizeLimitFilterTest (over->413, at-limit pass, no-CL pass, >2GB->413,
  malformed/negative->400); JsonUtilsDocumentLengthTest (over-doc->StreamConstraintsException
  via streamed InputStream, under parses, unlimited parses, maxStringLength still enforced);
  JacksonConfigTest (512MB defaults + request==document invariant).
- Integration (SpansResourceTest.LargePayloadTests, 4): ~93MB inline succeeds + strips;
  253MB single string -> 400 (maxStringLength); 280MB total document -> 400 (maxDocumentLength);
  buffered client (Grizzly + Expect:100-continue) with Content-Length over cap -> 413.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- docs(backend): mirror jacksonConfig docblocks in config-test.yml
config-test.yml added maxDocumentLength/maxRequestSizeBytes without the per-key
Default:/Description: docblocks that config.yml carries. Mirror them verbatim so the
test config stays structurally in sync with config.yml, matching the sibling
maxStringLength entry.
- feat(metrics): add ingestion size-guard rejection counter (OPIK-7333)
Emit ingestion_size_guard_rejections_total{guard,endpoint,workspace_id,
workspace_name} so the OPIK-7333/7334 size guards are observable in Grafana.
Neither rejection is otherwise captured: the request-size 413 aborts with a
Response (no throwable) and the document/string-length 4xx is handled by the
more-specific JsonProcessingExceptionMapper, so opik.errors.count never sees
them; the dedicated guard label also separates an oversized-payload reject from
an ordinary malformed-JSON 400.
- IngestionSizeGuardMetrics: @Singleton OTel counter, reusing ErrorMetricsResolver
  for endpoint + workspace (workspace_id falls back to unknown pre-auth on the 413
  path; workspace_name falls back to workspace_id per metrics-instrumentation §2.3).
- RequestSizeLimitFilter: emit guard=request_size on the 413 branch only (malformed/
  negative Content-Length 400s are bad framing, not size rejections, and stay uncounted).
- JsonProcessingExceptionMapper: emit guard=document_length/string_length when the
  parse failure is a StreamConstraintsException, classified from its message.
Tests: RequestSizeLimitFilterTest asserts emit-on-reject / never-on-pass;
IngestionSizeGuardMetricsTest covers the message classifier. mvn -o compile clean.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- fix(backend): enforce maxDocumentLength >= maxStringLength + dedup stream-constraint setup (OPIK-7333)
Addresses two Baz review findings on #7483:
- Cross-field validation (Logical Bug): JacksonConfig only had per-field @Min
  checks, so maxDocumentLength could be set below maxStringLength and reject a
  payload that already passed the string cap. Add an @AssertTrue invariant
  (documentLength >= stringLength, or <= 0 for unlimited) so a misordered env
  override fails fast at startup with a clear message. The config is @Valid, so
  it's enforced on load. Covered by a new JacksonConfigTest case.
- Dedup: the StreamReadConstraints (maxStringLength + maxDocumentLength) build was
  assembled independently in both OpikApplication (HTTP mapper) and
  JsonUtils.createConfiguredMapper (internal mapper). Extract a single
  JsonUtils.applyStreamReadConstraints(mapper, ...) helper used by both, so the
  two mappers can't drift.
Verified: spotless clean, module compiles, JacksonConfigTest + JsonUtilsDocumentLengthTest green (Java 25).

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- fix(backend): address second Baz review round on ingestion size guards (OPIK-7333)
- Config (A): add an @AssertTrue enforcing maxRequestSizeBytes >= maxDocumentLength
  (unless the document cap is <= 0 / unlimited), so a request-size cap set below the
  document cap — which would 413 requests the parser would accept — fails fast at
  startup. Mirrors the maxDocumentLength >= maxStringLength invariant.
- Metrics (C): add a stable component dimension (request_filter | json_parser) to
  ingestion_size_guard_rejections_total, so the 413-filter and 4xx-parser call sites
  are distinguishable without overloading guard (metrics-instrumentation skill §1.4).
- Logging (D+E): split JsonProcessingExceptionMapper — expected size-guard
  StreamConstraintsExceptions log at DEBUG (they're already counted, and were flooding
  INFO on every oversized ingest); genuine malformed JSON logs at INFO with the
  exception (stack trace/type) for diagnosis.
- Test (B): RequestSizeLimitFilterTest asserts the exact UriInfo passed to
  recordRequestSizeRejection instead of any(), catching a mislabeled endpoint.
Verified (Java 25): spotless clean, module compiles, 17/17 targeted tests pass
(JacksonConfigTest, RequestSizeLimitFilterTest, IngestionSizeGuardMetricsTest,
JsonUtilsDocumentLengthTest).

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- fix(backend): drop the maxRequestSizeBytes >= maxDocumentLength invariant (OPIK-7333)
Revert the round-2 Finding-A validator. It was both wrong and boot-breaking:
- Semantically wrong: maxRequestSizeBytes bounds the on-the-wire (compressed)
  Content-Length, while maxDocumentLength bounds the decompressed document. They
  measure different axes, so a request cap below the document cap is a legitimate
  configuration (e.g. cap edge uploads while still parsing larger decompressed
  batches), not a misconfiguration.
- Boot-breaking: config-test.yml deliberately sets maxRequestSizeBytes=50MB <
  maxDocumentLength=256MB to exercise the 413 path cheaply. The @AssertTrue rejected
  that at startup — SpansResourceTest.LargePayloadTests failed with
  ConfigurationValidationException (caught by running the app-booting integration
  test locally; the unit tests missed it).
Replaced with a comment explaining why there is intentionally no such cross-field
check. The maxDocumentLength >= maxStringLength invariant (real, same-axis) stays.

The other round-2 fixes are unchanged: component metric dimension, log-level split,
and the exact-UriInfo filter test.

Verified (Java 25, Docker): app boots, SpansResourceTest.LargePayloadTests 4/4 green
(413 + 400 end-to-end), plus JacksonConfig/RequestSizeLimitFilter/IngestionSizeGuard/
JsonUtilsDocumentLength unit tests — 20/20 total.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- fix(backend): count size-guard rejections wrapped in JsonMappingException (OPIK-7333)
The ingestion size-guard metric under-counted the real-world case. When
maxDocumentLength/maxStringLength trips while Jackson is binding a bean property
(a well-formed SpanBatch/TraceBatch whose input/output/metadata is oversized), the
parser-level StreamConstraintsException is re-wrapped in a JsonMappingException
(carrying the reference chain) by BeanDeserializer.wrapAndThrow. The mapper's
instanceof StreamConstraintsException check then matched the wrapper, not the
constraint, so ingestion_size_guard_rejections_total was never incremented for it.
Net effect: the OPIK-7390 dashboard would read ~0 document/string-length rejections
in prod even while the backend actively rejects oversized SDK payloads. Only an
unknown top-level field (which Jackson skips, with no bean binding) propagates the
constraint raw, which is why synthetic bodies incremented the metric but real SDK
traffic did not.

Fix: walk the exception cause chain (findStreamConstraint, bounded and
self-reference-safe) to find the underlying StreamConstraintsException, and pass the
unwrapped instance to recordStreamConstraintRejection so it still classifies
document- vs string-length from the clean message.

Verified end-to-end against a local low-limit backend driven by old opik 2.2.0 (no
client-side truncation): for the same 10 rejected batches the counter went from 0 to
10, correctly split document_length/string_length and attributed to
/v1/private/{spans,traces}/batch. Added JsonProcessingExceptionMapperTest (5 cases:
wrapped-in-JsonMappingException, raw, deeply nested, plain malformed JSON no-op, and
a self-referencing cause chain).

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- fix(backend): return 413 for ingestion size-guard rejections; tidy test imports (OPIK-7333)
Addresses two code-review comments on the size-guard mapper:
- Size-guard rejections now return 413 REQUEST_ENTITY_TOO_LARGE instead of 400. When
  Jackson's StreamConstraintsException trips during request deserialization (document-
  or string-length over the cap) it is a payload-too-large rejection, not malformed
  JSON, so JsonProcessingExceptionMapper surfaces it as 413 - consistent with
  RequestSizeLimitFilter's pre-parse 413. Genuine malformed JSON still returns 400.
  This is uniform across StreamConstraintsException, so an oversized single value
  (maxStringLength) now also returns 413.
- Replace the inline fully-qualified Jersey/Jakarta client names in the 413 test with
  imports (ClientBuilder, ClientConfig, ClientProperties, RequestEntityProcessing,
  GrizzlyConnectorProvider).
Tests updated to match: JsonProcessingExceptionMapperTest (size-guard -> 413, malformed
-> 400), SpansResourceTest LargePayloadTests (oversized attachment + oversized document
-> 413), JsonUtilsDocumentLengthTest display name.

Verified (Java 25, Docker): JsonProcessingExceptionMapperTest 5/5; SpansResourceTest
LargePayloadTests 4/4 - 413 confirmed end-to-end through the Jersey stack.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- fix(backend): address remaining CRA review findings on the ingestion size guards (OPIK-7333)
Verified locally: unit 19/19, SpansResourceTest LargePayloadTests 4/4 (integration), plus a
live dev-runner run driven by the old (pre-truncation) SDK - 10 rejections, all 413, with the
guard metric correctly split document_length/string_length by endpoint.
- JacksonConfig: maxDocumentLength drops @Min (a non-positive value is a deliberate "unlimited"
  escape hatch that @Min turned into a boot failure); its @AssertTrue now carries the floor
  (>= maxStringLength) AND a 2GB ceiling. maxRequestSizeBytes gains @Max(2GB). Beyond ~2GB the
  guard cannot protect the heap (Java 2^31 String/array limit), so an out-of-range override now
  fails fast at startup instead of silently disabling the guard.
- JsonUtils / JacksonConfig javadoc: clarify the document cap is enforced on stream-based reads
  (the HTTP boundary), not fully in-memory String/byte[] reads. Read-back hardening is tracked
  separately as OPIK-7426.
- JsonProcessingExceptionMapper: include the workspace id in the genuine-malformed-JSON log.
- RequestSizeLimitFilter javadoc: it is a global filter capping every request with a
  Content-Length (intentional memory protection), not ingestion-only.
- IngestionSizeGuardMetrics: document that classifyStreamConstraint depends on Jackson message
  substrings, now guarded by real-parse tests.
- Tests: JsonUtilsDocumentLengthTest adds a typed-bean (SpanBatch) case reproducing the real
  wrapped-JsonMappingException path (the metric-under-count blind spot); IngestionSizeGuardMetricsTest
  adds real-Jackson classification tests; JacksonConfigTest covers the 2GB ceiling + @Max;
  SpansResourceTest asserts the 413 body via JsonNode and documents the chunked-transfer dependency;
  config-test.yml comments corrected (no stale cloud values).
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- fix(backend): stop echoing parser detail in error responses; trim javadoc (OPIK-7333)
Second review round:
- Response leak: JsonProcessingExceptionMapper returned "Unable to process JSON. " +
  exception.getMessage(), leaking Jackson internals and fragments of the client's payload into the
  HTTP error body. It now returns a stable generic message (413 "Request payload exceeds the maximum
  allowed size."; 400 "Unable to process the request body: it is not valid JSON.") while the full
  exception (with workspace context) stays in the server log. Added a leak-guard test.
- JsonUtils javadoc no longer overstates internal coverage: it states maxDocumentLength is enforced
  only on stream/reader reads (parser buffer refill), not fully in-memory String/byte[] reads
  (dropped the misleading "already bounded at ingestion" claim).
- Trimmed the verbose javadoc/comments from the previous round to minimal across JacksonConfig,
  RequestSizeLimitFilter, IngestionSizeGuardMetrics, JsonUtils and the mapper.
- Removed the deployment-configurable size numbers (50MB/256MB request/document, the 512MB/cloud
  narrative) from code comments/javadoc, since they are set by config and drift; only the // 512MB
  decode of the default literal remains. Customer-facing docs keep the numbers.
Verified (Java 25, Docker): unit 19/19; SpansResourceTest LargePayloadTests 4/4 (the 413 tests now
assert the generic message end-to-end).

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- fix(backend): bound the exception cause-chain walk against cyclic causes (OPIK-7333)
findStreamConstraint only broke on a DIRECT self-cause (cause == cause.getCause()), so a
multi-node cause cycle (A -> B -> A, which Throwable.initCause permits) spun the walk forever
and pinned the JAX-RS request thread. Replace the fragile self-cause check with a hard depth
cap (MAX_CAUSE_DEPTH = 50, far beyond any real chain; no per-call allocation). Adds a
timeout-guarded regression test that reproduces the hang on the old code.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- fix(backend): keep informative 400 message for non-size deserialization errors (OPIK-7333)
The size-guard leak-fix genericized the WHOLE 400 branch of the global
JsonProcessingExceptionMapper, changing the error body for every malformed/invalid-JSON
request API-wide and breaking ~8 integration tests (spans, project metrics, workspaces,
filters, provider keys) that assert the specific 'Unable to process JSON. '
message. Scope the generic message to the 413 size-guard branch (where internal limits /
Jackson internals are actually exposed) and restore the informative message on the 400
branch (the caller's own payload + our field names). Status codes unchanged; mapper unit
test helper updated to match.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- refactor(backend): trim size-guard comments to the essentials (OPIK-7333)
Reduce the javadoc/comments we added for the ingestion size guards to the load-bearing
rationale only (2GB ceiling, no cross-field check, in-memory bypass, Jackson-message
fragility, 413-vs-400, do-not-genericize-400, cause-cycle depth cap). Drops restatement,
internal skill-doc citations, and redundant @param/@return. Pre-existing comments untouched.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- fix(backend): address Thiago review findings on ingestion size guards (OPIK-7333)
- Force application/json on the error response: a non-JSON Accept (e.g. OTel protobuf)
  had no ErrorMessage writer, so the 413/400 was surfacing as a 500 (T2, real bug).
- Replace the custom cause-chain walk + MAX_CAUSE_DEPTH with the cycle-safe
  ExceptionUtils.getThrowableList (T3, code reduction).
- Classify the stream constraint: size (document/string length) -> 413; a structural
  limit (nesting/number depth) -> 400 instead of a misleading 413 (T4).
- MAX_CONFIGURABLE_BYTES -> Integer.MAX_VALUE; the old 2^31 was one byte past the real
  String/array limit (T5).
- @Priority on RequestSizeLimitFilter so the size guard runs deterministically before
  auth (413, not 401) (T6).
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- fix(backend): redact caller detail from deserialization error logs (OPIK-7333)
The deserialization-error log logged the full throwable, whose message carries the caller's
own body content (potential PII) - a log leak on ingestion endpoints (Baz medium). Log a
redacted summary instead: quoted workspace + exception type, no throwable. Also quote the
workspace placeholder per opik-backend/SKILL.md. Applies to both 400 log sites; the 413 line
stays at debug.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- refactor(backend): rename catch-all guard label stream_constraint -> unclassified (OPIK-7333)
The fallback bucket was labelled 'stream_constraint', which reads like a confirmed classification
even though it is only ever the catch-all for an unrecognized StreamConstraintsException - misleading
dashboards/alerts (Baz low). Rename the constant + emitted value to 'unclassified'.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- docs(backend): trim config.yml jackson comments; drop deployment-specific overrides (OPIK-7333)
Align maxDocumentLength/maxRequestSizeBytes comments with the file's Default/Description house
style and remove the self-hosted-vs-cloud (256MB) override prose - the override lives in the
deployment env, not this config's comments.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
- refactor(backend): drop hardcoded byte-cap defaults from JacksonConfig; config.yml is the source (OPIK-7333)
maxDocumentLength/maxRequestSizeBytes no longer hard-code 512MB - the value is supplied by
config.yml (Thiago T1: avoid two competing sources of truth). Drops the JacksonConfigTest
assertions that checked those in-code defaults; maxStringLength (a symbolic default, not a
duplicated literal) is unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

Co-authored-by: Claude Opus 4.8 (1M context) noreply@anthropic.com
下载附件
- 源代码 (ZIP)
- 源代码 (TAR.GZ)

0 发布 8,340 Git 标签

[OPIK-7333] [BE] server-side ingestion size guards (request 413 + document 4xx) (#7483)