-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat: implement comprehensive monitoring system for PraisonAI Agents #1053
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
Changes from all commits
031922f
590c128
da3531c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,6 +44,7 @@ | |
| from ..task.task import Task | ||
| from ..main import TaskOutput | ||
| from ..handoff import Handoff | ||
| from ..telemetry.metrics import MetricsCollector | ||
|
|
||
| class Agent: | ||
| def _generate_tool_definition(self, function_name): | ||
|
|
@@ -218,7 +219,9 @@ def __init__( | |
| max_guardrail_retries: int = 3, | ||
| handoffs: Optional[List[Union['Agent', 'Handoff']]] = None, | ||
| base_url: Optional[str] = None, | ||
| api_key: Optional[str] = None | ||
| api_key: Optional[str] = None, | ||
| track_metrics: bool = False, | ||
| metrics_collector: Optional[MetricsCollector] = None | ||
| ): | ||
| """Initialize an Agent instance. | ||
|
|
||
|
|
@@ -309,6 +312,11 @@ def __init__( | |
| If provided, automatically creates a custom LLM instance. Defaults to None. | ||
| api_key (Optional[str], optional): API key for LLM provider. If not provided, | ||
| falls back to environment variables. Defaults to None. | ||
| track_metrics (bool, optional): Enable detailed metrics tracking including token usage, | ||
| performance metrics (TTFT), and session-level aggregation. Defaults to False. | ||
| metrics_collector (Optional[MetricsCollector], optional): Custom MetricsCollector instance | ||
| for session-level metric aggregation. If None and track_metrics is True, a new | ||
| collector will be created automatically. Defaults to None. | ||
|
|
||
| Raises: | ||
| ValueError: If all of name, role, goal, backstory, and instructions are None. | ||
|
|
@@ -507,6 +515,16 @@ def __init__( | |
| if knowledge: | ||
| for source in knowledge: | ||
| self._process_knowledge(source) | ||
|
|
||
| # Initialize metrics tracking | ||
| self.track_metrics = track_metrics | ||
| self.metrics_collector = metrics_collector | ||
| self.last_metrics = {} # Store last execution metrics | ||
|
|
||
| if self.track_metrics and self.metrics_collector is None: | ||
| # Create a new MetricsCollector if none provided | ||
| from ..telemetry.metrics import MetricsCollector | ||
| self.metrics_collector = MetricsCollector() | ||
|
|
||
| @property | ||
| def _openai_client(self): | ||
|
|
@@ -1182,6 +1200,55 @@ def _chat_completion(self, messages, temperature=0.2, tools=None, stream=True, r | |
| max_iterations=10 | ||
| ) | ||
|
|
||
| # Extract metrics if tracking is enabled | ||
| if self.track_metrics and final_response and hasattr(final_response, 'usage'): | ||
| try: | ||
| from ..telemetry.metrics import TokenMetrics | ||
| from ..telemetry import get_telemetry | ||
|
|
||
| # Extract token metrics from the response | ||
| token_metrics = TokenMetrics.from_completion_usage(final_response.usage) | ||
|
|
||
| # Track performance metrics if available | ||
| perf_metrics = None | ||
| if hasattr(self, '_current_performance_metrics'): | ||
| perf_metrics = self._current_performance_metrics | ||
| # Calculate tokens per second | ||
| if token_metrics.output_tokens > 0 and perf_metrics.total_time > 0: | ||
| perf_metrics.tokens_per_second = token_metrics.output_tokens / perf_metrics.total_time | ||
|
|
||
| # Store last metrics for user access | ||
| self.last_metrics = { | ||
| 'tokens': token_metrics, | ||
| 'performance': perf_metrics | ||
| } | ||
|
|
||
| # Add to metrics collector if available | ||
| if self.metrics_collector: | ||
| # Get proper model name - handle dict configs and custom LLMs | ||
| model_name = self.llm | ||
| if isinstance(self.llm, dict): | ||
| model_name = self.llm.get('model', str(self.llm)) | ||
| elif self._using_custom_llm and hasattr(self, 'llm_instance'): | ||
| model_name = getattr(self.llm_instance, 'model', self.llm) | ||
|
|
||
| self.metrics_collector.add_agent_metrics( | ||
| agent_name=self.name, | ||
| token_metrics=token_metrics, | ||
| performance_metrics=perf_metrics, | ||
| model_name=str(model_name) # Ensure it's always a string | ||
| ) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Model Name Access IssueThe metrics collection code attempts to use |
||
|
|
||
| # Send to telemetry system | ||
| telemetry = get_telemetry() | ||
| telemetry.track_tokens(token_metrics) | ||
| if perf_metrics: | ||
| telemetry.track_performance(perf_metrics) | ||
|
|
||
| except Exception as metrics_error: | ||
| # Don't fail the main response if metrics collection fails | ||
| logging.debug(f"Failed to collect metrics: {metrics_error}") | ||
|
|
||
| return final_response | ||
|
|
||
| except Exception as e: | ||
|
|
@@ -1249,6 +1316,13 @@ def chat(self, prompt, temperature=0.2, tools=None, output_json=None, output_pyd | |
| # Reset the final display flag for each new conversation | ||
| self._final_display_shown = False | ||
|
|
||
| # Initialize metrics tracking for this request | ||
| performance_metrics = None | ||
| if self.track_metrics: | ||
| from ..telemetry.metrics import PerformanceMetrics | ||
| performance_metrics = PerformanceMetrics() | ||
| performance_metrics.start_timing() | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| # Log all parameter values when in debug mode | ||
| if logging.getLogger().getEffectiveLevel() == logging.DEBUG: | ||
| param_info = { | ||
|
|
@@ -1324,6 +1398,10 @@ def chat(self, prompt, temperature=0.2, tools=None, output_json=None, output_pyd | |
| self.chat_history.append({"role": "user", "content": normalized_content}) | ||
|
|
||
| try: | ||
| # Set performance metrics for custom LLM tracking | ||
| if performance_metrics: | ||
| self._current_performance_metrics = performance_metrics | ||
|
|
||
| # Pass everything to LLM class | ||
| response_text = self.llm_instance.get_response( | ||
| prompt=prompt, | ||
|
|
@@ -1350,6 +1428,13 @@ def chat(self, prompt, temperature=0.2, tools=None, output_json=None, output_pyd | |
| stream=stream # Pass the stream parameter from chat method | ||
| ) | ||
|
|
||
| # Clean up performance metrics after custom LLM call | ||
| if performance_metrics: | ||
| self._current_performance_metrics = None | ||
| # End timing for custom LLM performance metrics (estimate token count from response) | ||
| estimated_token_count = len(response_text.split()) if response_text else 0 | ||
| performance_metrics.end_timing(estimated_token_count) | ||
|
|
||
| self.chat_history.append({"role": "assistant", "content": response_text}) | ||
|
|
||
| # Log completion time if in debug mode | ||
|
|
@@ -1419,7 +1504,23 @@ def chat(self, prompt, temperature=0.2, tools=None, output_json=None, output_pyd | |
| agent_tools=agent_tools | ||
| ) | ||
|
|
||
| # Set performance metrics for access in _chat_completion | ||
| if performance_metrics: | ||
| self._current_performance_metrics = performance_metrics | ||
|
Comment on lines
+1508
to
+1509
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Setting a temporary attribute A more robust approach would be to pass |
||
|
|
||
| response = self._chat_completion(messages, temperature=temperature, tools=tools if tools else None, reasoning_steps=reasoning_steps, stream=stream, task_name=task_name, task_description=task_description, task_id=task_id) | ||
|
|
||
| # Clean up performance metrics after use | ||
| if performance_metrics: | ||
| self._current_performance_metrics = None | ||
|
|
||
| # End timing for performance metrics | ||
| if performance_metrics: | ||
| token_count = 0 | ||
| if response and hasattr(response, 'usage') and hasattr(response.usage, 'completion_tokens'): | ||
| token_count = response.usage.completion_tokens or 0 | ||
| performance_metrics.end_timing(token_count) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: LLM Metrics Collection IncompletePerformance metrics tracking is incomplete. For custom LLM providers, |
||
|
|
||
| if not response: | ||
| # Rollback chat history on response failure | ||
| self.chat_history = self.chat_history[:chat_history_length] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,7 @@ | |
|
|
||
| # Import the classes for real (not just type checking) | ||
| from .telemetry import MinimalTelemetry, TelemetryCollector | ||
| from .metrics import TokenMetrics, PerformanceMetrics, MetricsCollector | ||
|
|
||
| # Import performance monitoring tools | ||
| try: | ||
|
|
@@ -58,6 +59,9 @@ | |
| 'force_shutdown_telemetry', | ||
| 'MinimalTelemetry', | ||
| 'TelemetryCollector', # For backward compatibility | ||
| 'TokenMetrics', | ||
| 'PerformanceMetrics', | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| 'MetricsCollector', | ||
| ] | ||
|
|
||
| # Add performance monitoring to __all__ if available | ||
|
|
||
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.
This file uses local imports in several places (e.g., here, and in
_chat_completion). According to PEP 8, imports should usually be at the top of the file. This makes it easier to see what modules a script requires. Local imports are acceptable to resolve circular dependencies or for optional, heavy modules, but that doesn't seem to be the case here. Consider moving these imports to the top of the file for better readability and maintainability.