Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Instrumentation for PyMSSQL #239

Closed
eliasbrange opened this issue Jun 25, 2018 · 10 comments
Closed

Instrumentation for PyMSSQL #239

eliasbrange opened this issue Jun 25, 2018 · 10 comments

Comments

@eliasbrange
Copy link

eliasbrange commented Jun 25, 2018

We use SQLAlchemy heavily and would like to instrument it. Might be able to find time to make an implementation myself if no one beats me to it.

https://pypi.org/project/SQLAlchemy/
http://www.sqlalchemy.org/

@beniwohli
Copy link
Contributor

Hi @eliasbrange

We already instrument SQL database calls on the DB-API2 level (basically, cursor.execute and friends), so DB calls from SQLAlchemy should be captured. Currently, this is implemented for PostgreSQL, MySQL, SQLite and soon PyODBC.

Do you think there would be a benefit in instrumenting SQLAlchemy directly over the current DB-API2-level instrumentation?

@eliasbrange
Copy link
Author

Hmm. We are using http://pymssql.org/en/stable/ which might explain why I don't receive any captured spans then.

I honestly can't answer that. Just started playing around with APM a couple of days ago.

@beniwohli
Copy link
Contributor

Yes, that would explain it :)

Adding support for additional database drivers is (usually) quite simple, see e.g. #238 for the PyODBC instrumentation that we are currently working on. The most difficult part is getting tests to work, as we like to spin up a database to test that we don't break the actual connection between the driver and the database.

I'll have a look how difficult it would be to get pymssql instrumented and tested :)

@eliasbrange
Copy link
Author

Great! I'll try aswell. Got some of my DB calls in by instrumenting ("sqlalchemy.pool", "Pool.connect"), but somehow misses some of my calls. Will have to check and see if there's another method inside there anywhere that gets called instead.

@eliasbrange
Copy link
Author

eliasbrange commented Jun 25, 2018

This seems to work somewhat. Might be a good starting point.

from elasticapm.instrumentation.packages.dbapi2 import (ConnectionProxy,
                                                        CursorProxy,
                                                        DbApi2Instrumentation,
                                                        extract_signature)


class SQLAlchemyCursorProxy(CursorProxy):
    provider_name = 'SQLAlchemy'

    def extract_signature(self, sql):
        return extract_signature(sql)


class SQLAlchemyConnectionProxy(ConnectionProxy):
    cursor_proxy = SQLAlchemyCursorProxy


class SQLAlchemyInstrumentation(DbApi2Instrumentation):
    name = 'SQLAlchemy'

    instrument_list = [
        ("sqlalchemy.pool", "Pool.connect"),
        ("sqlalchemy.pool", "Pool.unique_connection"),
    ]

    def call(self, module, method, wrapped, instance, args, kwargs):
        return SQLAlchemyConnectionProxy(wrapped(*args, **kwargs))

@beniwohli
Copy link
Contributor

@eliasbrange nice! However I'd prefer going forward with the current strategy of directly instrumenting the driver, instead of SQLAlchemy. That ensures that the instrumentation even works when using the driver directly, or a different ORM (e.g. Django's ORM).

e.g. something like this (watch out, completely untested as of now :D)

from elasticapm.instrumentation.packages.dbapi2 import (ConnectionProxy,
                                                        CursorProxy,
                                                        DbApi2Instrumentation,
                                                        extract_signature)


class PyMSSQLCursorProxy(CursorProxy):
    provider_name = 'pymssql'

    def extract_signature(self, sql):
        return extract_signature(sql)


class PyMSSQLConnectionProxy(ConnectionProxy):
    cursor_proxy = PyMSSQLCursorProxy


class PyMSSQLInstrumentation(DbApi2Instrumentation):
    name = 'pymssql'

    instrument_list = [
        ("pymssql", "connect"),
    ]

    def call(self, module, method, wrapped, instance, args, kwargs):
        return PyMSSQLConnectionProxy(wrapped(*args, **kwargs))

@eliasbrange
Copy link
Author

That sure seems a lot more clever @beniwohli ...

@eliasbrange
Copy link
Author

@beniwohli. How do I instrument the above instrumentation without adding it elasticapm/instrumentation/register.py after I have initialized APM with apm = ElasticAPM(app, ...)?

@beniwohli
Copy link
Contributor

@eliasbrange we currently have no public API to register custom instrumentations (it's high on my todo list, though). However, I will open a PR with the above code as soon as I've figured out how to integrate it with our Test/CI setup, hopefully we'll have a new release in a few days.

In the meantime, for testing and evaluation purposes, you technically could run PyMSSQLInstrumentation().instrument() somewhere in your code base before the first database connection is created. However, I wouldn't dare putting something like that anywhere near a production system :)

@beniwohli beniwohli changed the title Instrumentation for SQLAlchemy Instrumentation for PyMSSQL Jun 25, 2018
beniwohli added a commit to beniwohli/apm-agent-python that referenced this issue Jun 25, 2018
beniwohli added a commit to beniwohli/apm-agent-python that referenced this issue Jun 25, 2018
beniwohli added a commit to beniwohli/apm-agent-python that referenced this issue Jun 25, 2018
beniwohli added a commit to beniwohli/apm-agent-python that referenced this issue Jun 25, 2018
@soumithx
Copy link

is SQLAlchemy DB Calls logging available for apm?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants