fix(semconv): revert deleted semconv attributes#3750
Conversation
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 0b3e3f6 in 9 seconds. Click for details.
- Reviewed
29lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_piqclBsa3Za6OS4K
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds multiple new GenAI-related public span attribute constants to SpanAttributes and bumps the package version from 0.4.14 to 0.4.15; no removals or signature changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py`:
- Around line 79-87: Two class attributes in SpanAttributes are duplicated
(LLM_USAGE_REASONING_TOKENS and LLM_REQUEST_REASONING_EFFORT) causing the later
definitions to override the earlier restored semconv constants; rename one set
so the semconv and legacy keys are unique (e.g., prefix restored semconv
constants with SEMCONV_ or suffix legacy_ for the older keys), update the
attribute names in SpanAttributes (references: LLM_USAGE_REASONING_TOKENS,
LLM_REQUEST_REASONING_EFFORT, and any other duplicated LLM_* symbols), and then
update any import sites or usage to reference the new unique names so both
semconv and legacy constants are available.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py
| LLM_USAGE_REASONING_TOKENS = "gen_ai.usage.reasoning_tokens" | ||
| LLM_USAGE_PROMPT_TOKENS = "gen_ai.usage.prompt_tokens" | ||
| LLM_USAGE_CACHE_CREATION_INPUT_TOKENS = "gen_ai.usage.cache_creation_input_tokens" | ||
| LLM_USAGE_CACHE_READ_INPUT_TOKENS = "gen_ai.usage.cache_read_input_tokens" | ||
| LLM_TOKEN_TYPE = "gen_ai.token.type" | ||
| LLM_REQUEST_STRUCTURED_OUTPUT_SCHEMA = "gen_ai.request.structured_output_schema" | ||
| LLM_REQUEST_REASONING_EFFORT = "gen_ai.request.reasoning_effort" | ||
| LLM_REQUEST_REASONING_SUMMARY = "gen_ai.request.reasoning_summary" | ||
| LLM_RESPONSE_REASONING_EFFORT = "gen_ai.response.reasoning_effort" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the file to see its actual structure
wc -l packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.pyRepository: traceloop/openllmetry
Length of output: 152
🏁 Script executed:
# Read the relevant section around lines 70-110 to verify the duplicate attributes
sed -n '70,110p' packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py | cat -nRepository: traceloop/openllmetry
Length of output: 2437
🏁 Script executed:
# Run the AST verification script from the review to confirm duplicate attribute names
python - <<'PY'
import ast
from pathlib import Path
p = Path("packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py")
tree = ast.parse(p.read_text())
for node in tree.body:
if isinstance(node, ast.ClassDef) and node.name == "SpanAttributes":
seen = {}
for stmt in node.body:
if isinstance(stmt, ast.Assign):
for t in stmt.targets:
if isinstance(t, ast.Name):
seen.setdefault(t.id, []).append(stmt.lineno)
dups = {k: v for k, v in seen.items() if len(v) > 1}
print("=== Duplicate attributes found ===")
for name, lines in sorted(dups.items()):
print(f"{name}: lines {lines}")
if not dups:
print("No duplicate attributes found.")
PYRepository: traceloop/openllmetry
Length of output: 189
Duplicate class attributes are overriding restored semconv constants.
Lines 79 (LLM_USAGE_REASONING_TOKENS) and 85 (LLM_REQUEST_REASONING_EFFORT) are redefined later in the same SpanAttributes class at lines 106 and 105. In Python, the later class assignment wins, so these newly restored gen_ai.* constants are not actually accessible via SpanAttributes.
Proposed fix (make symbol names unique for semconv vs legacy keys)
- LLM_USAGE_REASONING_TOKENS = "gen_ai.usage.reasoning_tokens"
+ LLM_GEN_AI_USAGE_REASONING_TOKENS = "gen_ai.usage.reasoning_tokens"- LLM_REQUEST_REASONING_EFFORT = "gen_ai.request.reasoning_effort"
+ LLM_GEN_AI_REQUEST_REASONING_EFFORT = "gen_ai.request.reasoning_effort"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py`
around lines 79 - 87, Two class attributes in SpanAttributes are duplicated
(LLM_USAGE_REASONING_TOKENS and LLM_REQUEST_REASONING_EFFORT) causing the later
definitions to override the earlier restored semconv constants; rename one set
so the semconv and legacy keys are unique (e.g., prefix restored semconv
constants with SEMCONV_ or suffix legacy_ for the older keys), update the
attribute names in SpanAttributes (references: LLM_USAGE_REASONING_TOKENS,
LLM_REQUEST_REASONING_EFFORT, and any other duplicated LLM_* symbols), and then
update any import sites or usage to reference the new unique names so both
semconv and legacy constants are available.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/version.py`:
- Line 1: Update the backport release on the 0.4.x line by adding a 0.4.15
release and changing the package version constant: modify
opentelemetry/semconv_ai/version.py to set __version__ = "0.4.15" (instead of
only bumping to 0.5.0), create a 0.4.15 release branch or tag, and ensure any
packaging/metadata (if present) that reads version.py is consistent so the
compatibility fix is published on the 0.4.x line without touching the 0.5.0
release.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.pypackages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/version.pypackages/opentelemetry-semantic-conventions-ai/pyproject.toml
✅ Files skipped from review due to trivial changes (1)
- packages/opentelemetry-semantic-conventions-ai/pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/init.py
Important
Reverts deletion of several semantic convention attributes in
SpanAttributesclass made in this PR to maintain backward compatibility.SpanAttributesclass in__init__.py.LLM_SYSTEM,LLM_REQUEST_MODEL,LLM_REQUEST_MAX_TOKENS, and others for backward compatibility.This description was created by
for 0b3e3f6. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Chores
fix #3748
fix #3712