Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQL comment propagation full mode with traceparent #2464

Merged
merged 3 commits into from
Dec 7, 2022

Conversation

TonyCTHsu
Copy link
Contributor

What does this PR do?

Implement SQL comment propagation with full mode, which includes traceparent

@github-actions github-actions bot added integrations Involves tracing integrations tracing labels Dec 5, 2022
@TonyCTHsu TonyCTHsu force-pushed the tonycthsu/sql-comment-propagation-full branch from 9e9e33b to 8cd795c Compare December 5, 2022 14:59
Copy link
Contributor

@delner delner left a comment

Choose a reason for hiding this comment

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

Any general call outs that we should review more closely here? Overall looks good (intuitive) from a skim,.

@@ -12,36 +14,30 @@ module SqlComment
def self.annotate!(span_op, mode)
return unless mode.enabled?

# PENDING: Until `traceparent`` implementation in `full` mode
# span_op.set_tag(Ext::TAG_DBM_TRACE_INJECTED, true) if mode.full?
span_op.set_tag(Ext::TAG_DBM_TRACE_INJECTED, true) if mode.full?
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this tag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tag is defined to help filter for spans on the APM that have full trace context in their sql comment

@TonyCTHsu TonyCTHsu marked this pull request as ready for review December 5, 2022 22:31
@TonyCTHsu TonyCTHsu requested a review from a team December 5, 2022 22:31
@TonyCTHsu TonyCTHsu self-assigned this Dec 6, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #2464 (6ce4135) into master (91cc56a) will increase coverage by 0.01%.
The diff coverage is 99.29%.

@@            Coverage Diff             @@
##           master    #2464      +/-   ##
==========================================
+ Coverage   98.01%   98.02%   +0.01%     
==========================================
  Files        1105     1108       +3     
  Lines       59276    59772     +496     
==========================================
+ Hits        58100    58593     +493     
- Misses       1176     1179       +3     
Impacted Files Coverage Δ
spec/support/core_helpers.rb 85.00% <85.00%> (ø)
lib/datadog/tracing/distributed/trace_context.rb 99.32% <99.32%> (ø)
lib/datadog/core.rb 93.33% <100.00%> (+5.83%) ⬆️
lib/datadog/core/configuration/settings.rb 97.66% <100.00%> (+0.13%) ⬆️
lib/datadog/core/environment/variable_helpers.rb 100.00% <100.00%> (ø)
lib/datadog/tracing/configuration/ext.rb 100.00% <100.00%> (ø)
...og/tracing/contrib/grpc/distributed/propagation.rb 100.00% <100.00%> (ø)
...og/tracing/contrib/http/distributed/propagation.rb 100.00% <100.00%> (ø)
.../datadog/tracing/contrib/mysql2/instrumentation.rb 100.00% <100.00%> (ø)
lib/datadog/tracing/contrib/pg/instrumentation.rb 100.00% <100.00%> (ø)
... and 19 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@TonyCTHsu TonyCTHsu merged commit 030ca5f into master Dec 7, 2022
@TonyCTHsu TonyCTHsu deleted the tonycthsu/sql-comment-propagation-full branch December 7, 2022 10:36
@github-actions github-actions bot added this to the 1.8.0 milestone Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrations Involves tracing integrations tracing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants