feat: Add response api to gen_ai.input.messages and gen_ai.output.messages#1650
feat: Add response api to gen_ai.input.messages and gen_ai.output.messages#1650brightsparc wants to merge 21 commits intopydantic:mainfrom
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
There are now big merge conflicts because I just merged #1652, that was step 1 of #1586 Similar to #1593 (comment), please move changes which only add small attributes to a separate PR, these can be easily merged into main without breaking anything or significantly increasing span size. The messages format is a whole thing on its own. |
Okay, will look into this thanks. |
…make it simpler to add semvconv for chat completions / responses
| 'logfire.json_schema': { | ||
| 'type': 'object', | ||
| 'properties': { | ||
| 'request_data': {'type': 'object'}, | ||
| 'gen_ai.provider.name': {}, | ||
| 'gen_ai.request.model': {}, | ||
| 'gen_ai.operation.name': {}, | ||
| 'gen_ai.input.messages': {'type': 'array'}, | ||
| 'async': {}, | ||
| 'gen_ai.system': {}, | ||
| 'gen_ai.response.model': {}, | ||
| 'operation.cost': {}, | ||
| 'gen_ai.response.id': {}, | ||
| 'gen_ai.usage.input_tokens': {}, | ||
| 'gen_ai.usage.output_tokens': {}, | ||
| 'response_data': { | ||
| 'type': 'object', | ||
| 'properties': { | ||
| 'message': { | ||
| 'type': 'object', | ||
| 'title': 'ChatCompletionMessage', | ||
| 'x-python-datatype': 'PydanticModel', | ||
| }, | ||
| 'usage': { | ||
| 'type': 'object', | ||
| 'title': 'CompletionUsage', | ||
| 'x-python-datatype': 'PydanticModel', | ||
| }, | ||
| }, | ||
| }, | ||
| 'gen_ai.output.messages': {'type': 'array'}, | ||
| 'gen_ai.response.finish_reasons': {'type': 'array'}, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
@alexmojaki is there a reason why the json_schema doesn't include object type for things like string, integer, boolean, number? We could update to include these types? (see below)
def create_json_schema(obj: Any, seen: set[int]) -> JsonDict:
"""Create a JSON Schema from the given object.
Args:
obj: The object to create the JSON Schema from.
seen: A set of object IDs that have already been processed.
Returns:
The JSON Schema.
"""
if obj is None:
return {'type': 'null'}
try:
# cover common types first before calling `type_to_schema` to avoid the overhead of imports if not necessary
obj_type = obj.__class__
# TODO: add these types?
if obj_type is str:
return {'type': 'string'}
elif obj_type is int:
return {'type': 'integer'}
elif obj_type is bool:
return {'type': 'boolean'}
elif obj_type is float:
return {'type': 'number'}
if id(obj) in seen:
return {}
|
@brightsparc these changes look great but are you still going to split the PR in two? |
This PR adds I'm looking to make sure test coverage is 100% so this should mitigate most concerns? |
| span.set_attribute(RESPONSE_MODEL, response.model) | ||
| span.set_attribute(RESPONSE_ID, response.id) | ||
|
|
||
| # Add token usage | ||
| if response.usage: | ||
| span.set_attribute(INPUT_TOKENS, response.usage.input_tokens) | ||
| span.set_attribute(OUTPUT_TOKENS, response.usage.output_tokens) | ||
|
|
||
| # Add finish reason | ||
| if response.stop_reason: | ||
| span.set_attribute(RESPONSE_FINISH_REASONS, [response.stop_reason]) |
There was a problem hiding this comment.
This is the kind of thing I'm talking about when I say:
Similar to #1593 (comment), please move changes which only add small attributes to a separate PR, these can be easily merged into main without breaking anything or significantly increasing span size. The messages format is a whole thing on its own.
There was a problem hiding this comment.
Right, I will work on breaking this up into 3 PRs.
- Scalar values
- Input Messages
- Output Messages
Once I have these I will link to this PR and it can be closed.
There was a problem hiding this comment.
2 and 3 can stay together
There was a problem hiding this comment.
The 3 PRS are
- feat: Add OTel Gen AI semantic convention scalar attributes #1657
- feat: Add OTel Gen AI semantic convention input messages for Anthropic #1658
- feat: Add OTel Gen AI semantic convention output messages for Anthropic #1659
I suggest we land them in this order, as the input/output do overlap a bit and may need to rebase.
There was a problem hiding this comment.
Right, I missed this message, will combine 3 into 2
3 participants
No description provided.