fix(genai-utils): use AttributeValue instead of Any for attributes/metric_attributes#153
Open
bhumikadangayach wants to merge 4 commits into
Open
Conversation
…tric_attributes Replaces typing.Any with opentelemetry.util.types.AttributeValue for attributes and metric_attributes dicts across invocation types, since these values must serialize to OTel span/metric attributes. Scoped to the span/metric attribute path per discussion in open-telemetry#41. Follow-up needed for: dataclass fields in types.py that hold nested/ structured payloads (ToolCallRequest.arguments, ServerToolCall, etc.), and metrics/logs usages not yet audited. Fixes open-telemetry#41 (partial)
|
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens typing in opentelemetry-util-genai invocation helpers by replacing typing.Any with opentelemetry.util.types.AttributeValue for span/metric attribute dictionaries, aligning the public surface with what OpenTelemetry spans/metrics can actually serialize.
Changes:
- Updates
GenAIInvocationand multiple invocation subclasses to typeattributes/metric_attributesasdict[str, AttributeValue]. - Updates helper/getter methods that build attribute maps to return
dict[str, AttributeValue]. - Removes now-unneeded
typing.Anyimports from several modules.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| util/opentelemetry-util-genai/src/opentelemetry/util/genai/_invocation.py | Tightens base invocation attribute types; updates get_content_attributes signature to AttributeValue. |
| util/opentelemetry-util-genai/src/opentelemetry/util/genai/_tool_invocation.py | Updates tool invocation attribute typing to AttributeValue. |
| util/opentelemetry-util-genai/src/opentelemetry/util/genai/_embedding_invocation.py | Updates embedding invocation attribute typing to AttributeValue. |
| util/opentelemetry-util-genai/src/opentelemetry/util/genai/_agent_invocation.py | Updates agent invocation attribute typing to AttributeValue. |
| util/opentelemetry-util-genai/src/opentelemetry/util/genai/_workflow_invocation.py | Updates workflow invocation attribute typing to AttributeValue. |
| util/opentelemetry-util-genai/src/opentelemetry/util/genai/_inference_invocation.py | Updates inference invocation attribute typing to AttributeValue (including message attributes). |
Comments suppressed due to low confidence (1)
util/opentelemetry-util-genai/src/opentelemetry/util/genai/_inference_invocation.py:107
_get_message_attributesis annotated as returningdict[str, AttributeValue], but whenfor_span=Falseit includes structured message attributes intended for log events (lists/dicts). The annotation is therefore inaccurate and will become a typecheck blocker onceget_content_attributesis typed correctly for the event case. Consider adding overloads keyed onLiteral[True/False](or otherwise splitting span vs event attribute builders) so span attributes remainAttributeValuewhile event attributes can be structured.
def _get_message_attributes(self, *, for_span: bool) -> dict[str, AttributeValue]:
return get_content_attributes(
input_messages=self.input_messages,
output_messages=self.output_messages,
system_instruction=self.system_instruction,
tool_definitions=self.tool_definitions,
for_span=for_span,
)
Comment on lines
4
to
8
| from __future__ import annotations | ||
|
|
||
| from typing import Any | ||
|
|
||
|
|
||
| from opentelemetry._logs import Logger |
Comment on lines
4
to
8
| from __future__ import annotations | ||
|
|
||
| from typing import Any | ||
|
|
||
|
|
||
| from opentelemetry._logs import Logger |
Comment on lines
+12
to
+13
| from typing import TYPE_CHECKING, Any, Sequence, TypeAlias | ||
| from opentelemetry.util.types import AttributeValue |
Comment on lines
+183
to
+190
| tool_definitions: Sequence[ToolDefinition] | None, | ||
| for_span: bool, | ||
| ) -> dict[str, Any]: | ||
| ) -> dict[str, AttributeValue]: |
| ) | ||
|
|
||
| def _get_metric_attributes(self) -> dict[str, Any]: | ||
| def _get_metric_attributes(self) ->dict[str, AttributeValue]: |
Comment on lines
+89
to
91
| attributes: dict[str, AttributeValue] = { | ||
| GenAI.GEN_AI_OPERATION_NAME: self._operation_name | ||
| } |
- Revert get_content_attributes return type to dict[str, Any] in _invocation.py since it returns list[dict] for event attributes when for_span=False - Fix import grouping in _invocation.py and _agent_invocation.py - Minor formatting fixes (arrow spacing, trailing comma)
Contributor
|
Hello there!! Thanks for your contribution. Can you please take a look at the copilot suggestions around indentation and formatting? |
lmolkova
approved these changes
Jun 19, 2026
Contributor
|
I think even the data class fields should be typed to Attribute value because they end up in attributes.. this lgtm though |
Author
|
@eternalcuriouslearner Thanks for checking! I addressed the Copilot suggestions (import grouping, formatting, the get_content_attributes return type) in the follow-up commit c133de9. Happy to take another pass if you spot anything else. |
eternalcuriouslearner
approved these changes
Jun 20, 2026
Contributor
There was a problem hiding this comment.
LGTM, Thanks again for your contribution!! @DylanRussell can we address data class fields in another pr or do you want to address that in this pr?
Contributor
|
Separate is fine.. smaller prs usually better |
Contributor
|
Thanks for confirming @DylanRussell. |
eternalcuriouslearner
requested changes
Jun 20, 2026
eternalcuriouslearner
left a comment
eternalcuriouslearner
left a comment
Contributor
There was a problem hiding this comment.
@bhumikadangayach could you do the following:
- can you please add a changelog in towncrirer format.
- Fix lint, precommit and typecheck.
This file contains hidden or 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
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.
Addresses the span/metric attribute portion of #41.
Replaces
typing.Anywithopentelemetry.util.types.AttributeValueforattributesandmetric_attributesfields and their related getter methods, since these values are ultimately serialized to OTel span/metric attributes andAttributeValueis the correct constraint.Files changed:
_invocation.py(base class)_tool_invocation.py_embedding_invocation.py_agent_invocation.py_workflow_invocation.py_inference_invocation.pyOut of scope (left for follow-up, per discussion in #41):
types.pyholding structured/nested payloads (ToolCallRequest.arguments,ServerToolCall.server_tool_call,FunctionToolDefinition.parameters, etc.) — these aren't flat attribute values and need separate discussion on the right type given semantic-conventions/messages models.All existing tests pass (240/240, no regressions).
Fixes #41 (partial — see scope note above)