GenAI Utils | Embedding Type and Span Creation#4219
GenAI Utils | Embedding Type and Span Creation#4219shuningc wants to merge 7 commits intoopen-telemetry:mainfrom
Conversation
util/opentelemetry-util-genai/src/opentelemetry/util/genai/span_utils.py
Outdated
Show resolved
Hide resolved
util/opentelemetry-util-genai/src/opentelemetry/util/genai/types.py
Outdated
Show resolved
Hide resolved
util/opentelemetry-util-genai/src/opentelemetry/util/genai/types.py
Outdated
Show resolved
Hide resolved
util/opentelemetry-util-genai/src/opentelemetry/util/genai/span_utils.py
Show resolved
Hide resolved
util/opentelemetry-util-genai/src/opentelemetry/util/genai/types.py
Outdated
Show resolved
Hide resolved
4b4e710 to
e16f49f
Compare
| dependencies = [ | ||
| "opentelemetry-instrumentation ~= 0.58b0", | ||
| "opentelemetry-semantic-conventions ~= 0.58b0", | ||
| "opentelemetry-instrumentation ~= 0.61b0.dev", |
There was a problem hiding this comment.
AFAIR the latest embeddings related addition has been gen_ai.embeddings.dimension.count in 1.38.0. The bump to semconv 1.38.0 has been done in 0.60b0.
|
|
||
| request_model: str | None = None | ||
| # Chat by default | ||
| operation_name: str = GenAI.GenAiOperationNameValues.CHAT.value |
There was a problem hiding this comment.
Has this line a different outcome than the __post_init__ ?
There was a problem hiding this comment.
I moved the two common attributes from children level to parent level(GenAIInvocation), but this is open for discussion
| context_token: ContextToken | None = None | ||
| span: Span | None = None | ||
| attributes: dict[str, Any] = field(default_factory=_new_str_any_dict) | ||
| request_model: str | None = None |
There was a problem hiding this comment.
Step, Tool may have no request_model
| span: Span | None = None | ||
| attributes: dict[str, Any] = field(default_factory=_new_str_any_dict) | ||
| request_model: str | None = None | ||
| operation_name: str | None = None |
There was a problem hiding this comment.
All of the types in semconv require operation name, it looks good
https://github.com/open-telemetry/semantic-conventions/blob/main/docs/gen-ai/gen-ai-spans.md#spans
Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>
lmolkova
left a comment
There was a problem hiding this comment.
tried it with with OpenAI instrumentation and left a couple of comments.
Looks good otherwise!
| span.end() | ||
| return invocation | ||
|
|
||
| def fail_embedding( |
There was a problem hiding this comment.
does public API need to be specific to embedding? Can we have one stop/fail for all invocations and apply different logic depending on the invocation type?
|
|
||
| provider: str | None = None # e.g., azure.ai.openai, openai, aws.bedrock | ||
| server_address: str | None = None | ||
| server_port: int | None = None |
There was a problem hiding this comment.
we should also take response model. I think we just forgot to add it to semconv, fixing it here open-telemetry/semantic-conventions#3499. OpenAI instrumentation already sets it
Description
This PR adds initial embedding lifecycle coverage, to add metrics in the next PR.
Type of change
How Has This Been Tested?
Ran from repo root with local package path:
PYTHONPATH=util/opentelemetry-util-genai/src pytest -q util/opentelemetry-util-genai/tests/test_utils.py
PYTHONPATH=util/opentelemetry-util-genai/src pytest -q util/opentelemetry-util-genai/tests/test_handler_metrics.py
Results:
[test_utils.py]: 23 passed
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.