From baeaadf3fa3b483208d9d1db6eb8ee7c446e5107 Mon Sep 17 00:00:00 2001 From: brettlangdon Date: Thu, 13 Dec 2018 13:05:38 -0500 Subject: [PATCH 01/13] [dbapi2] allow disabling commit/rollback/fetch* tracing --- ddtrace/contrib/dbapi/__init__.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/ddtrace/contrib/dbapi/__init__.py b/ddtrace/contrib/dbapi/__init__.py index 76c31dd010f..5ff59e1cf6f 100644 --- a/ddtrace/contrib/dbapi/__init__.py +++ b/ddtrace/contrib/dbapi/__init__.py @@ -8,9 +8,16 @@ from ddtrace import Pin from ddtrace.ext import AppTypes, sql +from ddtrace.settings import config log = logging.getLogger(__name__) +config._add('_dbapi2', dict( + connection_trace_commit=True, + connection_trace_rollback=True, + cursor_trace_fetch=True, +)) + class TracedCursor(wrapt.ObjectProxy): """ TracedCursor wraps a psql cursor and traces it's queries. """ @@ -74,18 +81,27 @@ def execute(self, query, *args, **kwargs): def fetchone(self, *args, **kwargs): """ Wraps the cursor.fetchone method""" + if not config._dbapi2.cursor_trace_fetch: + return self.__wrapped__.fetchone(*args, **kwargs) + span_name = '{}.{}'.format(self._self_datadog_name, 'fetchone') return self._trace_method(self.__wrapped__.fetchone, span_name, self._self_last_execute_operation, {}, *args, **kwargs) def fetchall(self, *args, **kwargs): """ Wraps the cursor.fetchall method""" + if not config._dbapi2.cursor_trace_fetch: + return self.__wrapped__.fetchall(*args, **kwargs) + span_name = '{}.{}'.format(self._self_datadog_name, 'fetchall') return self._trace_method(self.__wrapped__.fetchall, span_name, self._self_last_execute_operation, {}, *args, **kwargs) def fetchmany(self, *args, **kwargs): """ Wraps the cursor.fetchmany method""" + if not config._dbapi2.cursor_trace_fetch: + return self.__wrapped__.fetchmany(*args, **kwargs) + span_name = '{}.{}'.format(self._self_datadog_name, 'fetchmany') # We want to trace the information about how many rows were requested. Note that this number may be larger # the number of rows actually returned if less then requested are available from the query. @@ -149,10 +165,16 @@ def cursor(self, *args, **kwargs): return self._self_cursor_cls(cursor, pin) def commit(self, *args, **kwargs): + if not config._dbapi2.connection_trace_commit: + return self.__wrapped__.commit(*args, **kwargs) + span_name = '{}.{}'.format(self._self_datadog_name, 'commit') return self._trace_method(self.__wrapped__.commit, span_name, {}, *args, **kwargs) def rollback(self, *args, **kwargs): + if not config._dbapi2.connection_trace_rollback: + return self.__wrapped__.rollback(*args, **kwargs) + span_name = '{}.{}'.format(self._self_datadog_name, 'rollback') return self._trace_method(self.__wrapped__.rollback, span_name, {}, *args, **kwargs) From 3c5a577356ccdc157376af178a1121fa07009852 Mon Sep 17 00:00:00 2001 From: brettlangdon Date: Mon, 17 Dec 2018 09:14:39 -0500 Subject: [PATCH 02/13] [dbapi2] Use subclass for tracing fetch* methods --- ddtrace/contrib/dbapi/__init__.py | 65 +++++++++++++++---------------- 1 file changed, 31 insertions(+), 34 deletions(-) diff --git a/ddtrace/contrib/dbapi/__init__.py b/ddtrace/contrib/dbapi/__init__.py index 5ff59e1cf6f..030dc7139e4 100644 --- a/ddtrace/contrib/dbapi/__init__.py +++ b/ddtrace/contrib/dbapi/__init__.py @@ -12,10 +12,8 @@ log = logging.getLogger(__name__) -config._add('_dbapi2', dict( - connection_trace_commit=True, - connection_trace_rollback=True, - cursor_trace_fetch=True, +config._add('dbapi2', dict( + cursor_trace_fetch=False, )) @@ -79,29 +77,41 @@ def execute(self, query, *args, **kwargs): self._trace_method(self.__wrapped__.execute, self._self_datadog_name, query, {}, query, *args, **kwargs) return self + def callproc(self, proc, args): + """ Wraps the cursor.callproc method""" + self._self_last_execute_operation = proc + return self._trace_method(self.__wrapped__.callproc, self._self_datadog_name, proc, {}, proc, args) + + def __enter__(self): + # previous versions of the dbapi didn't support context managers. let's + # reference the func that would be called to ensure that errors + # messages will be the same. + self.__wrapped__.__enter__ + + # and finally, yield the traced cursor. + return self + + +class FetchTracedCursor(TracedCursor): + """ + Sub-class of :class:`TracedCursor` that also instruments `fetchone`, `fetchall`, and `fetchmany` methods. + + We do not trace these functions by default since they can get very noisy (e.g. `fetchone` with 100k rows). + """ def fetchone(self, *args, **kwargs): """ Wraps the cursor.fetchone method""" - if not config._dbapi2.cursor_trace_fetch: - return self.__wrapped__.fetchone(*args, **kwargs) - span_name = '{}.{}'.format(self._self_datadog_name, 'fetchone') return self._trace_method(self.__wrapped__.fetchone, span_name, self._self_last_execute_operation, {}, *args, **kwargs) def fetchall(self, *args, **kwargs): """ Wraps the cursor.fetchall method""" - if not config._dbapi2.cursor_trace_fetch: - return self.__wrapped__.fetchall(*args, **kwargs) - span_name = '{}.{}'.format(self._self_datadog_name, 'fetchall') return self._trace_method(self.__wrapped__.fetchall, span_name, self._self_last_execute_operation, {}, *args, **kwargs) def fetchmany(self, *args, **kwargs): """ Wraps the cursor.fetchmany method""" - if not config._dbapi2.cursor_trace_fetch: - return self.__wrapped__.fetchmany(*args, **kwargs) - span_name = '{}.{}'.format(self._self_datadog_name, 'fetchmany') # We want to trace the information about how many rows were requested. Note that this number may be larger # the number of rows actually returned if less then requested are available from the query. @@ -117,25 +127,18 @@ def fetchmany(self, *args, **kwargs): return self._trace_method(self.__wrapped__.fetchmany, span_name, self._self_last_execute_operation, extra_tags, *args, **kwargs) - def callproc(self, proc, args): - """ Wraps the cursor.callproc method""" - self._self_last_execute_operation = proc - return self._trace_method(self.__wrapped__.callproc, self._self_datadog_name, proc, {}, proc, args) - - def __enter__(self): - # previous versions of the dbapi didn't support context managers. let's - # reference the func that would be called to ensure that errors - # messages will be the same. - self.__wrapped__.__enter__ - - # and finally, yield the traced cursor. - return self - class TracedConnection(wrapt.ObjectProxy): """ TracedConnection wraps a Connection with tracing code. """ - def __init__(self, conn, pin=None, cursor_cls=TracedCursor): + def __init__(self, conn, pin=None, cursor_cls=None): + # Set default cursor class if one was not provided + if not cursor_cls: + # Do not trace `fetch*` methods by default + cursor_cls = TracedCursor + if config.dbapi2.cursor_trace_fetch: + cursor_cls = FetchTracedCursor + super(TracedConnection, self).__init__(conn) name = _get_vendor(conn) self._self_datadog_name = '{}.connection'.format(name) @@ -165,16 +168,10 @@ def cursor(self, *args, **kwargs): return self._self_cursor_cls(cursor, pin) def commit(self, *args, **kwargs): - if not config._dbapi2.connection_trace_commit: - return self.__wrapped__.commit(*args, **kwargs) - span_name = '{}.{}'.format(self._self_datadog_name, 'commit') return self._trace_method(self.__wrapped__.commit, span_name, {}, *args, **kwargs) def rollback(self, *args, **kwargs): - if not config._dbapi2.connection_trace_rollback: - return self.__wrapped__.rollback(*args, **kwargs) - span_name = '{}.{}'.format(self._self_datadog_name, 'rollback') return self._trace_method(self.__wrapped__.rollback, span_name, {}, *args, **kwargs) From 4e76e0d99681a368e23db5dc63cfb00c2efbf715 Mon Sep 17 00:00:00 2001 From: brettlangdon Date: Mon, 17 Dec 2018 09:28:23 -0500 Subject: [PATCH 03/13] change configuration name --- ddtrace/contrib/dbapi/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ddtrace/contrib/dbapi/__init__.py b/ddtrace/contrib/dbapi/__init__.py index 030dc7139e4..67e58e82bd8 100644 --- a/ddtrace/contrib/dbapi/__init__.py +++ b/ddtrace/contrib/dbapi/__init__.py @@ -13,7 +13,7 @@ log = logging.getLogger(__name__) config._add('dbapi2', dict( - cursor_trace_fetch=False, + trace_fetch_methods=False, )) @@ -136,7 +136,7 @@ def __init__(self, conn, pin=None, cursor_cls=None): if not cursor_cls: # Do not trace `fetch*` methods by default cursor_cls = TracedCursor - if config.dbapi2.cursor_trace_fetch: + if config.dbapi2.trace_fetch_methods: cursor_cls = FetchTracedCursor super(TracedConnection, self).__init__(conn) From 53937312d351fd5b4014a65b86a4fcea70626fb9 Mon Sep 17 00:00:00 2001 From: brettlangdon Date: Mon, 17 Dec 2018 09:29:47 -0500 Subject: [PATCH 04/13] [dbapi2] update tests --- tests/contrib/dbapi/test_unit.py | 241 ++++++++++++++++++++++++++++--- 1 file changed, 224 insertions(+), 17 deletions(-) diff --git a/tests/contrib/dbapi/test_unit.py b/tests/contrib/dbapi/test_unit.py index fe9fc75b25e..407e0e193f3 100644 --- a/tests/contrib/dbapi/test_unit.py +++ b/tests/contrib/dbapi/test_unit.py @@ -2,15 +2,16 @@ import mock from ddtrace import Pin -from ddtrace.contrib.dbapi import TracedCursor, TracedConnection +from ddtrace.contrib.dbapi import FetchTracedCursor, TracedCursor, TracedConnection from tests.test_tracer import get_dummy_tracer +from ...base import BaseTracerTestCase -class TestTracedCursor(unittest.TestCase): +class TestTracedCursor(BaseTracerTestCase): def setUp(self): + super(TestTracedCursor, self).setUp() self.cursor = mock.Mock() - self.tracer = get_dummy_tracer() def test_execute_wrapped_is_called_and_returned(self): cursor = self.cursor @@ -63,22 +64,25 @@ def test_correct_span_names(self): traced_cursor = TracedCursor(cursor, pin) traced_cursor.execute('arg_1', kwarg1='kwarg1') - assert tracer.writer.pop()[0].name == 'sql.query' + self.assert_structure(dict(name='sql.query')) + self.reset() traced_cursor.executemany('arg_1', kwarg1='kwarg1') - assert tracer.writer.pop()[0].name == 'sql.query' + self.assert_structure(dict(name='sql.query')) + self.reset() traced_cursor.callproc('arg_1', 'arg2') - assert tracer.writer.pop()[0].name == 'sql.query' + self.assert_structure(dict(name='sql.query')) + self.reset() traced_cursor.fetchone('arg_1', kwarg1='kwarg1') - assert tracer.writer.pop()[0].name == 'sql.query.fetchone' + self.assert_has_no_spans() traced_cursor.fetchmany('arg_1', kwarg1='kwarg1') - assert tracer.writer.pop()[0].name == 'sql.query.fetchmany' + self.assert_has_no_spans() traced_cursor.fetchall('arg_1', kwarg1='kwarg1') - assert tracer.writer.pop()[0].name == 'sql.query.fetchall' + self.assert_has_no_spans() def test_correct_span_names_can_be_overridden_by_pin(self): cursor = self.cursor @@ -88,22 +92,25 @@ def test_correct_span_names_can_be_overridden_by_pin(self): traced_cursor = TracedCursor(cursor, pin) traced_cursor.execute('arg_1', kwarg1='kwarg1') - assert tracer.writer.pop()[0].name == 'changed.query' + self.assert_structure(dict(name='changed.query')) + self.reset() traced_cursor.executemany('arg_1', kwarg1='kwarg1') - assert tracer.writer.pop()[0].name == 'changed.query' + self.assert_structure(dict(name='changed.query')) + self.reset() traced_cursor.callproc('arg_1', 'arg2') - assert tracer.writer.pop()[0].name == 'changed.query' + self.assert_structure(dict(name='changed.query')) + self.reset() traced_cursor.fetchone('arg_1', kwarg1='kwarg1') - assert tracer.writer.pop()[0].name == 'changed.query.fetchone' + self.assert_has_no_spans() traced_cursor.fetchmany('arg_1', kwarg1='kwarg1') - assert tracer.writer.pop()[0].name == 'changed.query.fetchmany' + self.assert_has_no_spans() traced_cursor.fetchall('arg_1', kwarg1='kwarg1') - assert tracer.writer.pop()[0].name == 'changed.query.fetchall' + self.assert_has_no_spans() def test_when_pin_disabled_then_no_tracing(self): cursor = self.cursor @@ -177,10 +184,210 @@ def method(): assert span.get_tag('sql.rows') == '123', 'Row count is set as a tag (for legacy django cursor replacement)' -class TestTracedConnection(unittest.TestCase): +class TestFetchTracedCursor(BaseTracerTestCase): + def setUp(self): + super(TestFetchTracedCursor, self).setUp() + self.cursor = mock.Mock() + + def test_execute_wrapped_is_called_and_returned(self): + cursor = self.cursor + cursor.rowcount = 0 + pin = Pin('pin_name', tracer=self.tracer) + traced_cursor = FetchTracedCursor(cursor, pin) + assert traced_cursor is traced_cursor.execute('__query__', 'arg_1', kwarg1='kwarg1') + cursor.execute.assert_called_once_with('__query__', 'arg_1', kwarg1='kwarg1') + + def test_executemany_wrapped_is_called_and_returned(self): + cursor = self.cursor + cursor.rowcount = 0 + pin = Pin('pin_name', tracer=self.tracer) + traced_cursor = FetchTracedCursor(cursor, pin) + assert traced_cursor is traced_cursor.executemany('__query__', 'arg_1', kwarg1='kwarg1') + cursor.executemany.assert_called_once_with('__query__', 'arg_1', kwarg1='kwarg1') + + def test_fetchone_wrapped_is_called_and_returned(self): + cursor = self.cursor + cursor.rowcount = 0 + cursor.fetchone.return_value = '__result__' + pin = Pin('pin_name', tracer=self.tracer) + traced_cursor = FetchTracedCursor(cursor, pin) + assert '__result__' == traced_cursor.fetchone('arg_1', kwarg1='kwarg1') + cursor.fetchone.assert_called_once_with('arg_1', kwarg1='kwarg1') + + def test_fetchall_wrapped_is_called_and_returned(self): + cursor = self.cursor + cursor.rowcount = 0 + cursor.fetchall.return_value = '__result__' + pin = Pin('pin_name', tracer=self.tracer) + traced_cursor = FetchTracedCursor(cursor, pin) + assert '__result__' == traced_cursor.fetchall('arg_1', kwarg1='kwarg1') + cursor.fetchall.assert_called_once_with('arg_1', kwarg1='kwarg1') + + def test_fetchmany_wrapped_is_called_and_returned(self): + cursor = self.cursor + cursor.rowcount = 0 + cursor.fetchmany.return_value = '__result__' + pin = Pin('pin_name', tracer=self.tracer) + traced_cursor = FetchTracedCursor(cursor, pin) + assert '__result__' == traced_cursor.fetchmany('arg_1', kwarg1='kwarg1') + cursor.fetchmany.assert_called_once_with('arg_1', kwarg1='kwarg1') + + def test_correct_span_names(self): + cursor = self.cursor + tracer = self.tracer + cursor.rowcount = 0 + pin = Pin('pin_name', tracer=tracer) + traced_cursor = FetchTracedCursor(cursor, pin) + + traced_cursor.execute('arg_1', kwarg1='kwarg1') + self.assert_structure(dict(name='sql.query')) + self.reset() + + traced_cursor.executemany('arg_1', kwarg1='kwarg1') + self.assert_structure(dict(name='sql.query')) + self.reset() + + traced_cursor.callproc('arg_1', 'arg2') + self.assert_structure(dict(name='sql.query')) + self.reset() + + traced_cursor.fetchone('arg_1', kwarg1='kwarg1') + self.assert_structure(dict(name='sql.query.fetchone')) + self.reset() + + traced_cursor.fetchmany('arg_1', kwarg1='kwarg1') + self.assert_structure(dict(name='sql.query.fetchmany')) + self.reset() + + traced_cursor.fetchall('arg_1', kwarg1='kwarg1') + self.assert_structure(dict(name='sql.query.fetchall')) + self.reset() + + def test_correct_span_names_can_be_overridden_by_pin(self): + cursor = self.cursor + tracer = self.tracer + cursor.rowcount = 0 + pin = Pin('pin_name', app='changed', tracer=tracer) + traced_cursor = FetchTracedCursor(cursor, pin) + + traced_cursor.execute('arg_1', kwarg1='kwarg1') + self.assert_structure(dict(name='changed.query')) + self.reset() + + traced_cursor.executemany('arg_1', kwarg1='kwarg1') + self.assert_structure(dict(name='changed.query')) + self.reset() + + traced_cursor.callproc('arg_1', 'arg2') + self.assert_structure(dict(name='changed.query')) + self.reset() + + traced_cursor.fetchone('arg_1', kwarg1='kwarg1') + self.assert_structure(dict(name='changed.query.fetchone')) + self.reset() + + traced_cursor.fetchmany('arg_1', kwarg1='kwarg1') + self.assert_structure(dict(name='changed.query.fetchmany')) + self.reset() + + traced_cursor.fetchall('arg_1', kwarg1='kwarg1') + self.assert_structure(dict(name='changed.query.fetchall')) + self.reset() + + def test_when_pin_disabled_then_no_tracing(self): + cursor = self.cursor + tracer = self.tracer + cursor.rowcount = 0 + tracer.enabled = False + pin = Pin('pin_name', tracer=tracer) + traced_cursor = FetchTracedCursor(cursor, pin) + + assert traced_cursor is traced_cursor.execute('arg_1', kwarg1='kwarg1') + assert len(tracer.writer.pop()) == 0 + + assert traced_cursor is traced_cursor.executemany('arg_1', kwarg1='kwarg1') + assert len(tracer.writer.pop()) == 0 + + cursor.callproc.return_value = 'callproc' + assert 'callproc' == traced_cursor.callproc('arg_1', 'arg_2') + assert len(tracer.writer.pop()) == 0 + + cursor.fetchone.return_value = 'fetchone' + assert 'fetchone' == traced_cursor.fetchone('arg_1', 'arg_2') + assert len(tracer.writer.pop()) == 0 + + cursor.fetchmany.return_value = 'fetchmany' + assert 'fetchmany' == traced_cursor.fetchmany('arg_1', 'arg_2') + assert len(tracer.writer.pop()) == 0 + + cursor.fetchall.return_value = 'fetchall' + assert 'fetchall' == traced_cursor.fetchall('arg_1', 'arg_2') + assert len(tracer.writer.pop()) == 0 + + def test_span_info(self): + cursor = self.cursor + tracer = self.tracer + cursor.rowcount = 123 + pin = Pin('my_service', app='my_app', tracer=tracer, tags={'pin1': 'value_pin1'}) + traced_cursor = FetchTracedCursor(cursor, pin) + + def method(): + pass + + traced_cursor._trace_method(method, 'my_name', 'my_resource', {'extra1': 'value_extra1'}) + span = tracer.writer.pop()[0] # type: Span + assert span.meta['pin1'] == 'value_pin1', 'Pin tags are preserved' + assert span.meta['extra1'] == 'value_extra1', 'Extra tags are merged into pin tags' + assert span.name == 'my_name', 'Span name is respected' + assert span.service == 'my_service', 'Service from pin' + assert span.resource == 'my_resource', 'Resource is respected' + assert span.span_type == 'sql', 'Span has the correct span type' + # Row count + assert span.get_metric('db.rowcount') == 123, 'Row count is set as a metric' + assert span.get_tag('sql.rows') == '123', 'Row count is set as a tag (for legacy django cursor replacement)' + + def test_django_traced_cursor_backward_compatibility(self): + cursor = self.cursor + tracer = self.tracer + # Django integration used to have its own FetchTracedCursor implementation. When we replaced such custom + # implementation with the generic dbapi traced cursor, we had to make sure to add the tag 'sql.rows' that was + # set by the legacy replaced implementation. + cursor.rowcount = 123 + pin = Pin('my_service', app='my_app', tracer=tracer, tags={'pin1': 'value_pin1'}) + traced_cursor = FetchTracedCursor(cursor, pin) + + def method(): + pass + + traced_cursor._trace_method(method, 'my_name', 'my_resource', {'extra1': 'value_extra1'}) + span = tracer.writer.pop()[0] # type: Span + # Row count + assert span.get_metric('db.rowcount') == 123, 'Row count is set as a metric' + assert span.get_tag('sql.rows') == '123', 'Row count is set as a tag (for legacy django cursor replacement)' + + +class TestTracedConnection(BaseTracerTestCase): + def setUp(self): + super(TestTracedConnection, self).setUp() self.connection = mock.Mock() - self.tracer = get_dummy_tracer() + + def test_cursor_class(self): + pin = Pin('pin_name', tracer=self.tracer) + + # Default + traced_connection = TracedConnection(self.connection, pin=pin) + self.assertTrue(traced_connection._self_cursor_cls is TracedCursor) + + # Trace fetched methods + with self.override_config('dbapi2', dict(trace_fetch_methods=True)): + traced_connection = TracedConnection(self.connection, pin=pin) + self.assertTrue(traced_connection._self_cursor_cls is FetchTracedCursor) + + # Manually provided cursor class + with self.override_config('dbapi2', dict(trace_fetch_methods=True)): + traced_connection = TracedConnection(self.connection, pin=pin, cursor_cls=TracedCursor) + self.assertTrue(traced_connection._self_cursor_cls is TracedCursor) def test_commit_is_traced(self): connection = self.connection From f686a5537b5ef6a8b2149d835c039e5dd7623ab5 Mon Sep 17 00:00:00 2001 From: brettlangdon Date: Mon, 17 Dec 2018 09:31:18 -0500 Subject: [PATCH 05/13] remove unused imports --- tests/contrib/dbapi/test_unit.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/contrib/dbapi/test_unit.py b/tests/contrib/dbapi/test_unit.py index 407e0e193f3..0d5b8e7838a 100644 --- a/tests/contrib/dbapi/test_unit.py +++ b/tests/contrib/dbapi/test_unit.py @@ -1,9 +1,7 @@ -import unittest import mock from ddtrace import Pin from ddtrace.contrib.dbapi import FetchTracedCursor, TracedCursor, TracedConnection -from tests.test_tracer import get_dummy_tracer from ...base import BaseTracerTestCase From f3a7c2897ba0eb8724152fa91782032497f715e3 Mon Sep 17 00:00:00 2001 From: brettlangdon Date: Tue, 18 Dec 2018 16:18:59 -0500 Subject: [PATCH 06/13] Fix sqlite3 tests --- tests/contrib/sqlite3/test_sqlite3.py | 335 ++++++++++++-------------- tests/utils/span.py | 14 ++ 2 files changed, 171 insertions(+), 178 deletions(-) diff --git a/tests/contrib/sqlite3/test_sqlite3.py b/tests/contrib/sqlite3/test_sqlite3.py index b0445ac8c5d..78fb10ef54a 100644 --- a/tests/contrib/sqlite3/test_sqlite3.py +++ b/tests/contrib/sqlite3/test_sqlite3.py @@ -2,9 +2,6 @@ import sqlite3 import time -# 3p -from nose.tools import eq_, ok_ - # project import ddtrace from ddtrace import Pin @@ -14,58 +11,54 @@ # testing from tests.opentracer.utils import init_tracer -from tests.test_tracer import get_dummy_tracer - - -def test_backwards_compat(): - # a small test to ensure that if the previous interface is used - # things still work - tracer = get_dummy_tracer() - factory = connection_factory(tracer, service='my_db_service') - conn = sqlite3.connect(':memory:', factory=factory) - q = 'select * from sqlite_master' - rows = conn.execute(q) - assert not rows.fetchall() - assert not tracer.writer.pop() +from ...base import BaseTracerTestCase -class TestSQLite(object): +class TestSQLite(BaseTracerTestCase): def setUp(self): + super(TestSQLite, self).setUp() patch() def tearDown(self): unpatch() + super(TestSQLite, self).tearDown() + + def test_backwards_compat(self): + # a small test to ensure that if the previous interface is used + # things still work + factory = connection_factory(self.tracer, service='my_db_service') + conn = sqlite3.connect(':memory:', factory=factory) + q = 'select * from sqlite_master' + rows = conn.execute(q) + assert not rows.fetchall() + assert not self.spans def test_service_info(self): - tracer = get_dummy_tracer() backup_tracer = ddtrace.tracer - ddtrace.tracer = tracer + ddtrace.tracer = self.tracer sqlite3.connect(':memory:') - services = tracer.writer.pop_services() - eq_(len(services), 1) + services = self.tracer.writer.pop_services() + self.assertEqual(len(services), 1) expected = { 'sqlite': {'app': 'sqlite', 'app_type': 'db'} } - eq_(expected, services) + self.assertEqual(expected, services) ddtrace.tracer = backup_tracer def test_sqlite(self): - tracer = get_dummy_tracer() - writer = tracer.writer - # ensure we can trace multiple services without stomping services = ['db', 'another'] for service in services: db = sqlite3.connect(':memory:') pin = Pin.get_from(db) assert pin - eq_('db', pin.app_type) + self.assertEqual('db', pin.app_type) pin.clone( service=service, - tracer=tracer).onto(db) + tracer=self.tracer).onto(db) # Ensure we can run a query and it's correctly traced q = 'select * from sqlite_master' @@ -74,21 +67,14 @@ def test_sqlite(self): rows = cursor.fetchall() end = time.time() assert not rows - spans = writer.pop() - assert spans - eq_(len(spans), 2) - span = spans[0] - eq_(span.name, 'sqlite.query') - eq_(span.span_type, 'sql') - eq_(span.resource, q) - eq_(span.service, service) - ok_(span.get_tag('sql.query') is None) - eq_(span.error, 0) - assert start <= span.start <= end - assert span.duration <= end - start - - fetch_span = spans[1] - eq_(fetch_span.name, 'sqlite.query.fetchall') + self.assert_structure( + dict(name='sqlite.query', span_type='sql', resource=q, service=service, error=0), + ) + root = self.get_root_span() + self.assertIsNone(root.get_tag('sql.query')) + assert start <= root.start <= end + assert root.duration <= end - start + self.reset() # run a query with an error and ensure all is well q = 'select * from some_non_existant_table' @@ -98,109 +84,110 @@ def test_sqlite(self): pass else: assert 0, 'should have an error' - spans = writer.pop() - assert spans - eq_(len(spans), 1) - span = spans[0] - eq_(span.name, 'sqlite.query') - eq_(span.resource, q) - eq_(span.service, service) - ok_(span.get_tag('sql.query') is None) - eq_(span.error, 1) - eq_(span.span_type, 'sql') - assert span.get_tag(errors.ERROR_STACK) - assert 'OperationalError' in span.get_tag(errors.ERROR_TYPE) - assert 'no such table' in span.get_tag(errors.ERROR_MSG) + + self.assert_structure( + dict(name='sqlite.query', span_type='sql', resource=q, service=service, error=1), + ) + root = self.get_root_span() + self.assertIsNone(root.get_tag('sql.query')) + self.assertIsNotNone(root.get_tag(errors.ERROR_STACK)) + self.assertIn('OperationalError', root.get_tag(errors.ERROR_TYPE)) + self.assertIn('no such table', root.get_tag(errors.ERROR_MSG)) + self.reset() def test_sqlite_fetchall_is_traced(self): - tracer = get_dummy_tracer() - connection = self._given_a_traced_connection(tracer) q = 'select * from sqlite_master' + + # Not traced by default + connection = self._given_a_traced_connection(self.tracer) cursor = connection.execute(q) cursor.fetchall() + self.assert_structure(dict(name='sqlite.query', resource=q)) + self.reset() - spans = tracer.writer.pop() + with self.override_config('dbapi2', dict(trace_fetch_methods=True)): + connection = self._given_a_traced_connection(self.tracer) + cursor = connection.execute(q) + cursor.fetchall() - eq_(len(spans), 2) + # We have two spans side by side + query_span, fetchall_span = self.get_root_spans() - execute_span = spans[0] - fetchall_span = spans[1] + # Assert query + query_span.assert_structure(dict(name='sqlite.query', resource=q)) - # Execute span - eq_(execute_span.name, 'sqlite.query') - eq_(execute_span.span_type, 'sql') - eq_(execute_span.resource, q) - ok_(execute_span.get_tag('sql.query') is None) - eq_(execute_span.error, 0) - # Fetchall span - eq_(fetchall_span.parent_id, None) - eq_(fetchall_span.name, 'sqlite.query.fetchall') - eq_(fetchall_span.span_type, 'sql') - eq_(fetchall_span.resource, q) - ok_(fetchall_span.get_tag('sql.query') is None) - eq_(fetchall_span.error, 0) + # Assert fetchall + fetchall_span.assert_structure(dict(name='sqlite.query.fetchall', resource=q, span_type='sql', error=0)) + self.assertIsNone(fetchall_span.get_tag('sql.query')) def test_sqlite_fetchone_is_traced(self): - tracer = get_dummy_tracer() - connection = self._given_a_traced_connection(tracer) q = 'select * from sqlite_master' + + # Not traced by default + connection = self._given_a_traced_connection(self.tracer) cursor = connection.execute(q) cursor.fetchone() - - spans = tracer.writer.pop() - - eq_(len(spans), 2) - - execute_span = spans[0] - fetchone_span = spans[1] - - # Execute span - eq_(execute_span.name, 'sqlite.query') - eq_(execute_span.span_type, 'sql') - eq_(execute_span.resource, q) - ok_(execute_span.get_tag('sql.query') is None) - eq_(execute_span.error, 0) - # Fetchone span - eq_(fetchone_span.parent_id, None) - eq_(fetchone_span.name, 'sqlite.query.fetchone') - eq_(fetchone_span.span_type, 'sql') - eq_(fetchone_span.resource, q) - ok_(fetchone_span.get_tag('sql.query') is None) - eq_(fetchone_span.error, 0) + self.assert_structure(dict(name='sqlite.query', resource=q)) + self.reset() + + with self.override_config('dbapi2', dict(trace_fetch_methods=True)): + connection = self._given_a_traced_connection(self.tracer) + cursor = connection.execute(q) + cursor.fetchone() + + # We have two spans side by side + query_span, fetchone_span = self.get_root_spans() + + # Assert query + query_span.assert_structure(dict(name='sqlite.query', resource=q)) + + # Assert fetchone + fetchone_span.assert_structure( + dict( + name='sqlite.query.fetchone', + resource=q, + span_type='sql', + error=0, + ), + ) + self.assertIsNone(fetchone_span.get_tag('sql.query')) def test_sqlite_fetchmany_is_traced(self): - tracer = get_dummy_tracer() - connection = self._given_a_traced_connection(tracer) q = 'select * from sqlite_master' + + # Not traced by default + connection = self._given_a_traced_connection(self.tracer) cursor = connection.execute(q) cursor.fetchmany(123) - - spans = tracer.writer.pop() - - eq_(len(spans), 2) - - execute_span = spans[0] - fetchmany_span = spans[1] - - # Execute span - eq_(execute_span.name, 'sqlite.query') - eq_(execute_span.span_type, 'sql') - eq_(execute_span.resource, q) - ok_(execute_span.get_tag('sql.query') is None) - eq_(execute_span.error, 0) - # Fetchmany span - eq_(fetchmany_span.parent_id, None) - eq_(fetchmany_span.name, 'sqlite.query.fetchmany') - eq_(fetchmany_span.span_type, 'sql') - eq_(fetchmany_span.resource, q) - ok_(fetchmany_span.get_tag('sql.query') is None) - eq_(fetchmany_span.error, 0) - eq_(fetchmany_span.get_tag('db.fetch.size'), '123') + self.assert_structure(dict(name='sqlite.query', resource=q)) + self.reset() + + with self.override_config('dbapi2', dict(trace_fetch_methods=True)): + connection = self._given_a_traced_connection(self.tracer) + cursor = connection.execute(q) + cursor.fetchmany(123) + + # We have two spans side by side + query_span, fetchmany_span = self.get_root_spans() + + # Assert query + query_span.assert_structure(dict(name='sqlite.query', resource=q)) + + # Assert fetchmany + fetchmany_span.assert_structure( + dict( + name='sqlite.query.fetchmany', + resource=q, + span_type='sql', + error=0, + meta={'db.fetch.size': '123'}, + ), + ) + self.assertIsNone(fetchmany_span.get_tag('sql.query')) def test_sqlite_ot(self): """Ensure sqlite works with the opentracer.""" - tracer = get_dummy_tracer() - ot_tracer = init_tracer('sqlite_svc', tracer) + ot_tracer = init_tracer('sqlite_svc', self.tracer) # Ensure we can run a query and it's correctly traced q = 'select * from sqlite_master' @@ -208,63 +195,55 @@ def test_sqlite_ot(self): db = sqlite3.connect(':memory:') pin = Pin.get_from(db) assert pin - eq_('db', pin.app_type) - pin.clone(tracer=tracer).onto(db) + self.assertEqual('db', pin.app_type) + pin.clone(tracer=self.tracer).onto(db) cursor = db.execute(q) rows = cursor.fetchall() assert not rows - spans = tracer.writer.pop() - assert spans - print(spans) - eq_(len(spans), 3) - ot_span, dd_span, fetchall_span = spans - - # confirm the parenting - eq_(ot_span.parent_id, None) - eq_(dd_span.parent_id, ot_span.span_id) - - eq_(ot_span.name, 'sqlite_op') - eq_(ot_span.service, 'sqlite_svc') - - eq_(dd_span.name, 'sqlite.query') - eq_(dd_span.span_type, 'sql') - eq_(dd_span.resource, q) - ok_(dd_span.get_tag('sql.query') is None) - eq_(dd_span.error, 0) - - eq_(fetchall_span.name, 'sqlite.query.fetchall') - eq_(fetchall_span.span_type, 'sql') - eq_(fetchall_span.resource, q) - ok_(fetchall_span.get_tag('sql.query') is None) - eq_(fetchall_span.error, 0) + self.assert_structure( + dict(name='sqlite_op', service='sqlite_svc'), + ( + dict(name='sqlite.query', span_type='sql', resource=q, error=0), + ) + ) + self.reset() + + with self.override_config('dbapi2', dict(trace_fetch_methods=True)): + with ot_tracer.start_active_span('sqlite_op'): + db = sqlite3.connect(':memory:') + pin = Pin.get_from(db) + assert pin + self.assertEqual('db', pin.app_type) + pin.clone(tracer=self.tracer).onto(db) + cursor = db.execute(q) + rows = cursor.fetchall() + assert not rows + + self.assert_structure( + dict(name='sqlite_op', service='sqlite_svc'), + ( + dict(name='sqlite.query', span_type='sql', resource=q, error=0), + dict(name='sqlite.query.fetchall', span_type='sql', resource=q, error=0), + ), + ) def test_commit(self): - tracer = get_dummy_tracer() - connection = self._given_a_traced_connection(tracer) - writer = tracer.writer + connection = self._given_a_traced_connection(self.tracer) connection.commit() - spans = writer.pop() - eq_(len(spans), 1) - span = spans[0] - eq_(span.service, 'sqlite') - eq_(span.name, 'sqlite.connection.commit') + self.assertEqual(len(self.spans), 1) + span = self.spans[0] + self.assertEqual(span.service, 'sqlite') + self.assertEqual(span.name, 'sqlite.connection.commit') def test_rollback(self): - tracer = get_dummy_tracer() - connection = self._given_a_traced_connection(tracer) - writer = tracer.writer + connection = self._given_a_traced_connection(self.tracer) connection.rollback() - spans = writer.pop() - eq_(len(spans), 1) - span = spans[0] - eq_(span.service, 'sqlite') - eq_(span.name, 'sqlite.connection.rollback') + self.assert_structure( + dict(name='sqlite.connection.rollback', service='sqlite'), + ) def test_patch_unpatch(self): - tracer = get_dummy_tracer() - writer = tracer.writer - # Test patch idempotence patch() patch() @@ -272,12 +251,13 @@ def test_patch_unpatch(self): db = sqlite3.connect(':memory:') pin = Pin.get_from(db) assert pin - pin.clone(tracer=tracer).onto(db) + pin.clone(tracer=self.tracer).onto(db) db.cursor().execute('select \'blah\'').fetchall() - spans = writer.pop() - assert spans, spans - eq_(len(spans), 2) + self.assert_structure( + dict(name='sqlite.query'), + ) + self.reset() # Test unpatch unpatch() @@ -285,8 +265,7 @@ def test_patch_unpatch(self): db = sqlite3.connect(':memory:') db.cursor().execute('select \'blah\'').fetchall() - spans = writer.pop() - assert not spans, spans + self.assert_has_no_spans() # Test patch again patch() @@ -294,12 +273,12 @@ def test_patch_unpatch(self): db = sqlite3.connect(':memory:') pin = Pin.get_from(db) assert pin - pin.clone(tracer=tracer).onto(db) + pin.clone(tracer=self.tracer).onto(db) db.cursor().execute('select \'blah\'').fetchall() - spans = writer.pop() - assert spans, spans - eq_(len(spans), 2) + self.assert_structure( + dict(name='sqlite.query'), + ) def _given_a_traced_connection(self, tracer): db = sqlite3.connect(':memory:') diff --git a/tests/utils/span.py b/tests/utils/span.py index e93ec044f72..8cb44e78057 100644 --- a/tests/utils/span.py +++ b/tests/utils/span.py @@ -234,6 +234,20 @@ def get_root_span(self): return self._build_tree(root) + def get_root_spans(self): + """ + Helper to get all root spans from the list of spans in this container + + :returns: The root spans if any were found, None if not + :rtype: list of :class:`tests.utils.span.TestSpanNode`, None + """ + roots = [] + for span in self.spans: + if span.parent_id is None: + roots.append(self._build_tree(span)) + + return sorted(roots, key=lambda s: s.start) + def assert_span_count(self, count): """Assert this container has the expected number of spans""" assert len(self.spans) == count, 'Span count {0} != {1}'.format(len(self.spans), count) From c1baa2f7b7342036d0fe043dd525f3d25f8f51b9 Mon Sep 17 00:00:00 2001 From: brettlangdon Date: Tue, 18 Dec 2018 17:10:11 -0500 Subject: [PATCH 07/13] fix psycopg2 tests --- ddtrace/contrib/psycopg/patch.py | 19 ++- tests/contrib/psycopg/test_psycopg.py | 221 ++++++++++++-------------- 2 files changed, 119 insertions(+), 121 deletions(-) diff --git a/ddtrace/contrib/psycopg/patch.py b/ddtrace/contrib/psycopg/patch.py index d10415a7275..0cc6936f509 100644 --- a/ddtrace/contrib/psycopg/patch.py +++ b/ddtrace/contrib/psycopg/patch.py @@ -3,7 +3,7 @@ import wrapt # project -from ddtrace import Pin +from ddtrace import Pin, config from ddtrace.contrib import dbapi from ddtrace.ext import sql, net, db @@ -51,14 +51,21 @@ def _trace_method(self, method, name, resource, extra_tags, *args, **kwargs): return super(Psycopg2TracedCursor, self)._trace_method(method, name, resource, extra_tags, *args, **kwargs) +class Psycopg2FetchTracedCursor(Psycopg2TracedCursor, dbapi.FetchTracedCursor): + """ FetchTracedCursor for psycopg2 """ + + class Psycopg2TracedConnection(dbapi.TracedConnection): """ TracedConnection wraps a Connection with tracing code. """ - def __init__(self, conn, pin=None, cursor_cls=Psycopg2TracedCursor): - super(Psycopg2TracedConnection, self).__init__(conn, pin) - # wrapt requires prefix of `_self` for attributes that are only in the - # proxy (since some of our source objects will use `__slots__`) - self._self_cursor_cls = cursor_cls + def __init__(self, conn, pin=None, cursor_cls=None): + if not cursor_cls: + # Do not trace `fetch*` methods by default + cursor_cls = Psycopg2TracedCursor + if config.dbapi2.trace_fetch_methods: + cursor_cls = Psycopg2FetchTracedCursor + + super(Psycopg2TracedConnection, self).__init__(conn, pin, cursor_cls=cursor_cls) def patch_conn(conn, traced_conn_cls=Psycopg2TracedConnection): diff --git a/tests/contrib/psycopg/test_psycopg.py b/tests/contrib/psycopg/test_psycopg.py index 8e8123d8d35..4bf3554a880 100644 --- a/tests/contrib/psycopg/test_psycopg.py +++ b/tests/contrib/psycopg/test_psycopg.py @@ -6,7 +6,6 @@ from psycopg2 import extensions from psycopg2 import extras -import unittest from unittest import skipIf # project @@ -17,7 +16,9 @@ # testing from tests.opentracer.utils import init_tracer from tests.contrib.config import POSTGRES_CONFIG -from tests.test_tracer import get_dummy_tracer +from ...base import BaseTracerTestCase +from ...utils.tracer import DummyTracer + if PSYCOPG2_VERSION >= (2, 7): from psycopg2.sql import SQL @@ -25,63 +26,56 @@ TEST_PORT = str(POSTGRES_CONFIG['port']) -class PsycopgCore(unittest.TestCase): +class PsycopgCore(BaseTracerTestCase): # default service TEST_SERVICE = 'postgres' def setUp(self): + super(PsycopgCore, self).setUp() + patch() def tearDown(self): + super(PsycopgCore, self).tearDown() + unpatch() - def _get_conn_and_tracer(self): + def _get_conn(self, service=None): conn = psycopg2.connect(**POSTGRES_CONFIG) - tracer = get_dummy_tracer() - Pin.get_from(conn).clone(tracer=tracer).onto(conn) + pin = Pin.get_from(conn) + if pin: + pin.clone(service=service, tracer=self.tracer).onto(conn) - return conn, tracer + return conn def test_patch_unpatch(self): - tracer = get_dummy_tracer() - writer = tracer.writer - # Test patch idempotence patch() patch() service = 'fo' - conn = psycopg2.connect(**POSTGRES_CONFIG) - Pin.get_from(conn).clone(service=service, tracer=tracer).onto(conn) + conn = self._get_conn(service=service) conn.cursor().execute("""select 'blah'""") - - spans = writer.pop() - assert spans, spans - self.assertEquals(len(spans), 1) + self.assert_structure(dict(name='postgres.query', service=service)) + self.reset() # Test unpatch unpatch() - conn = psycopg2.connect(**POSTGRES_CONFIG) + conn = self._get_conn() conn.cursor().execute("""select 'blah'""") - - spans = writer.pop() - assert not spans, spans + self.assert_has_no_spans() # Test patch again patch() - conn = psycopg2.connect(**POSTGRES_CONFIG) - Pin.get_from(conn).clone(service=service, tracer=tracer).onto(conn) + conn = self._get_conn(service=service) conn.cursor().execute("""select 'blah'""") + self.assert_structure(dict(name='postgres.query', service=service)) - spans = writer.pop() - assert spans, spans - self.assertEquals(len(spans), 1) - - def assert_conn_is_traced(self, tracer, db, service): + def assert_conn_is_traced(self, db, service): # ensure the trace pscyopg client doesn't add non-standard # methods @@ -90,31 +84,25 @@ def assert_conn_is_traced(self, tracer, db, service): except AttributeError: pass - writer = tracer.writer # Ensure we can run a query and it's correctly traced q = """select 'foobarblah'""" + start = time.time() cursor = db.cursor() cursor.execute(q) rows = cursor.fetchall() end = time.time() + self.assertEquals(rows, [('foobarblah',)]) - assert rows - spans = writer.pop() - assert spans - self.assertEquals(len(spans), 2) - span = spans[0] - self.assertEquals(span.name, 'postgres.query') - self.assertEquals(span.resource, q) - self.assertEquals(span.service, service) - self.assertIsNone(span.get_tag('sql.query')) - self.assertEquals(span.error, 0) - self.assertEquals(span.span_type, 'sql') - assert start <= span.start <= end - assert span.duration <= end - start - - fetch_span = spans[1] - self.assertEquals(fetch_span.name, "postgres.query.fetchall") + + self.assert_structure( + dict(name='postgres.query', resource=q, service=service, error=0, span_type='sql'), + ) + root = self.get_root_span() + self.assertIsNone(root.get_tag('sql.query')) + assert start <= root.start <= end + assert root.duration <= end - start + self.reset() # run a query with an error and ensure all is well q = """select * from some_non_existant_table""" @@ -125,24 +113,30 @@ def assert_conn_is_traced(self, tracer, db, service): pass else: assert 0, 'should have an error' - spans = writer.pop() - assert spans, spans - self.assertEquals(len(spans), 1) - span = spans[0] - self.assertEquals(span.name, 'postgres.query') - self.assertEquals(span.resource, q) - self.assertEquals(span.service, service) - self.assertIsNone(span.get_tag('sql.query')) - self.assertEquals(span.error, 1) - self.assertEquals(span.meta['out.host'], 'localhost') - self.assertEquals(span.meta['out.port'], TEST_PORT) - self.assertEquals(span.span_type, 'sql') + + self.assert_structure( + dict( + name='postgres.query', + resource=q, + service=service, + error=1, + span_type='sql', + meta={ + 'out.host': 'localhost', + 'out.port': TEST_PORT, + }, + ), + ) + root = self.get_root_span() + self.assertIsNone(root.get_tag('sql.query')) + self.reset() def test_opentracing_propagation(self): # ensure OpenTracing plays well with our integration query = """SELECT 'tracing'""" - db, tracer = self._get_conn_and_tracer() - ot_tracer = init_tracer('psycopg-svc', tracer) + + db = self._get_conn() + ot_tracer = init_tracer('psycopg-svc', self.tracer) with ot_tracer.start_active_span('db.access'): cursor = db.cursor() @@ -150,30 +144,39 @@ def test_opentracing_propagation(self): rows = cursor.fetchall() self.assertEquals(rows, [('tracing',)]) - spans = tracer.writer.pop() - self.assertEquals(len(spans), 3) - ot_span, dd_span, fetch_span = spans - # confirm the parenting - self.assertEquals(ot_span.parent_id, None) - self.assertEquals(dd_span.parent_id, ot_span.span_id) - # check the OpenTracing span - self.assertEquals(ot_span.name, "db.access") - self.assertEquals(ot_span.service, "psycopg-svc") - # make sure the Datadog span is unaffected by OpenTracing - self.assertEquals(dd_span.name, "postgres.query") - self.assertEquals(dd_span.resource, query) - self.assertEquals(dd_span.service, 'postgres') - self.assertTrue(dd_span.get_tag("sql.query") is None) - self.assertEquals(dd_span.error, 0) - self.assertEquals(dd_span.span_type, "sql") - - self.assertEquals(fetch_span.name, 'postgres.query.fetchall') + + self.assert_structure( + dict(name='db.access', service='psycopg-svc'), + ( + dict(name='postgres.query', resource=query, service='postgres', error=0, span_type='sql'), + ), + ) + self.reset() + + with self.override_config('dbapi2', dict(trace_fetch_methods=True)): + db = self._get_conn() + ot_tracer = init_tracer('psycopg-svc', self.tracer) + + with ot_tracer.start_active_span('db.access'): + cursor = db.cursor() + cursor.execute(query) + rows = cursor.fetchall() + + self.assertEquals(rows, [('tracing',)]) + + self.assert_structure( + dict(name='db.access', service='psycopg-svc'), + ( + dict(name='postgres.query', resource=query, service='postgres', error=0, span_type='sql'), + dict(name='postgres.query.fetchall', resource=query, service='postgres', error=0, span_type='sql'), + ), + ) @skipIf(PSYCOPG2_VERSION < (2, 5), 'context manager not available in psycopg2==2.4') def test_cursor_ctx_manager(self): # ensure cursors work with context managers # https://github.com/DataDog/dd-trace-py/issues/228 - conn, tracer = self._get_conn_and_tracer() + conn = self._get_conn() t = type(conn.cursor()) with conn.cursor() as cur: assert t == type(cur), '{} != {}'.format(t, type(cur)) @@ -182,23 +185,21 @@ def test_cursor_ctx_manager(self): assert len(rows) == 1, rows assert rows[0][0] == 'blah' - spans = tracer.writer.pop() - assert len(spans) == 2 - span, fetch_span = spans - self.assertEquals(span.name, 'postgres.query') - self.assertEquals(fetch_span.name, 'postgres.query.fetchall') + self.assert_structure( + dict(name='postgres.query'), + ) def test_disabled_execute(self): - conn, tracer = self._get_conn_and_tracer() - tracer.enabled = False + conn = self._get_conn() + self.tracer.enabled = False # these calls were crashing with a previous version of the code. conn.cursor().execute(query="""select 'blah'""") conn.cursor().execute("""select 'blah'""") - assert not tracer.writer.pop() + self.assert_has_no_spans() @skipIf(PSYCOPG2_VERSION < (2, 5), '_json is not available in psycopg2==2.4') def test_manual_wrap_extension_types(self): - conn, _ = self._get_conn_and_tracer() + conn = self._get_conn() # NOTE: this will crash if it doesn't work. # _ext.register_type(_ext.UUID, conn_or_curs) # TypeError: argument 2 must be a connection, cursor or None @@ -210,7 +211,7 @@ def test_manual_wrap_extension_types(self): extras.register_default_json(conn) def test_manual_wrap_extension_adapt(self): - conn, _ = self._get_conn_and_tracer() + conn = self._get_conn() # NOTE: this will crash if it doesn't work. # items = _ext.adapt([1, 2, 3]) # items.prepare(conn) @@ -237,16 +238,13 @@ def test_manual_wrap_extension_quote_ident(self): quote_ident('foo', conn) def test_connect_factory(self): - tracer = get_dummy_tracer() - services = ['db', 'another'] for service in services: - conn, _ = self._get_conn_and_tracer() - Pin.get_from(conn).clone(service=service, tracer=tracer).onto(conn) - self.assert_conn_is_traced(tracer, conn, service) + conn = self._get_conn(service=service) + self.assert_conn_is_traced(conn, service) # ensure we have the service types - service_meta = tracer.writer.pop_services() + service_meta = self.tracer.writer.pop_services() expected = { 'db': {'app': 'postgres', 'app_type': 'db'}, 'another': {'app': 'postgres', 'app_type': 'db'}, @@ -254,24 +252,20 @@ def test_connect_factory(self): self.assertEquals(service_meta, expected) def test_commit(self): - conn, tracer = self._get_conn_and_tracer() - writer = tracer.writer + conn = self._get_conn() conn.commit() - spans = writer.pop() - self.assertEquals(len(spans), 1) - span = spans[0] - self.assertEquals(span.service, self.TEST_SERVICE) - self.assertEquals(span.name, 'postgres.connection.commit') + + self.assert_structure( + dict(name='postgres.connection.commit', service=self.TEST_SERVICE) + ) def test_rollback(self): - conn, tracer = self._get_conn_and_tracer() - writer = tracer.writer + conn = self._get_conn() conn.rollback() - spans = writer.pop() - self.assertEquals(len(spans), 1) - span = spans[0] - self.assertEquals(span.service, self.TEST_SERVICE) - self.assertEquals(span.name, 'postgres.connection.rollback') + + self.assert_structure( + dict(name='postgres.connection.rollback', service=self.TEST_SERVICE) + ) @skipIf(PSYCOPG2_VERSION < (2, 7), 'SQL string composition not available in psycopg2<2.7') def test_composed_query(self): @@ -279,7 +273,7 @@ def test_composed_query(self): query = SQL(' union all ').join( [SQL("""select 'one' as x"""), SQL("""select 'two' as x""")]) - db, tracer = self._get_conn_and_tracer() + db = self._get_conn() with db.cursor() as cur: cur.execute(query=query) @@ -288,16 +282,13 @@ def test_composed_query(self): assert rows[0][0] == 'one' assert rows[1][0] == 'two' - spans = tracer.writer.pop() - assert len(spans) == 2 - span, fetch_span = spans - self.assertEquals(span.name, 'postgres.query') - self.assertEquals(span.resource, query.as_string(db)) - self.assertEquals(fetch_span.name, 'postgres.query.fetchall') + self.assert_structure( + dict(name='postgres.query', resource=query.as_string(db)), + ) def test_backwards_compatibilty_v3(): - tracer = get_dummy_tracer() + tracer = DummyTracer() factory = connection_factory(tracer, service='my-postgres-db') conn = psycopg2.connect(connection_factory=factory, **POSTGRES_CONFIG) conn.cursor().execute("""select 'blah'""") From 66c7349af7fbae5b0893b0f89fe842e216d91a5a Mon Sep 17 00:00:00 2001 From: brettlangdon Date: Thu, 20 Dec 2018 11:06:25 -0500 Subject: [PATCH 08/13] fix mysqldb tests --- tests/contrib/mysqldb/test_mysql.py | 187 +++++++++++++++++++++++----- 1 file changed, 158 insertions(+), 29 deletions(-) diff --git a/tests/contrib/mysqldb/test_mysql.py b/tests/contrib/mysqldb/test_mysql.py index 9f500aa29ba..051e42703b6 100644 --- a/tests/contrib/mysqldb/test_mysql.py +++ b/tests/contrib/mysqldb/test_mysql.py @@ -7,8 +7,8 @@ from tests.opentracer.utils import init_tracer from ..config import MYSQL_CONFIG +from ...base import BaseTracerTestCase from ...util import assert_dict_issuperset -from ...test_tracer import get_dummy_tracer class MySQLCore(object): @@ -17,9 +17,13 @@ class MySQLCore(object): TEST_SERVICE = 'test-mysql' def setUp(self): + super(MySQLCore, self).setUp() + patch() def tearDown(self): + super(MySQLCore, self).tearDown() + # Reuse the connection across tests if self.conn: try: @@ -42,7 +46,7 @@ def test_simple_query(self): rows = cursor.fetchall() eq_(len(rows), 1) spans = writer.pop() - eq_(len(spans), 2) + eq_(len(spans), 1) span = spans[0] eq_(span.service, self.TEST_SERVICE) @@ -55,8 +59,31 @@ def test_simple_query(self): 'db.name': u'test', 'db.user': u'test', }) - fetch_span = spans[1] - eq_(fetch_span.name, 'mysql.query.fetchall') + + def test_simple_query_fetchall(self): + with self.override_config('dbapi2', dict(trace_fetch_methods=True)): + conn, tracer = self._get_conn_tracer() + writer = tracer.writer + cursor = conn.cursor() + cursor.execute("SELECT 1") + rows = cursor.fetchall() + eq_(len(rows), 1) + spans = writer.pop() + eq_(len(spans), 2) + + span = spans[0] + eq_(span.service, self.TEST_SERVICE) + eq_(span.name, 'mysql.query') + eq_(span.span_type, 'sql') + eq_(span.error, 0) + assert_dict_issuperset(span.meta, { + 'out.host': u'127.0.0.1', + 'out.port': u'3306', + 'db.name': u'test', + 'db.user': u'test', + }) + fetch_span = spans[1] + eq_(fetch_span.name, 'mysql.query.fetchall') def test_simple_query_with_positional_args(self): conn, tracer = self._get_conn_tracer_with_positional_args() @@ -66,7 +93,7 @@ def test_simple_query_with_positional_args(self): rows = cursor.fetchall() eq_(len(rows), 1) spans = writer.pop() - eq_(len(spans), 2) + eq_(len(spans), 1) span = spans[0] eq_(span.service, self.TEST_SERVICE) @@ -79,8 +106,31 @@ def test_simple_query_with_positional_args(self): 'db.name': u'test', 'db.user': u'test', }) - fetch_span = spans[1] - eq_(fetch_span.name, 'mysql.query.fetchall') + + def test_simple_query_with_positional_args_fetchall(self): + with self.override_config('dbapi2', dict(trace_fetch_methods=True)): + conn, tracer = self._get_conn_tracer_with_positional_args() + writer = tracer.writer + cursor = conn.cursor() + cursor.execute("SELECT 1") + rows = cursor.fetchall() + eq_(len(rows), 1) + spans = writer.pop() + eq_(len(spans), 2) + + span = spans[0] + eq_(span.service, self.TEST_SERVICE) + eq_(span.name, 'mysql.query') + eq_(span.span_type, 'sql') + eq_(span.error, 0) + assert_dict_issuperset(span.meta, { + 'out.host': u'127.0.0.1', + 'out.port': u'3306', + 'db.name': u'test', + 'db.user': u'test', + }) + fetch_span = spans[1] + eq_(fetch_span.name, 'mysql.query.fetchall') def test_query_with_several_rows(self): conn, tracer = self._get_conn_tracer() @@ -91,11 +141,25 @@ def test_query_with_several_rows(self): rows = cursor.fetchall() eq_(len(rows), 3) spans = writer.pop() - eq_(len(spans), 2) + eq_(len(spans), 1) span = spans[0] ok_(span.get_tag('sql.query') is None) - fetch_span = spans[1] - eq_(fetch_span.name, 'mysql.query.fetchall') + + def test_query_with_several_rows_fetchall(self): + with self.override_config('dbapi2', dict(trace_fetch_methods=True)): + conn, tracer = self._get_conn_tracer() + writer = tracer.writer + cursor = conn.cursor() + query = "SELECT n FROM (SELECT 42 n UNION SELECT 421 UNION SELECT 4210) m" + cursor.execute(query) + rows = cursor.fetchall() + eq_(len(rows), 3) + spans = writer.pop() + eq_(len(spans), 2) + span = spans[0] + ok_(span.get_tag('sql.query') is None) + fetch_span = spans[1] + eq_(fetch_span.name, 'mysql.query.fetchall') def test_query_many(self): # tests that the executemany method is correctly wrapped. @@ -126,12 +190,47 @@ def test_query_many(self): eq_(rows[1][1], "this is foo") spans = writer.pop() - eq_(len(spans), 3) + eq_(len(spans), 2) span = spans[1] ok_(span.get_tag('sql.query') is None) cursor.execute("drop table if exists dummy") - fetch_span = spans[2] - eq_(fetch_span.name, 'mysql.query.fetchall') + + def test_query_many_fetchall(self): + with self.override_config('dbapi2', dict(trace_fetch_methods=True)): + # tests that the executemany method is correctly wrapped. + conn, tracer = self._get_conn_tracer() + writer = tracer.writer + tracer.enabled = False + cursor = conn.cursor() + + cursor.execute(""" + create table if not exists dummy ( + dummy_key VARCHAR(32) PRIMARY KEY, + dummy_value TEXT NOT NULL)""") + tracer.enabled = True + + stmt = "INSERT INTO dummy (dummy_key, dummy_value) VALUES (%s, %s)" + data = [ + ('foo', 'this is foo'), + ('bar', 'this is bar'), + ] + cursor.executemany(stmt, data) + query = "SELECT dummy_key, dummy_value FROM dummy ORDER BY dummy_key" + cursor.execute(query) + rows = cursor.fetchall() + eq_(len(rows), 2) + eq_(rows[0][0], "bar") + eq_(rows[0][1], "this is bar") + eq_(rows[1][0], "foo") + eq_(rows[1][1], "this is foo") + + spans = writer.pop() + eq_(len(spans), 3) + span = spans[1] + ok_(span.get_tag('sql.query') is None) + cursor.execute("drop table if exists dummy") + fetch_span = spans[2] + eq_(fetch_span.name, 'mysql.query.fetchall') def test_query_proc(self): conn, tracer = self._get_conn_tracer() @@ -188,8 +287,8 @@ def test_simple_query_ot(self): eq_(len(rows), 1) spans = writer.pop() - eq_(len(spans), 3) - ot_span, dd_span, fetch_span = spans + eq_(len(spans), 2) + ot_span, dd_span = spans # confirm parenting eq_(ot_span.parent_id, None) @@ -209,7 +308,42 @@ def test_simple_query_ot(self): 'db.user': u'test', }) - eq_(fetch_span.name, 'mysql.query.fetchall') + + def test_simple_query_ot_fetchall(self): + """OpenTracing version of test_simple_query.""" + with self.override_config('dbapi2', dict(trace_fetch_methods=True)): + conn, tracer = self._get_conn_tracer() + writer = tracer.writer + ot_tracer = init_tracer('mysql_svc', tracer) + with ot_tracer.start_active_span('mysql_op'): + cursor = conn.cursor() + cursor.execute("SELECT 1") + rows = cursor.fetchall() + eq_(len(rows), 1) + + spans = writer.pop() + eq_(len(spans), 3) + ot_span, dd_span, fetch_span = spans + + # confirm parenting + eq_(ot_span.parent_id, None) + eq_(dd_span.parent_id, ot_span.span_id) + + eq_(ot_span.service, 'mysql_svc') + eq_(ot_span.name, 'mysql_op') + + eq_(dd_span.service, self.TEST_SERVICE) + eq_(dd_span.name, 'mysql.query') + eq_(dd_span.span_type, 'sql') + eq_(dd_span.error, 0) + assert_dict_issuperset(dd_span.meta, { + 'out.host': u'127.0.0.1', + 'out.port': u'3306', + 'db.name': u'test', + 'db.user': u'test', + }) + + eq_(fetch_span.name, 'mysql.query.fetchall') def test_commit(self): conn, tracer = self._get_conn_tracer() @@ -232,7 +366,7 @@ def test_rollback(self): eq_(span.name, 'MySQLdb.connection.rollback') -class TestMysqlPatch(MySQLCore): +class TestMysqlPatch(MySQLCore, BaseTracerTestCase): """Ensures MysqlDB is properly patched""" def _connect_with_kwargs(self): @@ -246,7 +380,6 @@ def _connect_with_kwargs(self): def _get_conn_tracer(self): if not self.conn: - tracer = get_dummy_tracer() self.conn = self._connect_with_kwargs() self.conn.ping() # Ensure that the default pin is there, with its default value @@ -255,13 +388,12 @@ def _get_conn_tracer(self): assert pin.service == 'mysql' # Customize the service # we have to apply it on the existing one since new one won't inherit `app` - pin.clone(service=self.TEST_SERVICE, tracer=tracer).onto(self.conn) + pin.clone(service=self.TEST_SERVICE, tracer=self.tracer).onto(self.conn) - return self.conn, tracer + return self.conn, self.tracer def _get_conn_tracer_with_positional_args(self): if not self.conn: - tracer = get_dummy_tracer() self.conn = MySQLdb.Connect( MYSQL_CONFIG['host'], MYSQL_CONFIG['user'], @@ -276,9 +408,9 @@ def _get_conn_tracer_with_positional_args(self): assert pin.service == 'mysql' # Customize the service # we have to apply it on the existing one since new one won't inherit `app` - pin.clone(service=self.TEST_SERVICE, tracer=tracer).onto(self.conn) + pin.clone(service=self.TEST_SERVICE, tracer=self.tracer).onto(self.conn) - return self.conn, tracer + return self.conn, self.tracer def test_patch_unpatch(self): unpatch() @@ -289,12 +421,11 @@ def test_patch_unpatch(self): patch() try: - tracer = get_dummy_tracer() - writer = tracer.writer + writer = self.tracer.writer conn = self._connect_with_kwargs() pin = Pin.get_from(conn) assert pin - pin.clone(service=self.TEST_SERVICE, tracer=tracer).onto(conn) + pin.clone(service=self.TEST_SERVICE, tracer=self.tracer).onto(conn) conn.ping() cursor = conn.cursor() @@ -302,7 +433,7 @@ def test_patch_unpatch(self): rows = cursor.fetchall() eq_(len(rows), 1) spans = writer.pop() - eq_(len(spans), 2) + eq_(len(spans), 1) span = spans[0] eq_(span.service, self.TEST_SERVICE) @@ -316,8 +447,6 @@ def test_patch_unpatch(self): 'db.user': u'test', }) ok_(span.get_tag('sql.query') is None) - fetch_span = spans[1] - eq_(fetch_span.name, 'mysql.query.fetchall') finally: unpatch() From f62a214925ca9d6a21c24690047005e2e8f9e60d Mon Sep 17 00:00:00 2001 From: brettlangdon Date: Thu, 20 Dec 2018 11:15:29 -0500 Subject: [PATCH 09/13] fix more mysql tests --- tests/contrib/mysql/test_mysql.py | 150 +++++++++++++++++++++++++----- 1 file changed, 129 insertions(+), 21 deletions(-) diff --git a/tests/contrib/mysql/test_mysql.py b/tests/contrib/mysql/test_mysql.py index 26f3c9a668c..ba49d8e47a8 100644 --- a/tests/contrib/mysql/test_mysql.py +++ b/tests/contrib/mysql/test_mysql.py @@ -9,7 +9,7 @@ # tests from tests.contrib.config import MYSQL_CONFIG from tests.opentracer.utils import init_tracer -from tests.test_tracer import get_dummy_tracer +from ...base import BaseTracerTestCase from ...util import assert_dict_issuperset @@ -19,6 +19,8 @@ class MySQLCore(object): TEST_SERVICE = 'test-mysql' def tearDown(self): + super(MySQLCore, self).tearDown() + # Reuse the connection across tests if self.conn: try: @@ -41,7 +43,7 @@ def test_simple_query(self): rows = cursor.fetchall() eq_(len(rows), 1) spans = writer.pop() - eq_(len(spans), 2) + eq_(len(spans), 1) span = spans[0] eq_(span.service, self.TEST_SERVICE) @@ -55,7 +57,30 @@ def test_simple_query(self): 'db.user': u'test', }) - eq_(spans[1].name, 'mysql.query.fetchall') + def test_simple_query_fetchll(self): + with self.override_config('dbapi2', dict(trace_fetch_methods=True)): + conn, tracer = self._get_conn_tracer() + writer = tracer.writer + cursor = conn.cursor() + cursor.execute("SELECT 1") + rows = cursor.fetchall() + eq_(len(rows), 1) + spans = writer.pop() + eq_(len(spans), 2) + + span = spans[0] + eq_(span.service, self.TEST_SERVICE) + eq_(span.name, 'mysql.query') + eq_(span.span_type, 'sql') + eq_(span.error, 0) + assert_dict_issuperset(span.meta, { + 'out.host': u'127.0.0.1', + 'out.port': u'3306', + 'db.name': u'test', + 'db.user': u'test', + }) + + eq_(spans[1].name, 'mysql.query.fetchall') def test_query_with_several_rows(self): conn, tracer = self._get_conn_tracer() @@ -66,10 +91,24 @@ def test_query_with_several_rows(self): rows = cursor.fetchall() eq_(len(rows), 3) spans = writer.pop() - eq_(len(spans), 2) + eq_(len(spans), 1) span = spans[0] ok_(span.get_tag('sql.query') is None) - eq_(spans[1].name, 'mysql.query.fetchall') + + def test_query_with_several_rows_fetchall(self): + with self.override_config('dbapi2', dict(trace_fetch_methods=True)): + conn, tracer = self._get_conn_tracer() + writer = tracer.writer + cursor = conn.cursor() + query = "SELECT n FROM (SELECT 42 n UNION SELECT 421 UNION SELECT 4210) m" + cursor.execute(query) + rows = cursor.fetchall() + eq_(len(rows), 3) + spans = writer.pop() + eq_(len(spans), 2) + span = spans[0] + ok_(span.get_tag('sql.query') is None) + eq_(spans[1].name, 'mysql.query.fetchall') def test_query_many(self): # tests that the executemany method is correctly wrapped. @@ -100,12 +139,47 @@ def test_query_many(self): eq_(rows[1][1], "this is foo") spans = writer.pop() - eq_(len(spans), 3) + eq_(len(spans), 2) span = spans[-1] ok_(span.get_tag('sql.query') is None) cursor.execute("drop table if exists dummy") - eq_(spans[2].name, 'mysql.query.fetchall') + def test_query_many_fetchall(self): + with self.override_config('dbapi2', dict(trace_fetch_methods=True)): + # tests that the executemany method is correctly wrapped. + conn, tracer = self._get_conn_tracer() + writer = tracer.writer + tracer.enabled = False + cursor = conn.cursor() + + cursor.execute(""" + create table if not exists dummy ( + dummy_key VARCHAR(32) PRIMARY KEY, + dummy_value TEXT NOT NULL)""") + tracer.enabled = True + + stmt = "INSERT INTO dummy (dummy_key, dummy_value) VALUES (%s, %s)" + data = [ + ('foo', 'this is foo'), + ('bar', 'this is bar'), + ] + cursor.executemany(stmt, data) + query = "SELECT dummy_key, dummy_value FROM dummy ORDER BY dummy_key" + cursor.execute(query) + rows = cursor.fetchall() + eq_(len(rows), 2) + eq_(rows[0][0], "bar") + eq_(rows[0][1], "this is bar") + eq_(rows[1][0], "foo") + eq_(rows[1][1], "this is foo") + + spans = writer.pop() + eq_(len(spans), 3) + span = spans[-1] + ok_(span.get_tag('sql.query') is None) + cursor.execute("drop table if exists dummy") + + eq_(spans[2].name, 'mysql.query.fetchall') def test_query_proc(self): conn, tracer = self._get_conn_tracer() @@ -161,9 +235,9 @@ def test_simple_query_ot(self): eq_(len(rows), 1) spans = writer.pop() - eq_(len(spans), 3) + eq_(len(spans), 2) - ot_span, dd_span, fetch_span = spans + ot_span, dd_span = spans # confirm parenting eq_(ot_span.parent_id, None) @@ -183,7 +257,44 @@ def test_simple_query_ot(self): 'db.user': u'test', }) - eq_(fetch_span.name, 'mysql.query.fetchall') + def test_simple_query_ot_fetchall(self): + """OpenTracing version of test_simple_query.""" + with self.override_config('dbapi2', dict(trace_fetch_methods=True)): + conn, tracer = self._get_conn_tracer() + writer = tracer.writer + + ot_tracer = init_tracer('mysql_svc', tracer) + + with ot_tracer.start_active_span('mysql_op'): + cursor = conn.cursor() + cursor.execute("SELECT 1") + rows = cursor.fetchall() + eq_(len(rows), 1) + + spans = writer.pop() + eq_(len(spans), 3) + + ot_span, dd_span, fetch_span = spans + + # confirm parenting + eq_(ot_span.parent_id, None) + eq_(dd_span.parent_id, ot_span.span_id) + + eq_(ot_span.service, 'mysql_svc') + eq_(ot_span.name, 'mysql_op') + + eq_(dd_span.service, self.TEST_SERVICE) + eq_(dd_span.name, 'mysql.query') + eq_(dd_span.span_type, 'sql') + eq_(dd_span.error, 0) + assert_dict_issuperset(dd_span.meta, { + 'out.host': u'127.0.0.1', + 'out.port': u'3306', + 'db.name': u'test', + 'db.user': u'test', + }) + + eq_(fetch_span.name, 'mysql.query.fetchall') def test_commit(self): conn, tracer = self._get_conn_tracer() @@ -206,18 +317,18 @@ def test_rollback(self): eq_(span.name, 'mysql.connection.rollback') -class TestMysqlPatch(MySQLCore): +class TestMysqlPatch(MySQLCore, BaseTracerTestCase): def setUp(self): + super(TestMysqlPatch, self).setUp() patch() def tearDown(self): + super(TestMysqlPatch, self).tearDown() unpatch() - MySQLCore.tearDown(self) def _get_conn_tracer(self): if not self.conn: - tracer = get_dummy_tracer() self.conn = mysql.connector.connect(**MYSQL_CONFIG) assert self.conn.is_connected() # Ensure that the default pin is there, with its default value @@ -227,9 +338,9 @@ def _get_conn_tracer(self): # Customize the service # we have to apply it on the existing one since new one won't inherit `app` pin.clone( - service=self.TEST_SERVICE, tracer=tracer).onto(self.conn) + service=self.TEST_SERVICE, tracer=self.tracer).onto(self.conn) - return self.conn, tracer + return self.conn, self.tracer def test_patch_unpatch(self): unpatch() @@ -240,13 +351,12 @@ def test_patch_unpatch(self): patch() try: - tracer = get_dummy_tracer() - writer = tracer.writer + writer = self.tracer.writer conn = mysql.connector.connect(**MYSQL_CONFIG) pin = Pin.get_from(conn) assert pin pin.clone( - service=self.TEST_SERVICE, tracer=tracer).onto(conn) + service=self.TEST_SERVICE, tracer=self.tracer).onto(conn) assert conn.is_connected() cursor = conn.cursor() @@ -254,7 +364,7 @@ def test_patch_unpatch(self): rows = cursor.fetchall() eq_(len(rows), 1) spans = writer.pop() - eq_(len(spans), 2) + eq_(len(spans), 1) span = spans[0] eq_(span.service, self.TEST_SERVICE) @@ -269,8 +379,6 @@ def test_patch_unpatch(self): }) ok_(span.get_tag('sql.query') is None) - eq_(spans[1].name, 'mysql.query.fetchall') - finally: unpatch() From e8de269d79fcdec9a6e9d4955bfc5b4bb7cb648e Mon Sep 17 00:00:00 2001 From: brettlangdon Date: Thu, 20 Dec 2018 11:17:42 -0500 Subject: [PATCH 10/13] fix flake8 issue --- tests/contrib/mysqldb/test_mysql.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/contrib/mysqldb/test_mysql.py b/tests/contrib/mysqldb/test_mysql.py index 051e42703b6..90a0ca12f83 100644 --- a/tests/contrib/mysqldb/test_mysql.py +++ b/tests/contrib/mysqldb/test_mysql.py @@ -308,7 +308,6 @@ def test_simple_query_ot(self): 'db.user': u'test', }) - def test_simple_query_ot_fetchall(self): """OpenTracing version of test_simple_query.""" with self.override_config('dbapi2', dict(trace_fetch_methods=True)): From f19b105a3bfa17ee8213452f0b00f65e7401088d Mon Sep 17 00:00:00 2001 From: brettlangdon Date: Thu, 20 Dec 2018 11:51:30 -0500 Subject: [PATCH 11/13] fix pymysql tests --- tests/contrib/pymysql/test_pymysql.py | 142 +++++++++++++++++++++----- 1 file changed, 116 insertions(+), 26 deletions(-) diff --git a/tests/contrib/pymysql/test_pymysql.py b/tests/contrib/pymysql/test_pymysql.py index 6fce206ce9f..1e21b1ca3ea 100644 --- a/tests/contrib/pymysql/test_pymysql.py +++ b/tests/contrib/pymysql/test_pymysql.py @@ -1,7 +1,6 @@ # 3p import pymysql -from unittest import TestCase from nose.tools import eq_ # project @@ -12,8 +11,8 @@ # testing from tests.opentracer.utils import init_tracer +from ...base import BaseTracerTestCase from ...util import assert_dict_issuperset -from ...test_tracer import get_dummy_tracer from ...contrib.config import MYSQL_CONFIG @@ -38,9 +37,11 @@ class PyMySQLCore(object): }) def setUp(self): + super(PyMySQLCore, self).setUp() patch() def tearDown(self): + super(PyMySQLCore, self).tearDown() if self.conn and not self.conn._closed: self.conn.close() unpatch() @@ -57,7 +58,7 @@ def test_simple_query(self): rows = cursor.fetchall() eq_(len(rows), 1) spans = writer.pop() - eq_(len(spans), 2) + eq_(len(spans), 1) span = spans[0] eq_(span.service, self.TEST_SERVICE) @@ -68,8 +69,28 @@ def test_simple_query(self): meta.update(self.DB_INFO) assert_dict_issuperset(span.meta, meta) - fetch_span = spans[1] - eq_(fetch_span.name, 'pymysql.query.fetchall') + def test_simple_query_fetchall(self): + with self.override_config('dbapi2', dict(trace_fetch_methods=True)): + conn, tracer = self._get_conn_tracer() + writer = tracer.writer + cursor = conn.cursor() + cursor.execute('SELECT 1') + rows = cursor.fetchall() + eq_(len(rows), 1) + spans = writer.pop() + eq_(len(spans), 2) + + span = spans[0] + eq_(span.service, self.TEST_SERVICE) + eq_(span.name, 'pymysql.query') + eq_(span.span_type, 'sql') + eq_(span.error, 0) + meta = {} + meta.update(self.DB_INFO) + assert_dict_issuperset(span.meta, meta) + + fetch_span = spans[1] + eq_(fetch_span.name, 'pymysql.query.fetchall') def test_query_with_several_rows(self): conn, tracer = self._get_conn_tracer() @@ -80,10 +101,23 @@ def test_query_with_several_rows(self): rows = cursor.fetchall() eq_(len(rows), 3) spans = writer.pop() - eq_(len(spans), 2) + eq_(len(spans), 1) + self.assertEqual(spans[0].name, 'pymysql.query') - fetch_span = spans[1] - eq_(fetch_span.name, 'pymysql.query.fetchall') + def test_query_with_several_rows_fetchall(self): + with self.override_config('dbapi2', dict(trace_fetch_methods=True)): + conn, tracer = self._get_conn_tracer() + writer = tracer.writer + cursor = conn.cursor() + query = 'SELECT n FROM (SELECT 42 n UNION SELECT 421 UNION SELECT 4210) m' + cursor.execute(query) + rows = cursor.fetchall() + eq_(len(rows), 3) + spans = writer.pop() + eq_(len(spans), 2) + + fetch_span = spans[1] + eq_(fetch_span.name, 'pymysql.query.fetchall') def test_query_many(self): # tests that the executemany method is correctly wrapped. @@ -112,11 +146,42 @@ def test_query_many(self): eq_(rows[1][1], "this is foo") spans = writer.pop() - eq_(len(spans), 3) + eq_(len(spans), 2) cursor.execute("drop table if exists dummy") - fetch_span = spans[2] - eq_(fetch_span.name, 'pymysql.query.fetchall') + def test_query_many_fetchall(self): + with self.override_config('dbapi2', dict(trace_fetch_methods=True)): + # tests that the executemany method is correctly wrapped. + conn, tracer = self._get_conn_tracer() + writer = tracer.writer + tracer.enabled = False + cursor = conn.cursor() + + cursor.execute(""" + create table if not exists dummy ( + dummy_key VARCHAR(32) PRIMARY KEY, + dummy_value TEXT NOT NULL)""") + tracer.enabled = True + + stmt = "INSERT INTO dummy (dummy_key, dummy_value) VALUES (%s, %s)" + data = [("foo", "this is foo"), + ("bar", "this is bar")] + cursor.executemany(stmt, data) + query = "SELECT dummy_key, dummy_value FROM dummy ORDER BY dummy_key" + cursor.execute(query) + rows = cursor.fetchall() + eq_(len(rows), 2) + eq_(rows[0][0], "bar") + eq_(rows[0][1], "this is bar") + eq_(rows[1][0], "foo") + eq_(rows[1][1], "this is foo") + + spans = writer.pop() + eq_(len(spans), 3) + cursor.execute("drop table if exists dummy") + + fetch_span = spans[2] + eq_(fetch_span.name, 'pymysql.query.fetchall') def test_query_proc(self): conn, tracer = self._get_conn_tracer() @@ -174,8 +239,8 @@ def test_simple_query_ot(self): eq_(len(rows), 1) spans = writer.pop() - eq_(len(spans), 3) - ot_span, dd_span, fetch_span = spans + eq_(len(spans), 2) + ot_span, dd_span = spans # confirm parenting eq_(ot_span.parent_id, None) @@ -192,7 +257,38 @@ def test_simple_query_ot(self): meta.update(self.DB_INFO) assert_dict_issuperset(dd_span.meta, meta) - eq_(fetch_span.name, 'pymysql.query.fetchall') + def test_simple_query_ot_fetchall(self): + """OpenTracing version of test_simple_query.""" + with self.override_config('dbapi2', dict(trace_fetch_methods=True)): + conn, tracer = self._get_conn_tracer() + writer = tracer.writer + ot_tracer = init_tracer('mysql_svc', tracer) + with ot_tracer.start_active_span('mysql_op'): + cursor = conn.cursor() + cursor.execute("SELECT 1") + rows = cursor.fetchall() + eq_(len(rows), 1) + + spans = writer.pop() + eq_(len(spans), 3) + ot_span, dd_span, fetch_span = spans + + # confirm parenting + eq_(ot_span.parent_id, None) + eq_(dd_span.parent_id, ot_span.span_id) + + eq_(ot_span.service, 'mysql_svc') + eq_(ot_span.name, 'mysql_op') + + eq_(dd_span.service, self.TEST_SERVICE) + eq_(dd_span.name, 'pymysql.query') + eq_(dd_span.span_type, 'sql') + eq_(dd_span.error, 0) + meta = {} + meta.update(self.DB_INFO) + assert_dict_issuperset(dd_span.meta, meta) + + eq_(fetch_span.name, 'pymysql.query.fetchall') def test_commit(self): conn, tracer = self._get_conn_tracer() @@ -215,10 +311,9 @@ def test_rollback(self): eq_(span.name, 'pymysql.connection.rollback') -class TestPyMysqlPatch(PyMySQLCore, TestCase): +class TestPyMysqlPatch(PyMySQLCore, BaseTracerTestCase): def _get_conn_tracer(self): if not self.conn: - tracer = get_dummy_tracer() self.conn = pymysql.connect(**MYSQL_CONFIG) assert not self.conn._closed # Ensure that the default pin is there, with its default value @@ -227,9 +322,9 @@ def _get_conn_tracer(self): assert pin.service == 'pymysql' # Customize the service # we have to apply it on the existing one since new one won't inherit `app` - pin.clone(service=self.TEST_SERVICE, tracer=tracer).onto(self.conn) + pin.clone(service=self.TEST_SERVICE, tracer=self.tracer).onto(self.conn) - return self.conn, tracer + return self.conn, self.tracer def test_patch_unpatch(self): unpatch() @@ -240,12 +335,11 @@ def test_patch_unpatch(self): patch() try: - tracer = get_dummy_tracer() - writer = tracer.writer + writer = self.tracer.writer conn = pymysql.connect(**MYSQL_CONFIG) pin = Pin.get_from(conn) assert pin - pin.clone(service=self.TEST_SERVICE, tracer=tracer).onto(conn) + pin.clone(service=self.TEST_SERVICE, tracer=self.tracer).onto(conn) assert not conn._closed cursor = conn.cursor() @@ -253,7 +347,7 @@ def test_patch_unpatch(self): rows = cursor.fetchall() eq_(len(rows), 1) spans = writer.pop() - eq_(len(spans), 2) + eq_(len(spans), 1) span = spans[0] eq_(span.service, self.TEST_SERVICE) @@ -264,10 +358,6 @@ def test_patch_unpatch(self): meta = {} meta.update(self.DB_INFO) assert_dict_issuperset(span.meta, meta) - - fetch_span = spans[1] - eq_(fetch_span.name, 'pymysql.query.fetchall') - finally: unpatch() From ae7bbf1b8a072edbb44e1845b7dc0c6bb264ac34 Mon Sep 17 00:00:00 2001 From: brettlangdon Date: Thu, 20 Dec 2018 13:14:16 -0500 Subject: [PATCH 12/13] fix django tests --- tests/contrib/django/test_cache_views.py | 8 ++++---- tests/contrib/django/test_connection.py | 6 ++---- tests/contrib/django/test_middleware.py | 17 ++++++----------- tests/contrib/django/utils.py | 3 ++- 4 files changed, 14 insertions(+), 20 deletions(-) diff --git a/tests/contrib/django/test_cache_views.py b/tests/contrib/django/test_cache_views.py index 611c06d3312..c3024f34874 100644 --- a/tests/contrib/django/test_cache_views.py +++ b/tests/contrib/django/test_cache_views.py @@ -18,12 +18,12 @@ def test_cached_view(self): # check the first call for a non-cached view spans = self.tracer.writer.pop() - eq_(len(spans), 7) + eq_(len(spans), 6) # the cache miss eq_(spans[1].resource, 'get') # store the result in the cache + eq_(spans[4].resource, 'set') eq_(spans[5].resource, 'set') - eq_(spans[6].resource, 'set') # check if the cache hit is traced response = self.client.get(url) @@ -69,11 +69,11 @@ def test_cached_template(self): # check the first call for a non-cached view spans = self.tracer.writer.pop() - eq_(len(spans), 6) + eq_(len(spans), 5) # the cache miss eq_(spans[2].resource, 'get') # store the result in the cache - eq_(spans[5].resource, 'set') + eq_(spans[4].resource, 'set') # check if the cache hit is traced response = self.client.get(url) diff --git a/tests/contrib/django/test_connection.py b/tests/contrib/django/test_connection.py index 30f3c095bae..86801c5b13d 100644 --- a/tests/contrib/django/test_connection.py +++ b/tests/contrib/django/test_connection.py @@ -24,7 +24,7 @@ def test_connection(self): # tests spans = self.tracer.writer.pop() assert spans, spans - eq_(len(spans), 2) + eq_(len(spans), 1) span = spans[0] eq_(span.name, 'sqlite.query') @@ -34,8 +34,6 @@ def test_connection(self): eq_(span.get_tag('django.db.alias'), 'default') assert start < span.start < span.start + span.duration < end - eq_(spans[1].name, 'sqlite.query.fetchone') - def test_django_db_query_in_resource_not_in_tags(self): User.objects.count() spans = self.tracer.writer.pop() @@ -60,7 +58,7 @@ def test_should_append_database_prefix(self): User.objects.count() traces = self.tracer.writer.pop_traces() - eq_(len(traces), 2) + eq_(len(traces), 1) eq_(len(traces[0]), 1) span = traces[0][0] eq_(span.service, 'my_prefix_db-defaultdb') diff --git a/tests/contrib/django/test_middleware.py b/tests/contrib/django/test_middleware.py index 4086db2d581..16de59ead46 100644 --- a/tests/contrib/django/test_middleware.py +++ b/tests/contrib/django/test_middleware.py @@ -27,11 +27,10 @@ def test_middleware_trace_request(self): # check for spans spans = self.tracer.writer.pop() - eq_(len(spans), 4) + eq_(len(spans), 3) sp_request = spans[0] sp_template = spans[1] sp_database = spans[2] - sp_database_fetch = spans[3] eq_(sp_database.get_tag('django.db.vendor'), 'sqlite') eq_(sp_template.get_tag('django.template_name'), 'users_list.html') eq_(sp_request.get_tag('http.status_code'), '200') @@ -40,7 +39,6 @@ def test_middleware_trace_request(self): eq_(sp_request.get_tag('http.method'), 'GET') eq_(sp_request.span_type, 'http') eq_(sp_request.resource, 'tests.contrib.django.app.views.UserList') - eq_(sp_database_fetch.name, 'sqlite.query.fetchmany') def test_database_patch(self): # We want to test that a connection-recreation event causes connections @@ -57,11 +55,10 @@ def test_database_patch(self): # We would be missing span #3, the database span, if the connection # wasn't patched. spans = self.tracer.writer.pop() - eq_(len(spans), 4) + eq_(len(spans), 3) eq_(spans[0].name, 'django.request') eq_(spans[1].name, 'django.template') eq_(spans[2].name, 'sqlite.query') - eq_(spans[3].name, 'sqlite.query.fetchmany') def test_middleware_trace_errors(self): # ensures that the internals are properly traced @@ -166,7 +163,7 @@ def test_middleware_without_user(self): # check for spans spans = self.tracer.writer.pop() - eq_(len(spans), 4) + eq_(len(spans), 3) sp_request = spans[0] eq_(sp_request.get_tag('http.status_code'), '200') eq_(sp_request.get_tag('django.user.is_authenticated'), None) @@ -185,7 +182,7 @@ def test_middleware_propagation(self): # check for spans spans = self.tracer.writer.pop() - eq_(len(spans), 4) + eq_(len(spans), 3) sp_request = spans[0] # Check for proper propagated attributes @@ -206,7 +203,7 @@ def test_middleware_no_propagation(self): # check for spans spans = self.tracer.writer.pop() - eq_(len(spans), 4) + eq_(len(spans), 3) sp_request = spans[0] # Check that propagation didn't happen @@ -278,12 +275,11 @@ def test_middleware_trace_request_ot(self): # check for spans spans = self.tracer.writer.pop() - eq_(len(spans), 5) + eq_(len(spans), 4) ot_span = spans[0] sp_request = spans[1] sp_template = spans[2] sp_database = spans[3] - sp_database_fetch = spans[4] # confirm parenting eq_(ot_span.parent_id, None) @@ -298,7 +294,6 @@ def test_middleware_trace_request_ot(self): eq_(sp_request.get_tag('http.url'), '/users/') eq_(sp_request.get_tag('django.user.is_authenticated'), 'False') eq_(sp_request.get_tag('http.method'), 'GET') - eq_(sp_database_fetch.name, 'sqlite.query.fetchmany') def test_middleware_trace_request_404(self): """ diff --git a/tests/contrib/django/utils.py b/tests/contrib/django/utils.py index 98bd6fb5269..9cd2c420f38 100644 --- a/tests/contrib/django/utils.py +++ b/tests/contrib/django/utils.py @@ -13,6 +13,7 @@ from ddtrace.contrib.django.middleware import remove_exception_middleware, remove_trace_middleware # testing +from ...base import BaseTestCase from ...test_tracer import DummyWriter @@ -21,7 +22,7 @@ tracer.writer = DummyWriter() -class DjangoTraceTestCase(TestCase): +class DjangoTraceTestCase(BaseTestCase, TestCase): """ Base class that provides an internal tracer according to given Datadog settings. This class ensures that the tracer spans are From b4110903379ce57f44f11b4e936978908f66f57b Mon Sep 17 00:00:00 2001 From: brettlangdon Date: Thu, 20 Dec 2018 13:30:22 -0500 Subject: [PATCH 13/13] Allow enabling fetch* tracing from env variable --- ddtrace/contrib/dbapi/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ddtrace/contrib/dbapi/__init__.py b/ddtrace/contrib/dbapi/__init__.py index 67e58e82bd8..ca320f94056 100644 --- a/ddtrace/contrib/dbapi/__init__.py +++ b/ddtrace/contrib/dbapi/__init__.py @@ -9,11 +9,12 @@ from ddtrace import Pin from ddtrace.ext import AppTypes, sql from ddtrace.settings import config +from ddtrace.utils.formats import asbool, get_env log = logging.getLogger(__name__) config._add('dbapi2', dict( - trace_fetch_methods=False, + trace_fetch_methods=asbool(get_env('dbapi2', 'trace_fetch_methods', 'false')), ))