Add LLMObs.submitFeedback for end-user feedback submission - #12129
Add LLMObs.submitFeedback for end-user feedback submission#12129ddog-thibault-nadin wants to merge 5 commits into
Conversation
Ports the end-user feedback SDK from dd-trace-py (#19347) and dd-trace-js (#9654) to Java. Feedback is submitted through a builder rather than overloads: with 4 target kinds, 5 value types and 3 optional fields, an overload-based API would need ~25 signatures. The builder also makes the runtime type/value coherence checks that Python and Node.js need statically impossible, since each value type has its own setter. Feedback is a v2 eval-metric concept -- event_kind, submitter and the non-score value types only exist there -- while Java evaluations post to v1. Since one batch maps to one HTTP request, the two cannot share a worker, so EvalProcessingWorker is generalised into LLMObsIntakeWorker<T> parameterised by API path, thread and batch serializer. The eval path keeps every parameter it had (queue capacity, flush interval, flush threshold, retry policy, URL and header construction) and its payload is byte-for-byte identical; only the log message wording differs, now that it is templated per payload kind. LLMObsEvalTest pins the v1 payload against regression. submitFeedback goes through its own LLMObsFeedbackProcessor interface rather than a new method on LLMObsEvalProcessor, to avoid breaking binary compatibility for external implementors of a published interface.
|
🎯 Code Coverage (details) 🔗 Commit SHA: 894040e | Docs | Datadog PR Page | Give us feedback! |
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
There was a problem hiding this comment.
The new feedback score builder accepts NaN and infinity, but the Moshi serializer rejects non-finite JSON numbers. A single such feedback item makes the intake worker throw before its network error handling and terminate, preventing subsequent feedback from being submitted; the builder should reject non-finite scores at the call site.
🤖 Datadog Autotest · Commit fcd1138 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
| public Builder scoreValue(double value) { | ||
| return value(MetricType.SCORE, value); | ||
| } |
There was a problem hiding this comment.
Reject non-finite feedback scores
One malformed numeric feedback can permanently stop the dedicated feedback worker and cause later end-user feedback to be silently stranded until the process restarts.
Assertion details
- Input: A caller builds feedback with scoreValue(Double.NaN) or scoreValue(Double.POSITIVE_INFINITY), then the enabled agent flushes the feedback queue.
- Expected:
The public builder rejects a non-finite score, or the worker safely handles the invalid item without terminating feedback submission. - Actual:
scoreValue accepts NaN and infinity. During LLMObsFeedbackEvent serialization, Moshi's JSON writer rejects the non-finite number with IllegalArgumentException. Serialization occurs before the try/catch in LLMObsIntakeWorker.SerializingHandler.flushIfNecessary(), and run() only catches InterruptedException, so the feedback worker thread exits and queued/subsequent feedback is no longer submitted.
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
| public Builder scoreValue(double value) { | |
| return value(MetricType.SCORE, value); | |
| } | |
| public Builder scoreValue(double value) { | |
| if (!Double.isFinite(value)) { | |
| throw new IllegalArgumentException("score value must be finite"); | |
| } | |
| return value(MetricType.SCORE, value); | |
| } |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fcd1138fe2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| this.targetValue = builder.targetValue; | ||
| this.label = builder.label; | ||
| this.metricType = builder.metricType; | ||
| this.value = builder.value; |
There was a problem hiding this comment.
Snapshot JSON feedback values before enqueueing
When callers use .jsonValue(details), this stores the caller-owned Map directly in the supposedly immutable Feedback. Since submitFeedback enqueues the Feedback for later serialization on the feedback worker, any mutation to that map after build() or immediately after submission changes what is sent to intake (and concurrent mutation can make serialization fail). Copy and wrap JSON map values the same way tags are copied so the submitted feedback is a stable snapshot.
Useful? React with 👍 / 👎.
| submissionUrl = | ||
| HttpUrl.get( | ||
| sco.agentUrl.toString() | ||
| + DDAgentFeaturesDiscovery.V2_EVP_PROXY_ENDPOINT | ||
| + EVAL_METRIC_API_PATH); | ||
| headers = Headers.of(EVP_SUBDOMAIN_HEADER_NAME, EVAL_METRIC_API_DOMAIN); | ||
| sco.agentUrl.toString() + DDAgentFeaturesDiscovery.V2_EVP_PROXY_ENDPOINT + apiPath); |
There was a problem hiding this comment.
Use the shared agent client for feedback proxy submissions
In non-agentless configurations that use a Unix domain socket or named pipe for the Agent, sco.agentUrl is only a placeholder URL while the real socket transport is configured on sco.agentHttpClient. This worker still builds the feedback proxy URL from that placeholder and sends it with its own raw OkHttpClient, so submitFeedback cannot reach the Agent in those supported setups; use the shared agent client (and the shared intake client for agentless) when posting these batches.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I think was already there, proposed a fix
What does this PR do?
Adds
llmobs.submitFeedback(options)for submitting end-user feedback through the Node.js SDK.Today this is only reachable by calling the Evaluations API directly. (requested by customers)
Testing
Apart from passing new and old tests, I also created a demo app to confirm that things are working.
Here is a span with feedback and evals submitted to it, using the NodeJS SDK.
here is a submitted feedback attached to the key
support-ticket-4242and not a span