[dbapi] Trace db fetch and session methods #664

labbati · 2018-10-22T14:30:55Z

In addition to execute, executemany and callproc methods, we want to trace also the various fetch methods to provide users with a more powerful set of information.
In the process of doing so, we realized that the django integration was using a custom tailored version of the dbapi traced cursor that could be replaced with the generic dbapi traced cursor.

Further, we want to create spans for the session-level commit and rollback methods.

brettlangdon · 2018-10-22T18:59:53Z

do you have any sample traces? mostly curious to see what these look like, it is usually a 1:1 of sql.query to sql.query.fetch* method?

labbati · 2018-10-23T08:17:36Z

do you have any sample traces? mostly curious to see what these look like, it is usually a 1:1 of sql.query to sql.query.fetch* method?

Sure here it is

To answer you question, correct, there is one sql.query.fetch* (or more in the case of multiple calls to fetchone/fetchmany ) for each sql.query (of course when a fetch is called).

brettlangdon · 2018-10-23T13:03:08Z

cool, looks great. I think my concern was if we end up with a library that calls .fetchmany(1) like 1k times to pull in 1k items or something funky... but.... lets hope that no one does that :)

labbati · 2018-10-25T07:45:09Z

Your concern is absolutely reasonable. I would say that in the real world if someone calls .fetchmany(1) (or .execute()) 1k times, both the span overhead in terms of cpu and IO (to transfer spans to the agent) would still be (probably) irrelevant compared to the DB interactions. But good thing we are thinking about it.

brettlangdon · 2018-10-25T11:30:15Z

is this still considered a WIP?

labbati · 2018-10-25T14:20:51Z

@brettlangdon I need to take some time to go through it for final checks. Will do asap!

ddtrace/contrib/dbapi/__init__.py

ddtrace/contrib/django/db.py

brettlangdon

big picture looks pretty good, few minor changes.

ddtrace/contrib/dbapi/__init__.py

tests/contrib/dbapi/test_unit.py

tests/contrib/sqlite3/test_sqlite3.py

tox.ini

Resolved

… integration backward compatibilityè

* [core] add Tracer.on and Tracer.emit for span hooks * [core] rename Tracer.emit to Tracer._emit * [core] fix mispelling of Tracer._hooks * [core] do not raise an exception or error log * [falcon] add span hook documentation for Falcon * [core] add tests for Tracer.on/Tracer._emit * [core] add tracer hook argument tests * [core] register span hooks on config object instead * [core] fix flake8 issues * Update ddtrace/settings.py * Update ddtrace/settings.py * Update ddtrace/settings.py * [core] remove Hooks.__getattr__, add Hooks.on alias for Hooks.register

tox.ini

Kyle-Verhoog

Just a couple things to clean-up. Looks good otherwise.

ddtrace/contrib/dbapi/__init__.py

tests/contrib/dbapi/__init__.py

Kyle-Verhoog

Good stuff. Caught a find and replace mistake, but isn't show stopping.

tests/contrib/sqlite3/test_sqlite3.py

Co-Authored-By: majorgreys <tahir@tahirbutt.com>

* [dbapi] Trace fetch methods * [dbapi] Fix unit tests after adding the row count as a tag for django integration backward compatibilityè * [dbapi] Minor fixes to the the dbapi tracing implementation * Add tracing to commit and rollback to dbapi * Add tests for dbapi TracedConnections * [core] add support for integration span hooks (#679) * [core] add Tracer.on and Tracer.emit for span hooks * [core] rename Tracer.emit to Tracer._emit * [core] fix mispelling of Tracer._hooks * [core] do not raise an exception or error log * [falcon] add span hook documentation for Falcon * [core] add tests for Tracer.on/Tracer._emit * [core] add tracer hook argument tests * [core] register span hooks on config object instead * [core] fix flake8 issues * Update ddtrace/settings.py * Update ddtrace/settings.py * Update ddtrace/settings.py * [core] remove Hooks.__getattr__, add Hooks.on alias for Hooks.register * Add tests to dbapi for commit/rollback * Update test for fetch tracing * Update tests for changes to commit/rollback tracing * Replace double quotes * Replace pytest+fixtures with unittest * Add tracer to pin * Convert double quotes to single * Remove unnecessary tox changes * Update tests * Clean-up * Remove duplicate test * Update tests/contrib/sqlite3/test_sqlite3.py Co-Authored-By: majorgreys <tahir@tahirbutt.com>

labbati added core labels Oct 22, 2018

brettlangdon reviewed Oct 25, 2018

View reviewed changes

ddtrace/contrib/dbapi/__init__.py Outdated Show resolved Hide resolved

ddtrace/contrib/django/db.py Show resolved Hide resolved

ddtrace/contrib/django/db.py Outdated Show resolved Hide resolved

majorgreys changed the title ~~Trace fetch methods~~ Trace dbapi fetch and session methods Nov 7, 2018

brettlangdon previously requested changes Nov 8, 2018

View reviewed changes

majorgreys self-assigned this Nov 8, 2018

brettlangdon closed this Nov 9, 2018

brettlangdon reopened this Nov 9, 2018

brettlangdon added this to the 0.17.0 milestone Nov 9, 2018

brettlangdon changed the base branch from 0.16-dev to 0.17-dev November 12, 2018 17:31

labbati and others added 13 commits November 12, 2018 16:25

[dbapi] Trace fetch methods

9b6add9

[dbapi] Fix unit tests after adding the row count as a tag for django…

98dc828

… integration backward compatibilityè

[dbapi] Minor fixes to the the dbapi tracing implementation

a6fa187

Add tracing to commit and rollback to dbapi

2bb0cda

Add tests for dbapi TracedConnections

4851ee0

Add tests to dbapi for commit/rollback

b179235

Update test for fetch tracing

052862c

Update tests for changes to commit/rollback tracing

4bd6333

Replace double quotes

6389896

Replace pytest+fixtures with unittest

8287dfe

Add tracer to pin

5d01b2e

Convert double quotes to single

81cfcac

Remove unnecessary tox changes

0929589

majorgreys force-pushed the labbati/dbapi-trace-fetch branch from b0b9de9 to 0929589 Compare November 12, 2018 21:38

brettlangdon reviewed Nov 12, 2018

View reviewed changes

tox.ini Outdated Show resolved Hide resolved

Kyle-Verhoog reviewed Nov 13, 2018

View reviewed changes

ddtrace/contrib/dbapi/__init__.py Outdated Show resolved Hide resolved

tests/contrib/dbapi/__init__.py Outdated Show resolved Hide resolved

majorgreys added 3 commits November 13, 2018 12:57

Update tests

5ae3c00

Clean-up

e7c50b4

Remove duplicate test

c49c18f

majorgreys changed the title ~~Trace dbapi fetch and session methods~~ [dbapi] Trace db fetch and session methods Nov 13, 2018

Merge branch '0.17-dev' into labbati/dbapi-trace-fetch

014062a

Kyle-Verhoog previously approved these changes Nov 14, 2018

View reviewed changes

tests/contrib/sqlite3/test_sqlite3.py Outdated Show resolved Hide resolved

majorgreys and others added 3 commits November 14, 2018 10:23

Merge branch '0.17-dev' into labbati/dbapi-trace-fetch

dbe926e

Merge branch '0.17-dev' into labbati/dbapi-trace-fetch

1510e26

Update tests/contrib/sqlite3/test_sqlite3.py

415e5b1

Co-Authored-By: majorgreys <tahir@tahirbutt.com>

majorgreys dismissed stale reviews from Kyle-Verhoog via 415e5b1 November 14, 2018 15:45

Kyle-Verhoog removed the do-not-merge/WIP label Nov 14, 2018

Kyle-Verhoog approved these changes Nov 14, 2018

View reviewed changes

majorgreys merged commit 04da524 into 0.17-dev Nov 14, 2018

majorgreys deleted the labbati/dbapi-trace-fetch branch November 14, 2018 16:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[dbapi] Trace db fetch and session methods #664

[dbapi] Trace db fetch and session methods #664

labbati commented Oct 22, 2018 •

edited by majorgreys

brettlangdon commented Oct 22, 2018

labbati commented Oct 23, 2018 •

edited

brettlangdon commented Oct 23, 2018

labbati commented Oct 25, 2018

brettlangdon commented Oct 25, 2018

labbati commented Oct 25, 2018

brettlangdon left a comment

Kyle-Verhoog left a comment

Kyle-Verhoog left a comment

[dbapi] Trace db fetch and session methods #664

[dbapi] Trace db fetch and session methods #664

Conversation

labbati commented Oct 22, 2018 • edited by majorgreys

brettlangdon commented Oct 22, 2018

labbati commented Oct 23, 2018 • edited

brettlangdon commented Oct 23, 2018

labbati commented Oct 25, 2018

brettlangdon commented Oct 25, 2018

labbati commented Oct 25, 2018

brettlangdon left a comment

Choose a reason for hiding this comment

Kyle-Verhoog left a comment

Choose a reason for hiding this comment

Kyle-Verhoog left a comment

Choose a reason for hiding this comment

labbati commented Oct 22, 2018 •

edited by majorgreys

labbati commented Oct 23, 2018 •

edited