Fix support for OpenAI developer messages #539
Conversation
| @@ -102,7 +115,8 @@ public static void emitPromptLogEvents( | |||
| bodyBuilder.put("id", toolMsg.toolCallId()); | |||
| } | |||
| } else { | |||
| throw new IllegalStateException("Unhandled type : " + msg.getClass().getName()); | |||
| LOG.log(Level.WARNING, "Unhandled OpenAI message type will be dropped: {0}", msg); | |||
There was a problem hiding this comment.
This would previously cause the instrumentation to fail, which is a bit too strict.
| @@ -228,7 +255,7 @@ private static LogRecordBuilder newEvent(String name) { | |||
| private static Value<?> buildToolCallEventObject(ChatCompletionMessageToolCall call) { | |||
| Map<String, Value<?>> result = new HashMap<>(); | |||
| result.put("id", Value.of(call.id())); | |||
| result.put("type", Value.of(call._type().toString())); | |||
There was a problem hiding this comment.
This was not detected by muzzle: In some versions inbetween _type() was not present in the openAI lib.
| } else if (val.isResponseFormatJsonSchema()) { | ||
| return val.asResponseFormatJsonSchema()._type().toString(); | ||
| return "json_schema"; |
There was a problem hiding this comment.
For some versions _type() returns null here, so I instead hardcoded the strings for these older versions.
|
I'm giving up on muzzle, the checks for 8e050f4 should have failed but did not: In that commit muzzle should have checked against the For now we should not trust muzzle being green, but rather as a additional safety net which might catch additional problems (or not...) |
| ChatCompletionDeveloperMessageParam sysMsg = | ||
| (ChatCompletionDeveloperMessageParam) concreteMessageParam; | ||
| eventType = "gen_ai.system.message"; | ||
| putIfNotEmpty(bodyBuilder, "role", "developer"); |
There was a problem hiding this comment.
I see. where the SDK isn't explicit in how these map to json, we make a good guess
Closes #532 . For now, these messages will simply be reported as
gen_ai.system.messageevents withroleset todeveloper, which seems to be kind of sem conv compliant. We'll have to revisit later anyway for future semconv changes (e.g. open-telemetry/semantic-conventions#1851), but this at least keeps things working for now.