Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: Add LLM call events for improved observability #2214

Merged
merged 8 commits into from
Feb 24, 2025

Conversation

lorenzejay
Copy link
Collaborator

  • Introduce new LLM call events: LLMCallStartedEvent, LLMCallCompletedEvent, and LLMCallFailedEvent
  • Emit events for LLM calls and tool calls to provide better tracking and debugging
  • Add event handling in the LLM class to track call lifecycle
  • Update event bus to support new LLM-related events
  • Add test cases to validate LLM event emissions

- Introduce new LLM call events: LLMCallStartedEvent, LLMCallCompletedEvent, and LLMCallFailedEvent
- Emit events for LLM calls and tool calls to provide better tracking and debugging
- Add event handling in the LLM class to track call lifecycle
- Update event bus to support new LLM-related events
- Add test cases to validate LLM event emissions
@joaomdmoura
Copy link
Collaborator

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

Code Review Comment for PR #2214: Add LLM Call Events

Overview

The PR implements new LLM call events for improved observability in the crewAI system. It introduces event tracking for the LLM calls' lifecycle, including start, completion, and failure states. This is a significant enhancement that will facilitate better debugging and operational insights.

Code Quality Findings

1. Type Hinting

  • Issue: The _handle_emit_call_events method is missing a return type hint.
  • Suggestion: Update the method signature to include -> None to enhance code clarity.
    def _handle_emit_call_events(self, response: str, call_type: LLMCallType) -> None:

2. Redundant Casting

  • Issue: The code redundantly casts the call_type when creating an LLMCallCompletedEvent.
  • Suggestion: Simplify the instantiation by removing the unnecessary casting:
    event=LLMCallCompletedEvent(
        response=response, call_type=call_type  # call_type is already the correct type
    ),

3. Documentation

  • Issue: The new events lack docstrings which can lead to misunderstandings in the future.
  • Suggestion: Add docstrings for the events to describe their purpose and parameters:
    """
    Emits an event when LLM call starts.
    
    Args:
        messages: List of messages for the LLM
        tools: Available tools for the LLM
        callbacks: Callback functions
        available_functions: Dictionary of available functions
    """

Historical Context and Learnings

  • Review related pull requests that also dealt with event emissions and observability. There is a consistent emphasis on improving the robustness of our error handling and ensuring that emitted events are well-structured.
  • Previous PRs have documented how other components handle similar event emissions and error cases, providing learnings that can be beneficial for this implementation.

Implications for Related Files

  • Changes to __init__.py and task_events.py are minimal but crucial for maintaining a coherent module structure. These should be validated as they integrate new events seamlessly into the current architecture.
  • Ensure that the tests in tests/utilities/test_events.py cover new functionalities, especially edge cases regarding the lifecycle events of LLM calls.

Specific Improvement Suggestions

  1. Enhanced Error Handling: Consider implementing more specific error types that can distinguish different failure modes, leading to better diagnostics.

    class LLMToolCallError(LLMCallError):
        """Exception raised for tool call errors during LLM execution."""
        pass
  2. Event Data Validation: Introduce validators for event data to ensure the integrity and accuracy of the emitted data.

    class LLMCallStartedEvent(CrewEvent):
        @validator('messages')
        def validate_messages(cls, v):
            if not v:
                raise ValueError("Messages cannot be empty")
            return v
  3. Performance Optimization: If the LLM calls are frequent, consider implementing a batching mechanism for events to reduce potential overhead.

    class EventBatcher:
        def add_event(self, event):
            # Logic to batch events for efficiency

Conclusion

This implementation is solid, significantly improving observability through structured event handling. The suggested improvements will enhance type safety, documentation, error handling, and overall code quality. The PR is recommended for merge after addressing the minor suggestions above, particularly the type hints and documentation additions.

Looking forward to seeing these changes incorporated!

- Implement event listeners for LLM call events in EventListener
- Add logging for LLM call start, completion, and failure events
- Import and register new LLM-specific event types
Remove unnecessary LLMCallType conversion when emitting LLMCallCompletedEvent
Improve docstrings for LLM call events to more accurately describe their purpose and lifecycle
lorenzejay and others added 2 commits February 24, 2025 11:52
Enhance error handling by emitting a specific event when tool execution fails during LLM calls
@bhancockio bhancockio merged commit c62fb61 into main Feb 24, 2025
4 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants