[mysqlb] Fixing missing db type #468
Conversation
ddtrace/contrib/dbapi/__init__.py
Outdated
| super(TracedConnection, self).__init__(conn) | ||
| name = _get_vendor(conn) | ||
| Pin(service=name, app=name).onto(self) | ||
| if pin and pin.enabled(): |
There was a problem hiding this comment.
if we pass a disabled pin, we'll create a new one that is enabled. Not sure it's the right thing to do. There is an edge case that you're trying to address?
Also what if we write this code as (in case enabled() is not required):
name = _get_vendor(conn)
db_pin = pin or Pin(service=name, app=name, app_type="db")
db_pin.onto(self)There was a problem hiding this comment.
No particular reason. I'll use that.
ddtrace/contrib/mysqldb/patch.py
Outdated
| @@ -58,6 +58,6 @@ def patch_conn(conn, *args, **kwargs): | |||
| pin = Pin(service="mysql", app="mysql", app_type="db", tags=tags) | |||
|
|
|||
| # grab the metadata from the conn | |||
| wrapped = TracedConnection(conn) | |||
| wrapped = TracedConnection(conn, pin) | |||
There was a problem hiding this comment.
Let's use the kwarg:
wrapped = TracedConnection(conn, pin=pin)Also can you check other mysql integrations? do they have the same issue?
If yes, let's do the same thing you did here in this PR, so that we fix all of them at once.
There was a problem hiding this comment.
I tested the other 2 mysql integrations.
pymysql was working fine.
mysql-connector had the same issue.
I patched both to be safe, tell me if it's not relevant.
ddtrace/contrib/dbapi/__init__.py
Outdated
| if pin and pin.enabled(): | ||
| pin.onto(self) | ||
| else: | ||
| Pin(service=name, app=name, app_type="db").onto(self) |
There was a problem hiding this comment.
instead of using db can you:
from ...ext import http, AppTypes # from here should be the right import
Pin(service=name, app=name, app_type=AppTypes.db).onto(self)There was a problem hiding this comment.
It should be applied everywhere since we're here at this point.
alxch-
force-pushed
the
alex.charrier/mysql-msgpack
branch
from
e57333c
to
5f7df06
Compare
The way the
MySQLdblibrary is patched usesdbapi. Then when initializing aTracedConnectionobject, a new Pin was created that did not havetype, that where the error came from.This PR solves this issue and prevents from getting 2 services reported for the
MySQLdbintegration. If passing the pin parameter when initializing, it will not create a new one.