Change gen_ai.response.finish_reasons type from array to string, use it on metrics
#1291
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lmolkova
commented
Jul 26, 2024
| @@ -266,8 +274,9 @@ This metric SHOULD be specified with [ExplicitBucketBoundaries] of | |||
| | [`gen_ai.system`](/docs/attributes-registry/gen-ai.md) | string | The Generative AI product as identified by the client or server instrumentation. [2] | `openai` | `Required` |  | | |||
| | [`error.type`](/docs/attributes-registry/error.md) | string | Describes a class of error the operation ended with. [3] | `timeout`; `java.net.UnknownHostException`; `server_certificate_invalid`; `500` | `Conditionally Required` if the operation ended in an error |  | | |||
| | [`server.port`](/docs/attributes-registry/server.md) | int | Server port number. [4] | `80`; `8080`; `443` | `Conditionally Required` If `sever.address` is set. |  | | |||
| | [`gen_ai.response.finish_reason`](/docs/attributes-registry/gen-ai.md) | string | The reason(s) the model stopped generating tokens. [5] | `stop`; `stop,length` | `Recommended` |  | | |||
There was a problem hiding this comment.
@achandrasekar any thoughts/objections? on adding finish_reason attribute to LLM server metrics?
There was a problem hiding this comment.
This looks good to me. Adding a finish_reason attribute makes sense. It would be useful in a lot of cases including benchmarking where we can see if we generated to the max_length or stopped early due to the EOS token.
lmolkova
changed the title
gen_ai.responmse.finish_reasons type from array to string, use it on metricsgen_ai.response.finish_reasons type from array to string, use it on metrics
|
I'm going to close this PR and pursue other ways that work better for batching and multiple choices outlined here |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1277
Changes
While OpenAI supports returning multiple completions (and therefore could have multiple finish reasons), I did not find models outside of OpenAI that return multiple choices and many SDKs (e.g. vercel) don't support returning multiple choices.
So effectively there is usually one finish reason.
Having an array for it:
This PR:
Having comma-separated list has high-ish theoretical cardinality in edge cases, but results in best experience for common case.
I'm suggesting to start with it and consider making it opt-in on metrics if it's proven to be problematic.
Merge requirement checklist
[chore]