Genai utils | Add AgentInvocation type#4274
Genai utils | Add AgentInvocation type#4274etserend wants to merge 6 commits intoopen-telemetry:mainfrom
Conversation
1b5cd9f to
08e2b07
Compare
|
Sample trace from demo app Trace: ad3749665a3bffcef592d6bacc2a2924 |
util/opentelemetry-util-genai/src/opentelemetry/util/genai/span_utils.py
Outdated
Show resolved
Hide resolved
e0ee125 to
25411ca
Compare
There was a problem hiding this comment.
Overall LGTM!! I left few questions to understand:
- If we need to improve overall robustness of error handling.
- early capture of few sem conv.
I used this doc for reviewing if sem conv are captured correctly or not. Additionally does it make sense to make this pr specific for agent invocation and removing agent creation specific code.
util/opentelemetry-util-genai/src/opentelemetry/util/genai/handler.py
Outdated
Show resolved
Hide resolved
util/opentelemetry-util-genai/src/opentelemetry/util/genai/handler.py
Outdated
Show resolved
Hide resolved
util/opentelemetry-util-genai/src/opentelemetry/util/genai/span_utils.py
Outdated
Show resolved
Hide resolved
util/opentelemetry-util-genai/src/opentelemetry/util/genai/handler.py
Outdated
Show resolved
Hide resolved
util/opentelemetry-util-genai/src/opentelemetry/util/genai/types.py
Outdated
Show resolved
Hide resolved
wrisa
left a comment
There was a problem hiding this comment.
LGTM. Though would need refactoring in handler.py in case other PRs are merged first.
eternalcuriouslearner
left a comment
There was a problem hiding this comment.
question: Do we have GEN_AI_AGENT_VERSION, GEN_AI_USAGE_CACHE_CREATION_INPUT_TOKENS, GEN_AI_TOOL_DEFINITIONS and GEN_AI_USAGE_CACHE_READ_INPUT_TOKENS in opentelemetry.semconv._incubating.attributes?
Yes @eternalcuriouslearner , all four constants in semconv package and referenced them via the |
|
Verified the Demo PR: #4340 Steps exercised:
|
a3a0f9e to
c320ca6
Compare
| (GenAI.GEN_AI_USAGE_INPUT_TOKENS, invocation.input_tokens), | ||
| (GenAI.GEN_AI_USAGE_OUTPUT_TOKENS, invocation.output_tokens), | ||
| ( | ||
| "gen_ai.usage.cache_creation_input_tokens", |
There was a problem hiding this comment.
wrong name, should be gen_ai.usage.cache_creation.input_tokens. Also if this is not available in the current version of semconv attributes maybe extra to constant?
| invocation.cache_creation_input_tokens, | ||
| ), | ||
| ( | ||
| "gen_ai.usage.cache_read_input_tokens", |
There was a problem hiding this comment.
wrong name, should be gen_ai.usage.cache_read.input_tokens. Also if this is not available in the current version of semconv attributes maybe extra to constant?
| return _get_span_name(invocation) | ||
|
|
||
|
|
||
| def _get_system_instructions_for_span( |
| self.stop(invocation) | ||
|
|
||
| # Agent-specific convenience methods | ||
| def start_agent(self, invocation: AgentInvocation) -> AgentInvocation: |
There was a problem hiding this comment.
do we still need these methods if we are handling all invocation types in start, stop and fail methods?
|
|
||
| def _start(self, invocation: _T) -> _T: | ||
| """Start a GenAI invocation and create a pending span entry.""" | ||
| kind = SpanKind.CLIENT |
There was a problem hiding this comment.
should we add a span_kind field on AgentInvocation or base GenAIInvocation defaulting to SpanKind.CLIENT so that this can be supported?
Description
This PR adds the
AgentInvocationtype extending_BaseAgentfor agent invocation (invoke_agent) spans. It providesTelemetryHandlerlifecycle methods and an agent() context manager — aligned with the GenAI agent span semantic conventions.Builds on the
_BaseAgentbase class and AgentCreation type added in #4217. Agent metrics and events to come in follow-up PRs.Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.