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

Initialize Client.llm_tracker on all codepaths. #257

Closed

Conversation

mbarnathan
Copy link
Contributor

@mbarnathan mbarnathan commented Jun 14, 2024

Prevents an issue with the check in stop_instrumenting().
To properly test this, I also needed to add a parameter to override the behavior of singleton.

📥 Pull Request

📘 Description
Initialize llm_tracker to None when instrument_llm_calls is False.
Add "allow_multiple_instances" kwargs to suppress singleton decorator so the client may be tested.

🔄 Related Issue (if applicable)
#256

🎯 Goal
Fix an issue that may occur when Crew and Autogen packages exist but you aren't using them.
(In my case, it popped up in a secondary file's __ main __ section)

🧪 Testing
Added a unit test; passed on tox.

Prevents an issue with the check in stop_instrumenting().
To properly test this, I also needed to add a parameter to
override the behavior of @singleton.
@areibman
Copy link
Contributor

This is awesome-- Adding @bboynton97 as a reviewer, as there might be some conflicts with the new session management strategy

@siyangqiu
Copy link
Contributor

Thanks for making this PR, but due to a number of updates to AgentOps, I think this PR is no longer necessary to achieve what you are looking for:

LlmTracker is only disabled by default if you have crewai or autogen in sys.modules rather than simply installed on your system. Furthermore, the instrument_llm_calls parameter of agentops.init() should now have precedence over the default disable behavior, so you can always just set that option to True regardless of packages being used (though it may still result in double logging of LLM calls)

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.

3 participants