diff --git a/AGENTS.md b/AGENTS.md index 42ee2aaf..75db5c5b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -160,11 +160,12 @@ VCR_MODE=record ./gradlew :braintrust-sdk:test --tests 'dev.braintrust.devserver - when running btx, use the spec filter to target what is specifically under development: `VCR_MODE=off ./gradlew :btx:test -Pbtx.spec.filter=openai/prompt_cach --rerun` - don't reformat the whole repo, but do run `./gradlew spotlessApply` on files you changed before committing. the pre-commit hook and `./gradlew check` both run `spotlessCheck`, which fails on unformatted code. -## Gotchas +## Misc Tips and Best Practices - **don't hand-edit cassettes.** they're content-hashed and guarded against committed secrets. a failing VCR test means the recorded interaction changed — re-record it (see the VCR section), don't patch the json. - **`braintrust-api` is generated code.** don't edit sources under it by hand; it's regenerated from the braintrust openapi spec pinned as `braintrustOpenApiRef` in gradle.properties. - **there are no version constants to bump.** the sdk version is derived from git tags at build time (`generateVersion()` in build.gradle) and written into braintrust.properties. "bump the version" is not a source change. +- When adding test cases, favor adding to the test file of the module being changed rather than making a new file. For example, if you fix a bug in the `Foo` module, add the test case to `FooTest.java` instead of making a new file, `FooTestMyBuggyCase.java` ## Releasing diff --git a/braintrust-sdk/instrumentation/anthropic_2_2_0/src/main/java/dev/braintrust/instrumentation/anthropic/v2_2_0/ResponseReassembler.java b/braintrust-sdk/instrumentation/anthropic_2_2_0/src/main/java/dev/braintrust/instrumentation/anthropic/v2_2_0/ResponseReassembler.java new file mode 100644 index 00000000..5decf031 --- /dev/null +++ b/braintrust-sdk/instrumentation/anthropic_2_2_0/src/main/java/dev/braintrust/instrumentation/anthropic/v2_2_0/ResponseReassembler.java @@ -0,0 +1,117 @@ +package dev.braintrust.instrumentation.anthropic.v2_2_0; + +import com.anthropic.helpers.MessageAccumulator; +import com.anthropic.models.messages.RawMessageStreamEvent; +import dev.braintrust.json.BraintrustJsonMapper; +import java.io.BufferedReader; +import java.io.ByteArrayInputStream; +import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; +import javax.annotation.Nullable; +import lombok.extern.slf4j.Slf4j; + +/** + * Turns the raw bytes of an Anthropic response into a single JSON document the semconv layer can + * tag. + * + *

All of the wire-format bookkeeping lives here — SSE-vs-plain-JSON detection and chunk + * reassembly — so that {@code TracingHttpClient} is left holding only the span lifecycle and one + * flat call into {@code InstrumentationSemConv}. + */ +@Slf4j +class ResponseReassembler { + + private ResponseReassembler() {} + + /** + * A reassembled response body plus the timing that belongs with it. + * + *

{@code body} is null when there was nothing usable to reassemble — an empty response, or + * one we couldn't parse. Callers should still tag the response in that case; the headers remain + * worth recording. + * + *

{@code timeToFirstTokenNanos} is only populated for a stream, since a non-streaming + * response has no first token to time. + */ + record Result(@Nullable String body, @Nullable Long timeToFirstTokenNanos) { + static final Result EMPTY = new Result(null, null); + } + + /** Detects the wire format and reassembles accordingly. Never throws. */ + static Result reassemble(byte[] bytes, long timeToFirstTokenNanos) { + if (bytes.length == 0) { + return Result.EMPTY; + } + try { + String firstLine = firstNonEmptyLine(bytes); + // Anthropic SSE starts with "event: message_start\ndata: ..." so we detect either + // prefix. OpenAI SSE starts directly with "data:". + boolean isSse = + firstLine != null + && (firstLine.startsWith("data:") || firstLine.startsWith("event:")); + if (isSse) { + return new Result(reassembleSse(bytes), timeToFirstTokenNanos); + } + // Non-streaming: plain Message JSON — pass it whole, no time_to_first_token + return new Result(new String(bytes, StandardCharsets.UTF_8), null); + } catch (Exception e) { + log.error("Could not reassemble Anthropic response buffer", e); + return Result.EMPTY; + } + } + + @Nullable + private static String firstNonEmptyLine(byte[] bytes) { + int start = 0; + for (int i = 0; i <= bytes.length; i++) { + if (i == bytes.length || bytes[i] == '\n') { + String line = new String(bytes, start, i - start, StandardCharsets.UTF_8).strip(); + if (!line.isEmpty()) return line; + start = i + 1; + } + } + return null; + } + + /** + * Anthropic SSE wire format has named events: + * + *

+     * event: message_start
+     * data: {"type":"message_start","message":{...}}
+     *
+     * event: content_block_delta
+     * data: {"type":"content_block_delta","index":0,"delta":{"type":"text_delta","text":"Hi"}}
+     * 
+ * + * We only need the {@code data:} lines — the event name is redundant with the {@code type} + * field inside the JSON. Feed each data payload to {@link MessageAccumulator} and serialize the + * assembled {@link com.anthropic.models.messages.Message}. + */ + @Nullable + private static String reassembleSse(byte[] sseBytes) { + try { + var mapper = BraintrustJsonMapper.get(); + var reader = + new BufferedReader( + new InputStreamReader( + new ByteArrayInputStream(sseBytes), StandardCharsets.UTF_8)); + var accumulator = MessageAccumulator.create(); + String line; + while ((line = reader.readLine()) != null) { + if (!line.startsWith("data:")) continue; + String data = line.substring("data:".length()).strip(); + if (data.isEmpty()) continue; + try { + accumulator.accumulate(mapper.readValue(data, RawMessageStreamEvent.class)); + } catch (Exception ignored) { + // skip unrecognized event types (e.g. ping) + } + } + return BraintrustJsonMapper.toJson(accumulator.message()); + } catch (Exception e) { + log.error("Could not parse Anthropic SSE buffer to tag streaming span output", e); + return null; + } + } +} diff --git a/braintrust-sdk/instrumentation/anthropic_2_2_0/src/main/java/dev/braintrust/instrumentation/anthropic/v2_2_0/TracingHttpClient.java b/braintrust-sdk/instrumentation/anthropic_2_2_0/src/main/java/dev/braintrust/instrumentation/anthropic/v2_2_0/TracingHttpClient.java index 7fc9f905..65dd2fe8 100644 --- a/braintrust-sdk/instrumentation/anthropic_2_2_0/src/main/java/dev/braintrust/instrumentation/anthropic/v2_2_0/TracingHttpClient.java +++ b/braintrust-sdk/instrumentation/anthropic_2_2_0/src/main/java/dev/braintrust/instrumentation/anthropic/v2_2_0/TracingHttpClient.java @@ -5,11 +5,8 @@ import com.anthropic.core.http.HttpRequest; import com.anthropic.core.http.HttpRequestBody; import com.anthropic.core.http.HttpResponse; -import com.anthropic.helpers.MessageAccumulator; -import com.anthropic.models.messages.RawMessageStreamEvent; import dev.braintrust.bootstrap.BraintrustBridge; import dev.braintrust.instrumentation.InstrumentationSemConv; -import dev.braintrust.json.BraintrustJsonMapper; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanContext; @@ -17,13 +14,13 @@ import io.opentelemetry.api.trace.TraceState; import io.opentelemetry.api.trace.Tracer; import io.opentelemetry.context.Context; -import java.io.BufferedReader; -import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.InputStream; -import java.io.InputStreamReader; import java.io.OutputStream; import java.nio.charset.StandardCharsets; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; @@ -125,7 +122,9 @@ public void close() { bufferedRequest.baseUrl() != null ? bufferedRequest.baseUrl() : "", bufferedRequest.pathSegments(), bufferedRequest.method().name(), - inputJson); + inputJson, + null, + headersAsMap(bufferedRequest.headers())); var response = underlying.execute(bufferedRequest, requestOptions); return new TeeingStreamHttpResponse(response, span, tracer); @@ -153,7 +152,9 @@ public void close() { bufferedRequest.baseUrl() != null ? bufferedRequest.baseUrl() : "", bufferedRequest.pathSegments(), bufferedRequest.method().name(), - inputJson); + inputJson, + null, + headersAsMap(bufferedRequest.headers())); return underlying .executeAsync(bufferedRequest, requestOptions) .thenApply( @@ -264,9 +265,39 @@ private void onStreamClosed() { synchronized (teeBuffer) { bytes = teeBuffer.toByteArray(); } + + // Recorded before tagging: the anthropic sdk raises above this layer, so the + // error status is ours alone to set, and losing it to a body-parsing problem is + // worse than losing the parsed output. + // Anything outside 2xx, not just 4xx/5xx: both vendor SDKs treat success as + // exactly 200..299, so a final 3xx that the http client did not follow (a 304, or + // a redirect with no usable Location) is raised to the caller as an + // UnexpectedStatusCodeException and must mark the span failed too. + int statusCode = delegate.statusCode(); + if (statusCode < 200 || statusCode >= 300) { + InstrumentationSemConv.tagLLMSpanHttpError( + span, statusCode, new String(bytes, StandardCharsets.UTF_8)); + } + + // Wire-format bookkeeping lives in ResponseReassembler; this hands semconv + // everything the response carried in one flat call. A null body (empty or + // unparseable response) still tags the headers. // tagLLMSpanResponse also emits child spans for any server-side tool calls (web // search, etc.) nested under the LLM span while it is still live. - tagSpanFromBuffer(tracer, span, bytes, timeToFirstTokenNanos.get()); + try { + var reassembled = + ResponseReassembler.reassemble(bytes, timeToFirstTokenNanos.get()); + InstrumentationSemConv.tagLLMSpanResponse( + tracer, + span, + InstrumentationSemConv.PROVIDER_NAME_ANTHROPIC, + reassembled.body(), + reassembled.timeToFirstTokenNanos(), + headersAsMap(delegate.headers())); + } catch (Exception e) { + // Observability must never change the response behavior seen by the caller. + log.error("Could not tag span from response buffer", e); + } } finally { span.end(); } @@ -360,89 +391,25 @@ private void notifyClosed() { // Span tagging from buffered bytes // ------------------------------------------------------------------------- - private static void tagSpanFromBuffer( - Tracer tracer, Span span, byte[] bytes, Long timeToFirstTokenNanos) { - if (bytes.length == 0) return; - try { - String firstLine = firstNonEmptyLine(bytes); - // Anthropic SSE starts with "event: message_start\ndata: ..." so we detect - // either prefix. OpenAI SSE starts directly with "data:". - boolean isSse = - firstLine != null - && (firstLine.startsWith("data:") || firstLine.startsWith("event:")); - if (isSse) { - tagSpanFromSseBytes(tracer, span, bytes, timeToFirstTokenNanos); - } else { - // Non-streaming: plain Message JSON — pass it whole, no time_to_first_token - String responseJson = new String(bytes, StandardCharsets.UTF_8); - InstrumentationSemConv.tagLLMSpanResponse( - tracer, - span, - InstrumentationSemConv.PROVIDER_NAME_ANTHROPIC, - responseJson, - null); - } - } catch (Exception e) { - log.error("Could not tag span from Anthropic response buffer", e); - } - } - - private static String firstNonEmptyLine(byte[] bytes) { - int start = 0; - for (int i = 0; i <= bytes.length; i++) { - if (i == bytes.length || bytes[i] == '\n') { - String line = new String(bytes, start, i - start, StandardCharsets.UTF_8).strip(); - if (!line.isEmpty()) return line; - start = i + 1; - } - } - return null; - } - /** - * Anthropic SSE wire format has named events: - * - *
-     * event: message_start
-     * data: {"type":"message_start","message":{...}}
-     *
-     * event: content_block_delta
-     * data: {"type":"content_block_delta","index":0,"delta":{"type":"text_delta","text":"Hi"}}
-     * 
- * - * We only need the {@code data:} lines — the event name is redundant with the {@code type} - * field inside the JSON. Feed each data payload to {@link MessageAccumulator} and serialize the - * assembled {@link com.anthropic.models.messages.Message} for the span. + * Adapts the anthropic sdk's {@code Headers} to the vendor-neutral shape {@link + * InstrumentationSemConv} consumes. Returns an empty map on failure so a header-shape change + * can never take down the tagging that follows it. */ - private static void tagSpanFromSseBytes( - Tracer tracer, Span span, byte[] sseBytes, Long timeToFirstTokenNanos) { + private static Map> headersAsMap( + @Nullable com.anthropic.core.http.Headers headers) { + if (headers == null) { + return Map.of(); + } try { - var mapper = BraintrustJsonMapper.get(); - var reader = - new BufferedReader( - new InputStreamReader( - new ByteArrayInputStream(sseBytes), StandardCharsets.UTF_8)); - var accumulator = MessageAccumulator.create(); - String line; - while ((line = reader.readLine()) != null) { - if (!line.startsWith("data:")) continue; - String data = line.substring("data:".length()).strip(); - if (data.isEmpty()) continue; - try { - accumulator.accumulate(mapper.readValue(data, RawMessageStreamEvent.class)); - } catch (Exception ignored) { - // skip unrecognized event types (e.g. ping) - } + var map = new HashMap>(); + for (String name : headers.names()) { + map.put(name, headers.values(name)); } - String assembledMessageJson = BraintrustJsonMapper.toJson(accumulator.message()); - InstrumentationSemConv.tagLLMSpanResponse( - tracer, - span, - InstrumentationSemConv.PROVIDER_NAME_ANTHROPIC, - assembledMessageJson, - timeToFirstTokenNanos); + return map; } catch (Exception e) { - log.error("Could not parse Anthropic SSE buffer to tag streaming span output", e); + log.debug("could not read headers", e); + return Map.of(); } } } diff --git a/braintrust-sdk/instrumentation/anthropic_2_2_0/src/main/java/dev/braintrust/instrumentation/anthropic/v2_2_0/auto/AnthropicInstrumentationModule.java b/braintrust-sdk/instrumentation/anthropic_2_2_0/src/main/java/dev/braintrust/instrumentation/anthropic/v2_2_0/auto/AnthropicInstrumentationModule.java index 24e4a137..da387a39 100644 --- a/braintrust-sdk/instrumentation/anthropic_2_2_0/src/main/java/dev/braintrust/instrumentation/anthropic/v2_2_0/auto/AnthropicInstrumentationModule.java +++ b/braintrust-sdk/instrumentation/anthropic_2_2_0/src/main/java/dev/braintrust/instrumentation/anthropic/v2_2_0/auto/AnthropicInstrumentationModule.java @@ -36,6 +36,8 @@ public List getHelperClassNames() { MANUAL_INSTRUMENTATION_PACKAGE + "TracingHttpClient$ExtractedRequest", MANUAL_INSTRUMENTATION_PACKAGE + "BraintrustAnthropic", MANUAL_INSTRUMENTATION_PACKAGE + "ContextCapturingProxy", + MANUAL_INSTRUMENTATION_PACKAGE + "ResponseReassembler", + MANUAL_INSTRUMENTATION_PACKAGE + "ResponseReassembler$Result", "dev.braintrust.json.BraintrustJsonMapper", "dev.braintrust.instrumentation.InstrumentationSemConv"); } diff --git a/braintrust-sdk/instrumentation/anthropic_2_2_0/src/test/java/dev/braintrust/instrumentation/anthropic/v2_2_0/BraintrustAnthropicTest.java b/braintrust-sdk/instrumentation/anthropic_2_2_0/src/test/java/dev/braintrust/instrumentation/anthropic/v2_2_0/BraintrustAnthropicTest.java index 6778a354..54227f23 100644 --- a/braintrust-sdk/instrumentation/anthropic_2_2_0/src/test/java/dev/braintrust/instrumentation/anthropic/v2_2_0/BraintrustAnthropicTest.java +++ b/braintrust-sdk/instrumentation/anthropic_2_2_0/src/test/java/dev/braintrust/instrumentation/anthropic/v2_2_0/BraintrustAnthropicTest.java @@ -4,6 +4,11 @@ import com.anthropic.client.AnthropicClient; import com.anthropic.client.okhttp.AnthropicOkHttpClient; +import com.anthropic.core.RequestOptions; +import com.anthropic.core.http.Headers; +import com.anthropic.core.http.HttpMethod; +import com.anthropic.core.http.HttpRequest; +import com.anthropic.core.http.HttpResponse; import com.anthropic.models.messages.MessageCreateParams; import com.anthropic.models.messages.Model; import com.fasterxml.jackson.databind.JsonNode; @@ -11,12 +16,18 @@ import dev.braintrust.TestHarness; import dev.braintrust.instrumentation.Instrumenter; import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.trace.StatusCode; +import java.io.ByteArrayInputStream; +import java.io.InputStream; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; import lombok.SneakyThrows; import net.bytebuddy.agent.ByteBuddyAgent; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; public class BraintrustAnthropicTest { private static final String TEST_MODEL = "claude-haiku-4-5"; @@ -562,4 +573,196 @@ void testWrappedClientObjectContract() { wrapped.toString().contains("ContextCapturingProxy"), "toString should identify the proxy, got: " + wrapped); } + + /** + * Anthropic returns its correlation ID as {@code request-id} (no {@code x-} prefix, unlike + * OpenAI) and its object ID as {@code msg_*}. Both must land on the span for streaming and + * non-streaming alike — the header comes off the HTTP response, the object ID out of the + * reassembled body, so the two travel independent paths. + */ + @Test + @SneakyThrows + void testCorrelationIdsCaptured() { + AnthropicClient anthropicClient = + AnthropicOkHttpClient.builder() + .baseUrl(testHarness.anthropicBaseUrl()) + .apiKey(testHarness.anthropicApiKey()) + .build(); + + var request = + MessageCreateParams.builder() + .model(Model.of(TEST_MODEL)) + .system("You are a helpful assistant") + .addUserMessage("What is the capital of France?") + .maxTokens(50) + .temperature(0.0) + .build(); + + anthropicClient.messages().create(request); + assertAnthropicIdsCaptured(testHarness.awaitExportedSpans().get(0)); + } + + @Test + @SneakyThrows + void testCorrelationIdsCapturedStreaming() { + AnthropicClient anthropicClient = + AnthropicOkHttpClient.builder() + .baseUrl(testHarness.anthropicBaseUrl()) + .apiKey(testHarness.anthropicApiKey()) + .build(); + + var request = + MessageCreateParams.builder() + .model(Model.of(TEST_MODEL)) + .system("You are a helpful assistant") + .addUserMessage("What is the capital of France?") + .maxTokens(50) + .temperature(0.0) + .build(); + + try (var stream = anthropicClient.messages().createStreaming(request)) { + stream.stream().forEach(event -> {}); + } + assertAnthropicIdsCaptured(testHarness.awaitExportedSpans().get(0)); + } + + /** + * Asserts presence only for the header: its value is an opaque vendor string, so pinning its + * shape would encode an assumption the provider never made. + */ + private static void assertAnthropicIdsCaptured(io.opentelemetry.sdk.trace.data.SpanData span) { + var attributes = span.getAttributes(); + + String requestId = attributes.get(AttributeKey.stringKey("request-id")); + assertNotNull(requestId, "request-id header must be captured"); + assertFalse(requestId.isBlank(), "request-id must not be blank"); + + String responseId = attributes.get(AttributeKey.stringKey("response_id")); + assertNotNull(responseId, "response_id must be captured from the response body"); + assertTrue(responseId.startsWith("msg_"), "unexpected response_id: " + responseId); + + assertNull( + attributes.get(AttributeKey.stringKey("x-request-id")), + "OpenAI's header name must not appear on an Anthropic span"); + } + + // ------------------------------------------------------------------------- + // Status-code handling + // + // Driven against a stub delegate rather than the VCR proxy: a 3xx or a 1xx cannot + // realistically be recorded from the vendor, and those are exactly the statuses that + // distinguish "non-2xx" from "4xx and up". + // ------------------------------------------------------------------------- + + /** + * anthropic-java's ErrorHandler treats success as exactly 200..299 and raises everything else + * to the caller, so every one of these must mark the span failed. 304 is the realistic case: + * the http client does not follow it, so it arrives here as a final response. + */ + @ParameterizedTest(name = "status {0}") + @ValueSource(ints = {100, 204, 301, 304, 400, 500}) + @SneakyThrows + void nonSuccessStatusMarksSpanFailed(int statusCode) { + var client = + new TracingHttpClient( + testHarness.openTelemetry(), new StubHttpClient(statusCode, "{}")); + + try (var response = client.execute(messagesRequest(), RequestOptions.none())) { + response.body().readAllBytes(); + } + + var span = testHarness.awaitExportedSpans().get(0); + boolean isSuccess = statusCode >= 200 && statusCode < 300; + assertEquals( + isSuccess ? StatusCode.UNSET : StatusCode.ERROR, + span.getStatus().getStatusCode(), + "status " + + statusCode + + (isSuccess ? " should not" : " should") + + " mark the span failed"); + } + + /** The correlation header is still captured on a status the SDK will reject. */ + @Test + @SneakyThrows + void requestIdIsCapturedOnANonSuccessStatus() { + var client = + new TracingHttpClient(testHarness.openTelemetry(), new StubHttpClient(304, "")); + + try (var response = client.execute(messagesRequest(), RequestOptions.none())) { + response.body().readAllBytes(); + } + + var span = testHarness.awaitExportedSpans().get(0); + assertEquals( + "stubbed-request-id", + span.getAttributes().get(AttributeKey.stringKey("request-id")), + "an empty-bodied non-2xx must still yield the vendor request id"); + } + + @Test + void nonJsonResponseTaggingIsBestEffort() { + var client = + new TracingHttpClient( + testHarness.openTelemetry(), + new StubHttpClient(502, "Bad Gateway")); + + assertDoesNotThrow( + () -> { + try (var response = client.execute(messagesRequest(), RequestOptions.none())) { + response.body().readAllBytes(); + } + }); + + var span = testHarness.awaitExportedSpans().get(0); + assertEquals(StatusCode.ERROR, span.getStatus().getStatusCode()); + assertEquals( + "stubbed-request-id", + span.getAttributes().get(AttributeKey.stringKey("request-id"))); + } + + private static HttpRequest messagesRequest() { + return HttpRequest.builder() + .method(HttpMethod.POST) + .baseUrl("https://api.openai.com/v1") + .addPathSegments("v1", "messages") + .build(); + } + + /** Returns a canned response; never touches the network. */ + private record StubHttpClient(int statusCode, String body) + implements com.anthropic.core.http.HttpClient { + + @Override + public HttpResponse execute(HttpRequest request, RequestOptions requestOptions) { + return new HttpResponse() { + @Override + public int statusCode() { + return statusCode; + } + + @Override + public Headers headers() { + return Headers.builder().put("request-id", "stubbed-request-id").build(); + } + + @Override + public InputStream body() { + return new ByteArrayInputStream(body.getBytes()); + } + + @Override + public void close() {} + }; + } + + @Override + public CompletableFuture executeAsync( + HttpRequest request, RequestOptions requestOptions) { + return CompletableFuture.completedFuture(execute(request, requestOptions)); + } + + @Override + public void close() {} + } } diff --git a/braintrust-sdk/instrumentation/langchain_1_14_0/src/main/java/dev/braintrust/instrumentation/langchain/v1_14_0/WrappedHttpClient.java b/braintrust-sdk/instrumentation/langchain_1_14_0/src/main/java/dev/braintrust/instrumentation/langchain/v1_14_0/WrappedHttpClient.java index cdac0cb0..4ea9ab3d 100644 --- a/braintrust-sdk/instrumentation/langchain_1_14_0/src/main/java/dev/braintrust/instrumentation/langchain/v1_14_0/WrappedHttpClient.java +++ b/braintrust-sdk/instrumentation/langchain_1_14_0/src/main/java/dev/braintrust/instrumentation/langchain/v1_14_0/WrappedHttpClient.java @@ -22,6 +22,8 @@ import java.time.Instant; import java.util.Arrays; import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; import lombok.extern.slf4j.Slf4j; @@ -53,7 +55,12 @@ public SuccessfulHttpResponse execute(HttpRequest request) tagRequest(span, request); var response = underlying.execute(request); InstrumentationSemConv.tagLLMSpanResponse( - tracer, span, options.providerName(), response.body()); + tracer, + span, + options.providerName(), + response.body(), + null, + response.headers()); return response; } catch (Throwable t) { InstrumentationSemConv.tagLLMSpanResponse(span, t); @@ -122,7 +129,14 @@ private void tagRequest(Span span, HttpRequest request) { List pathSegments = Arrays.stream(uri.getPath().split("/")).filter(s -> !s.isEmpty()).toList(); InstrumentationSemConv.tagLLMSpanRequest( - span, options.providerName(), baseUrl, pathSegments, "POST", request.body()); + span, + options.providerName(), + baseUrl, + pathSegments, + "POST", + request.body(), + null, + request.headers()); } catch (Exception e) { log.debug("Failed to tag request span", e); } @@ -153,6 +167,13 @@ static class WrappedServerSentEventListener implements ServerSentEventListener { // failure here is what stops onClose from finalizing a failed call as a successful span. @javax.annotation.Nullable private volatile String streamFailure; + // onOpen is the only point the SSE transport exposes response headers, but the body is + // not assembled until the stream closes — so they are copied aside here and handed to the + // semconv layer together with the body in finalizeSpan, potentially from another thread. + // A copy rather than the client's own map: we outlive the callback that handed it to us. + // Starts empty, which is also the right answer for a stream that errors before it opens. + private final Map> responseHeaders = new ConcurrentHashMap<>(); + WrappedServerSentEventListener( ServerSentEventListener delegate, Span span, String providerName, Tracer tracer) { this.delegate = delegate; @@ -164,10 +185,34 @@ static class WrappedServerSentEventListener implements ServerSentEventListener { @Override public void onOpen(SuccessfulHttpResponse response) { try (Scope ignored = span.makeCurrent()) { + captureResponseHeaders(response); delegate.onOpen(response); } } + /** + * Copies the response headers aside, entry by entry rather than in bulk: {@link + * ConcurrentHashMap} rejects null keys and values, and header maps from some HttpClient + * implementations carry a null key for the status line. Best-effort throughout — throwing + * here would break the caller's stream for the sake of a span attribute. + */ + private void captureResponseHeaders(SuccessfulHttpResponse response) { + try { + Map> headers = response.headers(); + if (headers == null) { + return; + } + headers.forEach( + (name, values) -> { + if (name != null && values != null) { + responseHeaders.put(name, values); + } + }); + } catch (Exception e) { + log.debug("could not capture response headers", e); + } + } + @Override public void onEvent(ServerSentEvent event, ServerSentEventContext context) { try (Scope ignored = span.makeCurrent()) { @@ -262,7 +307,7 @@ private void finalizeSpan() { Long ttft = timeToFirstTokenNanos(); String responseBody = accumulator.build(); InstrumentationSemConv.tagLLMSpanResponse( - tracer, span, providerName, responseBody, ttft); + tracer, span, providerName, responseBody, ttft, responseHeaders); } catch (Exception e) { log.debug("Failed to finalize streaming span", e); } diff --git a/braintrust-sdk/instrumentation/langchain_1_14_0/src/test/java/dev/braintrust/instrumentation/langchain/v1_14_0/BraintrustLangchainTest.java b/braintrust-sdk/instrumentation/langchain_1_14_0/src/test/java/dev/braintrust/instrumentation/langchain/v1_14_0/BraintrustLangchainTest.java index 49baf8b9..0eebba6a 100644 --- a/braintrust-sdk/instrumentation/langchain_1_14_0/src/test/java/dev/braintrust/instrumentation/langchain/v1_14_0/BraintrustLangchainTest.java +++ b/braintrust-sdk/instrumentation/langchain_1_14_0/src/test/java/dev/braintrust/instrumentation/langchain/v1_14_0/BraintrustLangchainTest.java @@ -86,6 +86,8 @@ void testSyncChatCompletion() { assertNotNull(metadataJson, "Metadata should be present"); JsonNode metadata = JSON_MAPPER.readTree(metadataJson); assertEquals("openai", metadata.get("provider").asText(), "Provider should be 'openai'"); + + assertOpenAiIdsCaptured(span); assertEquals( "gpt-4o-mini", metadata.get("model").asText(), "Model should be 'gpt-4o-mini'"); @@ -156,6 +158,8 @@ void testResponsesApi() { JSON_MAPPER.readTree(attributes.get(AttributeKey.stringKey("braintrust.metadata"))); assertEquals("openai", metadata.get("provider").asText(), "Provider should be 'openai'"); + assertOpenAiIdsCaptured(span); + String metricsJson = attributes.get(AttributeKey.stringKey("braintrust.metrics")); assertNotNull(metricsJson, "Metrics should be present"); JsonNode metrics = JSON_MAPPER.readTree(metricsJson); @@ -254,6 +258,8 @@ public void onError(Throwable error) { JSON_MAPPER.readTree(attributes.get(AttributeKey.stringKey("braintrust.metadata"))); assertEquals("openai", metadata.get("provider").asText(), "Provider should be 'openai'"); + assertOpenAiIdsCaptured(llmSpan); + String metricsJson = attributes.get(AttributeKey.stringKey("braintrust.metrics")); assertNotNull(metricsJson, "Metrics should be present"); JsonNode metrics = JSON_MAPPER.readTree(metricsJson); @@ -442,6 +448,8 @@ public void onError(Throwable error) { assertNotNull(metadataJson, "Metadata should be present"); JsonNode metadata = JSON_MAPPER.readTree(metadataJson); assertEquals("openai", metadata.get("provider").asText(), "Provider should be 'openai'"); + + assertOpenAiIdsCaptured(llmSpan); assertEquals( "gpt-4o-mini", metadata.get("model").asText(), "Model should be 'gpt-4o-mini'"); @@ -1078,4 +1086,24 @@ public String getForecast(String location, int days) { days, location); } } + + /** + * langchain4j surfaces response headers in two unrelated places — {@code execute()} for a + * blocking call and {@code onOpen()} for an SSE stream — so both are asserted, over both + * endpoints. The header value is checked for presence but not shape: OpenAI returns both {@code + * req_*} and bare UUIDs for it. + */ + private static void assertOpenAiIdsCaptured(SpanData span) { + var attributes = span.getAttributes(); + + String requestId = attributes.get(AttributeKey.stringKey("x-request-id")); + assertNotNull(requestId, "x-request-id header must be captured"); + assertFalse(requestId.isBlank(), "x-request-id must not be blank"); + + String responseId = attributes.get(AttributeKey.stringKey("response_id")); + assertNotNull(responseId, "response_id must be captured from the response body"); + assertTrue( + responseId.startsWith("resp_") || responseId.startsWith("chatcmpl-"), + "unexpected response_id: " + responseId); + } } diff --git a/braintrust-sdk/instrumentation/langchain_1_8_0/src/main/java/dev/braintrust/instrumentation/langchain/v1_8_0/WrappedHttpClient.java b/braintrust-sdk/instrumentation/langchain_1_8_0/src/main/java/dev/braintrust/instrumentation/langchain/v1_8_0/WrappedHttpClient.java index bff3fb45..da5f00ca 100644 --- a/braintrust-sdk/instrumentation/langchain_1_8_0/src/main/java/dev/braintrust/instrumentation/langchain/v1_8_0/WrappedHttpClient.java +++ b/braintrust-sdk/instrumentation/langchain_1_8_0/src/main/java/dev/braintrust/instrumentation/langchain/v1_8_0/WrappedHttpClient.java @@ -22,6 +22,8 @@ import java.net.URI; import java.util.Arrays; import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; import lombok.extern.slf4j.Slf4j; @@ -51,7 +53,12 @@ public SuccessfulHttpResponse execute(HttpRequest request) tagRequest(span, request); var response = underlying.execute(request); InstrumentationSemConv.tagLLMSpanResponse( - tracer, span, options.providerName(), response.body()); + tracer, + span, + options.providerName(), + response.body(), + null, + response.headers()); return response; } catch (Throwable t) { InstrumentationSemConv.tagLLMSpanResponse(span, t); @@ -116,7 +123,14 @@ private void tagRequest(Span span, HttpRequest request) { List pathSegments = Arrays.stream(uri.getPath().split("/")).filter(s -> !s.isEmpty()).toList(); InstrumentationSemConv.tagLLMSpanRequest( - span, options.providerName(), baseUrl, pathSegments, "POST", request.body()); + span, + options.providerName(), + baseUrl, + pathSegments, + "POST", + request.body(), + null, + request.headers()); } catch (Exception e) { log.debug("Failed to tag request span", e); } @@ -137,6 +151,13 @@ static class WrappedServerSentEventListener implements ServerSentEventListener { // failure here is what stops onClose from finalizing a failed call as a successful span. @javax.annotation.Nullable private volatile String streamFailure; + // onOpen is the only point the SSE transport exposes response headers, but the body is + // not assembled until the stream closes — so they are copied aside here and handed to the + // semconv layer together with the body in finalizeSpan, potentially from another thread. + // A copy rather than the client's own map: we outlive the callback that handed it to us. + // Starts empty, which is also the right answer for a stream that errors before it opens. + private final Map> responseHeaders = new ConcurrentHashMap<>(); + WrappedServerSentEventListener( ServerSentEventListener delegate, Span span, String providerName, Tracer tracer) { this.delegate = delegate; @@ -148,10 +169,34 @@ static class WrappedServerSentEventListener implements ServerSentEventListener { @Override public void onOpen(SuccessfulHttpResponse response) { try (Scope ignored = span.makeCurrent()) { + captureResponseHeaders(response); delegate.onOpen(response); } } + /** + * Copies the response headers aside, entry by entry rather than in bulk: {@link + * ConcurrentHashMap} rejects null keys and values, and header maps from some HttpClient + * implementations carry a null key for the status line. Best-effort throughout — throwing + * here would break the caller's stream for the sake of a span attribute. + */ + private void captureResponseHeaders(SuccessfulHttpResponse response) { + try { + Map> headers = response.headers(); + if (headers == null) { + return; + } + headers.forEach( + (name, values) -> { + if (name != null && values != null) { + responseHeaders.put(name, values); + } + }); + } catch (Exception e) { + log.debug("could not capture response headers", e); + } + } + @Override public void onEvent(ServerSentEvent event, ServerSentEventContext context) { try (Scope ignored = span.makeCurrent()) { @@ -216,7 +261,7 @@ private void finalizeSpan() { Long ttft = elapsed != 0L ? elapsed : null; String responseBody = accumulator.build(); InstrumentationSemConv.tagLLMSpanResponse( - tracer, span, providerName, responseBody, ttft); + tracer, span, providerName, responseBody, ttft, responseHeaders); } catch (Exception e) { log.debug("Failed to finalize streaming span", e); } diff --git a/braintrust-sdk/instrumentation/langchain_1_8_0/src/test/java/dev/braintrust/instrumentation/langchain/v1_8_0/BraintrustLangchainTest.java b/braintrust-sdk/instrumentation/langchain_1_8_0/src/test/java/dev/braintrust/instrumentation/langchain/v1_8_0/BraintrustLangchainTest.java index cf84bf74..ed9a992d 100644 --- a/braintrust-sdk/instrumentation/langchain_1_8_0/src/test/java/dev/braintrust/instrumentation/langchain/v1_8_0/BraintrustLangchainTest.java +++ b/braintrust-sdk/instrumentation/langchain_1_8_0/src/test/java/dev/braintrust/instrumentation/langchain/v1_8_0/BraintrustLangchainTest.java @@ -84,6 +84,8 @@ void testSyncChatCompletion() { assertNotNull(metadataJson, "Metadata should be present"); JsonNode metadata = JSON_MAPPER.readTree(metadataJson); assertEquals("openai", metadata.get("provider").asText(), "Provider should be 'openai'"); + + assertOpenAiIdsCaptured(span); assertEquals( "gpt-4o-mini", metadata.get("model").asText(), "Model should be 'gpt-4o-mini'"); @@ -212,6 +214,8 @@ public void onError(Throwable error) { assertNotNull(metadataJson, "Metadata should be present"); JsonNode metadata = JSON_MAPPER.readTree(metadataJson); assertEquals("openai", metadata.get("provider").asText(), "Provider should be 'openai'"); + + assertOpenAiIdsCaptured(llmSpan); assertEquals( "gpt-4o-mini", metadata.get("model").asText(), "Model should be 'gpt-4o-mini'"); @@ -575,4 +579,23 @@ public String getForecast(String location, int days) { days, location); } } + + /** + * Covers both places langchain4j surfaces response headers: {@code execute()} for a blocking + * call and {@code onOpen()} for an SSE stream. Presence only for the header — OpenAI returns + * both {@code req_*} and bare UUIDs for it. + */ + private static void assertOpenAiIdsCaptured(SpanData span) { + var attributes = span.getAttributes(); + + String requestId = attributes.get(AttributeKey.stringKey("x-request-id")); + assertNotNull(requestId, "x-request-id header must be captured"); + assertFalse(requestId.isBlank(), "x-request-id must not be blank"); + + String responseId = attributes.get(AttributeKey.stringKey("response_id")); + assertNotNull(responseId, "response_id must be captured from the response body"); + assertTrue( + responseId.startsWith("resp_") || responseId.startsWith("chatcmpl-"), + "unexpected response_id: " + responseId); + } } diff --git a/braintrust-sdk/instrumentation/openai_2_15_0/src/main/java/dev/braintrust/instrumentation/openai/v2_15_0/ResponseReassembler.java b/braintrust-sdk/instrumentation/openai_2_15_0/src/main/java/dev/braintrust/instrumentation/openai/v2_15_0/ResponseReassembler.java new file mode 100644 index 00000000..fd95e6ef --- /dev/null +++ b/braintrust-sdk/instrumentation/openai_2_15_0/src/main/java/dev/braintrust/instrumentation/openai/v2_15_0/ResponseReassembler.java @@ -0,0 +1,139 @@ +package dev.braintrust.instrumentation.openai.v2_15_0; + +import com.fasterxml.jackson.databind.json.JsonMapper; +import com.openai.core.ObjectMappers; +import com.openai.helpers.ChatCompletionAccumulator; +import com.openai.helpers.ResponseAccumulator; +import com.openai.models.chat.completions.ChatCompletionChunk; +import com.openai.models.responses.ResponseStreamEvent; +import dev.braintrust.json.BraintrustJsonMapper; +import java.io.BufferedReader; +import java.io.ByteArrayInputStream; +import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; +import javax.annotation.Nullable; +import lombok.extern.slf4j.Slf4j; + +/** + * Turns the raw bytes of an OpenAI response into a single JSON document the semconv layer can tag. + * + *

All of the wire-format bookkeeping lives here — SSE-vs-plain-JSON detection, per-endpoint + * accumulator selection, chunk reassembly — so that {@code TracingHttpClient} is left holding only + * the span lifecycle and one flat call into {@code InstrumentationSemConv}. + */ +@Slf4j +class ResponseReassembler { + private static final JsonMapper JSON_MAPPER = ObjectMappers.jsonMapper(); + + private ResponseReassembler() {} + + /** + * A reassembled response body plus the timing that belongs with it. + * + *

{@code body} is null when there was nothing usable to reassemble — an empty response, or + * an SSE stream whose shape we don't recognize. Callers should still tag the response in that + * case; the headers remain worth recording. + * + *

{@code timeToFirstTokenNanos} is only populated for a stream, since a non-streaming + * response has no first token to time and a zero would land in latency aggregates as if it were + * a real measurement. + */ + record Result(@Nullable String body, @Nullable Long timeToFirstTokenNanos) { + static final Result EMPTY = new Result(null, null); + } + + /** Detects the wire format and reassembles accordingly. Never throws. */ + static Result reassemble(byte[] bytes, long timeToFirstTokenNanos) { + if (bytes.length == 0) { + return Result.EMPTY; + } + try { + String firstLine = firstNonEmptyLine(bytes); + boolean isSse = + firstLine != null + && (firstLine.startsWith("data:") || firstLine.startsWith("event:")); + if (isSse) { + return new Result(reassembleSse(bytes), timeToFirstTokenNanos); + } + return new Result(new String(bytes, StandardCharsets.UTF_8), null); + } catch (Exception e) { + log.error("Could not reassemble OpenAI response buffer", e); + return Result.EMPTY; + } + } + + @Nullable + private static String firstNonEmptyLine(byte[] bytes) { + int start = 0; + for (int i = 0; i <= bytes.length; i++) { + if (i == bytes.length || bytes[i] == '\n') { + String line = new String(bytes, start, i - start, StandardCharsets.UTF_8).strip(); + if (!line.isEmpty()) return line; + start = i + 1; + } + } + return null; + } + + /** + * Parses SSE wire bytes and feeds each {@code data:} chunk through the accumulator matching the + * stream's shape, returning the reassembled response JSON — or null if the shape was not + * recognized. + */ + @Nullable + private static String reassembleSse(byte[] sseBytes) { + try { + var reader = + new BufferedReader( + new InputStreamReader( + new ByteArrayInputStream(sseBytes), StandardCharsets.UTF_8)); + String line; + String responseJson = null; + while ((line = reader.readLine()) != null) { + if (!line.startsWith("data:")) continue; + var firstEventJson = line.substring("data:".length()).strip(); + // after the first data chunk is found, read the rest of the stream with the proper + // accumulator type + var jsonTree = JSON_MAPPER.readTree(firstEventJson); + if (jsonTree.has("type") && jsonTree.get("type").asText().startsWith("response")) { + // response API SSEvents + ResponseAccumulator accumulator = ResponseAccumulator.create(); + accumulator.accumulate( + JSON_MAPPER.readValue(firstEventJson, ResponseStreamEvent.class)); + while ((line = reader.readLine()) != null) { + if (!line.startsWith("data:")) continue; + String data = line.substring("data:".length()).strip(); + if (data.isEmpty() || data.equals("[DONE]")) continue; + ResponseStreamEvent rse = + JSON_MAPPER.readValue(data, ResponseStreamEvent.class); + accumulator.accumulate(rse); + } + responseJson = JSON_MAPPER.writeValueAsString(accumulator.response()); + } else if (jsonTree.has("object") + && jsonTree.get("object").asText().equals("chat.completion.chunk")) { + // completions API SSEvents + var accumulator = ChatCompletionAccumulator.create(); + accumulator.accumulate( + JSON_MAPPER.readValue(firstEventJson, ChatCompletionChunk.class)); + while ((line = reader.readLine()) != null) { + if (!line.startsWith("data:")) continue; + String data = line.substring("data:".length()).strip(); + if (data.isEmpty() || data.equals("[DONE]")) continue; + ChatCompletionChunk chunk = + BraintrustJsonMapper.get() + .readValue(data, ChatCompletionChunk.class); + accumulator.accumulate(chunk); + } + responseJson = JSON_MAPPER.writeValueAsString(accumulator.chatCompletion()); + } else { + log.warn("unknown SSE object {}", firstEventJson); + } + break; + } + return responseJson; + } catch (Exception e) { + log.error("Could not parse SSE buffer to tag streaming span output", e); + return null; + } + } +} diff --git a/braintrust-sdk/instrumentation/openai_2_15_0/src/main/java/dev/braintrust/instrumentation/openai/v2_15_0/TracingHttpClient.java b/braintrust-sdk/instrumentation/openai_2_15_0/src/main/java/dev/braintrust/instrumentation/openai/v2_15_0/TracingHttpClient.java index 2d4d958b..dcb1f017 100644 --- a/braintrust-sdk/instrumentation/openai_2_15_0/src/main/java/dev/braintrust/instrumentation/openai/v2_15_0/TracingHttpClient.java +++ b/braintrust-sdk/instrumentation/openai_2_15_0/src/main/java/dev/braintrust/instrumentation/openai/v2_15_0/TracingHttpClient.java @@ -4,13 +4,8 @@ import com.openai.core.ObjectMappers; import com.openai.core.RequestOptions; import com.openai.core.http.*; -import com.openai.helpers.ChatCompletionAccumulator; -import com.openai.helpers.ResponseAccumulator; -import com.openai.models.chat.completions.ChatCompletionChunk; -import com.openai.models.responses.ResponseStreamEvent; import dev.braintrust.bootstrap.BraintrustBridge; import dev.braintrust.instrumentation.InstrumentationSemConv; -import dev.braintrust.json.BraintrustJsonMapper; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanContext; @@ -20,6 +15,9 @@ import io.opentelemetry.context.Context; import java.io.*; import java.nio.charset.StandardCharsets; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; @@ -68,7 +66,7 @@ private static ExtractedRequest extractCallerContext(HttpRequest request) { Context context = contextFromTraceparent(values.get(0)); HttpRequest stripped = request.toBuilder() - .replaceHeaders(ContextCapturingProxy.CONTEXT_HEADER, java.util.List.of()) + .replaceHeaders(ContextCapturingProxy.CONTEXT_HEADER, List.of()) .build(); return new ExtractedRequest(stripped, context); } @@ -126,7 +124,9 @@ public void close() { bufferedRequest.baseUrl() != null ? bufferedRequest.baseUrl() : "", bufferedRequest.pathSegments(), bufferedRequest.method().name(), - inputJson); + inputJson, + null, + headersAsMap(bufferedRequest.headers())); var response = underlying.execute(bufferedRequest, requestOptions); // Always tee the response body. onStreamClosed() detects whether the collected // bytes are SSE or plain JSON and tags the span accordingly. @@ -155,7 +155,9 @@ public void close() { bufferedRequest.baseUrl() != null ? bufferedRequest.baseUrl() : "", bufferedRequest.pathSegments(), bufferedRequest.method().name(), - inputJson); + inputJson, + null, + headersAsMap(bufferedRequest.headers())); return underlying .executeAsync(bufferedRequest, requestOptions) .thenApply( @@ -237,104 +239,30 @@ private static String readBodyAsString(HttpRequestBody body) { } /** - * Tags the span from bytes collected by {@link TeeingStreamHttpResponse}. Auto-detects whether - * the bytes are an SSE stream (first non-empty line starts with {@code "data: "}) or a plain - * JSON response, and parses accordingly. + * The response body as a single JSON document, plus the timing that goes with it. Reassembly is + * kept separate from tagging so that {@link TeeingStreamHttpResponse#onStreamClosed} can hand + * the semconv layer everything about the response in one call — a body we couldn't produce + * becomes a null {@code body} rather than a skipped tagging call, which is what keeps the + * response headers from being lost on an empty or unrecognized body. */ - private static void tagSpanFromBuffer( - Tracer tracer, Span span, byte[] bytes, Long timeToFirstTokenNanos) { - if (bytes.length == 0) return; - try { - String firstLine = firstNonEmptyLine(bytes); - if (firstLine != null - && (firstLine.startsWith("data:") || firstLine.startsWith("event:"))) { - tagSpanFromSseBytes(tracer, span, bytes, timeToFirstTokenNanos); - } else { - String responseJson = new String(bytes, StandardCharsets.UTF_8); - InstrumentationSemConv.tagLLMSpanResponse( - tracer, span, InstrumentationSemConv.PROVIDER_NAME_OPENAI, responseJson); - } - } catch (Exception e) { - log.error("Could not tag span from response buffer", e); - } - } - - private static String firstNonEmptyLine(byte[] bytes) { - int start = 0; - for (int i = 0; i <= bytes.length; i++) { - if (i == bytes.length || bytes[i] == '\n') { - String line = new String(bytes, start, i - start, StandardCharsets.UTF_8).strip(); - if (!line.isEmpty()) return line; - start = i + 1; - } - } - return null; - } - /** - * Parses SSE wire bytes, feeds each {@code data:} chunk through {@link - * ChatCompletionAccumulator}, then tags the span with the reassembled output JSON. + * Adapts openai-java's {@link Headers} to the vendor-neutral shape {@link + * InstrumentationSemConv} consumes. Returns an empty map on failure so a header-shape change + * can never take down the tagging that follows it. */ - private static void tagSpanFromSseBytes( - Tracer tracer, Span span, byte[] sseBytes, Long timeToFirstTokenNanos) { + private static Map> headersAsMap(@Nullable Headers headers) { + if (headers == null) { + return Map.of(); + } try { - var reader = - new BufferedReader( - new InputStreamReader( - new ByteArrayInputStream(sseBytes), StandardCharsets.UTF_8)); - String line; - String responseJson = null; - while ((line = reader.readLine()) != null) { - if (!line.startsWith("data:")) continue; - var firstEventJson = line.substring("data:".length()).strip(); - // after the first data chunk is found, read the rest of the stream with the proper - // accumulator type - var jsonTree = JSON_MAPPER.readTree(firstEventJson); - if (jsonTree.has("type") && jsonTree.get("type").asText().startsWith("response")) { - // response API SSEvents - ResponseAccumulator accumulator = ResponseAccumulator.create(); - accumulator.accumulate( - JSON_MAPPER.readValue(firstEventJson, ResponseStreamEvent.class)); - while ((line = reader.readLine()) != null) { - if (!line.startsWith("data:")) continue; - String data = line.substring("data:".length()).strip(); - if (data.isEmpty() || data.equals("[DONE]")) continue; - ResponseStreamEvent rse = - JSON_MAPPER.readValue(data, ResponseStreamEvent.class); - accumulator.accumulate(rse); - } - responseJson = JSON_MAPPER.writeValueAsString(accumulator.response()); - } else if (jsonTree.has("object") - && jsonTree.get("object").asText().equals("chat.completion.chunk")) { - // completions API SSEvents - var accumulator = ChatCompletionAccumulator.create(); - accumulator.accumulate( - JSON_MAPPER.readValue(firstEventJson, ChatCompletionChunk.class)); - while ((line = reader.readLine()) != null) { - if (!line.startsWith("data:")) continue; - String data = line.substring("data:".length()).strip(); - if (data.isEmpty() || data.equals("[DONE]")) continue; - ChatCompletionChunk chunk = - BraintrustJsonMapper.get() - .readValue(data, ChatCompletionChunk.class); - accumulator.accumulate(chunk); - } - responseJson = JSON_MAPPER.writeValueAsString(accumulator.chatCompletion()); - } else { - log.warn("unknown SSE object {}", firstEventJson); - } - break; - } - if (null != responseJson) { - InstrumentationSemConv.tagLLMSpanResponse( - tracer, - span, - InstrumentationSemConv.PROVIDER_NAME_OPENAI, - responseJson, - timeToFirstTokenNanos); + var map = new HashMap>(); + for (String name : headers.names()) { + map.put(name, headers.values(name)); } + return map; } catch (Exception e) { - log.error("Could not parse SSE buffer to tag streaming span output", e); + log.debug("could not read headers", e); + return Map.of(); } } @@ -374,10 +302,40 @@ private void onStreamClosed() { synchronized (teeBuffer) { bytes = teeBuffer.toByteArray(); } + + // Recorded before tagging: openai-java raises above this layer, so the error + // status is ours alone to set, and losing it to a body-parsing problem is worse + // than losing the parsed output. + // Anything outside 2xx, not just 4xx/5xx: both vendor SDKs treat success as + // exactly 200..299, so a final 3xx that the http client did not follow (a 304, or + // a redirect with no usable Location) is raised to the caller as an + // UnexpectedStatusCodeException and must mark the span failed too. + int statusCode = delegate.statusCode(); + if (statusCode < 200 || statusCode >= 300) { + InstrumentationSemConv.tagLLMSpanHttpError( + span, statusCode, new String(bytes, StandardCharsets.UTF_8)); + } + + // Wire-format bookkeeping lives in ResponseReassembler; this hands semconv + // everything the response carried in one flat call. A null body (empty response, + // or an SSE shape we didn't recognize) still tags the headers. // tagLLMSpanResponse also emits child spans for any server-side tool calls (web // search, etc.) nested under the LLM span while it is still live. No-op for Chat // Completions responses (no `output` array). - tagSpanFromBuffer(tracer, span, bytes, timeToFirstTokenNanos.get()); + try { + var reassembled = + ResponseReassembler.reassemble(bytes, timeToFirstTokenNanos.get()); + InstrumentationSemConv.tagLLMSpanResponse( + tracer, + span, + InstrumentationSemConv.PROVIDER_NAME_OPENAI, + reassembled.body(), + reassembled.timeToFirstTokenNanos(), + headersAsMap(delegate.headers())); + } catch (Exception e) { + // Observability must never change the response behavior seen by the caller. + log.error("Could not tag span from response buffer", e); + } } finally { span.end(); } diff --git a/braintrust-sdk/instrumentation/openai_2_15_0/src/main/java/dev/braintrust/instrumentation/openai/v2_15_0/auto/OpenAIInstrumentationModule.java b/braintrust-sdk/instrumentation/openai_2_15_0/src/main/java/dev/braintrust/instrumentation/openai/v2_15_0/auto/OpenAIInstrumentationModule.java index 61ff7de6..01cd0407 100644 --- a/braintrust-sdk/instrumentation/openai_2_15_0/src/main/java/dev/braintrust/instrumentation/openai/v2_15_0/auto/OpenAIInstrumentationModule.java +++ b/braintrust-sdk/instrumentation/openai_2_15_0/src/main/java/dev/braintrust/instrumentation/openai/v2_15_0/auto/OpenAIInstrumentationModule.java @@ -37,6 +37,8 @@ public List getHelperClassNames() { MANUAL_INSTRUMENTATION_PACKAGE + "TracingHttpClient$ExtractedRequest", MANUAL_INSTRUMENTATION_PACKAGE + "BraintrustOpenAI", MANUAL_INSTRUMENTATION_PACKAGE + "ContextCapturingProxy", + MANUAL_INSTRUMENTATION_PACKAGE + "ResponseReassembler", + MANUAL_INSTRUMENTATION_PACKAGE + "ResponseReassembler$Result", "dev.braintrust.json.BraintrustJsonMapper", "dev.braintrust.instrumentation.InstrumentationSemConv"); } diff --git a/braintrust-sdk/instrumentation/openai_2_15_0/src/test/java/dev/braintrust/instrumentation/openai/v2_15_0/BraintrustOpenAITest.java b/braintrust-sdk/instrumentation/openai_2_15_0/src/test/java/dev/braintrust/instrumentation/openai/v2_15_0/BraintrustOpenAITest.java index 218a4175..b81e8ea5 100644 --- a/braintrust-sdk/instrumentation/openai_2_15_0/src/test/java/dev/braintrust/instrumentation/openai/v2_15_0/BraintrustOpenAITest.java +++ b/braintrust-sdk/instrumentation/openai_2_15_0/src/test/java/dev/braintrust/instrumentation/openai/v2_15_0/BraintrustOpenAITest.java @@ -9,6 +9,11 @@ import com.openai.client.okhttp.OpenAIOkHttpClient; import com.openai.client.okhttp.OpenAIOkHttpClientAsync; import com.openai.core.JsonValue; +import com.openai.core.RequestOptions; +import com.openai.core.http.Headers; +import com.openai.core.http.HttpMethod; +import com.openai.core.http.HttpRequest; +import com.openai.core.http.HttpResponse; import com.openai.core.http.StreamResponse; import com.openai.helpers.ChatCompletionAccumulator; import com.openai.helpers.ResponseAccumulator; @@ -24,15 +29,21 @@ import dev.braintrust.TestHarness; import dev.braintrust.instrumentation.Instrumenter; import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.trace.StatusCode; import io.opentelemetry.sdk.trace.data.SpanData; +import java.io.ByteArrayInputStream; +import java.io.InputStream; import java.util.List; import java.util.Map; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; import lombok.SneakyThrows; import net.bytebuddy.agent.ByteBuddyAgent; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; public class BraintrustOpenAITest { private static final ObjectMapper JSON_MAPPER = new ObjectMapper(); @@ -649,5 +660,261 @@ private static void assertValidOpenAISpan(SpanData span, boolean isStreaming) { assertNotNull( attributes.get(AttributeKey.stringKey("braintrust.output_json")), "output must be set"); + assertOpenAIIdsCaptured(span); + } + + /** + * Both correlation IDs OpenAI hands back. Deliberately asserts only presence and the object-ID + * prefix: {@code x-request-id} is opaque, and OpenAI returns both {@code req_*} and bare UUIDs + * for it, so pinning its shape would be wrong. + */ + private static void assertOpenAIIdsCaptured(SpanData span) { + var attributes = span.getAttributes(); + + String requestId = attributes.get(AttributeKey.stringKey("x-request-id")); + assertNotNull(requestId, "x-request-id header must be captured"); + assertFalse(requestId.isBlank(), "x-request-id must not be blank"); + + String responseId = attributes.get(AttributeKey.stringKey("response_id")); + assertNotNull(responseId, "response_id must be captured from the response body"); + assertTrue( + responseId.startsWith("resp_") || responseId.startsWith("chatcmpl-"), + "unexpected response_id: " + responseId); + } + + /** + * A failed call is the case where the vendor's request id matters most, and the only one where + * it is the *sole* ID available: an error body carries no object id of its own. openai-java + * raises its exception above the HTTP layer we instrument, so the error status on the span is + * ours alone to set. + */ + @Test + @SneakyThrows + void testHttpErrorTagsSpan() { + OpenAIClient openAIClient = + OpenAIOkHttpClient.builder() + .baseUrl(testHarness.openAiBaseUrl()) + .apiKey(testHarness.openAiApiKey()) + .build(); + + var request = + ChatCompletionCreateParams.builder() + .model("gpt-4o-mini-nonexistent-model") + .addUserMessage("What is the capital of France?") + .build(); + + assertThrows(Exception.class, () -> openAIClient.chat().completions().create(request)); + + var spans = testHarness.awaitExportedSpans(); + assertEquals(1, spans.size()); + var span = spans.get(0); + + assertEquals( + StatusCode.ERROR, + span.getStatus().getStatusCode(), + "a non-2xx response must mark the span failed"); + + var attributes = span.getAttributes(); + String requestId = attributes.get(AttributeKey.stringKey("x-request-id")); + assertNotNull(requestId, "x-request-id must still be captured on a failed call"); + assertNull( + attributes.get(AttributeKey.stringKey("response_id")), + "an error body has no object id to capture"); + } + + /** + * Request headers now flow into the semconv layer, and outgoing OpenAI request headers carry + * the caller's API key — so this pins the allow-list: the only non-{@code braintrust.*} + * attributes on the span are the two correlation IDs, nothing else from either header set. + */ + @Test + @SneakyThrows + void testOnlyAllowListedHeadersAreTagged() { + OpenAIClient openAIClient = + OpenAIOkHttpClient.builder() + .baseUrl(testHarness.openAiBaseUrl()) + .apiKey(testHarness.openAiApiKey()) + .build(); + + var request = + ChatCompletionCreateParams.builder() + .model(ChatModel.GPT_4O_MINI) + .addSystemMessage("You are a helpful assistant") + .addUserMessage("What is the capital of France?") + .temperature(0.0) + .build(); + + openAIClient.chat().completions().create(request); + + var span = testHarness.awaitExportedSpans().get(0); + var foreignKeys = + span.getAttributes().asMap().keySet().stream() + .map(AttributeKey::getKey) + .filter(k -> !k.startsWith("braintrust.")) + .sorted() + .toList(); + assertEquals(List.of("response_id", "x-request-id"), foreignKeys); + + String rendered = span.getAttributes().toString(); + assertFalse( + rendered.contains(testHarness.openAiApiKey()), + "the api key must never reach a span attribute"); + assertFalse( + rendered.toLowerCase().contains("authorization"), + "no authorization header may reach a span attribute"); + } + + /** + * Proves the request-header path independently of the response one. OpenAI returns {@code + * x-request-id} but never {@code request-id}, so a caller-set {@code request-id} is the one + * allow-listed header that can only have come from the request. + */ + @Test + @SneakyThrows + void testCallerSuppliedCorrelationHeaderIsTagged() { + OpenAIClient openAIClient = + OpenAIOkHttpClient.builder() + .baseUrl(testHarness.openAiBaseUrl()) + .apiKey(testHarness.openAiApiKey()) + .build(); + + var request = + ChatCompletionCreateParams.builder() + .model(ChatModel.GPT_4O_MINI) + .addSystemMessage("You are a helpful assistant") + .addUserMessage("What is the capital of France?") + .temperature(0.0) + .putAdditionalHeader("request-id", "caller-supplied-correlation-id") + .build(); + + openAIClient.chat().completions().create(request); + + var span = testHarness.awaitExportedSpans().get(0); + assertEquals( + "caller-supplied-correlation-id", + span.getAttributes().get(AttributeKey.stringKey("request-id")), + "a caller-set correlation header must be captured from the request"); + // The vendor's own id still lands from the response, independently. + assertNotNull(span.getAttributes().get(AttributeKey.stringKey("x-request-id"))); + } + + // ------------------------------------------------------------------------- + // Status-code handling + // + // Driven against a stub delegate rather than the VCR proxy: a 3xx or a 1xx cannot + // realistically be recorded from the vendor, and those are exactly the statuses that + // distinguish "non-2xx" from "4xx and up". + // ------------------------------------------------------------------------- + + /** + * openai-java's ErrorHandler treats success as exactly 200..299 and raises everything else to + * the caller, so every one of these must mark the span failed. 304 is the realistic case: the + * http client does not follow it, so it arrives here as a final response. + */ + @ParameterizedTest(name = "status {0}") + @ValueSource(ints = {100, 204, 301, 304, 400, 500}) + @SneakyThrows + void nonSuccessStatusMarksSpanFailed(int statusCode) { + var client = + new TracingHttpClient( + testHarness.openTelemetry(), new StubHttpClient(statusCode, "{}")); + + try (var response = client.execute(chatCompletionsRequest(), RequestOptions.none())) { + response.body().readAllBytes(); + } + + var span = testHarness.awaitExportedSpans().get(0); + boolean isSuccess = statusCode >= 200 && statusCode < 300; + assertEquals( + isSuccess ? StatusCode.UNSET : StatusCode.ERROR, + span.getStatus().getStatusCode(), + "status " + + statusCode + + (isSuccess ? " should not" : " should") + + " mark the span failed"); + } + + /** The correlation header is still captured on a status the SDK will reject. */ + @Test + @SneakyThrows + void requestIdIsCapturedOnANonSuccessStatus() { + var client = + new TracingHttpClient(testHarness.openTelemetry(), new StubHttpClient(304, "")); + + try (var response = client.execute(chatCompletionsRequest(), RequestOptions.none())) { + response.body().readAllBytes(); + } + + var span = testHarness.awaitExportedSpans().get(0); + assertEquals( + "req_stubbed", + span.getAttributes().get(AttributeKey.stringKey("x-request-id")), + "an empty-bodied non-2xx must still yield the vendor request id"); + } + + @Test + void nonJsonResponseTaggingIsBestEffort() { + var client = + new TracingHttpClient( + testHarness.openTelemetry(), + new StubHttpClient(502, "Bad Gateway")); + + assertDoesNotThrow( + () -> { + try (var response = + client.execute(chatCompletionsRequest(), RequestOptions.none())) { + response.body().readAllBytes(); + } + }); + + var span = testHarness.awaitExportedSpans().get(0); + assertEquals(StatusCode.ERROR, span.getStatus().getStatusCode()); + assertEquals( + "req_stubbed", span.getAttributes().get(AttributeKey.stringKey("x-request-id"))); + } + + private static HttpRequest chatCompletionsRequest() { + return HttpRequest.builder() + .method(HttpMethod.POST) + .baseUrl("https://api.openai.com/v1") + .addPathSegments("chat", "completions") + .build(); + } + + /** Returns a canned response; never touches the network. */ + private record StubHttpClient(int statusCode, String body) + implements com.openai.core.http.HttpClient { + + @Override + public HttpResponse execute(HttpRequest request, RequestOptions requestOptions) { + return new HttpResponse() { + @Override + public int statusCode() { + return statusCode; + } + + @Override + public Headers headers() { + return Headers.builder().put("x-request-id", "req_stubbed").build(); + } + + @Override + public InputStream body() { + return new ByteArrayInputStream(body.getBytes()); + } + + @Override + public void close() {} + }; + } + + @Override + public CompletableFuture executeAsync( + HttpRequest request, RequestOptions requestOptions) { + return CompletableFuture.completedFuture(execute(request, requestOptions)); + } + + @Override + public void close() {} } } diff --git a/braintrust-sdk/instrumentation/springai_1_0_0/src/main/java/dev/braintrust/instrumentation/springai/v1_0_0/BraintrustSpringAI.java b/braintrust-sdk/instrumentation/springai_1_0_0/src/main/java/dev/braintrust/instrumentation/springai/v1_0_0/BraintrustSpringAI.java index a30ebfac..21ded46a 100644 --- a/braintrust-sdk/instrumentation/springai_1_0_0/src/main/java/dev/braintrust/instrumentation/springai/v1_0_0/BraintrustSpringAI.java +++ b/braintrust-sdk/instrumentation/springai_1_0_0/src/main/java/dev/braintrust/instrumentation/springai/v1_0_0/BraintrustSpringAI.java @@ -190,7 +190,9 @@ public ClientHttpResponse intercept( baseUrl, pathSegments, request.getMethod().name(), - requestBody); + requestBody, + null, + request.getHeaders()); ClientHttpResponse response = execution.execute(request, body); @@ -198,7 +200,8 @@ public ClientHttpResponse intercept( byte[] responseBytes = response.getBody().readAllBytes(); String responseBody = new String(responseBytes, StandardCharsets.UTF_8); - InstrumentationSemConv.tagLLMSpanResponse(tracer, span, providerName, responseBody); + InstrumentationSemConv.tagLLMSpanResponse( + tracer, span, providerName, responseBody, null, response.getHeaders()); span.end(); return new BufferedClientHttpResponse(response, responseBytes); @@ -295,11 +298,17 @@ public Mono filter(ClientRequest request, ExchangeFunction next) baseUrl, pathSegments, method, - capturedBody); + capturedBody, + null, + request.headers()); // Wrap the response body to intercept each chunk for TTFT // tracking and to reassemble the full SSE stream for response // tagging. + // Captured here rather than in wrapStreamingBody: the stream is + // finalized asynchronously on completion, long after the + // ClientResponse is in scope. + var responseHeaders = response.headers().asHttpHeaders(); return response.mutate() .body( originalBody -> @@ -307,7 +316,8 @@ public Mono filter(ClientRequest request, ExchangeFunction next) originalBody, span, startNanos, - streamCtx)) + streamCtx, + responseHeaders)) .build(); }) .doOnError( @@ -331,7 +341,8 @@ private Flux wrapStreamingBody( Publisher originalBody, Span span, long startNanos, - StreamContext streamCtx) { + StreamContext streamCtx, + HttpHeaders responseHeaders) { final long[] ttftNanos = {-1}; StringBuilder assembled = new StringBuilder(); @@ -362,7 +373,8 @@ private Flux wrapStreamingBody( span, streamCtx.providerName(), responseBody, - ttft); + ttft, + responseHeaders); } catch (Exception e) { log.debug("failed to tag streaming response", e); } @@ -602,6 +614,7 @@ private static String reassembleAnthropicSSE( var usage = mapper.createObjectNode(); String model = null; String stopReason = null; + String messageId = null; // Reconstruct each content block by index, preserving order and type. Map blocksByIndex = new LinkedHashMap<>(); @@ -627,6 +640,10 @@ private static String reassembleAnthropicSSE( if (model == null && message.hasNonNull("model")) { model = message.get("model").asText(); } + // The msg_* object id only ever appears on message_start. + if (messageId == null && message.hasNonNull("id")) { + messageId = message.get("id").asText(); + } copyFields(message.get("usage"), usage); } } @@ -696,6 +713,11 @@ private static String reassembleAnthropicSSE( .forEach(entry -> contentBlocks.add(entry.getValue())); var result = mapper.createObjectNode(); + // Carried through so the reassembled stream keeps the same object id a non-streaming + // response would have reported. + if (messageId != null) { + result.put("id", messageId); + } result.put("role", "assistant"); result.set("content", contentBlocks); if (stopReason != null) { diff --git a/braintrust-sdk/instrumentation/springai_1_0_0/src/test/java/dev/braintrust/instrumentation/springai/v1_0_0/BraintrustSpringAITest.java b/braintrust-sdk/instrumentation/springai_1_0_0/src/test/java/dev/braintrust/instrumentation/springai/v1_0_0/BraintrustSpringAITest.java index 60bd07a8..2489e564 100644 --- a/braintrust-sdk/instrumentation/springai_1_0_0/src/test/java/dev/braintrust/instrumentation/springai/v1_0_0/BraintrustSpringAITest.java +++ b/braintrust-sdk/instrumentation/springai_1_0_0/src/test/java/dev/braintrust/instrumentation/springai/v1_0_0/BraintrustSpringAITest.java @@ -380,6 +380,33 @@ private void assertCommonSpanAttributes(SpanData span, Provider provider) { provider.expectedBaseUrl().apply(testHarness), metadata(span).get("request_base_uri").asText(), "request_base_uri should match the configured base URL"); + + assertCorrelationIdsCaptured(span, provider); + } + + /** + * Both correlation IDs, over whichever transport the calling test exercised — the blocking + * RestClient interceptor and the reactive WebClient filter reach these from different places. + * The header name is vendor-specific: OpenAI sends {@code x-request-id}, Anthropic {@code + * request-id} with no prefix. Presence only for the header, since its value is opaque. + */ + private static void assertCorrelationIdsCaptured(SpanData span, Provider provider) { + boolean isAnthropic = "anthropic".equals(provider.expectedProvider()); + String idHeader = isAnthropic ? "request-id" : "x-request-id"; + + String requestId = span.getAttributes().get(AttributeKey.stringKey(idHeader)); + assertNotNull(requestId, idHeader + " header must be captured"); + assertFalse(requestId.isBlank(), idHeader + " must not be blank"); + + String responseId = span.getAttributes().get(AttributeKey.stringKey("response_id")); + assertNotNull(responseId, "response_id must be captured from the response body"); + if (isAnthropic) { + assertTrue(responseId.startsWith("msg_"), "unexpected response_id: " + responseId); + } else { + assertTrue( + responseId.startsWith("chatcmpl-") || responseId.startsWith("resp_"), + "unexpected response_id: " + responseId); + } } @SneakyThrows diff --git a/braintrust-sdk/instrumentation/springai_2_0_0/src/main/java/dev/braintrust/instrumentation/springai/v2_0_0/auto/SpringAIAnthropicInstrumentationModule.java b/braintrust-sdk/instrumentation/springai_2_0_0/src/main/java/dev/braintrust/instrumentation/springai/v2_0_0/auto/SpringAIAnthropicInstrumentationModule.java index 1b373af2..d9210ef6 100644 --- a/braintrust-sdk/instrumentation/springai_2_0_0/src/main/java/dev/braintrust/instrumentation/springai/v2_0_0/auto/SpringAIAnthropicInstrumentationModule.java +++ b/braintrust-sdk/instrumentation/springai_2_0_0/src/main/java/dev/braintrust/instrumentation/springai/v2_0_0/auto/SpringAIAnthropicInstrumentationModule.java @@ -38,6 +38,8 @@ public List getHelperClassNames() { ANTHROPIC_PACKAGE + "TracingHttpClient$TeeingStreamHttpResponse", ANTHROPIC_PACKAGE + "TracingHttpClient$TeeInputStream", ANTHROPIC_PACKAGE + "TracingHttpClient$ExtractedRequest", + ANTHROPIC_PACKAGE + "ResponseReassembler", + ANTHROPIC_PACKAGE + "ResponseReassembler$Result", "dev.braintrust.json.BraintrustJsonMapper", "dev.braintrust.instrumentation.InstrumentationSemConv"); } diff --git a/braintrust-sdk/instrumentation/springai_2_0_0/src/main/java/dev/braintrust/instrumentation/springai/v2_0_0/auto/SpringAIOpenAIInstrumentationModule.java b/braintrust-sdk/instrumentation/springai_2_0_0/src/main/java/dev/braintrust/instrumentation/springai/v2_0_0/auto/SpringAIOpenAIInstrumentationModule.java index 76ca3073..6d0e7aac 100644 --- a/braintrust-sdk/instrumentation/springai_2_0_0/src/main/java/dev/braintrust/instrumentation/springai/v2_0_0/auto/SpringAIOpenAIInstrumentationModule.java +++ b/braintrust-sdk/instrumentation/springai_2_0_0/src/main/java/dev/braintrust/instrumentation/springai/v2_0_0/auto/SpringAIOpenAIInstrumentationModule.java @@ -37,6 +37,8 @@ public List getHelperClassNames() { OPENAI_PACKAGE + "TracingHttpClient$TeeingStreamHttpResponse", OPENAI_PACKAGE + "TracingHttpClient$TeeInputStream", OPENAI_PACKAGE + "TracingHttpClient$ExtractedRequest", + OPENAI_PACKAGE + "ResponseReassembler", + OPENAI_PACKAGE + "ResponseReassembler$Result", "dev.braintrust.json.BraintrustJsonMapper", "dev.braintrust.instrumentation.InstrumentationSemConv"); } diff --git a/braintrust-sdk/src/main/java/dev/braintrust/instrumentation/InstrumentationSemConv.java b/braintrust-sdk/src/main/java/dev/braintrust/instrumentation/InstrumentationSemConv.java index 76ad2255..72cb6243 100644 --- a/braintrust-sdk/src/main/java/dev/braintrust/instrumentation/InstrumentationSemConv.java +++ b/braintrust-sdk/src/main/java/dev/braintrust/instrumentation/InstrumentationSemConv.java @@ -44,12 +44,26 @@ public static void tagLLMSpanRequest( tagLLMSpanRequest(span, providerName, baseUrl, pathSegments, method, requestBody, null); } + public static void tagLLMSpanRequest( + Span span, + @Nonnull String providerName, + @Nonnull String baseUrl, + @Nonnull List pathSegments, + @Nonnull String method, + @Nullable String requestBody, + @Nullable String modelId) { + tagLLMSpanRequest( + span, providerName, baseUrl, pathSegments, method, requestBody, modelId, Map.of()); + } + /** - * Tag a span with LLM request metadata. + * Tag a span with everything going out on the request: path, method, body and headers. * * @param modelId explicit model identifier — used by providers (e.g. Bedrock) where the model * is not present in the request body. When {@code null} the model is extracted from the * request body if possible. + * @param requestHeaders read for correlation IDs only, never copied wholesale — outgoing + * request headers carry the caller's API key. */ @SneakyThrows public static void tagLLMSpanRequest( @@ -59,7 +73,8 @@ public static void tagLLMSpanRequest( @Nonnull List pathSegments, @Nonnull String method, @Nullable String requestBody, - @Nullable String modelId) { + @Nullable String modelId, + @Nonnull Map> requestHeaders) { switch (providerName) { case PROVIDER_NAME_OPENAI -> tagOpenAIRequest( @@ -80,6 +95,9 @@ public static void tagLLMSpanRequest( tagOpenAIRequest( span, providerName, baseUrl, pathSegments, method, requestBody); } + // A caller-set correlation id. Tagged before the response, so if the vendor returns its + // own value it overwrites this one — the vendor's is the more useful of the two. + tagIdHeaders(span, requestHeaders); } public static void tagLLMSpanResponse( @@ -90,19 +108,39 @@ public static void tagLLMSpanResponse( tagLLMSpanResponse(tracer, span, providerName, responseBody, null); } + public static void tagLLMSpanResponse( + @Nonnull Tracer tracer, + Span span, + @Nonnull String providerName, + @Nonnull String responseBody, + @Nullable Long timeToFirstTokenNanoseconds) { + tagLLMSpanResponse( + tracer, span, providerName, responseBody, timeToFirstTokenNanoseconds, Map.of()); + } + /** - * Tag a span with the LLM response and emit child spans for any server-side tool calls the - * provider reported inline. The response body is parsed once here and the parsed tree is reused - * for both, so callers should route all response tagging through this method rather than - * parsing themselves. + * Tag a span with everything that came back on the response: headers, body, and timing. The + * body is parsed once here and the parsed tree reused for both attribute tagging and the + * server-side tool child spans, so callers should route all response tagging through this + * method rather than parsing themselves. + * + *

{@code responseBody} is nullable because a response can legitimately arrive with nothing + * usable in it — an empty error body, or an SSE stream whose shape we don't recognize. The + * headers are still worth tagging in those cases, so body handling is skipped rather than the + * whole call being abandoned. */ @SneakyThrows public static void tagLLMSpanResponse( @Nonnull Tracer tracer, Span span, @Nonnull String providerName, - @Nonnull String responseBody, - @Nullable Long timeToFirstTokenNanoseconds) { + @Nullable String responseBody, + @Nullable Long timeToFirstTokenNanoseconds, + @Nonnull Map> responseHeaders) { + tagIdHeaders(span, responseHeaders); + if (responseBody == null || responseBody.isBlank()) { + return; + } JsonNode responseJson = BraintrustJsonMapper.get().readTree(responseBody); switch (providerName) { case PROVIDER_NAME_OPENAI -> @@ -122,6 +160,94 @@ public static void tagLLMSpanResponse(Span span, @Nonnull Throwable responseErro span.recordException(responseError); } + /** whitelist of provider id headers to capture into an llm span */ + private static final List ID_HEADERS = List.of("x-request-id", "request-id"); + + /** + * Tags any {@link #ID_HEADERS} present onto the span. Best-effort: a header the vendor didn't + * send is simply not tagged. + * + *

Matching is case-insensitive here rather than at the call site because callers disagree — + * openai-java's {@code Headers} is backed by a case-insensitive {@code TreeMap}, while + * langchain4j hands over a plain {@code HashMap}. + */ + private static void tagIdHeaders( + @Nonnull Span span, @Nonnull Map> headers) { + if (headers.isEmpty()) { + return; + } + for (Map.Entry> entry : headers.entrySet()) { + String name = entry.getKey(); + if (name == null) { + continue; + } + for (String idHeader : ID_HEADERS) { + if (!idHeader.equalsIgnoreCase(name)) { + continue; + } + List values = entry.getValue(); + if (values == null || values.isEmpty()) { + break; + } + String value = values.get(0); + // Values are opaque — OpenAI returns both `req_*` and bare UUIDs for + // x-request-id — so never parse or validate the shape, only the emptiness. + if (value != null && !value.isBlank()) { + span.setAttribute(idHeader, value); + } + break; + } + } + } + + /** + * Marks the span failed for a non-2xx HTTP response. The vendor SDKs raise their exception + * above the HTTP client layer we instrument, so without this an errored call would otherwise + * end with an unset status and no indication anything went wrong. + */ + public static void tagLLMSpanHttpError( + @Nonnull Span span, int statusCode, @Nullable String responseBody) { + span.setStatus(StatusCode.ERROR, httpErrorMessage(statusCode, responseBody)); + } + + /** Prefers the provider's own error message, falling back to the bare status code. */ + private static String httpErrorMessage(int statusCode, @Nullable String responseBody) { + String fallback = "HTTP " + statusCode; + if (responseBody == null || responseBody.isBlank()) { + return fallback; + } + try { + JsonNode error = BraintrustJsonMapper.get().readTree(responseBody).get("error"); + if (error != null) { + // OpenAI nests the text under `error.message`; Anthropic uses the same shape. + JsonNode message = error.isTextual() ? error : error.get("message"); + if (message != null && message.isTextual() && !message.asText().isBlank()) { + return fallback + ": " + message.asText(); + } + } + } catch (Exception e) { + log.debug("could not parse error message out of response body", e); + } + return fallback; + } + + /** + * Tags the provider's object ID for the response ({@code resp_*}, {@code chatcmpl-*}, {@code + * msg_*}) onto the span. Like {@link #tagLLMSpanIdHeaders} this rides the collector's + * fall-through into metadata, but the raw field name {@code id} would be uselessly ambiguous + * there, so it is qualified — same reasoning as {@code tool_id} in {@link + * #openAIToolSpanMetadata}. + * + *

Absent on responses that carry no ID of their own (Bedrock, and any error body), which is + * exactly when the ID headers matter instead. + */ + private static void tagResponseId(Span span, JsonNode responseJson) { + JsonNode id = responseJson.get("id"); + if (id != null && id.isTextual() && !id.asText().isBlank()) { + span.setAttribute("response_id", id.asText()); + } + } + /** * Emit child {@code type:"tool"} spans for built-in tool calls the vendor executed server * side (web search, file search, code interpreter, image generation, remote MCP) that the @@ -200,6 +326,8 @@ private static void tagOpenAIRequest( @SneakyThrows private static void tagOpenAIResponse( Span span, JsonNode responseJson, @Nullable Long timeToFirstTokenNanoseconds) { + tagResponseId(span, responseJson); + // Output — chat completions API uses "choices"; Responses API uses "output"; audio // transcriptions and translations return a text-keyed object. if (responseJson.has("choices")) { @@ -493,6 +621,8 @@ private static void tagAnthropicResponse( String responseBody, JsonNode responseJson, @Nullable Long timeToFirstTokenNanoseconds) { + tagResponseId(span, responseJson); + // Anthropic response is the full Message object — output it whole span.setAttribute("braintrust.output_json", responseBody); diff --git a/test-harness/src/testFixtures/java/dev/braintrust/VCR.java b/test-harness/src/testFixtures/java/dev/braintrust/VCR.java index 5b519f60..073baa6b 100644 --- a/test-harness/src/testFixtures/java/dev/braintrust/VCR.java +++ b/test-harness/src/testFixtures/java/dev/braintrust/VCR.java @@ -20,6 +20,7 @@ import java.nio.file.StandardCopyOption; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; +import java.util.ArrayList; import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashMap; @@ -474,6 +475,7 @@ private void createProgrammaticStubFromMapping( com.github.tomakehurst.wiremock.client.WireMock.aResponse() .withStatus(status) .withHeader("Content-Type", responseContentType); + copyRecordedResponseHeaders(mapping, response); // Binary event-stream bodies must be served as raw bytes to avoid UTF-8 corruption if (isEventStream) { @@ -499,6 +501,45 @@ private void createProgrammaticStubFromMapping( wireMock.stubFor(stub.willReturn(response)); } + /** + * Replays the recorded response headers onto a programmatically-built stub. + * + *

Mappings that WireMock loads natively serve their recorded headers for free, but the + * programmatic path above rebuilds the response from scratch and would otherwise serve only + * {@code Content-Type} — silently hiding provider headers (rate limits, {@code x-request-id}) + * from any instrumentation under test on exactly the SSE responses these stubs exist for. + */ + private static void copyRecordedResponseHeaders( + JsonNode mapping, + com.github.tomakehurst.wiremock.client.ResponseDefinitionBuilder response) { + JsonNode headers = mapping.at("/response/headers"); + if (!headers.isObject()) { + return; + } + Iterator> fields = headers.fields(); + while (fields.hasNext()) { + Map.Entry header = fields.next(); + String name = header.getKey(); + // Content-Type is already set from the cassette; the framing headers belong to + // Jetty, and replaying a recorded Content-Length would contradict the body we serve. + if (name.equalsIgnoreCase("Content-Type") + || name.equalsIgnoreCase("Content-Length") + || name.equalsIgnoreCase("Transfer-Encoding")) { + continue; + } + JsonNode value = header.getValue(); + if (value.isArray()) { + List values = new ArrayList<>(); + value.forEach(v -> values.add(v.asText())); + if (!values.isEmpty()) { + response.withHeader(name, values.toArray(new String[0])); + } + } else if (value.isTextual()) { + response.withHeader(name, value.asText()); + } + } + } + /** * Remove dynamic fields from JSON that change between test runs. Specifically removes * parent.row_ids.span_id and parent.row_ids.root_span_id which are generated by OTEL. diff --git a/test-harness/src/testFixtures/resources/cassettes/openai/__files/chat_completions-6f74c5653e73.json b/test-harness/src/testFixtures/resources/cassettes/openai/__files/chat_completions-6f74c5653e73.json new file mode 100644 index 00000000..b37e9a10 --- /dev/null +++ b/test-harness/src/testFixtures/resources/cassettes/openai/__files/chat_completions-6f74c5653e73.json @@ -0,0 +1,8 @@ +{ + "error": { + "message": "The model `gpt-4o-mini-nonexistent-model` does not exist or you do not have access to it.", + "type": "invalid_request_error", + "param": null, + "code": "model_not_found" + } +} diff --git a/test-harness/src/testFixtures/resources/cassettes/openai/mappings/chat_completions-6f74c5653e73.json b/test-harness/src/testFixtures/resources/cassettes/openai/mappings/chat_completions-6f74c5653e73.json new file mode 100644 index 00000000..688a7045 --- /dev/null +++ b/test-harness/src/testFixtures/resources/cassettes/openai/mappings/chat_completions-6f74c5653e73.json @@ -0,0 +1,40 @@ +{ + "id" : "8644d738-98a1-3e0d-a6a1-a4be31ef694b", + "name" : "chat_completions", + "request" : { + "url" : "/chat/completions", + "method" : "POST", + "headers" : { + "Content-Type" : { + "equalTo" : "application/json" + } + }, + "bodyPatterns" : [ { + "equalToJson" : "{\"messages\":[{\"content\":\"What is the capital of France?\",\"role\":\"user\"}],\"model\":\"gpt-4o-mini-nonexistent-model\"}", + "ignoreArrayOrder" : true, + "ignoreExtraElements" : false + } ] + }, + "response" : { + "status" : 404, + "bodyFileName" : "chat_completions-6f74c5653e73.json", + "headers" : { + "x-request-id" : "req_0b1561af84e7450b9cd1d6ef27b7d390", + "Server" : "cloudflare", + "CF-Ray" : "a30f1ec42fb7cc88-SEA", + "X-Content-Type-Options" : "nosniff", + "x-openai-proxy-wasm" : "v0.1", + "Date" : "Wed, 26 Aug 2026 01:39:01 GMT", + "set-cookie" : "__cf_bm=mHSfCIGgGG8_gp599rnyPmGl0VaCpsimYqNgbF_d9ag-1787708339.8657904-1.0.1.1-iF3F7PUuAiv8CJm_Yz75CJx8Op47zSr3OLJjs8nmGQHQkgE7ntZg9tOhLcCOVpvr01ltGLo3ka01qoycRLso4skn.L6rCAvhAVxxiV2TB.U6H1wkOhLgKmKKsFx.bX2P; HttpOnly; SameSite=None; Secure; Path=/; Domain=api.openai.com; Expires=Wed, 26 Aug 2026 02:09:01 GMT", + "Access-Control-Expose-Headers" : [ "CF-Ray", "CF-Ray" ], + "Strict-Transport-Security" : "max-age=31536000; includeSubDomains; preload", + "CF-Cache-Status" : "DYNAMIC", + "Vary" : "Origin", + "alt-svc" : "h3=\":443\"; ma=86400", + "Content-Type" : "application/json; charset=utf-8" + } + }, + "uuid" : "8644d738-98a1-3e0d-a6a1-a4be31ef694b", + "persistent" : true, + "insertionIndex" : 63 +} \ No newline at end of file