Skip to content

Commit d2208fc

Browse files
sabrennersabrenner
authored
fix(openai, llmobs): update openai integration consistency (#6932)
* updates for openai integration consistency * update other tests for empty role default
1 parent 80ea153 commit d2208fc

File tree

10 files changed

+187
-119
lines changed

10 files changed

+187
-119
lines changed

‎packages/dd-trace/src/llmobs/plugins/openai.js‎

Lines changed: 41 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,12 @@ class OpenAiLLMObsPlugin extends LLMObsPlugin {
6767
if (!error) {
6868
const metrics = this._extractMetrics(response)
6969
this._tagger.tagMetrics(span, metrics)
70+
71+
const responseModel = response.model
72+
if (responseModel) {
73+
// override the model name with the response model (more accurate)
74+
this._tagger.tagModelName(span, responseModel)
75+
}
7076
}
7177
}
7278

@@ -85,11 +91,11 @@ class OpenAiLLMObsPlugin extends LLMObsPlugin {
8591

8692
if (tokenUsage) {
8793
// Responses API uses input_tokens, Chat/Completions use prompt_tokens
88-
const inputTokens = tokenUsage.input_tokens ?? tokenUsage.prompt_tokens
94+
const inputTokens = tokenUsage.input_tokens ?? tokenUsage.prompt_tokens ?? 0
8995
if (inputTokens !== undefined) metrics.inputTokens = inputTokens
9096

9197
// Responses API uses output_tokens, Chat/Completions use completion_tokens
92-
const outputTokens = tokenUsage.output_tokens ?? tokenUsage.completion_tokens
98+
const outputTokens = tokenUsage.output_tokens ?? tokenUsage.completion_tokens ?? 0
9399
if (outputTokens !== undefined) metrics.outputTokens = outputTokens
94100

95101
const totalTokens = tokenUsage.total_tokens || (inputTokens + outputTokens)
@@ -105,7 +111,7 @@ class OpenAiLLMObsPlugin extends LLMObsPlugin {
105111
} else if (tokenUsage.prompt_tokens_details) {
106112
// Chat/Completions API - only include if > 0
107113
const cacheReadTokens = tokenUsage.prompt_tokens_details.cached_tokens
108-
if (cacheReadTokens) {
114+
if (cacheReadTokens != null) {
109115
metrics.cacheReadTokens = cacheReadTokens
110116
}
111117
}
@@ -159,6 +165,16 @@ class OpenAiLLMObsPlugin extends LLMObsPlugin {
159165
_tagChatCompletion (span, inputs, response, error) {
160166
const { messages, model, ...parameters } = inputs
161167

168+
const metadata = Object.entries(parameters).reduce((obj, [key, value]) => {
169+
if (!['tools', 'functions'].includes(key)) {
170+
obj[key] = value
171+
}
172+
173+
return obj
174+
}, {})
175+
176+
this._tagger.tagMetadata(span, metadata)
177+
162178
if (error) {
163179
this._tagger.tagLLMIO(span, messages, [{ content: '' }])
164180
return
@@ -200,16 +216,6 @@ class OpenAiLLMObsPlugin extends LLMObsPlugin {
200216
}
201217

202218
this._tagger.tagLLMIO(span, messages, outputMessages)
203-
204-
const metadata = Object.entries(parameters).reduce((obj, [key, value]) => {
205-
if (!['tools', 'functions'].includes(key)) {
206-
obj[key] = value
207-
}
208-
209-
return obj
210-
}, {})
211-
212-
this._tagger.tagMetadata(span, metadata)
213219
}
214220

215221
#tagResponse (span, inputs, response, error) {
@@ -269,6 +275,15 @@ class OpenAiLLMObsPlugin extends LLMObsPlugin {
269275
inputMessages.push({ role: 'user', content: input })
270276
}
271277

278+
const inputMetadata = Object.entries(parameters).reduce((obj, [key, value]) => {
279+
if (allowedParamKeys.has(key)) {
280+
obj[key] = value
281+
}
282+
return obj
283+
}, {})
284+
285+
this._tagger.tagMetadata(span, inputMetadata)
286+
272287
if (error) {
273288
this._tagger.tagLLMIO(span, inputMessages, [{ content: '' }])
274289
return
@@ -287,17 +302,13 @@ class OpenAiLLMObsPlugin extends LLMObsPlugin {
287302
for (const item of response.output) {
288303
// Handle reasoning type (reasoning responses)
289304
if (item.type === 'reasoning') {
290-
// Extract reasoning text from summary
291-
let reasoningText = ''
292-
if (Array.isArray(item.summary) && item.summary.length > 0) {
293-
const summaryItem = item.summary[0]
294-
if (summaryItem.type === 'summary_text' && summaryItem.text) {
295-
reasoningText = summaryItem.text
296-
}
297-
}
298305
outputMessages.push({
299306
role: 'reasoning',
300-
content: reasoningText
307+
content: JSON.stringify({
308+
summary: item.summary ?? [],
309+
encrypted_content: item.encrypted_content ?? null,
310+
id: item.id ?? ''
311+
})
301312
})
302313
} else if (item.type === 'function_call') {
303314
// Handle function_call type (responses API tool calls)
@@ -369,24 +380,19 @@ class OpenAiLLMObsPlugin extends LLMObsPlugin {
369380

370381
this._tagger.tagLLMIO(span, inputMessages, outputMessages)
371382

372-
const metadata = Object.entries(parameters).reduce((obj, [key, value]) => {
373-
if (allowedParamKeys.has(key)) {
374-
obj[key] = value
375-
}
376-
return obj
377-
}, {})
383+
const outputMetadata = {}
378384

379385
// Add fields from response object (convert numbers to floats)
380-
if (response.temperature !== undefined) metadata.temperature = Number(response.temperature)
381-
if (response.top_p !== undefined) metadata.top_p = Number(response.top_p)
382-
if (response.tool_choice !== undefined) metadata.tool_choice = response.tool_choice
383-
if (response.truncation !== undefined) metadata.truncation = response.truncation
384-
if (response.text !== undefined) metadata.text = response.text
386+
if (response.temperature !== undefined) outputMetadata.temperature = Number(response.temperature)
387+
if (response.top_p !== undefined) outputMetadata.top_p = Number(response.top_p)
388+
if (response.tool_choice !== undefined) outputMetadata.tool_choice = response.tool_choice
389+
if (response.truncation !== undefined) outputMetadata.truncation = response.truncation
390+
if (response.text !== undefined) outputMetadata.text = response.text
385391
if (response.usage?.output_tokens_details?.reasoning_tokens !== undefined) {
386-
metadata.reasoning_tokens = response.usage.output_tokens_details.reasoning_tokens
392+
outputMetadata.reasoning_tokens = response.usage.output_tokens_details.reasoning_tokens
387393
}
388394

389-
this._tagger.tagMetadata(span, metadata)
395+
this._tagger.tagMetadata(span, outputMetadata) // update the metadata with the output metadata
390396
}
391397
}
392398

‎packages/dd-trace/src/llmobs/tagger.js‎

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ class LLMObsTagger {
8585
if (name) this._setTag(span, NAME, name)
8686

8787
this._setTag(span, SPAN_KIND, kind)
88-
if (modelName) this._setTag(span, MODEL_NAME, modelName)
88+
if (modelName) this.tagModelName(span, modelName)
8989
if (modelProvider) this._setTag(span, MODEL_PROVIDER, modelProvider)
9090

9191
sessionId = sessionId || registry.get(parent)?.[SESSION_ID]
@@ -194,6 +194,10 @@ class LLMObsTagger {
194194
this._setTag(span, SPAN_KIND, newKind)
195195
}
196196

197+
tagModelName (span, modelName) {
198+
this._setTag(span, MODEL_NAME, modelName)
199+
}
200+
197201
#tagText (span, data, key) {
198202
if (data) {
199203
if (typeof data === 'string') {
@@ -324,36 +328,46 @@ class LLMObsTagger {
324328

325329
for (const message of data) {
326330
if (typeof message === 'string') {
327-
messages.push({ content: message })
331+
messages.push({ content: message, role: '' })
328332
continue
329333
}
330334
if (message == null || typeof message !== 'object') {
331335
this.#handleFailure('Messages must be a string, object, or list of objects', 'invalid_io_messages')
332336
continue
333337
}
334338

335-
const { content = '', role } = message
336-
const toolCalls = message.toolCalls
337-
const toolResults = message.toolResults
338-
const toolId = message.toolId
339-
const messageObj = { content }
339+
const {
340+
role = '',
341+
content,
342+
toolCalls,
343+
toolResults,
344+
toolId
345+
} = message
346+
const messageObj = {}
340347

341348
let condition = this.#tagConditionalString(role, 'Message role', messageObj, 'role')
342349

343-
const valid = typeof content === 'string'
344-
if (!valid) {
345-
this.#handleFailure('Message content must be a string.', 'invalid_io_messages')
350+
if (
351+
content == null &&
352+
toolCalls == null &&
353+
toolResults == null
354+
) {
355+
messageObj.content = ''
356+
}
357+
358+
if (content != null) {
359+
condition = this.#tagConditionalString(content, 'Message content', messageObj, 'content') && condition
346360
}
347361

348-
if (toolCalls) {
362+
if (toolCalls != null) {
349363
const filteredToolCalls = this.#filterToolCalls(toolCalls)
350364

351365
if (filteredToolCalls.length) {
352366
messageObj.tool_calls = filteredToolCalls
353367
}
354368
}
355369

356-
if (toolResults) {
370+
if (toolResults != null) {
357371
const filteredToolResults = this.#filterToolResults(toolResults)
358372

359373
if (filteredToolResults.length) {
@@ -363,13 +377,13 @@ class LLMObsTagger {
363377

364378
if (toolId) {
365379
if (role === 'tool') {
366-
condition = this.#tagConditionalString(toolId, 'Tool ID', messageObj, 'tool_id')
380+
condition = this.#tagConditionalString(toolId, 'Tool ID', messageObj, 'tool_id') && condition
367381
} else {
368382
log.warn(`Tool ID for tool message not associated with a "tool" role, instead got "${role}"`)
369383
}
370384
}
371385

372-
if (valid && condition) {
386+
if (condition) {
373387
messages.push(messageObj)
374388
}
375389
}
@@ -380,7 +394,7 @@ class LLMObsTagger {
380394
}
381395

382396
#tagConditionalString (data, type, carrier, key) {
383-
if (!data) return true
397+
if (data == null) return true
384398
if (typeof data !== 'string') {
385399
this.#handleFailure(`"${type}" must be a string.`)
386400
return false
@@ -390,7 +404,7 @@ class LLMObsTagger {
390404
}
391405

392406
#tagConditionalNumber (data, type, carrier, key) {
393-
if (!data) return true
407+
if (data == null) return true
394408
if (typeof data !== 'number') {
395409
this.#handleFailure(`"${type}" must be a number.`)
396410
return false
@@ -400,7 +414,7 @@ class LLMObsTagger {
400414
}
401415

402416
#tagConditionalObject (data, type, carrier, key) {
403-
if (!data) return true
417+
if (data == null) return true
404418
if (typeof data !== 'object') {
405419
this.#handleFailure(`"${type}" must be an object.`)
406420
return false

‎packages/dd-trace/test/llmobs/plugins/ai/index.spec.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,6 @@ describe('Plugin', () => {
444444
{ content: 'What is the weather in Tokyo?', role: 'user' }
445445
],
446446
outputMessages: [{
447-
content: MOCK_STRING,
448447
role: 'assistant',
449448
tool_calls: [{
450449
tool_id: toolCallId,

‎packages/dd-trace/test/llmobs/plugins/aws-sdk/bedrockruntime.spec.js‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,7 @@ describe('Plugin', () => {
6262
const command = new AWS.InvokeModelCommand(request)
6363
await bedrockRuntimeClient.send(command)
6464

65-
const expectedOutput = { content: model.response.text }
66-
if (model.outputRole) expectedOutput.role = model.outputRole
65+
const expectedOutput = { content: model.response.text, role: model.outputRole ?? '' }
6766

6867
const { apmSpans, llmobsSpans } = await getEvents()
6968
assertLlmObsSpanEvent(llmobsSpans[0], {
@@ -76,7 +75,7 @@ describe('Plugin', () => {
7675
{ content: model.userPrompt, role: 'user' }
7776
]
7877
: [
79-
{ content: model.userPrompt }
78+
{ content: model.userPrompt, role: '' }
8079
],
8180
outputMessages: [expectedOutput],
8281
metrics: {
@@ -125,7 +124,7 @@ describe('Plugin', () => {
125124
{ content: model.userPrompt, role: 'user' }
126125
]
127126
: [
128-
{ content: model.userPrompt }
127+
{ content: model.userPrompt, role: '' }
129128
],
130129
outputMessages: [{ content: expectedResponseObject.text, role: 'assistant' }],
131130
metrics: {
@@ -171,7 +170,9 @@ describe('Plugin', () => {
171170
span: apmSpans[0],
172171
spanKind: 'llm',
173172
name: 'bedrock-runtime.command',
174-
inputMessages: [{ content: 'You are a geography expert'.repeat(200) + cacheWriteRequest.userPrompt }],
173+
inputMessages: [
174+
{ content: 'You are a geography expert'.repeat(200) + cacheWriteRequest.userPrompt, role: '' }
175+
],
175176
outputMessages: [expectedOutput],
176177
metrics: {
177178
input_tokens: cacheWriteRequest.response.inputTokens,
@@ -214,7 +215,9 @@ describe('Plugin', () => {
214215
span: apmSpans[0],
215216
spanKind: 'llm',
216217
name: 'bedrock-runtime.command',
217-
inputMessages: [{ content: 'You are a geography expert'.repeat(200) + cacheWriteRequest.userPrompt }],
218+
inputMessages: [
219+
{ content: 'You are a geography expert'.repeat(200) + cacheWriteRequest.userPrompt, role: '' }
220+
],
218221
outputMessages: [expectedOutput],
219222
metrics: {
220223
input_tokens: cacheWriteRequest.response.inputTokens,
@@ -260,7 +263,9 @@ describe('Plugin', () => {
260263
span: apmSpans[0],
261264
spanKind: 'llm',
262265
name: 'bedrock-runtime.command',
263-
inputMessages: [{ content: 'You are a geography expert'.repeat(200) + cacheReadRequest.userPrompt }],
266+
inputMessages: [
267+
{ content: 'You are a geography expert'.repeat(200) + cacheReadRequest.userPrompt, role: '' }
268+
],
264269
outputMessages: [expectedOutput],
265270
metrics: {
266271
input_tokens: cacheReadRequest.response.inputTokens,
@@ -303,7 +308,9 @@ describe('Plugin', () => {
303308
span: apmSpans[0],
304309
spanKind: 'llm',
305310
name: 'bedrock-runtime.command',
306-
inputMessages: [{ content: 'You are a geography expert'.repeat(200) + cacheReadRequest.userPrompt }],
311+
inputMessages: [
312+
{ content: 'You are a geography expert'.repeat(200) + cacheReadRequest.userPrompt, role: '' }
313+
],
307314
outputMessages: [expectedOutput],
308315
metrics: {
309316
input_tokens: cacheReadRequest.response.inputTokens,

‎packages/dd-trace/test/llmobs/plugins/google-cloud-vertexai/index.spec.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ describe('integrations', () => {
203203

204204
inputMessages.push({ role: 'user', content: 'Foobar?' })
205205
inputMessages.push({ role: 'model', content: 'Foobar!' })
206-
inputMessages.push({ content: 'Hello, how are you?' })
206+
inputMessages.push({ content: 'Hello, how are you?', role: '' })
207207

208208
assertLlmObsSpanEvent(llmobsSpans[0], {
209209
span: apmSpans[0],

0 commit comments

Comments
 (0)