otelsql icon indicating copy to clipboard operation
otelsql copied to clipboard

Add an option to tag spans with SQL query args

Open jhchabran opened this issue 2 years ago • 4 comments

Hi @XSAM,

This is a follow-up to https://github.com/XSAM/otelsql/pull/115, though were you not interested in merging it, I'll be happy to untangle the git history from those other changes.

This introduces an additional configuration option that enables to tag the SQL queries with their values, so anyone looking at the traces can also see the values.

Here is a screenshot of what it looks like: (see db.args.$1)

image

If that's good for you, I'll also update the README 🙏


Requires #115 to be merged first, albeit the changes are not really related, but as we did it this way, I'd rather check with you before reworking the PR if that's really needed.

jhchabran avatar Sep 05 '22 09:09 jhchabran

Codecov Report

Merging #116 (74375c8) into main (4285d8e) will increase coverage by 0.5%. The diff coverage is 100.0%.

@@           Coverage Diff           @@
##            main    #116     +/-   ##
=======================================
+ Coverage   80.6%   81.1%   +0.5%     
=======================================
  Files         13      13             
  Lines        692     711     +19     
=======================================
+ Hits         558     577     +19     
  Misses       109     109             
  Partials      25      25             
Impacted Files Coverage Δ
config.go 93.3% <100.0%> (+2.7%) :arrow_up:
conn.go 79.0% <100.0%> (+0.7%) :arrow_up:
connector.go 100.0% <100.0%> (ø)
rows.go 56.5% <100.0%> (ø)
stmt.go 78.9% <100.0%> (ø)
tx.go 100.0% <100.0%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Sep 05 '22 09:09 codecov[bot]

Hi @jhchabran, thanks for this PR.

Could I know more about the specific use case of this feature?

XSAM avatar Sep 10 '22 03:09 XSAM

@XSAM absolutely!

When we're looking at traces in prod to debug stuff, some queries can be really hairy and the added ability to look at what are the values for the query args is really useful. Basically everything needed to debug is at the same place, which is very convenient.

jhchabran avatar Sep 21 '22 09:09 jhchabran

@XSAM absolutely!

When we're looking at traces in prod to debug stuff, some queries can be really hairy and the added ability to look at what are the values for the query args is really useful. Basically everything needed to debug is at the same place, which is very convenient.

This sounds useful! But db.args.%1 is not a standard semantic on Semantic conventions for database client calls. I would prefer to create a new function or interface as an option for end-users so that they can generate extra attributes by their preference.

Like

CustomAttributes(ctx context.Context, cfg config, query string, args []driver.NamedValue) []attribute.KeyValue

@jhchabran I can create a PR to provide a such option if you like this proposal.

XSAM avatar Sep 21 '22 10:09 XSAM