Merge branch 'main' into instrument-connection-optional-connect-module

Author: tammy-baylis-swi

CHANGELOG.md

@@ -17,11 +17,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#2976](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2976))
   - Add `opentelemetry-instrumentation-openai-v2` to `opentelemetry-bootstrap`
   ([#2996](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2996))
+  - `opentelemetry-instrumentation-sqlalchemy` Add sqlcomment to `db.statement` attribute
+  ([#2937](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2937))
+  - `opentelemetry-instrumentation-dbapi` Add sqlcomment to `db.statement` attribute
+  ([#2935](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2935))
 
 ### Fixed
 
 - `opentelemetry-instrumentation-httpx`: instrument_client is a static method again
   ([#3003](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3003))
+- `opentelemetry-instrumentation-httpx`: Check if mount transport is none before wrap it
+  ([#3022](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3022))
 
 ### Breaking changes
 

instrumentation/opentelemetry-instrumentation-confluent-kafka/README.rst

@@ -19,5 +19,5 @@ Installation
 References
 ----------
 
-* `OpenTelemetry confluent-kafka/ Tracing <https://opentelemetry-python-contrib.readthedocs.io/en/latest/instrumentation/confluent-kafka/confluent-kafka.html>`_
+* `OpenTelemetry confluent-kafka/ Tracing <https://opentelemetry-python-contrib.readthedocs.io/en/latest/instrumentation/confluent_kafka/confluent_kafka.html>`_
 * `OpenTelemetry Project <https://opentelemetry.io/>`_

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

@@ -495,49 +495,54 @@ def traced_execution(
         with self._db_api_integration._tracer.start_as_current_span(
             name, kind=SpanKind.CLIENT
         ) as span:
-            self._populate_span(span, cursor, *args)
-            if args and self._commenter_enabled:
-                try:
-                    args_list = list(args)
-
-                    # lazy capture of mysql-connector client version using cursor
-                    if (
-                        self._db_api_integration.database_system == "mysql"
-                        and self._db_api_integration.connect_module.__name__
-                        == "mysql.connector"
-                        and not self._db_api_integration.commenter_data[
-                            "mysql_client_version"
-                        ]
-                    ):
-                        self._db_api_integration.commenter_data[
-                            "mysql_client_version"
-                        ] = cursor._cnx._cmysql.get_client_info()
-
-                    commenter_data = dict(
-                        self._db_api_integration.commenter_data
-                    )
-                    if self._commenter_options.get(
-                        "opentelemetry_values", True
-                    ):
-                        commenter_data.update(**_get_opentelemetry_values())
-
-                    # Filter down to just the requested attributes.
-                    commenter_data = {
-                        k: v
-                        for k, v in commenter_data.items()
-                        if self._commenter_options.get(k, True)
-                    }
-                    statement = _add_sql_comment(
-                        args_list[0], **commenter_data
-                    )
-
-                    args_list[0] = statement
-                    args = tuple(args_list)
-
-                except Exception as exc:  # pylint: disable=broad-except
-                    _logger.exception(
-                        "Exception while generating sql comment: %s", exc
-                    )
+            if span.is_recording():
+                if args and self._commenter_enabled:
+                    try:
+                        args_list = list(args)
+
+                        # lazy capture of mysql-connector client version using cursor
+                        if (
+                            self._db_api_integration.database_system == "mysql"
+                            and self._db_api_integration.connect_module.__name__
+                            == "mysql.connector"
+                            and not self._db_api_integration.commenter_data[
+                                "mysql_client_version"
+                            ]
+                        ):
+                            self._db_api_integration.commenter_data[
+                                "mysql_client_version"
+                            ] = cursor._cnx._cmysql.get_client_info()
+
+                        commenter_data = dict(
+                            self._db_api_integration.commenter_data
+                        )
+                        if self._commenter_options.get(
+                            "opentelemetry_values", True
+                        ):
+                            commenter_data.update(
+                                **_get_opentelemetry_values()
+                            )
+
+                        # Filter down to just the requested attributes.
+                        commenter_data = {
+                            k: v
+                            for k, v in commenter_data.items()
+                            if self._commenter_options.get(k, True)
+                        }
+                        statement = _add_sql_comment(
+                            args_list[0], **commenter_data
+                        )
+
+                        args_list[0] = statement
+                        args = tuple(args_list)
+
+                    except Exception as exc:  # pylint: disable=broad-except
+                        _logger.exception(
+                            "Exception while generating sql comment: %s", exc
+                        )
+
+                self._populate_span(span, cursor, *args)
+
             return query_method(*args, **kwargs)
 
 

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

@@ -14,6 +14,7 @@
 
 
 import logging
+import re
 from unittest import mock
 
 from opentelemetry import context
@@ -306,6 +307,44 @@ def __getattr__(self, name):
             r"Select 1 /\*dbapi_level='1.0',dbapi_threadsafety='unknown',driver_paramstyle='unknown',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;",
         )
 
+    def test_executemany_comment_matches_db_statement_attribute(self):
+        connect_module = mock.MagicMock()
+        connect_module.__version__ = mock.MagicMock()
+        connect_module.__libpq_version__ = 123
+        connect_module.apilevel = 123
+        connect_module.threadsafety = 123
+        connect_module.paramstyle = "test"
+
+        db_integration = dbapi.DatabaseApiIntegration(
+            "testname",
+            "postgresql",
+            enable_commenter=True,
+            commenter_options={"db_driver": False, "dbapi_level": False},
+            connect_module=connect_module,
+        )
+        mock_connection = db_integration.wrapped_connection(
+            mock_connect, {}, {}
+        )
+        cursor = mock_connection.cursor()
+        cursor.executemany("Select 1;")
+        self.assertRegex(
+            cursor.query,
+            r"Select 1 /\*dbapi_threadsafety=123,driver_paramstyle='test',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;",
+        )
+        spans_list = self.memory_exporter.get_finished_spans()
+        self.assertEqual(len(spans_list), 1)
+        span = spans_list[0]
+        self.assertRegex(
+            span.attributes[SpanAttributes.DB_STATEMENT],
+            r"Select 1 /\*dbapi_threadsafety=123,driver_paramstyle='test',libpq_version=123,traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/",
+        )
+
+        cursor_span_id = re.search(r"[a-zA-Z0-9_]{16}", cursor.query).group()
+        db_statement_span_id = re.search(
+            r"[a-zA-Z0-9_]{16}", span.attributes[SpanAttributes.DB_STATEMENT]
+        ).group()
+        self.assertEqual(cursor_span_id, db_statement_span_id)
+
     def test_compatible_build_version_psycopg_psycopg2_libpq(self):
         connect_module = mock.MagicMock()
         connect_module.__name__ = "test"

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

@@ -1005,17 +1005,18 @@ def instrument_client(
                 ),
             )
             for transport in client._mounts.values():
-                wrap_function_wrapper(
-                    transport,
-                    "handle_request",
-                    partial(
-                        cls._handle_request_wrapper,
-                        tracer=tracer,
-                        sem_conv_opt_in_mode=sem_conv_opt_in_mode,
-                        request_hook=request_hook,
-                        response_hook=response_hook,
-                    ),
-                )
+                if hasattr(transport, "handle_request"):
+                    wrap_function_wrapper(
+                        transport,
+                        "handle_request",
+                        partial(
+                            cls._handle_request_wrapper,
+                            tracer=tracer,
+                            sem_conv_opt_in_mode=sem_conv_opt_in_mode,
+                            request_hook=request_hook,
+                            response_hook=response_hook,
+                        ),
+                    )
             client._is_instrumented_by_opentelemetry = True
         if hasattr(client._transport, "handle_async_request"):
             wrap_function_wrapper(
@@ -1030,17 +1031,18 @@ def instrument_client(
                 ),
             )
             for transport in client._mounts.values():
-                wrap_function_wrapper(
-                    transport,
-                    "handle_async_request",
-                    partial(
-                        cls._handle_async_request_wrapper,
-                        tracer=tracer,
-                        sem_conv_opt_in_mode=sem_conv_opt_in_mode,
-                        async_request_hook=async_request_hook,
-                        async_response_hook=async_response_hook,
-                    ),
-                )
+                if hasattr(transport, "handle_async_request"):
+                    wrap_function_wrapper(
+                        transport,
+                        "handle_async_request",
+                        partial(
+                            cls._handle_async_request_wrapper,
+                            tracer=tracer,
+                            sem_conv_opt_in_mode=sem_conv_opt_in_mode,
+                            async_request_hook=async_request_hook,
+                            async_response_hook=async_response_hook,
+                        ),
+                    )
             client._is_instrumented_by_opentelemetry = True
 
     @staticmethod

instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py

@@ -741,6 +741,10 @@ def create_client(
         def create_proxy_transport(self, url: str):
             pass
 
+        @abc.abstractmethod
+        def get_transport_handler(self, transport):
+            pass
+
         def setUp(self):
             super().setUp()
             self.client = self.create_client()
@@ -763,17 +767,15 @@ def assert_proxy_mounts(self, mounts, num_mounts, transport_type=None):
             self.assertEqual(len(mounts), num_mounts)
             for transport in mounts:
                 with self.subTest(transport):
+                    if transport is None:
+                        continue
                     if transport_type:
                         self.assertIsInstance(
                             transport,
                             transport_type,
                         )
                     else:
-                        handler = getattr(transport, "handle_request", None)
-                        if not handler:
-                            handler = getattr(
-                                transport, "handle_async_request"
-                            )
+                        handler = self.get_transport_handler(transport)
                         self.assertTrue(
                             isinstance(handler, ObjectProxy)
                             and getattr(handler, "__wrapped__")
@@ -983,6 +985,21 @@ def test_uninstrument_new_client(self):
             self.assertEqual(result.text, "Hello!")
             self.assert_span()
 
+        @mock.patch.dict(
+            "os.environ", {"NO_PROXY": "http://mock/status/200"}, clear=True
+        )
+        def test_instrument_with_no_proxy(self):
+            proxy_mounts = self.create_proxy_mounts()
+            HTTPXClientInstrumentor().instrument()
+            client = self.create_client(mounts=proxy_mounts)
+            result = self.perform_request(self.URL, client=client)
+            self.assert_span(num_spans=1)
+            self.assertEqual(result.text, "Hello!")
+            self.assert_proxy_mounts(
+                client._mounts.values(),
+                3,
+            )
+
         def test_instrument_proxy(self):
             proxy_mounts = self.create_proxy_mounts()
             HTTPXClientInstrumentor().instrument()
@@ -994,6 +1011,27 @@ def test_instrument_proxy(self):
                 2,
             )
 
+        @mock.patch.dict(
+            "os.environ", {"NO_PROXY": "http://mock/status/200"}, clear=True
+        )
+        def test_instrument_client_with_no_proxy(self):
+            proxy_mounts = self.create_proxy_mounts()
+            client = self.create_client(mounts=proxy_mounts)
+            self.assert_proxy_mounts(
+                client._mounts.values(),
+                3,
+                (httpx.HTTPTransport, httpx.AsyncHTTPTransport),
+            )
+            HTTPXClientInstrumentor.instrument_client(client)
+            result = self.perform_request(self.URL, client=client)
+            self.assertEqual(result.text, "Hello!")
+            self.assert_span(num_spans=1)
+            self.assert_proxy_mounts(
+                client._mounts.values(),
+                3,
+            )
+            HTTPXClientInstrumentor.uninstrument_client(client)
+
         def test_instrument_client_with_proxy(self):
             proxy_mounts = self.create_proxy_mounts()
             client = self.create_client(mounts=proxy_mounts)
@@ -1188,6 +1226,9 @@ def perform_request(
     def create_proxy_transport(self, url):
         return httpx.HTTPTransport(proxy=httpx.Proxy(url))
 
+    def get_transport_handler(self, transport):
+        return getattr(transport, "handle_request", None)
+
     def test_can_instrument_subclassed_client(self):
         class CustomClient(httpx.Client):
             pass
@@ -1241,6 +1282,9 @@ async def _perform_request():
     def create_proxy_transport(self, url):
         return httpx.AsyncHTTPTransport(proxy=httpx.Proxy(url))
 
+    def get_transport_handler(self, transport):
+        return getattr(transport, "handle_async_request", None)
+
     def test_basic_multiple(self):
         # We need to create separate clients because in httpx >= 0.19,
         # closing the client after "with" means the second http call fails

instrumentation/opentelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py

@@ -219,28 +219,31 @@ def _before_cur_exec(
         )
         with trace.use_span(span, end_on_exit=False):
             if span.is_recording():
+                if self.enable_commenter:
+                    commenter_data = {
+                        "db_driver": conn.engine.driver,
+                        # Driver/framework centric information.
+                        "db_framework": f"sqlalchemy:{sqlalchemy.__version__}",
+                    }
+
+                    if self.commenter_options.get(
+                        "opentelemetry_values", True
+                    ):
+                        commenter_data.update(**_get_opentelemetry_values())
+
+                    # Filter down to just the requested attributes.
+                    commenter_data = {
+                        k: v
+                        for k, v in commenter_data.items()
+                        if self.commenter_options.get(k, True)
+                    }
+
+                    statement = _add_sql_comment(statement, **commenter_data)
+
                 span.set_attribute(SpanAttributes.DB_STATEMENT, statement)
                 span.set_attribute(SpanAttributes.DB_SYSTEM, self.vendor)
                 for key, value in attrs.items():
                     span.set_attribute(key, value)
-            if self.enable_commenter:
-                commenter_data = {
-                    "db_driver": conn.engine.driver,
-                    # Driver/framework centric information.
-                    "db_framework": f"sqlalchemy:{sqlalchemy.__version__}",
-                }
-
-                if self.commenter_options.get("opentelemetry_values", True):
-                    commenter_data.update(**_get_opentelemetry_values())
-
-                # Filter down to just the requested attributes.
-                commenter_data = {
-                    k: v
-                    for k, v in commenter_data.items()
-                    if self.commenter_options.get(k, True)
-                }
-
-                statement = _add_sql_comment(statement, **commenter_data)
 
         context._otel_span = span