From 75982185b787606073bf43ca7648a3835eb29f9c Mon Sep 17 00:00:00 2001 From: Adam Tankanow Date: Tue, 28 Aug 2018 22:51:30 -0400 Subject: [PATCH 1/8] ISSUE-80: Basic psycopg2 patcher and test --- aws_xray_sdk/core/patcher.py | 1 + aws_xray_sdk/ext/psycopg2/__init__.py | 4 ++ aws_xray_sdk/ext/psycopg2/patch.py | 25 ++++++++ tests/ext/psycopg2/__init__.py | 0 tests/ext/psycopg2/test_psycopg2.py | 87 +++++++++++++++++++++++++++ 5 files changed, 117 insertions(+) create mode 100644 aws_xray_sdk/ext/psycopg2/__init__.py create mode 100644 aws_xray_sdk/ext/psycopg2/patch.py create mode 100644 tests/ext/psycopg2/__init__.py create mode 100644 tests/ext/psycopg2/test_psycopg2.py diff --git a/aws_xray_sdk/core/patcher.py b/aws_xray_sdk/core/patcher.py index 2535f2c7..888e1a00 100644 --- a/aws_xray_sdk/core/patcher.py +++ b/aws_xray_sdk/core/patcher.py @@ -12,6 +12,7 @@ 'mysql', 'httplib', 'pymongo', + 'psycopg2' ) _PATCHED_MODULES = set() diff --git a/aws_xray_sdk/ext/psycopg2/__init__.py b/aws_xray_sdk/ext/psycopg2/__init__.py new file mode 100644 index 00000000..0e327081 --- /dev/null +++ b/aws_xray_sdk/ext/psycopg2/__init__.py @@ -0,0 +1,4 @@ +from .patch import patch + + +__all__ = ['patch'] diff --git a/aws_xray_sdk/ext/psycopg2/patch.py b/aws_xray_sdk/ext/psycopg2/patch.py new file mode 100644 index 00000000..7ca90b17 --- /dev/null +++ b/aws_xray_sdk/ext/psycopg2/patch.py @@ -0,0 +1,25 @@ +import re +import wrapt + +from aws_xray_sdk.ext.dbapi2 import XRayTracedConn + + +def patch(): + + wrapt.wrap_function_wrapper( + 'psycopg2', + 'connect', + _xray_traced_connect + ) + + +def _xray_traced_connect(wrapped, instance, args, kwargs): + + conn = wrapped(*args, **kwargs) + + meta = { + 'dbname': kwargs['dbname'] if 'dbname' in kwargs else re.match(r'dbname=(\S+)\b', args[0]).groups()[0], + 'database_type': 'postgresql' + } + + return XRayTracedConn(conn, meta) diff --git a/tests/ext/psycopg2/__init__.py b/tests/ext/psycopg2/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/ext/psycopg2/test_psycopg2.py b/tests/ext/psycopg2/test_psycopg2.py new file mode 100644 index 00000000..53664962 --- /dev/null +++ b/tests/ext/psycopg2/test_psycopg2.py @@ -0,0 +1,87 @@ +import psycopg2 + +import pytest +import testing.postgresql + +from aws_xray_sdk.core import patch +from aws_xray_sdk.core import xray_recorder +from aws_xray_sdk.core.context import Context + +patch(('psycopg2',)) + + +@pytest.fixture(autouse=True) +def construct_ctx(): + """ + Clean up context storage on each test run and begin a segment + so that later subsegment can be attached. After each test run + it cleans up context storage again. + """ + xray_recorder.configure(service='test', sampling=False, context=Context()) + xray_recorder.clear_trace_entities() + xray_recorder.begin_segment('name') + yield + xray_recorder.clear_trace_entities() + + +def test_execute_dsn_kwargs(): + q = 'SELECT 1' + with testing.postgresql.Postgresql() as postgresql: + dsn = postgresql.dsn() + conn = psycopg2.connect(dbname=dsn['database'], + user=dsn['user'], + password='', + host=dsn['host'], + port=dsn['port']) + cur = conn.cursor() + cur.execute(q) + + subsegment = xray_recorder.current_segment().subsegments[0] + assert subsegment.name == 'execute' + sql = subsegment.sql + assert sql['database_type'] == 'postgresql' + assert sql['dbname'] == dsn['database'] + + +def test_execute_dsn_string(): + q = 'SELECT 1' + with testing.postgresql.Postgresql() as postgresql: + dsn = postgresql.dsn() + conn = psycopg2.connect('dbname=' + dsn['database'] + + ' user=' + dsn['user'] + + ' password=mypassword' + + ' host=' + dsn['host'] + + ' port=' + str(dsn['port'])) + cur = conn.cursor() + cur.execute(q) + + subsegment = xray_recorder.current_segment().subsegments[0] + assert subsegment.name == 'execute' + sql = subsegment.sql + assert sql['database_type'] == 'postgresql' + assert sql['dbname'] == dsn['database'] + + +def test_execute_bad_query(): + q = 'SELECT blarg' + with testing.postgresql.Postgresql() as postgresql: + dsn = postgresql.dsn() + conn = psycopg2.connect(dbname=dsn['database'], + user=dsn['user'], + password='', + host=dsn['host'], + port=dsn['port']) + cur = conn.cursor() + try: + cur.execute(q) + except Exception: + pass + + subsegment = xray_recorder.current_segment().subsegments[0] + assert subsegment.name == 'execute' + sql = subsegment.sql + assert sql['database_type'] == 'postgresql' + assert sql['dbname'] == dsn['database'] + + exception = subsegment.cause['exceptions'][0] + assert exception.type == 'ProgrammingError' From ca904069aa29560e5f3fdd2407472f4620c18bb4 Mon Sep 17 00:00:00 2001 From: Adam Tankanow Date: Wed, 29 Aug 2018 07:21:13 -0400 Subject: [PATCH 2/8] ISSUE-80: Add pool test --- tests/ext/psycopg2/test_psycopg2.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/ext/psycopg2/test_psycopg2.py b/tests/ext/psycopg2/test_psycopg2.py index 53664962..7d3e4faf 100644 --- a/tests/ext/psycopg2/test_psycopg2.py +++ b/tests/ext/psycopg2/test_psycopg2.py @@ -1,4 +1,5 @@ import psycopg2 +import psycopg2.pool import pytest import testing.postgresql @@ -62,6 +63,26 @@ def test_execute_dsn_string(): assert sql['dbname'] == dsn['database'] +def test_execute_in_pool(): + q = 'SELECT 1' + with testing.postgresql.Postgresql() as postgresql: + dsn = postgresql.dsn() + pool = psycopg2.pool.SimpleConnectionPool(1, 1, + dbname=dsn['database'], + user=dsn['user'], + password='', + host=dsn['host'], + port=dsn['port']) + cur = pool.getconn(key=dsn['user']).cursor() + cur.execute(q) + + subsegment = xray_recorder.current_segment().subsegments[0] + assert subsegment.name == 'execute' + sql = subsegment.sql + assert sql['database_type'] == 'postgresql' + assert sql['dbname'] == dsn['database'] + + def test_execute_bad_query(): q = 'SELECT blarg' with testing.postgresql.Postgresql() as postgresql: From 96a3a62c37caff293458fef480d5f594aee35d5a Mon Sep 17 00:00:00 2001 From: Adam Tankanow Date: Wed, 29 Aug 2018 09:41:14 -0400 Subject: [PATCH 3/8] ISSUE-80: Update dbname; Add host and user --- aws_xray_sdk/ext/psycopg2/patch.py | 7 ++++--- tests/ext/psycopg2/test_psycopg2.py | 21 +++++++++++++++------ 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/aws_xray_sdk/ext/psycopg2/patch.py b/aws_xray_sdk/ext/psycopg2/patch.py index 7ca90b17..f0992757 100644 --- a/aws_xray_sdk/ext/psycopg2/patch.py +++ b/aws_xray_sdk/ext/psycopg2/patch.py @@ -16,10 +16,11 @@ def patch(): def _xray_traced_connect(wrapped, instance, args, kwargs): conn = wrapped(*args, **kwargs) - meta = { - 'dbname': kwargs['dbname'] if 'dbname' in kwargs else re.match(r'dbname=(\S+)\b', args[0]).groups()[0], - 'database_type': 'postgresql' + 'database_type': 'postgresql', + 'database_host': kwargs['host'] if 'host' in kwargs else re.search(r'host=(\S+)\b', args[0]).groups()[0], + 'database_name': kwargs['dbname'] if 'dbname' in kwargs else re.search(r'dbname=(\S+)\b', args[0]).groups()[0], + 'database_user': kwargs['user'] if 'user' in kwargs else re.search(r'user=(\S+)\b', args[0]).groups()[0], } return XRayTracedConn(conn, meta) diff --git a/tests/ext/psycopg2/test_psycopg2.py b/tests/ext/psycopg2/test_psycopg2.py index 7d3e4faf..d75b076c 100644 --- a/tests/ext/psycopg2/test_psycopg2.py +++ b/tests/ext/psycopg2/test_psycopg2.py @@ -41,7 +41,9 @@ def test_execute_dsn_kwargs(): assert subsegment.name == 'execute' sql = subsegment.sql assert sql['database_type'] == 'postgresql' - assert sql['dbname'] == dsn['database'] + assert sql['database_host'] == dsn['host'] + assert sql['database_name'] == dsn['database'] + assert sql['database_user'] == dsn['user'] def test_execute_dsn_string(): @@ -49,10 +51,10 @@ def test_execute_dsn_string(): with testing.postgresql.Postgresql() as postgresql: dsn = postgresql.dsn() conn = psycopg2.connect('dbname=' + dsn['database'] + - ' user=' + dsn['user'] + ' password=mypassword' + ' host=' + dsn['host'] + - ' port=' + str(dsn['port'])) + ' port=' + str(dsn['port']) + + ' user=' + dsn['user']) cur = conn.cursor() cur.execute(q) @@ -60,7 +62,10 @@ def test_execute_dsn_string(): assert subsegment.name == 'execute' sql = subsegment.sql assert sql['database_type'] == 'postgresql' - assert sql['dbname'] == dsn['database'] + assert sql['database_host'] == dsn['host'] + assert sql['database_name'] == dsn['database'] + assert sql['database_user'] == dsn['user'] + def test_execute_in_pool(): @@ -80,7 +85,9 @@ def test_execute_in_pool(): assert subsegment.name == 'execute' sql = subsegment.sql assert sql['database_type'] == 'postgresql' - assert sql['dbname'] == dsn['database'] + assert sql['database_host'] == dsn['host'] + assert sql['database_name'] == dsn['database'] + assert sql['database_user'] == dsn['user'] def test_execute_bad_query(): @@ -102,7 +109,9 @@ def test_execute_bad_query(): assert subsegment.name == 'execute' sql = subsegment.sql assert sql['database_type'] == 'postgresql' - assert sql['dbname'] == dsn['database'] + assert sql['database_host'] == dsn['host'] + assert sql['database_name'] == dsn['database'] + assert sql['database_user'] == dsn['user'] exception = subsegment.cause['exceptions'][0] assert exception.type == 'ProgrammingError' From d3fc58e3bf10becceea814a15d3ed73f88084f15 Mon Sep 17 00:00:00 2001 From: Adam Tankanow Date: Wed, 29 Aug 2018 09:41:29 -0400 Subject: [PATCH 4/8] ISSUE-80: Add testing resources to tox.ini --- tox.ini | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tox.ini b/tox.ini index adffd627..ae392dae 100644 --- a/tox.ini +++ b/tox.ini @@ -20,6 +20,8 @@ deps = # the sdk doesn't support earlier version of django django >= 1.10, <2.0 pynamodb + psycopg2 + testing.postgresql # Python3.5+ only deps py{35,36}: aiohttp >= 3.0.0 py{35,36}: pytest-aiohttp From c1952fd842c0618c7e10f3d10f28c5d198e49aec Mon Sep 17 00:00:00 2001 From: Adam Tankanow Date: Wed, 29 Aug 2018 21:21:54 -0400 Subject: [PATCH 5/8] ISSUE-80: Update meta keys to match supported keys --- aws_xray_sdk/ext/psycopg2/patch.py | 13 ++++++---- tests/ext/psycopg2/test_psycopg2.py | 37 ++++++++++++++++------------- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/aws_xray_sdk/ext/psycopg2/patch.py b/aws_xray_sdk/ext/psycopg2/patch.py index f0992757..579d9917 100644 --- a/aws_xray_sdk/ext/psycopg2/patch.py +++ b/aws_xray_sdk/ext/psycopg2/patch.py @@ -16,11 +16,16 @@ def patch(): def _xray_traced_connect(wrapped, instance, args, kwargs): conn = wrapped(*args, **kwargs) + host = kwargs['host'] if 'host' in kwargs else re.search(r'host=(\S+)\b', args[0]).groups()[0] + dbname = kwargs['dbname'] if 'dbname' in kwargs else re.search(r'dbname=(\S+)\b', args[0]).groups()[0] + port = kwargs['port'] if 'port' in kwargs else re.search(r'port=(\S+)\b', args[0]).groups()[0] + user = kwargs['user'] if 'user' in kwargs else re.search(r'user=(\S+)\b', args[0]).groups()[0] meta = { - 'database_type': 'postgresql', - 'database_host': kwargs['host'] if 'host' in kwargs else re.search(r'host=(\S+)\b', args[0]).groups()[0], - 'database_name': kwargs['dbname'] if 'dbname' in kwargs else re.search(r'dbname=(\S+)\b', args[0]).groups()[0], - 'database_user': kwargs['user'] if 'user' in kwargs else re.search(r'user=(\S+)\b', args[0]).groups()[0], + 'database_type': 'PostgreSQL', + 'url': 'postgresql://{}@{}:{}/{}'.format(user, host, port, dbname), + 'user': user, + 'database_version': conn.server_version, + 'driver_version': 'Psycopg 2' } return XRayTracedConn(conn, meta) diff --git a/tests/ext/psycopg2/test_psycopg2.py b/tests/ext/psycopg2/test_psycopg2.py index d75b076c..31823b66 100644 --- a/tests/ext/psycopg2/test_psycopg2.py +++ b/tests/ext/psycopg2/test_psycopg2.py @@ -28,6 +28,7 @@ def construct_ctx(): def test_execute_dsn_kwargs(): q = 'SELECT 1' with testing.postgresql.Postgresql() as postgresql: + url = postgresql.url() dsn = postgresql.dsn() conn = psycopg2.connect(dbname=dsn['database'], user=dsn['user'], @@ -40,15 +41,16 @@ def test_execute_dsn_kwargs(): subsegment = xray_recorder.current_segment().subsegments[0] assert subsegment.name == 'execute' sql = subsegment.sql - assert sql['database_type'] == 'postgresql' - assert sql['database_host'] == dsn['host'] - assert sql['database_name'] == dsn['database'] - assert sql['database_user'] == dsn['user'] + assert sql['database_type'] == 'PostgreSQL' + assert sql['user'] == dsn['user'] + assert sql['url'] == url + assert sql['database_version'] == 100002 def test_execute_dsn_string(): q = 'SELECT 1' with testing.postgresql.Postgresql() as postgresql: + url = postgresql.url() dsn = postgresql.dsn() conn = psycopg2.connect('dbname=' + dsn['database'] + ' password=mypassword' + @@ -61,16 +63,16 @@ def test_execute_dsn_string(): subsegment = xray_recorder.current_segment().subsegments[0] assert subsegment.name == 'execute' sql = subsegment.sql - assert sql['database_type'] == 'postgresql' - assert sql['database_host'] == dsn['host'] - assert sql['database_name'] == dsn['database'] - assert sql['database_user'] == dsn['user'] - + assert sql['database_type'] == 'PostgreSQL' + assert sql['user'] == dsn['user'] + assert sql['url'] == url + assert sql['database_version'] == 100002 def test_execute_in_pool(): q = 'SELECT 1' with testing.postgresql.Postgresql() as postgresql: + url = postgresql.url() dsn = postgresql.dsn() pool = psycopg2.pool.SimpleConnectionPool(1, 1, dbname=dsn['database'], @@ -84,15 +86,16 @@ def test_execute_in_pool(): subsegment = xray_recorder.current_segment().subsegments[0] assert subsegment.name == 'execute' sql = subsegment.sql - assert sql['database_type'] == 'postgresql' - assert sql['database_host'] == dsn['host'] - assert sql['database_name'] == dsn['database'] - assert sql['database_user'] == dsn['user'] + assert sql['database_type'] == 'PostgreSQL' + assert sql['user'] == dsn['user'] + assert sql['url'] == url + assert sql['database_version'] == 100002 def test_execute_bad_query(): q = 'SELECT blarg' with testing.postgresql.Postgresql() as postgresql: + url = postgresql.url() dsn = postgresql.dsn() conn = psycopg2.connect(dbname=dsn['database'], user=dsn['user'], @@ -108,10 +111,10 @@ def test_execute_bad_query(): subsegment = xray_recorder.current_segment().subsegments[0] assert subsegment.name == 'execute' sql = subsegment.sql - assert sql['database_type'] == 'postgresql' - assert sql['database_host'] == dsn['host'] - assert sql['database_name'] == dsn['database'] - assert sql['database_user'] == dsn['user'] + assert sql['database_type'] == 'PostgreSQL' + assert sql['user'] == dsn['user'] + assert sql['url'] == url + assert sql['database_version'] == 100002 exception = subsegment.cause['exceptions'][0] assert exception.type == 'ProgrammingError' From 609c0f0d92f208ec702f183402d5097f5f5abe18 Mon Sep 17 00:00:00 2001 From: Adam Tankanow Date: Wed, 29 Aug 2018 21:38:38 -0400 Subject: [PATCH 6/8] ISSUE-80: Fix failing tests; simply assert database_version rather than compare versions --- aws_xray_sdk/ext/psycopg2/patch.py | 2 +- tests/ext/psycopg2/test_psycopg2.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/aws_xray_sdk/ext/psycopg2/patch.py b/aws_xray_sdk/ext/psycopg2/patch.py index 579d9917..f6f1876d 100644 --- a/aws_xray_sdk/ext/psycopg2/patch.py +++ b/aws_xray_sdk/ext/psycopg2/patch.py @@ -24,7 +24,7 @@ def _xray_traced_connect(wrapped, instance, args, kwargs): 'database_type': 'PostgreSQL', 'url': 'postgresql://{}@{}:{}/{}'.format(user, host, port, dbname), 'user': user, - 'database_version': conn.server_version, + 'database_version': str(conn.server_version), 'driver_version': 'Psycopg 2' } diff --git a/tests/ext/psycopg2/test_psycopg2.py b/tests/ext/psycopg2/test_psycopg2.py index 31823b66..dd394990 100644 --- a/tests/ext/psycopg2/test_psycopg2.py +++ b/tests/ext/psycopg2/test_psycopg2.py @@ -44,7 +44,7 @@ def test_execute_dsn_kwargs(): assert sql['database_type'] == 'PostgreSQL' assert sql['user'] == dsn['user'] assert sql['url'] == url - assert sql['database_version'] == 100002 + assert sql['database_version'] def test_execute_dsn_string(): @@ -66,7 +66,7 @@ def test_execute_dsn_string(): assert sql['database_type'] == 'PostgreSQL' assert sql['user'] == dsn['user'] assert sql['url'] == url - assert sql['database_version'] == 100002 + assert sql['database_version'] def test_execute_in_pool(): @@ -89,7 +89,7 @@ def test_execute_in_pool(): assert sql['database_type'] == 'PostgreSQL' assert sql['user'] == dsn['user'] assert sql['url'] == url - assert sql['database_version'] == 100002 + assert sql['database_version'] def test_execute_bad_query(): @@ -114,7 +114,7 @@ def test_execute_bad_query(): assert sql['database_type'] == 'PostgreSQL' assert sql['user'] == dsn['user'] assert sql['url'] == url - assert sql['database_version'] == 100002 + assert sql['database_version'] exception = subsegment.cause['exceptions'][0] assert exception.type == 'ProgrammingError' From 7bbdc797c8752da5967902a2d9798063c2339a79 Mon Sep 17 00:00:00 2001 From: Adam Tankanow Date: Thu, 30 Aug 2018 16:32:21 -0400 Subject: [PATCH 7/8] ISSUE-80: Add psycopg2 to NO_DOUBLE_PATCH --- aws_xray_sdk/core/patcher.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/aws_xray_sdk/core/patcher.py b/aws_xray_sdk/core/patcher.py index 80f8a149..d9169b21 100644 --- a/aws_xray_sdk/core/patcher.py +++ b/aws_xray_sdk/core/patcher.py @@ -10,7 +10,7 @@ 'mysql', 'httplib', 'pymongo', - 'psycopg2' + 'psycopg2', ) NO_DOUBLE_PATCH = ( @@ -19,6 +19,7 @@ 'sqlite3', 'mysql', 'pymongo', + 'psycopg2', ) _PATCHED_MODULES = set() @@ -34,6 +35,7 @@ def patch_all(double_patch=False): def patch(modules_to_patch, raise_errors=True): modules = set() for module_to_patch in modules_to_patch: + print('adding {}'.format(module_to_patch)) # boto3 depends on botocore and patching botocore is sufficient if module_to_patch == 'boto3': modules.add('botocore') @@ -65,7 +67,7 @@ def _patch_module(module_to_patch, raise_errors=True): def _patch(module_to_patch): - + print('patching {}'.format(module_to_patch)) path = 'aws_xray_sdk.ext.%s' % module_to_patch if module_to_patch in _PATCHED_MODULES: From 971139b4b4972a96c09cf3862a82b986f6a25498 Mon Sep 17 00:00:00 2001 From: Adam Tankanow Date: Thu, 30 Aug 2018 16:34:09 -0400 Subject: [PATCH 8/8] ISSUE-80: Remove debug print statements --- aws_xray_sdk/core/patcher.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/aws_xray_sdk/core/patcher.py b/aws_xray_sdk/core/patcher.py index d9169b21..d51fad93 100644 --- a/aws_xray_sdk/core/patcher.py +++ b/aws_xray_sdk/core/patcher.py @@ -35,7 +35,6 @@ def patch_all(double_patch=False): def patch(modules_to_patch, raise_errors=True): modules = set() for module_to_patch in modules_to_patch: - print('adding {}'.format(module_to_patch)) # boto3 depends on botocore and patching botocore is sufficient if module_to_patch == 'boto3': modules.add('botocore') @@ -67,7 +66,7 @@ def _patch_module(module_to_patch, raise_errors=True): def _patch(module_to_patch): - print('patching {}'.format(module_to_patch)) + path = 'aws_xray_sdk.ext.%s' % module_to_patch if module_to_patch in _PATCHED_MODULES: