-
Notifications
You must be signed in to change notification settings - Fork 836
feat(agno): add instrumentation for agno framework #3452
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add OpenTelemetry instrumentation for the agno multi-agent framework. The instrumentation captures agent and team execution with full observability support. Features: - Agent.run() and Agent.arun() instrumentation (sync/async) - Team.run() and Team.arun() instrumentation for multi-agent workflows - OpenTelemetry semantic conventions compliance - Token usage and duration metrics - Comprehensive test suite with vcrpy cassettes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughThis pull request introduces OpenTelemetry instrumentation for the Agno agent framework. The implementation includes a new instrumentation package with wrapper classes that capture spans and metrics for Agent/Team execution and FunctionCall operations, along with test fixtures, sample applications, and Traceloop SDK integration. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Agno as Agno Agent/Team
participant Tracer as OpenTelemetry Tracer
participant Span as Span
participant Meter as Meter
participant Histogram as Duration Histogram
User->>Agno: call run/arun
activate Agno
rect rgb(200, 220, 255)
note over Tracer,Span: Span Creation & Setup
Agno->>Tracer: start_as_current_span
activate Tracer
Tracer->>Span: create span (agent/team name)
activate Span
Deactivate Tracer
end
rect rgb(220, 250, 220)
note over Span: Attribute & Data Capture
Span->>Span: set GEN_AI_SYSTEM="agno"
Span->>Span: set TRACELOOP attributes
opt if should_send_prompts()
Span->>Span: record input prompts
Span->>Span: record model details
end
end
rect rgb(255, 240, 200)
note over Agno: Execute Agent/Team Logic
Agno->>Agno: run instrumented logic
Agno->>Span: record output/results
end
rect rgb(250, 220, 220)
note over Meter: Metrics Recording
Meter->>Histogram: record duration_ms
Histogram->>Histogram: add attributes (GEN_AI_SYSTEM, span_kind)
end
Agno->>Span: set status (OK/ERROR)
opt on exception
Span->>Span: record exception
end
Span->>User: return result
deactivate Span
deactivate Agno
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Areas requiring extra attention:
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed everything up to b39082f in 5 minutes and 0 seconds. Click for details.
- Reviewed
1551lines of code in18files - Skipped
1files when reviewing. - Skipped posting
7draft 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.
1. packages/opentelemetry-instrumentation-agno/.flake8:1
- Draft comment:
Flake8 configuration looks appropriate. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/opentelemetry-instrumentation-agno/README.md:1
- Draft comment:
The README clearly documents installation and usage. Ensure that the supported features list remains synced with the implemented wrappers. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py:61
- Draft comment:
The token_histogram is created and passed to all wrapper classes but is never used inside them. Consider recording token usage metrics (if intended) or remove the unused parameter to avoid confusion. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py:112
- Draft comment:
The use of the @dont_throw decorator prevents instrumentation errors from propagating, which is common for non-intrusive instrumentation. Ensure this behavior is well-documented to avoid masking real issues. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
5. packages/opentelemetry-instrumentation-agno/tests/conftest.py:60
- Draft comment:
Test fixtures are well set up for both trace and metrics. Consider adding comments for future customizations. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
6. packages/opentelemetry-instrumentation-agno/tests/test_agent.py:9
- Draft comment:
Test cases cover synchronous, asynchronous, tools, and metrics instrumentation comprehensively. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
7. packages/opentelemetry-instrumentation-agno/pyproject.toml:14
- Draft comment:
Typo in repository URL: it appears "openllmetry" should be "opentelemetry". - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 15% vs. threshold = 50% The comment assumes "openllmetry" is a typo, but this is likely incorrect. The repository name "openllmetry" appears to be intentional branding - it's a portmanteau of "open", "LLM", and "telemetry". This is a project that provides OpenTelemetry instrumentation for LLM-related libraries (like Agno, OpenAI, etc.). The package uses the "opentelemetry" namespace for Python packages (which is standard), but the GitHub repository itself is named "openllmetry". Without being able to verify the actual GitHub repository exists, I cannot be 100% certain, but the naming pattern strongly suggests this is intentional, not a typo. I cannot definitively verify whether the GitHub repository "traceloop/openllmetry" actually exists without external access. It's possible that this is indeed a typo and the repository should be "opentelemetry" instead. The comment could be correct. While I can't verify the repository exists, the consistent use of "openllmetry" as a brand name (combining "open", "LLM", and "telemetry") is a very common pattern for LLM observability tools. The fact that this is under the "traceloop" organization (not the official OpenTelemetry org) and deals with LLM instrumentation strongly suggests "openllmetry" is the correct, intentional name. If this were truly a typo, it would likely cause build/deployment failures that would be caught immediately. This comment should be deleted. "openllmetry" is almost certainly the intentional repository name (a portmanteau of "open", "LLM", and "telemetry"), not a typo. The comment lacks strong evidence and appears to be based on an incorrect assumption about the repository naming.
Workflow ID: wflow_4n1WmJInR9YZXt1C
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
♻️ Duplicate comments (4)
packages/opentelemetry-instrumentation-agno/tests/cassettes/test_agent/test_agent_arun_basic.yaml (1)
1-147: Same sensitive data scrubbing issue as other cassettes.This cassette contains the same categories of sensitive data (organization names, project IDs, cookies, session IDs) that need to be filtered. See the comment on
test_agent_run_basic.yamlfor the fix.packages/opentelemetry-instrumentation-agno/tests/cassettes/test_agent/test_agent_metrics.yaml (1)
1-147: Same sensitive data scrubbing issue as other cassettes.This cassette contains the same categories of sensitive data that need to be filtered. See the comment on
test_agent_run_basic.yamlfor the fix.packages/opentelemetry-instrumentation-agno/tests/cassettes/test_agent/test_agent_with_tools.yaml (1)
1-255: Same sensitive data scrubbing issue as other cassettes.This cassette contains the same categories of sensitive data that need to be filtered. See the comment on
test_agent_run_basic.yamlfor the fix.packages/opentelemetry-instrumentation-agno/tests/conftest.py (1)
67-69: Expand VCR filtering to scrub additional sensitive data.The current
vcr_configonly filters authorization headers, but the cassettes contain additional sensitive data (cookies, organization names, project IDs, session IDs) that should be scrubbed. See the detailed comment ontest_agent_run_basic.yamlfor the complete fix.Based on learnings
🧹 Nitpick comments (11)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/utils.py (3)
8-15: LGTM with note on exception handling.The broad exception catching is appropriate for instrumentation code to ensure that instrumentation failures don't break the application. The debug logging provides visibility into any issues.
Consider making the
Nonereturn explicit for clarity:def dont_throw(func): """Decorator to prevent exceptions from being thrown.""" def wrapper(*args, **kwargs): try: return func(*args, **kwargs) except Exception as e: logger.debug(f"Error in {func.__name__}: {e}") + return None return wrapper
18-28: Add safeguards for edge cases in recursive attribute setting.The recursive implementation should handle potential edge cases:
- Depth limit: Deep nesting could cause stack overflow
- Circular references: Could cause infinite recursion if objects reference themselves
- Non-serializable values: OpenTelemetry span attributes support only primitives and sequences; complex objects may fail
Consider adding safeguards:
-def set_span_attribute(span: Span, name: str, value: Any) -> None: +def set_span_attribute(span: Span, name: str, value: Any, _depth: int = 0, _max_depth: int = 10) -> None: """Set an attribute on a span if the value is not None.""" + if _depth >= _max_depth: + logger.debug(f"Max depth reached for attribute {name}") + return + if value is not None: if isinstance(value, dict): for key, val in value.items(): - set_span_attribute(span, f"{name}.{key}", val) + set_span_attribute(span, f"{name}.{key}", val, _depth + 1, _max_depth) elif isinstance(value, list): for index, item in enumerate(value): - set_span_attribute(span, f"{name}.{index}", item) + set_span_attribute(span, f"{name}.{index}", item, _depth + 1, _max_depth) else: - span.set_attribute(name, value) + try: + span.set_attribute(name, value) + except (TypeError, ValueError) as e: + logger.debug(f"Failed to set attribute {name}: {e}")
31-34: Move import to module level.The
osimport should be at the module level for efficiency, as this function may be called frequently during instrumentation.Apply this diff:
import logging +import os from typing import Any from opentelemetry.trace import SpanAnd update the function:
def should_send_prompts() -> bool: """Check if prompts should be sent based on environment variables.""" - import os return os.getenv("TRACELOOP_TRACE_CONTENT", "true").lower() == "true"packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/config.py (1)
1-5: Consider adding type hints and documentation.The
Configclass uses class attributes for global configuration, which works but could be clearer with type hints and docstrings.+from typing import Callable, Optional + class Config: + """Global configuration for Agno instrumentation.""" + - exception_logger = None - enrich_assistant = True - enrich_token_usage = False - use_legacy_attributes = True + exception_logger: Optional[Callable] = None + enrich_assistant: bool = True + enrich_token_usage: bool = False + use_legacy_attributes: bool = Truepackages/opentelemetry-instrumentation-agno/tests/conftest.py (1)
48-58: Remove unusedreaderparameter.The
readerparameter is not used in theinstrumentfixture body. While the fixture needstracer_providerandmeter_providerfor instrumentation,readeris unnecessary here.@pytest.fixture(scope="function") -def instrument(reader, tracer_provider, meter_provider): +def instrument(tracer_provider, meter_provider): instrumentor = AgnoInstrumentor() instrumentor.instrument( tracer_provider=tracer_provider, meter_provider=meter_provider, ) yield instrumentor instrumentor.uninstrument()packages/opentelemetry-instrumentation-agno/tests/test_agent.py (1)
10-10: Remove unusedreaderparameter from tests that don't check metrics.The
readerparameter is only used intest_agent_metricsto check metrics data. The other three tests don't examine metrics, so the parameter can be removed.@pytest.mark.vcr -def test_agent_run_basic(instrument, span_exporter, reader): +def test_agent_run_basic(instrument, span_exporter): """Test basic agent.run() instrumentation."""Apply the same change to
test_agent_arun_basic(line 40) andtest_agent_with_tools(line 63).Note: The
instrumentparameter warnings are false positives—this fixture has necessary side effects (instrumentation setup) even though the variable isn't directly referenced.Also applies to: 40-40, 63-63
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py (5)
27-35: Unused_get_agno_versionhelper; consider either using it for span attributes or removing it
_get_agno_version()is currently unused. Either:
- use it to set e.g.
SpanAttributes.TRACELOOP_ENTITY_VERSION/ a version attribute on spans, or- drop the helper to avoid dead code.
40-44: Config mutation in__init__is global; clarify intent or scope if needed
AgnoInstrumentor.__init__mutates class-levelConfigattributes (exception_logger,enrich_token_usage) globally. If multiple instrumentors or tests configure these differently, the last one wins, which can be surprising.Consider documenting that these are effectively global settings, or refactoring
Configto be instance‑based if per‑instrumentor isolation is desired.
80-95: Overly broad exception catch around Team instrumentationCatching a broad
Exceptionaroundwrap_function_wrapperhides all errors, not just “team module not present / incompatible version”. That can mask real bugs in instrumentation or module resolution.Recommend narrowing this to likely errors (e.g.,
ImportError,AttributeError) or at least logging atwarninglevel for unexpected failures so they are visible.
96-103: Silent swallow of errors during uninstrumentation
_uninstrumentignoreskwargsand usestry/except Exception: passfor Team unwrapping. While benign, this makes debugging failed uninstrumentation harder and triggers static analysis warnings.Two lightweight options:
- Log failures at debug level instead of bare
pass, and/or- Catch narrower exceptions such as
AttributeError.
145-151: Minor: movetimeimport to module scope to avoid repeated imports in hot pathsEach wrapper imports
timeinside the call body. While function‑local imports are valid, these wrappers are on hot paths and the module is already lightweight. Movingimport timeto the module level slightly reduces per‑call overhead and simplifies the bodies.Also applies to: 232-238, 313-319, 379-385
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
packages/opentelemetry-instrumentation-agno/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
packages/opentelemetry-instrumentation-agno/.flake8(1 hunks)packages/opentelemetry-instrumentation-agno/README.md(1 hunks)packages/opentelemetry-instrumentation-agno/opentelemetry/__init__.py(1 hunks)packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/__init__.py(1 hunks)packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py(1 hunks)packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/config.py(1 hunks)packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/utils.py(1 hunks)packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/version.py(1 hunks)packages/opentelemetry-instrumentation-agno/poetry.toml(1 hunks)packages/opentelemetry-instrumentation-agno/project.json(1 hunks)packages/opentelemetry-instrumentation-agno/pyproject.toml(1 hunks)packages/opentelemetry-instrumentation-agno/tests/cassettes/test_agent/test_agent_arun_basic.yaml(1 hunks)packages/opentelemetry-instrumentation-agno/tests/cassettes/test_agent/test_agent_metrics.yaml(1 hunks)packages/opentelemetry-instrumentation-agno/tests/cassettes/test_agent/test_agent_run_basic.yaml(1 hunks)packages/opentelemetry-instrumentation-agno/tests/cassettes/test_agent/test_agent_with_tools.yaml(1 hunks)packages/opentelemetry-instrumentation-agno/tests/conftest.py(1 hunks)packages/opentelemetry-instrumentation-agno/tests/test_agent.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/cassettes/**/*.{yaml,yml,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Never commit secrets or PII in VCR cassettes; scrub sensitive data
Files:
packages/opentelemetry-instrumentation-agno/tests/cassettes/test_agent/test_agent_arun_basic.yamlpackages/opentelemetry-instrumentation-agno/tests/cassettes/test_agent/test_agent_run_basic.yamlpackages/opentelemetry-instrumentation-agno/tests/cassettes/test_agent/test_agent_metrics.yamlpackages/opentelemetry-instrumentation-agno/tests/cassettes/test_agent/test_agent_with_tools.yaml
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/opentelemetry-instrumentation-agno/opentelemetry/__init__.pypackages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/__init__.pypackages/opentelemetry-instrumentation-agno/tests/test_agent.pypackages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/utils.pypackages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/config.pypackages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/version.pypackages/opentelemetry-instrumentation-agno/tests/conftest.pypackages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: traceloop/openllmetry PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-17T15:06:48.109Z
Learning: Instrumentation packages must leverage the semantic conventions package and emit OTel-compliant spans
📚 Learning: 2025-08-22T14:41:26.962Z
Learnt from: prane-eth
Repo: traceloop/openllmetry PR: 3336
File: packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/utils.py:8-8
Timestamp: 2025-08-22T14:41:26.962Z
Learning: In the openllmetry project, the `packaging` library is available as a transitive dependency through other packages (visible in poetry.lock) and doesn't need to be explicitly declared in pyproject.toml dependencies.
Applied to files:
packages/opentelemetry-instrumentation-agno/pyproject.tomlpackages/opentelemetry-instrumentation-agno/poetry.tomlpackages/opentelemetry-instrumentation-agno/project.json
📚 Learning: 2025-08-17T15:06:48.109Z
Learnt from: CR
Repo: traceloop/openllmetry PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-17T15:06:48.109Z
Learning: Applies to **/*.py : Use Flake8 for code linting and adhere to its rules
Applied to files:
packages/opentelemetry-instrumentation-agno/.flake8
📚 Learning: 2025-08-02T17:22:27.489Z
Learnt from: nirga
Repo: traceloop/openllmetry PR: 3217
File: packages/opentelemetry-instrumentation-haystack/project.json:0-0
Timestamp: 2025-08-02T17:22:27.489Z
Learning: In Nx monorepos, when targetDefaults are defined in nx.json with specific configurations like lintFilePatterns, individual project.json files can have empty target configurations (e.g., "lint": {}) that automatically inherit these defaults. This provides consistent behavior across all projects without needing to duplicate configuration in each project file.
Applied to files:
packages/opentelemetry-instrumentation-agno/project.json
📚 Learning: 2025-08-17T15:06:48.109Z
Learnt from: CR
Repo: traceloop/openllmetry PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-17T15:06:48.109Z
Learning: Applies to tests/**/conftest.py : Use VCR filters (e.g., filter_headers, before_record) or framework equivalents to scrub secrets/PII during recording
Applied to files:
packages/opentelemetry-instrumentation-agno/tests/conftest.py
🧬 Code graph analysis (3)
packages/opentelemetry-instrumentation-agno/tests/test_agent.py (3)
packages/opentelemetry-instrumentation-agno/tests/conftest.py (1)
instrument(49-58)packages/opentelemetry-instrumentation-milvus/tests/conftest.py (1)
reader(37-41)packages/traceloop-sdk/traceloop/sdk/utils/in_memory_span_exporter.py (1)
get_finished_spans(40-43)
packages/opentelemetry-instrumentation-agno/tests/conftest.py (3)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py (1)
AgnoInstrumentor(37-103)packages/traceloop-sdk/traceloop/sdk/utils/in_memory_span_exporter.py (2)
export(45-51)InMemorySpanExporter(22-61)packages/opentelemetry-instrumentation-milvus/tests/conftest.py (2)
reader(37-41)meter_provider(45-50)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py (3)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/config.py (1)
Config(1-5)packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/utils.py (3)
dont_throw(8-15)should_send_prompts(31-34)wrapper(10-14)packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py (3)
Meters(36-61)SpanAttributes(64-245)TraceloopSpanKindValues(285-290)
🪛 Ruff (0.14.5)
packages/opentelemetry-instrumentation-agno/tests/test_agent.py
10-10: Unused function argument: instrument
(ARG001)
10-10: Unused function argument: reader
(ARG001)
40-40: Unused function argument: instrument
(ARG001)
40-40: Unused function argument: reader
(ARG001)
63-63: Unused function argument: instrument
(ARG001)
63-63: Unused function argument: reader
(ARG001)
87-87: Unused function argument: instrument
(ARG001)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/utils.py
13-13: Do not catch blind exception: Exception
(BLE001)
packages/opentelemetry-instrumentation-agno/tests/conftest.py
49-49: Unused function argument: reader
(ARG001)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py
93-93: Do not catch blind exception: Exception
(BLE001)
96-96: Unused method argument: kwargs
(ARG002)
102-103: try-except-pass detected, consider logging the exception
(S110)
102-102: Do not catch blind exception: Exception
(BLE001)
185-185: Consider moving this statement to an else block
(TRY300)
272-272: Consider moving this statement to an else block
(TRY300)
338-338: Consider moving this statement to an else block
(TRY300)
404-404: Consider moving this statement to an else block
(TRY300)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Test Packages (3.10)
- GitHub Check: Build Packages (3.11)
- GitHub Check: Test Packages (3.11)
- GitHub Check: Lint
🔇 Additional comments (12)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/version.py (1)
1-1: LGTM!Version definition is straightforward and matches the version declared in pyproject.toml.
packages/opentelemetry-instrumentation-agno/poetry.toml (1)
1-2: LGTM!Standard Poetry configuration for development environments. Creating the virtualenv in-project simplifies development workflows.
packages/opentelemetry-instrumentation-agno/pyproject.toml (4)
1-8: LGTM!Coverage configuration is appropriate, targeting the correct source directory and excluding type-checking blocks from coverage reports.
9-20: LGTM!Package metadata is well-defined with appropriate repository link, license, and package inclusion configuration.
45-53: LGTM!Build system configuration and plugin registration follow OpenTelemetry instrumentation package conventions correctly. The instrumentor will be discoverable via the
opentelemetry_instrumentorentry point.
21-43: No issues identified—dependencies are appropriately specified.The latest version of opentelemetry-semantic-conventions-ai is 0.4.13, and the current constraint
^0.4.13correctly pins to the latest stable release. The latest agno release is 2.2.13, and requiring>=2.2.2provides reasonable stability while allowing patch updates. The run/arun APIs have been available since agno v2.0.0, so the current minimum version is valid for the instrumentation features used.packages/opentelemetry-instrumentation-agno/.flake8 (1)
1-4: LGTM!Standard Flake8 configuration that aligns with project requirements. The ignored error codes (E203, E266, E501, W503) are commonly excluded in modern Python projects for compatibility with formatters.
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/__init__.py (1)
1-1: LGTM!Standard namespace package declaration that enables the
opentelemetry.instrumentationnamespace to be distributed across multiple packages. This is the correct pattern for OpenTelemetry instrumentation packages.packages/opentelemetry-instrumentation-agno/opentelemetry/__init__.py (1)
1-1: LGTM!Standard namespace package declaration for the
opentelemetrynamespace, following the same pattern as the instrumentation sub-namespace.packages/opentelemetry-instrumentation-agno/README.md (1)
1-31: LGTM!Documentation is clear, concise, and provides the essential information for users to get started with the instrumentation. The usage example is straightforward and the supported features list gives users a good understanding of what's instrumented.
packages/opentelemetry-instrumentation-agno/project.json (1)
1-29: LGTM!The Nx project configuration follows standard patterns for Python packages in the monorepo. The targets (lock, test, lint) are properly configured with Poetry and Flake8.
packages/opentelemetry-instrumentation-agno/tests/test_agent.py (1)
29-30: No critical issues with attribute extraction logic; implementation is consistent internally but deviates from current OpenTelemetry semantic conventions.The instrumentation code and tests use a consistent attribute naming pattern (
gen_ai.prompt.0.content) across all locations. However, OpenTelemetry's current semantic conventions recommend usinggen_ai.input.messages, which when recorded on spans MAY be recorded as a JSON string if structured format is not supported. Thegen_ai.prompt.N.contentpattern appears to be a legacy or pre-release convention. Since the instrumentation and tests are internally aligned, the verification is complete, but consider aligning with the current OTel structured format guidelines for gen_ai.input.messages following the inputs JSON schema for long-term compliance.
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py
Show resolved
Hide resolved
| @dont_throw | ||
| def __call__(self, wrapped, instance, args, kwargs): | ||
| if context_api.get_value( | ||
| context_api._SUPPRESS_INSTRUMENTATION_KEY | ||
| ) or context_api.get_value("suppress_agno_instrumentation"): | ||
| return wrapped(*args, **kwargs) | ||
|
|
||
| span_name = f"agno.agent.{getattr(instance, 'name', 'unknown')}" | ||
|
|
||
| with self._tracer.start_as_current_span( | ||
| span_name, | ||
| kind=SpanKind.CLIENT, | ||
| ) as span: | ||
| try: | ||
| span.set_attribute(GenAIAttributes.GEN_AI_SYSTEM, "agno") | ||
| span.set_attribute(SpanAttributes.TRACELOOP_SPAN_KIND, | ||
| TraceloopSpanKindValues.AGENT.value) | ||
|
|
||
| if hasattr(instance, 'name'): | ||
| span.set_attribute(GenAIAttributes.GEN_AI_AGENT_NAME, instance.name) | ||
|
|
||
| if hasattr(instance, 'model') and instance.model: | ||
| model_name = getattr(instance.model, 'id', | ||
| getattr(instance.model, 'name', 'unknown')) | ||
| span.set_attribute(GenAIAttributes.GEN_AI_REQUEST_MODEL, | ||
| model_name) | ||
|
|
||
| if args and should_send_prompts(): | ||
| input_message = str(args[0]) | ||
| span.set_attribute( | ||
| f"{GenAIAttributes.GEN_AI_PROMPT}.0.content", | ||
| input_message) | ||
|
|
||
| import time | ||
| start_time = time.time() | ||
|
|
||
| result = wrapped(*args, **kwargs) | ||
|
|
||
| duration = time.time() - start_time | ||
|
|
||
| if hasattr(result, 'content') and should_send_prompts(): | ||
| span.set_attribute( | ||
| f"{GenAIAttributes.GEN_AI_COMPLETION}.0.content", | ||
| str(result.content)) | ||
|
|
||
| if hasattr(result, 'run_id'): | ||
| span.set_attribute("agno.run.id", result.run_id) | ||
|
|
||
| if hasattr(result, 'metrics'): | ||
| metrics = result.metrics | ||
| if hasattr(metrics, 'input_tokens'): | ||
| span.set_attribute( | ||
| GenAIAttributes.GEN_AI_USAGE_INPUT_TOKENS, | ||
| metrics.input_tokens) | ||
| if hasattr(metrics, 'output_tokens'): | ||
| span.set_attribute( | ||
| GenAIAttributes.GEN_AI_USAGE_OUTPUT_TOKENS, | ||
| metrics.output_tokens) | ||
| if hasattr(metrics, 'total_tokens'): | ||
| span.set_attribute( | ||
| SpanAttributes.LLM_USAGE_TOTAL_TOKENS, | ||
| metrics.total_tokens) | ||
|
|
||
| span.set_status(Status(StatusCode.OK)) | ||
|
|
||
| self._duration_histogram.record( | ||
| duration, | ||
| attributes={ | ||
| GenAIAttributes.GEN_AI_SYSTEM: "agno", | ||
| SpanAttributes.TRACELOOP_SPAN_KIND: TraceloopSpanKindValues.AGENT.value, | ||
| } | ||
| ) | ||
|
|
||
| return result | ||
|
|
||
| except Exception as e: | ||
| span.set_status(Status(StatusCode.ERROR, str(e))) | ||
| span.record_exception(e) | ||
| raise | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dont_throw decorator will swallow user exceptions from Agent.run
_AgentRunWrapper.__call__ is decorated with @dont_throw, whose implementation catches all Exception and does not re‑raise. That means:
- Any exception from
wrapped(*args, **kwargs)(i.e.,Agent.run) or from the instrumentation logic is swallowed. - The inner
try/exceptthat re‑raises on error is effectively neutralized bydont_throw.
This changes the behavior of Agno agents by turning failures into None returns, which is a correctness and observability bug.
Suggestion:
- Remove
@dont_throwfrom this wrapper (and rely on the innertry/exceptto correctly mark spans and re‑raise), or - Change
dont_throwsemantics so it only swallows instrumentation‑internal failures and always re‑raises exceptions from the wrapped Agno call.
🧰 Tools
🪛 Ruff (0.14.5)
185-185: Consider moving this statement to an else block
(TRY300)
🤖 Prompt for AI Agents
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py
lines 112-191: the @dont_throw decorator on _AgentRunWrapper.__call__ swallows
exceptions from the wrapped Agent.run and neutralizes the inner try/except that
re-raises; remove the @dont_throw decorator from this method so agent exceptions
propagate and the existing inner try/except can set span status/record exception
and re-raise; after removal, clean up any now-unused dont_throw imports.
| class _AgentARunWrapper: | ||
| def __init__(self, tracer, duration_histogram, token_histogram): | ||
| self._tracer = tracer | ||
| self._duration_histogram = duration_histogram | ||
| self._token_histogram = token_histogram | ||
|
|
||
| @dont_throw | ||
| async def __call__(self, wrapped, instance, args, kwargs): | ||
| if context_api.get_value( | ||
| context_api._SUPPRESS_INSTRUMENTATION_KEY | ||
| ) or context_api.get_value("suppress_agno_instrumentation"): | ||
| return await wrapped(*args, **kwargs) | ||
|
|
||
| span_name = f"agno.agent.{getattr(instance, 'name', 'unknown')}" | ||
|
|
||
| with self._tracer.start_as_current_span( | ||
| span_name, | ||
| kind=SpanKind.CLIENT, | ||
| ) as span: | ||
| try: | ||
| span.set_attribute(GenAIAttributes.GEN_AI_SYSTEM, "agno") | ||
| span.set_attribute(SpanAttributes.TRACELOOP_SPAN_KIND, | ||
| TraceloopSpanKindValues.AGENT.value) | ||
|
|
||
| if hasattr(instance, 'name'): | ||
| span.set_attribute(GenAIAttributes.GEN_AI_AGENT_NAME, instance.name) | ||
|
|
||
| if hasattr(instance, 'model') and instance.model: | ||
| model_name = getattr(instance.model, 'id', | ||
| getattr(instance.model, 'name', 'unknown')) | ||
| span.set_attribute(GenAIAttributes.GEN_AI_REQUEST_MODEL, | ||
| model_name) | ||
|
|
||
| if args and should_send_prompts(): | ||
| input_message = str(args[0]) | ||
| span.set_attribute( | ||
| f"{GenAIAttributes.GEN_AI_PROMPT}.0.content", | ||
| input_message) | ||
|
|
||
| import time | ||
| start_time = time.time() | ||
|
|
||
| result = await wrapped(*args, **kwargs) | ||
|
|
||
| duration = time.time() - start_time | ||
|
|
||
| if hasattr(result, 'content') and should_send_prompts(): | ||
| span.set_attribute( | ||
| f"{GenAIAttributes.GEN_AI_COMPLETION}.0.content", | ||
| str(result.content)) | ||
|
|
||
| if hasattr(result, 'run_id'): | ||
| span.set_attribute("agno.run.id", result.run_id) | ||
|
|
||
| if hasattr(result, 'metrics'): | ||
| metrics = result.metrics | ||
| if hasattr(metrics, 'input_tokens'): | ||
| span.set_attribute( | ||
| GenAIAttributes.GEN_AI_USAGE_INPUT_TOKENS, | ||
| metrics.input_tokens) | ||
| if hasattr(metrics, 'output_tokens'): | ||
| span.set_attribute( | ||
| GenAIAttributes.GEN_AI_USAGE_OUTPUT_TOKENS, | ||
| metrics.output_tokens) | ||
| if hasattr(metrics, 'total_tokens'): | ||
| span.set_attribute( | ||
| SpanAttributes.LLM_USAGE_TOTAL_TOKENS, | ||
| metrics.total_tokens) | ||
|
|
||
| span.set_status(Status(StatusCode.OK)) | ||
|
|
||
| self._duration_histogram.record( | ||
| duration, | ||
| attributes={ | ||
| GenAIAttributes.GEN_AI_SYSTEM: "agno", | ||
| SpanAttributes.TRACELOOP_SPAN_KIND: TraceloopSpanKindValues.AGENT.value, | ||
| } | ||
| ) | ||
|
|
||
| return result | ||
|
|
||
| except Exception as e: | ||
| span.set_status(Status(StatusCode.ERROR, str(e))) | ||
| span.record_exception(e) | ||
| raise | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same exception‑swallowing problem for async Agent.arun
The async wrapper has the same @dont_throw decoration and inner try/except as the sync variant. Any exception from await wrapped(*args, **kwargs) will be swallowed by dont_throw, breaking expected error propagation for async agent runs.
Apply the same fix as for _AgentRunWrapper: remove @dont_throw here (and let the inner try/except re‑raise), or adjust dont_throw so it never suppresses real user exceptions.
🧰 Tools
🪛 Ruff (0.14.5)
272-272: Consider moving this statement to an else block
(TRY300)
🤖 Prompt for AI Agents
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py
lines 193-278: the async wrapper _AgentARunWrapper.__call__ is decorated with
@dont_throw which causes real exceptions from await wrapped(*args, **kwargs) to
be swallowed; remove the @dont_throw decorator from the async __call__ method
(or modify dont_throw to never suppress genuine user exceptions) so the existing
inner try/except can re-raise errors and propagate them normally; also remove
any now-unused import of dont_throw if applicable.
| class _TeamRunWrapper: | ||
| def __init__(self, tracer, duration_histogram, token_histogram): | ||
| self._tracer = tracer | ||
| self._duration_histogram = duration_histogram | ||
| self._token_histogram = token_histogram | ||
|
|
||
| @dont_throw | ||
| def __call__(self, wrapped, instance, args, kwargs): | ||
| if context_api.get_value( | ||
| context_api._SUPPRESS_INSTRUMENTATION_KEY | ||
| ) or context_api.get_value("suppress_agno_instrumentation"): | ||
| return wrapped(*args, **kwargs) | ||
|
|
||
| span_name = f"agno.team.{getattr(instance, 'name', 'unknown')}" | ||
|
|
||
| with self._tracer.start_as_current_span( | ||
| span_name, | ||
| kind=SpanKind.CLIENT, | ||
| ) as span: | ||
| try: | ||
| span.set_attribute(GenAIAttributes.GEN_AI_SYSTEM, "agno") | ||
| span.set_attribute(SpanAttributes.TRACELOOP_SPAN_KIND, | ||
| TraceloopSpanKindValues.WORKFLOW.value) | ||
|
|
||
| if hasattr(instance, 'name'): | ||
| span.set_attribute("agno.team.name", instance.name) | ||
|
|
||
| if args and should_send_prompts(): | ||
| input_message = str(args[0]) | ||
| span.set_attribute( | ||
| f"{GenAIAttributes.GEN_AI_PROMPT}.0.content", | ||
| input_message) | ||
|
|
||
| import time | ||
| start_time = time.time() | ||
|
|
||
| result = wrapped(*args, **kwargs) | ||
|
|
||
| duration = time.time() - start_time | ||
|
|
||
| if hasattr(result, 'content') and should_send_prompts(): | ||
| span.set_attribute( | ||
| f"{GenAIAttributes.GEN_AI_COMPLETION}.0.content", | ||
| str(result.content)) | ||
|
|
||
| if hasattr(result, 'run_id'): | ||
| span.set_attribute("agno.run.id", result.run_id) | ||
|
|
||
| span.set_status(Status(StatusCode.OK)) | ||
|
|
||
| self._duration_histogram.record( | ||
| duration, | ||
| attributes={ | ||
| GenAIAttributes.GEN_AI_SYSTEM: "agno", | ||
| SpanAttributes.TRACELOOP_SPAN_KIND: TraceloopSpanKindValues.WORKFLOW.value, | ||
| } | ||
| ) | ||
|
|
||
| return result | ||
|
|
||
| except Exception as e: | ||
| span.set_status(Status(StatusCode.ERROR, str(e))) | ||
| span.record_exception(e) | ||
| raise | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Team sync wrapper also swallows exceptions due to @dont_throw
_TeamRunWrapper.__call__ is decorated with @dont_throw, so any exception from Team.run (or from the instrumentation) is caught and silently dropped. That prevents failures in multi‑agent workflows from surfacing to callers and contradicts the inner try/except that intends to re‑raise.
As above, recommend removing @dont_throw from this wrapper or updating dont_throw not to suppress user exceptions.
🧰 Tools
🪛 Ruff (0.14.5)
338-338: Consider moving this statement to an else block
(TRY300)
🤖 Prompt for AI Agents
In
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py
around lines 280-344, the @dont_throw decorator on _TeamRunWrapper.__call__
swallows all exceptions (including user errors) preventing them from
propagating; remove the @dont_throw decorator from this method so the inner
try/except can set span status and record then re-raise exceptions, or if you
prefer keeping dont_throw, change dont_throw so it only suppresses its own
instrumentation-internal errors and not exceptions raised by the wrapped
function; after the change, run unit/integration tests to confirm exceptions
propagate and spans still record status and exceptions correctly.
| class _TeamARunWrapper: | ||
| def __init__(self, tracer, duration_histogram, token_histogram): | ||
| self._tracer = tracer | ||
| self._duration_histogram = duration_histogram | ||
| self._token_histogram = token_histogram | ||
|
|
||
| @dont_throw | ||
| async def __call__(self, wrapped, instance, args, kwargs): | ||
| if context_api.get_value( | ||
| context_api._SUPPRESS_INSTRUMENTATION_KEY | ||
| ) or context_api.get_value("suppress_agno_instrumentation"): | ||
| return await wrapped(*args, **kwargs) | ||
|
|
||
| span_name = f"agno.team.{getattr(instance, 'name', 'unknown')}" | ||
|
|
||
| with self._tracer.start_as_current_span( | ||
| span_name, | ||
| kind=SpanKind.CLIENT, | ||
| ) as span: | ||
| try: | ||
| span.set_attribute(GenAIAttributes.GEN_AI_SYSTEM, "agno") | ||
| span.set_attribute(SpanAttributes.TRACELOOP_SPAN_KIND, | ||
| TraceloopSpanKindValues.WORKFLOW.value) | ||
|
|
||
| if hasattr(instance, 'name'): | ||
| span.set_attribute("agno.team.name", instance.name) | ||
|
|
||
| if args and should_send_prompts(): | ||
| input_message = str(args[0]) | ||
| span.set_attribute( | ||
| f"{GenAIAttributes.GEN_AI_PROMPT}.0.content", | ||
| input_message) | ||
|
|
||
| import time | ||
| start_time = time.time() | ||
|
|
||
| result = await wrapped(*args, **kwargs) | ||
|
|
||
| duration = time.time() - start_time | ||
|
|
||
| if hasattr(result, 'content') and should_send_prompts(): | ||
| span.set_attribute( | ||
| f"{GenAIAttributes.GEN_AI_COMPLETION}.0.content", | ||
| str(result.content)) | ||
|
|
||
| if hasattr(result, 'run_id'): | ||
| span.set_attribute("agno.run.id", result.run_id) | ||
|
|
||
| span.set_status(Status(StatusCode.OK)) | ||
|
|
||
| self._duration_histogram.record( | ||
| duration, | ||
| attributes={ | ||
| GenAIAttributes.GEN_AI_SYSTEM: "agno", | ||
| SpanAttributes.TRACELOOP_SPAN_KIND: TraceloopSpanKindValues.WORKFLOW.value, | ||
| } | ||
| ) | ||
|
|
||
| return result | ||
|
|
||
| except Exception as e: | ||
| span.set_status(Status(StatusCode.ERROR, str(e))) | ||
| span.record_exception(e) | ||
| raise |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Async team wrapper has same exception‑swallowing behavior
_TeamARunWrapper.__call__ is affected by the same @dont_throw pattern, swallowing exceptions from Team.arun and from the wrapper itself. This can make async workflow failures invisible to callers.
Apply the same resolution (drop @dont_throw from this wrapper, or change dont_throw to always re‑raise user exceptions) for consistency and correctness.
🧰 Tools
🪛 Ruff (0.14.5)
404-404: Consider moving this statement to an else block
(TRY300)
🤖 Prompt for AI Agents
In
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py
around lines 346 to 409, the async _TeamARunWrapper.__call__ is decorated with
@dont_throw which swallows exceptions from Team.arun and the wrapper itself;
remove the @dont_throw decorator from this method so exceptions propagate to
callers (or alternatively change dont_throw to re-raise user exceptions), and
run/update tests to ensure async failures surface correctly.
| Set-Cookie: | ||
| - __cf_bm=j1HmyBbmOo_vbqZNBbzZ3YT3rhrsFkgJ9oQuviJq6ec-1763640358-1.0.1.1-7J4DSSib19X13jK70GwaQlYf_aXs.MciMb2ityRo6Xd4o.p0MTU1HBAJJ4CyyHjTl483JuWEIEBFiOpqJrhrgInMMO3f4rIj30FDVB0SntA; | ||
| path=/; expires=Thu, 20-Nov-25 12:35:58 GMT; domain=.api.openai.com; HttpOnly; | ||
| Secure; SameSite=None | ||
| - _cfuvid=b3Ibgiaod9sVeSM84TmRwkxMXvNX_z8qpiHCpV2IaBc-1763640358812-0.0.1.1-604800000; | ||
| path=/; domain=.api.openai.com; HttpOnly; Secure; SameSite=None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scrub additional sensitive data from VCR cassettes.
The cassettes contain sensitive information beyond authorization headers that should be filtered:
- Organization name "traceloop" (line 83)
- Project IDs (line 87)
- Cookie values including session tokens (lines 65-69)
- Session and run IDs (line 112)
Per coding guidelines, VCR cassettes must not contain secrets or PII.
Apply VCR filters to scrub these fields. In conftest.py, expand the vcr_config fixture:
@pytest.fixture(scope="module")
def vcr_config():
- return {"filter_headers": ["authorization", "x-api-key"]}
+ return {
+ "filter_headers": ["authorization", "x-api-key", "cookie", "set-cookie"],
+ "before_record_response": lambda response: _scrub_response_headers(response),
+ "before_record_request": lambda request: _scrub_request_body(request),
+ }Add helper functions to scrub organization names, project IDs, and UUIDs from request/response bodies and headers.
Based on learnings
Also applies to: 82-87, 106-107, 112-112
🤖 Prompt for AI Agents
In
packages/opentelemetry-instrumentation-agno/tests/cassettes/test_agent/test_agent_run_basic.yaml
around lines 64-69 (and also lines 82-87, 106-107, 112), the cassette contains
sensitive cookies, organization name "traceloop", project IDs and session/run
UUIDs that must be scrubbed; update conftest.py's vcr_config fixture to add
filters that replace cookie values (Set-Cookie and Cookie headers), organization
names, project IDs and any UUID-like session/run IDs in both request/response
headers and bodies with deterministic placeholders (e.g. <COOKIE>, <ORG>,
<PROJECT_ID>, <UUID>), and add small helper functions used by the vcr hooks to
perform regex-based replacement for org names, project-id patterns and UUIDs so
cassettes are saved without secrets.
Add comprehensive examples and tests for agno instrumentation: Sample Applications: - agno_example.py: Basic agent with weather tool - agno_async_example.py: Async agent with math tools - agno_team_example.py: Multi-agent team (researcher + writer) - agno_discussion_team.py: Expert discussion team with synthesis Tests: - test_team.py: Team instrumentation tests with VCR cassettes - test_team_discussion: Multi-agent discussion scenario - test_team_basic: Basic team functionality All examples and tests validated: - ✅ Sample apps run successfully with full tracing - ✅ All 6 tests pass (4 agent + 2 team) - ✅ Flake8 linting passes - ✅ VCR cassettes recorded for reproducibility 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Address PR review feedback: - Add comprehensive docstrings to all wrapper classes - Add docstrings to Config class attributes - Improve VCR cassette filtering to scrub sensitive headers: - cookie, set-cookie headers - x-request-id, x-openai-organization - api_key parameters This improves documentation coverage and ensures no sensitive data leaks into version control through test cassettes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (13)
packages/sample-app/sample_app/agno_example.py (2)
1-8: Import-timeTraceloop.initis fine for a script but may surprise importersFor a standalone sample this is OK, but if
agno_exampleever gets imported as a module, it will auto-initialize Traceloop on import (including network config, env resolution, etc.). Consider movingTraceloop.init(...)into theif __name__ == "__main__":block (or a smallmain()function) to keep side-effects tied to explicit execution.
15-32: Improve API key handling and verify decorator/response usageOverall the agent wiring looks good, but a few points to tighten:
- Using
os.environ.get("OPENAI_API_KEY")can silently passNoneintoOpenAIChatwhen the key is missing, which tends to produce less clear errors later. Prefer failing fast with an explicit lookup or validation, e.g. raising if the env var is unset, or usingos.environ["OPENAI_API_KEY"]and catchingKeyErrorif you want a custom message. This keeps secrets in env vars as required and improves debuggability. As per coding guidelines- You’re decorating
run_agentwith@workflow(...)even though there is an@agent(...)decorator available intraceloop.sdk.decorators. If you intend this span to represent an agent rather than a generic workflow, it may be worth switching to@agentfor clearer semantics and consistency with other agent examples. Please double-check against how you want spans to appear in Traceloop.- Accessing
result.contentassumes the AgnoAgent.run()return type exposes a.contentattribute for the pinnedagnoversion. Please verify this still holds for the version declared in the sample-apppyproject.tomlto avoid runtime attribute errors.packages/sample-app/sample_app/agno_team_example.py (2)
13-21: Consider validating the API key for better error messages.While retrieving the API key from environment variables correctly follows security guidelines,
os.environ.get()returnsNoneif the key is missing, which can lead to unclear runtime errors when the model is used. For a sample app, consider adding a check at the start of the function to provide a clear error message ifOPENAI_API_KEYis not set.Example validation at the start of
run_team():@workflow(name="agno_team_example") def run_team(): api_key = os.environ.get("OPENAI_API_KEY") if not api_key: raise ValueError("OPENAI_API_KEY environment variable must be set") researcher = Agent( name="Researcher", model=OpenAIChat( id="gpt-4o-mini", api_key=api_key, ), # ... rest of config
49-53: Consider adding basic error handling for team execution.The team execution demonstrates the multi-agent workflow clearly. For a more robust sample, consider wrapping the team execution in a try/except to provide clear feedback if the execution fails (e.g., API errors, rate limits, network issues).
Example:
try: print("Running multi-agent team...") result = team.run("Create a short article about OpenTelemetry observability") print(f"\nTeam response: {result.content}") return result except Exception as e: print(f"\nError during team execution: {e}") raisepackages/sample-app/sample_app/agno_discussion_team.py (4)
9-9: Consider aligning app_name with workflow name for consistency.The Traceloop app name is
"agno_discussion_team"while the workflow decorator uses"agno_discussion_team_example". If these are intended to match, consider using the same name for clarity. If the distinction is intentional (app-level vs workflow-level naming), the current setup is fine.
19-65: Add validation for missing OPENAI_API_KEY environment variable.The agents use
os.environ.get("OPENAI_API_KEY")which returnsNoneif the key is unset. This can lead to cryptic errors when the OpenAI model attempts authentication. For a better user experience in this sample application, consider adding early validation:api_key = os.environ.get("OPENAI_API_KEY") if not api_key: raise ValueError( "OPENAI_API_KEY environment variable is required. " "Please set it before running this example." )Then reference
api_keyin each model instantiation.
Optional: Reduce code duplication in agent creation.
The three agents follow an identical pattern. While the explicit code aids clarity for demonstration purposes, you could optionally extract a helper function to reduce duplication:
def create_expert_agent(name: str, role: str, instructions: str, api_key: str) -> Agent: return Agent( name=name, role=role, model=OpenAIChat(id="gpt-4o-mini", api_key=api_key), add_name_to_context=True, instructions=dedent(instructions), )Similarly, the model ID
"gpt-4o-mini"could be extracted to a module-level constant.
67-87: Add validation for missing OPENAI_API_KEY (same as agents).The team model initialization also uses
os.environ.get("OPENAI_API_KEY")at line 72. Apply the same early validation recommended for the agent models to ensure a clear error message if the environment variable is not set.
89-96: Consider adding error handling around team execution.The
discussion_team.run()call has no error handling. While letting exceptions propagate is acceptable for sample code, adding a try-except block could improve the user experience by providing clearer guidance when things go wrong (e.g., network issues, API quota limits, authentication failures):try: print("Starting expert discussion team...\n") result = discussion_team.run( "Discuss the topic: 'Should we adopt AI-powered code review tools in our development workflow?'" ) print(f"\n{'='*80}\nFinal Team Consensus:\n{'='*80}") print(result.content) return result except Exception as e: print(f"Error during team discussion: {e}") raisepackages/sample-app/pyproject.toml (1)
61-61: Tightenagnoversion constraint to avoid unexpected breaking upgrades
agno = ">=2.2.2"will accept any future major version and could break the sample app if Agno introduces breaking changes. Other deps here typically use^or~.Consider switching to a caret constraint for safer upgrades, e.g.:
-agno = ">=2.2.2" +agno = "^2.2.2"This still allows compatible 2.x updates while avoiding accidental 3.x+ breakage. The new
opentelemetry-instrumentation-agnopath dependency looks consistent with the other instrumentation packages.Please double‑check Agno’s published versioning policy and your tested range before changing this, in case they don’t strictly follow semver.
Also applies to: 104-106
packages/sample-app/sample_app/agno_async_example.py (2)
1-8: Consider avoidingTraceloop.initat import timeInitializing Traceloop at module import means any test or code that imports
agno_async_examplewill automatically initialize tracing, which may be surprising and harder to control.You could move initialization under the main guard or into a small bootstrap function, for example:
-Traceloop.init(app_name="agno_async_example") - - -if __name__ == "__main__": - asyncio.run(run_async_agent()) +def _init_traceloop() -> None: + Traceloop.init(app_name="agno_async_example") + + +if __name__ == "__main__": + _init_traceloop() + asyncio.run(run_async_agent())This keeps behavior the same when run as a script but avoids side effects on mere import.
If
Traceloop.inithas specific expectations around being called once or multiple times, please confirm in its docs that this pattern matches recommended usage.Also applies to: 43-44
21-40: Fail fast whenOPENAI_API_KEYis missing for a clearer error pathRight now the API key is pulled inline via
os.environ.get("OPENAI_API_KEY"). If it’s missing, you’ll passNoneintoOpenAIChat, which may fail later with a less obvious error.Consider checking once and failing fast with a clear message, and reusing the variable:
@workflow(name="agno_async_agent_example") async def run_async_agent(): - agent = Agent( + api_key = os.environ.get("OPENAI_API_KEY") + if not api_key: + raise RuntimeError("OPENAI_API_KEY environment variable is not set") + + agent = Agent( name="MathAssistant", model=OpenAIChat( id="gpt-4o-mini", - api_key=os.environ.get("OPENAI_API_KEY"), + api_key=api_key, ),This keeps secrets in env vars (as required) while improving developer experience.
packages/opentelemetry-instrumentation-agno/tests/test_team.py (2)
46-53: Consider validating metrics in addition to spans.The PR objectives state that the instrumentation "Records token usage and duration metrics using histograms." However, the test only validates span attributes and does not verify that metrics are being recorded. Consider adding assertions to validate metric collection.
For example, after line 53, add:
# Validate metrics were recorded metrics = reader.get_metrics_data() # Add assertions for token usage and duration histograms
77-83: Consider validating metrics collection.Similar to
test_team_discussion, this test should validate that token usage and duration metrics are being recorded as stated in the PR objectives.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
packages/sample-app/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
packages/opentelemetry-instrumentation-agno/tests/cassettes/test_team/test_team_basic.yaml(1 hunks)packages/opentelemetry-instrumentation-agno/tests/cassettes/test_team/test_team_discussion.yaml(1 hunks)packages/opentelemetry-instrumentation-agno/tests/test_team.py(1 hunks)packages/sample-app/pyproject.toml(2 hunks)packages/sample-app/sample_app/agno_async_example.py(1 hunks)packages/sample-app/sample_app/agno_discussion_team.py(1 hunks)packages/sample-app/sample_app/agno_example.py(1 hunks)packages/sample-app/sample_app/agno_team_example.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/cassettes/**/*.{yaml,yml,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Never commit secrets or PII in VCR cassettes; scrub sensitive data
Files:
packages/opentelemetry-instrumentation-agno/tests/cassettes/test_team/test_team_basic.yamlpackages/opentelemetry-instrumentation-agno/tests/cassettes/test_team/test_team_discussion.yaml
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/opentelemetry-instrumentation-agno/tests/test_team.pypackages/sample-app/sample_app/agno_async_example.pypackages/sample-app/sample_app/agno_team_example.pypackages/sample-app/sample_app/agno_discussion_team.pypackages/sample-app/sample_app/agno_example.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: traceloop/openllmetry PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-17T15:06:48.109Z
Learning: Instrumentation packages must leverage the semantic conventions package and emit OTel-compliant spans
📚 Learning: 2025-08-22T14:41:26.962Z
Learnt from: prane-eth
Repo: traceloop/openllmetry PR: 3336
File: packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/utils.py:8-8
Timestamp: 2025-08-22T14:41:26.962Z
Learning: In the openllmetry project, the `packaging` library is available as a transitive dependency through other packages (visible in poetry.lock) and doesn't need to be explicitly declared in pyproject.toml dependencies.
Applied to files:
packages/sample-app/pyproject.toml
🧬 Code graph analysis (5)
packages/opentelemetry-instrumentation-agno/tests/test_team.py (2)
packages/opentelemetry-instrumentation-milvus/tests/conftest.py (1)
reader(37-41)packages/traceloop-sdk/traceloop/sdk/utils/in_memory_span_exporter.py (1)
get_finished_spans(40-43)
packages/sample-app/sample_app/agno_async_example.py (2)
packages/traceloop-sdk/traceloop/sdk/decorators/__init__.py (1)
agent(50-60)packages/traceloop-sdk/traceloop/sdk/__init__.py (2)
Traceloop(37-275)init(49-206)
packages/sample-app/sample_app/agno_team_example.py (2)
packages/traceloop-sdk/traceloop/sdk/decorators/__init__.py (1)
agent(50-60)packages/traceloop-sdk/traceloop/sdk/__init__.py (2)
Traceloop(37-275)init(49-206)
packages/sample-app/sample_app/agno_discussion_team.py (2)
packages/traceloop-sdk/traceloop/sdk/decorators/__init__.py (1)
agent(50-60)packages/traceloop-sdk/traceloop/sdk/__init__.py (2)
Traceloop(37-275)init(49-206)
packages/sample-app/sample_app/agno_example.py (2)
packages/traceloop-sdk/traceloop/sdk/decorators/__init__.py (1)
agent(50-60)packages/traceloop-sdk/traceloop/sdk/__init__.py (2)
Traceloop(37-275)init(49-206)
🪛 Ruff (0.14.5)
packages/opentelemetry-instrumentation-agno/tests/test_team.py
12-12: Unused function argument: instrument
(ARG001)
12-12: Unused function argument: reader
(ARG001)
57-57: Unused function argument: instrument
(ARG001)
57-57: Unused function argument: reader
(ARG001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Packages (3.10)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Test Packages (3.11)
- GitHub Check: Build Packages (3.11)
- GitHub Check: Lint
🔇 Additional comments (8)
packages/sample-app/sample_app/agno_example.py (2)
10-12:get_weatherhelper is clear and side-effect freeThe function is simple, deterministic, and well-suited as a demo tool function for the agent. No issues from a correctness or style perspective.
35-36: Script entrypoint is conventional and appropriateUsing the
if __name__ == "__main__":guard to triggerrun_agent()makes this module usable both as an importable example and a runnable script. No changes needed here.packages/sample-app/sample_app/agno_team_example.py (3)
1-8: LGTM! Clean setup.The imports and Traceloop initialization are appropriate for demonstrating the Agno framework with observability.
23-34: Writer agent configuration looks good.The agent is properly configured with role-specific instructions that complement the researcher agent.
56-57: LGTM! Standard main guard pattern.The main guard allows the sample to be run directly or imported as a module.
packages/sample-app/sample_app/agno_async_example.py (1)
11-19: Helper tool functions are simple and correct
calculate_sumandcalculate_productare clearly named, typed, and implement the expected math operations; they’re suitable as tools for the agent.packages/opentelemetry-instrumentation-agno/tests/test_team.py (2)
57-57: Verify fixture usage is intentional.Same as the previous test, the
instrumentandreaderfixtures are not explicitly used in the test body. Verify these fixtures perform necessary setup/teardown operations.
12-12: Fixturesinstrumentandreaderare correctly used and necessary.Both fixtures have important side effects and should remain:
instrument(conftest.py:48-53): Performs AgnoInstrumentor setup by callinginstrumentor.instrument()with tracer and meter providersreader(conftest.py:33-38): Initializes InMemoryMetricReader needed by the instrumentation contextThese are passed as test parameters for pytest dependency injection and are required for proper test setup, not unused fixtures.
| Set-Cookie: | ||
| - __cf_bm=qfaDZQv2phcDvwqvX6GyFcJ1sWvZAe4nYKZxzyMzNL0-1763642370-1.0.1.1-8p3Uw61_AlP9ro7r5nUpB4jNtnwcj_TnTaLiYlMdtnaKmvbvL__k_6jblj5Mf23PYilFukjJc07lEKEpX3qmQ1hnxyBgHDM9OsAZCOIbjDY; | ||
| path=/; expires=Thu, 20-Nov-25 13:09:30 GMT; domain=.api.openai.com; HttpOnly; | ||
| Secure; SameSite=None | ||
| - _cfuvid=HCYwDFbEfKYFULXMBTKw2uh21fyJxKSyl54MA21Np.k-1763642370106-0.0.1.1-604800000; | ||
| path=/; domain=.api.openai.com; HttpOnly; Secure; SameSite=None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scrub sensitive data from VCR cassettes.
This cassette contains potentially sensitive information that should be scrubbed:
- Session cookies (
__cf_bm,_cfuvid) in Set-Cookie headers (lines 89-94) - Session IDs and run IDs in telemetry requests (line 137)
- OpenAI project ID
proj_tzz1TbPPOXaf6j9tEkVUBIAa(line 112)
As per coding guidelines, VCR cassettes must not contain secrets or PII. Consider using VCR's filter capabilities to scrub these values before committing.
Apply scrubbing in the VCR configuration (in conftest.py):
@pytest.fixture(scope="module")
def vcr_config():
return {
"filter_headers": ["authorization", "cookie", "set-cookie"],
"filter_post_data_parameters": ["session_id", "run_id"],
# Add other sensitive fields
}Also applies to: 137-137
| Set-Cookie: | ||
| - __cf_bm=BhRFtj93vCoNVFJcv_iaU84WeFSDgSWfwZpPERI6Vy8-1763642339-1.0.1.1-BGoBQJ92ZptAm0nckizAh4Nlamhc6NvnZaKxDdCOqTz4czdO0iOrd4TD4a4E8ayXJ9rIqUcfXf0dRpVN_9bbIdbTA8YCLtxxrvMoVZbPCIE; | ||
| path=/; expires=Thu, 20-Nov-25 13:08:59 GMT; domain=.api.openai.com; HttpOnly; | ||
| Secure; SameSite=None | ||
| - _cfuvid=crap2UtZDB1vAkCZF4C4ckS7lxjJuVAlGnxS.ZUBDiM-1763642339428-0.0.1.1-604800000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scrub sensitive data from VCR cassettes.
This cassette contains multiple instances of potentially sensitive information:
- Session cookies (
__cf_bm,_cfuvid) appearing in multiple Set-Cookie headers throughout the file - Session IDs and run IDs in telemetry requests (lines 262, 429, 659)
- OpenAI project ID
proj_tzz1TbPPOXaf6j9tEkVUBIAaappearing in multiple responses
These values should be scrubbed before committing to comply with the coding guideline: "Never commit secrets or PII in VCR cassettes; scrub sensitive data."
As per coding guidelines
Also applies to: 214-219, 262-262, 429-429, 554-556, 659-659
🤖 Prompt for AI Agents
packages/opentelemetry-instrumentation-agno/tests/cassettes/test_team/test_team_discussion.yaml
lines 84-88 (and also apply to lines 214-219, 262, 429, 554-556, 659): this
cassette contains sensitive Set-Cookie values, session/run IDs and a real OpenAI
project ID; replace all cookie values (__cf_bm, _cfuvid) with a neutral
placeholder (e.g. <SCRUBBED_COOKIE>), replace any session IDs/run IDs with
<SCRUBBED_SESSION_ID> or <SCRUBBED_RUN_ID>, and replace the project id
proj_tzz1TbPPOXaf6j9tEkVUBIAa with <SCRUBBED_PROJECT_ID> consistently throughout
the file; ensure all occurrences (headers and response bodies) are replaced,
then save the cassette and re-run tests to confirm no sensitive values remain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed 71688ca in 3 minutes and 59 seconds. Click for details.
- Reviewed
1251lines of code in8files - Skipped
0files when reviewing. - Skipped posting
9draft 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.
1. packages/opentelemetry-instrumentation-agno/tests/cassettes/test_team/test_team_basic.yaml:1
- Draft comment:
Cassette file appears auto‐generated and comprehensive. Verify that it stays in sync with any updates to instrumentation responses. - Reason this comment was not posted:
Confidence changes required:10%<= threshold50%None
2. packages/opentelemetry-instrumentation-agno/tests/cassettes/test_team/test_team_discussion.yaml:1
- Draft comment:
The large cassette file captures multiple interactions and responses. Consider maintaining its structure with documentation—ensure long-term maintainability if API responses change. - Reason this comment was not posted:
Confidence changes required:10%<= threshold50%None
3. packages/opentelemetry-instrumentation-agno/tests/test_team.py:52
- Draft comment:
Test assertions check that the span's attributes include the expected GEN_AI_SYSTEM value. Consider adding more detailed span validations (e.g., span name formats) if needed. - Reason this comment was not posted:
Confidence changes required:10%<= threshold50%None
4. packages/sample-app/sample_app/agno_async_example.py:35
- Draft comment:
Async agent example using tool calls is clean. In production, consider adding error handling for tool integration failures. - Reason this comment was not posted:
Confidence changes required:10%<= threshold50%None
5. packages/sample-app/sample_app/agno_discussion_team.py:80
- Draft comment:
The multi-agent discussion team example is clear and well-organized. For future enhancements, consider modularizing the instructions to allow easier updates to each agent's role. - Reason this comment was not posted:
Confidence changes required:10%<= threshold50%None
6. packages/sample-app/pyproject.toml:61
- Draft comment:
The dependency for 'agno' is set to ">=2.2.2". Ensure version compatibility across packages and consider pinning a known stable version if needed. - Reason this comment was not posted:
Confidence changes required:10%<= threshold50%None
7. packages/sample-app/sample_app/agno_example.py:10
- Draft comment:
The weather example is straightforward. For robustness, consider error handling when the weather function is integrated into the agent. - Reason this comment was not posted:
Confidence changes required:10%<= threshold50%None
8. packages/sample-app/sample_app/agno_team_example.py:1
- Draft comment:
The team example combining a Researcher and Writer is clear. As the team grows, consider validating that the team composition logic scales and preserves response quality. - Reason this comment was not posted:
Confidence changes required:10%<= threshold50%None
9. packages/opentelemetry-instrumentation-agno/tests/cassettes/test_team/test_team_discussion.yaml:470
- Draft comment:
There appears to be a spurious backslash before the dash in the "Business perspective" entry. It should likely be removed. - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_6sYeyNxGEs6tckpB
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py (1)
40-43:enrich_token_usageparameter is set but never used.The
enrich_token_usageparameter is stored inConfig.enrich_token_usagebut is never checked in any of the wrapper classes. This suggests incomplete implementation of token usage enrichment functionality.Additionally, the
token_histogramcreated in_instrument(lines 61-65) is never used to record metrics anywhere in the code.Either:
- Implement the token usage enrichment logic by checking
Config.enrich_token_usagein the wrappers and recording totoken_histogramwhen token metrics are available, or- Remove the unused parameter and histogram until the feature is fully implemented.
🧹 Nitpick comments (3)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py (2)
149-150: Importtimeat module level for better performance.The
timemodule is imported inside the method on line 149 (and similarly in other wrappers at lines 240, 325, 395). Module imports inside frequently-called methods add unnecessary overhead.Apply this diff to move the import to the top of the file:
import logging +import time from typing import Collection from importlib.metadata import version as package_version, PackageNotFoundErrorThen remove the inline imports from all wrapper methods.
317-317: Consider using consistent semantic conventions for team names.The team wrapper uses the custom attribute
"agno.team.name"(line 317), while the agent wrapper usesGenAIAttributes.GEN_AI_AGENT_NAME(lines 135, 226).For consistency with OpenTelemetry semantic conventions, consider either:
- Using a GenAI semantic convention attribute if one exists for teams/workflows, or
- Documenting why the custom
agno.team.nameattribute is necessary.Based on learnings.
packages/opentelemetry-instrumentation-agno/tests/conftest.py (1)
48-59: Remove unusedreaderparameter from fixture signature.The
readerparameter is not used in the fixture body. While it's a dependency ofmeter_provider, pytest will automatically resolve the dependency chain without explicitly listingreaderhere.Apply this diff:
@pytest.fixture(scope="function") -def instrument(reader, tracer_provider, meter_provider): +def instrument(tracer_provider, meter_provider): instrumentor = AgnoInstrumentor()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py(1 hunks)packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/config.py(1 hunks)packages/opentelemetry-instrumentation-agno/tests/conftest.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/config.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/opentelemetry-instrumentation-agno/tests/conftest.pypackages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: traceloop/openllmetry PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-17T15:06:48.109Z
Learning: Instrumentation packages must leverage the semantic conventions package and emit OTel-compliant spans
🧬 Code graph analysis (2)
packages/opentelemetry-instrumentation-agno/tests/conftest.py (2)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py (1)
AgnoInstrumentor(37-103)packages/traceloop-sdk/traceloop/sdk/utils/in_memory_span_exporter.py (2)
export(45-51)InMemorySpanExporter(22-61)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py (3)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/config.py (1)
Config(1-14)packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/utils.py (3)
dont_throw(8-15)should_send_prompts(31-34)wrapper(10-14)packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py (3)
Meters(36-61)SpanAttributes(64-245)TraceloopSpanKindValues(285-290)
🪛 Ruff (0.14.5)
packages/opentelemetry-instrumentation-agno/tests/conftest.py
49-49: Unused function argument: reader
(ARG001)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py
93-93: Do not catch blind exception: Exception
(BLE001)
96-96: Unused method argument: kwargs
(ARG002)
102-103: try-except-pass detected, consider logging the exception
(S110)
102-102: Do not catch blind exception: Exception
(BLE001)
189-189: Consider moving this statement to an else block
(TRY300)
280-280: Consider moving this statement to an else block
(TRY300)
350-350: Consider moving this statement to an else block
(TRY300)
420-420: Consider moving this statement to an else block
(TRY300)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Packages (3.10)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Test Packages (3.11)
- GitHub Check: Lint
- GitHub Check: Build Packages (3.11)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed 02f4b6a in 5 minutes and 6 seconds. Click for details.
- Reviewed
115lines of code in3files - Skipped
0files when reviewing. - Skipped posting
4draft 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.
1. packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py:109
- Draft comment:
The token_histogram parameter is passed into the wrapper constructors but is never used. If token usage metrics are intended as mentioned in the PR description, consider recording those metrics here; otherwise, remove the unused parameter to avoid confusion. - Reason this comment was not posted:
Comment was on unchanged code.
2. packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py:150
- Draft comment:
For duration measurement, consider using a monotonic timer (e.g., time.perf_counter() or time.monotonic()) instead of time.time() to prevent skew from system clock adjustments. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py:240
- Draft comment:
Similarly, in the async wrapper, using time.time() for duration measurement may be affected by clock changes. Consider switching to a monotonic timer such as time.perf_counter(). - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. packages/opentelemetry-instrumentation-agno/tests/conftest.py:68
- Draft comment:
The vcr_config fixture now filters additional sensitive headers and post data parameters. This update improves security by redacting data, so ensure these filters cover all sensitive fields for your use cases. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_rYDWuW5tJf3X7f6F
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Fix CI failures by properly separating dev and test dependencies: - Move dev-only dependencies (autopep8, flake8, pytest, pytest-sugar) to [tool.poetry.group.dev.dependencies] - Move test-specific dependencies (agno, openai, vcrpy, etc.) to [tool.poetry.group.test.dependencies] - Update .cz.toml to track agno package version This matches the pattern used by all other instrumentation packages and ensures dependencies are properly installed in CI environments. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed f0a7dc6 in 1 minute and 41 seconds. Click for details.
- Reviewed
44lines of code in2files - Skipped
1files when reviewing. - Skipped posting
2draft 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.
1. .cz.toml:10
- Draft comment:
Added version file entries for agno instrumentation look correct. Please ensure the referenced 'version.py' file exists and is correctly updated on version bumps. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/opentelemetry-instrumentation-agno/pyproject.toml:29
- Draft comment:
Dependency versions in dev and test groups have been lowered (e.g. autopep8, flake8, pytest, etc.). Confirm that these downgrades align with project-wide version consistency. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_BGUYsbEfgtrHItUQ
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/opentelemetry-instrumentation-agno/pyproject.toml (1)
43-43: Consider pinning the local instrumentation dependency version.The
opentelemetry-instrumentation-openaiis declared as a local path withdevelop = true, which pulls the development version. For reproducibility and stability, consider either pinning to a specific version or verifying this is intended for active concurrent development.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
packages/opentelemetry-instrumentation-agno/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
.cz.toml(1 hunks)packages/opentelemetry-instrumentation-agno/pyproject.toml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .cz.toml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: traceloop/openllmetry PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-17T15:06:48.109Z
Learning: Instrumentation packages must leverage the semantic conventions package and emit OTel-compliant spans
📚 Learning: 2025-08-22T14:41:26.962Z
Learnt from: prane-eth
Repo: traceloop/openllmetry PR: 3336
File: packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/utils.py:8-8
Timestamp: 2025-08-22T14:41:26.962Z
Learning: In the openllmetry project, the `packaging` library is available as a transitive dependency through other packages (visible in poetry.lock) and doesn't need to be explicitly declared in pyproject.toml dependencies.
Applied to files:
packages/opentelemetry-instrumentation-agno/pyproject.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Packages (3.11)
- GitHub Check: Test Packages (3.11)
- GitHub Check: Test Packages (3.10)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Lint
🔇 Additional comments (2)
packages/opentelemetry-instrumentation-agno/pyproject.toml (2)
18-19: Namespace packaging and plugin registration configured correctly.The namespace packaging approach (lines 18–19) properly enables distributed instrumentation under the
opentelemetry.instrumentationnamespace, and the plugin registration (lines 52–53) correctly maps the entry point to theAgnoInstrumentorclass, aligning with OpenTelemetry conventions.Also applies to: 52-53
24-25: Pre-release constraints are necessary and appropriate. Web searches confirm that both opentelemetry-instrumentation and opentelemetry-semantic-conventions have no stable releases on PyPI; 0.59b0 is the latest available version for both packages. The specified constraints (>=0.50b0 and >=0.55b0) are necessarily pre-release and allow for newer pre-release versions, which is the only option available.
| [tool.poetry.group.dev.dependencies] | ||
| autopep8 = "^2.2.0" | ||
| flake8 = "7.0.0" | ||
| pytest = "^8.2.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Remove duplicate pytest dependency.
The pytest package is declared in both dev.dependencies (line 31) and test.dependencies (line 37). Consolidate to avoid redundancy and reduce maintenance burden.
Apply this diff to remove the duplicate from dev dependencies:
[tool.poetry.group.dev.dependencies]
autopep8 = "^2.2.0"
flake8 = "7.0.0"
-pytest = "^8.2.2"
pytest-sugar = "1.0.0"Also applies to: 37-37
🤖 Prompt for AI Agents
In packages/opentelemetry-instrumentation-agno/pyproject.toml around lines 31
and 37, pytest is declared twice (in dev.dependencies and test.dependencies);
remove the duplicate entry from dev.dependencies (line 31) so pytest remains
declared only under test.dependencies (line 37), and ensure the file remains
valid TOML (keep a single pytest entry with the desired version constraint).
Switch from generic nx:run-commands to @nxlv/python executors: - Use @nxlv/python:flake8 for lint target (handles auto-install) - Use @nxlv/python:run-commands for test target - Add install, build, and other standard targets - Align with other instrumentation packages structure This ensures dependencies are automatically installed in CI before running lint and test commands. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/opentelemetry-instrumentation-agno/project.json (1)
46-54: Minor: Lint output filename uses "pylint" for flake8 executor.Line 49 names the output file
pylint.txt, but the executor (line 47) is@nxlv/python:flake8. Consider renaming toflake8.txtfor clarity, or confirm this naming convention is consistent across the project.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/opentelemetry-instrumentation-agno/project.json(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: traceloop/openllmetry PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-17T15:06:48.109Z
Learning: Instrumentation packages must leverage the semantic conventions package and emit OTel-compliant spans
📚 Learning: 2025-08-02T17:22:27.489Z
Learnt from: nirga
Repo: traceloop/openllmetry PR: 3217
File: packages/opentelemetry-instrumentation-haystack/project.json:0-0
Timestamp: 2025-08-02T17:22:27.489Z
Learning: In Nx monorepos, when targetDefaults are defined in nx.json with specific configurations like lintFilePatterns, individual project.json files can have empty target configurations (e.g., "lint": {}) that automatically inherit these defaults. This provides consistent behavior across all projects without needing to duplicate configuration in each project file.
Applied to files:
packages/opentelemetry-instrumentation-agno/project.json
📚 Learning: 2025-08-22T14:41:26.962Z
Learnt from: prane-eth
Repo: traceloop/openllmetry PR: 3336
File: packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/utils.py:8-8
Timestamp: 2025-08-22T14:41:26.962Z
Learning: In the openllmetry project, the `packaging` library is available as a transitive dependency through other packages (visible in poetry.lock) and doesn't need to be explicitly declared in pyproject.toml dependencies.
Applied to files:
packages/opentelemetry-instrumentation-agno/project.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Test Packages (3.10)
- GitHub Check: Test Packages (3.11)
- GitHub Check: Build Packages (3.11)
- GitHub Check: Lint
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed 646286a in 5 minutes and 54 seconds. Click for details.
- Reviewed
91lines of code in1files - Skipped
0files when reviewing. - Skipped posting
4draft 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.
1. packages/opentelemetry-instrumentation-agno/project.json:5
- Draft comment:
Verify that the updated sourceRoot path is correct. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. packages/opentelemetry-instrumentation-agno/project.json:10
- Draft comment:
Confirm that removing '--no-update' from the poetry lock command is intentional. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
3. packages/opentelemetry-instrumentation-agno/project.json:62
- Draft comment:
Ensure that explicitly running tests from 'tests/' is correct. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. packages/opentelemetry-instrumentation-agno/project.json:69
- Draft comment:
Check if using 'chmod +x' in the build-release commands is cross-platform safe. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_hjyhcJu2C0QYgSk5
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Update sourceRoot in project.json from non-existent 'opentelemetry_instrumentation_agno' to the actual namespace package path 'packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno' to match the repository layout used by other namespace-based instrumentors. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/opentelemetry-instrumentation-agno/project.json (1)
46-54: Consider inheriting lint defaults from nx.json for consistency.The explicit lint target configuration differs from the pattern used by similar instrumentor packages (per learnings), where empty
"lint": {}entries inherit fromnx.jsontargetDefaults to avoid duplication and ensure consistent behavior across the monorepo.If the lint configuration does not require package-specific overrides, consider simplifying to:
"lint": { - "executor": "@nxlv/python:flake8", - "outputs": [ - "{workspaceRoot}/reports/packages/opentelemetry-instrumentation-agno/pylint.txt" - ], - "options": { - "outputFile": "reports/packages/opentelemetry-instrumentation-agno/pylint.txt" - } + "executor": "@nxlv/python:flake8" }This allows the target to inherit flake8 patterns and output paths from monorepo-wide defaults. If package-specific linting rules are needed, retain the explicit configuration as-is.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/opentelemetry-instrumentation-agno/project.json(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: traceloop/openllmetry PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-17T15:06:48.109Z
Learning: Instrumentation packages must leverage the semantic conventions package and emit OTel-compliant spans
📚 Learning: 2025-08-02T17:22:27.489Z
Learnt from: nirga
Repo: traceloop/openllmetry PR: 3217
File: packages/opentelemetry-instrumentation-haystack/project.json:0-0
Timestamp: 2025-08-02T17:22:27.489Z
Learning: In Nx monorepos, when targetDefaults are defined in nx.json with specific configurations like lintFilePatterns, individual project.json files can have empty target configurations (e.g., "lint": {}) that automatically inherit these defaults. This provides consistent behavior across all projects without needing to duplicate configuration in each project file.
Applied to files:
packages/opentelemetry-instrumentation-agno/project.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Test Packages (3.11)
- GitHub Check: Build Packages (3.11)
- GitHub Check: Test Packages (3.10)
- GitHub Check: Lint
🔇 Additional comments (1)
packages/opentelemetry-instrumentation-agno/project.json (1)
5-5: ✅ SourceRoot corrected to match actual namespace package structure.Line 5 correctly specifies the namespace package path
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno, which aligns with the actual Python package structure and resolves the critical issue flagged in the previous review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed 3c1f5d7 in 1 minute and 51 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft 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.
1. packages/opentelemetry-instrumentation-agno/project.json:5
- Draft comment:
Update to the new sourceRoot is correct; please ensure the directory structure (and all relative paths in targets/tests) matches the new path 'opentelemetry/instrumentation/agno'. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_wWEIcgNqgrzqNBEt
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
galkleinman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/__init__.py
Outdated
Show resolved
Hide resolved
packages/opentelemetry-instrumentation-agno/opentelemetry/__init__.py
Outdated
Show resolved
Hide resolved
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| _instruments = ("agno >= 2.2.2",) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider relaxing this one.. i guess ">= 2.0.0" will work
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py
Show resolved
Hide resolved
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py
Show resolved
Hide resolved
| model_name = getattr(instance.model, 'id', | ||
| getattr(instance.model, 'name', 'unknown')) | ||
| span.set_attribute(GenAIAttributes.GEN_AI_REQUEST_MODEL, | ||
| model_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imo we shouldn't report rather than reporting "unknown"
| kind=SpanKind.CLIENT, | ||
| ) as span: | ||
| try: | ||
| span.set_attribute(GenAIAttributes.GEN_AI_SYSTEM, "agno") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const the agno?
in general maybe we should think of adding "well known systems" to the semconv
Changes:
- Update span names to follow pattern: {name}.agent, {name}.team, {name}.tool
- Add FunctionCall.execute() and FunctionCall.aexecute() instrumentation for tool calls
- Switch from gen_ai.prompt/completion attributes to traceloop.entity.input/output
- Add traceloop.entity.name for all span types
- Create new _tool_wrappers.py module for tool instrumentation
- Update tests to verify new span names and attributes
Span hierarchy now properly captures:
WeatherAssistant.agent [AGENT]
└─ get_weather.tool [TOOL]
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
…files - Bump version from 0.48.1 to 0.48.2 - Remove opentelemetry/__init__.py and opentelemetry/instrumentation/__init__.py - These files are not needed for namespace packages in Python 3.3+ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
♻️ Duplicate comments (2)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py (2)
140-147: dont_throw on agent/team wrappers still swallows user exceptions (critical)
_AgentRunWrapper.__call__,_AgentARunWrapper.__call__,_TeamRunWrapper.__call__, and_TeamARunWrapper.__call__are all decorated with@dont_throw. That decorator catches everyExceptionand does not re‑raise, so:
- Any error from
wrapped(*args, **kwargs)/await wrapped(...)is swallowed and turned into aNonereturn.- This happens even when instrumentation is suppressed (the
wrappedcall in the suppression branch is still inside the decorator).- The inner
try/exceptblocks that set span status and thenraiseare effectively neutralized.This is the same correctness issue that was called out in a previous review and is still present, now affecting all agent and team executions.
I recommend removing
@dont_throwfrom these methods and relying on the innertry/exceptto both update the span and re‑raise the error:- @dont_throw - def __call__(self, wrapped, instance, args, kwargs): + def __call__(self, wrapped, instance, args, kwargs): ... - @dont_throw - async def __call__(self, wrapped, instance, args, kwargs): + async def __call__(self, wrapped, instance, args, kwargs):If you need to prevent instrumentation bugs from breaking users, handle that with a narrow
try/exceptaround the attribute/metrics enrichment only, or by usingConfig.exception_logger, rather than swallowing the entire agent/team call.Also applies to: 150-217, 228-235, 238-305, 316-323, 326-372, 383-390, 393-438
59-69: Token usage histogram is created and threaded but never recorded
token_histogramis created in_instrumentand passed into all wrappers, but there are no calls toself._token_histogram.record(...)anywhere (including the tool wrappers). Only span attributes are set fromresult.metrics.This means:
- No token usage metrics are actually emitted, despite the presence of
Meters.LLM_TOKEN_USAGEand the constructor parameter.- The extra histogram and wrapper parameters add complexity without providing value.
Either:
Use it — when
Config.enrich_token_usageisTrueand token counts are available, record them to the histogram. For example in_AgentRunWrapper:if Config.enrich_token_usage and hasattr(metrics, 'input_tokens'): span.set_attribute( GenAIAttributes.GEN_AI_USAGE_INPUT_TOKENS, metrics.input_tokens, ) self._token_histogram.record( metrics.input_tokens, attributes={ GenAIAttributes.GEN_AI_SYSTEM: "agno", SpanAttributes.LLM_USAGE_TOKEN_TYPE: "input", }, )and similarly for
output_tokens/total_tokensand in the async wrapper.Or remove it — if you don't plan to expose a metric yet, drop the
token_histogramcreation and corresponding constructor arguments/fields to keep the API lean.Also applies to: 134-139, 222-227, 310-315, 377-382
🧹 Nitpick comments (2)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/_tool_wrappers.py (1)
17-21: token_histogram is passed into tool wrappers but never usedBoth tool wrappers accept and store
token_histogrambut never callself._token_histogram.record(...). This is slightly misleading given the PR mentions token usage metrics.Either start recording token counts here when such data becomes available, or drop the
token_histogramparameter/field for now to avoid confusion:- def __init__(self, tracer, duration_histogram, token_histogram): + def __init__(self, tracer, duration_histogram, token_histogram): """Initialize the wrapper with OpenTelemetry instrumentation objects.""" self._tracer = tracer self._duration_histogram = duration_histogram - self._token_histogram = token_histogram + # self._token_histogram is currently unused; consider wiring it when token + # data is available, or remove this parameter if you don't plan to emit a + # metric for tool token usage yet.Also applies to: 86-90
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py (1)
116-123: Minor: unused_uninstrumentkwargs and silent unwrap failures
_uninstrumentaccepts**kwargsbut doesn't use them, and the twotryblocks swallow all exceptions withpass. This is fine functionally but a bit noisy for linters and makes debugging uninstrumentation failures harder.Consider:
- Renaming the parameter to
**_kwargsto signal intentional non‑use.- Logging unwrap failures at
debuginstead of bareexcept Exception: pass, similar to the_instrumentside.For example:
- def _uninstrument(self, **kwargs): + def _uninstrument(self, **_kwargs): - try: - unwrap("agno.team", "Team.run") - unwrap("agno.team", "Team.arun") - except Exception: - pass + try: + unwrap("agno.team", "Team.run") + unwrap("agno.team", "Team.arun") + except Exception as exc: + logger.debug("Could not uninstrument Team: %s", exc)Same idea for the
FunctionCallunwrap block.Also applies to: 124-128
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py(1 hunks)packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/_tool_wrappers.py(1 hunks)packages/opentelemetry-instrumentation-agno/tests/test_agent.py(1 hunks)packages/opentelemetry-instrumentation-agno/tests/test_team.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/_tool_wrappers.pypackages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.pypackages/opentelemetry-instrumentation-agno/tests/test_agent.pypackages/opentelemetry-instrumentation-agno/tests/test_team.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: traceloop/openllmetry PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-17T15:06:48.109Z
Learning: Instrumentation packages must leverage the semantic conventions package and emit OTel-compliant spans
🧬 Code graph analysis (2)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/_tool_wrappers.py (1)
packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py (2)
SpanAttributes(64-245)TraceloopSpanKindValues(285-290)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py (3)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/config.py (1)
Config(1-14)packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/utils.py (3)
dont_throw(8-15)should_send_prompts(31-34)wrapper(10-14)packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py (3)
Meters(36-61)SpanAttributes(64-245)TraceloopSpanKindValues(285-290)
🪛 Ruff (0.14.5)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/_tool_wrappers.py
75-75: Consider moving this statement to an else block
(TRY300)
144-144: Consider moving this statement to an else block
(TRY300)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py
97-97: Do not catch blind exception: Exception
(BLE001)
113-113: Do not catch blind exception: Exception
(BLE001)
116-116: Unused method argument: kwargs
(ARG002)
122-123: try-except-pass detected, consider logging the exception
(S110)
122-122: Do not catch blind exception: Exception
(BLE001)
127-128: try-except-pass detected, consider logging the exception
(S110)
127-127: Do not catch blind exception: Exception
(BLE001)
211-211: Consider moving this statement to an else block
(TRY300)
299-299: Consider moving this statement to an else block
(TRY300)
366-366: Consider moving this statement to an else block
(TRY300)
433-433: Consider moving this statement to an else block
(TRY300)
packages/opentelemetry-instrumentation-agno/tests/test_agent.py
11-11: Unused function argument: instrument
(ARG001)
11-11: Unused function argument: reader
(ARG001)
39-39: Unused function argument: instrument
(ARG001)
39-39: Unused function argument: reader
(ARG001)
62-62: Unused function argument: instrument
(ARG001)
62-62: Unused function argument: reader
(ARG001)
95-95: Unused function argument: instrument
(ARG001)
packages/opentelemetry-instrumentation-agno/tests/test_team.py
12-12: Unused function argument: instrument
(ARG001)
12-12: Unused function argument: reader
(ARG001)
57-57: Unused function argument: instrument
(ARG001)
57-57: Unused function argument: reader
(ARG001)
| def __init__(self, exception_logger=None, enrich_token_usage: bool = False): | ||
| super().__init__() | ||
| Config.exception_logger = exception_logger | ||
| Config.enrich_token_usage = enrich_token_usage | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enrich_token_usage flag is never read; token usage enrichment is always on
AgnoInstrumentor.__init__ sets Config.enrich_token_usage based on the constructor argument, but the wrappers always emit token usage attributes whenever result.metrics is present:
_AgentRunWrapper.__call__(Lines 186‑200)_AgentARunWrapper.__call__(Lines 274‑288)
There is no check of Config.enrich_token_usage, so callers cannot actually disable token usage enrichment even if they pass enrich_token_usage=False. That contradicts the Config docs and the PR description, and can be a concern where token counts are considered sensitive.
A minimal fix is to gate the metrics block on the config flag, e.g.:
- if hasattr(result, 'metrics'):
+ if Config.enrich_token_usage and hasattr(result, 'metrics'):
# same logic for metrics.input_tokens / output_tokens / total_tokensApply the same guard in both sync and async agent wrappers (and anywhere else you expose token data).
Also applies to: 186-200, 274-288
🤖 Prompt for AI Agents
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py
lines 44-48 (and wrappers at 186-200 and 274-288): the constructor sets
Config.enrich_token_usage but the wrapper code always emits token usage
attributes when result.metrics exists; update the wrappers to respect the flag
by guarding the token/metrics emission with if Config.enrich_token_usage: ... so
token metrics are only added when the config is True (apply the same guard in
both sync and async agent wrappers and any other places that expose token
counts).
| @dont_throw | ||
| def __call__(self, wrapped, instance, args, kwargs): | ||
| """Wrap the FunctionCall.execute() call with tracing instrumentation.""" | ||
| if context_api.get_value( | ||
| context_api._SUPPRESS_INSTRUMENTATION_KEY | ||
| ) or context_api.get_value("suppress_agno_instrumentation"): | ||
| return wrapped(*args, **kwargs) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dont_throw on tool wrappers swallows FunctionCall exceptions (critical)
dont_throw catches all Exception and does not re‑raise, so any error from wrapped(*args, **kwargs) (including those you explicitly raise in the inner except) is swallowed. That changes FunctionCall.execute/aexecute semantics: callers will see None instead of the original exception, even when instrumentation is suppressed via the context keys.
I strongly recommend removing @dont_throw from these wrappers and letting the inner try/except mark span status and re‑raise the error:
- @dont_throw
- def __call__(self, wrapped, instance, args, kwargs):
+ def __call__(self, wrapped, instance, args, kwargs):
...
- @dont_throw
- async def __call__(self, wrapped, instance, args, kwargs):
+ async def __call__(self, wrapped, instance, args, kwargs):If you still want to protect against instrumentation‑internal failures, wrap only the attribute/metrics logic in a local try/except instead of using dont_throw around the entire call.
Also applies to: 34-80, 92-99, 103-149
🤖 Prompt for AI Agents
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/_tool_wrappers.py
lines 23-30 (and also apply same change to ranges 34-80, 92-99, 103-149): the
dont_throw decorator around the wrapper methods swallows exceptions from
wrapped(*args, **kwargs) and changes FunctionCall.execute/aexecute semantics;
remove the @dont_throw decorator from these wrapper methods so exceptions from
the wrapped call propagate, and if protection against instrumentation internal
errors is desired, replace the broad decorator with a narrow try/except that
only surrounds span attribute/metric setting and status updates (log the
instrumentation error but re-raise or avoid catching the wrapped call), ensuring
the original exceptions from wrapped are not swallowed.
| @pytest.mark.vcr | ||
| def test_agent_run_basic(instrument, span_exporter, reader): | ||
| """Test basic agent.run() instrumentation.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mark unused instrument / reader fixtures as intentionally unused
Several tests accept instrument and/or reader only to activate the fixtures but never use the variables, which Ruff flags as ARG001:
test_agent_run_basictest_agent_arun_basictest_agent_with_toolstest_agent_metrics(instrumentonly)
To preserve the fixture side effects and keep linters happy, consider:
- def test_agent_run_basic(instrument, span_exporter, reader):
+ def test_agent_run_basic(_instrument, span_exporter, _reader):
- async def test_agent_arun_basic(instrument, span_exporter, reader):
+ async def test_agent_arun_basic(_instrument, span_exporter, _reader):
- def test_agent_with_tools(instrument, span_exporter, reader):
+ def test_agent_with_tools(_instrument, span_exporter, _reader):
- def test_agent_metrics(instrument, span_exporter, reader):
+ def test_agent_metrics(_instrument, span_exporter, reader):This keeps the behavior while eliminating the lint warnings.
Also applies to: 37-39, 61-63, 94-96
🧰 Tools
🪛 Ruff (0.14.5)
11-11: Unused function argument: instrument
(ARG001)
11-11: Unused function argument: reader
(ARG001)
🤖 Prompt for AI Agents
In packages/opentelemetry-instrumentation-agno/tests/test_agent.py around lines
10-12 (and also apply the same change at 37-39, 61-63, and 94-96), the test
functions accept fixtures like instrument and reader only for side effects and
Ruff flags them as unused; rename those parameters to start with an underscore
(e.g., _instrument, _reader) so the fixtures still run but linters recognize
them as intentionally unused.
| @pytest.mark.vcr | ||
| def test_team_discussion(instrument, span_exporter, reader): | ||
| """Test team with multiple agents having a discussion.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mark unused instrument / reader fixtures to satisfy linters
In both tests the instrument and reader fixtures are only used for their side effects, not as variables, so Ruff/Flake8 will flag them as unused (ARG001).
To keep the fixtures while silencing lint errors, consider:
- Prefixing them with
_to signal intentional non‑use, or - Adding a
# noqa: ARG001comment on the function definition.
For example:
- def test_team_discussion(instrument, span_exporter, reader):
+ def test_team_discussion(_instrument, span_exporter, _reader):Same idea for test_team_basic.
Also applies to: 56-58
🧰 Tools
🪛 Ruff (0.14.5)
12-12: Unused function argument: instrument
(ARG001)
12-12: Unused function argument: reader
(ARG001)
🤖 Prompt for AI Agents
In packages/opentelemetry-instrumentation-agno/tests/test_team.py around lines
11-13 (and similarly at 56-58), the test functions accept fixtures `instrument`
and `reader` only for side effects which linters flag as unused; fix by either
renaming those parameters to `_instrument` and `_reader` to indicate intentional
non‑use, or append a `# noqa: ARG001` to the function definition line to
suppress the ARG001 warning—apply the same change to both test_team_discussion
and test_team_basic at the noted locations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skipped PR review on 7fe5d15 because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (3)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py (3)
59-69: Token usage histogram is created and passed around but never recordedThe
token_histogramis created and threaded through all wrappers’ constructors, but none of the agent/team wrappers in this file (and the tool wrappers in_tool_wrappers.py) ever callself._token_histogram.record(...). This means:
- No
Meters.LLM_TOKEN_USAGEmetrics are actually emitted.- The PR’s stated goal of capturing token usage metrics isn’t met; token data is only exposed as span attributes.
You have two clear options:
- Use it: record token counts into
token_histogramwhere you already readresult.metrics(and in tool wrappers, if/when token counts are available).- Remove it: if you decide to expose tokens only as span attributes for now, delete the histogram creation and the
token_histogramparameters/fields to reduce confusion.Until one of these is done, this histogram is effectively dead weight.
Also applies to: 138-143, 234-239, 330-335
196-211:Config.enrich_token_usageflag is ignored; token usage is always emittedThe agent wrappers always emit token usage attributes whenever
result.metricsexists:
GEN_AI_USAGE_INPUT_TOKENSGEN_AI_USAGE_OUTPUT_TOKENSLLM_USAGE_TOTAL_TOKENSThere is no check of
Config.enrich_token_usage, so callers cannot disable token enrichment even if they constructAgnoInstrumentor(enrich_token_usage=False).To make the config effective (and to honor privacy/sensitivity expectations), gate the metrics block on the flag, e.g.:
- if hasattr(result, "metrics"): + if Config.enrich_token_usage and hasattr(result, "metrics"): metrics = result.metrics if hasattr(metrics, "input_tokens"): span.set_attribute( GenAIAttributes.GEN_AI_USAGE_INPUT_TOKENS, metrics.input_tokens, ) if hasattr(metrics, "output_tokens"): span.set_attribute( GenAIAttributes.GEN_AI_USAGE_OUTPUT_TOKENS, metrics.output_tokens, ) if hasattr(metrics, "total_tokens"): span.set_attribute( SpanAttributes.LLM_USAGE_TOTAL_TOKENS, metrics.total_tokens, )and apply the same guard in the async wrapper.
This restores the intended behavior of the config flag.
Also applies to: 292-307
144-151:dont_throwdecorator swallows real Agno exceptions and changes behaviorAll four wrappers (
_AgentRunWrapper.__call__,_AgentARunWrapper.__call__,_TeamRunWrapper.__call__,_TeamARunWrapper.__call__) are decorated with@dont_throw. Given the implementation ofdont_throw:def dont_throw(func): def wrapper(*args, **kwargs): try: return func(*args, **kwargs) except Exception as e: logger.debug(f"Error in {func.__name__}: {e}") return wrapperthe control flow is:
- Any exception from
wrapped(*args, **kwargs)is caught by the innertry/exceptin the wrapper, which callsspan.set_status(...)/span.record_exception(e)and thenraise.- That re‑raised exception is then caught again by
dont_throw, logged at debug, and suppressed (no re‑raise).Net effect: exceptions from
Agent.run,Agent.arun,Team.run, andTeam.arunnever reach the caller when these wrappers are active — they are silently converted intoNonereturns. This is a correctness break and contradicts the intent of the innertry/exceptblocks that clearly expect to re‑raise.Recommended fix: remove
@dont_throwfrom these wrappers and let the innertry/exceptmanage span status while propagating errors:-class _AgentRunWrapper: +class _AgentRunWrapper: @@ - @dont_throw - def __call__(self, wrapped, instance, args, kwargs): + def __call__(self, wrapped, instance, args, kwargs): ... @@ -class _AgentARunWrapper: +class _AgentARunWrapper: @@ - @dont_throw - async def __call__(self, wrapped, instance, args, kwargs): + async def __call__(self, wrapped, instance, args, kwargs): ... @@ -class _TeamRunWrapper: +class _TeamRunWrapper: @@ - @dont_throw - def __call__(self, wrapped, instance, args, kwargs): + def __call__(self, wrapped, instance, args, kwargs): ... @@ -class _TeamARunWrapper: +class _TeamARunWrapper: @@ - @dont_throw - async def __call__(self, wrapped, instance, args, kwargs): + async def __call__(self, wrapped, instance, args, kwargs): ...If you still want protection against instrumentation‑internal failures, you can narrow
dont_throwusage to purely instrumentation code paths (excluding the actual Agno call), but it must not wrap the entire wrapper function like this.Note: the same pattern exists on the tool wrappers in
_tool_wrappers.py; it’s worth applying the same reasoning there.Also applies to: 240-247, 336-343, 409-416
🧹 Nitpick comments (9)
packages/sample-app/sample_app/agno_example.py (2)
1-12: Avoid side effects at import time for Traceloop.init in a reusable sampleInitializing Traceloop at module import can be surprising if this file is ever imported rather than run as a script. Consider moving
Traceloop.init(...)under theif __name__ == "__main__":guard (or intorun_agent) so initialization happens explicitly at execution time.Example refactor:
-from traceloop.sdk import Traceloop -from traceloop.sdk.decorators import workflow +from traceloop.sdk import Traceloop +from traceloop.sdk.decorators import workflow @@ -load_dotenv() - -Traceloop.init(app_name="agno_example") +load_dotenv() @@ -if __name__ == "__main__": - run_agent() +if __name__ == "__main__": + Traceloop.init(app_name="agno_example") + run_agent()Please double-check in the wider codebase that calling
Traceloop.initfrom__main__(potentially more than once across samples) is acceptable per the SDK’s initialization semantics.Also applies to: 42-43
19-39: Refine agent tracing decorator and OPENAI_API_KEY handlingTwo small improvements to consider in
run_agent:
Use the
agentdecorator for clearer semantics
Since this is conceptually an agent entrypoint, thetraceloop.sdk.decorators.agenthelper (which wrapsworkflowwithtlp_span_kind=AGENT) is more explicit than@workflow(...). This keeps span semantics aligned with the Traceloop SDK’s agent notion.
- from traceloop.sdk.decorators import workflow
- from traceloop.sdk.decorators import agent
@@
-@workflow(name="agno_agent_example")
+@agent(name="agno_agent_example")
def run_agent():
...
Fail fast if
OPENAI_API_KEYis missing
To avoid confusing runtime errors when running the sample, you could explicitly check that the env var is set before constructingOpenAIChat:
- agent = Agent(
- api_key = os.environ.get("OPENAI_API_KEY")
- if not api_key:
raise RuntimeError("OPENAI_API_KEY environment variable is not set")- agent = Agent(
name="WeatherAssistant",
model=OpenAIChat(
id="gpt-4o-mini",
api_key=os.environ.get("OPENAI_API_KEY"),
api_key=api_key, ),Please verify against the current Traceloop SDK docs that
agentis the preferred decorator for this use case and that repeated runs of this sample with the added API‑key check won’t conflict with your Agno instrumentation story.packages/sample-app/sample_app/agno_discussion_team.py (1)
98-105: Consider adding error handling for better user experience.The
team.run()call could fail for various reasons (API errors, network issues, rate limits). Adding a try-except block would provide clearer error messages for users running this sample app.print("Starting expert discussion team...\n") - result = discussion_team.run( - "Discuss the topic: 'Should we adopt AI-powered code review tools in our development workflow?'" - ) - print(f"\n{'='*80}\nFinal Team Consensus:\n{'='*80}") - print(result.content) - - return result + try: + result = discussion_team.run( + "Discuss the topic: 'Should we adopt AI-powered code review tools in our development workflow?'" + ) + print(f"\n{'='*80}\nFinal Team Consensus:\n{'='*80}") + print(result.content) + return result + except Exception as e: + print(f"\nError during team discussion: {e}") + raisepackages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (3)
92-95: Tracer provider extraction looks correct; consider handling the “unsupported provider” caseThe new
init_tracer_provider()helper preserves the existing logic: it replaces the proxy provider when needed and reuses an existing concrete provider otherwise. One edge case remains: whendefault_providerlacksadd_span_processor, the function logs an error and returnsNone, which would later breakget_tracer()onself.__tracer_provider.If you care about that scenario, consider marking
TracerWrapperas disabled or raising a clearer exception instead of returningNone.Also applies to: 422-443
453-463: Instrumentation wiring: AGNO path is correct, but some flags/args are unused
- Adding
Instruments.AGNOtoinit_instrumentationsand delegating toinit_agno_instrumentor()is consistent with the other instrument branches.- Defaulting
instrumentstoset(Instruments)whenNonemaintains the existing behavior of “all instruments” if the caller doesn’t specify a set.There are a couple of follow‑ups worth addressing:
Unused
should_enrich_metricsparameters (Ruff ARG001)
init_google_generativeai_instrumentor(should_enrich_metrics, base64_image_uploader, ...)andinit_vertexai_instrumentor(should_enrich_metrics, base64_image_uploader, ...)acceptshould_enrich_metricsbut don’t use it internally. This is flagged by Ruff and can be confusing to readers.Consider either:
- Actually wiring
should_enrich_metricsinto the respective instrumentors (if they support token usage / metrics enrichment), or- Dropping the argument (or renaming to
_should_enrich_metrics) until it is used.AGNO metrics enrichment flag isn’t plumbed through
init_agno_instrumentor()always constructsAgnoInstrumentorwith onlyexception_logger=..., so theenrich_token_usageflag onAgnoInstrumentoris never influenced byshould_enrich_metricsininit_instrumentations. Combined with the Agno config, this means SDK users can’t control token usage enrichment via the global “should_enrich_metrics” knob.Consider updating the signature to accept
should_enrich_metricsand passing it through:-def init_agno_instrumentor(): +def init_agno_instrumentor(should_enrich_metrics: bool): try: if is_package_installed("agno"): Telemetry().capture("instrumentation:agno:init") from opentelemetry.instrumentation.agno import AgnoInstrumentor instrumentor = AgnoInstrumentor(
exception_logger=lambda e: Telemetry().log_exception(e),
exception_logger=lambda e: Telemetry().log_exception(e),enrich_token_usage=should_enrich_metrics, )and adjust the `init_instrumentations` call site accordingly.These changes would clear up the Ruff warnings and ensure the global metrics enrichment flag has the intended effect across all instrumentors, including AGNO.
Please verify in the opentelemetry instrumentor APIs that the enrichment‑related constructor arguments you intend to pass (for Gemini/Vertex/Agno) are supported in the exact versions you depend on before wiring them through.
Also applies to: 485-487, 555-556, 587-590, 613-616, 710-713, 715-717, 977-979, 1054-1069
1054-1069: AGNO instrumentor init follows existing patterns but exception handling mirrors prior choices
init_agno_instrumentor()matches the pattern used for the other LLM/vector instrumentors: it gates on package installation, emits a telemetry event, imports lazily, and guardsinstrument()behindis_instrumented_by_opentelemetry. That aligns well with the rest of the file.The broad
except Exceptionandlogging.errorusage are consistent with otherinit_*_instrumentorhelpers here, so while Ruff complains about BLE001/TRY400, changing just this one would make it inconsistent with the rest. If you decide to tighten this, consider doing it uniformly across all initializers.packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py (3)
31-38: Instrumentation dependency string and version helper
_instruments = ("agno >= 2.0.0",)is a sensible minimum constraint and matches the prior suggestion to relax to>=._get_agno_version()is fine as a helper, though it appears unused in this file; if it’s not consumed elsewhere, you could drop it in a follow‑up to avoid dead code.
44-47: Config fields are set but not used by the wrappers
AgnoInstrumentor.__init__updatesConfig.exception_loggerandConfig.enrich_token_usage, but none of the wrappers in this module (nor the tool wrappers) read these values. As a result:
- Passing
exception_loggerhas no effect.- The
enrich_token_usageflag doesn’t control whether token usage is emitted.Either wire these config values into the wrappers (e.g., call
Config.exception_loggerinside error paths and gate token attributes/metrics onConfig.enrich_token_usage) or remove the unused config fields to avoid misleading API surface.
120-132:_uninstrumentswallows uninstrument errors and has an unusedkwargsargThe
_uninstrumentmethod ignores itskwargsparameter and usestry/except: passblocks when unwrapping team and tool methods. That’s consistent with a “best‑effort” uninstrumentation approach but:
- The unused
kwargsis flagged by Ruff (ARG002).- Silent
except: passmakes debugging uninstrumentation issues harder.You might consider:
- def _uninstrument(self, **kwargs): + def _uninstrument(self, **_kwargs): @@ - except Exception: - pass + except Exception as e: + logger.debug(f"Could not uninstrument Team: {e}") @@ - except Exception: - pass + except Exception as e: + logger.debug(f"Could not uninstrument FunctionCall: {e}")This keeps failures non‑fatal but at least surfaces them at debug level and satisfies the static analysis warning on the unused arg.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
packages/opentelemetry-instrumentation-agno/poetry.lockis excluded by!**/*.lockpackages/sample-app/poetry.lockis excluded by!**/*.lockpackages/traceloop-sdk/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
.cz.toml(1 hunks)packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py(1 hunks)packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/version.py(1 hunks)packages/opentelemetry-instrumentation-agno/pyproject.toml(1 hunks)packages/sample-app/sample_app/agno_discussion_team.py(1 hunks)packages/sample-app/sample_app/agno_example.py(1 hunks)packages/sample-app/sample_app/agno_team_example.py(1 hunks)packages/traceloop-sdk/pyproject.toml(1 hunks)packages/traceloop-sdk/traceloop/sdk/instruments.py(1 hunks)packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py(18 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/sample-app/sample_app/agno_team_example.py
- packages/opentelemetry-instrumentation-agno/pyproject.toml
- .cz.toml
- packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/version.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/traceloop-sdk/traceloop/sdk/tracing/tracing.pypackages/sample-app/sample_app/agno_discussion_team.pypackages/traceloop-sdk/traceloop/sdk/instruments.pypackages/sample-app/sample_app/agno_example.pypackages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: traceloop/openllmetry PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-17T15:06:48.109Z
Learning: Instrumentation packages must leverage the semantic conventions package and emit OTel-compliant spans
📚 Learning: 2025-08-22T14:41:26.962Z
Learnt from: prane-eth
Repo: traceloop/openllmetry PR: 3336
File: packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/utils.py:8-8
Timestamp: 2025-08-22T14:41:26.962Z
Learning: In the openllmetry project, the `packaging` library is available as a transitive dependency through other packages (visible in poetry.lock) and doesn't need to be explicitly declared in pyproject.toml dependencies.
Applied to files:
packages/traceloop-sdk/pyproject.toml
🧬 Code graph analysis (3)
packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (4)
packages/traceloop-sdk/traceloop/sdk/telemetry.py (3)
Telemetry(12-94)capture(66-73)log_exception(75-86)packages/traceloop-sdk/traceloop/sdk/instruments.py (1)
Instruments(4-39)packages/traceloop-sdk/traceloop/sdk/utils/package_check.py (1)
is_package_installed(14-15)packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py (1)
AgnoInstrumentor(41-132)
packages/sample-app/sample_app/agno_example.py (2)
packages/traceloop-sdk/traceloop/sdk/decorators/__init__.py (1)
agent(50-60)packages/traceloop-sdk/traceloop/sdk/__init__.py (2)
Traceloop(37-275)init(49-206)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py (5)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/_tool_wrappers.py (2)
_FunctionCallExecuteWrapper(14-80)_FunctionCallAExecuteWrapper(83-149)packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/config.py (1)
Config(1-14)packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/utils.py (3)
dont_throw(8-15)should_send_prompts(31-34)wrapper(10-14)packages/opentelemetry-semantic-conventions-ai/opentelemetry/semconv_ai/__init__.py (3)
Meters(36-61)SpanAttributes(64-245)TraceloopSpanKindValues(285-290)packages/opentelemetry-instrumentation-milvus/tests/conftest.py (1)
meter_provider(45-50)
🪛 Ruff (0.14.5)
packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py
711-711: Unused function argument: should_enrich_metrics
(ARG001)
977-977: Unused function argument: should_enrich_metrics
(ARG001)
1066-1066: Do not catch blind exception: Exception
(BLE001)
1067-1067: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py
97-97: Do not catch blind exception: Exception
(BLE001)
117-117: Do not catch blind exception: Exception
(BLE001)
120-120: Unused method argument: kwargs
(ARG002)
126-127: try-except-pass detected, consider logging the exception
(S110)
126-126: Do not catch blind exception: Exception
(BLE001)
131-132: try-except-pass detected, consider logging the exception
(S110)
131-131: Do not catch blind exception: Exception
(BLE001)
223-223: Consider moving this statement to an else block
(TRY300)
319-319: Consider moving this statement to an else block
(TRY300)
392-392: Consider moving this statement to an else block
(TRY300)
465-465: Consider moving this statement to an else block
(TRY300)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Packages (3.10)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Test Packages (3.11)
- GitHub Check: Build Packages (3.11)
- GitHub Check: Lint
🔇 Additional comments (13)
packages/sample-app/sample_app/agno_example.py (2)
14-16: Weather tool stub looks good for a simple sample
get_weatheris a clear, side‑effect‑free tool function and correctly uses thelocationargument in the returned string. No changes needed here from a correctness/style perspective.
42-43: Main guard pattern is correct and aligns with sample‑app usageUsing
if __name__ == "__main__":to trigger the example run is appropriate and keeps the module import‑safe. Once you decide where to putTraceloop.init, this structure will support it cleanly.packages/sample-app/sample_app/agno_discussion_team.py (3)
1-10: LGTM!The imports are well-organized and
load_dotenv()is properly called to support local development with environment variables.
13-13: LGTM!Traceloop initialization with a descriptive app name is appropriate for demonstrating the Agno instrumentation capabilities.
108-109: LGTM!The main entry point follows standard Python conventions for executable modules.
packages/traceloop-sdk/traceloop/sdk/instruments.py (1)
5-5: AGNO enum entry is consistent with existing instrumentsName and value follow the established pattern and align with the new AGNO instrumentation path. No issues here.
packages/traceloop-sdk/pyproject.toml (1)
39-39: Local AGNO instrumentation dependency matches existing patternThe new
opentelemetry-instrumentation-agnopath/develop dependency is consistent with the other instrumentation packages wired into the SDK. Looks good.packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (5)
38-40: UsingGEN_AI_AGENT_NAMEaligns with GenAI semantic conventionsWiring the agent name through
GEN_AI_AGENT_NAMEindefault_span_processor_on_startis a good move toward semconv alignment and keeps agent labeling consistent across frameworks.Please double‑check that
GEN_AI_AGENT_NAMEand its import path match the exact version ofopentelemetry-semconvyou depend on, as incubating modules can change between releases.Also applies to: 331-333
152-154: Custom exporter plumbing and default span processor composition are soundAllowing
get_default_span_processorto accept an explicitexporterand wiring it through fromTracerWrapper.__new__is consistent with the existing design and doesn’t change behavior for callers that don’t pass a custom exporter. The default processor is still created with the same batch/simple decision and wired todefault_span_processor_on_start.Also applies to: 410-419
242-246:flush()now correctly supports single and multiple span processorsThe updated
flush()implementation handles both the single__spans_processorand multiple__spans_processorscases, which matches the initialization branches above and avoids missing flushes when a list of processors is provided.
379-381: Additional type guard onprompt_template_variablesThe extra
isinstance(prompt_template_variables, dict)check prevents iterating over non‑mapping values and is consistent with the pattern used forassociation_properties. Looks good.
672-676: Qdrant client name handling is robustChecking for both
qdrant_clientandqdrant-clientpackage names makes the Qdrant detection resilient to install‑name differences. This is a nice improvement.packages/opentelemetry-instrumentation-agno/opentelemetry/instrumentation/agno/__init__.py (1)
152-195: Span construction and attribute mapping for agents/teams look semantically correctWithin each wrapper, the span setup itself is solid:
- Uses
SpanKind.CLIENTand setsGenAIAttributes.GEN_AI_SYSTEM = "agno".- Assigns
TRACELOOP_SPAN_KINDappropriately (AGENTfor agents,WORKFLOWfor teams).- Uses
GEN_AI_AGENT_NAMEfor agent names and a namespaced"agno.team.name"for teams.- Gathers model id/name, input and output content (guarded by
should_send_prompts()), andrun_idwhere present.- Records duration into
Meters.LLM_OPERATION_DURATIONwith consistent attributes.Once the
dont_throwissue is fixed, this span content should align well with your existing semantic conventions.Also applies to: 248-291, 344-381, 417-454
| technical_expert = Agent( | ||
| name="Technical Expert", | ||
| role="Provide technical perspective and implementation details", | ||
| model=OpenAIChat( | ||
| id="gpt-4o-mini", | ||
| api_key=os.environ.get("OPENAI_API_KEY"), | ||
| ), | ||
| add_name_to_context=True, | ||
| instructions=dedent( | ||
| """ | ||
| You are a technical expert focused on implementation details. | ||
| Analyze topics from a technical feasibility perspective. | ||
| Discuss technical challenges, solutions, and best practices. | ||
| Keep responses concise and focused. | ||
| """ | ||
| ), | ||
| ) | ||
|
|
||
| business_expert = Agent( | ||
| name="Business Expert", | ||
| role="Provide business perspective and practical implications", | ||
| model=OpenAIChat( | ||
| id="gpt-4o-mini", | ||
| api_key=os.environ.get("OPENAI_API_KEY"), | ||
| ), | ||
| add_name_to_context=True, | ||
| instructions=dedent( | ||
| """ | ||
| You are a business expert focused on practical value. | ||
| Analyze topics from a business impact and ROI perspective. | ||
| Discuss market implications, costs, and business benefits. | ||
| Keep responses concise and focused. | ||
| """ | ||
| ), | ||
| ) | ||
|
|
||
| user_experience_expert = Agent( | ||
| name="UX Expert", | ||
| role="Provide user experience perspective", | ||
| model=OpenAIChat( | ||
| id="gpt-4o-mini", | ||
| api_key=os.environ.get("OPENAI_API_KEY"), | ||
| ), | ||
| add_name_to_context=True, | ||
| instructions=dedent( | ||
| """ | ||
| You are a user experience expert focused on usability. | ||
| Analyze topics from an end-user perspective. | ||
| Discuss user needs, pain points, and user satisfaction. | ||
| Keep responses concise and focused. | ||
| """ | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add validation for missing OPENAI_API_KEY.
The code uses os.environ.get("OPENAI_API_KEY") which returns None if the environment variable is not set. Passing None to OpenAIChat(api_key=...) will likely cause a runtime error. Since this is a sample app that users will run directly, add early validation to provide a clear error message.
Add validation at the start of run_discussion_team():
def run_discussion_team():
"""
A multi-agent discussion team where different experts discuss
and provide perspectives on a given topic.
"""
+ api_key = os.environ.get("OPENAI_API_KEY")
+ if not api_key:
+ raise ValueError(
+ "OPENAI_API_KEY environment variable is required. "
+ "Please set it in your .env file or environment."
+ )
technical_expert = Agent(
name="Technical Expert",
role="Provide technical perspective and implementation details",
model=OpenAIChat(
id="gpt-4o-mini",
- api_key=os.environ.get("OPENAI_API_KEY"),
+ api_key=api_key,
),Then reuse the api_key variable in all other OpenAIChat instances.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/sample-app/sample_app/agno_discussion_team.py around lines 22 to 74,
the OpenAI API key is fetched inline with os.environ.get("OPENAI_API_KEY") for
each OpenAIChat instance which can return None and cause runtime errors; at the
start of run_discussion_team() fetch os.environ["OPENAI_API_KEY"] (or
os.environ.get and explicitly check) into a single api_key variable, validate it
and raise a clear error or exit with a descriptive message if missing, then pass
that api_key variable into every OpenAIChat(...) call instead of calling
os.environ.get(...) repeatedly.
Add OpenTelemetry instrumentation for the agno multi-agent framework. The instrumentation captures agent and team execution with full observability support.
Features:
🤖 Generated with Claude Code
Fixes #3361
feat(instrumentation): ...orfix(instrumentation): ....Important
Add OpenTelemetry instrumentation for Agno framework, including agent and team execution tracing, with comprehensive tests and examples.
AgnoInstrumentorclass inopentelemetry/instrumentation/agno/__init__.pyto instrumentAgent.run(),Agent.arun(),Team.run(), andTeam.arun()._AgentRunWrapper,_AgentARunWrapper,_TeamRunWrapper, and_TeamARunWrapperfor tracing execution.Configclass inconfig.pyfor global settings.dont_throwdecorator andset_span_attributefunction inutils.py.tests/with VCR cassettes for integration testing.sample_app/demonstrating agent and team workflows.pyproject.tomlandproject.jsonfor project configuration..flake8for linting configuration.This description was created by
for 3c1f5d7. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.