Merge branch 'main' into add-type-hints-to-sqlite3

aabmass · web-flow · commit a5b2a7f0989e · 2024-12-03T14:59:53.000-05:00
diff --git a/.readthedocs.yml b/.readthedocs.yml
@@ -12,4 +12,4 @@ build:
 
 python:
   install:
-    - requirements: docs-requirements.txt
+    - requirements: rtd-requirements.txt
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- `opentelemetry-instrumentation-starlette` Add type hints to the instrumentation
+  ([#3045](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3045))
 - `opentelemetry-distro` default to OTLP log exporter.
   ([#3042](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3042))
 - `opentelemetry-instrumentation-sqlalchemy` Update unit tests to run with SQLALchemy 2
@@ -44,6 +46,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#3022](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3022))
 - Replace all instrumentor unit test `assertEqualSpanInstrumentationInfo` calls with `assertEqualSpanInstrumentationScope` calls
   ([#3037](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3037))
+- `opentelemetry-instrumentation-sqlalchemy`: Fix a remaining memory leak in EngineTracer
+  ([#3053](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3053))
 
 ### Breaking changes
 
diff --git a/docs-requirements.txt b/docs-requirements.txt
@@ -2,14 +2,6 @@ sphinx==7.1.2
 sphinx-rtd-theme==2.0.0rc4
 sphinx-autodoc-typehints==1.25.2
 
-# Need to install the api/sdk in the venv for autodoc. Modifying sys.path
-# doesn't work for pkg_resources.
--e "git+https://github.com/open-telemetry/opentelemetry-python.git#egg=opentelemetry-api&subdirectory=opentelemetry-api"
--e "git+https://github.com/open-telemetry/opentelemetry-python.git#egg=opentelemetry-semantic-conventions&subdirectory=opentelemetry-semantic-conventions"
--e "git+https://github.com/open-telemetry/opentelemetry-python.git#egg=opentelemetry-sdk&subdirectory=opentelemetry-sdk"
--e "git+https://github.com/open-telemetry/opentelemetry-python-contrib.git#egg=opentelemetry-util-http&subdirectory=util/opentelemetry-util-http"
-./opentelemetry-instrumentation
-
 # Required by opentelemetry-instrumentation
 fastapi>=0.65.2
 pymemcache~=1.3
diff --git a/docs/instrumentation/aiohttp_server/aiohttp_server.rst b/docs/instrumentation/aiohttp_server/aiohttp_server.rst
@@ -0,0 +1,7 @@
+.. include:: ../../../instrumentation/opentelemetry-instrumentation-aiohttp-server/README.rst
+    :end-before: References
+
+.. automodule:: opentelemetry.instrumentation.aiohttp_server
+    :members:
+    :undoc-members:
+    :show-inheritance:
diff --git a/docs/instrumentation/click/click.rst b/docs/instrumentation/click/click.rst
@@ -0,0 +1,7 @@
+.. include:: ../../../instrumentation/opentelemetry-instrumentation-click/README.rst
+    :end-before: References
+
+.. automodule:: opentelemetry.instrumentation.click
+    :members:
+    :undoc-members:
+    :show-inheritance:
diff --git a/docs/nitpick-exceptions.ini b/docs/nitpick-exceptions.ini
@@ -29,6 +29,7 @@ py-class=
     httpx.SyncByteStream
     httpx.AsyncByteStream
     httpx.Response
+    aiohttp.web_request.Request
     yarl.URL
     cimpl.Producer
     cimpl.Consumer
@@ -65,7 +66,7 @@ any=
 
 py-obj=
     opentelemetry.propagators.textmap.CarrierT
-    
+
 py-func=
     poll
     flush
diff --git a/instrumentation/opentelemetry-instrumentation-click/README.rst b/instrumentation/opentelemetry-instrumentation-click/README.rst
@@ -1,5 +1,5 @@
 OpenTelemetry click Instrumentation
-===========================
+====================================
 
 |pypi|
 
diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py
@@ -167,13 +167,26 @@ def _pool_checkout(
         self._add_idle_to_connection_usage(-1)
         self._add_used_to_connection_usage(1)
 
+    @classmethod
+    def _dispose_of_event_listener(cls, obj):
+        try:
+            cls._remove_event_listener_params.remove(obj)
+        except ValueError:
+            pass
+
     @classmethod
     def _register_event_listener(cls, target, identifier, func, *args, **kw):
         listen(target, identifier, func, *args, **kw)
         cls._remove_event_listener_params.append(
             (weakref.ref(target), identifier, func)
         )
 
+        weakref.finalize(
+            target,
+            cls._dispose_of_event_listener,
+            (weakref.ref(target), identifier, func),
+        )
+
     @classmethod
     def remove_all_event_listeners(cls):
         for (
diff --git a/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py b/instrumentation/opentelemetry-instrumentation-sqlalchemy/tests/test_sqlalchemy.py
@@ -417,6 +417,10 @@ def test_no_memory_leakage_if_engine_diposed(self):
 
         from sqlalchemy import create_engine
 
+        from opentelemetry.instrumentation.sqlalchemy.engine import (
+            EngineTracer,
+        )
+
         callback = mock.Mock()
 
         def make_shortlived_engine():
@@ -432,3 +436,4 @@ def make_shortlived_engine():
 
         gc.collect()
         assert callback.call_count == 5
+        assert len(EngineTracer._remove_event_listener_params) == 0
diff --git a/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/py.typed b/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/py.typed
diff --git a/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py b/instrumentation/opentelemetry-instrumentation-starlette/src/opentelemetry/instrumentation/starlette/__init__.py
@@ -170,7 +170,9 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A
 ---
 """
 
-from typing import Collection
+from __future__ import annotations
+
+from typing import TYPE_CHECKING, Any, Collection, cast
 
 from starlette import applications
 from starlette.routing import Match
@@ -184,18 +186,29 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A
 from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
 from opentelemetry.instrumentation.starlette.package import _instruments
 from opentelemetry.instrumentation.starlette.version import __version__
-from opentelemetry.metrics import get_meter
+from opentelemetry.metrics import MeterProvider, get_meter
 from opentelemetry.semconv.trace import SpanAttributes
-from opentelemetry.trace import get_tracer
+from opentelemetry.trace import TracerProvider, get_tracer
 from opentelemetry.util.http import get_excluded_urls
 
+if TYPE_CHECKING:
+    from typing import TypedDict, Unpack
+
+    class InstrumentKwargs(TypedDict, total=False):
+        tracer_provider: TracerProvider
+        meter_provider: MeterProvider
+        server_request_hook: ServerRequestHook
+        client_request_hook: ClientRequestHook
+        client_response_hook: ClientResponseHook
+
+
 _excluded_urls = get_excluded_urls("STARLETTE")
 
 
 class StarletteInstrumentor(BaseInstrumentor):
-    """An instrumentor for starlette
+    """An instrumentor for Starlette.
 
-    See `BaseInstrumentor`
+    See `BaseInstrumentor`.
     """
 
     _original_starlette = None
@@ -206,8 +219,8 @@ def instrument_app(
         server_request_hook: ServerRequestHook = None,
         client_request_hook: ClientRequestHook = None,
         client_response_hook: ClientResponseHook = None,
-        meter_provider=None,
-        tracer_provider=None,
+        meter_provider: MeterProvider | None = None,
+        tracer_provider: TracerProvider | None = None,
     ):
         """Instrument an uninstrumented Starlette application."""
         tracer = get_tracer(
@@ -253,7 +266,7 @@ def uninstrument_app(app: applications.Starlette):
     def instrumentation_dependencies(self) -> Collection[str]:
         return _instruments
 
-    def _instrument(self, **kwargs):
+    def _instrument(self, **kwargs: Unpack[InstrumentKwargs]):
         self._original_starlette = applications.Starlette
         _InstrumentedStarlette._tracer_provider = kwargs.get("tracer_provider")
         _InstrumentedStarlette._server_request_hook = kwargs.get(
@@ -269,7 +282,7 @@ def _instrument(self, **kwargs):
 
         applications.Starlette = _InstrumentedStarlette
 
-    def _uninstrument(self, **kwargs):
+    def _uninstrument(self, **kwargs: Any):
         """uninstrumenting all created apps by user"""
         for instance in _InstrumentedStarlette._instrumented_starlette_apps:
             self.uninstrument_app(instance)
@@ -278,14 +291,14 @@ def _uninstrument(self, **kwargs):
 
 
 class _InstrumentedStarlette(applications.Starlette):
-    _tracer_provider = None
-    _meter_provider = None
+    _tracer_provider: TracerProvider | None = None
+    _meter_provider: MeterProvider | None = None
     _server_request_hook: ServerRequestHook = None
     _client_request_hook: ClientRequestHook = None
     _client_response_hook: ClientResponseHook = None
-    _instrumented_starlette_apps = set()
+    _instrumented_starlette_apps: set[applications.Starlette] = set()
 
-    def __init__(self, *args, **kwargs):
+    def __init__(self, *args: Any, **kwargs: Any):
         super().__init__(*args, **kwargs)
         tracer = get_tracer(
             __name__,
@@ -318,21 +331,22 @@ def __del__(self):
         _InstrumentedStarlette._instrumented_starlette_apps.remove(self)
 
 
-def _get_route_details(scope):
+def _get_route_details(scope: dict[str, Any]) -> str | None:
     """
-    Function to retrieve Starlette route from scope.
+    Function to retrieve Starlette route from ASGI scope.
 
     TODO: there is currently no way to retrieve http.route from
     a starlette application from scope.
     See: https://github.com/encode/starlette/pull/804
 
     Args:
-        scope: A Starlette scope
+        scope: The ASGI scope that contains the Starlette application in the "app" key.
+
     Returns:
-        A string containing the route or None
+        The path to the route if found, otherwise None.
     """
-    app = scope["app"]
-    route = None
+    app = cast(applications.Starlette, scope["app"])
+    route: str | None = None
 
     for starlette_route in app.routes:
         match, _ = starlette_route.matches(scope)
@@ -344,18 +358,20 @@ def _get_route_details(scope):
     return route
 
 
-def _get_default_span_details(scope):
-    """
-    Callback to retrieve span name and attributes from scope.
+def _get_default_span_details(
+    scope: dict[str, Any],
+) -> tuple[str, dict[str, Any]]:
+    """Callback to retrieve span name and attributes from ASGI scope.
 
     Args:
-        scope: A Starlette scope
+        scope: The ASGI scope that contains the Starlette application in the "app" key.
+
     Returns:
-        A tuple of span name and attributes
+        A tuple of span name and attributes.
     """
     route = _get_route_details(scope)
-    method = scope.get("method", "")
-    attributes = {}
+    method: str = scope.get("method", "")
+    attributes: dict[str, Any] = {}
     if route:
         attributes[SpanAttributes.HTTP_ROUTE] = route
     if method and route:  # http
diff --git a/rtd-requirements.txt b/rtd-requirements.txt
@@ -0,0 +1,7 @@
+# Need to install the api/sdk in the venv for autodoc. Used by .readthedocs.yml
+opentelemetry-api @ git+https://github.com/open-telemetry/opentelemetry-python.git@main#egg=opentelemetry-api&subdirectory=opentelemetry-api
+opentelemetry-semantic-conventions @ git+https://github.com/open-telemetry/opentelemetry-python.git@main#egg=opentelemetry-semantic-conventions&subdirectory=opentelemetry-semantic-conventions
+opentelemetry-sdk @ git+https://github.com/open-telemetry/opentelemetry-python.git@main#egg=opentelemetry-sdk&subdirectory=opentelemetry-sdk
+./opentelemetry-instrumentation
+./util/opentelemetry-util-http
+-r ./docs-requirements.txt
