Skip to content

DBM: Add option to append SQL comment propagation#3809

Merged
marcotc merged 7 commits into
masterfrom
appendsql
Aug 12, 2024
Merged

DBM: Add option to append SQL comment propagation#3809
marcotc merged 7 commits into
masterfrom
appendsql

Conversation

@marcotc

@marcotc marcotc commented Jul 25, 2024

Copy link
Copy Markdown
Member

Fixes #3601

Add an option (defaults to false) to append the DBM trace propagation comment instead, of prepending it.
This allow users using database monitor tools to see the SQL query itself first, instead of the Datadog trace propagation comment first.

Motivation:
To correlate Database Monitoring and Traces, the tracer add a SQL comments to instrumented SQL queries.

Currently, the comments are prepended to the query, to prevent any possible truncation from removing the propagation data.
But this means that, in any external tools use to monitor the database, the queries will have the Datadog propagation comment first, and not the query itself. This makes inspecting the query quite inconvenient in many tools.

How to test the change?
There are tests covering all the changes.

@marcotc marcotc self-assigned this Jul 25, 2024
@marcotc
marcotc requested review from a team as code owners July 25, 2024 20:08
@github-actions github-actions Bot added integrations Involves tracing integrations tracing labels Jul 25, 2024
Comment thread lib/datadog/tracing/contrib/mysql2/instrumentation.rb Outdated
Comment thread lib/datadog/tracing/contrib/mysql2/instrumentation.rb Outdated
Comment thread lib/datadog/tracing/contrib/trilogy/instrumentation.rb Outdated
Comment thread lib/datadog/tracing/contrib/pg/instrumentation.rb Outdated
Comment thread lib/datadog/tracing/contrib/mysql2/instrumentation.rb Outdated
Comment thread lib/datadog/tracing/contrib/mysql2/instrumentation.rb Outdated
Comment thread lib/datadog/tracing/contrib/trilogy/instrumentation.rb Outdated
Comment thread lib/datadog/tracing/contrib/pg/instrumentation.rb Outdated
@marcotc marcotc added the dbm Database Monitoring product label Jul 25, 2024
@pr-commenter

pr-commenter Bot commented Jul 25, 2024

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2024-08-12 21:58:03

Comparing candidate commit 8f4efc5 in PR branch appendsql with baseline commit 6b50227 in branch master.

Found 1 performance improvements and 1 performance regressions! Performance is the same for 21 metrics, 2 unstable metrics.

scenario:profiler - Allocations (baseline)

  • 🟥 throughput [-176084.521op/s; -162279.211op/s] or [-3.301%; -3.042%]

scenario:tracing - Tracing.log_correlation

  • 🟩 throughput [+3339.867op/s; +3713.190op/s] or [+2.843%; +3.161%]

@codecov-commenter

codecov-commenter commented Jul 25, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.91%. Comparing base (8fdf5a3) to head (8caeadc).
Report is 34 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3809   +/-   ##
=======================================
  Coverage   97.90%   97.91%           
=======================================
  Files        1261     1261           
  Lines       75620    75665   +45     
  Branches     3706     3710    +4     
=======================================
+ Hits        74036    74084   +48     
+ Misses       1584     1581    -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment thread lib/datadog/tracing/contrib/trilogy/instrumentation.rb Outdated
Comment thread lib/datadog/tracing/contrib/mysql2/instrumentation.rb Outdated
Comment thread lib/datadog/tracing/contrib/mysql2/instrumentation.rb Outdated
Comment thread lib/datadog/tracing/contrib/pg/instrumentation.rb Outdated
Comment thread lib/datadog/tracing/contrib/trilogy/instrumentation.rb Outdated
Comment thread lib/datadog/tracing/contrib/mysql2/instrumentation.rb Outdated
Comment thread lib/datadog/tracing/contrib/mysql2/instrumentation.rb Outdated
Comment thread lib/datadog/tracing/contrib/pg/instrumentation.rb Outdated
Comment thread lib/datadog/tracing/contrib/trilogy/instrumentation.rb Outdated
Comment thread lib/datadog/tracing/contrib/mysql2/instrumentation.rb Outdated
Comment thread lib/datadog/tracing/contrib/mysql2/instrumentation.rb Outdated
Comment thread lib/datadog/tracing/contrib/pg/instrumentation.rb Outdated
Comment thread lib/datadog/tracing/contrib/mysql2/instrumentation.rb Outdated
Comment thread lib/datadog/tracing/contrib/pg/instrumentation.rb Outdated
Comment thread lib/datadog/tracing/contrib/mysql2/instrumentation.rb Outdated
Comment thread lib/datadog/tracing/contrib/trilogy/instrumentation.rb Outdated
@marcotc
marcotc requested a review from TonyCTHsu July 26, 2024 21:54
Comment thread docs/GettingStarted.md Outdated
Comment thread lib/datadog/tracing/contrib/propagation/sql_comment/mode.rb Outdated
Comment thread docs/GettingStarted.md Outdated
Comment thread docs/GettingStarted.md
| `peer_service` | `DD_TRACE_TRILOGY_PEER_SERVICE` | `String` | Name of external service the application connects to | `nil` |
| `comment_propagation` | `DD_DBM_PROPAGATION_MODE` | `String` | SQL comment propagation mode for database monitoring. <br />(example: `disabled` \| `service`\| `full`). <br /><br />**Important**: _Note that enabling SQL comment propagation results in potentially confidential data (service names) being stored in the databases which can then be accessed by other third parties that have been granted access to the database._ | `'disabled'` |
| `append_propagation` | | `Bool` | Appends the SQL comment propagation to the query string. Prepends the comment if `false`. For long query strings, the appended propagation comment might be truncated, causing loss of correlation between the query and trace. | `false` |
| `on_error` | | `Proc` | Custom error handler invoked when MySQL raises an error. Provided `span` and `error` as arguments. Sets error on the span by default. Useful for ignoring errors that are handled at the application level. | `proc { \|span, error\| span.set_error(error) unless span.nil? }` |

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this documenting an existing option that wasn't documented? Would be clearer to have this addition in its own PR.

@marcotc marcotc Aug 9, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this documenting an existing option that wasn't documented?

Yes.

Can I keep it here, please? I will save me like 20 minutes now, then a context switch later, and some stress.

Comment thread lib/datadog/tracing/contrib/mysql2/instrumentation.rb Outdated
Comment thread lib/datadog/tracing/contrib/pg/instrumentation.rb Outdated
@marcotc
marcotc merged commit 4b7ec04 into master Aug 12, 2024
@marcotc
marcotc deleted the appendsql branch August 12, 2024 22:43
@github-actions github-actions Bot added this to the 2.3.0 milestone Aug 12, 2024
@TonyCTHsu TonyCTHsu mentioned this pull request Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dbm Database Monitoring product integrations Involves tracing integrations tracing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow to append (instead of prepend) Propagation::SqlComment comment

5 participants