feat(openai): align embedding instrumentation with pending spec #2210
Conversation
Signed-off-by: Adrian Cole <[email protected]>
dosubot
bot
added
the
size:L
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
| # Use consistent span names: "CreateEmbeddings" for embeddings, class name for others | ||
| if cast_to is self._openai.types.CreateEmbeddingResponse: | ||
| span_name = "CreateEmbeddings" |
There was a problem hiding this comment.
just for my understanding here - what does the consistent name buy us rather than maybe naming based on function name.
There was a problem hiding this comment.
backends typically index the span name field for query and sometimes people do aggregates/auto-complete on the span name. For example, zipkin does auto-complete on span name, but not arbitrary attributes. Some tools dereive metrics from spans and need to aggregate on something, typically that's a span name. So, if something that is in a spec leaves the span name out, it ends up unable to be usable for things like this.
There was a problem hiding this comment.
here's an oldie example of this, which is subverted when span names are subtly different for the same operation https://github.com/openzipkin/zipkin-api/blob/master/zipkin2-api.yaml#L44
There was a problem hiding this comment.
I see. I mainly ask because I tend to face a fair amount of pressure to cram things like agent operations in names because there's a need for operators to groc the control flow.
With embedding's generation, though I think this makes a lot of sense
There was a problem hiding this comment.
Agreed. I don't think agent ops are commodity yet maybe not for a long while. Wont go trying to normalize those ;)
...rumentation-openai/src/openinference/instrumentation/openai/_request_attributes_extractor.py
Show resolved
Hide resolved
...rumentation-openai/src/openinference/instrumentation/openai/_request_attributes_extractor.py
Show resolved
Hide resolved
| try: | ||
| _NUMPY: Optional[ModuleType] = import_module("numpy") | ||
| except ImportError: | ||
| _NUMPY = None | ||
|
|
There was a problem hiding this comment.
I honestly don't remember why this was needed.
There was a problem hiding this comment.
originally it was doing this
vector = _NUMPY.frombuffer(base64.b64decode(_vector), dtype="float32").tolist()
but we have test cases to prove we can get the vectors at the moment without it. If we need to re-introduce it we probably should with a failing test
Signed-off-by: Adrian Cole <[email protected]>
| # openai.types.completion_create_params.CompletionCreateParamsBase | ||
| # See https://github.com/openai/openai-python/blob/f1c7d714914e3321ca2e72839fe2d132a8646e7f/src/openai/types/completion_create_params.py#L11 # noqa: E501 | ||
| """ | ||
| Extract attributes from parameters for the LEGACY completions API. |
There was a problem hiding this comment.
here's a rewording of the comments which I think better explains the prompt union weirdness
|
gonna be away this weekend, so lemme know if there's anything else to do here. regardless, I'll chase up the others next week. |
|
thanks, put to action here! envoyproxy/ai-gateway#1232 |
This PR aligns litellm with the specification changes around embeddings in Arize-ai#2162 **Spec changes from 2162** - Consistent span name: `"CreateEmbeddings"` - Standardized attribute structure: `embedding.embeddings.N.embedding.{text|vector}` - Unified invocation parameter tracking: `embedding.invocation_parameters` - Proper `llm.system` attribute for provider identification **Code improvements:** - Full batch embedding support with indexed attributes - Separated invocation parameters from input data - Improved handling of token IDs vs text inputs - Vectors stored as tuples instead of JSON strings This is the same as Arize-ai#2210, except litellm. Signed-off-by: Adrian Cole <[email protected]>
3 participants
Impact
This PR aligns openai with the specification changes around embeddings in #2162
Spec changes from 2162
"CreateEmbeddings"embedding.embeddings.N.embedding.{text|vector}embedding.invocation_parametersllm.systemattribute for provider identificationCode improvements: