Auto instrumentation for mysqli #318
Conversation
intuibase
commented
Dec 4, 2024
•
intuibase
commented
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #318 +/- ##
============================================
+ Coverage 79.60% 86.02% +6.41%
- Complexity 729 1070 +341
============================================
Files 69 73 +4
Lines 2952 4450 +1498
============================================
+ Hits 2350 3828 +1478
- Misses 602 622 +20
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 46 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
| $attributes = $tracker->getMySqliAttributes($mysqli); | ||
|
|
||
| if ($transactionName) { | ||
| $attributes['db.transaction.name'] = $transactionName; |
There was a problem hiding this comment.
In reference to the discussion on SIG and the custom attribute db.transaction.name, I looked into it and found that there is no corresponding entity in sem-conv. I started digging deeper into the topic and discovered that this is typical for the MySQLi extension in PHP. PDO does not allow setting a transaction name.
I checked the code of mysqlnd (the base driver for MySQLi and PDO), and it seems that there is no such concept as a transaction name. In mysqlnd, this is implemented by setting a comment in the SQL code—it is not interpreted by the database itself as a name but is treated as a regular comment.
Therefore, I suspect it will not be possible to add this entity to the semantic conventions, as it is specific to this particular driver.
There was a problem hiding this comment.
Discussed this in SIG, and we decided that transaction name is useful telemetry metadata and should be added. Since this instrumentation is unstable, the spec says we can add additional telemetry: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/telemetry-stability.md#unstable-instrumentations
It would be worth pursuing having db.transaction.name or something like it added to semconv.
|
@intuibase are you happy for me to merge this and get a 0.0.1 version out? |
Yes, please :) |
Co-authored-by: Chris Lightfoot-Wild <[email protected]>
|
@ChrisLightfootWild thanks, I've improved parser a bit, now it should work with semicolons inside quotes. |
