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

agentops_property - internal objects tracking the right way #506

Merged
merged 9 commits into from
Nov 18, 2024

Conversation

teocns
Copy link
Contributor

@teocns teocns commented Nov 14, 2024

Introduces the agentops_property descriptor for safe and consistent objects tracking;

  • The check_call_stack_for_agent_id function has been refactored to utilize this property descriptor improving the process of identifying agent IDs from the call stack.

  • Property access nomenclature is standardized to _agentops_<name> for private attrs and agentops_<name> for public attrs

The function `check_call_stack_for_agent_id` is designed to search
through the call stack to find an instance of a class that contains
specific `AgentProperty` descriptors, particularly looking for an
`agent_ops_agent_id`. Here's a step-by-step explanation:1. **Function
Definition and Docstring**:   - The function is defined to return either
a `UUID` or `None`.   - The docstring explains that it looks through the
call stack for the class that called the LLM (Language Learning Model)
and checks for `AgentProperty` descriptors.2. **Inspecting the Call
Stack**:   - The function uses `inspect.stack()` to get the current call
stack, which is a list of `FrameInfo` objects representing the stack
frames.3. **Iterating Through Stack Frames**:   - It iterates over each
`frame_info` in the call stack.4. **Accessing Local Variables**:   - For
each frame, it accesses the local variables using
`frame_info.frame.f_locals`.5. **Checking Each Local Variable**:   - It
iterates over each local variable (`var_name`, `var`) in the current
frame.6. **Stopping at Main**:   - If the variable name is `"__main__"`,
it returns `None` and stops further processing.7. **Checking for
AgentProperty Descriptors**:   - It tries to check if the variable
(`var`) has `AgentProperty` descriptors.   - It gets the type of the
variable (`var_type`).   - It retrieves all class attributes of
`var_type` into a dictionary `class_attrs`.8. **Looking for Specific
Descriptors**:   - It looks for an attribute named `agent_ops_agent_id`
in `class_attrs`.   - If `agent_ops_agent_id` is an instance of
`AgentProperty`, it retrieves the agent ID using
`agent_id_desc.__get__(var, var_type)`.9. **Returning the Agent ID**:
- If an agent ID is found, it optionally retrieves the agent name using
a similar process and returns the agent ID.   - If no agent ID is found,
it continues to the next variable.10. **Handling Exceptions**:    - If
any exception occurs during the process, it catches the exception and
continues to the next variable.11. **Returning None**:    - If no agent
ID is found in the entire call stack, it returns `None`.This function is
useful for debugging or tracking purposes, where you need to identify
the agent that initiated a particular call in a complex system.

Signed-off-by: Teo <[email protected]>
Copy link

codecov bot commented Nov 14, 2024

Codecov Report

Attention: Patch coverage is 86.72199% with 32 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
agentops/descriptor.py 67.69% 12 Missing and 9 partials ⚠️
agentops/decorators.py 63.15% 6 Missing and 1 partial ⚠️
tests/test_agent.py 97.29% 3 Missing and 1 partial ⚠️
Flag Coverage Δ
unittests 54.93% <86.72%> (+2.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
agentops/helpers.py 60.43% <100.00%> (+2.27%) ⬆️
tests/test_agent.py 97.57% <97.29%> (-2.43%) ⬇️
agentops/decorators.py 68.27% <63.15%> (+0.32%) ⬆️
agentops/descriptor.py 67.69% <67.69%> (ø)

agentops/decorators.py Outdated Show resolved Hide resolved
agentops/descriptor.py Show resolved Hide resolved
agentops/helpers.py Outdated Show resolved Hide resolved
@teocns
Copy link
Contributor Author

teocns commented Nov 14, 2024

The previous helper was just a helper in the middle of a helpers utility file. This introduces a new concept that's quite fundamental for what's going on in the AgentOps library: mocking / patching, property lookup, etc. Challenge being faced right now is tracking of AgentOps' own properties installed onto objects.

For instance, say you wanna lookup _agentops_agent_id onto a property.


class Lies:

    def __init__(self):
        self._agentops_agent_id = uuid4()
      

    def __getattr__(self):
      return "lied"



Lies()._agentops_agent_id # lied

Without a descriptor, there is no way to determine whether the value accessed at _agentops_agent_id is genuine, which is exactly what was happening with retrieving the agent_id from call stack: a sea of lies.

@teocns teocns changed the title AgentProperty descriptor AgentOpsDescriptor - a safe way for internal objects tracking Nov 14, 2024
@teocns teocns changed the title AgentOpsDescriptor - a safe way for internal objects tracking AgentOpsDescriptor - internal objects tracking the safe way Nov 14, 2024
@areibman
Copy link
Contributor

The previous helper was just a helper in the middle of a helpers utility file. This introduces a new concept that's quite fundamental for what's going on in the AgentOps library: mocking / patching, property lookup, etc. Challenge being faced right now is tracking of AgentOps' own properties installed onto objects.

For instance, say you wanna lookup _agentops_agent_id onto a property.


class Lies:

    def __init__(self):
        self._agentops_agent_id = uuid4()
      

    def __getattr__(self):
      return "lied"



Lies()._agentops_agent_id # lied

Without a descriptor, there is no way to determine whether the value accessed at _agentops_agent_id is genuine, which is exactly what was happening with retrieving the agent_id from call stack: a sea of lies.

Great catch.

Signed-off-by: Teo <[email protected]>
@teocns
Copy link
Contributor Author

teocns commented Nov 14, 2024

I can't get black to pass, do we have a pre-commit hook to run? might aswell be black version/rules mismatches: how are we standardizing that across development environments?

@teocns teocns changed the title AgentOpsDescriptor - internal objects tracking the safe way AgentOpsDescriptor - internal objects tracking the right way Nov 14, 2024
@the-praxs
Copy link
Member

@teocns @areibman is this good to merge? Will be helpful for #500.

@areibman
Copy link
Contributor

areibman commented Nov 15, 2024

Not good to merge. This is failing our integration with CrewAI:

(env) (base) ➜  job-posting git:(main) ✗ python main.py
🖇 AgentOps: Session Replay: https://app.agentops.ai/drilldown?session_id=fd72ba4e-8d25-43e4-97b9-dd0fb88baf91
What is the company description?
job
What is the company domain?
jobby
What are the hiring needs?
jobbb
What are specific_benefits you offer?
good job
🖇 AgentOps: Failed to track an agent with the @track_agent decorator.
🖇 AgentOps: Failed to track an agent with the @track_agent decorator.
🖇 AgentOps: Failed to track an agent with the @track_agent decorator.

Here's a good example repo to use. Use the job posting example, I find it easiest
https://github.com/AgentOps-AI/crewAI-examples

@the-praxs
Copy link
Member

@teocns I think you should update the examples with the track_agent decorator to demonstrate its usage and that would also help test it out for those agents frameworks.

@bboynton97
Copy link
Contributor

I can't get black to pass, do we have a pre-commit hook to run? might aswell be black version/rules mismatches: how are we standardizing that across development environments?

black is super opinionated and has no configuration. you can run it locally to see what its upset about

@teocns teocns force-pushed the agent-property-descriptor branch from daa87b9 to 91088b5 Compare November 15, 2024 19:02
1. Renamed AgentOpsDescriptor to agentops_property;
2. Thanks to __set_name__, The descriptor will now automatically know
its own name when it's assigned as a class attribute

Signed-off-by: Teo <[email protected]>
@teocns teocns force-pushed the agent-property-descriptor branch from 91088b5 to df9fbc5 Compare November 15, 2024 19:04
@teocns teocns changed the title AgentOpsDescriptor - internal objects tracking the right way agentops_property - internal objects tracking the right way Nov 15, 2024
@teocns
Copy link
Contributor Author

teocns commented Nov 15, 2024

Key Changes

  1. Improved property name resolution:

    • Added fallback mechanisms when __set_name__ isn't called
    • Better handling of dynamically added properties
    • Proper handling of agentops_ prefix stripping
  2. Enhanced error handling:

    • Graceful handling of property name resolution failures
    • Better exception handling in stack inspection
    • Clear error messages when property names can't be determined
  3. Added comprehensive test suite:

    • Tests for basic property get/set operations
    • Tests for stack frame inspection
    • Tests for inheritance scenarios
    • Tests for error cases and edge conditions

Technical Details

  • The descriptor now properly handles both static class attribute definition and dynamic property addition
  • Stack frame inspection is more resilient to exceptions
  • Property naming is more consistent with automatic prefix handling
  • Added proper type hints and documentation

Testing

Added comprehensive test suite covering:

  • Direct property access
  • Nested function calls
  • Multiple agents in different stack frames
  • Inheritance scenarios
  • Error conditions
  • Dynamic property addition

Signed-off-by: Teo <[email protected]>
@teocns
Copy link
Contributor Author

teocns commented Nov 16, 2024

Big Win. It now comes with general pydantic support 🥳

For CrewAI, it requires the following patch; here's why:

For clean code reasons I renamed the property prefix standard to agentops (unified), rather than agent_ops
examples:

agentops_agent_id
_agentops_agent_name

Which makes a lot more sense to contain the scope and purpose within a single snake case group _scope_

@areibman let's merge this - let me know if you wanna move back to agent_ops property nomenclature for an express merge

Signed-off-by: Teo <[email protected]>
@areibman
Copy link
Contributor

Amazing, works for me on several CrewAI and Autogen examples

Copy link
Contributor

@areibman areibman left a comment

Choose a reason for hiding this comment

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

Great stuff

@teocns
Copy link
Contributor Author

teocns commented Nov 18, 2024

@the-praxs second pair of eyes before merge pls?

Copy link
Member

@the-praxs the-praxs left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@teocns teocns merged commit 6f6b956 into main Nov 18, 2024
11 checks passed
@teocns teocns deleted the agent-property-descriptor branch November 18, 2024 09:04
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.

4 participants