Repository navigation
Conversation
Aias00
left a comment
There was a problem hiding this comment.
Stream Agent analysis
PR #8412 adds an opt-in, metadata-only AI evaluation sample export plus tenant-isolated feedback correlation. Full diff read (78745 chars, complete) across 19 files; CI 67 checks all success. Security contracts verified: dual opt-in gating, no request content captured, bounded join registry and queue, server-generated sampleId join with tenant/label/bound rejection, non-blocking drop-on-full export, and failure isolation around record(). Tests cover accepted/updated/unknown/forged/tenant-mismatch/bound/invalid paths.
Evidence
- Full VERSIONED_DIFF read (complete=true, 78745 chars) for 19 files; CI 67 checks success, failed_checks=[].
- AiEvaluationSampleManager: opt-in gating (gateway+route), metadata-only record (no prompt/completion), bounded WindowTinyLFUMap registry, tenant-isolated joinFeedback with UNKNOWN_SAMPLE/TENANT_MISMATCH/REJECTED_BOUND/REJECTED_INVALID; exporter offer() non-blocking (drop on full).
- Tests (AiEvaluationSampleManagerTest, AiEvaluationExporterTest) exercise accepted/update/unknown/unforged sample, tenant mismatch, bound and invalid rejection; record() swallows RuntimeException so eval never affects request fate.
Recommended next steps
Scope: captured diff and GitHub check metadata only; no local tests were executed.
Task: 50e4259f-0f98-4e34-b34d-ca38b9e480c7 · Commit: 404dbe4a544a3d4997fa0b62636f1bcd7055e3a7
Review verdict: PASS
Complete diff coverage with passing CI and no concrete blocking defect; the deliberately narrow, opt-in, metadata-only design with bounded state and tenant-isolated feedback join matches its stated safety goals and is covered by tests.
Publication: APPROVE (EXPLICIT_PASS)
Fixes #8256.
Problem
An operator who wants to evaluate an LLM workload routed through the gateway has no sanctioned way to obtain evaluation data: the body-free observation contracts carry the facts, but nothing turns them into versioned samples an offline runner can consume, and nothing lets a judge's score rejoin the request it belongs to without trusting a client-supplied request id. Anything naive here is dangerous — collecting request content implicitly, extending a response's lifetime, or keeping unbounded join state — so this first delivery of #8256 is deliberately narrow: metadata-only samples, two independent opt-in layers, a bounded asynchronous exporter that cannot touch the request path, and feedback joins authorized through gateway-minted opaque references.
Approach
Both layers must opt in: the gateway-wide
shenyu.plugins.ai.evaluation.enabledproperty and a per-selectorevaluationblock on the route handle (enabled,sampleRate,includeIdentity). With either layer off, the gateway holds no evaluation state and behaves byte-identically to today; the manager and exporter beans only exist when the feature is on. Sampling uses a deterministic 100-slot probability gate scoped to this pipeline — the logging family's count-sampler mechanism, not a second telemetry transport.The exported sample (
eval-sample-v1) is body-free metadata by construction: opaque gateway-mintedsampleId, request/selector/rule ids, the configuration generation, terminal outcome, status, duration, attempt count and the provider-reported usage figures with their quality status — unknown figures are exported asnull, never as zero or an invented value, and there are no prompt/model/policy version slots yet because no gateway source for them exists (they land with the #8051 integration). Trusted identity fields enter a sample only when the route allow-listedincludeIdentity; the recorded tenant always stays inside the bounded join registry for authorization but is exported only when allow-listed.Export is O(1) on the request path — a cached route lookup, a sampler check and a bounded-queue offer whose overflow drops and counts instead of blocking — and a dedicated daemon thread drains batches into a sink. The default sink writes one structured line per record for an operator-owned offline runner; an application-owned sink replaces it via
@ConditionalOnMissingBean, and the exporter survives a failing sink by dropping the batch rather than dying. No exchange or request object is ever retained, no additional upstream or model call is made, and all retained state is bounded by the queue capacity.Feedback joins (
eval-feedback-v1) authorize through the opaque sample reference plus the tenant the gateway recorded — a request id is never an authority. Unknown, forged or expired references are rejected asUNKNOWN_SAMPLEwithout creating state; a tenant mismatch asTENANT_MISMATCH; malformed label sets asREJECTED_INVALID. A repeatedjudgeIdupdates its labels idempotently, distinct judges append up to the bounded per-sample budget, and accepted joins are exported through the same pipeline. The join registry is capacity-bounded and its entries expire with the configured retention, so "expired" versus "forged" versus "unauthorized" stays a distinct, documented outcome.Testing
All tests are deterministic and sleep-free; the exporter is driven through its package-private single-batch drain so nothing waits on the worker thread. They cover the double opt-in chain (gateway off, route off, zero rate), the metadata-only contract with identity allow-listing, unknown usage staying unknown, the full join vocabulary (accepted with feedback export, idempotent judge update, per-sample bound, forged reference, tenant mismatch, malformed labels, expiry), a broken sampler configuration never throwing out of the observation, bounded admission with drop counting, batched draining, a failing sink not breaking the drain, and close-flush. The starter's optional-bean wiring keeps its existing tests green.
shenyu-common663 tests,shenyu-plugin-ai-common40,shenyu-plugin-ai-proxy477 (16 new),shenyu-spring-boot-starter-plugin-ai-proxy18 — all green; Checkstyle clean on all four modules. The contract tables, authorization semantics and follow-ups (content capture behind an independent allow-list, ES/Kafka sinks, #8051/#8052 version references) are documented inshenyu-plugin-ai/AI_EVALUATION_SAMPLES.md.