Skip to content

Add Tracing for SQLAlchemy and Flask-SQLAlcemy #14

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

Merged
merged 21 commits into from
Feb 20, 2018
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
6d6c99d
Initial checkin of Query and BaseQuery overrides
greengiant Dec 6, 2017
8c613d0
Fix ext name
greengiant Dec 6, 2017
1c75aa6
Fix import
greengiant Dec 6, 2017
0729f16
Add support for SQLAlchemy.orm and Flask-SQLAlchemy
greengiant Dec 8, 2017
31070bd
Remove print() statement
greengiant Dec 8, 2017
8f8468f
Attempt to fix handling of Flask not having a request with a xray seg…
greengiant Dec 9, 2017
b9a00b8
Fix handling of missing segment
greengiant Dec 9, 2017
d576942
Fix test and add docstrings
greengiant Dec 9, 2017
ea672bb
Fix bug with End segment
greengiant Dec 10, 2017
c332d3a
Code Review Cleanup. Files now all pass flake8 tests
greengiant Dec 12, 2017
941097a
Move find_subsegment and _search_entity functions to tests/util.py
greengiant Dec 13, 2017
730bef0
Uset set_sql to corectly test the sanitized_query value. Add test to …
greengiant Dec 13, 2017
21195c1
Merge remote-tracking branch 'upstream/master' into FlaskSQLAlchemy
greengiant Dec 14, 2017
4c26818
Comment out set_sql for sanitized_query for seperate code review
greengiant Dec 14, 2017
c3646f1
Starting to add in set_sql
greengiant Dec 16, 2017
a9f1b0c
Add more SQL info to trace
greengiant Dec 16, 2017
d494d15
Correct URL handling for connection strings
greengiant Dec 16, 2017
e3f171c
Merge remote-tracking branch 'upstream/master' into set_sql
greengiant Feb 16, 2018
84c5ae0
Bug fix and remove sanitized_query
greengiant Feb 16, 2018
bf8d99a
Fix unit test and add helper util for finding subsegment by annotatio…
greengiant Feb 16, 2018
087dd1f
Minor cleanups
greengiant Feb 20, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,61 @@ app.router.add_get("/", handler)

web.run_app(app)
```
**Add Flask middleware**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a duplicate?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Removing.


```python
from aws_xray_sdk.core import xray_recorder
from aws_xray_sdk.ext.flask.middleware import XRayMiddleware

app = Flask(__name__)

xray_recorder.configure(service='fallback_name', dynamic_naming='*mysite.com*')
XRayMiddleware(app, xray_recorder)
```


**Use SQLAlchemy ORM**
The SQLAlchemy integration requires you to override the Session and Query Classes for SQL Alchemy

SQLAlchemy integration uses subsegments so you need to have a segment started before you make a query.

```python
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy import *
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason you use wild import?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, in fact that entire import can go. I refactored the way i was doing imports and that is not needed.

from sqlalchemy.orm import sessionmaker
from aws_xray_sdk.core import xray_recorder
from aws_xray_sdk.core.context import Context
from aws_xray_sdk.ext.sqlalchemy.query import XRaySessionMaker

xray_recorder.configure(service='test', sampling=False, context=Context())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For quick start guide for a SQL library you don't need to configure anything on the recorder. And you could also remove the import on context as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

xray_recorder.clear_trace_entities()
xray_recorder.begin_segment('SQLAlchemyTest')

Session = XRaySessionMaker(bind=engine)
session = Session()

xray_recorder.end_segment()
app = Flask(__name__)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for a general SQLAlchemy guide there is no need to involve the web framework.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have to initialize the app from Flask in order to Initialize Flask-SQLAlchemy

db = XRayFlaskSqlAlchemy(app)

was just trying to show complete example..

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable. I'm fine with this.


xray_recorder.configure(service='fallback_name', dynamic_naming='*mysite.com*')
XRayMiddleware(app, xray_recorder)
```

**Add Flask-SQLAlchemy**

```python
from aws_xray_sdk.core import xray_recorder
from aws_xray_sdk.ext.flask.middleware import XRayMiddleware
from aws_xray_sdk.ext.flask_sqlalchemy.query import XRayFlaskSqlAlchemy

app = Flask(__name__)
app.config["SQLALCHEMY_DATABASE_URI"] = "sqlite:///:memory:"

xray_recorder.configure(service='fallback_name', dynamic_naming='*mysite.com*')
XRayMiddleware(app, xray_recorder)
db = XRayFlaskSqlAlchemy(app)

```
## License

The AWS X-Ray SDK for Python is licensed under the Apache 2.0 License. See LICENSE and NOTICE.txt for more information.
Empty file.
95 changes: 95 additions & 0 deletions aws_xray_sdk/ext/flask_sqlalchemy/query.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
from __future__ import print_function
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused import? Print statement should not be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Was used for debugging.

from builtins import super
import aws_xray_sdk
from aws_xray_sdk.core import xray_recorder
from aws_xray_sdk.core.context import Context
from sqlalchemy.orm.base import _generative
from sqlalchemy.orm.query import Query
from flask_sqlalchemy.model import Model
from sqlalchemy.orm.session import Session, sessionmaker
from functools import wraps
from flask_sqlalchemy import SQLAlchemy, BaseQuery, _SessionSignalEvents, get_state
from aws_xray_sdk.ext.sqlalchemy.query import XRaySession, XRayQuery

def decorate_all_functions(function_decorator):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this file be in the ext/flask folder? aws_xray_sdk/ext/flask/sqlalchemy.py for example

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flask-SQLAlchemy is a separate python package then Flask, which is why i kept them separated. Willing to change if needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can leave it for now.

def decorator(cls):
for c in cls.__bases__:
for name, obj in vars(c).items():
if name.startswith("_"):
continue
if callable(obj):
try:
obj = obj.__func__ # unwrap Python 2 unbound method
except AttributeError:
pass # not needed in Python 3
setattr(c, name, function_decorator(c, obj))
return cls
return decorator

def xray_on_call(cls, func):
def wrapper(*args, **kw):
class_name = str(cls.__module__)
c = xray_recorder._context
if getattr(c._local, 'entities', None) is not None:
trace = xray_recorder.begin_subsegment(class_name+'.'+func.__name__)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trace in definition means a collection of segments and subsegments that share one ID. It'd be better to rename this variable to subsegment

else:
trace = None
res = func(*args, **kw)
if trace is not None:
if class_name == 'sqlalchemy.orm.query':
for arg in args:
if isinstance(arg, aws_xray_sdk.ext.sqlalchemy.query.XRayQuery):
trace.put_metadata("sql", str(arg));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can check https://github.com/aws/aws-xray-sdk-python/blob/master/aws_xray_sdk/core/models/subsegment.py#L35 'sql' is a special metadata that is actually in parallel with 'metadata' field. Getting the hierarchy correct will make sure X-Ray console and back-end can process the information properly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So just change this call to set_sql(). Is there any special keys for this sql metadata, or can I just the key sql as I have have here?

xray_recorder.end_subsegment()
return res
return wrapper

@decorate_all_functions(xray_on_call)
class XRayBaseQuery(BaseQuery):
BaseQuery.__bases__ = (XRayQuery,)

class XRaySignallingSession(XRaySession):
"""The signalling session is the default session that Flask-SQLAlchemy
uses. It extends the default session system with bind selection and
modification tracking.
If you want to use a different session you can override the
:meth:`SQLAlchemy.create_session` function.
.. versionadded:: 2.0
.. versionadded:: 2.1
The `binds` option was added, which allows a session to be joined
to an external transaction.
"""

def __init__(self, db, autocommit=False, autoflush=True, **options):
#: The application that this session belongs to.
self.app = app = db.get_app()
track_modifications = app.config['SQLALCHEMY_TRACK_MODIFICATIONS']
bind = options.pop('bind', None) or db.engine
binds = options.pop('binds', db.get_binds(app))

if track_modifications is None or track_modifications:
_SessionSignalEvents.register(self)

XRaySession.__init__(
self, autocommit=autocommit, autoflush=autoflush,
bind=bind, binds=binds, **options
)

def get_bind(self, mapper=None, clause=None):
# mapper is None if someone tries to just get a connection
if mapper is not None:
info = getattr(mapper.mapped_table, 'info', {})
bind_key = info.get('bind_key')
if bind_key is not None:
state = get_state(self.app)
return state.db.get_engine(self.app, bind=bind_key)
return XRaySession.get_bind(self, mapper, clause)

class XRayFlaskSqlAlchemy(SQLAlchemy):
def __init__(self, app=None, use_native_unicode=True, session_options=None,
metadata=None, query_class=XRayBaseQuery, model_class=Model):
super().__init__(app, use_native_unicode, session_options,
metadata, query_class, model_class)

def create_session(self, options):
return sessionmaker(class_=XRaySignallingSession, db=self, **options)
Empty file.
61 changes: 61 additions & 0 deletions aws_xray_sdk/ext/sqlalchemy/query.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
from __future__ import print_function
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

from builtins import super
import aws_xray_sdk
from aws_xray_sdk.core import xray_recorder
from aws_xray_sdk.core.context import Context
from sqlalchemy.orm.base import _generative
from sqlalchemy.orm.query import Query
from sqlalchemy.orm.session import Session, sessionmaker
from functools import wraps


def decorate_all_functions(function_decorator):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you can create a sqlalchemy util module that holds these shared functions so that you don't duplicate code.
decorate_all_functions and xray_on_call can be shared in flask_sqlalchemy

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes good point. I meant to go back and do this, and completely forgot.

def decorator(cls):
for c in cls.__bases__:
for name, obj in vars(c).items():
if name.startswith("_"):
continue
if callable(obj):
try:
obj = obj.__func__ # unwrap Python 2 unbound method
except AttributeError:
pass # not needed in Python 3
setattr(c, name, function_decorator(c, obj))
return cls
return decorator

def xray_on_call(cls, func):
def wrapper(*args, **kw):
class_name = str(cls.__module__)
c = xray_recorder._context
if getattr(c._local, 'entities', None) is not None:
trace = xray_recorder.begin_subsegment(class_name+'.'+func.__name__)
else:
trace = None
res = func(*args, **kw)
if trace is not None:
if class_name == 'sqlalchemy.orm.query':
for arg in args:
if isinstance(arg, aws_xray_sdk.ext.sqlalchemy.query.XRayQuery):
trace.put_metadata("sql", str(arg));
xray_recorder.end_subsegment()
return res
return wrapper

@decorate_all_functions(xray_on_call)
class XRaySession(Session):
pass

@decorate_all_functions(xray_on_call)
class XRayQuery(Query):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason you decorate functions from Query class?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Query Class called by flask-sqlalchemyt, so this gets us complete traces

pass

@decorate_all_functions(xray_on_call)
class XRaySessionMaker(sessionmaker):
def __init__(self, bind=None, class_=XRaySession, autoflush=True,
autocommit=False,
expire_on_commit=True,
info=None, **kw):
kw['query_cls'] = XRayQuery
super().__init__(bind, class_, autoflush, autocommit, expire_on_commit,
info, **kw)
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
'Programming Language :: Python :: 3.6',
],

install_requires=['jsonpickle', 'wrapt', 'requests'],
install_requires=['jsonpickle', 'wrapt', 'requests', 'future'],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need future if you remove all print statement?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No future will still be needed for the super() builtin
from builtins import super


keywords='aws xray sdk',

Expand Down
Empty file.
79 changes: 79 additions & 0 deletions tests/ext/flask_sqlalchemy/test_query.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
from __future__ import absolute_import
import json
import jsonpickle
import pytest
from aws_xray_sdk.core import xray_recorder
from aws_xray_sdk.core.context import Context
from aws_xray_sdk.ext.sqlalchemy.query import XRayQuery, XRaySession
from aws_xray_sdk.ext.flask_sqlalchemy.query import XRayFlaskSqlAlchemy
from flask import Flask
from flask_sqlalchemy import SQLAlchemy


app = Flask(__name__)
app.config["SQLALCHEMY_TRACK_MODIFICATIONS"] = False
app.config["SQLALCHEMY_DATABASE_URI"] = "sqlite:///:memory:"
db = XRayFlaskSqlAlchemy(app)
class User(db.Model):
__tablename__ = "users"

id = db.Column(db.Integer, primary_key=True)
name = db.Column(db.String(255), nullable=False, unique=True)
fullname = db.Column(db.String(255), nullable=False)
password = db.Column(db.String(255), nullable=False)


def _search_entity(entity, name):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This along with find_sub could also be a helper class shared between your two query_test.py. And since these two functions are very general they could be under tests/ root so any other unit test can utilize your contribution.

"""Helper function to that recursivly looks at subentities
Returns a serialized entity that matches the name given or None"""
if 'name' in entity:
my_name = entity['name']
if my_name == name:
return entity
else:
if "subsegments" in entity:
for s in entity['subsegments']:
result = _search_entity(s,name)
if result != None:
return result
return None
def find_sub(segment, name):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would recommend rename it to find_subsegment. Abbreviate on functions is not a good practice in general if it is not absolutely needed.

"""Helper function to find a subsegment by name in the entity tree"""
segment = jsonpickle.encode(segment, unpicklable=False)
segment = json.loads(segment)
for entity in segment['subsegments']:
result = _search_entity(entity, name)
if result != None:
return result
return None

@pytest.fixture()
def session():
"""Test Fixture to Create DataBase Tables and start a trace segment"""
xray_recorder.configure(service='test', sampling=False, context=Context())
xray_recorder.clear_trace_entities()
xray_recorder.begin_segment('SQLAlchemyTest')
db.create_all()
yield
xray_recorder.end_segment()
xray_recorder.clear_trace_entities()

def test_all(capsys, session):
""" Test calling all() on get all records.
Verify that we capture trace of query and return the SQL as metdata"""
# with capsys.disabled():
User.query.all()
subsegment = find_sub(xray_recorder.current_segment(),'sqlalchemy.orm.query.all')
assert subsegment['name'] == 'sqlalchemy.orm.query.all'
assert subsegment['metadata']['default']['sql']

def test_add(capsys, session):
""" Test calling add() on insert a row.
Verify we that we capture trace for the add"""
# with capsys.disabled():
john = User(name='John', fullname = "John Doe", password="password")
db.session.add(john)
subsegment = find_sub(xray_recorder.current_segment(),'sqlalchemy.orm.session.add')
assert subsegment['name'] == 'sqlalchemy.orm.session.add'


Empty file.
82 changes: 82 additions & 0 deletions tests/ext/sqlalchemy/test_query.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
from __future__ import absolute_import
import pytest
import json
import jsonpickle
from aws_xray_sdk.core import xray_recorder
from aws_xray_sdk.core.context import Context
from aws_xray_sdk.ext.sqlalchemy.query import XRaySessionMaker
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy import *
# from sqlalchemy.orm import sessionmaker



Base = declarative_base()

class User(Base):
__tablename__ = 'users'

id = Column(Integer, primary_key=True)
name = Column(String)
fullname = Column(String)
password = Column(String)


def _search_entity(entity, name):
"""Helper function to that recursivly looks at subentities
Returns a serialized entity that matches the name given or None"""
if 'name' in entity:
my_name = entity['name']
if my_name == name:
return entity
else:
if "subsegments" in entity:
for s in entity['subsegments']:
result = _search_entity(s,name)
if result != None:
return result
return None
def find_sub(segment, name):
"""Helper function to find a subsegment by name in the entity tree"""
segment = jsonpickle.encode(segment, unpicklable=False)
segment = json.loads(segment)
for entity in segment['subsegments']:
result = _search_entity(entity, name)
if result != None:
return result
return None

@pytest.fixture()
def session():
"""Test Fixture to Create DataBase Tables and start a trace segment"""
engine = create_engine('sqlite:///:memory:')
xray_recorder.configure(service='test', sampling=False, context=Context())
xray_recorder.clear_trace_entities()
xray_recorder.begin_segment('SQLAlchemyTest')
Session = XRaySessionMaker(bind=engine)
Base.metadata.create_all(engine)
session = Session()
yield session
xray_recorder.end_segment()
xray_recorder.clear_trace_entities()

def test_all(capsys, session):
""" Test calling all() on get all records.
Verify we run the query and return the SQL as metdata"""
# with capsys.disabled():
session.query(User).all()
sub = find_sub(xray_recorder.current_segment(), 'sqlalchemy.orm.query.all')
assert sub['name'] == 'sqlalchemy.orm.query.all'
assert sub['metadata']['default']['sql']


def test_add(capsys, session):
""" Test calling add() on insert a row.
Verify we that we capture trace for the add"""
# with capsys.disabled():
john = User(name='John', fullname = "John Doe", password="password")
session.add(john)
sub = find_sub(xray_recorder.current_segment(), 'sqlalchemy.orm.session.add')
assert sub['name'] == 'sqlalchemy.orm.session.add'


Loading