-
Notifications
You must be signed in to change notification settings - Fork 559
fix: restore get_analytics() on Session and refresh env vars on init #1311
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -135,6 +135,32 @@ class Config: | |||||
| default_factory=lambda: None, metadata={"description": "Custom span processor for OpenTelemetry trace data"} | ||||||
| ) | ||||||
|
|
||||||
| def refresh_from_env(self) -> None: | ||||||
| """Re-read all configuration values from environment variables. | ||||||
|
|
||||||
| This allows configuration to be updated after import by setting | ||||||
| environment variables and calling init() or configure(). | ||||||
|
||||||
| environment variables and calling init() or configure(). | |
| environment variables and calling init(). |
Copilot
AI
Mar 26, 2026
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.
refresh_from_env() introduces new behavior but there are existing unit tests for Config env parsing (tests/unit/test_config.py) and none that cover this method. Please add a test that mutates os.environ after Config() construction, calls refresh_from_env(), and asserts fields update (and that explicit configure(...) kwargs still win).
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -60,6 +60,46 @@ def end_session(self, **kwargs: Any): | |||||||||||||||||||||
| """Ends the session for CrewAI >= 0.105.0 compatibility. Calls the global legacy end_session.""" | ||||||||||||||||||||||
| end_session(session_or_status=self, **kwargs) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def get_analytics(self) -> Dict[str, Any]: | ||||||||||||||||||||||
| """ | ||||||||||||||||||||||
| Returns analytics data for this session. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Provides backward compatibility with older AgentOps versions where | ||||||||||||||||||||||
| session.get_analytics() was the standard way to retrieve session metrics. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Returns: | ||||||||||||||||||||||
| Dict containing token counts, costs, and other session metrics. | ||||||||||||||||||||||
| """ | ||||||||||||||||||||||
| analytics: Dict[str, Any] = { | ||||||||||||||||||||||
| "prompt_tokens": 0, | ||||||||||||||||||||||
| "completion_tokens": 0, | ||||||||||||||||||||||
| "total_tokens": 0, | ||||||||||||||||||||||
| "total_cost": 0.0, | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
Comment on lines
+63
to
+78
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if not self.trace_context or not self.trace_context.span: | ||||||||||||||||||||||
| return analytics | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| span = self.trace_context.span | ||||||||||||||||||||||
| try: | ||||||||||||||||||||||
| if hasattr(span, "_attributes"): | ||||||||||||||||||||||
| attrs = span._attributes | ||||||||||||||||||||||
| elif hasattr(span, "attributes"): | ||||||||||||||||||||||
| attrs = span.attributes | ||||||||||||||||||||||
|
Comment on lines
+85
to
+88
|
||||||||||||||||||||||
| if hasattr(span, "_attributes"): | |
| attrs = span._attributes | |
| elif hasattr(span, "attributes"): | |
| attrs = span.attributes | |
| if hasattr(span, "attributes"): | |
| # Prefer public OpenTelemetry API to avoid SDK-version coupling | |
| attrs = span.attributes | |
| elif hasattr(span, "_attributes"): | |
| # Backwards compatibility: some OpenTelemetry SDK versions expose attributes via _attributes | |
| attrs = span._attributes |
Copilot
AI
Mar 26, 2026
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.
total_tokens is derived as prompt_tokens + completion_tokens, but spans in this codebase can also set gen_ai.usage.total_tokens directly (and some may not populate prompt/completion). Prefer reading gen_ai.usage.total_tokens when present and only falling back to a sum, otherwise get_analytics() can incorrectly report 0 tokens even when total tokens are recorded on the span.
| analytics["total_tokens"] = analytics["prompt_tokens"] + analytics["completion_tokens"] | |
| total_tokens_attr = attrs.get("gen_ai.usage.total_tokens") | |
| if total_tokens_attr is not None: | |
| analytics["total_tokens"] = int(total_tokens_attr or 0) | |
| else: | |
| analytics["total_tokens"] = analytics["prompt_tokens"] + analytics["completion_tokens"] |
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.
Calling
Config()already reads env vars via the dataclassdefault_factorylambdas, so immediately callingrefresh_from_env()re-reads the same values and adds work without changing outcomes in the common case. Consider either relying onConfig()recreation alone, or switching to reusing the existingConfiginstance and callingrefresh_from_env()(to avoid duplication and clarify the intended initialization path).