Merge branch 'main' into TID-rule

Author: JayeshHire

CHANGELOG.md

@@ -15,6 +15,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Bump `pylint` to `4.0.5`
   ([#4244](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4244))
+- `opentelemetry-instrumentation-sqlite3`: Add uninstrument, error status, suppress, and no-op tests
+  ([#4335](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4335))
+
+### Fixed
+
+- `opentelemetry-instrumentation-pika` Use `ObjectProxy` instead of `BaseObjectProxy` for `ReadyMessagesDequeProxy` to restore iterability with wrapt 2.x
+  ([#4461](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4461))
+- `opentelemetry-instrumentation-dbapi` Use `ObjectProxy` instead of `BaseObjectProxy` for `TracedCursorProxy` to restore iterability with wrapt 2.x
+  ([#4427](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4427))
+- `opentelemetry-instrumentation-flask`: Clean up environ keys in `_teardown_request` to prevent duplicate execution
+  ([#4341](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4341))
 
 ### Breaking changes
 
@@ -42,6 +53,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- `opentelemetry-instrumentation-celery`: Coerce non-string values to strings in `CeleryGetter.get()` to prevent `TypeError` in `TraceState.from_header()` when Celery request attributes contain ints
+  ([#4360](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4360))
 - `opentelemetry-docker-tests`: Replace deprecated `SpanAttributes` from `opentelemetry.semconv.trace` with `opentelemetry.semconv._incubating.attributes`
  ([#4339](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4339))
 - `opentelemetry-instrumentation-confluent-kafka`: Skip `recv` span creation when `poll()` returns no message or `consume()` returns an empty list, avoiding empty spans on idle polls

docs-requirements.txt

@@ -49,6 +49,7 @@ sqlalchemy>=1.0
 starlette~=0.50
 tornado>=5.1.1
 tortoise-orm>=0.17.0
+wrapt~=2.1
 
 # required by opamp
 uuid_utils

docs/nitpick-exceptions.ini

@@ -44,6 +44,7 @@ py-class=
     psycopg.Connection
     psycopg.AsyncConnection
     ObjectProxy
+    wrapt.proxies.ObjectProxy
     fastapi.applications.FastAPI
     starlette.applications.Starlette
     _contextvars.Token

instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py

@@ -104,8 +104,18 @@ def get(self, carrier, key):
         value = getattr(carrier, key, None)
         if value is None:
             return None
-        if isinstance(value, str) or not isinstance(value, Iterable):
+        # Celery's Context copies all message properties as instance
+        # attributes, including non-string values like timelimit (tuple
+        # of ints).  The TextMapPropagator contract requires string
+        # values, so coerce anything that isn't already a string.
+        if isinstance(value, str):
             value = (value,)
+        elif isinstance(value, Iterable):
+            value = tuple(
+                str(v) if not isinstance(v, str) else v for v in value
+            )
+        else:
+            value = (str(value),)
         return value
 
     def keys(self, carrier):

instrumentation/opentelemetry-instrumentation-celery/tests/test_getter.py

@@ -36,7 +36,40 @@ def test_get_iter(self):
         getter = CeleryGetter()
         mock_obj.test = ["val"]
         val = getter.get(mock_obj, "test")
-        self.assertEqual(val, ["val"])
+        self.assertEqual(val, ("val",))
+
+    def test_get_int(self):
+        """Non-string scalar values should be coerced to strings.
+
+        Celery's Context stores some attributes as ints (e.g. priority).
+        The TextMapPropagator contract requires string values; passing
+        an int to re.split() in TraceState.from_header() causes a
+        TypeError.
+        """
+        mock_obj = mock.Mock()
+        getter = CeleryGetter()
+        mock_obj.test = 42
+        val = getter.get(mock_obj, "test")
+        self.assertEqual(val, ("42",))
+
+    def test_get_iter_with_non_string_elements(self):
+        """Iterable values containing non-strings should be coerced.
+
+        Celery's timelimit attribute is a tuple of ints, e.g. (300, 60).
+        """
+        mock_obj = mock.Mock()
+        getter = CeleryGetter()
+        mock_obj.test = (300, 60)
+        val = getter.get(mock_obj, "test")
+        self.assertEqual(val, ("300", "60"))
+
+    def test_get_iter_with_mixed_types(self):
+        """Iterables with a mix of strings and non-strings."""
+        mock_obj = mock.Mock()
+        getter = CeleryGetter()
+        mock_obj.test = ["val", 123]
+        val = getter.get(mock_obj, "test")
+        self.assertEqual(val, ("val", "123"))
 
     def test_keys(self):
         getter = CeleryGetter()

instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py

@@ -177,8 +177,12 @@
 
 try:
     # wrapt 2.0.0+
-    from wrapt import BaseObjectProxy  # pylint: disable=no-name-in-module
+    from wrapt import (  # pylint: disable=no-name-in-module
+        BaseObjectProxy,
+        ObjectProxy,
+    )
 except ImportError:
+    from wrapt import ObjectProxy
     from wrapt import ObjectProxy as BaseObjectProxy
 
 from opentelemetry import trace as trace_api
@@ -805,14 +809,14 @@ async def traced_execution_async(
 
 
 # pylint: disable=abstract-method,no-member
-class TracedCursorProxy(BaseObjectProxy, Generic[CursorT]):
+class TracedCursorProxy(ObjectProxy, Generic[CursorT]):
     # pylint: disable=unused-argument
     def __init__(
         self,
         cursor: CursorT,
         db_api_integration: DatabaseApiIntegration,
     ):
-        BaseObjectProxy.__init__(self, cursor)
+        ObjectProxy.__init__(self, cursor)
         self._self_cursor_tracer = CursorTracer[CursorT](db_api_integration)
 
     def execute(self, *args: Any, **kwargs: Any):

instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py

@@ -306,6 +306,20 @@ def test_executemany(self):
         span = spans_list[0]
         self.assertEqual(span.attributes[DB_STATEMENT], "Test query")
 
+    # pylint: disable=no-self-use
+    def test_executemany_iterable_cursor(self):
+        db_integration = dbapi.DatabaseApiIntegration(
+            "instrumenting_module_test_name", "testcomponent"
+        )
+        mock_connection = db_integration.wrapped_connection(
+            mock_connect, {}, {}
+        )
+        cursor = mock_connection.cursor()
+        cursor.executemany("Test query")
+
+        for _row in cursor:
+            pass
+
     def test_executemany_comment(self):
         connect_module = mock.MagicMock()
         connect_module.__name__ = "test"
@@ -1296,13 +1310,17 @@ def __init__(self) -> None:
         self._cnx._cmysql.get_client_info = mock.MagicMock(
             return_value="1.2.3"
         )
+        self._items = []
 
     # pylint: disable=unused-argument, no-self-use
     def execute(self, query, params=None, throw_exception=False):
         if throw_exception:
             # pylint: disable=broad-exception-raised
             raise Exception("Test Exception")
 
+    def __iter__(self):
+        yield from self._items
+
     # pylint: disable=unused-argument, no-self-use
     def executemany(self, query, params=None, throw_exception=False):
         if throw_exception:

instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py

@@ -636,6 +636,8 @@ def _teardown_request(exc):
 
             if token:
                 context.detach(token)
+                flask.request.environ.pop(_ENVIRON_ACTIVATION_KEY, None)
+                flask.request.environ.pop(_ENVIRON_TOKEN, None)
 
         except (RuntimeError, AttributeError, ValueError) as teardown_exc:
             # Log the error but don't raise it to avoid breaking the request handling

instrumentation/opentelemetry-instrumentation-flask/tests/test_flask_compatibility.py

@@ -26,6 +26,8 @@
 
 from opentelemetry import trace
 from opentelemetry.instrumentation.flask import (
+    _ENVIRON_ACTIVATION_KEY,
+    _ENVIRON_TOKEN,
     FlaskInstrumentor,
     _request_ctx_ref,
 )
@@ -354,3 +356,59 @@ def generate():
         # This ensures OpenTelemetry contexts are properly cleaned up for streaming responses
         # following Logfire's recommendations (see open-telemetry/opentelemetry-python#2606)
         # If we reach this point, the Flask 3.1+ streaming context cleanup is working
+
+    def test_duplicate_teardown_request_does_not_cause_errors(self):
+        """Test that _teardown_request can be called multiple times without errors.
+
+        Flask may call teardown_request multiple times in some scenarios.
+        After the first call, _ENVIRON_ACTIVATION_KEY and _ENVIRON_TOKEN
+        should be cleaned up so subsequent calls are no-ops.
+        """
+        app = flask.Flask(__name__)
+
+        # Register the check handler BEFORE instrumenting so it runs AFTER
+        # the instrumentation's teardown (Flask uses LIFO order).
+        cleaned_up = {}
+        call_count = {"teardown_calls": 0}
+
+        @app.teardown_request
+        def check_cleanup(exc):
+            cleaned_up["activation_present"] = (
+                _ENVIRON_ACTIVATION_KEY in flask.request.environ
+            )
+            cleaned_up["token_present"] = (
+                _ENVIRON_TOKEN in flask.request.environ
+            )
+
+        FlaskInstrumentor().instrument_app(app)
+
+        # Wrap the instrumentation's teardown to count calls and invoke it
+        # a second time to simulate duplicate teardown.
+        instrumentation_teardown = app.teardown_request_funcs[None][-1]
+
+        def counting_teardown(exc):
+            call_count["teardown_calls"] += 1
+            instrumentation_teardown(exc)
+            # Call it again to simulate duplicate teardown - should not raise
+            instrumentation_teardown(exc)
+
+        app.teardown_request_funcs[None][-1] = counting_teardown
+
+        @app.route("/test")
+        def test_endpoint():
+            return "OK"
+
+        client = app.test_client()
+        response = client.get("/test")
+        self.assertEqual(response.status_code, 200)
+        # Verify the teardown was actually called
+        self.assertGreater(call_count["teardown_calls"], 0)
+        # Verify env keys are cleaned up after teardown
+        self.assertFalse(
+            cleaned_up.get("activation_present", True),
+            "_ENVIRON_ACTIVATION_KEY should be cleaned up after teardown",
+        )
+        self.assertFalse(
+            cleaned_up.get("token_present", True),
+            "_ENVIRON_TOKEN should be cleaned up after teardown",
+        )

instrumentation/opentelemetry-instrumentation-pika/src/opentelemetry/instrumentation/pika/utils.py

@@ -7,12 +7,7 @@
 )
 from pika.channel import Channel
 from pika.spec import Basic, BasicProperties
-
-try:
-    # wrapt 2.0.0+
-    from wrapt import BaseObjectProxy  # pylint: disable=no-name-in-module
-except ImportError:
-    from wrapt import ObjectProxy as BaseObjectProxy
+from wrapt import ObjectProxy
 
 from opentelemetry import context, propagate, trace
 from opentelemetry.instrumentation.utils import is_instrumentation_enabled
@@ -201,7 +196,7 @@ def _enrich_span(
 
 
 # pylint:disable=abstract-method
-class ReadyMessagesDequeProxy(BaseObjectProxy):
+class ReadyMessagesDequeProxy(ObjectProxy):
     def __init__(
         self,
         wrapped,