From c99fbad44d25870555a737e83a56a5ebf219588b Mon Sep 17 00:00:00 2001 From: Federico Busetti <729029+febus982@users.noreply.github.com> Date: Wed, 18 Dec 2024 18:25:57 +0000 Subject: [PATCH 1/8] Rename bootstrap package in common --- Dockerfile | 2 +- docs/inversion-of-control.md | 4 ++-- pyproject.toml | 4 ++-- src/alembic/env.py | 4 ++-- src/celery_worker/__init__.py | 2 +- src/{bootstrap => common}/__init__.py | 0 src/{bootstrap => common}/bootstrap.py | 0 src/{bootstrap => common}/celery.py | 0 src/{bootstrap => common}/config.py | 0 src/{bootstrap => common}/di_container.py | 4 ++-- src/{bootstrap => common}/logs/__init__.py | 0 src/{bootstrap => common}/logs/processors.py | 0 src/{bootstrap => common}/storage/SQLAlchemy/__init__.py | 0 .../storage/SQLAlchemy/default_bind_tables.py | 0 src/{bootstrap => common}/storage/__init__.py | 0 src/http_app/__init__.py | 2 +- tests/conftest.py | 2 +- tests/http_app/conftest.py | 2 +- tests/http_app/routes/books/conftest.py | 2 +- tests/http_app/test_factory.py | 6 +++--- tests/storage/conftest.py | 2 +- tests/storage/test_sqlalchemy_init.py | 6 +++--- 22 files changed, 21 insertions(+), 21 deletions(-) rename src/{bootstrap => common}/__init__.py (100%) rename src/{bootstrap => common}/bootstrap.py (100%) rename src/{bootstrap => common}/celery.py (100%) rename src/{bootstrap => common}/config.py (100%) rename src/{bootstrap => common}/di_container.py (96%) rename src/{bootstrap => common}/logs/__init__.py (100%) rename src/{bootstrap => common}/logs/processors.py (100%) rename src/{bootstrap => common}/storage/SQLAlchemy/__init__.py (100%) rename src/{bootstrap => common}/storage/SQLAlchemy/default_bind_tables.py (100%) rename src/{bootstrap => common}/storage/__init__.py (100%) diff --git a/Dockerfile b/Dockerfile index b1167284..ae972a94 100644 --- a/Dockerfile +++ b/Dockerfile @@ -68,7 +68,7 @@ COPY --chown=nonroot:nonroot poetry.lock . COPY --chown=nonroot:nonroot src/alembic ./alembic COPY --chown=nonroot:nonroot src/domains ./domains COPY --chown=nonroot:nonroot src/gateways ./gateways -COPY --chown=nonroot:nonroot src/bootstrap ./bootstrap +COPY --chown=nonroot:nonroot src/common ./bootstrap COPY --chown=nonroot:nonroot src/alembic.ini . COPY --chown=nonroot:nonroot Makefile . diff --git a/docs/inversion-of-control.md b/docs/inversion-of-control.md index 1d77202a..2bac97f2 100644 --- a/docs/inversion-of-control.md +++ b/docs/inversion-of-control.md @@ -216,7 +216,7 @@ def book_repository_factory() -> BookRepositoryInterface: # file `domains/books/_service.py` from domains.books._gateway_interfaces import BookRepositoryInterface -from bootstrap.factories import book_repository_factory +from common.factories import book_repository_factory class BookService: @@ -274,7 +274,7 @@ def inject_book_repository(f): def wrapper(*args, **kwds): # This allows overriding the decorator if "book_repository" not in kwds.keys(): - from bootstrap.storage import BookRepository + from common.storage import BookRepository kwds["book_repository"] = BookRepository() elif not isinstance(kwds["book_repository"], BookRepositoryInterface): import warnings diff --git a/pyproject.toml b/pyproject.toml index 6f45f0c9..9e4787d5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -82,8 +82,8 @@ branch = true source = ["src"] omit = [ "src/alembic/*", - "src/bootstrap/config.py", - "src/bootstrap/logs/*", + "src/common/config.py", + "src/common/logs/*", ] # It's not necessary to configure concurrency here # because pytest-cov takes care of that diff --git a/src/alembic/env.py b/src/alembic/env.py index 995cc3de..00629292 100644 --- a/src/alembic/env.py +++ b/src/alembic/env.py @@ -4,8 +4,8 @@ from sqlalchemy.ext.asyncio import AsyncEngine from alembic import context -from bootstrap.bootstrap import application_init -from bootstrap.config import AppConfig +from common.bootstrap import application_init +from common.config import AppConfig USE_TWOPHASE = False diff --git a/src/celery_worker/__init__.py b/src/celery_worker/__init__.py index 741fba90..c3d28474 100644 --- a/src/celery_worker/__init__.py +++ b/src/celery_worker/__init__.py @@ -7,7 +7,7 @@ from celery.signals import worker_process_init from opentelemetry.instrumentation.celery import CeleryInstrumentor -from bootstrap import AppConfig, application_init +from common import AppConfig, application_init @worker_process_init.connect(weak=False) diff --git a/src/bootstrap/__init__.py b/src/common/__init__.py similarity index 100% rename from src/bootstrap/__init__.py rename to src/common/__init__.py diff --git a/src/bootstrap/bootstrap.py b/src/common/bootstrap.py similarity index 100% rename from src/bootstrap/bootstrap.py rename to src/common/bootstrap.py diff --git a/src/bootstrap/celery.py b/src/common/celery.py similarity index 100% rename from src/bootstrap/celery.py rename to src/common/celery.py diff --git a/src/bootstrap/config.py b/src/common/config.py similarity index 100% rename from src/bootstrap/config.py rename to src/common/config.py diff --git a/src/bootstrap/di_container.py b/src/common/di_container.py similarity index 96% rename from src/bootstrap/di_container.py rename to src/common/di_container.py index 6e3479e4..e9ceb93b 100644 --- a/src/bootstrap/di_container.py +++ b/src/common/di_container.py @@ -3,7 +3,7 @@ from sqlalchemy_bind_manager import SQLAlchemyBindManager from sqlalchemy_bind_manager.repository import SQLAlchemyAsyncRepository -from bootstrap.config import AppConfig +from common.config import AppConfig from domains.books._gateway_interfaces import ( BookEventGatewayInterface, BookRepositoryInterface, @@ -21,7 +21,7 @@ class Container(DeclarativeContainer): wiring_config = WiringConfiguration( packages=[ - "bootstrap", + "common", "domains", ] ) diff --git a/src/bootstrap/logs/__init__.py b/src/common/logs/__init__.py similarity index 100% rename from src/bootstrap/logs/__init__.py rename to src/common/logs/__init__.py diff --git a/src/bootstrap/logs/processors.py b/src/common/logs/processors.py similarity index 100% rename from src/bootstrap/logs/processors.py rename to src/common/logs/processors.py diff --git a/src/bootstrap/storage/SQLAlchemy/__init__.py b/src/common/storage/SQLAlchemy/__init__.py similarity index 100% rename from src/bootstrap/storage/SQLAlchemy/__init__.py rename to src/common/storage/SQLAlchemy/__init__.py diff --git a/src/bootstrap/storage/SQLAlchemy/default_bind_tables.py b/src/common/storage/SQLAlchemy/default_bind_tables.py similarity index 100% rename from src/bootstrap/storage/SQLAlchemy/default_bind_tables.py rename to src/common/storage/SQLAlchemy/default_bind_tables.py diff --git a/src/bootstrap/storage/__init__.py b/src/common/storage/__init__.py similarity index 100% rename from src/bootstrap/storage/__init__.py rename to src/common/storage/__init__.py diff --git a/src/http_app/__init__.py b/src/http_app/__init__.py index e02b029b..21a1902e 100644 --- a/src/http_app/__init__.py +++ b/src/http_app/__init__.py @@ -6,7 +6,7 @@ from starlette_prometheus import PrometheusMiddleware, metrics from structlog import get_logger -from bootstrap import AppConfig, application_init +from common import AppConfig, application_init from http_app.routes import init_routes diff --git a/tests/conftest.py b/tests/conftest.py index d0b33288..67219f23 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,6 +1,6 @@ import pytest -from bootstrap import AppConfig +from common import AppConfig @pytest.fixture(autouse=True) diff --git a/tests/http_app/conftest.py b/tests/http_app/conftest.py index 1a04d949..a6cafd7b 100644 --- a/tests/http_app/conftest.py +++ b/tests/http_app/conftest.py @@ -10,6 +10,6 @@ @pytest.fixture(scope="function") def testapp(test_config) -> Iterator[FastAPI]: # We don't need the storage to test the HTTP app - with patch("bootstrap.bootstrap.init_storage", return_value=None): + with patch("common.bootstrap.init_storage", return_value=None): app = create_app(test_config=test_config) yield app diff --git a/tests/http_app/routes/books/conftest.py b/tests/http_app/routes/books/conftest.py index b914a8d0..1966453d 100644 --- a/tests/http_app/routes/books/conftest.py +++ b/tests/http_app/routes/books/conftest.py @@ -32,5 +32,5 @@ def book_service() -> Iterator[MagicMock]: @pytest.fixture(scope="function") def testapp(test_config, book_service) -> Iterator[FastAPI]: # We don't need the storage to test the HTTP app - with patch("bootstrap.bootstrap.init_storage", return_value=None): + with patch("common.bootstrap.init_storage", return_value=None): yield create_app(test_config=test_config) diff --git a/tests/http_app/test_factory.py b/tests/http_app/test_factory.py index d4087f1f..1bddea1e 100644 --- a/tests/http_app/test_factory.py +++ b/tests/http_app/test_factory.py @@ -1,19 +1,19 @@ from unittest.mock import patch -from bootstrap.config import AppConfig +from common.config import AppConfig from http_app import create_app def test_with_default_config() -> None: """Test create_app without passing test config.""" - with patch("bootstrap.bootstrap.init_storage", return_value=None): + with patch("common.bootstrap.init_storage", return_value=None): app = create_app() assert app.debug is False def test_with_debug_config() -> None: # We don't need the storage to test the HTTP app - with patch("bootstrap.bootstrap.init_storage", return_value=None): + with patch("common.bootstrap.init_storage", return_value=None): app = create_app( test_config=AppConfig( SQLALCHEMY_CONFIG={}, diff --git a/tests/storage/conftest.py b/tests/storage/conftest.py index 7709b863..88d0d68f 100644 --- a/tests/storage/conftest.py +++ b/tests/storage/conftest.py @@ -4,7 +4,7 @@ from sqlalchemy.orm import clear_mappers from sqlalchemy_bind_manager import SQLAlchemyBindManager, SQLAlchemyConfig -from bootstrap.storage.SQLAlchemy import init_tables +from common.storage.SQLAlchemy import init_tables @pytest.fixture(scope="function") diff --git a/tests/storage/test_sqlalchemy_init.py b/tests/storage/test_sqlalchemy_init.py index 52bbb082..d721d5ce 100644 --- a/tests/storage/test_sqlalchemy_init.py +++ b/tests/storage/test_sqlalchemy_init.py @@ -4,8 +4,8 @@ from sqlalchemy_bind_manager import SQLAlchemyBindManager, SQLAlchemyConfig -from bootstrap.storage import init_storage -from bootstrap.storage.SQLAlchemy import TABLE_INIT_REGISTRY, init_tables +from common.storage import init_storage +from common.storage.SQLAlchemy import TABLE_INIT_REGISTRY, init_tables def test_init_tables_calls_only_supported_bind_initialisation(): @@ -57,7 +57,7 @@ def test_init_tables_calls_only_supported_bind_initialisation(): def test_init_storage_calls_sqlalchemy_init_tables(): with patch( - "bootstrap.storage.SQLAlchemy.init_tables", return_value=None + "common.storage.SQLAlchemy.init_tables", return_value=None ) as mocked_init_tables: init_storage() From 55b9ce4406c411ceaa149b9cc1a9e482eec39e21 Mon Sep 17 00:00:00 2001 From: Federico Busetti <729029+febus982@users.noreply.github.com> Date: Wed, 18 Dec 2024 22:42:55 +0000 Subject: [PATCH 2/8] Create helper class decorator --- src/common/utils.py | 27 +++++++++++++++ tests/common/__init__.py | 0 tests/common/test_utils.py | 67 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+) create mode 100644 src/common/utils.py create mode 100644 tests/common/__init__.py create mode 100644 tests/common/test_utils.py diff --git a/src/common/utils.py b/src/common/utils.py new file mode 100644 index 00000000..842e69b8 --- /dev/null +++ b/src/common/utils.py @@ -0,0 +1,27 @@ +def apply_decorator_to_methods( + decorator, protected_methods: bool = False, private_methods: bool = False +): + """ + Class decorator to apply a given function or coroutine decorator + to all functions and coroutines within a class. + """ + + def class_decorator(cls): + for attr_name, attr_value in cls.__dict__.items(): + # Check if the attribute is a callable (method or coroutine) + if not callable(attr_value): + continue + + if attr_name.startswith(f"_{cls.__name__}__"): + if not private_methods: + continue + + elif attr_name.startswith("_"): + if not protected_methods: + continue + + # Replace the original callable with the decorated version + setattr(cls, attr_name, decorator(attr_value)) + return cls + + return class_decorator diff --git a/tests/common/__init__.py b/tests/common/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/common/test_utils.py b/tests/common/test_utils.py new file mode 100644 index 00000000..a458db34 --- /dev/null +++ b/tests/common/test_utils.py @@ -0,0 +1,67 @@ +import asyncio + +import pytest + +from common.utils import apply_decorator_to_methods + + +@pytest.mark.parametrize( + "apply_to_protected_methods", + [ + True, + False, + ], +) +@pytest.mark.parametrize( + "apply_to_private_methods", + [ + True, + False, + ], +) +async def test_class_decorator( + apply_to_protected_methods: bool, + apply_to_private_methods: bool, +): + def add_ten_decorator(func): + def wrapper(*args, **kwargs): + result = func(*args, **kwargs) + return result + 10 + + async def async_wrapper(*args, **kwargs): + result = await func(*args, **kwargs) + return result + 10 + + return wrapper if not asyncio.iscoroutinefunction(func) else async_wrapper + + @apply_decorator_to_methods( + decorator=add_ten_decorator, + protected_methods=apply_to_protected_methods, + private_methods=apply_to_private_methods, + ) + class MyClass: + def get_public(self): + return 10 + + def _get_protected(self): + return 10 + + def __get_private(self): + return 10 + + async def get_apublic(self): + return 10 + + async def _get_aprotected(self): + return 10 + + async def __get_aprivate(self): + return 10 + + c = MyClass() + assert c.get_public() == 20 + assert c._get_protected() == 20 if apply_to_protected_methods else 10 + assert c._MyClass__get_private() == 20 if apply_to_private_methods else 10 + assert await c.get_apublic() == 20 + assert await c._get_aprotected() == 20 if apply_to_protected_methods else 10 + assert await c._MyClass__get_aprivate() == 20 if apply_to_private_methods else 10 From 695a9616c09b19edd78967972601f8ea29cec23a Mon Sep 17 00:00:00 2001 From: Federico Busetti <729029+febus982@users.noreply.github.com> Date: Wed, 18 Dec 2024 23:29:23 +0000 Subject: [PATCH 3/8] Implement tracing decorator and add it to example service --- .idea/inspectionProfiles/Project_Default.xml | 11 +- src/common/tracing.py | 56 +++++++ src/domains/books/_service.py | 3 + tests/common/test_tracing.py | 156 +++++++++++++++++++ 4 files changed, 221 insertions(+), 5 deletions(-) create mode 100644 src/common/tracing.py create mode 100644 tests/common/test_tracing.py diff --git a/.idea/inspectionProfiles/Project_Default.xml b/.idea/inspectionProfiles/Project_Default.xml index f0797d0b..0c3737ba 100644 --- a/.idea/inspectionProfiles/Project_Default.xml +++ b/.idea/inspectionProfiles/Project_Default.xml @@ -5,11 +5,12 @@ diff --git a/src/common/tracing.py b/src/common/tracing.py new file mode 100644 index 00000000..d31137e0 --- /dev/null +++ b/src/common/tracing.py @@ -0,0 +1,56 @@ +import asyncio +from functools import wraps + +from opentelemetry import trace + +# Get the _tracer instance (You can set your own _tracer name) +tracer = trace.get_tracer(__name__) + + +def trace_function(trace_attributes: bool = True, trace_result: bool = True): + """ + Decorator to trace callables using OpenTelemetry spans. + + Parameters: + - trace_attributes (bool): If False, disables adding function arguments to the span. + - trace_result (bool): If False, disables adding the function's result to the span. + """ + + def decorator(func): + @wraps(func) + def sync_or_async_wrapper(*args, **kwargs): + with tracer.start_as_current_span(func.__name__) as span: + try: + # Set function arguments as attributes + if trace_attributes: + span.set_attribute("function.args", str(args)) + span.set_attribute("function.kwargs", str(kwargs)) + + async def async_handler(): + result = await func(*args, **kwargs) + # Add result to span + if trace_result: + span.set_attribute("function.result", str(result)) + return result + + def sync_handler(): + result = func(*args, **kwargs) + # Add result to span + if trace_result: + span.set_attribute("function.result", str(result)) + return result + + if asyncio.iscoroutinefunction(func): + return async_handler() + else: + return sync_handler() + + except Exception as e: + # Record the exception in the span + span.record_exception(e) + span.set_status(trace.status.Status(trace.status.StatusCode.ERROR)) + raise + + return sync_or_async_wrapper + + return decorator diff --git a/src/domains/books/_service.py b/src/domains/books/_service.py index fabc3b26..ed8e2261 100644 --- a/src/domains/books/_service.py +++ b/src/domains/books/_service.py @@ -4,6 +4,8 @@ from dependency_injector.wiring import Provide, inject from structlog import get_logger +from common.tracing import trace_function +from common.utils import apply_decorator_to_methods from ._gateway_interfaces import BookEventGatewayInterface, BookRepositoryInterface from ._models import BookModel from ._tasks import book_cpu_intensive_task @@ -11,6 +13,7 @@ from .events import BookCreatedV1, BookCreatedV1Data +@apply_decorator_to_methods(trace_function()) class BookService: _book_repository: BookRepositoryInterface _event_gateway: BookEventGatewayInterface diff --git a/tests/common/test_tracing.py b/tests/common/test_tracing.py new file mode 100644 index 00000000..64359c17 --- /dev/null +++ b/tests/common/test_tracing.py @@ -0,0 +1,156 @@ +import asyncio +from unittest.mock import MagicMock, call, patch + +import pytest + +from common.tracing import trace_function + + +@pytest.fixture +def mock_tracer(): + """ + Fixture to mock the OpenTelemetry tracer and span. + """ + mock_tracer = MagicMock() + mock_span = MagicMock() + mock_tracer.start_as_current_span.return_value.__enter__.return_value = mock_span + + with ( + patch("opentelemetry.trace.get_tracer", return_value=mock_tracer), + patch("common.tracing.tracer", mock_tracer), + ): + yield mock_tracer, mock_span + + +def test_sync_function_default_params(mock_tracer): + """ + Test a synchronous function with default decorator parameters. + """ + mock_tracer, mock_span = mock_tracer + + # Define a sync function to wrap with the decorator + @trace_function() + def add_nums(a, b): + return a + b + + # Call the function + result = add_nums(2, 3) + + # Assertions + assert result == 5 + mock_tracer.start_as_current_span.assert_called_once_with("add_nums") + mock_span.set_attribute.assert_any_call("function.args", "(2, 3)") + mock_span.set_attribute.assert_any_call("function.result", "5") + + +@pytest.mark.asyncio +async def test_async_function_default_params(mock_tracer): + """ + Test an asynchronous function with default decorator parameters. + """ + mock_tracer, mock_span = mock_tracer + + # Define an async function to wrap with the decorator + @trace_function() + async def async_func(a, b): + await asyncio.sleep(0.1) + return a * b + + # Run the async function + result = await async_func(4, 5) + + # Assertions + assert result == 20 + mock_tracer.start_as_current_span.assert_called_once_with("async_func") + mock_span.set_attribute.assert_any_call("function.args", "(4, 5)") + mock_span.set_attribute.assert_any_call("function.result", "20") + + +def test_disable_function_attributes(mock_tracer): + """ + Test a synchronous function with `add_function_attributes` set to False. + """ + mock_tracer, mock_span = mock_tracer + + # Define a sync function with attributes disabled + @trace_function(trace_attributes=False) + def sync_func(a, b): + return a - b + + # Call the function + result = sync_func(10, 6) + + # Assertions + assert result == 4 + mock_tracer.start_as_current_span.assert_called_once_with("sync_func") + mock_span.set_attribute.assert_any_call("function.result", "4") + assert ( + call("function.args", "(10, 6)") not in mock_span.set_attribute.call_args_list + ) + + +def test_disable_result_in_span_sync(mock_tracer): + """ + Test an asynchronous function with `add_result_to_span` set to False. + """ + mock_tracer, mock_span = mock_tracer + + # Define an async function with result disabled + @trace_function(trace_result=False) + def sync_func(a, b): + return a / b + + # Run the async function + result = sync_func(10, 2) + + # Assertions + assert result == 5.0 + mock_tracer.start_as_current_span.assert_called_once_with("sync_func") + mock_span.set_attribute.assert_any_call("function.args", "(10, 2)") + assert call("function.result") not in mock_span.set_attribute.call_args_list + + +@pytest.mark.asyncio +async def test_disable_result_in_span(mock_tracer): + """ + Test an asynchronous function with `add_result_to_span` set to False. + """ + mock_tracer, mock_span = mock_tracer + + # Define an async function with result disabled + @trace_function(trace_result=False) + async def async_func(a, b): + await asyncio.sleep(0.1) + return a / b + + # Run the async function + result = await async_func(10, 2) + + # Assertions + assert result == 5.0 + mock_tracer.start_as_current_span.assert_called_once_with("async_func") + mock_span.set_attribute.assert_any_call("function.args", "(10, 2)") + assert call("function.result") not in mock_span.set_attribute.call_args_list + + +def test_exception_in_function(mock_tracer): + """ + Test behavior when the function raises an exception. + """ + mock_tracer, mock_span = mock_tracer + + # Define a failing function + @trace_function() + def failing_func(a, b): + if b == 0: + raise ValueError("Division by zero!") + return a / b + + # Use pytest to assert the exception is raised + with pytest.raises(ValueError, match="Division by zero!"): + failing_func(10, 0) + + # Assertions + mock_tracer.start_as_current_span.assert_called_once_with("failing_func") + mock_span.record_exception.assert_called_once() + mock_span.set_status.assert_called_once() From 0fadae559ad9a6a55ddef4c374b7d3628823052d Mon Sep 17 00:00:00 2001 From: Federico Busetti <729029+febus982@users.noreply.github.com> Date: Wed, 18 Dec 2024 23:42:43 +0000 Subject: [PATCH 4/8] Lint and typing --- src/common/utils.py | 5 ++--- src/domains/books/_service.py | 1 + tests/common/test_utils.py | 12 ++++++------ 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/common/utils.py b/src/common/utils.py index 842e69b8..979a0c22 100644 --- a/src/common/utils.py +++ b/src/common/utils.py @@ -16,9 +16,8 @@ def class_decorator(cls): if not private_methods: continue - elif attr_name.startswith("_"): - if not protected_methods: - continue + elif attr_name.startswith("_") and not protected_methods: + continue # Replace the original callable with the decorated version setattr(cls, attr_name, decorator(attr_value)) diff --git a/src/domains/books/_service.py b/src/domains/books/_service.py index ed8e2261..45b4bf3e 100644 --- a/src/domains/books/_service.py +++ b/src/domains/books/_service.py @@ -6,6 +6,7 @@ from common.tracing import trace_function from common.utils import apply_decorator_to_methods + from ._gateway_interfaces import BookEventGatewayInterface, BookRepositoryInterface from ._models import BookModel from ._tasks import book_cpu_intensive_task diff --git a/tests/common/test_utils.py b/tests/common/test_utils.py index a458db34..90c106df 100644 --- a/tests/common/test_utils.py +++ b/tests/common/test_utils.py @@ -8,15 +8,15 @@ @pytest.mark.parametrize( "apply_to_protected_methods", [ - True, - False, + pytest.param(True, id="protected_methods"), + pytest.param(False, id="no_protected_methods"), ], ) @pytest.mark.parametrize( "apply_to_private_methods", [ - True, - False, + pytest.param(True, id="private_methods"), + pytest.param(False, id="no_private_methods"), ], ) async def test_class_decorator( @@ -61,7 +61,7 @@ async def __get_aprivate(self): c = MyClass() assert c.get_public() == 20 assert c._get_protected() == 20 if apply_to_protected_methods else 10 - assert c._MyClass__get_private() == 20 if apply_to_private_methods else 10 + assert c._MyClass__get_private() == 20 if apply_to_private_methods else 10 # type: ignore assert await c.get_apublic() == 20 assert await c._get_aprotected() == 20 if apply_to_protected_methods else 10 - assert await c._MyClass__get_aprivate() == 20 if apply_to_private_methods else 10 + assert await c._MyClass__get_aprivate() == 20 if apply_to_private_methods else 10 # type: ignore From 2d4d670c83dfa03a05df2aa893fb8df264fc5e33 Mon Sep 17 00:00:00 2001 From: Federico Busetti <729029+febus982@users.noreply.github.com> Date: Wed, 18 Dec 2024 23:51:23 +0000 Subject: [PATCH 5/8] Attempt to reduce cognitive complexity --- src/common/utils.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/common/utils.py b/src/common/utils.py index 979a0c22..23d268d9 100644 --- a/src/common/utils.py +++ b/src/common/utils.py @@ -6,21 +6,34 @@ def apply_decorator_to_methods( to all functions and coroutines within a class. """ + def is_private_method(attr_name, cls_name): + """Check if the attribute is a private method.""" + return attr_name.startswith(f"_{cls_name}__") + + def is_protected_method(attr_name, cls_name): + """Check if the attribute is a protected method.""" + return attr_name.startswith("_") and not attr_name.startswith(f"_{cls_name}__") + def class_decorator(cls): + cls_name = cls.__name__ + for attr_name, attr_value in cls.__dict__.items(): - # Check if the attribute is a callable (method or coroutine) + # Skip attributes that are not callable if not callable(attr_value): continue - if attr_name.startswith(f"_{cls.__name__}__"): + # Check for private methods + if is_private_method(attr_name, cls_name): if not private_methods: continue - elif attr_name.startswith("_") and not protected_methods: + # Check for protected methods + elif is_protected_method(attr_name, cls_name) and not protected_methods: continue # Replace the original callable with the decorated version setattr(cls, attr_name, decorator(attr_value)) + return cls return class_decorator From d3ae2c76a43d27291e1dc4e63ff314f5b82ad4f9 Mon Sep 17 00:00:00 2001 From: Federico Busetti <729029+febus982@users.noreply.github.com> Date: Wed, 18 Dec 2024 23:57:09 +0000 Subject: [PATCH 6/8] Attempt 2 to reduce cognitive complexity --- src/common/tracing.py | 74 +++++++++++++++++++++++-------------------- 1 file changed, 39 insertions(+), 35 deletions(-) diff --git a/src/common/tracing.py b/src/common/tracing.py index d31137e0..fe5fa246 100644 --- a/src/common/tracing.py +++ b/src/common/tracing.py @@ -1,9 +1,7 @@ import asyncio from functools import wraps - from opentelemetry import trace -# Get the _tracer instance (You can set your own _tracer name) tracer = trace.get_tracer(__name__) @@ -16,41 +14,47 @@ def trace_function(trace_attributes: bool = True, trace_result: bool = True): - trace_result (bool): If False, disables adding the function's result to the span. """ + def set_span_attributes(span, args, kwargs, result=None): + """Helper to set function arguments and results as span attributes.""" + if trace_attributes: + span.set_attribute("function.args", str(args)) + span.set_attribute("function.kwargs", str(kwargs)) + if trace_result and result is not None: + span.set_attribute("function.result", str(result)) + + def record_exception(span, exception): + """Helper to handle exception recording in a span.""" + span.record_exception(exception) + span.set_status(trace.status.Status(trace.status.StatusCode.ERROR)) + + async def handle_async(span, func, *args, **kwargs): + """Handle asynchronous functions.""" + try: + result = await func(*args, **kwargs) + set_span_attributes(span, args, kwargs, result) + return result + except Exception as e: + record_exception(span, e) + raise + + def handle_sync(span, func, *args, **kwargs): + """Handle synchronous functions.""" + try: + result = func(*args, **kwargs) + set_span_attributes(span, args, kwargs, result) + return result + except Exception as e: + record_exception(span, e) + raise + def decorator(func): @wraps(func) - def sync_or_async_wrapper(*args, **kwargs): + def wrapper(*args, **kwargs): with tracer.start_as_current_span(func.__name__) as span: - try: - # Set function arguments as attributes - if trace_attributes: - span.set_attribute("function.args", str(args)) - span.set_attribute("function.kwargs", str(kwargs)) - - async def async_handler(): - result = await func(*args, **kwargs) - # Add result to span - if trace_result: - span.set_attribute("function.result", str(result)) - return result - - def sync_handler(): - result = func(*args, **kwargs) - # Add result to span - if trace_result: - span.set_attribute("function.result", str(result)) - return result - - if asyncio.iscoroutinefunction(func): - return async_handler() - else: - return sync_handler() - - except Exception as e: - # Record the exception in the span - span.record_exception(e) - span.set_status(trace.status.Status(trace.status.StatusCode.ERROR)) - raise - - return sync_or_async_wrapper + if asyncio.iscoroutinefunction(func): + return handle_async(span, func, *args, **kwargs) + return handle_sync(span, func, *args, **kwargs) + + return wrapper return decorator From b995b09e58cb6fb4a7394fab061f65be659df233 Mon Sep 17 00:00:00 2001 From: Federico Busetti <729029+febus982@users.noreply.github.com> Date: Thu, 19 Dec 2024 00:06:09 +0000 Subject: [PATCH 7/8] Revert "Attempt 2 to reduce cognitive complexity" This reverts commit d3ae2c76a43d27291e1dc4e63ff314f5b82ad4f9. --- src/common/tracing.py | 74 ++++++++++++++++++++----------------------- 1 file changed, 35 insertions(+), 39 deletions(-) diff --git a/src/common/tracing.py b/src/common/tracing.py index fe5fa246..d31137e0 100644 --- a/src/common/tracing.py +++ b/src/common/tracing.py @@ -1,7 +1,9 @@ import asyncio from functools import wraps + from opentelemetry import trace +# Get the _tracer instance (You can set your own _tracer name) tracer = trace.get_tracer(__name__) @@ -14,47 +16,41 @@ def trace_function(trace_attributes: bool = True, trace_result: bool = True): - trace_result (bool): If False, disables adding the function's result to the span. """ - def set_span_attributes(span, args, kwargs, result=None): - """Helper to set function arguments and results as span attributes.""" - if trace_attributes: - span.set_attribute("function.args", str(args)) - span.set_attribute("function.kwargs", str(kwargs)) - if trace_result and result is not None: - span.set_attribute("function.result", str(result)) - - def record_exception(span, exception): - """Helper to handle exception recording in a span.""" - span.record_exception(exception) - span.set_status(trace.status.Status(trace.status.StatusCode.ERROR)) - - async def handle_async(span, func, *args, **kwargs): - """Handle asynchronous functions.""" - try: - result = await func(*args, **kwargs) - set_span_attributes(span, args, kwargs, result) - return result - except Exception as e: - record_exception(span, e) - raise - - def handle_sync(span, func, *args, **kwargs): - """Handle synchronous functions.""" - try: - result = func(*args, **kwargs) - set_span_attributes(span, args, kwargs, result) - return result - except Exception as e: - record_exception(span, e) - raise - def decorator(func): @wraps(func) - def wrapper(*args, **kwargs): + def sync_or_async_wrapper(*args, **kwargs): with tracer.start_as_current_span(func.__name__) as span: - if asyncio.iscoroutinefunction(func): - return handle_async(span, func, *args, **kwargs) - return handle_sync(span, func, *args, **kwargs) - - return wrapper + try: + # Set function arguments as attributes + if trace_attributes: + span.set_attribute("function.args", str(args)) + span.set_attribute("function.kwargs", str(kwargs)) + + async def async_handler(): + result = await func(*args, **kwargs) + # Add result to span + if trace_result: + span.set_attribute("function.result", str(result)) + return result + + def sync_handler(): + result = func(*args, **kwargs) + # Add result to span + if trace_result: + span.set_attribute("function.result", str(result)) + return result + + if asyncio.iscoroutinefunction(func): + return async_handler() + else: + return sync_handler() + + except Exception as e: + # Record the exception in the span + span.record_exception(e) + span.set_status(trace.status.Status(trace.status.StatusCode.ERROR)) + raise + + return sync_or_async_wrapper return decorator From 49732be171d1f797db1ad3ef578ef73a00b21ff7 Mon Sep 17 00:00:00 2001 From: Federico Busetti <729029+febus982@users.noreply.github.com> Date: Thu, 19 Dec 2024 00:06:19 +0000 Subject: [PATCH 8/8] Revert "Attempt to reduce cognitive complexity" This reverts commit 2d4d670c83dfa03a05df2aa893fb8df264fc5e33. --- src/common/utils.py | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/src/common/utils.py b/src/common/utils.py index 23d268d9..979a0c22 100644 --- a/src/common/utils.py +++ b/src/common/utils.py @@ -6,34 +6,21 @@ def apply_decorator_to_methods( to all functions and coroutines within a class. """ - def is_private_method(attr_name, cls_name): - """Check if the attribute is a private method.""" - return attr_name.startswith(f"_{cls_name}__") - - def is_protected_method(attr_name, cls_name): - """Check if the attribute is a protected method.""" - return attr_name.startswith("_") and not attr_name.startswith(f"_{cls_name}__") - def class_decorator(cls): - cls_name = cls.__name__ - for attr_name, attr_value in cls.__dict__.items(): - # Skip attributes that are not callable + # Check if the attribute is a callable (method or coroutine) if not callable(attr_value): continue - # Check for private methods - if is_private_method(attr_name, cls_name): + if attr_name.startswith(f"_{cls.__name__}__"): if not private_methods: continue - # Check for protected methods - elif is_protected_method(attr_name, cls_name) and not protected_methods: + elif attr_name.startswith("_") and not protected_methods: continue # Replace the original callable with the decorated version setattr(cls, attr_name, decorator(attr_value)) - return cls return class_decorator