Skip to content

Commit d7351ef

Browse files
jessicagamioygree
andauthored
fix(llmobs): emit session_id as top-level field + propagate via context (#11371)
fix(llmobs): emit session_id as top-level field in LLMObs span event dd-trace-java was placing session_id only inside the `tags[]` array of each LLMObs span event on the wire, never as a top-level span field. The LLM Trace Explorer's Sessions filter keys off the top-level `session_id` field per the public HTTP intake API schema (https://docs.datadoghq.com/llm_observability/instrumentation/api?tab=model#span), so Java spans were invisible to that filter. dd-trace-py and dd-trace-js have conformed to this schema since the LLMObs v2 API launched (2024-06-21, MLOB-955). In LLMObsSpanMapper.map(), read the _ml_obs_tag.session_id tag from each span before opening the msgpack map; if present, extend the map size from 11 to 12 entries and emit the value as a top-level field. Mirrors the existing parent_id lift pattern, with one deliberate difference: the tag is not removed so the `session_id:<value>` entry remains in the tags[] array, matching dd-trace-py and dd-trace-js behavior. Originating issue: MLOS-646. fix(llmobs): propagate session_id from parent context to child LLMObs spans dd-trace-java's DDLLMObsSpan constructor only honored an explicitly passed sessionId argument. When a caller used the SDK's documented "set session_id on the root span only" pattern and started a child LLMObs span with sessionId=null, the child carried no session_id. dd-trace-py (ddtrace/llmobs/_llmobs.py:1911) and dd-trace-js (packages/dd-trace/src/llmobs/tagger.js:105-106) both auto-propagate session_id from parent context to descendant LLMObs spans. Add a SESSION_ID ContextKey to LLMObsContext alongside the existing span-context key, plus an attach(ctx, sessionId) overload and a currentSessionId() accessor. In DDLLMObsSpan's constructor, fall back to LLMObsContext.currentSessionId() when no explicit sessionId is passed. The constructor now also propagates the effective sessionId through the new context key so subsequent descendant spans can inherit it. Originating issue: MLOS-646. test(llmobs): consolidate session_id mapper tests, add transitive inheritance test fix(llmobs/openai-java): inherit session_id on auto-instrumented openai.request span OpenAiDecorator.afterStart() already consulted LLMObsContext.current() to set the LLMObs parent_id tag but never read session_id from the context. Auto-instrumented openai.request spans therefore carried no _ml_obs_tag.session_id even when a manual LLMObs workflow parent had one. dd-trace-py and dd-trace-js both auto-propagate session_id to auto-instrumented LLM spans via parent context. Add a SESSION_ID constant to CommonTags. In OpenAiDecorator.afterStart(), after the existing parent_id block, read LLMObsContext.currentSessionId() and stamp it on the span as CommonTags.SESSION_ID when present. With this change, auto-instrumented openai.request spans now appear under their session in the LLM Trace Explorer's Sessions view, matching Python and Node behavior. Depends on the LLMObsContext.currentSessionId() API added in the preceding fix(llmobs): propagate session_id from parent context commit. Originating issue: MLOS-646. test(llmobs/openai-java): migrate session_id propagation test to JUnit 5 + Java refactor(llmobs): drop redundant hasSessionId ternary on LLMObsContext.attach Merge remote-tracking branch 'origin/master' into jessica.gamio/llmobs-session-id-mlos-646 style(llmobs/openai-java): apply spotless formatting to SessionIdPropagationForkedTest fix(llmobs): address Codex review — safe session_id cast, gate inheritance on trace match - LLMObsSpanMapper: read session_id tag as Object and instanceof-check before casting, so a non-string value can't ClassCastException and drop the entire LLMObs payload. - DDLLMObsSpan: move session_id inheritance from the constructor preamble to inside the existing trace-id consistency check that already gates parent_id. Prevents a stale LLMObsContext from leaking session attribution across traces. - DDLLMObsSpanTest: existing 2-level + 3-level inheritance tests now activate the parent's AgentScope so they truly share a trace. New test asserts session_id is NOT inherited when the LLMObsContext belongs to a different trace. test(llmobs): rename cross-trace test to mention async-boundary leak as the canonical scenario Merge branch 'master' into jessica.gamio/llmobs-session-id-mlos-646 Co-authored-by: yury.gribkov <yury.gribkov@datadoghq.com>
1 parent cfb9996 commit d7351ef

8 files changed

Lines changed: 356 additions & 9 deletions

File tree

dd-java-agent/agent-llmobs/src/main/java/datadog/trace/llmobs/domain/DDLLMObsSpan.java

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,11 +89,9 @@ public DDLLMObsSpan(
8989
span.setTag(SPAN_KIND, kind);
9090
spanKind = kind;
9191
span.setTag(LLMOBS_TAG_PREFIX + LLMObsTags.ML_APP, mlApp);
92-
this.hasSessionId = sessionId != null && !sessionId.isEmpty();
93-
if (this.hasSessionId) {
94-
span.setTag(LLMOBS_TAG_PREFIX + LLMObsTags.SESSION_ID, sessionId);
95-
}
96-
92+
// Resolve effective parent_id and session_id from the LLMObs context, both gated on
93+
// trace-id consistency. A stale context from a different trace (e.g. async boundary
94+
// leakage) must not contribute either tag.
9795
AgentSpanContext parent = LLMObsContext.current();
9896
String parentSpanID = LLMObsContext.ROOT_SPAN_ID;
9997
if (null != parent) {
@@ -106,10 +104,25 @@ public DDLLMObsSpan(
106104
span.getSpanId());
107105
} else {
108106
parentSpanID = String.valueOf(parent.getSpanId());
107+
// Inherit session_id from parent context only when it belongs to the same trace.
108+
// Matches dd-trace-py and dd-trace-js: session_id need only be set on the root
109+
// span; descendants inherit transitively via context propagation.
110+
if (sessionId == null || sessionId.isEmpty()) {
111+
String inherited = LLMObsContext.currentSessionId();
112+
if (inherited != null && !inherited.isEmpty()) {
113+
sessionId = inherited;
114+
}
115+
}
109116
}
110117
}
118+
119+
this.hasSessionId = sessionId != null && !sessionId.isEmpty();
120+
if (this.hasSessionId) {
121+
span.setTag(LLMOBS_TAG_PREFIX + LLMObsTags.SESSION_ID, sessionId);
122+
}
111123
span.setTag(LLMOBS_TAG_PREFIX + PARENT_ID_TAG_INTERNAL, parentSpanID);
112-
scope = LLMObsContext.attach(span.context());
124+
// Propagate the effective sessionId to descendant LLMObs spans via the context.
125+
scope = LLMObsContext.attach(span.context(), sessionId);
113126
}
114127

115128
@Override

dd-java-agent/agent-llmobs/src/test/groovy/datadog/trace/llmobs/domain/DDLLMObsSpanTest.groovy

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,96 @@ class DDLLMObsSpanTest extends DDSpecification{
411411
null | "has_session_id:0"
412412
}
413413

414+
def "child LLMObs span inherits session_id from parent context when none is passed"() {
415+
setup:
416+
def expectedSessionId = "session-abc-123"
417+
def parent = llmObsSpan(Tags.LLMOBS_WORKFLOW_SPAN_KIND, "parent-workflow", expectedSessionId)
418+
// Activate the parent's AgentScope so the child span is created in the same trace.
419+
// Without this, the child gets a fresh trace_id and the trace-consistency gate in
420+
// DDLLMObsSpan would (correctly) skip session_id inheritance.
421+
def parentScope = AgentTracer.activateSpan((AgentSpan) parent.span)
422+
423+
when:
424+
// Child created with null sessionId — should inherit from the parent's LLMObsContext.
425+
def child = llmObsSpan(Tags.LLMOBS_LLM_SPAN_KIND, "child-llm", null)
426+
427+
then:
428+
def innerChild = (AgentSpan) child.span
429+
expectedSessionId == innerChild.getTag(LLMOBS_TAG_PREFIX + LLMObsTags.SESSION_ID)
430+
431+
cleanup:
432+
child.finish()
433+
parentScope.close()
434+
parent.finish()
435+
}
436+
437+
def "child LLMObs span has no session_id when neither parent nor child passes one"() {
438+
setup:
439+
def parent = llmObsSpan(Tags.LLMOBS_WORKFLOW_SPAN_KIND, "parent-workflow", null)
440+
441+
when:
442+
def child = llmObsSpan(Tags.LLMOBS_LLM_SPAN_KIND, "child-llm", null)
443+
444+
then:
445+
def innerChild = (AgentSpan) child.span
446+
null == innerChild.getTag(LLMOBS_TAG_PREFIX + LLMObsTags.SESSION_ID)
447+
448+
cleanup:
449+
child.finish()
450+
parent.finish()
451+
}
452+
453+
def "grandchild LLMObs span transitively inherits session_id through intermediate span"() {
454+
setup:
455+
def expectedSessionId = "session-grandparent-xyz"
456+
def grandparent = llmObsSpan(Tags.LLMOBS_WORKFLOW_SPAN_KIND, "grandparent-workflow", expectedSessionId)
457+
// Activate each ancestor's AgentScope so descendants stay in the same trace —
458+
// session_id inheritance is gated on trace-id consistency in DDLLMObsSpan.
459+
def grandparentScope = AgentTracer.activateSpan((AgentSpan) grandparent.span)
460+
def parent = llmObsSpan(Tags.LLMOBS_WORKFLOW_SPAN_KIND, "parent-workflow", null)
461+
def parentScope = AgentTracer.activateSpan((AgentSpan) parent.span)
462+
463+
when:
464+
// Grandchild created with null sessionId — should inherit transitively
465+
// through parent's re-attached LLMObsContext (which itself inherited from grandparent).
466+
def grandchild = llmObsSpan(Tags.LLMOBS_LLM_SPAN_KIND, "grandchild-llm", null)
467+
468+
then:
469+
def innerGrandchild = (AgentSpan) grandchild.span
470+
expectedSessionId == innerGrandchild.getTag(LLMOBS_TAG_PREFIX + LLMObsTags.SESSION_ID)
471+
472+
cleanup:
473+
grandchild.finish()
474+
parentScope.close()
475+
parent.finish()
476+
grandparentScope.close()
477+
grandparent.finish()
478+
}
479+
480+
def "child does NOT inherit session_id when stale LLMObsContext is from a different trace (e.g. async boundary leak)"() {
481+
setup:
482+
// Simulates a stale LLMObsContext (e.g. leaked across an async boundary). The parent's
483+
// LLMObsContext is attached, but its AgentScope is deliberately NOT activated — so the
484+
// next span we create starts a fresh trace and the trace-consistency gate must skip
485+
// session_id inheritance.
486+
def parent = llmObsSpan(Tags.LLMOBS_WORKFLOW_SPAN_KIND, "stale-workflow", "stale-session-id")
487+
488+
when:
489+
def child = llmObsSpan(Tags.LLMOBS_LLM_SPAN_KIND, "child-llm", null)
490+
491+
then:
492+
def innerParent = (AgentSpan) parent.span
493+
def innerChild = (AgentSpan) child.span
494+
// Sanity: traces differ — confirms the scenario is set up correctly.
495+
innerParent.getTraceId() != innerChild.getTraceId()
496+
// Session_id from the stale-trace context must NOT leak into the new span.
497+
null == innerChild.getTag(LLMOBS_TAG_PREFIX + LLMObsTags.SESSION_ID)
498+
499+
cleanup:
500+
child.finish()
501+
parent.finish()
502+
}
503+
414504
def "global dd_tags are included in LLMObs span tags"() {
415505
setup:
416506
injectSysConfig("trace.global.tags", "team:backend,owner:ml-platform")

dd-java-agent/instrumentation/openai-java/openai-java-3.0/src/main/java/datadog/trace/instrumentation/openai_java/CommonTags.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ interface CommonTags {
3030
String ENV = TAG_PREFIX + "env";
3131
String SERVICE = TAG_PREFIX + "service";
3232
String PARENT_ID = TAG_PREFIX + "parent_id";
33+
String SESSION_ID = TAG_PREFIX + LLMObsTags.SESSION_ID;
3334

3435
String TOOL_DEFINITIONS = TAG_PREFIX + "tool_definitions";
3536

dd-java-agent/instrumentation/openai-java/openai-java-3.0/src/main/java/datadog/trace/instrumentation/openai_java/OpenAiDecorator.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,16 @@ public AgentSpan afterStart(AgentSpan span) {
115115
parentSpanId = String.valueOf(parent.getSpanId());
116116
}
117117
span.setTag(CommonTags.PARENT_ID, parentSpanId);
118+
119+
// Inherit session_id from the active LLMObs parent (e.g. a manual workflow span).
120+
// Matches dd-trace-py / dd-trace-js, where auto-instrumented LLM spans inherit
121+
// session_id from the workflow root via context propagation. Without this, the
122+
// auto-instrumented openai.request span would not appear under its session in
123+
// the LLM Trace Explorer's Sessions view.
124+
String sessionId = LLMObsContext.currentSessionId();
125+
if (sessionId != null && !sessionId.isEmpty()) {
126+
span.setTag(CommonTags.SESSION_ID, sessionId);
127+
}
118128
}
119129
return super.afterStart(span);
120130
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
package datadog.trace.instrumentation.openai_java;
2+
3+
import static org.junit.jupiter.api.Assertions.assertEquals;
4+
import static org.junit.jupiter.api.Assertions.assertNotNull;
5+
import static org.junit.jupiter.api.Assertions.assertNull;
6+
7+
import com.openai.client.OpenAIClient;
8+
import com.openai.client.okhttp.OpenAIOkHttpClient;
9+
import com.openai.credential.BearerTokenCredential;
10+
import com.openai.models.ChatModel;
11+
import com.openai.models.chat.completions.ChatCompletionCreateParams;
12+
import com.sun.net.httpserver.HttpServer;
13+
import datadog.context.ContextScope;
14+
import datadog.trace.agent.test.AbstractInstrumentationTest;
15+
import datadog.trace.api.llmobs.LLMObsContext;
16+
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
17+
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
18+
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
19+
import datadog.trace.core.DDSpan;
20+
import datadog.trace.junit.utils.config.WithConfig;
21+
import java.io.IOException;
22+
import java.net.InetSocketAddress;
23+
import java.util.List;
24+
import org.junit.jupiter.api.AfterAll;
25+
import org.junit.jupiter.api.BeforeAll;
26+
import org.junit.jupiter.api.Test;
27+
28+
/**
29+
* Verifies that auto-instrumented openai.request spans inherit session_id from an active LLMObs
30+
* parent context. Forked + @WithConfig used together so the LLMObs system property is in place
31+
* before the agent installs and there's no leakage from prior test state.
32+
*
33+
* <p>The mock OpenAI backend returns a minimal 200 response — the test asserts on the span tag set
34+
* by OpenAiDecorator.afterStart(), which runs before the HTTP response is parsed, so the response
35+
* body shape doesn't matter for what's being tested.
36+
*/
37+
@WithConfig(key = "llmobs.enabled", value = "true")
38+
class SessionIdPropagationForkedTest extends AbstractInstrumentationTest {
39+
40+
private static HttpServer mockServer;
41+
private static OpenAIClient openAiClient;
42+
43+
@BeforeAll
44+
static void setupMockOpenAi() throws IOException {
45+
mockServer = HttpServer.create(new InetSocketAddress("localhost", 0), 0);
46+
mockServer.createContext(
47+
"/v1/",
48+
exchange -> {
49+
exchange.sendResponseHeaders(200, -1);
50+
exchange.close();
51+
});
52+
mockServer.start();
53+
54+
openAiClient =
55+
OpenAIOkHttpClient.builder()
56+
.baseUrl(
57+
"http://"
58+
+ mockServer.getAddress().getHostString()
59+
+ ":"
60+
+ mockServer.getAddress().getPort()
61+
+ "/v1")
62+
.credential(BearerTokenCredential.create(""))
63+
.build();
64+
}
65+
66+
@AfterAll
67+
static void tearDownMockOpenAi() {
68+
if (mockServer != null) {
69+
mockServer.stop(0);
70+
mockServer = null;
71+
}
72+
openAiClient = null;
73+
}
74+
75+
@Test
76+
void openAiRequestSpanInheritsSessionIdFromActiveContext() throws Exception {
77+
String expectedSessionId = "session-propagation-test-abc";
78+
79+
AgentSpan parentSpan = AgentTracer.startSpan("test", "parent");
80+
try (AgentScope ignored1 = AgentTracer.activateSpan(parentSpan)) {
81+
try (ContextScope ignored2 = LLMObsContext.attach(parentSpan.context(), expectedSessionId)) {
82+
try {
83+
openAiClient.chat().completions().create(buildMinimalChatParams());
84+
} catch (Exception ignored) {
85+
// Mock server returns no body — the SDK may throw on parse. The span we care about
86+
// is already created by the instrumentation advice before this point.
87+
}
88+
}
89+
} finally {
90+
parentSpan.finish();
91+
}
92+
93+
writer.waitForTraces(1);
94+
DDSpan openAiSpan = findSpanByOperationName(writer, "openai.request");
95+
assertNotNull(openAiSpan, "openai.request span should have been created");
96+
assertEquals(expectedSessionId, openAiSpan.getTag("_ml_obs_tag.session_id"));
97+
}
98+
99+
@Test
100+
void openAiRequestSpanHasNoSessionIdWhenNoLlmObsContext() throws Exception {
101+
try {
102+
openAiClient.chat().completions().create(buildMinimalChatParams());
103+
} catch (Exception ignored) {
104+
// Mock server returns no body — the SDK may throw on parse. The span we care about
105+
// is already created by the instrumentation advice before this point.
106+
}
107+
108+
writer.waitForTraces(1);
109+
DDSpan openAiSpan = findSpanByOperationName(writer, "openai.request");
110+
assertNotNull(openAiSpan, "openai.request span should have been created");
111+
assertNull(openAiSpan.getTag("_ml_obs_tag.session_id"));
112+
}
113+
114+
private static ChatCompletionCreateParams buildMinimalChatParams() {
115+
return ChatCompletionCreateParams.builder()
116+
.model(ChatModel.GPT_4O_MINI)
117+
.addSystemMessage("")
118+
.addUserMessage("")
119+
.build();
120+
}
121+
122+
private static DDSpan findSpanByOperationName(List<List<DDSpan>> traces, String operationName) {
123+
return traces.stream()
124+
.flatMap(List::stream)
125+
.filter(s -> operationName.equals(s.getOperationName().toString()))
126+
.findFirst()
127+
.orElse(null);
128+
}
129+
}

dd-trace-core/src/main/java/datadog/trace/llmobs/writer/ddintake/LLMObsSpanMapper.java

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ public class LLMObsSpanMapper implements RemoteMapper {
5555
private static final byte[] DD = "_dd".getBytes(StandardCharsets.UTF_8);
5656
private static final byte[] APM_TRACE_ID = "apm_trace_id".getBytes(StandardCharsets.UTF_8);
5757
private static final byte[] PARENT_ID = "parent_id".getBytes(StandardCharsets.UTF_8);
58+
private static final byte[] SESSION_ID = "session_id".getBytes(StandardCharsets.UTF_8);
5859
private static final byte[] NAME = "name".getBytes(StandardCharsets.UTF_8);
5960
private static final byte[] DURATION = "duration".getBytes(StandardCharsets.UTF_8);
6061
private static final byte[] START_NS = "start_ns".getBytes(StandardCharsets.UTF_8);
@@ -88,6 +89,8 @@ public class LLMObsSpanMapper implements RemoteMapper {
8889
private static final byte[] LLM_TOOL_RESULT_RESULT = "result".getBytes(StandardCharsets.UTF_8);
8990

9091
private static final String PARENT_ID_TAG_INTERNAL_FULL = LLMOBS_TAG_PREFIX + "parent_id";
92+
private static final String SESSION_ID_TAG_INTERNAL_FULL =
93+
LLMOBS_TAG_PREFIX + LLMObsTags.SESSION_ID;
9194

9295
private final MetaWriter metaWriter = new MetaWriter();
9396
private final int size;
@@ -126,7 +129,17 @@ public void map(List<? extends CoreSpan<?>> trace, Writable writable) {
126129
}
127130

128131
for (CoreSpan<?> span : llmobsSpans) {
129-
writable.startMap(11);
132+
// Read session_id off the span before opening the map so we can size it correctly.
133+
// We deliberately do NOT remove the tag (unlike parent_id) — the session_id:<value>
134+
// entry must remain in the tags[] array to match dd-trace-py and dd-trace-js behavior.
135+
// span.getTag returns Object — guard against generic tag APIs setting a non-string
136+
// session_id value, which would otherwise throw ClassCastException here and drop
137+
// the entire LLMObs payload for the trace.
138+
Object rawSessionId = span.getTag(SESSION_ID_TAG_INTERNAL_FULL);
139+
String sessionId = rawSessionId instanceof String ? (String) rawSessionId : null;
140+
boolean hasSessionId = sessionId != null && !sessionId.isEmpty();
141+
142+
writable.startMap(hasSessionId ? 12 : 11);
130143
// 1
131144
writable.writeUTF8(SPAN_ID);
132145
writable.writeString(String.valueOf(span.getSpanId()), null);
@@ -166,7 +179,14 @@ public void map(List<? extends CoreSpan<?>> trace, Writable writable) {
166179
writable.writeUTF8(APM_TRACE_ID);
167180
writable.writeString(span.getTraceId().toHexString(), null);
168181

169-
/* 9 (metrics), 10 (tags), 11 meta */
182+
// 9 — optional top-level session_id field. Required by the LLMObs HTTP intake schema
183+
// and by the LLM Trace Explorer's Sessions filter, which keys off this field.
184+
if (hasSessionId) {
185+
writable.writeUTF8(SESSION_ID);
186+
writable.writeString(sessionId, null);
187+
}
188+
189+
/* 10 (metrics), 11 (tags), 12 meta — shift down 1 if session_id absent */
170190
span.processTagsAndBaggage(metaWriter.withWritable(writable, getErrorsMap(span)));
171191
}
172192

0 commit comments

Comments
 (0)