From 2f12343dba1a6b18bcad9785f81af8061676463b Mon Sep 17 00:00:00 2001 From: fenilfaldu Date: Wed, 14 May 2025 04:34:06 +0530 Subject: [PATCH 1/7] refactored the factory implementation of decorator to handle the async calls --- agentops/sdk/decorators/factory.py | 50 ++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/agentops/sdk/decorators/factory.py b/agentops/sdk/decorators/factory.py index bc56ece59..fd750260e 100644 --- a/agentops/sdk/decorators/factory.py +++ b/agentops/sdk/decorators/factory.py @@ -33,7 +33,6 @@ def decorator(wrapped=None, *, name=None, version=None): # Create a proxy class that wraps the original class class WrappedClass(wrapped): def __init__(self, *args, **kwargs): - # Start span when instance is created operation_name = name or wrapped.__name__ self._agentops_span_context_manager = _create_as_current_span(operation_name, entity_kind, version) self._agentops_active_span = self._agentops_span_context_manager.__enter__() @@ -45,23 +44,58 @@ def __init__(self, *args, **kwargs): # Call the original __init__ super().__init__(*args, **kwargs) + + async def __aenter__(self): + # Added for async context manager support + # This allows using the class with 'async with' statement + + # If span is already created in __init__, just return self + if hasattr(self, '_agentops_active_span') and self._agentops_active_span is not None: + return self + + # Otherwise create span (for backward compatibility) + operation_name = name or wrapped.__name__ + self._agentops_span_context_manager = _create_as_current_span(operation_name, entity_kind, version) + self._agentops_active_span = self._agentops_span_context_manager.__enter__() + return self + + async def __aexit__(self, exc_type, exc_val, exc_tb): + # Added for proper async cleanup + # This ensures spans are properly closed when using 'async with' + + # Added proper async cleanup + if hasattr(self, '_agentops_active_span') and hasattr(self, '_agentops_span_context_manager'): + try: + _record_entity_output(self._agentops_active_span, self) + except Exception as e: + logger.warning(f"Failed to record entity output: {e}") + + self._agentops_span_context_manager.__exit__(exc_type, exc_val, exc_tb) + # Clear the span references after cleanup + self._agentops_span_context_manager = None + self._agentops_active_span = None + def __del__(self): - # End span when instance is destroyed - if hasattr(self, '_agentops_active_span') and hasattr(self, '_agentops_span_context_manager'): + # Only try to cleanup if we have valid span references + if (hasattr(self, '_agentops_active_span') and + hasattr(self, '_agentops_span_context_manager') and + self._agentops_span_context_manager is not None and + self._agentops_active_span is not None): try: _record_entity_output(self._agentops_active_span, self) except Exception as e: logger.warning(f"Failed to record entity output: {e}") - self._agentops_span_context_manager.__exit__(None, None, None) - + # Clear the span references after cleanup + self._agentops_span_context_manager = None + self._agentops_active_span = None # Preserve metadata of the original class WrappedClass.__name__ = wrapped.__name__ WrappedClass.__qualname__ = wrapped.__qualname__ WrappedClass.__module__ = wrapped.__module__ WrappedClass.__doc__ = wrapped.__doc__ - + return WrappedClass # Create the actual decorator wrapper function for functions @@ -114,6 +148,7 @@ async def _wrapped_async(): try: result = await wrapped(*args, **kwargs) + print(result,"result here in decorator factory is async") try: _record_entity_output(span, result) except Exception as e: @@ -128,13 +163,16 @@ async def _wrapped_async(): # Handle sync functions else: with _create_as_current_span(operation_name, entity_kind, version) as span: + print(operation_name,entity_kind,"operation name and entity kind here in decorator factory") try: _record_entity_input(span, args, kwargs) + print(span,"span here in decorator factory is sync") except Exception as e: logger.warning(f"Failed to record entity input: {e}") try: result = wrapped(*args, **kwargs) + print(result,"result here in decorator factory is sync") try: _record_entity_output(span, result) except Exception as e: From 5e1bafc7a01cb35ca795da74127739b22cc84adb Mon Sep 17 00:00:00 2001 From: fenilfaldu Date: Wed, 14 May 2025 04:51:51 +0530 Subject: [PATCH 2/7] code refactored --- agentops/sdk/decorators/factory.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/agentops/sdk/decorators/factory.py b/agentops/sdk/decorators/factory.py index fd750260e..22eaf5251 100644 --- a/agentops/sdk/decorators/factory.py +++ b/agentops/sdk/decorators/factory.py @@ -148,7 +148,6 @@ async def _wrapped_async(): try: result = await wrapped(*args, **kwargs) - print(result,"result here in decorator factory is async") try: _record_entity_output(span, result) except Exception as e: @@ -163,16 +162,15 @@ async def _wrapped_async(): # Handle sync functions else: with _create_as_current_span(operation_name, entity_kind, version) as span: - print(operation_name,entity_kind,"operation name and entity kind here in decorator factory") try: _record_entity_input(span, args, kwargs) - print(span,"span here in decorator factory is sync") + except Exception as e: logger.warning(f"Failed to record entity input: {e}") try: result = wrapped(*args, **kwargs) - print(result,"result here in decorator factory is sync") + try: _record_entity_output(span, result) except Exception as e: From 68cb1d1060642fc4271e54a5a0b5b31a77e6e976 Mon Sep 17 00:00:00 2001 From: fenilfaldu Date: Wed, 14 May 2025 05:16:30 +0530 Subject: [PATCH 3/7] added some test --- tests/unit/sdk/test_decorators.py | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/tests/unit/sdk/test_decorators.py b/tests/unit/sdk/test_decorators.py index e67c85c6c..7ebbd4746 100644 --- a/tests/unit/sdk/test_decorators.py +++ b/tests/unit/sdk/test_decorators.py @@ -1,5 +1,6 @@ from typing import TYPE_CHECKING, cast, AsyncGenerator, Generator import asyncio +import gc import pytest from opentelemetry import trace @@ -10,6 +11,7 @@ from agentops.semconv.span_attributes import SpanAttributes from agentops.semconv import SpanAttributes from tests.unit.sdk.instrumentation_tester import InstrumentationTester +from agentops.sdk.decorators.factory import create_entity_decorator class TestSpanNesting: @@ -568,4 +570,30 @@ def test_workflow_session(): # Verify transform_task is a child of the workflow span assert transform_task.parent is not None assert workflow_span.context is not None - assert transform_task.parent.span_id == workflow_span.context.span_id \ No newline at end of file + assert transform_task.parent.span_id == workflow_span.context.span_id + +@pytest.mark.asyncio +async def test_async_context_manager_and_del_coverage(): + """ + Covers async context manager (__aenter__, __aexit__) and __del__ cleanup logic. + """ + # Create a simple decorated class + @create_entity_decorator("test") + class TestClass: + def __init__(self): + self.value = 42 + + # Cover __aenter__ and __aexit__ (normal exit) + async with TestClass() as instance: + assert hasattr(instance, '_agentops_active_span') + assert instance._agentops_active_span is not None + + # Cover __aenter__ and __aexit__ (exceptional exit) + with pytest.raises(ValueError): + async with TestClass() as instance: + raise ValueError("Trigger exception for __aexit__ coverage") + + # Cover __del__ logic + obj = TestClass() + del obj + gc.collect() # Force garbage collection to trigger __del__ \ No newline at end of file From 7bbe027e412ff71ca7a302ad1a51718b7ae8cf14 Mon Sep 17 00:00:00 2001 From: fenilfaldu Date: Wed, 14 May 2025 22:32:42 +0530 Subject: [PATCH 4/7] ruff linters --- agentops/sdk/decorators/factory.py | 32 ++++++++++++++++-------------- tests/unit/sdk/test_decorators.py | 6 ++++-- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/agentops/sdk/decorators/factory.py b/agentops/sdk/decorators/factory.py index bff0c7d37..989fb6e40 100644 --- a/agentops/sdk/decorators/factory.py +++ b/agentops/sdk/decorators/factory.py @@ -49,43 +49,45 @@ def __init__(self, *args, **kwargs): # Call the original __init__ super().__init__(*args, **kwargs) + async def __aenter__(self): # Added for async context manager support # This allows using the class with 'async with' statement - + # If span is already created in __init__, just return self - if hasattr(self, '_agentops_active_span') and self._agentops_active_span is not None: + if hasattr(self, "_agentops_active_span") and self._agentops_active_span is not None: return self - + # Otherwise create span (for backward compatibility) operation_name = name or wrapped.__name__ self._agentops_span_context_manager = _create_as_current_span(operation_name, entity_kind, version) self._agentops_active_span = self._agentops_span_context_manager.__enter__() - return self + return self async def __aexit__(self, exc_type, exc_val, exc_tb): # Added for proper async cleanup # This ensures spans are properly closed when using 'async with' - + # Added proper async cleanup - if hasattr(self, '_agentops_active_span') and hasattr(self, '_agentops_span_context_manager'): + if hasattr(self, "_agentops_active_span") and hasattr(self, "_agentops_span_context_manager"): try: _record_entity_output(self._agentops_active_span, self) except Exception as e: logger.warning(f"Failed to record entity output: {e}") - + self._agentops_span_context_manager.__exit__(exc_type, exc_val, exc_tb) # Clear the span references after cleanup self._agentops_span_context_manager = None self._agentops_active_span = None - def __del__(self): # Only try to cleanup if we have valid span references - if (hasattr(self, '_agentops_active_span') and - hasattr(self, '_agentops_span_context_manager') and - self._agentops_span_context_manager is not None and - self._agentops_active_span is not None): + if ( + hasattr(self, "_agentops_active_span") + and hasattr(self, "_agentops_span_context_manager") + and self._agentops_span_context_manager is not None + and self._agentops_active_span is not None + ): try: _record_entity_output(self._agentops_active_span, self) except Exception as e: @@ -93,7 +95,7 @@ def __del__(self): self._agentops_span_context_manager.__exit__(None, None, None) # Clear the span references after cleanup self._agentops_span_context_manager = None - self._agentops_active_span = None + self._agentops_active_span = None # Preserve metadata of the original class WrappedClass.__name__ = wrapped.__name__ @@ -170,13 +172,13 @@ async def _wrapped_async(): with _create_as_current_span(operation_name, entity_kind, version) as span: try: _record_entity_input(span, args, kwargs) - + except Exception as e: logger.warning(f"Failed to record entity input: {e}") try: result = wrapped(*args, **kwargs) - + try: _record_entity_output(span, result) except Exception as e: diff --git a/tests/unit/sdk/test_decorators.py b/tests/unit/sdk/test_decorators.py index c7cbd4bbf..757a1d25f 100644 --- a/tests/unit/sdk/test_decorators.py +++ b/tests/unit/sdk/test_decorators.py @@ -1,7 +1,7 @@ from typing import AsyncGenerator import asyncio import gc - +import pytest from agentops.sdk.decorators import agent, operation, session, workflow, task from agentops.semconv import SpanKind @@ -603,11 +603,13 @@ def test_workflow_session(): assert workflow_span.context is not None assert transform_task.parent.span_id == workflow_span.context.span_id + @pytest.mark.asyncio async def test_async_context_manager_and_del_coverage(): """ Covers async context manager (__aenter__, __aexit__) and __del__ cleanup logic. """ + # Create a simple decorated class @create_entity_decorator("test") class TestClass: @@ -616,7 +618,7 @@ def __init__(self): # Cover __aenter__ and __aexit__ (normal exit) async with TestClass() as instance: - assert hasattr(instance, '_agentops_active_span') + assert hasattr(instance, "_agentops_active_span") assert instance._agentops_active_span is not None # Cover __aenter__ and __aexit__ (exceptional exit) From 67694541fdb4cc76438270463eb3067135478de7 Mon Sep 17 00:00:00 2001 From: fenilfaldu Date: Wed, 14 May 2025 23:27:54 +0530 Subject: [PATCH 5/7] removed the __del__ method --- agentops/sdk/decorators/factory.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/agentops/sdk/decorators/factory.py b/agentops/sdk/decorators/factory.py index 989fb6e40..13b09789d 100644 --- a/agentops/sdk/decorators/factory.py +++ b/agentops/sdk/decorators/factory.py @@ -68,7 +68,6 @@ async def __aexit__(self, exc_type, exc_val, exc_tb): # Added for proper async cleanup # This ensures spans are properly closed when using 'async with' - # Added proper async cleanup if hasattr(self, "_agentops_active_span") and hasattr(self, "_agentops_span_context_manager"): try: _record_entity_output(self._agentops_active_span, self) @@ -80,23 +79,6 @@ async def __aexit__(self, exc_type, exc_val, exc_tb): self._agentops_span_context_manager = None self._agentops_active_span = None - def __del__(self): - # Only try to cleanup if we have valid span references - if ( - hasattr(self, "_agentops_active_span") - and hasattr(self, "_agentops_span_context_manager") - and self._agentops_span_context_manager is not None - and self._agentops_active_span is not None - ): - try: - _record_entity_output(self._agentops_active_span, self) - except Exception as e: - logger.warning(f"Failed to record entity output: {e}") - self._agentops_span_context_manager.__exit__(None, None, None) - # Clear the span references after cleanup - self._agentops_span_context_manager = None - self._agentops_active_span = None - # Preserve metadata of the original class WrappedClass.__name__ = wrapped.__name__ WrappedClass.__qualname__ = wrapped.__qualname__ From 4934a35806150a5207562dbf4f9325cbbc7b744f Mon Sep 17 00:00:00 2001 From: fenilfaldu Date: Thu, 15 May 2025 04:42:39 +0530 Subject: [PATCH 6/7] removed the __del__ in unit tests --- tests/unit/sdk/test_decorators.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/tests/unit/sdk/test_decorators.py b/tests/unit/sdk/test_decorators.py index 757a1d25f..01a83e1c9 100644 --- a/tests/unit/sdk/test_decorators.py +++ b/tests/unit/sdk/test_decorators.py @@ -605,9 +605,9 @@ def test_workflow_session(): @pytest.mark.asyncio -async def test_async_context_manager_and_del_coverage(): +async def test_async_context_manager(): """ - Covers async context manager (__aenter__, __aexit__) and __del__ cleanup logic. + Tests async context manager functionality (__aenter__, __aexit__). """ # Create a simple decorated class @@ -625,8 +625,3 @@ def __init__(self): with pytest.raises(ValueError): async with TestClass() as instance: raise ValueError("Trigger exception for __aexit__ coverage") - - # Cover __del__ logic - obj = TestClass() - del obj - gc.collect() # Force garbage collection to trigger __del__ From 1ec10725b4b5cf8127fe1b32fbf5e971b9d52cb4 Mon Sep 17 00:00:00 2001 From: fenilfaldu Date: Thu, 15 May 2025 04:49:10 +0530 Subject: [PATCH 7/7] apply ruff linter and formatter fixes --- tests/unit/sdk/test_decorators.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/sdk/test_decorators.py b/tests/unit/sdk/test_decorators.py index 01a83e1c9..98f1cd402 100644 --- a/tests/unit/sdk/test_decorators.py +++ b/tests/unit/sdk/test_decorators.py @@ -1,6 +1,5 @@ from typing import AsyncGenerator import asyncio -import gc import pytest from agentops.sdk.decorators import agent, operation, session, workflow, task