feat: add sqlcommenter comment to mysql2 queries #1523
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
assigned blumamir, dyladan, haddasbronfman, legendecas, pichlermarc and rauno56
github-actions
bot
requested review from
blumamir,
dyladan,
haddasbronfman,
legendecas,
pichlermarc and
rauno56
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1523 +/- ##
==========================================
- Coverage 96.13% 93.70% -2.43%
==========================================
Files 14 49 +35
Lines 906 2702 +1796
Branches 197 504 +307
==========================================
+ Hits 871 2532 +1661
- Misses 35 170 +135
|
|
Is there anything blocking this PR at the moment given the coverage regressions aren't coming from the patch ? |
|
Yes. there is a conflict in |
haddasbronfman
approved these changes
Jun 21, 2023
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which problem is this PR solving?
This PR adds support for inserting sqlcommenter comments in the
mysql2instrumentation the same way #1286 did forpg.Short description of the changes
The common logic for detecting and writing SQL comments has been extracted to a
sql-commonpackage which is now depended on by both themysql2andpginstrumentations. I'm not super confident about the way new packages should be created (versioning, dependencies, etc etc) so let me know if anything there needs to be changed.I'm also planning to also add support to the
mysqlinstrumentation at some point after this is merged, and potentially improve the comment detection logic to reduce false positives.The new config option essentially works the exact same way as the existing one for
pg. It's not enabled for prepared queries for fairly obvious reasons.I had to change some of the MySQL options used for the local and CI containers to be able to check the comments are successfully added by looking at the log. The CI variant feels a little dirty but GitHub apparently doesn't support passing arguments to service containers so it's the best way I could think of.