From fcd1138fe2d2cb2f5aeb50871ec8881c8686b227 Mon Sep 17 00:00:00 2001 From: Thibault Nadin Date: Mon, 3 Aug 2026 16:21:40 +0200 Subject: [PATCH 1/5] Add LLMObs.submitFeedback for end-user feedback submission 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 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. --- ...ingWorker.java => LLMObsIntakeWorker.java} | 112 ++-- .../datadog/trace/llmobs/LLMObsSystem.java | 70 +- .../trace/llmobs/domain/LLMObsEval.java | 12 + .../llmobs/domain/LLMObsFeedbackEvent.java | 143 +++++ .../trace/llmobs/domain/LLMObsInternal.java | 4 + .../trace/llmobs/domain/LLMObsEvalTest.java | 107 ++++ .../domain/LLMObsFeedbackEventTest.java | 288 +++++++++ .../java/datadog/trace/api/llmobs/LLMObs.java | 606 ++++++++++++++++++ .../noop/NoOpLLMObsFeedbackProcessor.java | 10 + .../trace/api/llmobs/LLMObsFeedbackTest.java | 463 +++++++++++++ .../trace/util/AgentThreadFactory.java | 2 + 11 files changed, 1765 insertions(+), 52 deletions(-) rename dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/{EvalProcessingWorker.java => LLMObsIntakeWorker.java} (57%) create mode 100644 dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/domain/LLMObsFeedbackEvent.java create mode 100644 dd-java-agent/agent-llmobs/src/test/java/datadog/trace/llmobs/domain/LLMObsEvalTest.java create mode 100644 dd-java-agent/agent-llmobs/src/test/java/datadog/trace/llmobs/domain/LLMObsFeedbackEventTest.java create mode 100644 dd-trace-api/src/main/java/datadog/trace/api/llmobs/noop/NoOpLLMObsFeedbackProcessor.java create mode 100644 dd-trace-api/src/test/java/datadog/trace/api/llmobs/LLMObsFeedbackTest.java diff --git a/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/EvalProcessingWorker.java b/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/LLMObsIntakeWorker.java similarity index 57% rename from dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/EvalProcessingWorker.java rename to dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/LLMObsIntakeWorker.java index 15e207428f2..434d1dd063e 100644 --- a/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/EvalProcessingWorker.java +++ b/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/LLMObsIntakeWorker.java @@ -1,11 +1,8 @@ package datadog.trace.llmobs; -import static datadog.trace.util.AgentThreadFactory.AgentThread.LLMOBS_EVALS_PROCESSOR; import static datadog.trace.util.AgentThreadFactory.THREAD_JOIN_TIMOUT_MS; import static datadog.trace.util.AgentThreadFactory.newAgentThread; -import com.squareup.moshi.JsonAdapter; -import com.squareup.moshi.Moshi; import datadog.common.queue.MessagePassingBlockingQueue; import datadog.common.queue.Queues; import datadog.communication.ddagent.DDAgentFeaturesDiscovery; @@ -13,7 +10,7 @@ import datadog.communication.http.HttpRetryPolicy; import datadog.communication.http.OkHttpUtils; import datadog.trace.api.Config; -import datadog.trace.llmobs.domain.LLMObsEval; +import datadog.trace.util.AgentThreadFactory.AgentThread; import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; @@ -25,64 +22,74 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class EvalProcessingWorker implements AutoCloseable { +/** + * Batches LLM Observability payloads on a dedicated thread and posts them to an intake endpoint. + * + *

Evaluations and feedback share this machinery but not an endpoint: evaluations are submitted + * to {@code v1/eval-metric} and feedback to {@code v2/eval-metric}, so each gets its own instance + * with its own queue and its own batch. + * + * @param the payload type this worker submits + */ +public class LLMObsIntakeWorker implements AutoCloseable { - private static final String EVAL_METRIC_API_DOMAIN = "api"; - private static final String EVAL_METRIC_API_PATH = "api/intake/llm-obs/v1/eval-metric"; + private static final String INTAKE_API_DOMAIN = "api"; private static final String EVP_SUBDOMAIN_HEADER_NAME = "X-Datadog-EVP-Subdomain"; private static final String DD_API_KEY_HEADER_NAME = "DD-API-KEY"; - private static final Logger log = LoggerFactory.getLogger(EvalProcessingWorker.class); + private static final Logger log = LoggerFactory.getLogger(LLMObsIntakeWorker.class); - private final MessagePassingBlockingQueue queue; + /** Serializes a whole batch into the request body sent to the intake. */ + public interface BatchSerializer { + String toJson(List batch); + } + + private final MessagePassingBlockingQueue queue; private final Thread serializerThread; - public EvalProcessingWorker( + public LLMObsIntakeWorker( + final String payloadDescription, + final String apiPath, + final AgentThread agentThread, final int capacity, final long flushInterval, final TimeUnit timeUnit, final SharedCommunicationObjects sco, - Config config) { + final Config config, + final BatchSerializer serializer) { this.queue = Queues.mpscBlockingConsumerArrayQueue(capacity); boolean isAgentless = config.isLlmObsAgentlessEnabled(); if (isAgentless && (config.getApiKey() == null || config.getApiKey().isEmpty())) { - log.error("Agentless eval metric submission requires an API key"); + log.error("Agentless {} submission requires an API key", payloadDescription); } Headers headers; HttpUrl submissionUrl; if (isAgentless) { submissionUrl = - HttpUrl.get( - "https://" - + EVAL_METRIC_API_DOMAIN - + "." - + config.getSite() - + "/" - + EVAL_METRIC_API_PATH); + HttpUrl.get("https://" + INTAKE_API_DOMAIN + "." + config.getSite() + "/" + apiPath); headers = Headers.of(DD_API_KEY_HEADER_NAME, config.getApiKey()); } else { 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); + headers = Headers.of(EVP_SUBDOMAIN_HEADER_NAME, INTAKE_API_DOMAIN); } - EvalSerializingHandler serializingHandler = - new EvalSerializingHandler(queue, flushInterval, timeUnit, submissionUrl, headers); - this.serializerThread = newAgentThread(LLMOBS_EVALS_PROCESSOR, serializingHandler); + SerializingHandler serializingHandler = + new SerializingHandler<>( + payloadDescription, queue, flushInterval, timeUnit, submissionUrl, headers, serializer); + this.serializerThread = newAgentThread(agentThread, serializingHandler); } public void start() { this.serializerThread.start(); } - public boolean addToQueue(final LLMObsEval eval) { - return queue.offer(eval); + public boolean addToQueue(final T payload) { + return queue.offer(payload); } @Override @@ -94,33 +101,34 @@ public void close() { } } - public static class EvalSerializingHandler implements Runnable { + public static class SerializingHandler implements Runnable { - private static final Logger log = LoggerFactory.getLogger(EvalSerializingHandler.class); + private static final Logger log = LoggerFactory.getLogger(SerializingHandler.class); private static final int FLUSH_THRESHOLD = 50; - private final MessagePassingBlockingQueue queue; + private final String payloadDescription; + private final MessagePassingBlockingQueue queue; private final long ticksRequiredToFlush; private long lastTicks; - private final Moshi moshi; - private final JsonAdapter evalJsonAdapter; + private final BatchSerializer serializer; private final OkHttpClient httpClient; private final HttpUrl submissionUrl; private final Headers headers; - private final List buffer = new ArrayList<>(); + private final List buffer = new ArrayList<>(); - public EvalSerializingHandler( - final MessagePassingBlockingQueue queue, + public SerializingHandler( + final String payloadDescription, + final MessagePassingBlockingQueue queue, final long flushInterval, final TimeUnit timeUnit, final HttpUrl submissionUrl, - final Headers headers) { + final Headers headers, + final BatchSerializer serializer) { + this.payloadDescription = payloadDescription; this.queue = queue; - this.moshi = new Moshi.Builder().add(LLMObsEval.class, new LLMObsEval.Adapter()).build(); - - this.evalJsonAdapter = moshi.adapter(LLMObsEval.Request.class); + this.serializer = serializer; this.httpClient = new OkHttpClient(); this.submissionUrl = submissionUrl; this.headers = headers; @@ -128,7 +136,7 @@ public EvalSerializingHandler( this.lastTicks = System.nanoTime(); this.ticksRequiredToFlush = timeUnit.toNanos(flushInterval); - log.debug("starting eval metric serializer, url={}", submissionUrl); + log.debug("starting {} serializer, url={}", payloadDescription, submissionUrl); } @Override @@ -139,16 +147,17 @@ public void run() { Thread.currentThread().interrupt(); } log.debug( - "eval processor worker exited. submitting evals stopped. unsubmitted evals left: {}", + "{} processor worker exited. submitting stopped. unsubmitted payloads left: {}", + payloadDescription, !queuesAreEmpty()); } private void runDutyCycle() throws InterruptedException { Thread thread = Thread.currentThread(); while (!thread.isInterrupted()) { - LLMObsEval eval = queue.poll(100, TimeUnit.MILLISECONDS); - if (eval != null) { - buffer.add(eval); + T payload = queue.poll(100, TimeUnit.MILLISECONDS); + if (payload != null) { + buffer.add(payload); consumeBatch(); } flushIfNecessary(); @@ -164,10 +173,9 @@ protected void flushIfNecessary() { return; } if (shouldFlush()) { - LLMObsEval.Request llmobsEvalReq = new LLMObsEval.Request(this.buffer); HttpRetryPolicy.Factory retryPolicyFactory = new HttpRetryPolicy.Factory(5, 100, 2.0, true); - String reqBod = evalJsonAdapter.toJson(llmobsEvalReq); + String reqBod = serializer.toJson(this.buffer); RequestBody requestBody = RequestBody.create(okhttp3.MediaType.parse("application/json"), reqBod); @@ -178,16 +186,20 @@ protected void flushIfNecessary() { OkHttpUtils.sendWithRetries(httpClient, retryPolicyFactory, request)) { if (response.isSuccessful()) { - log.debug("successfully flushed evaluation request with {} evals", this.buffer.size()); + log.debug( + "successfully flushed {} request with {} payloads", + payloadDescription, + this.buffer.size()); this.buffer.clear(); } else { log.error( - "Could not submit eval metrics (HTTP code {}) {}", + "Could not submit {} (HTTP code {}) {}", + payloadDescription, response.code(), response.body() != null ? response.body().string() : ""); } } catch (Exception e) { - log.error("Could not submit eval metrics", e); + log.error("Could not submit " + payloadDescription, e); } } } diff --git a/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/LLMObsSystem.java b/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/LLMObsSystem.java index a57dd858b45..78b35bdfb09 100644 --- a/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/LLMObsSystem.java +++ b/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/LLMObsSystem.java @@ -9,7 +9,9 @@ import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.llmobs.domain.DDLLMObsSpan; import datadog.trace.llmobs.domain.LLMObsEval; +import datadog.trace.llmobs.domain.LLMObsFeedbackEvent; import datadog.trace.llmobs.domain.LLMObsInternal; +import datadog.trace.util.AgentThreadFactory.AgentThread; import java.lang.instrument.Instrumentation; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -23,6 +25,13 @@ public class LLMObsSystem { private static final String CUSTOM_MODEL_VAL = "custom"; + private static final String EVAL_METRIC_API_PATH = "api/intake/llm-obs/v1/eval-metric"; + // Feedback is a v2 concept: event_kind, submitter and the non-score value types only exist there. + private static final String FEEDBACK_API_PATH = "api/intake/llm-obs/v2/eval-metric"; + + private static final int QUEUE_CAPACITY = 1024; + private static final long FLUSH_INTERVAL_MS = 100; + public static void start(Instrumentation inst, SharedCommunicationObjects sco) { Config config = Config.get(); if (!config.isLlmObsEnabled()) { @@ -37,18 +46,75 @@ public static void start(Instrumentation inst, SharedCommunicationObjects sco) { LLMObsInternal.setLLMObsSpanFactory(new LLMObsManualSpanFactory(mlApp, wellKnownTags)); LLMObsInternal.setLLMObsEvalProcessor(new LLMObsCustomEvalProcessor(mlApp, sco, config)); + + LLMObsInternal.setLLMObsFeedbackProcessor( + new LLMObsCustomFeedbackProcessor(mlApp, sco, config)); + } + + private static class LLMObsCustomFeedbackProcessor implements LLMObs.LLMObsFeedbackProcessor { + private final String defaultMLApp; + private final LLMObsIntakeWorker feedbackProcessingWorker; + + public LLMObsCustomFeedbackProcessor( + String defaultMLApp, SharedCommunicationObjects sco, Config config) { + + this.defaultMLApp = defaultMLApp; + this.feedbackProcessingWorker = + new LLMObsIntakeWorker<>( + "feedback", + FEEDBACK_API_PATH, + AgentThread.LLMOBS_FEEDBACK_PROCESSOR, + QUEUE_CAPACITY, + FLUSH_INTERVAL_MS, + TimeUnit.MILLISECONDS, + sco, + config, + LLMObsFeedbackEvent.batchSerializer()); + this.feedbackProcessingWorker.start(); + } + + @Override + public void submitFeedback(LLMObs.Feedback feedback) { + if (feedback == null) { + LOGGER.error("null feedback provided, feedback not recorded"); + return; + } + + String mlApp = feedback.getMlApp(); + if (mlApp == null || mlApp.isEmpty()) { + mlApp = defaultMLApp; + } + + if (!this.feedbackProcessingWorker.addToQueue(new LLMObsFeedbackEvent(feedback, mlApp))) { + LOGGER.warn( + "queue full, failed to add feedback, ml_app={}, {}={}, label={}", + mlApp, + feedback.getTargetType().getWireKey(), + feedback.getTargetValue(), + feedback.getLabel()); + } + } } private static class LLMObsCustomEvalProcessor implements LLMObs.LLMObsEvalProcessor { private final String defaultMLApp; - private final EvalProcessingWorker evalProcessingWorker; + private final LLMObsIntakeWorker evalProcessingWorker; public LLMObsCustomEvalProcessor( String defaultMLApp, SharedCommunicationObjects sco, Config config) { this.defaultMLApp = defaultMLApp; this.evalProcessingWorker = - new EvalProcessingWorker(1024, 100, TimeUnit.MILLISECONDS, sco, config); + new LLMObsIntakeWorker<>( + "eval metrics", + EVAL_METRIC_API_PATH, + AgentThread.LLMOBS_EVALS_PROCESSOR, + QUEUE_CAPACITY, + FLUSH_INTERVAL_MS, + TimeUnit.MILLISECONDS, + sco, + config, + LLMObsEval.batchSerializer()); this.evalProcessingWorker.start(); } diff --git a/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/domain/LLMObsEval.java b/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/domain/LLMObsEval.java index 58d3c5dbb64..00a3b57f19d 100644 --- a/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/domain/LLMObsEval.java +++ b/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/domain/LLMObsEval.java @@ -5,6 +5,7 @@ import com.squareup.moshi.JsonReader; import com.squareup.moshi.JsonWriter; import com.squareup.moshi.Moshi; +import datadog.trace.llmobs.LLMObsIntakeWorker; import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -48,6 +49,17 @@ public LLMObsEval( } } + /** + * Returns a serializer turning a batch of evaluations into an intake request body. + * + * @return the batch serializer + */ + public static LLMObsIntakeWorker.BatchSerializer batchSerializer() { + Moshi moshi = new Moshi.Builder().add(LLMObsEval.class, new Adapter()).build(); + JsonAdapter requestAdapter = moshi.adapter(Request.class); + return batch -> requestAdapter.toJson(new Request(batch)); + } + public static final class Adapter extends JsonAdapter { private final Moshi moshi = new Moshi.Builder().build(); private final JsonAdapter scoreJsonAdapter = moshi.adapter(Score.class); diff --git a/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/domain/LLMObsFeedbackEvent.java b/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/domain/LLMObsFeedbackEvent.java new file mode 100644 index 00000000000..b49822b274b --- /dev/null +++ b/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/domain/LLMObsFeedbackEvent.java @@ -0,0 +1,143 @@ +package datadog.trace.llmobs.domain; + +import com.squareup.moshi.JsonAdapter; +import com.squareup.moshi.JsonDataException; +import com.squareup.moshi.JsonReader; +import com.squareup.moshi.JsonWriter; +import com.squareup.moshi.Moshi; +import datadog.trace.api.DDTraceApiInfo; +import datadog.trace.api.llmobs.LLMObs; +import datadog.trace.llmobs.LLMObsIntakeWorker; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; + +/** + * An end-user feedback event, as sent to the eval metric intake. + * + *

Feedback shares the {@code evaluation_metric} envelope with {@link LLMObsEval} and is told + * apart on the wire by {@code event_kind}, mirroring dd-trace-py and dd-trace-js. + */ +public final class LLMObsFeedbackEvent { + + private static final String EVENT_KIND_FEEDBACK = "feedback"; + + private final LLMObs.Feedback feedback; + private final String mlApp; + private final List tags; + + public LLMObsFeedbackEvent(LLMObs.Feedback feedback, String mlApp) { + this.feedback = feedback; + this.mlApp = mlApp; + this.tags = buildTags(feedback.getTags(), mlApp); + } + + public LLMObs.Feedback getFeedback() { + return feedback; + } + + public String getMlApp() { + return mlApp; + } + + public List getTags() { + return tags; + } + + private static List buildTags(@Nullable Map userTags, String mlApp) { + List tagList = new ArrayList<>((userTags == null ? 0 : userTags.size()) + 2); + tagList.add("ddtrace.version:" + DDTraceApiInfo.VERSION); + tagList.add("ml_app:" + mlApp); + if (userTags != null) { + for (Map.Entry entry : userTags.entrySet()) { + tagList.add(entry.getKey() + ":" + entry.getValue()); + } + } + return tagList; + } + + /** + * Returns a serializer turning a batch of feedback events into an intake request body. + * + * @return the batch serializer + */ + public static LLMObsIntakeWorker.BatchSerializer batchSerializer() { + Moshi moshi = new Moshi.Builder().add(LLMObsFeedbackEvent.class, new Adapter()).build(); + JsonAdapter requestAdapter = moshi.adapter(Request.class); + return batch -> requestAdapter.toJson(new Request(batch)); + } + + public static final class Adapter extends JsonAdapter { + private final JsonAdapter valueAdapter = + new Moshi.Builder().build().adapter(Object.class); + + @Nullable + @Override + public LLMObsFeedbackEvent fromJson(JsonReader reader) { + return null; + } + + @Override + public void toJson(JsonWriter writer, @Nullable LLMObsFeedbackEvent event) throws IOException { + if (event == null) { + throw new JsonDataException("unexpectedly got null llm obs feedback event"); + } + LLMObs.Feedback feedback = event.feedback; + + writer.beginObject(); + writer.name("event_kind").value(EVENT_KIND_FEEDBACK); + // Exactly one target, enforced by the builder. + writer.name(feedback.getTargetType().getWireKey()).value(feedback.getTargetValue()); + writer.name("label").value(feedback.getLabel()); + writer.name("metric_type").value(feedback.getMetricType().toString()); + writer.name(feedback.getMetricType() + "_value"); + valueAdapter.toJson(writer, feedback.getValue()); + writer.name("ml_app").value(event.mlApp); + writer.name("timestamp_ms").value(feedback.getTimestampMs()); + + writer.name("submitter").beginObject(); + writer.name("id").value(feedback.getSubmitter().getId()); + if (feedback.getSubmitter().getType() != null) { + writer.name("type").value(feedback.getSubmitter().getType()); + } + writer.endObject(); + + if (feedback.getAssessment() != null) { + writer.name("assessment").value(feedback.getAssessment().toString()); + } + if (feedback.getReasoning() != null) { + writer.name("reasoning").value(feedback.getReasoning()); + } + + writer.name("tags").beginArray(); + for (String tag : event.tags) { + writer.value(tag); + } + writer.endArray(); + + writer.endObject(); + } + } + + /** The request envelope, identical in shape to the one used for evaluations. */ + public static final class Request { + public final Data data; + + public static class Data { + public final String type = "evaluation_metric"; + public Attributes attributes; + } + + public static class Attributes { + public List metrics; + } + + public Request(List metrics) { + this.data = new Data(); + this.data.attributes = new Attributes(); + this.data.attributes.metrics = metrics; + } + } +} diff --git a/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/domain/LLMObsInternal.java b/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/domain/LLMObsInternal.java index 85e1482b412..ba3fcfe5dd4 100644 --- a/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/domain/LLMObsInternal.java +++ b/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/domain/LLMObsInternal.java @@ -10,4 +10,8 @@ public static void setLLMObsSpanFactory(final LLMObsSpanFactory factory) { public static void setLLMObsEvalProcessor(final LLMObsEvalProcessor evalProcessor) { LLMObs.EVAL_PROCESSOR = evalProcessor; } + + public static void setLLMObsFeedbackProcessor(final LLMObsFeedbackProcessor feedbackProcessor) { + LLMObs.FEEDBACK_PROCESSOR = feedbackProcessor; + } } diff --git a/dd-java-agent/agent-llmobs/src/test/java/datadog/trace/llmobs/domain/LLMObsEvalTest.java b/dd-java-agent/agent-llmobs/src/test/java/datadog/trace/llmobs/domain/LLMObsEvalTest.java new file mode 100644 index 00000000000..e149b9adc49 --- /dev/null +++ b/dd-java-agent/agent-llmobs/src/test/java/datadog/trace/llmobs/domain/LLMObsEvalTest.java @@ -0,0 +1,107 @@ +package datadog.trace.llmobs.domain; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; + +import com.squareup.moshi.JsonAdapter; +import com.squareup.moshi.Moshi; +import com.squareup.moshi.Types; +import java.io.IOException; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; + +/** + * Pins the v1 eval metric payload. + * + *

Evaluations and feedback now share {@link datadog.trace.llmobs.LLMObsIntakeWorker}, so the + * eval payload reaches the intake through {@link LLMObsEval#batchSerializer()} rather than through + * an adapter held by the worker. These tests exist to prove that indirection changed nothing on the + * wire: no {@code event_kind}, no {@code submitter}, and the same key set as before. + */ +class LLMObsEvalTest { + + private static final JsonAdapter> JSON_READER = + new Moshi.Builder() + .build() + .adapter(Types.newParameterizedType(Map.class, String.class, Object.class)); + + private static List serialize(LLMObsEval... evals) throws IOException { + String body = LLMObsEval.batchSerializer().toJson(Arrays.asList(evals)); + + Map data = asMap(JSON_READER.fromJson(body).get("data")); + assertEquals("evaluation_metric", data.get("type")); + return (List) asMap(data.get("attributes")).get("metrics"); + } + + @SuppressWarnings("unchecked") + private static Map asMap(Object value) { + return (Map) value; + } + + @Test + void testScoreEvalCarriesTheV1KeySetAndNothingElse() throws IOException { + List metrics = + serialize( + new LLMObsEval.Score( + "abc123", + 42L, + 1700000000000L, + "my-app", + "sentiment", + Collections.singletonMap("source", "web-ui"), + 0.75)); + + assertEquals(1, metrics.size()); + Map metric = asMap(metrics.get(0)); + + assertEquals("abc123", metric.get("trace_id")); + assertEquals("42", metric.get("span_id")); + assertEquals(1.7e12, metric.get("timestamp_ms")); + assertEquals("my-app", metric.get("ml_app")); + assertEquals("score", metric.get("metric_type")); + assertEquals("sentiment", metric.get("label")); + assertEquals(0.75, metric.get("score_value")); + assertEquals(Collections.singletonList("source:web-ui"), metric.get("tags")); + + // Feedback-only keys must never leak into the v1 payload. + assertFalse(metric.containsKey("event_kind"), metric.toString()); + assertFalse(metric.containsKey("submitter"), metric.toString()); + assertFalse(metric.containsKey("session_id"), metric.toString()); + assertFalse(metric.containsKey("feedback_join_key"), metric.toString()); + assertFalse(metric.containsKey("assessment"), metric.toString()); + assertFalse(metric.containsKey("reasoning"), metric.toString()); + } + + @Test + void testCategoricalEvalCarriesTheV1KeySet() throws IOException { + List metrics = + serialize( + new LLMObsEval.Categorical( + "abc123", 42L, 1700000000000L, "my-app", "tone", null, "positive")); + + Map metric = asMap(metrics.get(0)); + + assertEquals("categorical", metric.get("metric_type")); + assertEquals("positive", metric.get("categorical_value")); + assertFalse(metric.containsKey("score_value"), metric.toString()); + // A null tag map is omitted rather than serialized as an empty list. + assertFalse(metric.containsKey("tags"), metric.toString()); + assertFalse(metric.containsKey("event_kind"), metric.toString()); + } + + @Test + void testABatchMixesScoreAndCategoricalInOneEnvelope() throws IOException { + List metrics = + serialize( + new LLMObsEval.Score("abc123", 42L, 1700000000000L, "my-app", "sentiment", null, 0.75), + new LLMObsEval.Categorical( + "abc123", 42L, 1700000000000L, "my-app", "tone", null, "positive")); + + assertEquals(2, metrics.size()); + assertEquals(0.75, asMap(metrics.get(0)).get("score_value")); + assertEquals("positive", asMap(metrics.get(1)).get("categorical_value")); + } +} diff --git a/dd-java-agent/agent-llmobs/src/test/java/datadog/trace/llmobs/domain/LLMObsFeedbackEventTest.java b/dd-java-agent/agent-llmobs/src/test/java/datadog/trace/llmobs/domain/LLMObsFeedbackEventTest.java new file mode 100644 index 00000000000..ba31458cf5d --- /dev/null +++ b/dd-java-agent/agent-llmobs/src/test/java/datadog/trace/llmobs/domain/LLMObsFeedbackEventTest.java @@ -0,0 +1,288 @@ +package datadog.trace.llmobs.domain; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.squareup.moshi.JsonAdapter; +import com.squareup.moshi.Moshi; +import com.squareup.moshi.Types; +import datadog.trace.api.DDTraceApiInfo; +import datadog.trace.api.llmobs.LLMObs; +import java.io.IOException; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class LLMObsFeedbackEventTest { + + private static final JsonAdapter> JSON_READER = + new Moshi.Builder() + .build() + .adapter(Types.newParameterizedType(Map.class, String.class, Object.class)); + + private static Map serializeOne(LLMObs.Feedback feedback, String mlApp) + throws IOException { + String body = + LLMObsFeedbackEvent.batchSerializer() + .toJson(Collections.singletonList(new LLMObsFeedbackEvent(feedback, mlApp))); + + Map parsed = JSON_READER.fromJson(body); + Map data = asMap(parsed.get("data")); + assertEquals("evaluation_metric", data.get("type")); + + List metrics = (List) asMap(data.get("attributes")).get("metrics"); + assertEquals(1, metrics.size()); + return asMap(metrics.get(0)); + } + + @SuppressWarnings("unchecked") + private static Map asMap(Object value) { + return (Map) value; + } + + @Test + void testFeedbackIsToldApartFromEvaluationsByEventKind() throws IOException { + Map event = + serializeOne( + LLMObs.Feedback.builder() + .spanId("123") + .label("thumbs") + .booleanValue(true) + .submitter("user-123", "end_user") + .timestampMs(1700000000000L) + .build(), + "my-app"); + + assertEquals("feedback", event.get("event_kind")); + assertEquals("thumbs", event.get("label")); + assertEquals("boolean", event.get("metric_type")); + assertEquals(Boolean.TRUE, event.get("boolean_value")); + assertEquals("my-app", event.get("ml_app")); + assertEquals(1.7e12, event.get("timestamp_ms")); + assertEquals("123", event.get("span_id")); + } + + @Test + void testExactlyOneTargetIsEmitted() throws IOException { + Map bySessionId = + serializeOne( + LLMObs.Feedback.builder() + .sessionId("session-2") + .label("thumbs") + .booleanValue(true) + .submitter("user-123", null) + .build(), + "my-app"); + + assertEquals("session-2", bySessionId.get("session_id")); + assertFalse(bySessionId.containsKey("span_id"), bySessionId.toString()); + assertFalse(bySessionId.containsKey("trace_id"), bySessionId.toString()); + assertFalse(bySessionId.containsKey("feedback_join_key"), bySessionId.toString()); + } + + @Test + void testJoinKeyTargetCarriesNoSpanNorTraceIdentifier() throws IOException { + Map event = + serializeOne( + LLMObs.Feedback.builder() + .feedbackJoinKey("incident-123") + .label("user_comment") + .textValue("The investigation missed the customer impact.") + .submitter("user-123", "end_user") + .assessment(LLMObs.Feedback.Assessment.FAIL) + .build(), + "incident-agent"); + + assertEquals("incident-123", event.get("feedback_join_key")); + assertEquals("text", event.get("metric_type")); + assertEquals("The investigation missed the customer impact.", event.get("text_value")); + assertEquals("fail", event.get("assessment")); + assertFalse(event.containsKey("span_id"), event.toString()); + assertFalse(event.containsKey("trace_id"), event.toString()); + } + + @Test + void testExactlyOneValueKeyIsEmittedPerMetricType() throws IOException { + Map categorical = + serializeOne( + LLMObs.Feedback.builder() + .spanId("123") + .label("satisfaction") + .categoricalValue("satisfied") + .submitter("user-123", null) + .build(), + "my-app"); + assertEquals("satisfied", categorical.get("categorical_value")); + assertFalse(categorical.containsKey("boolean_value"), categorical.toString()); + assertFalse(categorical.containsKey("text_value"), categorical.toString()); + + Map score = + serializeOne( + LLMObs.Feedback.builder() + .spanId("123") + .label("rating") + .scoreValue(0.75) + .submitter("user-123", null) + .build(), + "my-app"); + assertEquals(0.75, score.get("score_value")); + + Map json = + serializeOne( + LLMObs.Feedback.builder() + .spanId("123") + .label("details") + .jsonValue(Collections.singletonMap("missing", "customer impact")) + .submitter("user-123", null) + .build(), + "my-app"); + assertEquals(Collections.singletonMap("missing", "customer impact"), json.get("json_value")); + } + + @Test + void testSubmitterTypeIsOmittedWhenAbsent() throws IOException { + Map withType = + serializeOne( + LLMObs.Feedback.builder() + .spanId("123") + .label("thumbs") + .booleanValue(true) + .submitter("user-123", "end_user") + .build(), + "my-app"); + assertEquals("user-123", asMap(withType.get("submitter")).get("id")); + assertEquals("end_user", asMap(withType.get("submitter")).get("type")); + + Map withoutType = + serializeOne( + LLMObs.Feedback.builder() + .spanId("123") + .label("thumbs") + .booleanValue(true) + .submitter("user-123", null) + .build(), + "my-app"); + assertEquals("user-123", asMap(withoutType.get("submitter")).get("id")); + assertFalse(asMap(withoutType.get("submitter")).containsKey("type"), withoutType.toString()); + } + + @Test + void testAssessmentAndReasoningAreOmittedWhenAbsent() throws IOException { + Map event = + serializeOne( + LLMObs.Feedback.builder() + .spanId("123") + .label("thumbs") + .booleanValue(true) + .submitter("user-123", null) + .build(), + "my-app"); + + assertFalse(event.containsKey("assessment"), event.toString()); + assertFalse(event.containsKey("reasoning"), event.toString()); + } + + @Test + void testReasoningIsEmittedWhenPresent() throws IOException { + Map event = + serializeOne( + LLMObs.Feedback.builder() + .spanId("123") + .label("thumbs") + .booleanValue(false) + .submitter("user-123", null) + .reasoning("did not answer the question") + .build(), + "my-app"); + + assertEquals("did not answer the question", event.get("reasoning")); + } + + @Test + void testTagsCarryTracerVersionAndMlAppAlongsideUserTags() throws IOException { + Map event = + serializeOne( + LLMObs.Feedback.builder() + .spanId("123") + .label("thumbs") + .booleanValue(true) + .submitter("user-123", null) + .tag("source", "web-ui") + .build(), + "my-app"); + + List tags = (List) event.get("tags"); + assertTrue(tags.contains("ddtrace.version:" + DDTraceApiInfo.VERSION), tags.toString()); + assertTrue(tags.contains("ml_app:my-app"), tags.toString()); + assertTrue(tags.contains("source:web-ui"), tags.toString()); + } + + @Test + void testTagsAreEmittedEvenWithoutUserTags() throws IOException { + Map event = + serializeOne( + LLMObs.Feedback.builder() + .spanId("123") + .label("thumbs") + .booleanValue(true) + .submitter("user-123", null) + .build(), + "my-app"); + + assertEquals( + Arrays.asList("ddtrace.version:" + DDTraceApiInfo.VERSION, "ml_app:my-app"), + event.get("tags")); + } + + @Test + void testABatchIsSerializedAsSeveralMetricsInOneEnvelope() throws IOException { + LLMObs.Feedback thumbs = + LLMObs.Feedback.builder() + .spanId("123") + .label("thumbs") + .booleanValue(true) + .submitter("user-123", null) + .build(); + LLMObs.Feedback comment = + LLMObs.Feedback.builder() + .spanId("123") + .label("comment") + .textValue("helpful") + .submitter("user-123", null) + .build(); + + String body = + LLMObsFeedbackEvent.batchSerializer() + .toJson( + Arrays.asList( + new LLMObsFeedbackEvent(thumbs, "my-app"), + new LLMObsFeedbackEvent(comment, "my-app"))); + + Map data = asMap(JSON_READER.fromJson(body).get("data")); + List metrics = (List) asMap(data.get("attributes")).get("metrics"); + + assertEquals(2, metrics.size()); + assertEquals("thumbs", asMap(metrics.get(0)).get("label")); + assertEquals("comment", asMap(metrics.get(1)).get("label")); + } + + @Test + void testMlAppFallsBackToTheProvidedDefault() throws IOException { + Map event = + serializeOne( + LLMObs.Feedback.builder() + .spanId("123") + .label("thumbs") + .booleanValue(true) + .submitter("user-123", null) + .build(), + "fallback-app"); + + assertEquals("fallback-app", event.get("ml_app")); + List tags = (List) event.get("tags"); + assertTrue(tags.contains("ml_app:fallback-app"), tags.toString()); + } +} diff --git a/dd-trace-api/src/main/java/datadog/trace/api/llmobs/LLMObs.java b/dd-trace-api/src/main/java/datadog/trace/api/llmobs/LLMObs.java index 25f3ff0a8ac..d108faabb3b 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/llmobs/LLMObs.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/llmobs/LLMObs.java @@ -1,9 +1,14 @@ package datadog.trace.api.llmobs; import datadog.trace.api.llmobs.noop.NoOpLLMObsEvalProcessor; +import datadog.trace.api.llmobs.noop.NoOpLLMObsFeedbackProcessor; import datadog.trace.api.llmobs.noop.NoOpLLMObsSpanFactory; +import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Locale; import java.util.Map; +import javax.annotation.Nonnull; import javax.annotation.Nullable; public class LLMObs { @@ -11,6 +16,8 @@ protected LLMObs() {} protected static LLMObsSpanFactory SPAN_FACTORY = NoOpLLMObsSpanFactory.INSTANCE; protected static LLMObsEvalProcessor EVAL_PROCESSOR = NoOpLLMObsEvalProcessor.INSTANCE; + protected static LLMObsFeedbackProcessor FEEDBACK_PROCESSOR = + NoOpLLMObsFeedbackProcessor.INSTANCE; public static LLMObsSpan startLLMSpan( String spanName, @@ -88,6 +95,31 @@ public static void SubmitEvaluation( EVAL_PROCESSOR.SubmitEvaluation(llmObsSpan, label, scoreValue, mlApp, tags); } + /** + * Submits end-user feedback on a span, trace, session or customer-defined join key. + * + *

Unlike an evaluation, which scores a span from an automated or offline judge, feedback + * carries the identity of whoever submitted it and can target an entity the submitting process + * has no Datadog context for. + * + * 

{@code
+   * LLMObs.submitFeedback(
+   *     LLMObs.Feedback.builder()
+   *         .span(span)
+   *         .label("thumbs")
+   *         .booleanValue(true)
+   *         .submitter("user-123", "end_user")
+   *         .assessment(LLMObs.Feedback.Assessment.PASS)
+   *         .reasoning("answered the question")
+   *         .build());
+   * }
+ * + * @param feedback the feedback to submit, built with {@link Feedback#builder()} + */ + public static void submitFeedback(Feedback feedback) { + FEEDBACK_PROCESSOR.submitFeedback(feedback); + } + public interface LLMObsSpanFactory { LLMObsSpan startLLMSpan( String spanName, @@ -138,6 +170,580 @@ void SubmitEvaluation( Map tags); } + public interface LLMObsFeedbackProcessor { + void submitFeedback(Feedback feedback); + } + + /** + * End-user feedback on a span, trace, session or customer-defined join key. + * + *

Instances are immutable and built through {@link #builder()}. The builder validates the + * feedback as a whole in {@link Builder#build()}, so an invalid combination fails at the call + * site rather than being silently dropped by the submission worker. + */ + public static class Feedback { + + /** The kind of value carried by a feedback metric. */ + public enum MetricType { + /** A value from a set of names, e.g. {@code "satisfied"}. */ + CATEGORICAL, + /** A numeric value. */ + SCORE, + /** A true/false value, e.g. a thumbs up or down. */ + BOOLEAN, + /** A structured value. */ + JSON, + /** Free-form text, e.g. a written comment. Feedback-only; evaluations reject it. */ + TEXT; + + /** + * Returns the wire representation of this metric type. + * + * @return the lower case name, as expected by the intake + */ + @Override + public String toString() { + return name().toLowerCase(Locale.ROOT); + } + } + + /** Whether the submitter considered the targeted operation a success. */ + public enum Assessment { + /** The operation was satisfactory. */ + PASS, + /** The operation was not satisfactory. */ + FAIL; + + /** + * Returns the wire representation of this assessment. + * + * @return the lower case name, as expected by the intake + */ + @Override + public String toString() { + return name().toLowerCase(Locale.ROOT); + } + } + + /** The entity a feedback is attached to. Exactly one is set on a given feedback. */ + public enum TargetType { + /** A single span. */ + SPAN_ID("span_id"), + /** A whole trace. */ + TRACE_ID("trace_id"), + /** A session, spanning several traces. */ + SESSION_ID("session_id"), + /** A customer-defined business entity key, opaque to the tracer. */ + FEEDBACK_JOIN_KEY("feedback_join_key"); + + private final String wireKey; + + TargetType(String wireKey) { + this.wireKey = wireKey; + } + + /** + * Returns the payload field name carrying this target. + * + * @return the wire key, e.g. {@code "span_id"} + */ + public String getWireKey() { + return wireKey; + } + } + + /** Who submitted a feedback. */ + public static class Submitter { + private final String id; + @Nullable private final String type; + + /** + * Creates a submitter. + * + * @param id the identifier of the submitter, must not be null or empty + * @param type an optional free-form qualifier, e.g. {@code "end_user"} + * @throws IllegalArgumentException if {@code id} is null or empty + */ + public Submitter(@Nonnull String id, @Nullable String type) { + if (id == null || id.isEmpty()) { + throw new IllegalArgumentException("submitter id must be a non-empty string"); + } + this.id = id; + this.type = type; + } + + /** + * Returns the submitter identifier. + * + * @return the identifier, never null nor empty + */ + @Nonnull + public String getId() { + return id; + } + + /** + * Returns the submitter qualifier. + * + * @return the qualifier, or null if none was provided + */ + @Nullable + public String getType() { + return type; + } + } + + private final TargetType targetType; + private final String targetValue; + private final String label; + private final MetricType metricType; + private final Object value; + private final Submitter submitter; + @Nullable private final String mlApp; + @Nullable private final Assessment assessment; + @Nullable private final String reasoning; + private final long timestampMs; + @Nullable private final Map tags; + + private Feedback(Builder builder, long timestampMs) { + this.timestampMs = timestampMs; + this.targetType = builder.targetType; + this.targetValue = builder.targetValue; + this.label = builder.label; + this.metricType = builder.metricType; + this.value = builder.value; + this.submitter = builder.submitter; + this.mlApp = builder.mlApp; + this.assessment = builder.assessment; + this.reasoning = builder.reasoning; + this.tags = + builder.tags == null ? null : Collections.unmodifiableMap(new HashMap<>(builder.tags)); + } + + /** + * Creates a builder for a feedback. + * + * @return a new builder + */ + public static Builder builder() { + return new Builder(); + } + + /** + * Returns which kind of entity this feedback targets. + * + * @return the target type + */ + @Nonnull + public TargetType getTargetType() { + return targetType; + } + + /** + * Returns the identifier of the targeted entity. + * + * @return the target value, never null nor empty + */ + @Nonnull + public String getTargetValue() { + return targetValue; + } + + /** + * Returns the name of the feedback metric. + * + * @return the label, never null nor empty + */ + @Nonnull + public String getLabel() { + return label; + } + + /** + * Returns the kind of value this feedback carries. + * + * @return the metric type + */ + @Nonnull + public MetricType getMetricType() { + return metricType; + } + + /** + * Returns the feedback value. Its runtime type matches {@link #getMetricType()}. + * + * @return the value, never null + */ + @Nonnull + public Object getValue() { + return value; + } + + /** + * Returns who submitted this feedback. + * + * @return the submitter, never null + */ + @Nonnull + public Submitter getSubmitter() { + return submitter; + } + + /** + * Returns the ML application this feedback belongs to. + * + * @return the ML app, or null to fall back on the tracer configured one + */ + @Nullable + public String getMlApp() { + return mlApp; + } + + /** + * Returns whether the submitter considered the targeted operation a success. + * + * @return the assessment, or null if none was provided + */ + @Nullable + public Assessment getAssessment() { + return assessment; + } + + /** + * Returns the free-form justification of this feedback. + * + * @return the reasoning, or null if none was provided + */ + @Nullable + public String getReasoning() { + return reasoning; + } + + /** + * Returns when this feedback was submitted. This is the only ordering signal available to the + * backend when the same feedback is re-submitted with a new value. + * + * @return the submission time, in milliseconds since the epoch + */ + public long getTimestampMs() { + return timestampMs; + } + + /** + * Returns the tags attached to this feedback. + * + * @return an unmodifiable view of the tags, or null if none were provided + */ + @Nullable + public Map getTags() { + return tags; + } + + /** + * Builds a {@link Feedback}. Exactly one target and exactly one value must be set; setting + * either twice, even to the same kind, is rejected so that a silently overwritten target cannot + * ship. + */ + public static class Builder { + private TargetType targetType; + private String targetValue; + private String label; + private MetricType metricType; + private Object value; + private Submitter submitter; + private String mlApp; + private Assessment assessment; + private String reasoning; + private long timestampMs; + private Map tags; + + private Builder() {} + + /** + * Targets the given span. Wire-equivalent to {@link #spanId(String)} with the span's id. + * + * @param span the span to attach the feedback to + * @return this builder + * @throws IllegalArgumentException if a target was already set, or {@code span} is null + */ + public Builder span(@Nonnull LLMObsSpan span) { + if (span == null) { + throw new IllegalArgumentException("span must not be null"); + } + return target(TargetType.SPAN_ID, String.valueOf(span.getSpanId())); + } + + /** + * Targets the span with the given id. + * + * @param spanId the span identifier + * @return this builder + * @throws IllegalArgumentException if a target was already set, or the id is null or empty + */ + public Builder spanId(@Nonnull String spanId) { + return target(TargetType.SPAN_ID, spanId); + } + + /** + * Targets the trace with the given id. + * + * @param traceId the trace identifier + * @return this builder + * @throws IllegalArgumentException if a target was already set, or the id is null or empty + */ + public Builder traceId(@Nonnull String traceId) { + return target(TargetType.TRACE_ID, traceId); + } + + /** + * Targets the session with the given id. + * + * @param sessionId the session identifier + * @return this builder + * @throws IllegalArgumentException if a target was already set, or the id is null or empty + */ + public Builder sessionId(@Nonnull String sessionId) { + return target(TargetType.SESSION_ID, sessionId); + } + + /** + * Targets a customer-defined business entity, e.g. {@code "incident-123"}. The key is opaque + * to the tracer: it is emitted as-is and never matched against any span. + * + * @param feedbackJoinKey the business entity key + * @return this builder + * @throws IllegalArgumentException if a target was already set, or the key is null or empty + */ + public Builder feedbackJoinKey(@Nonnull String feedbackJoinKey) { + return target(TargetType.FEEDBACK_JOIN_KEY, feedbackJoinKey); + } + + /** + * Sets the name of the feedback metric, e.g. {@code "thumbs"}. + * + * @param label the metric name, must not contain a {@code '.'} + * @return this builder + */ + public Builder label(@Nonnull String label) { + this.label = label; + return this; + } + + /** + * Sets a categorical value, e.g. {@code "satisfied"}. + * + * @param value the value + * @return this builder + * @throws IllegalArgumentException if a value was already set, or the value is null + */ + public Builder categoricalValue(@Nonnull String value) { + return value(MetricType.CATEGORICAL, value); + } + + /** + * Sets a numeric value. + * + * @param value the value + * @return this builder + * @throws IllegalArgumentException if a value was already set + */ + public Builder scoreValue(double value) { + return value(MetricType.SCORE, value); + } + + /** + * Sets a true/false value, e.g. a thumbs up or down. + * + * @param value the value + * @return this builder + * @throws IllegalArgumentException if a value was already set + */ + public Builder booleanValue(boolean value) { + return value(MetricType.BOOLEAN, value); + } + + /** + * Sets a structured value. + * + * @param value the value, serialized as a JSON object + * @return this builder + * @throws IllegalArgumentException if a value was already set, or the value is null + */ + public Builder jsonValue(@Nonnull Map value) { + return value(MetricType.JSON, value); + } + + /** + * Sets a free-form text value, e.g. a written comment. + * + * @param value the value + * @return this builder + * @throws IllegalArgumentException if a value was already set, or the value is null + */ + public Builder textValue(@Nonnull String value) { + return value(MetricType.TEXT, value); + } + + /** + * Sets who submitted this feedback. + * + * @param id the identifier of the submitter + * @param type an optional qualifier, e.g. {@code "end_user"} + * @return this builder + * @throws IllegalArgumentException if {@code id} is null or empty + */ + public Builder submitter(@Nonnull String id, @Nullable String type) { + this.submitter = new Submitter(id, type); + return this; + } + + /** + * Sets who submitted this feedback. + * + * @param submitter the submitter + * @return this builder + */ + public Builder submitter(@Nonnull Submitter submitter) { + this.submitter = submitter; + return this; + } + + /** + * Overrides the ML application this feedback belongs to. + * + * @param mlApp the ML app; when null or empty the tracer configured one is used + * @return this builder + */ + public Builder mlApp(@Nullable String mlApp) { + this.mlApp = mlApp; + return this; + } + + /** + * Sets whether the submitter considered the targeted operation a success. + * + * @param assessment the assessment + * @return this builder + */ + public Builder assessment(@Nullable Assessment assessment) { + this.assessment = assessment; + return this; + } + + /** + * Sets a free-form justification of this feedback. + * + * @param reasoning the reasoning + * @return this builder + */ + public Builder reasoning(@Nullable String reasoning) { + this.reasoning = reasoning; + return this; + } + + /** + * Overrides the submission time. Defaults to the time {@link #build()} is called. + * + * @param timestampMs the submission time, in milliseconds since the epoch + * @return this builder + */ + public Builder timestampMs(long timestampMs) { + this.timestampMs = timestampMs; + return this; + } + + /** + * Sets the tags attached to this feedback. + * + * @param tags a map of JSON serializable key-value pairs + * @return this builder + */ + public Builder tags(@Nullable Map tags) { + this.tags = tags == null ? null : new HashMap<>(tags); + return this; + } + + /** + * Adds a single tag to this feedback. + * + * @param key the tag key + * @param value the tag value + * @return this builder + */ + public Builder tag(@Nonnull String key, @Nonnull Object value) { + if (this.tags == null) { + this.tags = new HashMap<>(); + } + this.tags.put(key, value); + return this; + } + + /** + * Validates and builds the feedback. + * + * @return the built feedback + * @throws IllegalArgumentException if no target, no label, no value or no submitter was set, + * if the label contains a {@code '.'}, or if the timestamp is negative + */ + public Feedback build() { + if (targetType == null) { + throw new IllegalArgumentException( + "exactly one of span, spanId, traceId, sessionId or feedbackJoinKey must be specified" + + " to submit feedback"); + } + if (label == null || label.isEmpty()) { + throw new IllegalArgumentException( + "label must be the specified name of the feedback metric"); + } + if (label.indexOf('.') >= 0) { + throw new IllegalArgumentException("label must not contain a '.'"); + } + if (metricType == null) { + throw new IllegalArgumentException( + "exactly one of categoricalValue, scoreValue, booleanValue, jsonValue or textValue" + + " must be specified to submit feedback"); + } + if (submitter == null) { + throw new IllegalArgumentException("submitter must be specified to submit feedback"); + } + if (timestampMs < 0) { + throw new IllegalArgumentException("timestampMs must be a non-negative long"); + } + return new Feedback(this, timestampMs == 0 ? System.currentTimeMillis() : timestampMs); + } + + private Builder target(TargetType type, String value) { + if (targetType != null) { + throw new IllegalArgumentException( + "a feedback target was already set to " + + targetType.getWireKey() + + ", exactly one target must be specified"); + } + if (value == null || value.isEmpty()) { + throw new IllegalArgumentException(type.getWireKey() + " must be a non-empty string"); + } + this.targetType = type; + this.targetValue = value; + return this; + } + + private Builder value(MetricType type, Object value) { + if (metricType != null) { + throw new IllegalArgumentException( + "a feedback value was already set as " + + metricType + + ", exactly one value must be specified"); + } + if (value == null) { + throw new IllegalArgumentException("value must not be null for a " + type + " metric"); + } + this.metricType = type; + this.value = value; + return this; + } + } + } + public static class ToolCall { private String name; private String type; diff --git a/dd-trace-api/src/main/java/datadog/trace/api/llmobs/noop/NoOpLLMObsFeedbackProcessor.java b/dd-trace-api/src/main/java/datadog/trace/api/llmobs/noop/NoOpLLMObsFeedbackProcessor.java new file mode 100644 index 00000000000..d751a94e628 --- /dev/null +++ b/dd-trace-api/src/main/java/datadog/trace/api/llmobs/noop/NoOpLLMObsFeedbackProcessor.java @@ -0,0 +1,10 @@ +package datadog.trace.api.llmobs.noop; + +import datadog.trace.api.llmobs.LLMObs; + +public class NoOpLLMObsFeedbackProcessor implements LLMObs.LLMObsFeedbackProcessor { + public static final NoOpLLMObsFeedbackProcessor INSTANCE = new NoOpLLMObsFeedbackProcessor(); + + @Override + public void submitFeedback(LLMObs.Feedback feedback) {} +} diff --git a/dd-trace-api/src/test/java/datadog/trace/api/llmobs/LLMObsFeedbackTest.java b/dd-trace-api/src/test/java/datadog/trace/api/llmobs/LLMObsFeedbackTest.java new file mode 100644 index 00000000000..f4d1d6726a7 --- /dev/null +++ b/dd-trace-api/src/test/java/datadog/trace/api/llmobs/LLMObsFeedbackTest.java @@ -0,0 +1,463 @@ +package datadog.trace.api.llmobs; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import datadog.trace.api.llmobs.noop.NoOpLLMObsFeedbackProcessor; +import datadog.trace.api.llmobs.noop.NoOpLLMObsSpan; +import java.lang.reflect.Field; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +class LLMObsFeedbackTest { + + private static Object originalFeedbackProcessor; + + @BeforeAll + static void setupSpec() throws Exception { + originalFeedbackProcessor = getStaticField("FEEDBACK_PROCESSOR"); + } + + @AfterAll + static void cleanupSpec() throws Exception { + setStaticField("FEEDBACK_PROCESSOR", originalFeedbackProcessor); + } + + @AfterEach + void cleanup() throws Exception { + setStaticField("FEEDBACK_PROCESSOR", NoOpLLMObsFeedbackProcessor.INSTANCE); + } + + private static LLMObs.Feedback.Builder validBuilder() { + return LLMObs.Feedback.builder() + .spanId("123") + .label("thumbs") + .booleanValue(true) + .submitter("user-123", "end_user"); + } + + // --- targets --- + + @Test + void testMissingTargetIsRejected() { + LLMObs.Feedback.Builder builder = + LLMObs.Feedback.builder().label("thumbs").booleanValue(true).submitter("user-123", null); + + IllegalArgumentException error = assertThrows(IllegalArgumentException.class, builder::build); + assertTrue(error.getMessage().contains("feedbackJoinKey"), error.getMessage()); + } + + @Test + void testEachTargetTypeIsCarriedWithItsWireKey() { + LLMObs.Feedback bySpanId = validBuilder().build(); + assertEquals(LLMObs.Feedback.TargetType.SPAN_ID, bySpanId.getTargetType()); + assertEquals("span_id", bySpanId.getTargetType().getWireKey()); + assertEquals("123", bySpanId.getTargetValue()); + + LLMObs.Feedback byTraceId = + LLMObs.Feedback.builder() + .traceId("abc") + .label("thumbs") + .booleanValue(true) + .submitter("user-123", null) + .build(); + assertEquals(LLMObs.Feedback.TargetType.TRACE_ID, byTraceId.getTargetType()); + assertEquals("trace_id", byTraceId.getTargetType().getWireKey()); + assertEquals("abc", byTraceId.getTargetValue()); + + LLMObs.Feedback bySessionId = + LLMObs.Feedback.builder() + .sessionId("session-2") + .label("thumbs") + .booleanValue(true) + .submitter("user-123", null) + .build(); + assertEquals(LLMObs.Feedback.TargetType.SESSION_ID, bySessionId.getTargetType()); + assertEquals("session_id", bySessionId.getTargetType().getWireKey()); + assertEquals("session-2", bySessionId.getTargetValue()); + + LLMObs.Feedback byJoinKey = + LLMObs.Feedback.builder() + .feedbackJoinKey("incident-123") + .label("user_comment") + .textValue("missed the customer impact") + .submitter("user-123", null) + .build(); + assertEquals(LLMObs.Feedback.TargetType.FEEDBACK_JOIN_KEY, byJoinKey.getTargetType()); + assertEquals("feedback_join_key", byJoinKey.getTargetType().getWireKey()); + assertEquals("incident-123", byJoinKey.getTargetValue()); + } + + @Test + void testSpanTargetsItsSpanId() { + LLMObsSpan span = mock(LLMObsSpan.class); + when(span.getSpanId()).thenReturn(4242L); + + LLMObs.Feedback feedback = + LLMObs.Feedback.builder() + .span(span) + .label("thumbs") + .booleanValue(true) + .submitter("user-123", null) + .build(); + + assertEquals(LLMObs.Feedback.TargetType.SPAN_ID, feedback.getTargetType()); + assertEquals("4242", feedback.getTargetValue()); + } + + @Test + void testNullSpanIsRejected() { + assertThrows( + IllegalArgumentException.class, () -> LLMObs.Feedback.builder().span((LLMObsSpan) null)); + } + + @Test + void testTwoDifferentTargetsAreRejected() { + IllegalArgumentException error = + assertThrows( + IllegalArgumentException.class, + () -> LLMObs.Feedback.builder().spanId("123").sessionId("session-2")); + assertTrue(error.getMessage().contains("span_id"), error.getMessage()); + } + + @Test + void testSameTargetSetTwiceIsRejected() { + assertThrows( + IllegalArgumentException.class, + () -> LLMObs.Feedback.builder().spanId("123").spanId("456")); + } + + @Test + void testEmptyAndNullTargetValuesAreRejected() { + assertThrows(IllegalArgumentException.class, () -> LLMObs.Feedback.builder().spanId("")); + assertThrows(IllegalArgumentException.class, () -> LLMObs.Feedback.builder().spanId(null)); + assertThrows(IllegalArgumentException.class, () -> LLMObs.Feedback.builder().traceId("")); + assertThrows(IllegalArgumentException.class, () -> LLMObs.Feedback.builder().sessionId("")); + assertThrows( + IllegalArgumentException.class, () -> LLMObs.Feedback.builder().feedbackJoinKey("")); + } + + // --- label --- + + @Test + void testMissingAndEmptyLabelsAreRejected() { + LLMObs.Feedback.Builder missing = + LLMObs.Feedback.builder().spanId("123").booleanValue(true).submitter("user-123", null); + assertThrows(IllegalArgumentException.class, missing::build); + + LLMObs.Feedback.Builder empty = + LLMObs.Feedback.builder() + .spanId("123") + .label("") + .booleanValue(true) + .submitter("user-123", null); + assertThrows(IllegalArgumentException.class, empty::build); + } + + @Test + void testDottedLabelIsRejected() { + LLMObs.Feedback.Builder builder = validBuilder().label("thumbs.up"); + + IllegalArgumentException error = assertThrows(IllegalArgumentException.class, builder::build); + assertEquals("label must not contain a '.'", error.getMessage()); + } + + // --- values --- + + @Test + void testMissingValueIsRejected() { + LLMObs.Feedback.Builder builder = + LLMObs.Feedback.builder().spanId("123").label("thumbs").submitter("user-123", null); + + IllegalArgumentException error = assertThrows(IllegalArgumentException.class, builder::build); + assertTrue(error.getMessage().contains("booleanValue"), error.getMessage()); + } + + @Test + void testEachMetricTypeCarriesItsValue() { + LLMObs.Feedback categorical = + LLMObs.Feedback.builder() + .spanId("123") + .label("satisfaction") + .categoricalValue("satisfied") + .submitter("user-123", null) + .build(); + assertEquals(LLMObs.Feedback.MetricType.CATEGORICAL, categorical.getMetricType()); + assertEquals("categorical", categorical.getMetricType().toString()); + assertEquals("satisfied", categorical.getValue()); + + LLMObs.Feedback score = + LLMObs.Feedback.builder() + .spanId("123") + .label("rating") + .scoreValue(0.75) + .submitter("user-123", null) + .build(); + assertEquals(LLMObs.Feedback.MetricType.SCORE, score.getMetricType()); + assertEquals("score", score.getMetricType().toString()); + assertEquals(0.75, score.getValue()); + + LLMObs.Feedback bool = validBuilder().build(); + assertEquals(LLMObs.Feedback.MetricType.BOOLEAN, bool.getMetricType()); + assertEquals("boolean", bool.getMetricType().toString()); + assertEquals(true, bool.getValue()); + + Map details = Collections.singletonMap("missing", "customer impact"); + LLMObs.Feedback json = + LLMObs.Feedback.builder() + .spanId("123") + .label("details") + .jsonValue(details) + .submitter("user-123", null) + .build(); + assertEquals(LLMObs.Feedback.MetricType.JSON, json.getMetricType()); + assertEquals("json", json.getMetricType().toString()); + assertEquals(details, json.getValue()); + + LLMObs.Feedback text = + LLMObs.Feedback.builder() + .spanId("123") + .label("user_comment") + .textValue("missed the customer impact") + .submitter("user-123", null) + .build(); + assertEquals(LLMObs.Feedback.MetricType.TEXT, text.getMetricType()); + assertEquals("text", text.getMetricType().toString()); + assertEquals("missed the customer impact", text.getValue()); + } + + @Test + void testTwoValuesAreRejected() { + IllegalArgumentException error = + assertThrows( + IllegalArgumentException.class, + () -> LLMObs.Feedback.builder().booleanValue(true).textValue("also this")); + assertTrue(error.getMessage().contains("boolean"), error.getMessage()); + } + + @Test + void testSameValueKindSetTwiceIsRejected() { + assertThrows( + IllegalArgumentException.class, + () -> LLMObs.Feedback.builder().scoreValue(0.1).scoreValue(0.2)); + } + + @Test + void testNullValuesAreRejected() { + assertThrows( + IllegalArgumentException.class, () -> LLMObs.Feedback.builder().categoricalValue(null)); + assertThrows(IllegalArgumentException.class, () -> LLMObs.Feedback.builder().textValue(null)); + assertThrows(IllegalArgumentException.class, () -> LLMObs.Feedback.builder().jsonValue(null)); + } + + // --- submitter --- + + @Test + void testMissingSubmitterIsRejected() { + LLMObs.Feedback.Builder builder = + LLMObs.Feedback.builder().spanId("123").label("thumbs").booleanValue(true); + + IllegalArgumentException error = assertThrows(IllegalArgumentException.class, builder::build); + assertTrue(error.getMessage().contains("submitter"), error.getMessage()); + } + + @Test + void testSubmitterIdIsRequired() { + assertThrows( + IllegalArgumentException.class, () -> LLMObs.Feedback.builder().submitter("", "end_user")); + assertThrows( + IllegalArgumentException.class, + () -> LLMObs.Feedback.builder().submitter(null, "end_user")); + assertThrows(IllegalArgumentException.class, () -> new LLMObs.Feedback.Submitter("", null)); + } + + @Test + void testSubmitterTypeIsOptional() { + LLMObs.Feedback withType = validBuilder().build(); + assertEquals("user-123", withType.getSubmitter().getId()); + assertEquals("end_user", withType.getSubmitter().getType()); + + LLMObs.Feedback withoutType = + LLMObs.Feedback.builder() + .spanId("123") + .label("thumbs") + .booleanValue(true) + .submitter("user-123", null) + .build(); + assertEquals("user-123", withoutType.getSubmitter().getId()); + assertNull(withoutType.getSubmitter().getType()); + } + + @Test + void testSubmitterCanBeSuppliedAsAnInstance() { + LLMObs.Feedback.Submitter submitter = new LLMObs.Feedback.Submitter("user-123", "end_user"); + + LLMObs.Feedback feedback = + LLMObs.Feedback.builder() + .spanId("123") + .label("thumbs") + .booleanValue(true) + .submitter(submitter) + .build(); + + assertEquals("user-123", feedback.getSubmitter().getId()); + assertEquals("end_user", feedback.getSubmitter().getType()); + } + + // --- optional fields --- + + @Test + void testAssessmentAndReasoningDefaultToAbsent() { + LLMObs.Feedback feedback = validBuilder().build(); + + assertNull(feedback.getAssessment()); + assertNull(feedback.getReasoning()); + assertNull(feedback.getMlApp()); + assertNull(feedback.getTags()); + } + + @Test + void testAssessmentAndReasoningAreCarried() { + LLMObs.Feedback feedback = + validBuilder() + .assessment(LLMObs.Feedback.Assessment.FAIL) + .reasoning("missed the customer impact") + .mlApp("incident-agent") + .build(); + + assertEquals(LLMObs.Feedback.Assessment.FAIL, feedback.getAssessment()); + assertEquals("fail", feedback.getAssessment().toString()); + assertEquals("missed the customer impact", feedback.getReasoning()); + assertEquals("incident-agent", feedback.getMlApp()); + assertEquals("pass", LLMObs.Feedback.Assessment.PASS.toString()); + } + + // --- timestamp --- + + @Test + void testTimestampDefaultsToNow() { + long before = System.currentTimeMillis(); + LLMObs.Feedback feedback = validBuilder().build(); + long after = System.currentTimeMillis(); + + assertTrue( + feedback.getTimestampMs() >= before && feedback.getTimestampMs() <= after, + "expected " + feedback.getTimestampMs() + " within [" + before + ", " + after + "]"); + } + + @Test + void testExplicitTimestampIsPreserved() { + assertEquals(1234L, validBuilder().timestampMs(1234L).build().getTimestampMs()); + } + + @Test + void testNegativeTimestampIsRejected() { + LLMObs.Feedback.Builder builder = validBuilder().timestampMs(-1L); + + assertThrows(IllegalArgumentException.class, builder::build); + } + + @Test + void testBuilderIsReusableAndDoesNotFreezeTheDefaultTimestamp() throws Exception { + LLMObs.Feedback.Builder builder = validBuilder(); + + LLMObs.Feedback first = builder.build(); + Thread.sleep(2); + LLMObs.Feedback second = builder.build(); + + assertNotEquals(first.getTimestampMs(), second.getTimestampMs()); + } + + // --- tags --- + + @Test + void testTagsAreCopiedAndExposedAsUnmodifiable() { + Map source = new HashMap<>(); + source.put("source", "web-ui"); + + LLMObs.Feedback feedback = validBuilder().tags(source).build(); + source.put("added-after", "should not appear"); + + assertEquals(Collections.singletonMap("source", "web-ui"), feedback.getTags()); + assertThrows(UnsupportedOperationException.class, () -> feedback.getTags().put("nope", "nope")); + } + + @Test + void testSingleTagsAccumulate() { + LLMObs.Feedback feedback = validBuilder().tag("source", "web-ui").tag("revision", "2").build(); + + Map expected = new HashMap<>(); + expected.put("source", "web-ui"); + expected.put("revision", "2"); + assertEquals(expected, feedback.getTags()); + } + + @Test + void testTagsAddedAfterBuildDoNotLeakIntoTheBuiltFeedback() { + LLMObs.Feedback.Builder builder = validBuilder().tag("source", "web-ui"); + + LLMObs.Feedback feedback = builder.build(); + builder.tag("revision", "2"); + + assertEquals(Collections.singletonMap("source", "web-ui"), feedback.getTags()); + } + + @Test + void testNullTagsAreAccepted() { + assertNull(validBuilder().tags(null).build().getTags()); + } + + // --- submission --- + + @Test + void testDefaultNoOpFeedbackProcessorBehavior() { + assertDoesNotThrow( + () -> { + LLMObs.submitFeedback(validBuilder().build()); + LLMObs.submitFeedback(null); + }); + } + + @Test + void testSubmitFeedbackDelegatesToTheProcessor() throws Exception { + LLMObs.LLMObsFeedbackProcessor mockProcessor = mock(LLMObs.LLMObsFeedbackProcessor.class); + setStaticField("FEEDBACK_PROCESSOR", mockProcessor); + + LLMObs.Feedback feedback = + LLMObs.Feedback.builder() + .span(NoOpLLMObsSpan.INSTANCE) + .label("thumbs") + .booleanValue(false) + .submitter("user-123", "end_user") + .assessment(LLMObs.Feedback.Assessment.FAIL) + .build(); + + LLMObs.submitFeedback(feedback); + + verify(mockProcessor).submitFeedback(feedback); + } + + private static void setStaticField(String fieldName, Object value) throws Exception { + Field field = LLMObs.class.getDeclaredField(fieldName); + field.setAccessible(true); + field.set(null, value); + } + + private static Object getStaticField(String fieldName) throws Exception { + Field field = LLMObs.class.getDeclaredField(fieldName); + field.setAccessible(true); + return field.get(null); + } +} diff --git a/internal-api/src/main/java/datadog/trace/util/AgentThreadFactory.java b/internal-api/src/main/java/datadog/trace/util/AgentThreadFactory.java index e9ef282cc7d..a30c5f44f3f 100644 --- a/internal-api/src/main/java/datadog/trace/util/AgentThreadFactory.java +++ b/internal-api/src/main/java/datadog/trace/util/AgentThreadFactory.java @@ -66,6 +66,8 @@ public enum AgentThread { LLMOBS_EVALS_PROCESSOR("dd-llmobs-evals-processor"), + LLMOBS_FEEDBACK_PROCESSOR("dd-llmobs-feedback-processor"), + FEATURE_FLAG_EXPOSURE_PROCESSOR("dd-ffe-exposure-processor"), FEATURE_FLAG_CONFIGURATION_POLLER("dd-feature-flagging-http-poller"); From 8f65747d803a549e42d4a193462a3737d58e8bbe Mon Sep 17 00:00:00 2001 From: Thibault Nadin Date: Tue, 4 Aug 2026 18:01:09 +0200 Subject: [PATCH 2/5] Align submitFeedback with the dd-trace-py implementation --- .../datadog/trace/llmobs/LLMObsSystem.java | 29 ++- .../trace/llmobs/domain/LLMObsEval.java | 8 + .../trace/llmobs/domain/LLMObsEvalTest.java | 11 +- .../java/datadog/trace/api/llmobs/LLMObs.java | 188 ++++++++++++------ .../trace/api/llmobs/LLMObsFeedbackTest.java | 141 +++++++------ .../api/telemetry/LLMObsMetricCollector.java | 37 ++++ .../telemetry/LLMObsFeedbackMetricTest.java | 71 +++++++ 7 files changed, 368 insertions(+), 117 deletions(-) create mode 100644 internal-api/src/test/java/datadog/trace/api/telemetry/LLMObsFeedbackMetricTest.java diff --git a/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/LLMObsSystem.java b/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/LLMObsSystem.java index 78b35bdfb09..9c01eb4a2d1 100644 --- a/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/LLMObsSystem.java +++ b/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/LLMObsSystem.java @@ -6,6 +6,7 @@ import datadog.trace.api.llmobs.LLMObs; import datadog.trace.api.llmobs.LLMObsSpan; import datadog.trace.api.llmobs.LLMObsTags; +import datadog.trace.api.telemetry.LLMObsMetricCollector; import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.llmobs.domain.DDLLMObsSpan; import datadog.trace.llmobs.domain.LLMObsEval; @@ -26,7 +27,9 @@ public class LLMObsSystem { private static final String CUSTOM_MODEL_VAL = "custom"; private static final String EVAL_METRIC_API_PATH = "api/intake/llm-obs/v1/eval-metric"; - // Feedback is a v2 concept: event_kind, submitter and the non-score value types only exist there. + // Feedback is a v2 concept: submitter, the feedback-only targets and the non-score value types + // only exist there. Evaluations deliberately stay on v1 with their existing flat payload; both + // now carry event_kind, so the two are told apart the same way as in dd-trace-py and dd-trace-js. private static final String FEEDBACK_API_PATH = "api/intake/llm-obs/v2/eval-metric"; private static final int QUEUE_CAPACITY = 1024; @@ -76,23 +79,47 @@ public LLMObsCustomFeedbackProcessor( @Override public void submitFeedback(LLMObs.Feedback feedback) { if (feedback == null) { + LLMObsMetricCollector.get().recordFeedbackSubmitted(null, null, "invalid_feedback"); LOGGER.error("null feedback provided, feedback not recorded"); return; } + // The builder never throws so that instrumented code stays safe when the agent is absent; + // validation happens here instead, only once LLM Observability is actually enabled. + LLMObs.Feedback.ValidationError error = feedback.validate(); + if (error != null) { + recordFeedbackTelemetry(feedback, error.getCode()); + throw new IllegalArgumentException(error.getMessage()); + } + String mlApp = feedback.getMlApp(); if (mlApp == null || mlApp.isEmpty()) { mlApp = defaultMLApp; } if (!this.feedbackProcessingWorker.addToQueue(new LLMObsFeedbackEvent(feedback, mlApp))) { + recordFeedbackTelemetry(feedback, "queue_full"); LOGGER.warn( "queue full, failed to add feedback, ml_app={}, {}={}, label={}", mlApp, feedback.getTargetType().getWireKey(), feedback.getTargetValue(), feedback.getLabel()); + return; } + + recordFeedbackTelemetry(feedback, null); + } + + private static void recordFeedbackTelemetry( + LLMObs.Feedback feedback, @Nullable String errorCode) { + LLMObs.Feedback.MetricType metricType = feedback.getMetricType(); + LLMObs.Feedback.TargetType targetType = feedback.getTargetType(); + LLMObsMetricCollector.get() + .recordFeedbackSubmitted( + metricType == null ? null : metricType.toString(), + targetType == null ? null : targetType.getWireKey(), + errorCode); } } diff --git a/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/domain/LLMObsEval.java b/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/domain/LLMObsEval.java index 00a3b57f19d..c38d84438a4 100644 --- a/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/domain/LLMObsEval.java +++ b/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/domain/LLMObsEval.java @@ -16,6 +16,14 @@ public abstract class LLMObsEval { private static final String METRIC_TYPE_SCORE = "score"; private static final String METRIC_TYPE_CATEGORICAL = "categorical"; + private static final String EVENT_KIND_EVALUATION = "evaluation"; + + /** + * Discriminates evaluations from feedback, mirroring dd-trace-py and dd-trace-js. Purely + * additive: the rest of the v1 payload is unchanged. + */ + public final String event_kind = EVENT_KIND_EVALUATION; + public final String trace_id; public final String span_id; public final long timestamp_ms; diff --git a/dd-java-agent/agent-llmobs/src/test/java/datadog/trace/llmobs/domain/LLMObsEvalTest.java b/dd-java-agent/agent-llmobs/src/test/java/datadog/trace/llmobs/domain/LLMObsEvalTest.java index e149b9adc49..c293537a3f9 100644 --- a/dd-java-agent/agent-llmobs/src/test/java/datadog/trace/llmobs/domain/LLMObsEvalTest.java +++ b/dd-java-agent/agent-llmobs/src/test/java/datadog/trace/llmobs/domain/LLMObsEvalTest.java @@ -19,7 +19,10 @@ *

Evaluations and feedback now share {@link datadog.trace.llmobs.LLMObsIntakeWorker}, so the * eval payload reaches the intake through {@link LLMObsEval#batchSerializer()} rather than through * an adapter held by the worker. These tests exist to prove that indirection changed nothing on the - * wire: no {@code event_kind}, no {@code submitter}, and the same key set as before. + * wire: the same key set as before, no {@code submitter}, no feedback-only target. + * + *

The single intentional addition is {@code event_kind:"evaluation"}, which discriminates + * evaluations from feedback the way dd-trace-py and dd-trace-js do. */ class LLMObsEvalTest { @@ -66,8 +69,10 @@ void testScoreEvalCarriesTheV1KeySetAndNothingElse() throws IOException { assertEquals(0.75, metric.get("score_value")); assertEquals(Collections.singletonList("source:web-ui"), metric.get("tags")); + // The only addition to the v1 payload. + assertEquals("evaluation", metric.get("event_kind")); + // Feedback-only keys must never leak into the v1 payload. - assertFalse(metric.containsKey("event_kind"), metric.toString()); assertFalse(metric.containsKey("submitter"), metric.toString()); assertFalse(metric.containsKey("session_id"), metric.toString()); assertFalse(metric.containsKey("feedback_join_key"), metric.toString()); @@ -89,7 +94,7 @@ void testCategoricalEvalCarriesTheV1KeySet() throws IOException { assertFalse(metric.containsKey("score_value"), metric.toString()); // A null tag map is omitted rather than serialized as an empty list. assertFalse(metric.containsKey("tags"), metric.toString()); - assertFalse(metric.containsKey("event_kind"), metric.toString()); + assertEquals("evaluation", metric.get("event_kind")); } @Test diff --git a/dd-trace-api/src/main/java/datadog/trace/api/llmobs/LLMObs.java b/dd-trace-api/src/main/java/datadog/trace/api/llmobs/LLMObs.java index d108faabb3b..05aa39ed926 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/llmobs/LLMObs.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/llmobs/LLMObs.java @@ -114,7 +114,13 @@ public static void SubmitEvaluation( * .build()); * } * + *

This is where the feedback is validated. When LLM Observability is disabled, or the agent is + * not attached, the call is a no-op and an invalid feedback goes unnoticed rather than breaking + * the host application. + * * @param feedback the feedback to submit, built with {@link Feedback#builder()} + * @throws IllegalArgumentException if LLM Observability is enabled and the feedback is invalid, + * e.g. no target, no value, no submitter, or a label containing a {@code '.'} */ public static void submitFeedback(Feedback feedback) { FEEDBACK_PROCESSOR.submitFeedback(feedback); @@ -177,9 +183,11 @@ public interface LLMObsFeedbackProcessor { /** * End-user feedback on a span, trace, session or customer-defined join key. * - *

Instances are immutable and built through {@link #builder()}. The builder validates the - * feedback as a whole in {@link Builder#build()}, so an invalid combination fails at the call - * site rather than being silently dropped by the submission worker. + *

Instances are immutable and built through {@link #builder()}. Neither the builder nor {@link + * Builder#build()} ever throws: the first problem found is recorded and surfaced by {@link + * #validate()}, which {@link LLMObs#submitFeedback(Feedback)} runs. Validation therefore only + * fires when LLM Observability is actually enabled, matching dd-trace-py — instrumented code that + * runs without the agent attached never sees an exception it would not see in production. */ public static class Feedback { @@ -254,20 +262,16 @@ public String getWireKey() { /** Who submitted a feedback. */ public static class Submitter { - private final String id; + @Nullable private final String id; @Nullable private final String type; /** - * Creates a submitter. + * Creates a submitter. An invalid id is not rejected here but by {@link Feedback#validate()}. * * @param id the identifier of the submitter, must not be null or empty * @param type an optional free-form qualifier, e.g. {@code "end_user"} - * @throws IllegalArgumentException if {@code id} is null or empty */ public Submitter(@Nonnull String id, @Nullable String type) { - if (id == null || id.isEmpty()) { - throw new IllegalArgumentException("submitter id must be a non-empty string"); - } this.id = id; this.type = type; } @@ -275,9 +279,9 @@ public Submitter(@Nonnull String id, @Nullable String type) { /** * Returns the submitter identifier. * - * @return the identifier, never null nor empty + * @return the identifier, never null nor empty once {@link Feedback#validate()} returned null */ - @Nonnull + @Nullable public String getId() { return id; } @@ -293,19 +297,55 @@ public String getType() { } } - private final TargetType targetType; - private final String targetValue; - private final String label; - private final MetricType metricType; - private final Object value; - private final Submitter submitter; + /** + * Why a feedback cannot be submitted. The code is a stable, low cardinality identifier reported + * as telemetry; the message is meant for humans. + */ + public static final class ValidationError { + private final String code; + private final String message; + + private ValidationError(String code, String message) { + this.code = code; + this.message = message; + } + + /** + * Returns the telemetry code of this error, e.g. {@code "invalid_submitter"}. + * + * @return the error code, never null + */ + @Nonnull + public String getCode() { + return code; + } + + /** + * Returns the human readable description of this error. + * + * @return the error message, never null + */ + @Nonnull + public String getMessage() { + return message; + } + } + + @Nullable private final TargetType targetType; + @Nullable private final String targetValue; + @Nullable private final String label; + @Nullable private final MetricType metricType; + @Nullable private final Object value; + @Nullable private final Submitter submitter; @Nullable private final String mlApp; @Nullable private final Assessment assessment; @Nullable private final String reasoning; private final long timestampMs; @Nullable private final Map tags; + @Nullable private final ValidationError validationError; - private Feedback(Builder builder, long timestampMs) { + private Feedback(Builder builder, long timestampMs, @Nullable ValidationError validationError) { + this.validationError = validationError; this.timestampMs = timestampMs; this.targetType = builder.targetType; this.targetValue = builder.targetValue; @@ -329,12 +369,24 @@ public static Builder builder() { return new Builder(); } + /** + * Checks whether this feedback can be submitted. Called by {@link + * LLMObs#submitFeedback(Feedback)}; the getters below are only guaranteed non-null once it + * returned null. + * + * @return the first problem found while building this feedback, or null if it is valid + */ + @Nullable + public ValidationError validate() { + return validationError; + } + /** * Returns which kind of entity this feedback targets. * - * @return the target type + * @return the target type, never null once {@link #validate()} returned null */ - @Nonnull + @Nullable public TargetType getTargetType() { return targetType; } @@ -342,9 +394,9 @@ public TargetType getTargetType() { /** * Returns the identifier of the targeted entity. * - * @return the target value, never null nor empty + * @return the target value, never null nor empty once {@link #validate()} returned null */ - @Nonnull + @Nullable public String getTargetValue() { return targetValue; } @@ -352,9 +404,9 @@ public String getTargetValue() { /** * Returns the name of the feedback metric. * - * @return the label, never null nor empty + * @return the label, never null nor empty once {@link #validate()} returned null */ - @Nonnull + @Nullable public String getLabel() { return label; } @@ -362,9 +414,9 @@ public String getLabel() { /** * Returns the kind of value this feedback carries. * - * @return the metric type + * @return the metric type, never null once {@link #validate()} returned null */ - @Nonnull + @Nullable public MetricType getMetricType() { return metricType; } @@ -372,9 +424,9 @@ public MetricType getMetricType() { /** * Returns the feedback value. Its runtime type matches {@link #getMetricType()}. * - * @return the value, never null + * @return the value, never null once {@link #validate()} returned null */ - @Nonnull + @Nullable public Object getValue() { return value; } @@ -382,9 +434,9 @@ public Object getValue() { /** * Returns who submitted this feedback. * - * @return the submitter, never null + * @return the submitter, never null once {@link #validate()} returned null */ - @Nonnull + @Nullable public Submitter getSubmitter() { return submitter; } @@ -443,6 +495,9 @@ public Map getTags() { * Builds a {@link Feedback}. Exactly one target and exactly one value must be set; setting * either twice, even to the same kind, is rejected so that a silently overwritten target cannot * ship. + * + *

No method on this builder throws. The first problem found is remembered and reported by + * {@link Feedback#validate()} at submission time. */ public static class Builder { private TargetType targetType; @@ -456,6 +511,7 @@ public static class Builder { private String reasoning; private long timestampMs; private Map tags; + private ValidationError error; private Builder() {} @@ -464,11 +520,10 @@ private Builder() {} * * @param span the span to attach the feedback to * @return this builder - * @throws IllegalArgumentException if a target was already set, or {@code span} is null */ public Builder span(@Nonnull LLMObsSpan span) { if (span == null) { - throw new IllegalArgumentException("span must not be null"); + return fail("invalid_span", "span must not be null"); } return target(TargetType.SPAN_ID, String.valueOf(span.getSpanId())); } @@ -478,7 +533,6 @@ public Builder span(@Nonnull LLMObsSpan span) { * * @param spanId the span identifier * @return this builder - * @throws IllegalArgumentException if a target was already set, or the id is null or empty */ public Builder spanId(@Nonnull String spanId) { return target(TargetType.SPAN_ID, spanId); @@ -489,7 +543,6 @@ public Builder spanId(@Nonnull String spanId) { * * @param traceId the trace identifier * @return this builder - * @throws IllegalArgumentException if a target was already set, or the id is null or empty */ public Builder traceId(@Nonnull String traceId) { return target(TargetType.TRACE_ID, traceId); @@ -500,7 +553,6 @@ public Builder traceId(@Nonnull String traceId) { * * @param sessionId the session identifier * @return this builder - * @throws IllegalArgumentException if a target was already set, or the id is null or empty */ public Builder sessionId(@Nonnull String sessionId) { return target(TargetType.SESSION_ID, sessionId); @@ -512,7 +564,6 @@ public Builder sessionId(@Nonnull String sessionId) { * * @param feedbackJoinKey the business entity key * @return this builder - * @throws IllegalArgumentException if a target was already set, or the key is null or empty */ public Builder feedbackJoinKey(@Nonnull String feedbackJoinKey) { return target(TargetType.FEEDBACK_JOIN_KEY, feedbackJoinKey); @@ -534,7 +585,6 @@ public Builder label(@Nonnull String label) { * * @param value the value * @return this builder - * @throws IllegalArgumentException if a value was already set, or the value is null */ public Builder categoricalValue(@Nonnull String value) { return value(MetricType.CATEGORICAL, value); @@ -545,7 +595,6 @@ public Builder categoricalValue(@Nonnull String value) { * * @param value the value * @return this builder - * @throws IllegalArgumentException if a value was already set */ public Builder scoreValue(double value) { return value(MetricType.SCORE, value); @@ -556,7 +605,6 @@ public Builder scoreValue(double value) { * * @param value the value * @return this builder - * @throws IllegalArgumentException if a value was already set */ public Builder booleanValue(boolean value) { return value(MetricType.BOOLEAN, value); @@ -567,7 +615,6 @@ public Builder booleanValue(boolean value) { * * @param value the value, serialized as a JSON object * @return this builder - * @throws IllegalArgumentException if a value was already set, or the value is null */ public Builder jsonValue(@Nonnull Map value) { return value(MetricType.JSON, value); @@ -578,7 +625,6 @@ public Builder jsonValue(@Nonnull Map value) { * * @param value the value * @return this builder - * @throws IllegalArgumentException if a value was already set, or the value is null */ public Builder textValue(@Nonnull String value) { return value(MetricType.TEXT, value); @@ -590,7 +636,6 @@ public Builder textValue(@Nonnull String value) { * @param id the identifier of the submitter * @param type an optional qualifier, e.g. {@code "end_user"} * @return this builder - * @throws IllegalArgumentException if {@code id} is null or empty */ public Builder submitter(@Nonnull String id, @Nullable String type) { this.submitter = new Submitter(id, type); @@ -679,48 +724,76 @@ public Builder tag(@Nonnull String key, @Nonnull Object value) { } /** - * Validates and builds the feedback. + * Builds the feedback. Never throws: any problem is carried by the returned instance and + * reported by {@link Feedback#validate()} when it is submitted. * * @return the built feedback - * @throws IllegalArgumentException if no target, no label, no value or no submitter was set, - * if the label contains a {@code '.'}, or if the timestamp is negative */ public Feedback build() { + return new Feedback( + this, timestampMs == 0 ? System.currentTimeMillis() : timestampMs, validationError()); + } + + /** + * Returns the first problem preventing submission, earlier builder errors taking priority. + */ + @Nullable + private ValidationError validationError() { + if (error != null) { + return error; + } if (targetType == null) { - throw new IllegalArgumentException( + return new ValidationError( + "invalid_target_count", "exactly one of span, spanId, traceId, sessionId or feedbackJoinKey must be specified" + " to submit feedback"); } if (label == null || label.isEmpty()) { - throw new IllegalArgumentException( - "label must be the specified name of the feedback metric"); + return new ValidationError( + "invalid_metric_label", "label must be the specified name of the feedback metric"); } if (label.indexOf('.') >= 0) { - throw new IllegalArgumentException("label must not contain a '.'"); + return new ValidationError("invalid_label_value", "label must not contain a '.'"); } if (metricType == null) { - throw new IllegalArgumentException( + return new ValidationError( + "invalid_metric_type", "exactly one of categoricalValue, scoreValue, booleanValue, jsonValue or textValue" + " must be specified to submit feedback"); } if (submitter == null) { - throw new IllegalArgumentException("submitter must be specified to submit feedback"); + return new ValidationError( + "invalid_submitter", "submitter must be specified to submit feedback"); + } + if (submitter.getId() == null || submitter.getId().isEmpty()) { + return new ValidationError( + "invalid_submitter", "submitter id must be a non-empty string"); } if (timestampMs < 0) { - throw new IllegalArgumentException("timestampMs must be a non-negative long"); + return new ValidationError( + "invalid_timestamp", "timestampMs must be a non-negative long"); } - return new Feedback(this, timestampMs == 0 ? System.currentTimeMillis() : timestampMs); + return null; + } + + private Builder fail(String code, String message) { + if (this.error == null) { + this.error = new ValidationError(code, message); + } + return this; } private Builder target(TargetType type, String value) { if (targetType != null) { - throw new IllegalArgumentException( + return fail( + "invalid_target_count", "a feedback target was already set to " + targetType.getWireKey() + ", exactly one target must be specified"); } if (value == null || value.isEmpty()) { - throw new IllegalArgumentException(type.getWireKey() + " must be a non-empty string"); + return fail( + "invalid_" + type.getWireKey(), type.getWireKey() + " must be a non-empty string"); } this.targetType = type; this.targetValue = value; @@ -729,13 +802,14 @@ private Builder target(TargetType type, String value) { private Builder value(MetricType type, Object value) { if (metricType != null) { - throw new IllegalArgumentException( + return fail( + "invalid_metric_type", "a feedback value was already set as " + metricType + ", exactly one value must be specified"); } if (value == null) { - throw new IllegalArgumentException("value must not be null for a " + type + " metric"); + return fail("invalid_metric_value", "value must not be null for a " + type + " metric"); } this.metricType = type; this.value = value; diff --git a/dd-trace-api/src/test/java/datadog/trace/api/llmobs/LLMObsFeedbackTest.java b/dd-trace-api/src/test/java/datadog/trace/api/llmobs/LLMObsFeedbackTest.java index f4d1d6726a7..5a0a232039f 100644 --- a/dd-trace-api/src/test/java/datadog/trace/api/llmobs/LLMObsFeedbackTest.java +++ b/dd-trace-api/src/test/java/datadog/trace/api/llmobs/LLMObsFeedbackTest.java @@ -3,6 +3,7 @@ import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -48,14 +49,46 @@ private static LLMObs.Feedback.Builder validBuilder() { .submitter("user-123", "end_user"); } + /** + * Asserts that a builder produces an invalid feedback, without ever throwing. Validation is + * deferred to {@link LLMObs#submitFeedback} so that instrumented code stays safe when LLM + * Observability is disabled or the agent is not attached. + * + * @return the validation error, for further assertions on its message + */ + private static LLMObs.Feedback.ValidationError assertRejected( + String expectedCode, LLMObs.Feedback.Builder builder) { + LLMObs.Feedback feedback = assertDoesNotThrow(builder::build); + LLMObs.Feedback.ValidationError error = feedback.validate(); + assertNotNull(error, "expected the feedback to be rejected with " + expectedCode); + assertEquals(expectedCode, error.getCode(), error.getMessage()); + return error; + } + + // --- deferred validation --- + + @Test + void testAValidFeedbackHasNoValidationError() { + assertNull(validBuilder().build().validate()); + } + + @Test + void testTheFirstProblemWinsOverLaterOnes() { + // The empty span id is reported even though the label and the value are missing too. + assertRejected("invalid_span_id", LLMObs.Feedback.builder().spanId("").label("thumbs.up")); + } + // --- targets --- @Test void testMissingTargetIsRejected() { - LLMObs.Feedback.Builder builder = - LLMObs.Feedback.builder().label("thumbs").booleanValue(true).submitter("user-123", null); - - IllegalArgumentException error = assertThrows(IllegalArgumentException.class, builder::build); + LLMObs.Feedback.ValidationError error = + assertRejected( + "invalid_target_count", + LLMObs.Feedback.builder() + .label("thumbs") + .booleanValue(true) + .submitter("user-123", null)); assertTrue(error.getMessage().contains("feedbackJoinKey"), error.getMessage()); } @@ -119,58 +152,52 @@ void testSpanTargetsItsSpanId() { @Test void testNullSpanIsRejected() { - assertThrows( - IllegalArgumentException.class, () -> LLMObs.Feedback.builder().span((LLMObsSpan) null)); + assertRejected("invalid_span", LLMObs.Feedback.builder().span((LLMObsSpan) null)); } @Test void testTwoDifferentTargetsAreRejected() { - IllegalArgumentException error = - assertThrows( - IllegalArgumentException.class, - () -> LLMObs.Feedback.builder().spanId("123").sessionId("session-2")); + LLMObs.Feedback.ValidationError error = + assertRejected( + "invalid_target_count", LLMObs.Feedback.builder().spanId("123").sessionId("session-2")); assertTrue(error.getMessage().contains("span_id"), error.getMessage()); } @Test void testSameTargetSetTwiceIsRejected() { - assertThrows( - IllegalArgumentException.class, - () -> LLMObs.Feedback.builder().spanId("123").spanId("456")); + assertRejected("invalid_target_count", LLMObs.Feedback.builder().spanId("123").spanId("456")); } @Test void testEmptyAndNullTargetValuesAreRejected() { - assertThrows(IllegalArgumentException.class, () -> LLMObs.Feedback.builder().spanId("")); - assertThrows(IllegalArgumentException.class, () -> LLMObs.Feedback.builder().spanId(null)); - assertThrows(IllegalArgumentException.class, () -> LLMObs.Feedback.builder().traceId("")); - assertThrows(IllegalArgumentException.class, () -> LLMObs.Feedback.builder().sessionId("")); - assertThrows( - IllegalArgumentException.class, () -> LLMObs.Feedback.builder().feedbackJoinKey("")); + assertRejected("invalid_span_id", LLMObs.Feedback.builder().spanId("")); + assertRejected("invalid_span_id", LLMObs.Feedback.builder().spanId(null)); + assertRejected("invalid_trace_id", LLMObs.Feedback.builder().traceId("")); + assertRejected("invalid_session_id", LLMObs.Feedback.builder().sessionId("")); + assertRejected("invalid_feedback_join_key", LLMObs.Feedback.builder().feedbackJoinKey("")); } // --- label --- @Test void testMissingAndEmptyLabelsAreRejected() { - LLMObs.Feedback.Builder missing = - LLMObs.Feedback.builder().spanId("123").booleanValue(true).submitter("user-123", null); - assertThrows(IllegalArgumentException.class, missing::build); + assertRejected( + "invalid_metric_label", + LLMObs.Feedback.builder().spanId("123").booleanValue(true).submitter("user-123", null)); - LLMObs.Feedback.Builder empty = + assertRejected( + "invalid_metric_label", LLMObs.Feedback.builder() .spanId("123") .label("") .booleanValue(true) - .submitter("user-123", null); - assertThrows(IllegalArgumentException.class, empty::build); + .submitter("user-123", null)); } @Test void testDottedLabelIsRejected() { - LLMObs.Feedback.Builder builder = validBuilder().label("thumbs.up"); - - IllegalArgumentException error = assertThrows(IllegalArgumentException.class, builder::build); + LLMObs.Feedback.ValidationError error = + assertRejected("invalid_label_value", validBuilder().label("thumbs.up")); assertEquals("label must not contain a '.'", error.getMessage()); } @@ -178,10 +205,10 @@ void testDottedLabelIsRejected() { @Test void testMissingValueIsRejected() { - LLMObs.Feedback.Builder builder = - LLMObs.Feedback.builder().spanId("123").label("thumbs").submitter("user-123", null); - - IllegalArgumentException error = assertThrows(IllegalArgumentException.class, builder::build); + LLMObs.Feedback.ValidationError error = + assertRejected( + "invalid_metric_type", + LLMObs.Feedback.builder().spanId("123").label("thumbs").submitter("user-123", null)); assertTrue(error.getMessage().contains("booleanValue"), error.getMessage()); } @@ -240,47 +267,43 @@ void testEachMetricTypeCarriesItsValue() { @Test void testTwoValuesAreRejected() { - IllegalArgumentException error = - assertThrows( - IllegalArgumentException.class, - () -> LLMObs.Feedback.builder().booleanValue(true).textValue("also this")); + LLMObs.Feedback.ValidationError error = + assertRejected( + "invalid_metric_type", + LLMObs.Feedback.builder().booleanValue(true).textValue("also this")); assertTrue(error.getMessage().contains("boolean"), error.getMessage()); } @Test void testSameValueKindSetTwiceIsRejected() { - assertThrows( - IllegalArgumentException.class, - () -> LLMObs.Feedback.builder().scoreValue(0.1).scoreValue(0.2)); + assertRejected( + "invalid_metric_type", LLMObs.Feedback.builder().scoreValue(0.1).scoreValue(0.2)); } @Test void testNullValuesAreRejected() { - assertThrows( - IllegalArgumentException.class, () -> LLMObs.Feedback.builder().categoricalValue(null)); - assertThrows(IllegalArgumentException.class, () -> LLMObs.Feedback.builder().textValue(null)); - assertThrows(IllegalArgumentException.class, () -> LLMObs.Feedback.builder().jsonValue(null)); + assertRejected("invalid_metric_value", LLMObs.Feedback.builder().categoricalValue(null)); + assertRejected("invalid_metric_value", LLMObs.Feedback.builder().textValue(null)); + assertRejected("invalid_metric_value", LLMObs.Feedback.builder().jsonValue(null)); } // --- submitter --- @Test void testMissingSubmitterIsRejected() { - LLMObs.Feedback.Builder builder = - LLMObs.Feedback.builder().spanId("123").label("thumbs").booleanValue(true); - - IllegalArgumentException error = assertThrows(IllegalArgumentException.class, builder::build); + LLMObs.Feedback.ValidationError error = + assertRejected( + "invalid_submitter", + LLMObs.Feedback.builder().spanId("123").label("thumbs").booleanValue(true)); assertTrue(error.getMessage().contains("submitter"), error.getMessage()); } @Test void testSubmitterIdIsRequired() { - assertThrows( - IllegalArgumentException.class, () -> LLMObs.Feedback.builder().submitter("", "end_user")); - assertThrows( - IllegalArgumentException.class, - () -> LLMObs.Feedback.builder().submitter(null, "end_user")); - assertThrows(IllegalArgumentException.class, () -> new LLMObs.Feedback.Submitter("", null)); + assertRejected("invalid_submitter", validBuilder().submitter("", "end_user")); + assertRejected("invalid_submitter", validBuilder().submitter(null, "end_user")); + assertRejected( + "invalid_submitter", validBuilder().submitter(new LLMObs.Feedback.Submitter("", null))); } @Test @@ -364,9 +387,7 @@ void testExplicitTimestampIsPreserved() { @Test void testNegativeTimestampIsRejected() { - LLMObs.Feedback.Builder builder = validBuilder().timestampMs(-1L); - - assertThrows(IllegalArgumentException.class, builder::build); + assertRejected("invalid_timestamp", validBuilder().timestampMs(-1L)); } @Test @@ -430,6 +451,14 @@ void testDefaultNoOpFeedbackProcessorBehavior() { }); } + @Test + void testAnInvalidFeedbackIsSilentWhenNoProcessorIsInstalled() { + // Without the agent, or with LLM Observability disabled, submitting garbage must not break the + // host application. The real processor is the one that rejects it. + assertDoesNotThrow( + () -> LLMObs.submitFeedback(LLMObs.Feedback.builder().label("thumbs.up").build())); + } + @Test void testSubmitFeedbackDelegatesToTheProcessor() throws Exception { LLMObs.LLMObsFeedbackProcessor mockProcessor = mock(LLMObs.LLMObsFeedbackProcessor.class); diff --git a/internal-api/src/main/java/datadog/trace/api/telemetry/LLMObsMetricCollector.java b/internal-api/src/main/java/datadog/trace/api/telemetry/LLMObsMetricCollector.java index f43d92cb741..2648f9fb80d 100644 --- a/internal-api/src/main/java/datadog/trace/api/telemetry/LLMObsMetricCollector.java +++ b/internal-api/src/main/java/datadog/trace/api/telemetry/LLMObsMetricCollector.java @@ -9,6 +9,7 @@ import java.util.List; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.BlockingQueue; +import javax.annotation.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -24,8 +25,12 @@ public static LLMObsMetricCollector get() { } public static final String SPAN_FINISHED_METRIC = "span.finished"; + public static final String FEEDBACK_SUBMITTED_METRIC = "feedback_submitted"; public static final String COUNT_METRIC_TYPE = "count"; + /** Tag value used when a submission failed before the real value could be determined. */ + private static final String OTHER = "other"; + private static final String IS_ROOT_SPAN_TRUE = "is_root_span:1"; private static final String IS_ROOT_SPAN_FALSE = "is_root_span:0"; private static final String AUTOINSTRUMENTED_TRUE = "autoinstrumented:1"; @@ -81,6 +86,38 @@ public void recordSpanFinished( } } + /** + * Record an end-user feedback submission attempt, successful or not. + * + *

Mirrors {@code record_llmobs_submit_feedback} in dd-trace-py: rejected submissions are + * counted too, tagged with the validation error, so that adoption and SDK misuse are both + * visible. + * + * @param metricType the feedback metric type (e.g. "boolean"), or null if it could not be + * determined + * @param targetType the wire key of the feedback target (e.g. "span_id"), or null if it could not + * be determined + * @param error the validation error code (e.g. "invalid_submitter"), or null if the submission + * was accepted + */ + public void recordFeedbackSubmitted( + @Nullable String metricType, @Nullable String targetType, @Nullable String error) { + List tags = new ArrayList<>(4); + tags.add(error == null ? ERROR_FALSE : ERROR_TRUE); + if (error != null) { + tags.add("error_type:" + error); + } + tags.add("metric_type:" + (metricType == null ? OTHER : metricType)); + tags.add("target_type:" + (targetType == null ? OTHER : targetType)); + + LLMObsMetric metric = + new LLMObsMetric( + METRIC_NAMESPACE, true, FEEDBACK_SUBMITTED_METRIC, COUNT_METRIC_TYPE, 1L, tags); + if (!metricsQueue.offer(metric)) { + log.debug("Unable to add telemetry metric {}", FEEDBACK_SUBMITTED_METRIC); + } + } + @Override public void prepareMetrics() { // metrics are added directly via recordSpanFinished; no additional preparation needed diff --git a/internal-api/src/test/java/datadog/trace/api/telemetry/LLMObsFeedbackMetricTest.java b/internal-api/src/test/java/datadog/trace/api/telemetry/LLMObsFeedbackMetricTest.java new file mode 100644 index 00000000000..5cfc28aaa84 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/api/telemetry/LLMObsFeedbackMetricTest.java @@ -0,0 +1,71 @@ +package datadog.trace.api.telemetry; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Collection; +import java.util.List; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Covers the {@code feedback_submitted} telemetry metric, the Java counterpart of {@code + * record_llmobs_submit_feedback} in dd-trace-py. + */ +class LLMObsFeedbackMetricTest { + + private LLMObsMetricCollector collector; + + @BeforeEach + void setup() { + collector = LLMObsMetricCollector.get(); + // The collector is a singleton shared with the rest of the suite. + collector.drain(); + } + + private LLMObsMetricCollector.LLMObsMetric drainOne() { + Collection drained = collector.drain(); + assertEquals(1, drained.size(), drained.toString()); + return drained.iterator().next(); + } + + @Test + void testAnAcceptedSubmissionIsCountedWithoutError() { + collector.recordFeedbackSubmitted("boolean", "span_id", null); + + LLMObsMetricCollector.LLMObsMetric metric = drainOne(); + assertEquals("mlobs", metric.namespace); + assertEquals("feedback_submitted", metric.metricName); + assertEquals("count", metric.type); + assertEquals(1L, metric.value.longValue()); + + List tags = metric.tags; + assertTrue(tags.contains("error:0"), tags.toString()); + assertTrue(tags.contains("metric_type:boolean"), tags.toString()); + assertTrue(tags.contains("target_type:span_id"), tags.toString()); + assertFalse(tags.toString().contains("error_type"), tags.toString()); + } + + @Test + void testARejectedSubmissionCarriesItsErrorType() { + collector.recordFeedbackSubmitted("text", "feedback_join_key", "invalid_submitter"); + + List tags = drainOne().tags; + assertTrue(tags.contains("error:1"), tags.toString()); + assertTrue(tags.contains("error_type:invalid_submitter"), tags.toString()); + assertTrue(tags.contains("metric_type:text"), tags.toString()); + assertTrue(tags.contains("target_type:feedback_join_key"), tags.toString()); + } + + @Test + void testUndeterminedMetricAndTargetTypesFallBackToOther() { + // A feedback rejected before its target or value was set still gets counted. + collector.recordFeedbackSubmitted(null, null, "invalid_target_count"); + + List tags = drainOne().tags; + assertTrue(tags.contains("error:1"), tags.toString()); + assertTrue(tags.contains("metric_type:other"), tags.toString()); + assertTrue(tags.contains("target_type:other"), tags.toString()); + } +} From 24076172b9f4fb986508e4da5b6032e400b725f5 Mon Sep 17 00:00:00 2001 From: Thibault Nadin Date: Wed, 5 Aug 2026 17:16:53 +0200 Subject: [PATCH 3/5] Align the dotted label error message with dd-trace-js --- dd-trace-api/src/main/java/datadog/trace/api/llmobs/LLMObs.java | 2 +- .../test/java/datadog/trace/api/llmobs/LLMObsFeedbackTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dd-trace-api/src/main/java/datadog/trace/api/llmobs/LLMObs.java b/dd-trace-api/src/main/java/datadog/trace/api/llmobs/LLMObs.java index 05aa39ed926..79465f1a321 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/llmobs/LLMObs.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/llmobs/LLMObs.java @@ -753,7 +753,7 @@ private ValidationError validationError() { "invalid_metric_label", "label must be the specified name of the feedback metric"); } if (label.indexOf('.') >= 0) { - return new ValidationError("invalid_label_value", "label must not contain a '.'"); + return new ValidationError("invalid_label_value", "label value must not contain a '.'"); } if (metricType == null) { return new ValidationError( diff --git a/dd-trace-api/src/test/java/datadog/trace/api/llmobs/LLMObsFeedbackTest.java b/dd-trace-api/src/test/java/datadog/trace/api/llmobs/LLMObsFeedbackTest.java index 5a0a232039f..5dc07a11965 100644 --- a/dd-trace-api/src/test/java/datadog/trace/api/llmobs/LLMObsFeedbackTest.java +++ b/dd-trace-api/src/test/java/datadog/trace/api/llmobs/LLMObsFeedbackTest.java @@ -198,7 +198,7 @@ void testMissingAndEmptyLabelsAreRejected() { void testDottedLabelIsRejected() { LLMObs.Feedback.ValidationError error = assertRejected("invalid_label_value", validBuilder().label("thumbs.up")); - assertEquals("label must not contain a '.'", error.getMessage()); + assertEquals("label value must not contain a '.'", error.getMessage()); } // --- values --- From 4b96f54d8411b9618bff16159934a3d08a596f9e Mon Sep 17 00:00:00 2001 From: Thibault Nadin Date: Wed, 5 Aug 2026 17:47:38 +0200 Subject: [PATCH 4/5] Keep the intake worker alive when a batch cannot be serialized --- .../datadog/trace/llmobs/LLMObsIntakeWorker.java | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/LLMObsIntakeWorker.java b/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/LLMObsIntakeWorker.java index 434d1dd063e..68916a33fe2 100644 --- a/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/LLMObsIntakeWorker.java +++ b/dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/LLMObsIntakeWorker.java @@ -175,7 +175,20 @@ protected void flushIfNecessary() { if (shouldFlush()) { HttpRetryPolicy.Factory retryPolicyFactory = new HttpRetryPolicy.Factory(5, 100, 2.0, true); - String reqBod = serializer.toJson(this.buffer); + String reqBod; + try { + reqBod = serializer.toJson(this.buffer); + } catch (Exception e) { + // A batch that cannot be serialized will never serialize, so it is dropped rather than + // retried. Letting this escape would kill the worker and strand every later payload. + log.error( + "Could not serialize {} payloads, dropping {} of them", + payloadDescription, + this.buffer.size(), + e); + this.buffer.clear(); + return; + } RequestBody requestBody = RequestBody.create(okhttp3.MediaType.parse("application/json"), reqBod); From 894040eda9e87816b5794347c6ef61aa0cfcf843 Mon Sep 17 00:00:00 2001 From: Thibault Nadin Date: Wed, 5 Aug 2026 17:47:40 +0200 Subject: [PATCH 5/5] Reject non-finite feedback scores and snapshot json values --- .../java/datadog/trace/api/llmobs/LLMObs.java | 11 +++++++++-- .../trace/api/llmobs/LLMObsFeedbackTest.java | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/dd-trace-api/src/main/java/datadog/trace/api/llmobs/LLMObs.java b/dd-trace-api/src/main/java/datadog/trace/api/llmobs/LLMObs.java index 79465f1a321..2a22bf7eadb 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/llmobs/LLMObs.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/llmobs/LLMObs.java @@ -593,10 +593,13 @@ public Builder categoricalValue(@Nonnull String value) { /** * Sets a numeric value. * - * @param value the value + * @param value the value, must be finite as JSON has no representation for NaN nor infinity * @return this builder */ public Builder scoreValue(double value) { + if (!Double.isFinite(value)) { + return fail("invalid_metric_value", "score value must be finite"); + } return value(MetricType.SCORE, value); } @@ -617,7 +620,11 @@ public Builder booleanValue(boolean value) { * @return this builder */ public Builder jsonValue(@Nonnull Map value) { - return value(MetricType.JSON, value); + // Serialization happens later, on the submission worker, so the caller-owned map is + // snapshotted here to keep the submitted value stable, the same way tags are. + return value( + MetricType.JSON, + value == null ? null : Collections.unmodifiableMap(new HashMap<>(value))); } /** diff --git a/dd-trace-api/src/test/java/datadog/trace/api/llmobs/LLMObsFeedbackTest.java b/dd-trace-api/src/test/java/datadog/trace/api/llmobs/LLMObsFeedbackTest.java index 5dc07a11965..b55cf7abd5f 100644 --- a/dd-trace-api/src/test/java/datadog/trace/api/llmobs/LLMObsFeedbackTest.java +++ b/dd-trace-api/src/test/java/datadog/trace/api/llmobs/LLMObsFeedbackTest.java @@ -280,6 +280,24 @@ void testSameValueKindSetTwiceIsRejected() { "invalid_metric_type", LLMObs.Feedback.builder().scoreValue(0.1).scoreValue(0.2)); } + @Test + void testNonFiniteScoresAreRejected() { + assertRejected("invalid_metric_value", LLMObs.Feedback.builder().scoreValue(Double.NaN)); + assertRejected( + "invalid_metric_value", LLMObs.Feedback.builder().scoreValue(Double.POSITIVE_INFINITY)); + assertRejected( + "invalid_metric_value", LLMObs.Feedback.builder().scoreValue(Double.NEGATIVE_INFINITY)); + LLMObs.Feedback largest = + LLMObs.Feedback.builder() + .spanId("123") + .label("thumbs") + .scoreValue(Double.MAX_VALUE) + .submitter("user-123", null) + .build(); + assertNull(largest.validate()); + assertEquals(Double.MAX_VALUE, largest.getValue()); + } + @Test void testNullValuesAreRejected() { assertRejected("invalid_metric_value", LLMObs.Feedback.builder().categoricalValue(null));