feat: Add id and finish_reason to ModelResponse #2325
Conversation
fatelei
force-pushed
the
issue-886
branch
2 times, most recently
from
7a34524 to
bacbc82
Compare
| @@ -792,6 +792,12 @@ class ModelResponse: | |||
| vendor_id: str | None = None | |||
| """Vendor ID as specified by the model provider. This can be used to track the specific request to the model.""" | |||
|
|
|||
| id: str | None = None | |||
| """Unique identifier for the model response, e.g. as returned by the model provider (OpenAI, etc).""" | |||
There was a problem hiding this comment.
vendor_id already exists for this same purpose, so we don't need a new field. Can you rename vendor_id to id? Note that this is a public class, so we'll need to continue supporting getting and setting vendor_id with a deprecation warning.
I'm also OK keeping it as vendor_id.
|
|
||
| finish_reason: str | None = None | ||
| """The reason the model finished generating this response, e.g. 'stop', 'length', etc.""" | ||
|
|
||
| def otel_events(self, settings: InstrumentationSettings) -> list[Event]: |
There was a problem hiding this comment.
The issue mentions:
These fields would be used to populate gen_ai.response.id and gen_ai.response.finish_reasons in opentelemetry.io/docs/specs/semconv/gen-ai/gen-ai-spans#genai-attributes
Can you please handle that here as well so this PR can close that issue?
There was a problem hiding this comment.
Note that genai.response.finish_reasons has specific allowed values: #1882 (comment)
I also left some other related suggestions on that older PR that tried to add finish_reason -- can you please check those out as well?
| id: str | None = None | ||
| """Unique identifier for the model response, e.g. as returned by the model provider (OpenAI, etc).""" | ||
|
|
||
| finish_reason: str | None = None |
There was a problem hiding this comment.
We're already storing finish_reason in the vendor_details dict in various places, can you please update those to also set this property? We'll want to keep it in vendor_details as well for backward compatibility.
Feature:
Fixes #886