include rows_affected count in dbapi2 instrumentations
#614
Conversation
beniwohli
changed the title
affected_rows count in dbapi2 instrumentationsrows_affected count in dbapi2 instrumentations
| return method(sql, params) | ||
| result = method(sql, params) | ||
| # store "rows affected", but only for insert/update/delete | ||
| if span and self.rowcount not in (-1, None) and signature[:6] in DML_QUERIES: |
There was a problem hiding this comment.
maybe extract 6 to the top of the file, and compute it from the lengths of the strings in DML_QUERIES? we might end up adding additional dialect-specific statement types
There was a problem hiding this comment.
yeah, this seemed like a cute little optimization at the time ("oh look, all 3 keywords are 6 characters long, nice!"), but probably not worth the hassle. I'll modify to signature.startswith(DML_QUERIES), which, funny story, according to timeit is about 10% faster ¯\_(ツ)_/¯.
But you bring up an interesting point regarding dialects. DML_QUERIES should probably live on the CursorProxy, so that driver-specific subclasses can override it.
# Conflicts: # CHANGELOG.md # elasticapm/traces.py # tests/instrumentation/pymssql_tests.py
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
|
* include `rows_affected` count in dbapi2 instrumentations fixes elastic#613 * fix tests * only capture `rows_affected` for INSERT/UPDATE/DELETE queries * use `startswith(DML_QUERIES)` instead of relying on all keywords being 6 characters * update changelog
* include `rows_affected` count in dbapi2 instrumentations fixes elastic#613 * fix tests * only capture `rows_affected` for INSERT/UPDATE/DELETE queries * use `startswith(DML_QUERIES)` instead of relying on all keywords being 6 characters * update changelog
* include `rows_affected` count in dbapi2 instrumentations fixes elastic#613 * fix tests * only capture `rows_affected` for INSERT/UPDATE/DELETE queries * use `startswith(DML_QUERIES)` instead of relying on all keywords being 6 characters * update changelog
fixes #613
depends on elastic/apm-server#2802