dd-trace-py icon indicating copy to clipboard operation
dd-trace-py copied to clipboard

aiopg monkey patching breaking with aiopg 0.15, psycopg2 2.7.5 and psycopg2-binary 2.7.5

Open carlosperello opened this issue 5 years ago • 5 comments

Which version of dd-trace-py are you using?

0.33.0

Which version of the libraries are you using?

aiopg==0.15.0 psycopg2-binary==2.7.5 psycopg2==2.7.5

How can we reproduce your problem?

Our python script had this on the top of the file:

from ddtrace import tracer, patch
from ddtrace.contrib.aiohttp import trace_app

patch(aiohttp=True, aiopg=True, logging=True, psycopg=True, requests=True, sqlalchemy=True)
...

async def setup_db_connection(app):
    conf = app['config']['postgres']
    db_engine = await aiopg.sa.create_engine(
        user=conf['user'],
        database=conf['db'],
        host=conf['host'],
        password=conf['password'],
        port=conf['port'],
        loop=app['loop'],
        maxsize=1000,
    )

...


async def init(loop):
    # setup application and extensions
    middlewares = [
        request_logger_middleware,
        error_middleware,
    ]
    app = web.Application(loop=loop, middlewares=middlewares)

    trace_app(app, tracer, service='hermes')
    app['datadog_trace']['analytics_enabled'] = True
...

    await setup_db_connection(app)
...

def main(_):
    loop = asyncio.get_event_loop()

    loop.set_exception_handler(custom_exception_handler)
    app = loop.run_until_complete(init(loop))
...

What is the result that you get?

The app start breaks with:

Traceback (most recent call last):
File "/usr/local/lib/python3.7/runpy.py", line 193, in _run_module_as_main
"__main__", mod_spec)
File "/usr/local/lib/python3.7/runpy.py", line 85, in _run_code
exec(code, run_globals)
File "/app/hermes/main.py", line 181, in <module>
main(sys.argv[1:])
File "/app/hermes/main.py", line 169, in main
app = loop.run_until_complete(init(loop))
File "/usr/local/lib/python3.7/asyncio/base_events.py", line 579, in run_until_complete
return future.result()
File "/app/hermes/main.py", line 129, in init
await setup_db_connection(app)
File "/app/hermes/main.py", line 52, in setup_db_connection
maxsize=1000, # TODO - figure out what is a reasonable number for this that won't eat up all of Hermes' memory
File "/usr/local/lib/python3.7/site-packages/aiopg/utils.py", line 71, in __await__
resp = yield from self._coro
File "/usr/local/lib/python3.7/site-packages/aiopg/sa/engine.py", line 76, in _create_engine
pool_recycle=pool_recycle, **kwargs)
File "/usr/local/lib/python3.7/site-packages/aiopg/utils.py", line 66, in __iter__
resp = yield from self._coro
File "/usr/local/lib/python3.7/site-packages/aiopg/pool.py", line 47, in _create_pool
yield from pool._fill_free_pool(False)
File "/usr/local/lib/python3.7/site-packages/aiopg/pool.py", line 208, in _fill_free_pool
**self._conn_kwargs)
File "/usr/local/lib/python3.7/site-packages/aiopg/utils.py", line 66, in __iter__
resp = yield from self._coro
File "/usr/local/lib/python3.7/site-packages/ddtrace/contrib/aiopg/patch.py", line 35, in patched_connect
conn = yield from connect_func(*args, **kwargs)
File "/usr/local/lib/python3.7/site-packages/aiopg/connection.py", line 81, in _connect
extras.register_uuid(conn_or_curs=conn._conn)
File "/usr/local/lib/python3.7/site-packages/psycopg2/extras.py", line 668, in register_uuid
_ext.register_type(_ext.UUID, conn_or_curs)
File "/usr/local/lib/python3.7/site-packages/ddtrace/contrib/aiopg/patch.py", line 47, in _extensions_register_type
scope = scope.__wrapped__._conn
AttributeError: 'psycopg2.extensions.connection' object has no attribute '_conn'

What is result that you expected?

App starting up and aiopg traces being sent to datadog.

carlosperello avatar Feb 20 '20 16:02 carlosperello

@carlosperello I'm trying to reproduce this issue locally and beginning with a minimal case to isolate the problem.

I see that aiopg==0.15.0 requires psycopg2>=2.7.0 whereas you have additionally installed psycopg2-binary. I ran our aiopg tests with both installed without problem, though I am not sure if that reproduces the ordering on your end. Can you try to run your tests with just psycopg2?

Also, your snippet disabled patching for aiopg but the error you provided suggests that this patching had occurred. Can you confirm that you are getting the error when the module is patched and that is the intended usage?

Here's my minimal reproduction though the patching does not produce an error:

https://gist.github.com/majorgreys/1fd51f9f10bbfa308b3474ae62c9a72f

To run, simply execute DD_API_KEY=... docker-compose up --build

majorgreys avatar Feb 20 '20 22:02 majorgreys

@majorgreys Thanks for checking.

I updated your gist at https://gist.github.com/carlosperello/01179b9b081d61d4df203644d7dd8f32 which reproduces exactly my issue. The missing bit on your example is that you were not importing or using aiohttp module which seems to be that causes the issue when used together with aiopg.

About your comment on my patching code not enabling aiopg is true, I copied it after testing the service with aiopg disabled and didn't notice when filing the ticket. I updated the description of the ticket to avoid confusion.

carlosperello avatar Feb 24 '20 11:02 carlosperello

@carlosperello I think the issue might be specifically the ordering how how aiopg and psycopg2 are patched. If I place psycopg=True before aiopg=True in patch(), I don't get an error, whereas if psycopg=True comes after then an error is produced. This is not a problem when you use ddtrace-run since it is calling patch_all where loads the psycopg integration before the aiopg integration.

Please tell me if changing the order addresses your problem.

I will include this in our backlog task for updating our aiopg integration.

majorgreys avatar Feb 26 '20 15:02 majorgreys

@carlosperello Did reordering the patch sequence address the problem you were having?

majorgreys avatar Mar 10 '20 13:03 majorgreys

@majorgreys sorry for the delay answering, had to disable the APM support on that project to debug another internal issue, for some reason the aiohttp instrumentation caused timeout errors in our admin pages. The aiopg issue seems to be fixed now, so this issue can be closed (although having it working no matter the order of the arguments in the patch command would be awesome).

For the aiohttp issue, I would need to debug it further before I can file a new ticket for it.

Thanks for your help!

carlosperello avatar Mar 12 '20 17:03 carlosperello