Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 102 additions & 1 deletion src/praisonai-agents/praisonaiagents/agent/agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

self.metrics_collector = MetricsCollector()

@property
def _openai_client(self):
Expand Down Expand Up @@ -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
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Model Name Access Issue

The metrics collection code attempts to use self.llm as the model name. However, self.llm is not consistently initialized when custom LLM configurations (e.g., base_url, provider/model strings, or dict configs) are used. This can lead to None or undefined values being passed as the model name, or an AttributeError if self.llm is not set at all. The self.llm_model property should be used instead for unified access to the model name.

Locations (1)

Fix in CursorFix in Web


# 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:
Expand Down Expand Up @@ -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()
Comment thread
coderabbitai[bot] marked this conversation as resolved.

# Log all parameter values when in debug mode
if logging.getLogger().getEffectiveLevel() == logging.DEBUG:
param_info = {
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Setting a temporary attribute _current_performance_metrics on the agent instance to pass data from chat to _chat_completion is a bit fragile and can lead to issues, especially in concurrent environments if the agent instance is shared. It creates a hidden dependency between the two methods.

A more robust approach would be to pass performance_metrics as an argument to _chat_completion. While this might be a larger change that you may want to address separately, it would improve maintainability and safety.


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)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: LLM Metrics Collection Incomplete

Performance metrics tracking is incomplete. For custom LLM providers, PerformanceMetrics is initialized and start_timing() is called, but end_timing() is never invoked, and _current_performance_metrics is not set, preventing metric collection. Additionally, Time To First Token (TTFT) is never tracked because mark_first_token() is not called during the chat flow, causing this metric to always be 0.0.

Locations (1)

Fix in CursorFix in Web


if not response:
# Rollback chat history on response failure
self.chat_history = self.chat_history[:chat_history_length]
Expand Down
4 changes: 4 additions & 0 deletions src/praisonai-agents/praisonaiagents/telemetry/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -58,6 +59,9 @@
'force_shutdown_telemetry',
'MinimalTelemetry',
'TelemetryCollector', # For backward compatibility
'TokenMetrics',
'PerformanceMetrics',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There is a trailing whitespace on this line. It should be removed for code cleanliness.

Suggested change
'PerformanceMetrics',
'PerformanceMetrics',

'MetricsCollector',
]

# Add performance monitoring to __all__ if available
Expand Down
Loading