From a737fc72101a96f27bde01e68bbe4b1e9cfe8128 Mon Sep 17 00:00:00 2001 From: Keith Zmudzinski Date: Thu, 12 Sep 2024 14:02:31 -0700 Subject: [PATCH 01/16] feat: augment span name, add span attributes --- .../instrumentation/dbapi/__init__.py | 37 ++++++++++++++----- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index 0857d2989b..d0890c9650 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -52,6 +52,7 @@ unwrap, ) from opentelemetry.semconv.trace import SpanAttributes +from opentelemetry.semconv._incubating.attributes.db_attributes import DB_COLLECTION_NAME from opentelemetry.trace import SpanKind, TracerProvider, get_tracer _logger = logging.getLogger(__name__) @@ -284,12 +285,16 @@ def wrapped_connection( ): """Add object proxy to connection object.""" connection = connect_method(*args, **kwargs) - self.get_connection_attributes(connection) + self.get_connection_attributes(kwargs, connection) return get_traced_connection_proxy(connection, self) - def get_connection_attributes(self, connection): - # Populate span fields using connection + def get_connection_attributes(self, kwargs, connection): + # Populate span fields using kwargs and connection for key, value in self.connection_attributes.items(): + # First set from kwargs + self.connection_props[key] = kwargs.get(value) + + # Then override from connection object # Allow attributes nested in connection object attribute = functools.reduce( lambda attribute, attribute_value: getattr( @@ -381,6 +386,7 @@ def _populate_span( SpanAttributes.DB_NAME, self._db_api_integration.database ) span.set_attribute(SpanAttributes.DB_STATEMENT, statement) + span.set_attribute(DB_COLLECTION_NAME, self.get_collection_name(statement)) for ( attribute_key, @@ -391,11 +397,23 @@ def _populate_span( if self._db_api_integration.capture_parameters and len(args) > 1: span.set_attribute("db.statement.parameters", str(args[1])) - def get_operation_name(self, cursor, args): # pylint: disable=no-self-use - if args and isinstance(args[0], str): - # Strip leading comments so we get the operation name. - return self._leading_comment_remover.sub("", args[0]).split()[0] - return "" + def get_span_name(self, statement): + operation_name = self.get_operation_name(statement) + collection_name = CursorTracer.get_collection_name(statement) + return " ".join(name for name in (operation_name, collection_name) if name) + + def get_operation_name(self, statement): + # Strip leading comments so we get the operation name. + return self._leading_comment_remover.sub("", statement).split()[0] + + @staticmethod + def get_collection_name(statement): + collection_name = "" + match = re.search(r"\b(?:FROM|JOIN|INTO|UPDATE|TABLE)\s+([\w`']+)", statement) + if match: + collection_name = match.group(1).strip('`\'') + + return collection_name def get_statement(self, cursor, args): # pylint: disable=no-self-use if not args: @@ -412,7 +430,8 @@ def traced_execution( *args: typing.Tuple[typing.Any, typing.Any], **kwargs: typing.Dict[typing.Any, typing.Any], ): - name = self.get_operation_name(cursor, args) + statement = self.get_statement(cursor, args) + name = self.get_span_name(statement) if not name: name = ( self._db_api_integration.database From b54bbca1c7eeb1cf744ddb54bd7c918f10fb2801 Mon Sep 17 00:00:00 2001 From: Keith Zmudzinski Date: Thu, 12 Sep 2024 14:19:19 -0700 Subject: [PATCH 02/16] chore: only add attributes if they are present --- .../src/opentelemetry/instrumentation/dbapi/__init__.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index d0890c9650..7aa8e6db12 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -292,7 +292,8 @@ def get_connection_attributes(self, kwargs, connection): # Populate span fields using kwargs and connection for key, value in self.connection_attributes.items(): # First set from kwargs - self.connection_props[key] = kwargs.get(value) + if value in kwargs: + self.connection_props[key] = kwargs.get(value) # Then override from connection object # Allow attributes nested in connection object @@ -378,7 +379,10 @@ def _populate_span( ): if not span.is_recording(): return + statement = self.get_statement(cursor, args) + collection_name = self.get_collection_name(statement) + span.set_attribute( SpanAttributes.DB_SYSTEM, self._db_api_integration.database_system ) @@ -386,7 +390,8 @@ def _populate_span( SpanAttributes.DB_NAME, self._db_api_integration.database ) span.set_attribute(SpanAttributes.DB_STATEMENT, statement) - span.set_attribute(DB_COLLECTION_NAME, self.get_collection_name(statement)) + if collection_name: + span.set_attribute(DB_COLLECTION_NAME, collection_name) for ( attribute_key, From f2971f45a00a8199360603876fd711db2be0ef55 Mon Sep 17 00:00:00 2001 From: Keith Zmudzinski Date: Thu, 12 Sep 2024 15:31:17 -0700 Subject: [PATCH 03/16] tests: add tests for collection name, span name --- .../instrumentation/dbapi/__init__.py | 6 +++--- .../tests/test_dbapi_integration.py | 15 +++++++++++---- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index 7aa8e6db12..66e7d5400d 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -218,7 +218,7 @@ def instrument_connection( enable_commenter=enable_commenter, commenter_options=commenter_options, ) - db_integration.get_connection_attributes(connection) + db_integration.get_connection_attributes(connection=connection) return get_traced_connection_proxy(connection, db_integration) @@ -285,10 +285,10 @@ def wrapped_connection( ): """Add object proxy to connection object.""" connection = connect_method(*args, **kwargs) - self.get_connection_attributes(kwargs, connection) + self.get_connection_attributes(connection=connection, kwargs=kwargs) return get_traced_connection_proxy(connection, self) - def get_connection_attributes(self, kwargs, connection): + def get_connection_attributes(self, connection, kwargs={}): # Populate span fields using kwargs and connection for key, value in self.connection_attributes.items(): # First set from kwargs diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index eb2d628a3a..f0ed595c80 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -21,6 +21,7 @@ from opentelemetry.instrumentation import dbapi from opentelemetry.sdk import resources from opentelemetry.semconv.trace import SpanAttributes +from opentelemetry.semconv._incubating.attributes.db_attributes import DB_COLLECTION_NAME from opentelemetry.test.test_base import TestBase @@ -49,11 +50,12 @@ def test_span_succeeded(self): mock_connect, {}, connection_props ) cursor = mock_connection.cursor() - cursor.execute("Test query", ("param1Value", False)) + expected_query = "Test query FROM test_table" + cursor.execute(expected_query, ("param1Value", False)) spans_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans_list), 1) span = spans_list[0] - self.assertEqual(span.name, "Test") + self.assertEqual(span.name, "Test test_table") self.assertIs(span.kind, trace_api.SpanKind.CLIENT) self.assertEqual( @@ -63,7 +65,10 @@ def test_span_succeeded(self): span.attributes[SpanAttributes.DB_NAME], "testdatabase" ) self.assertEqual( - span.attributes[SpanAttributes.DB_STATEMENT], "Test query" + span.attributes[SpanAttributes.DB_STATEMENT], expected_query + ) + self.assertEqual( + span.attributes[DB_COLLECTION_NAME], "test_table" ) self.assertFalse("db.statement.parameters" in span.attributes) self.assertEqual(span.attributes[SpanAttributes.DB_USER], "testuser") @@ -91,14 +96,16 @@ def test_span_name(self): cursor.execute("/* leading comment */ query") cursor.execute("/* leading comment */ query /* trailing comment */") cursor.execute("query /* trailing comment */") + cursor.execute("SELECT * FROM test_table") spans_list = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans_list), 6) + self.assertEqual(len(spans_list), 7) self.assertEqual(spans_list[0].name, "Test") self.assertEqual(spans_list[1].name, "multi") self.assertEqual(spans_list[2].name, "tab") self.assertEqual(spans_list[3].name, "query") self.assertEqual(spans_list[4].name, "query") self.assertEqual(spans_list[5].name, "query") + self.assertEqual(spans_list[6].name, "SELECT test_table") def test_span_succeeded_with_capture_of_statement_parameters(self): connection_props = { From e32234e4c7b4dd47a0287baf738c224f07f02331 Mon Sep 17 00:00:00 2001 From: Keith Zmudzinski Date: Thu, 12 Sep 2024 15:38:05 -0700 Subject: [PATCH 04/16] chore: lint --- .../instrumentation/dbapi/__init__.py | 18 ++++++++++++------ .../tests/test_dbapi_integration.py | 8 ++++---- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index 66e7d5400d..1923316e5a 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -51,8 +51,10 @@ _get_opentelemetry_values, unwrap, ) +from opentelemetry.semconv._incubating.attributes.db_attributes import ( + DB_COLLECTION_NAME, +) from opentelemetry.semconv.trace import SpanAttributes -from opentelemetry.semconv._incubating.attributes.db_attributes import DB_COLLECTION_NAME from opentelemetry.trace import SpanKind, TracerProvider, get_tracer _logger = logging.getLogger(__name__) @@ -288,11 +290,11 @@ def wrapped_connection( self.get_connection_attributes(connection=connection, kwargs=kwargs) return get_traced_connection_proxy(connection, self) - def get_connection_attributes(self, connection, kwargs={}): + def get_connection_attributes(self, connection, kwargs=None): # Populate span fields using kwargs and connection for key, value in self.connection_attributes.items(): # First set from kwargs - if value in kwargs: + if kwargs and value in kwargs: self.connection_props[key] = kwargs.get(value) # Then override from connection object @@ -405,7 +407,9 @@ def _populate_span( def get_span_name(self, statement): operation_name = self.get_operation_name(statement) collection_name = CursorTracer.get_collection_name(statement) - return " ".join(name for name in (operation_name, collection_name) if name) + return " ".join( + name for name in (operation_name, collection_name) if name + ) def get_operation_name(self, statement): # Strip leading comments so we get the operation name. @@ -414,9 +418,11 @@ def get_operation_name(self, statement): @staticmethod def get_collection_name(statement): collection_name = "" - match = re.search(r"\b(?:FROM|JOIN|INTO|UPDATE|TABLE)\s+([\w`']+)", statement) + match = re.search( + r"\b(?:FROM|JOIN|INTO|UPDATE|TABLE)\s+([\w`']+)", statement + ) if match: - collection_name = match.group(1).strip('`\'') + collection_name = match.group(1).strip("`'") return collection_name diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index f0ed595c80..7a7650d23e 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -20,8 +20,10 @@ from opentelemetry import trace as trace_api from opentelemetry.instrumentation import dbapi from opentelemetry.sdk import resources +from opentelemetry.semconv._incubating.attributes.db_attributes import ( + DB_COLLECTION_NAME, +) from opentelemetry.semconv.trace import SpanAttributes -from opentelemetry.semconv._incubating.attributes.db_attributes import DB_COLLECTION_NAME from opentelemetry.test.test_base import TestBase @@ -67,9 +69,7 @@ def test_span_succeeded(self): self.assertEqual( span.attributes[SpanAttributes.DB_STATEMENT], expected_query ) - self.assertEqual( - span.attributes[DB_COLLECTION_NAME], "test_table" - ) + self.assertEqual(span.attributes[DB_COLLECTION_NAME], "test_table") self.assertFalse("db.statement.parameters" in span.attributes) self.assertEqual(span.attributes[SpanAttributes.DB_USER], "testuser") self.assertEqual( From 95ed7bbb78db3eac2646aa3946ba4c7ace258a9d Mon Sep 17 00:00:00 2001 From: Keith Zmudzinski Date: Thu, 12 Sep 2024 15:51:18 -0700 Subject: [PATCH 05/16] docs: add to changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07510f643c..f3377eab7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2537](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2537)) - `opentelemetry-instrumentation-asgi`, `opentelemetry-instrumentation-fastapi` Add ability to disable internal HTTP send and receive spans ([#2802](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2802)) +- `opentelemetry-instrumentation-dbapi` Add db.collection.name, use connection kwargs for connection attributes + ([#2869](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2869)) ### Breaking changes From 38029b5c311ed862d369d4f55a7c50f37758bd0d Mon Sep 17 00:00:00 2001 From: Keith Zmudzinski Date: Thu, 12 Sep 2024 16:32:38 -0700 Subject: [PATCH 06/16] chore: be case insensitive --- .../src/opentelemetry/instrumentation/dbapi/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index 1923316e5a..78d77b5b47 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -419,7 +419,7 @@ def get_operation_name(self, statement): def get_collection_name(statement): collection_name = "" match = re.search( - r"\b(?:FROM|JOIN|INTO|UPDATE|TABLE)\s+([\w`']+)", statement + r"(?i)\b(?:FROM|JOIN|INTO|UPDATE|TABLE)\s+([\w`']+)", statement ) if match: collection_name = match.group(1).strip("`'") From ba0466aecf6967a114023b8c12e797990926f7ad Mon Sep 17 00:00:00 2001 From: Keith Zmudzinski Date: Fri, 13 Sep 2024 14:00:56 -0700 Subject: [PATCH 07/16] fix: don't allow single quotes around collection name --- .../src/opentelemetry/instrumentation/dbapi/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index 78d77b5b47..e82e1651de 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -419,10 +419,10 @@ def get_operation_name(self, statement): def get_collection_name(statement): collection_name = "" match = re.search( - r"(?i)\b(?:FROM|JOIN|INTO|UPDATE|TABLE)\s+([\w`']+)", statement + r"(?i)\b(?:FROM|JOIN|INTO|UPDATE|TABLE)\s+([\w`]+)", statement ) if match: - collection_name = match.group(1).strip("`'") + collection_name = match.group(1).strip("`") return collection_name From 09d761dfa9a56be85e742ad7e4e07d06d3990d70 Mon Sep 17 00:00:00 2001 From: Keith Zmudzinski Date: Tue, 24 Sep 2024 13:42:08 -0700 Subject: [PATCH 08/16] chore: allow for collection names with schema qualifiers --- .../src/opentelemetry/instrumentation/dbapi/__init__.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index e82e1651de..545eb33b34 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -418,11 +418,9 @@ def get_operation_name(self, statement): @staticmethod def get_collection_name(statement): collection_name = "" - match = re.search( - r"(?i)\b(?:FROM|JOIN|INTO|UPDATE|TABLE)\s+([\w`]+)", statement - ) + match = re.search(r"\b(?:FROM|JOIN|INTO|UPDATE|TABLE)\s+(['`]?(\w+)['`]?(?:\s*\.\s*['`]?(\w+)['`]?)?)", statement) if match: - collection_name = match.group(1).strip("`") + collection_name = match.group(1).replace("'`", "") return collection_name From fbc0fafd261738f2826d791dd75eeeaf1a648272 Mon Sep 17 00:00:00 2001 From: Keith Zmudzinski Date: Tue, 24 Sep 2024 13:44:23 -0700 Subject: [PATCH 09/16] test: test schema name and quote removal --- .../tests/test_dbapi_integration.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index 7a7650d23e..b6bf3b054d 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -97,8 +97,11 @@ def test_span_name(self): cursor.execute("/* leading comment */ query /* trailing comment */") cursor.execute("query /* trailing comment */") cursor.execute("SELECT * FROM test_table") + cursor.execute("SELECT * FROM schema.test_table") + cursor.execute("SELECT * FROM `schema`.`test_table`") + cursor.execute("SELECT * FROM 'schema'.'test_table'") spans_list = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans_list), 7) + self.assertEqual(len(spans_list), 9) self.assertEqual(spans_list[0].name, "Test") self.assertEqual(spans_list[1].name, "multi") self.assertEqual(spans_list[2].name, "tab") @@ -106,6 +109,9 @@ def test_span_name(self): self.assertEqual(spans_list[4].name, "query") self.assertEqual(spans_list[5].name, "query") self.assertEqual(spans_list[6].name, "SELECT test_table") + self.assertEqual(spans_list[7].name, "SELECT schema.test_table") + self.assertEqual(spans_list[8].name, "SELECT schema.test_table") + self.assertEqual(spans_list[9].name, "SELECT schema.test_table") def test_span_succeeded_with_capture_of_statement_parameters(self): connection_props = { From 93f13bf11526c667803531274af2ac0e3fdd950c Mon Sep 17 00:00:00 2001 From: Keith Zmudzinski Date: Tue, 24 Sep 2024 13:46:03 -0700 Subject: [PATCH 10/16] docs: move contribution to correct release --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9ce8accfb..d3fcef9b63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2860](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2860)) - `opentelemetry-instrumentation-aiokafka` Add instrumentor and auto instrumentation support for aiokafka ([#2082](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2082)) +- `opentelemetry-instrumentation-dbapi` Add db.collection.name, use connection kwargs for connection attributes + ([#2869](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2869)) ## Version 1.27.0/0.48b0 () @@ -23,8 +25,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2537](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2537)) - `opentelemetry-instrumentation-asgi`, `opentelemetry-instrumentation-fastapi` Add ability to disable internal HTTP send and receive spans ([#2802](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2802)) -- `opentelemetry-instrumentation-dbapi` Add db.collection.name, use connection kwargs for connection attributes - ([#2869](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2869)) ### Breaking changes From a9ed2b56266d9b22e6ec2247578aae925f5e27a9 Mon Sep 17 00:00:00 2001 From: Keith Zmudzinski Date: Mon, 30 Sep 2024 15:17:54 -0700 Subject: [PATCH 11/16] chore: don't break existing subclasses in consuming libraries --- .../instrumentation/dbapi/__init__.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index 545eb33b34..e3eb263a99 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -404,16 +404,19 @@ def _populate_span( if self._db_api_integration.capture_parameters and len(args) > 1: span.set_attribute("db.statement.parameters", str(args[1])) - def get_span_name(self, statement): - operation_name = self.get_operation_name(statement) + def get_span_name(self, cursor, args): + operation_name = self.get_operation_name(cursor, args) + statement = self.get_statement(cursor, args) collection_name = CursorTracer.get_collection_name(statement) return " ".join( name for name in (operation_name, collection_name) if name ) - def get_operation_name(self, statement): - # Strip leading comments so we get the operation name. - return self._leading_comment_remover.sub("", statement).split()[0] + def get_operation_name(self, cursor, args): + if args and isinstance(args[0], str): + # Strip leading comments so we get the operation name. + return self._leading_comment_remover.sub("", args[0]).split()[0] + return "" @staticmethod def get_collection_name(statement): @@ -440,7 +443,7 @@ def traced_execution( **kwargs: typing.Dict[typing.Any, typing.Any], ): statement = self.get_statement(cursor, args) - name = self.get_span_name(statement) + name = self.get_span_name(cursor, args) if not name: name = ( self._db_api_integration.database From a8273b6e206e23b3a6fbc90ccebf7b83ad9ab3cd Mon Sep 17 00:00:00 2001 From: Keith Zmudzinski Date: Mon, 30 Sep 2024 15:22:03 -0700 Subject: [PATCH 12/16] chore: remove unused call --- .../src/opentelemetry/instrumentation/dbapi/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index e3eb263a99..1711eec690 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -442,7 +442,6 @@ def traced_execution( *args: typing.Tuple[typing.Any, typing.Any], **kwargs: typing.Dict[typing.Any, typing.Any], ): - statement = self.get_statement(cursor, args) name = self.get_span_name(cursor, args) if not name: name = ( From 192d202c7d45ca928afd396cf4dce027ad08a039 Mon Sep 17 00:00:00 2001 From: Keith Zmudzinski Date: Mon, 7 Oct 2024 09:53:21 -0700 Subject: [PATCH 13/16] test: update number of tests and expected names --- .../src/opentelemetry/instrumentation/dbapi/__init__.py | 2 +- .../tests/test_dbapi_integration.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index 1711eec690..0dbba8ea62 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -423,7 +423,7 @@ def get_collection_name(statement): collection_name = "" match = re.search(r"\b(?:FROM|JOIN|INTO|UPDATE|TABLE)\s+(['`]?(\w+)['`]?(?:\s*\.\s*['`]?(\w+)['`]?)?)", statement) if match: - collection_name = match.group(1).replace("'`", "") + collection_name = match.group(1) return collection_name diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index b6bf3b054d..8690f3535b 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -101,7 +101,7 @@ def test_span_name(self): cursor.execute("SELECT * FROM `schema`.`test_table`") cursor.execute("SELECT * FROM 'schema'.'test_table'") spans_list = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans_list), 9) + self.assertEqual(len(spans_list), 10) self.assertEqual(spans_list[0].name, "Test") self.assertEqual(spans_list[1].name, "multi") self.assertEqual(spans_list[2].name, "tab") @@ -110,8 +110,8 @@ def test_span_name(self): self.assertEqual(spans_list[5].name, "query") self.assertEqual(spans_list[6].name, "SELECT test_table") self.assertEqual(spans_list[7].name, "SELECT schema.test_table") - self.assertEqual(spans_list[8].name, "SELECT schema.test_table") - self.assertEqual(spans_list[9].name, "SELECT schema.test_table") + self.assertEqual(spans_list[8].name, "SELECT `schema`.`test_table`") + self.assertEqual(spans_list[9].name, "SELECT 'schema'.'test_table'") def test_span_succeeded_with_capture_of_statement_parameters(self): connection_props = { From 0329dad67dece026835594e978953bfdce6ca039 Mon Sep 17 00:00:00 2001 From: Keith Zmudzinski Date: Mon, 7 Oct 2024 10:00:47 -0700 Subject: [PATCH 14/16] chore: lint --- .../src/opentelemetry/instrumentation/dbapi/__init__.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index 0dbba8ea62..1b6d31e447 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -421,7 +421,10 @@ def get_operation_name(self, cursor, args): @staticmethod def get_collection_name(statement): collection_name = "" - match = re.search(r"\b(?:FROM|JOIN|INTO|UPDATE|TABLE)\s+(['`]?(\w+)['`]?(?:\s*\.\s*['`]?(\w+)['`]?)?)", statement) + match = re.search( + r"\b(?:FROM|JOIN|INTO|UPDATE|TABLE)\s+(['`]?(\w+)['`]?(?:\s*\.\s*['`]?(\w+)['`]?)?)", + statement, + ) if match: collection_name = match.group(1) From 4df2fc4f88749ad219a31ab04dfed1c0700542f5 Mon Sep 17 00:00:00 2001 From: Keith Zmudzinski Date: Mon, 14 Oct 2024 11:05:40 -0700 Subject: [PATCH 15/16] test: update sqlite3 tests, correctly handle CREATE TABLE --- .../src/opentelemetry/instrumentation/dbapi/__init__.py | 2 +- .../tests/test_sqlite3.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index 1b6d31e447..67f209bcaa 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -422,7 +422,7 @@ def get_operation_name(self, cursor, args): def get_collection_name(statement): collection_name = "" match = re.search( - r"\b(?:FROM|JOIN|INTO|UPDATE|TABLE)\s+(['`]?(\w+)['`]?(?:\s*\.\s*['`]?(\w+)['`]?)?)", + r"\b(?:FROM|JOIN|INTO|UPDATE|TABLE(?: IF NOT EXISTS)?)\s+(['`]?(\w+)['`]?(?:\s*\.\s*['`]?(\w+)['`]?)?)", statement, ) if match: diff --git a/instrumentation/opentelemetry-instrumentation-sqlite3/tests/test_sqlite3.py b/instrumentation/opentelemetry-instrumentation-sqlite3/tests/test_sqlite3.py index 581920232b..d9f935fed7 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlite3/tests/test_sqlite3.py +++ b/instrumentation/opentelemetry-instrumentation-sqlite3/tests/test_sqlite3.py @@ -72,11 +72,11 @@ def test_execute(self): stmt = "CREATE TABLE IF NOT EXISTS test (id integer)" with self._tracer.start_as_current_span("rootSpan"): self._cursor.execute(stmt) - self.validate_spans("CREATE") + self.validate_spans("CREATE test") with self._tracer.start_as_current_span("rootSpan"): self._cursor2.execute(stmt) - self.validate_spans("CREATE") + self.validate_spans("CREATE test") def test_executemany(self): """Should create a child span for executemany""" @@ -87,11 +87,11 @@ def test_executemany(self): data = [("1",), ("2",), ("3",)] with self._tracer.start_as_current_span("rootSpan"): self._cursor.executemany(stmt, data) - self.validate_spans("INSERT") + self.validate_spans("INSERT test") with self._tracer.start_as_current_span("rootSpan"): self._cursor2.executemany(stmt, data) - self.validate_spans("INSERT") + self.validate_spans("INSERT test") def test_callproc(self): """Should create a child span for callproc""" From 2b8cdea02fa768f36d466a7a4d58ed340e85015a Mon Sep 17 00:00:00 2001 From: Keith Zmudzinski Date: Mon, 14 Oct 2024 16:06:41 -0700 Subject: [PATCH 16/16] fix: update remaining tests to expect collection name --- .../tests/mysql/test_mysql_functional.py | 8 ++++---- .../tests/pymysql/test_pymysql_functional.py | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/opentelemetry-docker-tests/tests/mysql/test_mysql_functional.py b/tests/opentelemetry-docker-tests/tests/mysql/test_mysql_functional.py index 4f1305e866..217103d3ff 100644 --- a/tests/opentelemetry-docker-tests/tests/mysql/test_mysql_functional.py +++ b/tests/opentelemetry-docker-tests/tests/mysql/test_mysql_functional.py @@ -84,7 +84,7 @@ def test_execute(self): stmt = "CREATE TABLE IF NOT EXISTS test (id INT)" with self._tracer.start_as_current_span("rootSpan"): self._cursor.execute(stmt) - self.validate_spans("CREATE") + self.validate_spans("CREATE test") def test_execute_with_connection_context_manager(self): """Should create a child span for execute with connection context""" @@ -93,7 +93,7 @@ def test_execute_with_connection_context_manager(self): with self._connection as conn: cursor = conn.cursor() cursor.execute(stmt) - self.validate_spans("CREATE") + self.validate_spans("CREATE test") def test_execute_with_cursor_context_manager(self): """Should create a child span for execute with cursor context""" @@ -101,7 +101,7 @@ def test_execute_with_cursor_context_manager(self): with self._tracer.start_as_current_span("rootSpan"): with self._connection.cursor() as cursor: cursor.execute(stmt) - self.validate_spans("CREATE") + self.validate_spans("CREATE test") def test_executemany(self): """Should create a child span for executemany""" @@ -109,7 +109,7 @@ def test_executemany(self): with self._tracer.start_as_current_span("rootSpan"): data = (("1",), ("2",), ("3",)) self._cursor.executemany(stmt, data) - self.validate_spans("INSERT") + self.validate_spans("INSERT test") def test_callproc(self): """Should create a child span for callproc""" diff --git a/tests/opentelemetry-docker-tests/tests/pymysql/test_pymysql_functional.py b/tests/opentelemetry-docker-tests/tests/pymysql/test_pymysql_functional.py index 599c2843a1..a843bb3b55 100644 --- a/tests/opentelemetry-docker-tests/tests/pymysql/test_pymysql_functional.py +++ b/tests/opentelemetry-docker-tests/tests/pymysql/test_pymysql_functional.py @@ -84,7 +84,7 @@ def test_execute(self): stmt = "CREATE TABLE IF NOT EXISTS test (id INT)" with self._tracer.start_as_current_span("rootSpan"): self._cursor.execute(stmt) - self.validate_spans("CREATE") + self.validate_spans("CREATE test") def test_execute_with_cursor_context_manager(self): """Should create a child span for execute with cursor context""" @@ -92,7 +92,7 @@ def test_execute_with_cursor_context_manager(self): with self._tracer.start_as_current_span("rootSpan"): with self._connection.cursor() as cursor: cursor.execute(stmt) - self.validate_spans("CREATE") + self.validate_spans("CREATE test") def test_executemany(self): """Should create a child span for executemany""" @@ -100,7 +100,7 @@ def test_executemany(self): with self._tracer.start_as_current_span("rootSpan"): data = (("1",), ("2",), ("3",)) self._cursor.executemany(stmt, data) - self.validate_spans("INSERT") + self.validate_spans("INSERT test") def test_callproc(self): """Should create a child span for callproc""" @@ -116,7 +116,7 @@ def test_commit(self): data = (("4",), ("5",), ("6",)) self._cursor.executemany(stmt, data) self._connection.commit() - self.validate_spans("INSERT") + self.validate_spans("INSERT test") def test_rollback(self): stmt = "INSERT INTO test (id) VALUES (%s)" @@ -124,4 +124,4 @@ def test_rollback(self): data = (("7",), ("8",), ("9",)) self._cursor.executemany(stmt, data) self._connection.rollback() - self.validate_spans("INSERT") + self.validate_spans("INSERT test")