-
Notifications
You must be signed in to change notification settings - Fork 509
Lazy Loading of LLM Libraries to Prevent Duplicate Instrumentation #987
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
Conversation
…sion checks. Introduced PROVIDERS and AGENTIC_LIBRARIES dictionaries for managing instrumentors, and implemented get_active_libraries function to identify currently used libraries. Updated InstrumentorLoader to validate library versions before activation. Refactored instrument_all function to prioritize agentic libraries before standard providers.
Codecov ReportAttention: Patch coverage is
📢 Thoughts on this report? Let us know! |
…or improved package monitoring and dynamic instrumentation. Added methods for managing active instrumentors, handling conflicts between agentic libraries and providers, and monitoring imports. Updated configuration for supported libraries and combined target packages for streamlined instrumentation.
|
Import Hooking Magic: We handle the tricky case of provider vs agentic library conflicts. For example, if someone's using CrewAI (which uses litellm, which uses OpenAI under the hood), we automatically uninstrument the direct OpenAI provider to avoid double-counting. The implementation uses a singleton
@the-praxs @tcdent @areibman Thoughts? checkout the PR, I have made changes with this approach and it works! |
|
The dance of instrumentation is a tedious one. But I will review thy PR. Time for some Easter eggs in the codebase :) |
|
Does this now incorporate a fix for modules imported in sub-modules? Super common use case to have a project span multiple modules. |
tcdent
left a comment
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.
Just left a couple comments on code style.
Yes it does, at first it checks using sys.modules to instrument existing direct imports, then relies on import hook to instrument indirect imports |
|
@tcdent Try this out #Example.py #llm_caller.py |
…tor configurations, improving type safety for PROVIDERS and AGENTIC_LIBRARIES dictionaries. This change facilitates better structure and clarity in managing instrumentor settings.
…implement module-level state management for active instrumentors. Introduced helper functions for package instrumentation, monitoring imports, and uninstrumenting providers, enhancing clarity and maintainability of the code. Updated instrumentation logic to handle conflicts between agentic libraries and providers more effectively.
…ntops into lazy-loading-enhancement
…menting processes. Removed internal helper functions for starting and stopping monitoring, integrating their logic directly into the `instrument_all` and `uninstrument_all` functions for improved clarity and maintainability.
… existing imports directly into the `instrument_all` function. This change removes the now-unnecessary `_check_existing_imports` helper function, enhancing code clarity and maintainability while preserving the functionality of monitoring already imported packages.
We've been facing issues with duplicate LLM instrumentation when multiple instrumentors (openai, crewai etc.) try to wrap the same LLM calls. This leads to redundant spans and potential conflicts in tracing.
Refactoring instrumentation module to be independent of each other is eating me up for the past week :)
The Solution
Me, @the-praxs and @fenilfaldu were discussing approaches to fix tightly coupled behavior and make it independent of each other. Thanks to @the-praxs (🐐) who came up with this solution from V3, we've implemented a lazy loading approach for LLM libraries. Instead of eagerly importing and instrumenting all possible LLM providers and agentic frameworks, we now check sys.modules for direct imports and prioritize Agentic Libararis over providers to eliminate redundant spans.
Why This is Better
Required Changes to Agentic Libraries
With this change, we need to update our agentic library integrations to record their own LLM spans:
This ensures that even when these frameworks make LLM calls internally, we'll have proper visibility into their operations without duplicate instrumentation.
Current Limitations
There's one known limitation: if a provider/agentic framework is imported in a separate file and then imported into the main file, we miss it. For example:
Possible fix is to scan imported modules' source files for additional imports
Testing
I've done some testing with this approach and found it to be more reliable than our previous method. It successfully prevents duplicate spans but we still need to update crewai, autogen and agents instrumentations.