opentelemetry-instrumentation-dbapi: fix libpq version detection

This fixes a crash in the detection of libpq version in the psycopg module
when we wrap a module that is not the one containing the pq module that
has the version.

Author: xrmx

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

@@ -350,11 +350,25 @@ def calculate_commenter_data(self) -> dict[str, Any]:
         }
 
         if self.database_system == "postgresql":
-            if hasattr(self.connect_module, "__libpq_version__"):
-                libpq_version = self.connect_module.__libpq_version__
-            else:
-                libpq_version = self.connect_module.pq.__build_version__
-            commenter_data.update({"libpq_version": libpq_version})
+            libpq_version = None
+            # psycopg
+            if hasattr(self.connect_module, "pq"):
+                try:
+                    libpq_version = self.connect_module.pq.version()
+                except Exception:
+                    pass
+
+            # psycopg2
+            if libpq_version is None:
+                # this the libpq version the client has been built against
+                libpq_version = getattr(
+                    self.connect_module, "__libpq_version__", None
+                )
+
+            # psycopg also instrument modules that are not the root one, in that case you
+            # won't get the libpq_version
+            if libpq_version is not None:
+                commenter_data.update({"libpq_version": libpq_version})
         elif self.database_system == "mysql":
             mysqlc_version = ""
             if db_driver == "MySQLdb":

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

@@ -456,12 +456,11 @@ def test_executemany_comment_stmt_enabled_matches_db_statement_attribute(
         ).group()
         self.assertEqual(cursor_span_id, db_statement_span_id)
 
-    def test_compatible_build_version_psycopg_psycopg2_libpq(self):
+    def test_compatible_build_version_psycopg2_libpq(self):
         connect_module = mock.MagicMock()
         connect_module.__name__ = "test"
         connect_module.__version__ = mock.MagicMock()
-        connect_module.pq = mock.MagicMock()
-        connect_module.pq.__build_version__ = 123
+        connect_module.__libpq_version__ = 123
         connect_module.apilevel = 123
         connect_module.threadsafety = 123
         connect_module.paramstyle = "test"
@@ -490,14 +489,46 @@ def test_compatible_build_version_psycopg_psycopg2_libpq(self):
             "Select 1;",
         )
 
-    def test_compatible_build_version_psycopg_psycopg2_libpq_stmt_enabled(
+    def test_compatible_build_version_psycopg_libpq(self):
+        connect_module = mock.MagicMock()
+        connect_module.__name__ = "test"
+        connect_module.__version__ = mock.MagicMock()
+        connect_module.pq.version.return_value = 123
+        connect_module.apilevel = 123
+        connect_module.threadsafety = 123
+        connect_module.paramstyle = "test"
+
+        db_integration = dbapi.DatabaseApiIntegration(
+            "instrumenting_module_test_name",
+            "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.assertEqual(
+            span.attributes[SpanAttributes.DB_STATEMENT],
+            "Select 1;",
+        )
+
+    def test_no_build_version_where_we_dont_instrument_the_root_module(
         self,
     ):
         connect_module = mock.MagicMock()
         connect_module.__name__ = "test"
         connect_module.__version__ = mock.MagicMock()
         connect_module.pq = mock.MagicMock()
-        connect_module.pq.__build_version__ = 123
         connect_module.apilevel = 123
         connect_module.threadsafety = 123
         connect_module.paramstyle = "test"
@@ -517,14 +548,14 @@ def test_compatible_build_version_psycopg_psycopg2_libpq_stmt_enabled(
         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}'\*/;",
+            r"Select 1 /\*dbapi_threadsafety=123,driver_paramstyle='test',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}'\*/;",
+            r"Select 1 /\*dbapi_threadsafety=123,driver_paramstyle='test',traceparent='\d{1,2}-[a-zA-Z0-9_]{32}-[a-zA-Z0-9_]{16}-\d{1,2}'\*/;",
         )
 
     def test_executemany_psycopg2_integration_comment(self):