From 57fdce52d282198b3e36d59be2de9ecd89e0f51a Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Wed, 18 Nov 2020 01:33:01 +0530 Subject: [PATCH 01/10] Fix span name for aiopg --- .../instrumentation/aiopg/aiopg_integration.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/aiopg_integration.py b/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/aiopg_integration.py index 14f986da06..9824237565 100644 --- a/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/aiopg_integration.py +++ b/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/aiopg_integration.py @@ -101,9 +101,16 @@ async def traced_execution( *args: typing.Tuple[typing.Any, typing.Any], **kwargs: typing.Dict[typing.Any, typing.Any] ): + name = "" + if len(args) > 0 and args[0]: + name = args[0] + elif self._db_api_integration.database: + name = self._db_api_integration.database + else: + name = self._db_api_integration.name with self._db_api_integration.get_tracer().start_as_current_span( - self._db_api_integration.name, kind=SpanKind.CLIENT + name, kind=SpanKind.CLIENT ) as span: self._populate_span(span, *args) try: From 3854f17e5aa03a8f4443d35e4731412dcbb2f81e Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Wed, 18 Nov 2020 01:33:55 +0530 Subject: [PATCH 02/10] Fix tests aiopg --- .../tests/test_aiopg_integration.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-aiopg/tests/test_aiopg_integration.py b/instrumentation/opentelemetry-instrumentation-aiopg/tests/test_aiopg_integration.py index 89e7cc05a5..c6f771e1b2 100644 --- a/instrumentation/opentelemetry-instrumentation-aiopg/tests/test_aiopg_integration.py +++ b/instrumentation/opentelemetry-instrumentation-aiopg/tests/test_aiopg_integration.py @@ -215,12 +215,12 @@ def test_span_succeeded(self): spans_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans_list), 1) span = spans_list[0] - self.assertEqual(span.name, "testcomponent.testdatabase") + self.assertEqual(span.name, "Test query") self.assertIs(span.kind, trace_api.SpanKind.CLIENT) self.assertEqual(span.attributes["component"], "testcomponent") - self.assertEqual(span.attributes["db.type"], "testtype") - self.assertEqual(span.attributes["db.instance"], "testdatabase") + self.assertEqual(span.attributes["db.system"], "testcomponent") + self.assertEqual(span.attributes["db.name"], "testdatabase") self.assertEqual(span.attributes["db.statement"], "Test query") self.assertEqual( span.attributes["db.statement.parameters"], @@ -230,7 +230,7 @@ def test_span_succeeded(self): self.assertEqual(span.attributes["net.peer.name"], "testhost") self.assertEqual(span.attributes["net.peer.port"], 123) self.assertIs( - span.status.status_code, trace_api.status.StatusCode.UNSET, + span.status.status_code, trace_api.status.StatusCode.UNSET ) def test_span_not_recording(self): @@ -281,7 +281,7 @@ def test_span_failed(self): span = spans_list[0] self.assertEqual(span.attributes["db.statement"], "Test query") self.assertIs( - span.status.status_code, trace_api.status.StatusCode.ERROR, + span.status.status_code, trace_api.status.StatusCode.ERROR ) self.assertEqual(span.status.description, "Test Exception") From 7c038ae2b4235250bce36ca9cea9ca2ba4508983 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Wed, 18 Nov 2020 01:35:23 +0530 Subject: [PATCH 03/10] Update dbapi instrumentation Fix span name for dbapi Decode db user for pymysql --- .../instrumentation/dbapi/__init__.py | 81 +++++++++++-------- 1 file changed, 46 insertions(+), 35 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 5b120d3dec..197f4ade44 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -62,19 +62,19 @@ def trace_integration( capture_parameters: bool = False, ): """Integrate with DB API library. - https://www.python.org/dev/peps/pep-0249/ - - Args: - connect_module: Module name where connect method is available. - connect_method_name: The connect method name. - database_component: Database driver name or database name "JDBI", - "jdbc", "odbc", "postgreSQL". - database_type: The Database type. For any SQL database, "sql". - connection_attributes: Attribute names for database, port, host and - user in Connection object. - tracer_provider: The :class:`opentelemetry.trace.TracerProvider` to - use. If ommited the current configured one is used. - capture_parameters: Configure if db.statement.parameters should be captured. + https://www.python.org/dev/peps/pep-0249/ + + Args: + connect_module: Module name where connect method is available. + connect_method_name: The connect method name. + database_component: Database driver name or database name "JDBI", + "jdbc", "odbc", "postgreSQL". + database_type: The Database type. For any SQL database, "sql". + connection_attributes: Attribute names for database, port, host and + user in Connection object. + tracer_provider: The :class:`opentelemetry.trace.TracerProvider` to + use. If ommited the current configured one is used. + capture_parameters: Configure if db.statement.parameters should be captured. """ wrap_connect( __name__, @@ -101,18 +101,18 @@ def wrap_connect( capture_parameters: bool = False, ): """Integrate with DB API library. - https://www.python.org/dev/peps/pep-0249/ - - Args: - tracer: The :class:`opentelemetry.trace.Tracer` to use. - connect_module: Module name where connect method is available. - connect_method_name: The connect method name. - database_component: Database driver name or database name "JDBI", - "jdbc", "odbc", "postgreSQL". - database_type: The Database type. For any SQL database, "sql". - connection_attributes: Attribute names for database, port, host and - user in Connection object. - capture_parameters: Configure if db.statement.parameters should be captured. + https://www.python.org/dev/peps/pep-0249/ + + Args: + tracer: The :class:`opentelemetry.trace.Tracer` to use. + connect_module: Module name where connect method is available. + connect_method_name: The connect method name. + database_component: Database driver name or database name "JDBI", + "jdbc", "odbc", "postgreSQL". + database_type: The Database type. For any SQL database, "sql". + connection_attributes: Attribute names for database, port, host and + user in Connection object. + capture_parameters: Configure if db.statement.parameters should be captured. """ @@ -143,14 +143,14 @@ def wrap_connect_( def unwrap_connect( - connect_module: typing.Callable[..., typing.Any], connect_method_name: str, + connect_module: typing.Callable[..., typing.Any], connect_method_name: str ): """Disable integration with DB API library. - https://www.python.org/dev/peps/pep-0249/ + https://www.python.org/dev/peps/pep-0249/ - Args: - connect_module: Module name where the connect method is available. - connect_method_name: The connect method name. + Args: + connect_module: Module name where the connect method is available. + connect_method_name: The connect method name. """ unwrap(connect_module, connect_method_name) @@ -251,8 +251,7 @@ def wrapped_connection( args: typing.Tuple[typing.Any, typing.Any], kwargs: typing.Dict[typing.Any, typing.Any], ): - """Add object proxy to connection object. - """ + """Add object proxy to connection object.""" connection = connect_method(*args, **kwargs) self.get_connection_attributes(connection) return get_traced_connection_proxy(connection, self) @@ -278,6 +277,9 @@ def get_connection_attributes(self, connection): self.database = self.database.decode(errors="ignore") self.name += "." + self.database user = self.connection_props.get("user") + # PyMySQL encodes this data + if user and isinstance(user, bytes): + user = user.decode() if user is not None: self.span_attributes["db.user"] = str(user) host = self.connection_props.get("host") @@ -325,8 +327,10 @@ def _populate_span( span.set_attribute( "component", self._db_api_integration.database_component ) - span.set_attribute("db.type", self._db_api_integration.database_type) - span.set_attribute("db.instance", self._db_api_integration.database) + span.set_attribute( + "db.system", self._db_api_integration.database_component + ) + span.set_attribute("db.name", self._db_api_integration.database) span.set_attribute("db.statement", statement) for ( @@ -344,9 +348,16 @@ def traced_execution( *args: typing.Tuple[typing.Any, typing.Any], **kwargs: typing.Dict[typing.Any, typing.Any] ): + name = "" + if args: + name = args[0] + elif self._db_api_integration.database: + name = self._db_api_integration.database + else: + name = self._db_api_integration.name with self._db_api_integration.get_tracer().start_as_current_span( - self._db_api_integration.name, kind=SpanKind.CLIENT + name, kind=SpanKind.CLIENT ) as span: self._populate_span(span, *args) try: From a208316dcab1f34c8d979deb6fb7d9421353a066 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Wed, 18 Nov 2020 01:35:50 +0530 Subject: [PATCH 04/10] Fix tests for dbapi --- .../tests/test_dbapi_integration.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index 40176363ce..e69bf60c9d 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -50,19 +50,19 @@ def test_span_succeeded(self): spans_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans_list), 1) span = spans_list[0] - self.assertEqual(span.name, "testcomponent.testdatabase") + self.assertEqual(span.name, "Test query") self.assertIs(span.kind, trace_api.SpanKind.CLIENT) self.assertEqual(span.attributes["component"], "testcomponent") - self.assertEqual(span.attributes["db.type"], "testtype") - self.assertEqual(span.attributes["db.instance"], "testdatabase") + self.assertEqual(span.attributes["db.system"], "testcomponent") + self.assertEqual(span.attributes["db.name"], "testdatabase") self.assertEqual(span.attributes["db.statement"], "Test query") self.assertFalse("db.statement.parameters" in span.attributes) self.assertEqual(span.attributes["db.user"], "testuser") self.assertEqual(span.attributes["net.peer.name"], "testhost") self.assertEqual(span.attributes["net.peer.port"], 123) self.assertIs( - span.status.status_code, trace_api.status.StatusCode.UNSET, + span.status.status_code, trace_api.status.StatusCode.UNSET ) def test_span_succeeded_with_capture_of_statement_parameters(self): @@ -93,12 +93,12 @@ def test_span_succeeded_with_capture_of_statement_parameters(self): spans_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans_list), 1) span = spans_list[0] - self.assertEqual(span.name, "testcomponent.testdatabase") + self.assertEqual(span.name, "Test query") self.assertIs(span.kind, trace_api.SpanKind.CLIENT) self.assertEqual(span.attributes["component"], "testcomponent") - self.assertEqual(span.attributes["db.type"], "testtype") - self.assertEqual(span.attributes["db.instance"], "testdatabase") + self.assertEqual(span.attributes["db.system"], "testcomponent") + self.assertEqual(span.attributes["db.name"], "testdatabase") self.assertEqual(span.attributes["db.statement"], "Test query") self.assertEqual( span.attributes["db.statement.parameters"], @@ -108,7 +108,7 @@ def test_span_succeeded_with_capture_of_statement_parameters(self): self.assertEqual(span.attributes["net.peer.name"], "testhost") self.assertEqual(span.attributes["net.peer.port"], 123) self.assertIs( - span.status.status_code, trace_api.status.StatusCode.UNSET, + span.status.status_code, trace_api.status.StatusCode.UNSET ) def test_span_not_recording(self): @@ -159,7 +159,7 @@ def test_span_failed(self): span = spans_list[0] self.assertEqual(span.attributes["db.statement"], "Test query") self.assertIs( - span.status.status_code, trace_api.status.StatusCode.ERROR, + span.status.status_code, trace_api.status.StatusCode.ERROR ) self.assertEqual(span.status.description, "Test Exception") From bd39b36b82b0ecef890d4f3bae67339d6643c4c2 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Wed, 18 Nov 2020 01:36:16 +0530 Subject: [PATCH 05/10] Fix tests for sqlite --- .../tests/test_sqlite3.py | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-sqlite3/tests/test_sqlite3.py b/instrumentation/opentelemetry-instrumentation-sqlite3/tests/test_sqlite3.py index 0e385cf3e7..a4fc887061 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlite3/tests/test_sqlite3.py +++ b/instrumentation/opentelemetry-instrumentation-sqlite3/tests/test_sqlite3.py @@ -37,7 +37,7 @@ def tearDownClass(cls): if cls._connection: cls._connection.close() - def validate_spans(self): + def validate_spans(self, span_name): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 2) for span in spans: @@ -50,34 +50,30 @@ def validate_spans(self): self.assertIsNotNone(root_span) self.assertIsNotNone(child_span) self.assertEqual(root_span.name, "rootSpan") - self.assertEqual(child_span.name, "sqlite3") + self.assertEqual(child_span.name, span_name) self.assertIsNotNone(child_span.parent) self.assertIs(child_span.parent, root_span.get_span_context()) self.assertIs(child_span.kind, trace_api.SpanKind.CLIENT) def test_execute(self): - """Should create a child span for execute method - """ + """Should create a child span for execute method""" + stmt = "CREATE TABLE IF NOT EXISTS test (id integer)" with self._tracer.start_as_current_span("rootSpan"): - self._cursor.execute( - "CREATE TABLE IF NOT EXISTS test (id integer)" - ) - self.validate_spans() + self._cursor.execute(stmt) + self.validate_spans(stmt) def test_executemany(self): - """Should create a child span for executemany - """ + """Should create a child span for executemany""" + stmt = "INSERT INTO test (id) VALUES (?)" with self._tracer.start_as_current_span("rootSpan"): data = [("1",), ("2",), ("3",)] - stmt = "INSERT INTO test (id) VALUES (?)" self._cursor.executemany(stmt, data) - self.validate_spans() + self.validate_spans(stmt) def test_callproc(self): - """Should create a child span for callproc - """ + """Should create a child span for callproc""" with self._tracer.start_as_current_span("rootSpan"), self.assertRaises( Exception ): self._cursor.callproc("test", ()) - self.validate_spans() + self.validate_spans("test") From 2228573bf33a487ae3a1584f23b3f0318e77be0b Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Wed, 18 Nov 2020 01:41:59 +0530 Subject: [PATCH 06/10] Fix docker tests --- .../tests/mysql/test_mysql_functional.py | 39 ++++++------ .../tests/postgres/test_aiopg_functional.py | 60 +++++++++---------- .../tests/postgres/test_psycopg_functional.py | 43 ++++++------- .../tests/pymysql/test_pymysql_functional.py | 34 ++++++----- 4 files changed, 90 insertions(+), 86 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 fc237fe12b..ec6eed3132 100644 --- a/tests/opentelemetry-docker-tests/tests/mysql/test_mysql_functional.py +++ b/tests/opentelemetry-docker-tests/tests/mysql/test_mysql_functional.py @@ -20,11 +20,11 @@ from opentelemetry.instrumentation.mysql import MySQLInstrumentor from opentelemetry.test.test_base import TestBase -MYSQL_USER = os.getenv("MYSQL_USER ", "testuser") -MYSQL_PASSWORD = os.getenv("MYSQL_PASSWORD ", "testpassword") -MYSQL_HOST = os.getenv("MYSQL_HOST ", "localhost") -MYSQL_PORT = int(os.getenv("MYSQL_PORT ", "3306")) -MYSQL_DB_NAME = os.getenv("MYSQL_DB_NAME ", "opentelemetry-tests") +MYSQL_USER = os.getenv("MYSQL_USER", "testuser") +MYSQL_PASSWORD = os.getenv("MYSQL_PASSWORD", "testpassword") +MYSQL_HOST = os.getenv("MYSQL_HOST", "localhost") +MYSQL_PORT = int(os.getenv("MYSQL_PORT", "3306")) +MYSQL_DB_NAME = os.getenv("MYSQL_DB_NAME", "opentelemetry-tests") class TestFunctionalMysql(TestBase): @@ -53,7 +53,7 @@ def setUp(self): ) self._cursor = self._connection.cursor() - def validate_spans(self): + def validate_spans(self, span_name): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 2) for span in spans: @@ -66,42 +66,47 @@ def validate_spans(self): self.assertIsNotNone(root_span) self.assertIsNotNone(db_span) self.assertEqual(root_span.name, "rootSpan") - self.assertEqual(db_span.name, "mysql.opentelemetry-tests") + self.assertEqual(db_span.name, span_name) self.assertIsNotNone(db_span.parent) self.assertIs(db_span.parent, root_span.get_span_context()) self.assertIs(db_span.kind, trace_api.SpanKind.CLIENT) - self.assertEqual(db_span.attributes["db.instance"], MYSQL_DB_NAME) + self.assertEqual(db_span.attributes["db.system"], "mysql") + self.assertEqual(db_span.attributes["db.name"], MYSQL_DB_NAME) + self.assertEqual(db_span.attributes["db.user"], MYSQL_USER) self.assertEqual(db_span.attributes["net.peer.name"], MYSQL_HOST) self.assertEqual(db_span.attributes["net.peer.port"], MYSQL_PORT) def test_execute(self): """Should create a child span for execute""" + stmt = "CREATE TABLE IF NOT EXISTS test (id INT)" with self._tracer.start_as_current_span("rootSpan"): - self._cursor.execute("CREATE TABLE IF NOT EXISTS test (id INT)") - self.validate_spans() + self._cursor.execute(stmt) + self.validate_spans(stmt) def test_execute_with_connection_context_manager(self): """Should create a child span for execute with connection context""" + stmt = "CREATE TABLE IF NOT EXISTS test (id INT)" with self._tracer.start_as_current_span("rootSpan"): with self._connection as conn: cursor = conn.cursor() - cursor.execute("CREATE TABLE IF NOT EXISTS test (id INT)") - self.validate_spans() + cursor.execute(stmt) + self.validate_spans(stmt) def test_execute_with_cursor_context_manager(self): """Should create a child span for execute with cursor context""" + stmt = "CREATE TABLE IF NOT EXISTS test (id INT)" with self._tracer.start_as_current_span("rootSpan"): with self._connection.cursor() as cursor: - cursor.execute("CREATE TABLE IF NOT EXISTS test (id INT)") - self.validate_spans() + cursor.execute(stmt) + self.validate_spans(stmt) def test_executemany(self): """Should create a child span for executemany""" + stmt = "INSERT INTO test (id) VALUES (%s)" with self._tracer.start_as_current_span("rootSpan"): data = (("1",), ("2",), ("3",)) - stmt = "INSERT INTO test (id) VALUES (%s)" self._cursor.executemany(stmt, data) - self.validate_spans() + self.validate_spans(stmt) def test_callproc(self): """Should create a child span for callproc""" @@ -109,4 +114,4 @@ def test_callproc(self): Exception ): self._cursor.callproc("test", ()) - self.validate_spans() + self.validate_spans("test") diff --git a/tests/opentelemetry-docker-tests/tests/postgres/test_aiopg_functional.py b/tests/opentelemetry-docker-tests/tests/postgres/test_aiopg_functional.py index d76cd702ee..030aecc66d 100644 --- a/tests/opentelemetry-docker-tests/tests/postgres/test_aiopg_functional.py +++ b/tests/opentelemetry-docker-tests/tests/postgres/test_aiopg_functional.py @@ -22,11 +22,11 @@ from opentelemetry.instrumentation.aiopg import AiopgInstrumentor from opentelemetry.test.test_base import TestBase -POSTGRES_HOST = os.getenv("POSTGRESQL_HOST ", "localhost") -POSTGRES_PORT = int(os.getenv("POSTGRESQL_PORT ", "5432")) -POSTGRES_DB_NAME = os.getenv("POSTGRESQL_DB_NAME ", "opentelemetry-tests") -POSTGRES_PASSWORD = os.getenv("POSTGRESQL_HOST ", "testpassword") -POSTGRES_USER = os.getenv("POSTGRESQL_HOST ", "testuser") +POSTGRES_HOST = os.getenv("POSTGRESQL_HOST", "localhost") +POSTGRES_PORT = int(os.getenv("POSTGRESQL_PORT", "5432")) +POSTGRES_DB_NAME = os.getenv("POSTGRESQL_DB_NAME", "opentelemetry-tests") +POSTGRES_PASSWORD = os.getenv("POSTGRESQL_PASSWORD", "testpassword") +POSTGRES_USER = os.getenv("POSTGRESQL_USER", "testuser") def async_call(coro): @@ -61,7 +61,7 @@ def tearDownClass(cls): cls._connection.close() AiopgInstrumentor().uninstrument() - def validate_spans(self): + def validate_spans(self, span_name): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 2) for span in spans: @@ -74,34 +74,31 @@ def validate_spans(self): self.assertIsNotNone(root_span) self.assertIsNotNone(child_span) self.assertEqual(root_span.name, "rootSpan") - self.assertEqual(child_span.name, "postgresql.opentelemetry-tests") + self.assertEqual(child_span.name, span_name) self.assertIsNotNone(child_span.parent) self.assertIs(child_span.parent, root_span.get_span_context()) self.assertIs(child_span.kind, trace_api.SpanKind.CLIENT) - self.assertEqual( - child_span.attributes["db.instance"], POSTGRES_DB_NAME - ) + self.assertEqual(child_span.attributes["db.system"], "postgresql") + self.assertEqual(child_span.attributes["db.name"], POSTGRES_DB_NAME) + self.assertEqual(child_span.attributes["db.user"], POSTGRES_USER) self.assertEqual(child_span.attributes["net.peer.name"], POSTGRES_HOST) self.assertEqual(child_span.attributes["net.peer.port"], POSTGRES_PORT) def test_execute(self): """Should create a child span for execute method""" + stmt = "CREATE TABLE IF NOT EXISTS test (id integer)" with self._tracer.start_as_current_span("rootSpan"): - async_call( - self._cursor.execute( - "CREATE TABLE IF NOT EXISTS test (id integer)" - ) - ) - self.validate_spans() + async_call(self._cursor.execute(stmt)) + self.validate_spans(stmt) def test_executemany(self): """Should create a child span for executemany""" + stmt = "INSERT INTO test (id) VALUES (%s)" with pytest.raises(psycopg2.ProgrammingError): with self._tracer.start_as_current_span("rootSpan"): data = (("1",), ("2",), ("3",)) - stmt = "INSERT INTO test (id) VALUES (%s)" async_call(self._cursor.executemany(stmt, data)) - self.validate_spans() + self.validate_spans(stmt) def test_callproc(self): """Should create a child span for callproc""" @@ -109,7 +106,7 @@ def test_callproc(self): Exception ): async_call(self._cursor.callproc("test", ())) - self.validate_spans() + self.validate_spans("test") class TestFunctionalAiopgCreatePool(TestBase): @@ -142,7 +139,7 @@ def tearDownClass(cls): cls._pool.close() AiopgInstrumentor().uninstrument() - def validate_spans(self): + def validate_spans(self, span_name): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 2) for span in spans: @@ -155,34 +152,31 @@ def validate_spans(self): self.assertIsNotNone(root_span) self.assertIsNotNone(child_span) self.assertEqual(root_span.name, "rootSpan") - self.assertEqual(child_span.name, "postgresql.opentelemetry-tests") + self.assertEqual(child_span.name, span_name) self.assertIsNotNone(child_span.parent) self.assertIs(child_span.parent, root_span.get_span_context()) self.assertIs(child_span.kind, trace_api.SpanKind.CLIENT) - self.assertEqual( - child_span.attributes["db.instance"], POSTGRES_DB_NAME - ) + self.assertEqual(child_span.attributes["db.system"], "postgresql") + self.assertEqual(child_span.attributes["db.name"], POSTGRES_DB_NAME) + self.assertEqual(child_span.attributes["db.user"], POSTGRES_USER) self.assertEqual(child_span.attributes["net.peer.name"], POSTGRES_HOST) self.assertEqual(child_span.attributes["net.peer.port"], POSTGRES_PORT) def test_execute(self): """Should create a child span for execute method""" + stmt = "CREATE TABLE IF NOT EXISTS test (id integer)" with self._tracer.start_as_current_span("rootSpan"): - async_call( - self._cursor.execute( - "CREATE TABLE IF NOT EXISTS test (id integer)" - ) - ) - self.validate_spans() + async_call(self._cursor.execute(stmt)) + self.validate_spans(stmt) def test_executemany(self): """Should create a child span for executemany""" + stmt = "INSERT INTO test (id) VALUES (%s)" with pytest.raises(psycopg2.ProgrammingError): with self._tracer.start_as_current_span("rootSpan"): data = (("1",), ("2",), ("3",)) - stmt = "INSERT INTO test (id) VALUES (%s)" async_call(self._cursor.executemany(stmt, data)) - self.validate_spans() + self.validate_spans(stmt) def test_callproc(self): """Should create a child span for callproc""" @@ -190,4 +184,4 @@ def test_callproc(self): Exception ): async_call(self._cursor.callproc("test", ())) - self.validate_spans() + self.validate_spans("test") diff --git a/tests/opentelemetry-docker-tests/tests/postgres/test_psycopg_functional.py b/tests/opentelemetry-docker-tests/tests/postgres/test_psycopg_functional.py index 28db4c064f..76116dfd28 100644 --- a/tests/opentelemetry-docker-tests/tests/postgres/test_psycopg_functional.py +++ b/tests/opentelemetry-docker-tests/tests/postgres/test_psycopg_functional.py @@ -20,11 +20,11 @@ from opentelemetry.instrumentation.psycopg2 import Psycopg2Instrumentor from opentelemetry.test.test_base import TestBase -POSTGRES_HOST = os.getenv("POSTGRESQL_HOST ", "localhost") -POSTGRES_PORT = int(os.getenv("POSTGRESQL_PORT ", "5432")) -POSTGRES_DB_NAME = os.getenv("POSTGRESQL_DB_NAME ", "opentelemetry-tests") -POSTGRES_PASSWORD = os.getenv("POSTGRESQL_HOST ", "testpassword") -POSTGRES_USER = os.getenv("POSTGRESQL_HOST ", "testuser") +POSTGRES_HOST = os.getenv("POSTGRESQL_HOST", "localhost") +POSTGRES_PORT = int(os.getenv("POSTGRESQL_PORT", "5432")) +POSTGRES_DB_NAME = os.getenv("POSTGRESQL_DB_NAME", "opentelemetry-tests") +POSTGRES_PASSWORD = os.getenv("POSTGRESQL_PASSWORD", "testpassword") +POSTGRES_USER = os.getenv("POSTGRESQL_USER", "testuser") class TestFunctionalPsycopg(TestBase): @@ -53,7 +53,7 @@ def tearDownClass(cls): cls._connection.close() Psycopg2Instrumentor().uninstrument() - def validate_spans(self): + def validate_spans(self, span_name): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 2) for span in spans: @@ -66,47 +66,48 @@ def validate_spans(self): self.assertIsNotNone(root_span) self.assertIsNotNone(child_span) self.assertEqual(root_span.name, "rootSpan") - self.assertEqual(child_span.name, "postgresql.opentelemetry-tests") + self.assertEqual(child_span.name, span_name) self.assertIsNotNone(child_span.parent) self.assertIs(child_span.parent, root_span.get_span_context()) self.assertIs(child_span.kind, trace_api.SpanKind.CLIENT) - self.assertEqual( - child_span.attributes["db.instance"], POSTGRES_DB_NAME - ) + self.assertEqual(child_span.attributes["db.system"], "postgresql") + self.assertEqual(child_span.attributes["db.name"], POSTGRES_DB_NAME) + self.assertEqual(child_span.attributes["db.user"], POSTGRES_USER) self.assertEqual(child_span.attributes["net.peer.name"], POSTGRES_HOST) self.assertEqual(child_span.attributes["net.peer.port"], POSTGRES_PORT) def test_execute(self): """Should create a child span for execute method""" + stmt = "CREATE TABLE IF NOT EXISTS test (id integer)" with self._tracer.start_as_current_span("rootSpan"): - self._cursor.execute( - "CREATE TABLE IF NOT EXISTS test (id integer)" - ) - self.validate_spans() + self._cursor.execute(stmt) + self.validate_spans(stmt) def test_execute_with_connection_context_manager(self): """Should create a child span for execute with connection context""" + stmt = "CREATE TABLE IF NOT EXISTS test (id INT)" with self._tracer.start_as_current_span("rootSpan"): with self._connection as conn: cursor = conn.cursor() - cursor.execute("CREATE TABLE IF NOT EXISTS test (id INT)") - self.validate_spans() + cursor.execute(stmt) + self.validate_spans(stmt) def test_execute_with_cursor_context_manager(self): """Should create a child span for execute with cursor context""" + stmt = "CREATE TABLE IF NOT EXISTS test (id INT)" with self._tracer.start_as_current_span("rootSpan"): with self._connection.cursor() as cursor: - cursor.execute("CREATE TABLE IF NOT EXISTS test (id INT)") - self.validate_spans() + cursor.execute(stmt) + self.validate_spans(stmt) self.assertTrue(cursor.closed) def test_executemany(self): """Should create a child span for executemany""" + stmt = "INSERT INTO test (id) VALUES (%s)" with self._tracer.start_as_current_span("rootSpan"): data = (("1",), ("2",), ("3",)) - stmt = "INSERT INTO test (id) VALUES (%s)" self._cursor.executemany(stmt, data) - self.validate_spans() + self.validate_spans(stmt) def test_callproc(self): """Should create a child span for callproc""" @@ -114,4 +115,4 @@ def test_callproc(self): Exception ): self._cursor.callproc("test", ()) - self.validate_spans() + self.validate_spans("test") 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 7c09025551..b8e4404805 100644 --- a/tests/opentelemetry-docker-tests/tests/pymysql/test_pymysql_functional.py +++ b/tests/opentelemetry-docker-tests/tests/pymysql/test_pymysql_functional.py @@ -20,11 +20,11 @@ from opentelemetry.instrumentation.pymysql import PyMySQLInstrumentor from opentelemetry.test.test_base import TestBase -MYSQL_USER = os.getenv("MYSQL_USER ", "testuser") -MYSQL_PASSWORD = os.getenv("MYSQL_PASSWORD ", "testpassword") -MYSQL_HOST = os.getenv("MYSQL_HOST ", "localhost") -MYSQL_PORT = int(os.getenv("MYSQL_PORT ", "3306")) -MYSQL_DB_NAME = os.getenv("MYSQL_DB_NAME ", "opentelemetry-tests") +MYSQL_USER = os.getenv("MYSQL_USER", "testuser") +MYSQL_PASSWORD = os.getenv("MYSQL_PASSWORD", "testpassword") +MYSQL_HOST = os.getenv("MYSQL_HOST", "localhost") +MYSQL_PORT = int(os.getenv("MYSQL_PORT", "3306")) +MYSQL_DB_NAME = os.getenv("MYSQL_DB_NAME", "opentelemetry-tests") class TestFunctionalPyMysql(TestBase): @@ -50,7 +50,7 @@ def tearDownClass(cls): cls._connection.close() PyMySQLInstrumentor().uninstrument() - def validate_spans(self): + def validate_spans(self, span_name): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 2) for span in spans: @@ -63,34 +63,38 @@ def validate_spans(self): self.assertIsNotNone(root_span) self.assertIsNotNone(db_span) self.assertEqual(root_span.name, "rootSpan") - self.assertEqual(db_span.name, "mysql.opentelemetry-tests") + self.assertEqual(db_span.name, span_name) self.assertIsNotNone(db_span.parent) self.assertIs(db_span.parent, root_span.get_span_context()) self.assertIs(db_span.kind, trace_api.SpanKind.CLIENT) - self.assertEqual(db_span.attributes["db.instance"], MYSQL_DB_NAME) + self.assertEqual(db_span.attributes["db.system"], "mysql") + self.assertEqual(db_span.attributes["db.name"], MYSQL_DB_NAME) + self.assertEqual(db_span.attributes["db.user"], MYSQL_USER) self.assertEqual(db_span.attributes["net.peer.name"], MYSQL_HOST) self.assertEqual(db_span.attributes["net.peer.port"], MYSQL_PORT) def test_execute(self): """Should create a child span for execute""" + stmt = "CREATE TABLE IF NOT EXISTS test (id INT)" with self._tracer.start_as_current_span("rootSpan"): - self._cursor.execute("CREATE TABLE IF NOT EXISTS test (id INT)") - self.validate_spans() + self._cursor.execute(stmt) + self.validate_spans(stmt) def test_execute_with_cursor_context_manager(self): """Should create a child span for execute with cursor context""" + stmt = "CREATE TABLE IF NOT EXISTS test (id INT)" with self._tracer.start_as_current_span("rootSpan"): with self._connection.cursor() as cursor: - cursor.execute("CREATE TABLE IF NOT EXISTS test (id INT)") - self.validate_spans() + cursor.execute(stmt) + self.validate_spans(stmt) def test_executemany(self): """Should create a child span for executemany""" + stmt = "INSERT INTO test (id) VALUES (%s)" with self._tracer.start_as_current_span("rootSpan"): data = (("1",), ("2",), ("3",)) - stmt = "INSERT INTO test (id) VALUES (%s)" self._cursor.executemany(stmt, data) - self.validate_spans() + self.validate_spans(stmt) def test_callproc(self): """Should create a child span for callproc""" @@ -98,4 +102,4 @@ def test_callproc(self): Exception ): self._cursor.callproc("test", ()) - self.validate_spans() + self.validate_spans("test") From 6df831b2f1be594ca93e3aa0723e936dbcc182ae Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Thu, 19 Nov 2020 23:43:28 +0530 Subject: [PATCH 07/10] Update gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index fa3e412d8f..92e9ccd051 100644 --- a/.gitignore +++ b/.gitignore @@ -16,6 +16,7 @@ var sdist develop-eggs .installed.cfg +pyvenv.cfg lib lib64 __pycache__ From b49b276f7a57430d8aa83e1f52cc4433e518db2d Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Fri, 20 Nov 2020 00:31:03 +0530 Subject: [PATCH 08/10] Update db.system for sqlite --- .../src/opentelemetry/instrumentation/sqlite3/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/__init__.py b/instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/__init__.py index 986b0c6210..bad033b292 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/__init__.py @@ -51,7 +51,7 @@ class SQLite3Instrumentor(BaseInstrumentor): # No useful attributes of sqlite3 connection object _CONNECTION_ATTRIBUTES = {} - _DATABASE_COMPONENT = "sqlite3" + _DATABASE_COMPONENT = "sqlite" _DATABASE_TYPE = "sql" def _instrument(self, **kwargs): From f601bb25638f2aa787e60f4cbd46234c8f6ce265 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Fri, 20 Nov 2020 00:54:54 +0530 Subject: [PATCH 09/10] Fix env vars --- .../tests/check_availability.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/opentelemetry-docker-tests/tests/check_availability.py b/tests/opentelemetry-docker-tests/tests/check_availability.py index 3082572193..0e066610c9 100644 --- a/tests/opentelemetry-docker-tests/tests/check_availability.py +++ b/tests/opentelemetry-docker-tests/tests/check_availability.py @@ -24,16 +24,16 @@ MONGODB_DB_NAME = os.getenv("MONGODB_DB_NAME", "opentelemetry-tests") MONGODB_HOST = os.getenv("MONGODB_HOST", "localhost") MONGODB_PORT = int(os.getenv("MONGODB_PORT", "27017")) -MYSQL_DB_NAME = os.getenv("MYSQL_DB_NAME ", "opentelemetry-tests") -MYSQL_HOST = os.getenv("MYSQL_HOST ", "localhost") -MYSQL_PORT = int(os.getenv("MYSQL_PORT ", "3306")) -MYSQL_USER = os.getenv("MYSQL_USER ", "testuser") -MYSQL_PASSWORD = os.getenv("MYSQL_PASSWORD ", "testpassword") +MYSQL_DB_NAME = os.getenv("MYSQL_DB_NAME", "opentelemetry-tests") +MYSQL_HOST = os.getenv("MYSQL_HOST", "localhost") +MYSQL_PORT = int(os.getenv("MYSQL_PORT", "3306")) +MYSQL_USER = os.getenv("MYSQL_USER", "testuser") +MYSQL_PASSWORD = os.getenv("MYSQL_PASSWORD", "testpassword") POSTGRES_DB_NAME = os.getenv("POSTGRESQL_DB_NAME", "opentelemetry-tests") POSTGRES_HOST = os.getenv("POSTGRESQL_HOST", "localhost") -POSTGRES_PASSWORD = os.getenv("POSTGRESQL_HOST", "testpassword") +POSTGRES_PASSWORD = os.getenv("POSTGRESQL_PASSWORD", "testpassword") POSTGRES_PORT = int(os.getenv("POSTGRESQL_PORT", "5432")) -POSTGRES_USER = os.getenv("POSTGRESQL_HOST", "testuser") +POSTGRES_USER = os.getenv("POSTGRESQL_USER", "testuser") REDIS_HOST = os.getenv("REDIS_HOST", "localhost") REDIS_PORT = int(os.getenv("REDIS_PORT ", "6379")) RETRY_COUNT = 8 From 7e513e6a4dc06716e9d0eedab48ac7235cf19069 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Fri, 20 Nov 2020 14:25:55 +0530 Subject: [PATCH 10/10] Add CHANGELOG entry --- .../opentelemetry-instrumentation-dbapi/CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/CHANGELOG.md b/instrumentation/opentelemetry-instrumentation-dbapi/CHANGELOG.md index 37f59c0187..0c13891d8d 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/CHANGELOG.md +++ b/instrumentation/opentelemetry-instrumentation-dbapi/CHANGELOG.md @@ -2,7 +2,10 @@ ## Unreleased -Stop capturing query parameters by default +- Update dbapi and its dependant instrumentations to follow semantic conventions + ([#195](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/195)) + +- Stop capturing query parameters by default ([#156](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/156)) ## Version 0.13b0