Skip to content
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

Fix issue #2366: Add Agent.execute_task wrapper for OpenTelemetry logging #2367

Closed
wants to merge 3 commits into from

Conversation

devin-ai-integration[bot]
Copy link
Contributor

This PR fixes issue #2366 where OpenTelemetry logs only store input.value field for agent calls but no output.value.

Problem

The current OpenInference instrumentation for CrewAI only instruments:

  • Task._execute_core
  • Crew.kickoff
  • ToolUsage._use

It's missing instrumentation for Agent.execute_task, which is where agent outputs are generated and emitted through the event system.

Solution

This PR adds a new wrapper for Agent.execute_task that properly captures both input and output values in OpenTelemetry spans. The implementation:

  1. Creates a resilient wrapper that handles missing OpenInference dependencies
  2. Properly captures both input and output values in spans
  3. Adds agent and task metadata to spans for better traceability
  4. Includes comprehensive error handling

Testing

Added tests that verify:

  • The patch function exists and is callable
  • The patch handles missing OpenInference dependencies gracefully
  • Agent execution events are properly emitted

Link to Devin run: https://app.devin.ai/sessions/040acf9550c345cba55cc9f90e3fc6e0

…ging

Co-Authored-By: Joe Moura <joao@crewai.com>
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add "(aside)" to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Co-Authored-By: Joe Moura <joao@crewai.com>
@joaomdmoura
Copy link
Collaborator

Disclaimer: This review was made by a crew of AI Agents.

Code Review Report for PR #2367

Overview

This pull request introduces OpenTelemetry logging for the Agent.execute_task method, addressing issue #2366. The changes enhance instrumentation to effectively capture both input and output values of agent tasks, facilitating improved observability.

File-by-File Analysis

1. src/crewai/telemetry/__init__.py

Positive Aspects:

  • The implementation successfully manages conditional patching, promoting modularity.
  • Error handling for importing dependencies is well-structured, ensuring that missing libraries do not disrupt functionality.

Suggestions for Improvement:

  • Type Hints: Integrating type hints for functions improves code clarity and maintainability.
  • Version Check: Implement version checks for the OpenInference library to avoid compatibility issues.

2. src/crewai/telemetry/patches/openinference_agent_wrapper.py

Positive Aspects:

  • The file features comprehensive logging and error handling, portraying a thoughtful approach to managing exceptions.
  • Documentation is clear, and class responsibilities are well-separated, which aids future developers.

Issues and Suggestions:

  1. Constant Management: Move constants related to span attributes to a dedicated configuration file. This can improve maintainability and clarity.

    class SpanAttributes:
        OUTPUT_VALUE = "output.value"
        INPUT_VALUE = "input.value"
        OPENINFERENCE_SPAN_KIND = "openinference.span.kind"
  2. Type Hinting: Adding type hints to classes and methods throughout this file will enhance overall code quality.

    from typing import Any, Optional, Callable, Dict
    
    class _AgentExecuteTaskWrapper:
        ...
  3. Span Context Management: Implement span context management to facilitate better data collection in telemetry.

    def _create_span_context(self, instance: Any, task: Any) -> Dict[str, Any]:
        ...

3. tests/telemetry/test_openinference_agent_wrapper.py

Positive Aspects:

  • The test suite provides a solid foundation, covering basic use cases and mocking dependencies appropriately.

Issues and Suggestions:

  1. Expanded Test Cases: Adding a wider variety of test cases will ensure robustness across different environments. For example:

    @pytest.mark.parametrize("has_openinference", [True, False])
    def test_patch_with_different_environments(has_openinference, monkeypatch):
        ...
  2. Integration Testing with OpenTelemetry: Checking that span attributes accurately capture both input and output values should be part of the test suite.

    def test_span_attributes_capture():
        ...

General Recommendations

  1. Documentation Enhancements:

    • Include detailed instructions for configuring OpenTelemetry logging.
    • Prepare a migration guide or changelog for future reference.
  2. Resilience in Error Handling:

    • Implement retry mechanisms for telemetry functions to handle transient failures more gracefully.
  3. Performance Considerations:

    • Introduce options for configuring sampling rates to mitigate overhead.
    • Look into batching telemetry submissions for efficiency.
  4. Security Practices:

    • Ensure sensitive data is sanitized before logging.
    • Establish limits for span attribute sizes and employ circuit breakers as needed.

Conclusion

The implementation presented in this PR is well-structured and effectively brings OpenTelemetry logging to the Agent.execute_task method. By implementing the suggested improvements, the maintainability and reliability of the telemetry system can be further enhanced, ensuring it meets both immediate and future needs.

Related Pull Requests

  • Review related pull requests in the broader context to see whether any existing patterns or learnings can be applied here.

This review highlights potential weaknesses and areas for improvement, ensuring that the codebase remains robust and maintainable over time.

…ent, and move constants to config file

Co-Authored-By: Joe Moura <joao@crewai.com>
Copy link
Contributor Author

Closing due to inactivity for more than 7 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant