From 0031952b8b871c6e6e99380ceb6ad408c63951f0 Mon Sep 17 00:00:00 2001 From: stschenk Date: Thu, 15 Oct 2020 14:26:57 -0700 Subject: [PATCH 1/5] add param to control capturing of db.statement.parameters --- .../tests/test_aiopg_integration.py | 6 ++- .../instrumentation/dbapi/__init__.py | 4 +- .../tests/test_dbapi_integration.py | 44 +++++++++++++++++++ 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-aiopg/tests/test_aiopg_integration.py b/instrumentation/opentelemetry-instrumentation-aiopg/tests/test_aiopg_integration.py index 135f9ee9a78..a0ecaf7ee5d 100644 --- a/instrumentation/opentelemetry-instrumentation-aiopg/tests/test_aiopg_integration.py +++ b/instrumentation/opentelemetry-instrumentation-aiopg/tests/test_aiopg_integration.py @@ -199,7 +199,11 @@ def test_span_succeeded(self): "user": "user", } db_integration = AiopgIntegration( - self.tracer, "testcomponent", "testtype", connection_attributes + self.tracer, + "testcomponent", + "testtype", + connection_attributes, + capture_parameters=True, ) mock_connection = async_call( db_integration.wrapped_connection( 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 0dcdd5ba606..45cda7761ae 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -211,6 +211,7 @@ def __init__( connection_attributes=None, version: str = "", tracer_provider: typing.Optional[TracerProvider] = None, + capture_parameters: bool = False, ): self.connection_attributes = connection_attributes if self.connection_attributes is None: @@ -223,6 +224,7 @@ def __init__( self._name = name self._version = version self._tracer_provider = tracer_provider + self.capture_parameters = capture_parameters self.database_component = database_component self.database_type = database_type self.connection_props = {} @@ -327,7 +329,7 @@ def _populate_span( ) in self._db_api_integration.span_attributes.items(): span.set_attribute(attribute_key, attribute_value) - if len(args) > 1: + if self._db_api_integration.capture_parameters and len(args) > 1: span.set_attribute("db.statement.parameters", str(args[1])) def traced_execution( diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index e342e15aa34..b2ed5628260 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -53,6 +53,50 @@ def test_span_succeeded(self): self.assertEqual(span.name, "testcomponent.testdatabase") 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.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.canonical_code, + trace_api.status.StatusCanonicalCode.OK, + ) + + def test_span_succeeded_with_params(self): + connection_props = { + "database": "testdatabase", + "server_host": "testhost", + "server_port": 123, + "user": "testuser", + } + connection_attributes = { + "database": "database", + "port": "server_port", + "host": "server_host", + "user": "user", + } + db_integration = dbapi.DatabaseApiIntegration( + self.tracer, + "testcomponent", + "testtype", + connection_attributes, + capture_parameters=True, + ) + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, connection_props + ) + cursor = mock_connection.cursor() + cursor.execute("Test 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, "testcomponent.testdatabase") + 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") From 0d838bd094d9fa79b83d0170205155c16d73edf4 Mon Sep 17 00:00:00 2001 From: stschenk Date: Thu, 15 Oct 2020 14:43:13 -0700 Subject: [PATCH 2/5] update change log --- .../opentelemetry-instrumentation-dbapi/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/CHANGELOG.md b/instrumentation/opentelemetry-instrumentation-dbapi/CHANGELOG.md index e110055da1a..2460993eed0 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/CHANGELOG.md +++ b/instrumentation/opentelemetry-instrumentation-dbapi/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Stop capturing query parameters by default + ([#1247](https://github.com/open-telemetry/opentelemetry-python/pull/1247)) ## Version 0.13b0 Released 2020-09-17 From cc1902ec5f67d5a58496c130c2c392bf06ddaf9b Mon Sep 17 00:00:00 2001 From: stschenk Date: Mon, 2 Nov 2020 13:28:01 -0800 Subject: [PATCH 3/5] Add parameter to disable collection of db.statement.parameters for DBAPI --- .../tests/test_aiopg_integration.py | 6 +-- .../instrumentation/dbapi/__init__.py | 9 +++- .../tests/test_dbapi_integration.py | 44 ------------------- 3 files changed, 9 insertions(+), 50 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-aiopg/tests/test_aiopg_integration.py b/instrumentation/opentelemetry-instrumentation-aiopg/tests/test_aiopg_integration.py index 89e7cc05a5c..78ea4552e23 100644 --- a/instrumentation/opentelemetry-instrumentation-aiopg/tests/test_aiopg_integration.py +++ b/instrumentation/opentelemetry-instrumentation-aiopg/tests/test_aiopg_integration.py @@ -199,11 +199,7 @@ def test_span_succeeded(self): "user": "user", } db_integration = AiopgIntegration( - self.tracer, - "testcomponent", - "testtype", - connection_attributes, - capture_parameters=True, + self.tracer, "testcomponent", "testtype", connection_attributes ) mock_connection = async_call( db_integration.wrapped_connection( 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 12aad95cc21..cb8f37fd291 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -62,6 +62,7 @@ def trace_integration( database_type: str = "", connection_attributes: typing.Dict = None, tracer_provider: typing.Optional[TracerProvider] = None, + capture_parameters: bool = True, ): """Integrate with DB API library. https://www.python.org/dev/peps/pep-0249/ @@ -76,6 +77,7 @@ def trace_integration( 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__, @@ -86,6 +88,7 @@ def trace_integration( connection_attributes, version=__version__, tracer_provider=tracer_provider, + capture_parameters=capture_parameters, ) @@ -98,6 +101,7 @@ def wrap_connect( connection_attributes: typing.Dict = None, version: str = "", tracer_provider: typing.Optional[TracerProvider] = None, + capture_parameters: bool = True, ): """Integrate with DB API library. https://www.python.org/dev/peps/pep-0249/ @@ -111,6 +115,8 @@ def wrap_connect( 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. + """ # pylint: disable=unused-argument @@ -127,6 +133,7 @@ def wrap_connect_( connection_attributes=connection_attributes, version=version, tracer_provider=tracer_provider, + capture_parameters=capture_parameters, ) return db_integration.wrapped_connection(wrapped, args, kwargs) @@ -211,7 +218,7 @@ def __init__( connection_attributes=None, version: str = "", tracer_provider: typing.Optional[TracerProvider] = None, - capture_parameters: bool = False, + capture_parameters: bool = True, ): self.connection_attributes = connection_attributes if self.connection_attributes is None: diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index 26a1ef36c94..f2abb8b6dca 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -53,50 +53,6 @@ def test_span_succeeded(self): self.assertEqual(span.name, "testcomponent.testdatabase") 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.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.canonical_code, - trace_api.status.StatusCanonicalCode.OK, - ) - - def test_span_succeeded_with_params(self): - connection_props = { - "database": "testdatabase", - "server_host": "testhost", - "server_port": 123, - "user": "testuser", - } - connection_attributes = { - "database": "database", - "port": "server_port", - "host": "server_host", - "user": "user", - } - db_integration = dbapi.DatabaseApiIntegration( - self.tracer, - "testcomponent", - "testtype", - connection_attributes, - capture_parameters=True, - ) - mock_connection = db_integration.wrapped_connection( - mock_connect, {}, connection_props - ) - cursor = mock_connection.cursor() - cursor.execute("Test 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, "testcomponent.testdatabase") - 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") From 4a6293a2aa026d0425d5f58ef6d0794c46204fcc Mon Sep 17 00:00:00 2001 From: stschenk Date: Mon, 2 Nov 2020 13:48:43 -0800 Subject: [PATCH 4/5] Set DBAPI default capture of db.statement.parameters to False --- .../instrumentation/dbapi/__init__.py | 2 +- .../tests/test_dbapi_integration.py | 43 +++++++++++++++++++ 2 files changed, 44 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 cb8f37fd291..268d6031113 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -62,7 +62,7 @@ def trace_integration( database_type: str = "", connection_attributes: typing.Dict = None, tracer_provider: typing.Optional[TracerProvider] = None, - capture_parameters: bool = True, + capture_parameters: bool = False, ): """Integrate with DB API library. https://www.python.org/dev/peps/pep-0249/ diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index f2abb8b6dca..5e5f8f30648 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -68,6 +68,49 @@ def test_span_succeeded(self): span.status.status_code, trace_api.status.StatusCode.UNSET, ) + def test_span_succeeded_without_capture_of_statement_parameters(self): + connection_props = { + "database": "testdatabase", + "server_host": "testhost", + "server_port": 123, + "user": "testuser", + } + connection_attributes = { + "database": "database", + "port": "server_port", + "host": "server_host", + "user": "user", + } + db_integration = dbapi.DatabaseApiIntegration( + self.tracer, + "testcomponent", + "testtype", + connection_attributes, + capture_parameters=False, + ) + mock_connection = db_integration.wrapped_connection( + mock_connect, {}, connection_props + ) + cursor = mock_connection.cursor() + cursor.execute("Test 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, "testcomponent.testdatabase") + 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.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, + ) + def test_span_not_recording(self): connection_props = { "database": "testdatabase", From 7373a20eb68b92949532994217cf6aabab34c74a Mon Sep 17 00:00:00 2001 From: stschenk Date: Tue, 3 Nov 2020 08:56:26 -0800 Subject: [PATCH 5/5] Set DBApiIntegration capture of db.statement.parameters to False --- .../tests/test_aiopg_integration.py | 6 +++++- .../opentelemetry/instrumentation/dbapi/__init__.py | 8 +++++--- .../tests/test_dbapi_integration.py | 12 ++++++------ 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-aiopg/tests/test_aiopg_integration.py b/instrumentation/opentelemetry-instrumentation-aiopg/tests/test_aiopg_integration.py index 78ea4552e23..89e7cc05a5c 100644 --- a/instrumentation/opentelemetry-instrumentation-aiopg/tests/test_aiopg_integration.py +++ b/instrumentation/opentelemetry-instrumentation-aiopg/tests/test_aiopg_integration.py @@ -199,7 +199,11 @@ def test_span_succeeded(self): "user": "user", } db_integration = AiopgIntegration( - self.tracer, "testcomponent", "testtype", connection_attributes + self.tracer, + "testcomponent", + "testtype", + connection_attributes, + capture_parameters=True, ) mock_connection = async_call( db_integration.wrapped_connection( 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 268d6031113..39d7e3f3613 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -101,7 +101,7 @@ def wrap_connect( connection_attributes: typing.Dict = None, version: str = "", tracer_provider: typing.Optional[TracerProvider] = None, - capture_parameters: bool = True, + capture_parameters: bool = False, ): """Integrate with DB API library. https://www.python.org/dev/peps/pep-0249/ @@ -166,6 +166,7 @@ def instrument_connection( connection_attributes: typing.Dict = None, version: str = "", tracer_provider: typing.Optional[TracerProvider] = None, + capture_parameters=False, ): """Enable instrumentation in a database connection. @@ -177,7 +178,7 @@ def instrument_connection( database_type: The Database type. For any SQL database, "sql". connection_attributes: Attribute names for database, port, host and user in a connection object. - + capture_parameters: Configure if db.statement.parameters should be captured. Returns: An instrumented connection. """ @@ -188,6 +189,7 @@ def instrument_connection( connection_attributes=connection_attributes, version=version, tracer_provider=tracer_provider, + capture_parameters=capture_parameters, ) db_integration.get_connection_attributes(connection) return get_traced_connection_proxy(connection, db_integration) @@ -218,7 +220,7 @@ def __init__( connection_attributes=None, version: str = "", tracer_provider: typing.Optional[TracerProvider] = None, - capture_parameters: bool = True, + capture_parameters: bool = False, ): self.connection_attributes = connection_attributes if self.connection_attributes is None: diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py index 5e5f8f30648..a43bf8b0b86 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/tests/test_dbapi_integration.py @@ -57,10 +57,7 @@ def test_span_succeeded(self): self.assertEqual(span.attributes["db.type"], "testtype") self.assertEqual(span.attributes["db.instance"], "testdatabase") self.assertEqual(span.attributes["db.statement"], "Test query") - self.assertEqual( - span.attributes["db.statement.parameters"], - "('param1Value', False)", - ) + 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) @@ -86,7 +83,7 @@ def test_span_succeeded_without_capture_of_statement_parameters(self): "testcomponent", "testtype", connection_attributes, - capture_parameters=False, + capture_parameters=True, ) mock_connection = db_integration.wrapped_connection( mock_connect, {}, connection_props @@ -103,7 +100,10 @@ def test_span_succeeded_without_capture_of_statement_parameters(self): self.assertEqual(span.attributes["db.type"], "testtype") self.assertEqual(span.attributes["db.instance"], "testdatabase") self.assertEqual(span.attributes["db.statement"], "Test query") - self.assertFalse("db.statement.parameters" in span.attributes) + self.assertEqual( + span.attributes["db.statement.parameters"], + "('param1Value', False)", + ) self.assertEqual(span.attributes["db.user"], "testuser") self.assertEqual(span.attributes["net.peer.name"], "testhost") self.assertEqual(span.attributes["net.peer.port"], 123)