From b82445b030ed548f288cc1fc11df9bb71f17e8b8 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Wed, 18 Dec 2024 10:18:45 +0100 Subject: [PATCH 1/3] Add type checker --- dev-requirements.txt | 1 + .../instrumentation/threading/__init__.py | 45 +++++++++++++++---- pyproject.toml | 15 +++++++ tox.ini | 13 +++++- 4 files changed, 64 insertions(+), 10 deletions(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index 2668677cc5..7740b08100 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -1,6 +1,7 @@ pylint==3.0.2 httpretty==1.1.4 mypy==0.931 +pyright==v1.1.390 sphinx==7.1.2 sphinx-rtd-theme==2.0.0rc4 sphinx-autodoc-typehints==1.25.2 diff --git a/instrumentation/opentelemetry-instrumentation-threading/src/opentelemetry/instrumentation/threading/__init__.py b/instrumentation/opentelemetry-instrumentation-threading/src/opentelemetry/instrumentation/threading/__init__.py index be1eec139e..9488e3ce47 100644 --- a/instrumentation/opentelemetry-instrumentation-threading/src/opentelemetry/instrumentation/threading/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-threading/src/opentelemetry/instrumentation/threading/__init__.py @@ -35,17 +35,29 @@ run method or the executor's worker thread." """ +from __future__ import annotations + import threading from concurrent import futures -from typing import Collection +from typing import TYPE_CHECKING, Any, Callable, Collection -from wrapt import wrap_function_wrapper +from wrapt import ( + wrap_function_wrapper, # type: ignore[reportUnknownVariableType] +) from opentelemetry import context from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.threading.package import _instruments from opentelemetry.instrumentation.utils import unwrap +if TYPE_CHECKING: + from typing import Protocol, TypeVar + + R = TypeVar("R") + + class HasOtelContext(Protocol): + _otel_context: context.Context + class ThreadingInstrumentor(BaseInstrumentor): __WRAPPER_START_METHOD = "start" @@ -55,12 +67,12 @@ class ThreadingInstrumentor(BaseInstrumentor): def instrumentation_dependencies(self) -> Collection[str]: return _instruments - def _instrument(self, **kwargs): + def _instrument(self, **kwargs: Any): self._instrument_thread() self._instrument_timer() self._instrument_thread_pool() - def _uninstrument(self, **kwargs): + def _uninstrument(self, **kwargs: Any): self._uninstrument_thread() self._uninstrument_timer() self._uninstrument_thread_pool() @@ -117,12 +129,22 @@ def _uninstrument_thread_pool(): ) @staticmethod - def __wrap_threading_start(call_wrapped, instance, args, kwargs): + def __wrap_threading_start( + call_wrapped: Callable[[], None], + instance: HasOtelContext, + args: ..., + kwargs: ..., + ) -> None: instance._otel_context = context.get_current() return call_wrapped(*args, **kwargs) @staticmethod - def __wrap_threading_run(call_wrapped, instance, args, kwargs): + def __wrap_threading_run( + call_wrapped: Callable[..., R], + instance: HasOtelContext, + args: tuple[Any, ...], + kwargs: dict[str, Any], + ) -> R: token = None try: token = context.attach(instance._otel_context) @@ -131,12 +153,17 @@ def __wrap_threading_run(call_wrapped, instance, args, kwargs): context.detach(token) @staticmethod - def __wrap_thread_pool_submit(call_wrapped, instance, args, kwargs): + def __wrap_thread_pool_submit( + call_wrapped: Callable[..., R], + instance: futures.ThreadPoolExecutor, + args: tuple[Callable[..., Any], ...], + kwargs: dict[str, Any], + ) -> R: # obtain the original function and wrapped kwargs original_func = args[0] otel_context = context.get_current() - def wrapped_func(*func_args, **func_kwargs): + def wrapped_func(*func_args: Any, **func_kwargs: Any) -> R: token = None try: token = context.attach(otel_context) @@ -145,5 +172,5 @@ def wrapped_func(*func_args, **func_kwargs): context.detach(token) # replace the original function with the wrapped function - new_args = (wrapped_func,) + args[1:] + new_args: tuple[Callable[..., Any], ...] = (wrapped_func,) + args[1:] return call_wrapped(*new_args, **kwargs) diff --git a/pyproject.toml b/pyproject.toml index fd5ee5716f..f151dacf1a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,3 +39,18 @@ known-third-party = [ "opencensus", ] +# https://github.com/microsoft/pyright/blob/main/docs/configuration.md#type-check-rule-overrides +[tool.pyright] +typeCheckingMode = "strict" +reportUnnecessaryTypeIgnoreComment = true +reportMissingTypeStubs = false +reportPrivateUsage = false # Ignore private attributes added by instrumentation packages. +# Add progressively instrumentation packages here. +include = [ + "instrumentation/opentelemetry-instrumentation-threading/**/*.py" +] +# We should also add type hints to the test suite - It helps on finding bugs. +# We are excluding for now because it's easier, and more important to add to the instrumentation packages. +exclude = [ + "instrumentation/opentelemetry-instrumentation-threading/tests/**", +] diff --git a/tox.ini b/tox.ini index 563f6cc34d..3557d3369d 100644 --- a/tox.ini +++ b/tox.ini @@ -391,6 +391,7 @@ envlist = generate-workflows shellcheck ruff + typecheck [testenv] test_deps = @@ -657,7 +658,6 @@ deps = util-http: -r {toxinidir}/util/opentelemetry-util-http/test-requirements.txt util-http: {toxinidir}/util/opentelemetry-util-http ; FIXME: add coverage testing - ; FIXME: add mypy testing allowlist_externals = sh @@ -963,3 +963,14 @@ deps = pre-commit commands = pre-commit run --color=always --all-files {posargs} + +[testenv:typecheck] +description = type checker +deps = + -c {toxinidir}/dev-requirements.txt + pyright + {[testenv]test_deps} + {toxinidir}/opentelemetry-instrumentation + {toxinidir}/util/opentelemetry-util-http +commands = + pyright From a9c312479a7500bd44e024e63b3c8bade9631921 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Wed, 18 Dec 2024 10:31:07 +0100 Subject: [PATCH 2/3] Update tox.ini --- tox.ini | 1 - 1 file changed, 1 deletion(-) diff --git a/tox.ini b/tox.ini index 3557d3369d..4273eabab6 100644 --- a/tox.ini +++ b/tox.ini @@ -965,7 +965,6 @@ commands = pre-commit run --color=always --all-files {posargs} [testenv:typecheck] -description = type checker deps = -c {toxinidir}/dev-requirements.txt pyright From bca86450f714d27491012eda3a12c8f3e4affe0a Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Wed, 18 Dec 2024 10:36:13 +0100 Subject: [PATCH 3/3] Obey pipeline --- .github/workflows/misc_0.yml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/.github/workflows/misc_0.yml b/.github/workflows/misc_0.yml index 1148f85abd..422669b86f 100644 --- a/.github/workflows/misc_0.yml +++ b/.github/workflows/misc_0.yml @@ -152,3 +152,21 @@ jobs: - name: Run tests run: tox -e ruff + + typecheck: + name: typecheck + runs-on: ubuntu-latest + steps: + - name: Checkout repo @ SHA - ${{ github.sha }} + uses: actions/checkout@v4 + + - name: Set up Python 3.11 + uses: actions/setup-python@v5 + with: + python-version: "3.11" + + - name: Install tox + run: pip install tox + + - name: Run tests + run: tox -e typecheck