[OPIK-8040] [BE] fix: apply online evaluation sampling to production traces only - #7957
Conversation
…traces only The sampling rate thins a continuous production stream, so it now applies to SDK-logged traces only. Experiment, playground and optimization traces are runs the user started one by one, so they bypass the sampling roll and are always scored in full. - OnlineScoringSampler skips the roll when the trace source is not SDK, and writes one line on the rule's user-facing log stream saying why. - The rule's enabled flag and filters still apply to those traces. - Documents the new meaning of the field in the API schema and the rules docs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
github-actions
Bot
added
documentation
⏱️ 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}
An out-of-range sampling rate passed @Valid and only failed on the automation_rules.sampling_rate CHECK constraint, so the API answered 500 instead of 400. @DecimalMin/@DecimalMax on both the create and update DTOs reject negative, greater-than-one, NaN and Infinity at the boundary. The docs line also over-promised: a playground trace is scored only when it carries selected_rule_ids, and optimization runs were left out. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
This change looks worth a test. OnlineScoringSampler now returns early for any non-SDK trace, so on a rule with trigger scope Would target What it would check
Deploying a test environment for this PR and exploring it — results will follow in a comment. also touches Backend (Java API / internal) Advisory, from the QA test radar. Nothing here blocks this PR, and anything it proposes is a draft for review. Re-checked after a push on 24 Aug 17:12 UTC. |
|
🔄 Test environment deployment process has started Phase 1: Deploying base version You can monitor the progress here. |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
thiagohora
left a comment
thiagohora
left a comment
There was a problem hiding this comment.
Opik reviewer (mined from your team's review history)
7 findings — 0 high · 5 medium · 2 low. Suppressed by team conventions: see suppressed.md.
React 👍/👎 on each comment — your feedback helps tune what it flags.
|
|
||
| private final float samplingRate; | ||
| @Schema(description = "Fraction of production (SDK-logged) traces the rule scores, from 0 to 1. Experiment, playground and optimization traces are always scored in full and ignore this value.") | ||
| @DecimalMin("0") @DecimalMax("1") private final float samplingRate; |
There was a problem hiding this comment.
[medium] New validation guard has no test on the update path
The new @DecimalMin("0")/@DecimalMax("1") guard is a semantically distinct validation branch on a public endpoint and nothing exercises it: no test asserts 422 for sampling_rate < 0 or > 1, nor 2xx for the inclusive boundaries 0 and 1. The sibling constraint on this very class (@SiZe(max = 150) on name, line 57) got dedicated 422 assertions for both create and update (AutomationRuleEvaluatorsResourceTest.java:959 and :979), so this change lacks the parity the team already established.
💡 Add a @ParameterizedTest over {-0.1, 1.1} asserting SC_UNPROCESSABLE_ENTITY (with the violation message) and over {0f, 1f} asserting success, for both POST /v1/private/automations/evaluators and PATCH/PUT of an existing rule.
rule: backend-testing-cover-every-new-branch-guard-and-edge-case-a-change-introduc
There was a problem hiding this comment.
Commit 9fd8f74 addressed this comment by adding parameterized create and update tests for out-of-range sampling rates and inclusive 0/1 boundaries, including validation error and success assertions.
|
|
||
| 1. **Name:** The name of the rule | ||
| 2. **Sampling rate:** The percentage of traces to score. When set to `100%`, all traces will be scored. | ||
| 2. **Sampling rate:** The percentage of production traces to score. When set to `100%`, all production traces will be scored. This setting applies to production traces only — traces from experiments, the playground and optimization runs are never sampled. |
There was a problem hiding this comment.
[medium] Playground/optimization traces are only scored when explicitly selected, not "never sampled"
For non-SDK, non-experiment sources (playground, optimization, evaluator) the trace never even reaches the sampling check unless it carries selected_rule_ids metadata: OnlineScoringSampler.sampleAndScore (apps/opik-backend/.../events/OnlineScoringSampler.java:220-241) adds such traces to scorableTraces only when extractSelectedRuleIds(trace) is non-empty, and skips the project batch otherwise. The sentence as written tells users every playground/optimization trace is scored regardless of the rate, when in reality most are not scored at all.
💡 State the actual behavior: playground and optimization traces are evaluated only when the user explicitly selects a rule for them, and in that case the sampling rate is ignored.
general finding
There was a problem hiding this comment.
Commit 9fd8f74 addressed this comment by clarifying that playground and optimization traces ignore the sampling rate and are evaluated only when the rule is explicitly selected.
The samplingRate @Schema sits on the sealed base class, so it reaches the OpenAPI schema of all six subtypes. The previous wording described the trace-level bypass, which is the opposite of what span and thread rules do: OnlineScoringSpanSampler and TraceThreadOnlineScoringSamplerListener drop non-SDK spans and threads before any rule is applied, so those are never scored rather than scored in full. Also states the decision the sampler owns instead of an outcome it cannot guarantee: publishSampled is fire-and-forget, so a failed enqueue would have left a user-facing "is scored" line for a trace that was never scored. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both rules pages said non-production traces are always scored in full. That is wrong twice over: thread and span rules never evaluate non-SDK data at all, and playground or optimization traces are only evaluated when the user explicitly selects the rule for that run. Also brings the in-product tooltip in line with the docs and the API schema; it still read "Percentage of traces to evaluate" and now follows the scope switch the surrounding copy already uses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…n assertion The [0,1] guard is a distinct validation branch on a public endpoint with no coverage, while the sibling @SiZe(max = 150) constraint on the same class has 422 assertions for both create and update. Adds the same parity: parameterized over {-0.1, 1.1} for 422 and {0, 1} for success, on create and update. The python bypass test verified with any(), so it would have passed had the bypass enqueued the wrong trace or built the message from the wrong evaluator. Extracts toPythonMessage next to the existing toLlmMessage, reuses it in the sibling test, and asserts the exact expected list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Backend Tests - Integration Group 10 37 files 37 suites 7m 37s ⏱️ For more details on these errors, see this check. Results for commit 9fd8f74. |
|
🌙 Nightly cleanup: The test environment for this PR ( |
Two files main added or changed under fern/docs/ collide with the removal of that tree. Both changes already exist on their Opik 2 pages, so the v1 copies stay deleted: - production/rules.mdx: the sampling-rate wording from #7957 is already in docs-v2/production/online-evaluation/rules.mdx, which merged cleanly. - changelog/2026-08-24.mdx: #7968 added the same entry to docs-v2/changelog, and the existing /docs/opik/v1/:slug* redirect covers the old URL.
3 participants
Details
An online evaluation rule holds one sampling rate, and it applied to every trace the rule saw. A user who wanted to sample production traffic but score every experiment run could not do it, so they had to keep two copies of the same rule. This makes the sampling rate a production-only setting:
OnlineScoringSamplernow skips the sampling roll for any trace whose source is not SDK, so experiment, playground and optimization traces are always scored in full.Only the sampling half of the ticket is in this PR. The other suggested improvement — built-in
{{trace.output}}/{{trace.input}}/{{trace.metadata}}placeholders — is not addressed here.Change checklist
Issues
AI-WATERMARK
AI-WATERMARK: yes
Testing
Commands run in
apps/opik-backend:mvn test -Dtest=OnlineScoringSamplerTest— 50 tests, 0 failures.mvn test -Dtest='OnlineScoringSpanSamplerTest,TraceThreadOnlineScoringSamplerListenerTest,AutomationRuleEvaluatorFiltersDeserializerTest'— 37 tests, 0 failures. These cover the two samplers the change deliberately leaves alone.mvn spotless:applythenmvn spotless:check— clean.Scenarios added to
OnlineScoringSamplerTest.SamplingRateTests:0.0, for both the LLM-as-a-judge and the Python evaluator.0.0enqueues the experiment trace only.selected_rule_idsis scored at rate0.0(parameterized over the three sources).Not run: the backend integration and E2E suites, which need MySQL, ClickHouse and Redis. The behaviour under test is pure in-process sampler logic and is covered by the unit tests above.
Documentation
@Schemadescription onsamplingRateinAutomationRuleEvaluatorandAutomationRuleEvaluatorUpdate, so the generated OpenAPI spec and the SDKs carry the same statement. The committedopenapi/opik.yamlis regenerated by thesdks_generate_openapi_spec_and_fern_codeworkflow, so it is not touched here.🤖 Generated with Claude Code