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

Move Tracing config settings from Core to Tracing #2459

Merged
merged 1 commit into from
Dec 5, 2022

Conversation

delner
Copy link
Contributor

@delner delner commented Dec 3, 2022

What does this PR do?

Moves Tracing specific configuration out of Core and to Tracing.

Motivation

The larger goal is to reduce Core's knowledge of Tracing components, thereby reducing coupling.

This particular pull request does not fundamentally change the dependency of Core upon Tracing, but it does at least move the code itself into the appropriate folder. The resulting changes exposes the dependency a bit more explicitly, and should make it easier to invert later.

Additional Notes

The settings themselves did not change, despite the large line count. settings :tracing merely moved into tracing/configuration/settings.rb instead. Consequently, so did the specs for these settings.

It's worth noting that there's at least one lingering configuration setting agent.tracer.port. This one likely constitutes a breaking change, but its ownership should also likely be moved to Tracing. Left this alone for now.

Follow up from #2453

@delner delner added core Involves Datadog core libraries dev/refactor Involves refactoring existing components dev/internal Other internal work that does not need to be included in the changelog labels Dec 3, 2022
@delner delner requested review from marcotc and a team December 3, 2022 00:01
@delner delner self-assigned this Dec 3, 2022
@codecov-commenter
Copy link

codecov-commenter commented Dec 3, 2022

Codecov Report

Merging #2459 (dc759cd) into master (c213f61) will increase coverage by 0.00%.
The diff coverage is 99.02%.

@@           Coverage Diff           @@
##           master    #2459   +/-   ##
=======================================
  Coverage   98.02%   98.02%           
=======================================
  Files        1108     1110    +2     
  Lines       59756    59771   +15     
=======================================
+ Hits        58576    58593   +17     
+ Misses       1180     1178    -2     
Impacted Files Coverage Δ
lib/datadog/core/configuration/ext.rb 100.00% <ø> (ø)
lib/datadog/tracing/configuration/ext.rb 100.00% <ø> (ø)
spec/datadog/core/configuration/settings_spec.rb 100.00% <ø> (ø)
lib/datadog/tracing/configuration/settings.rb 95.91% <95.91%> (ø)
lib/datadog/core/configuration/settings.rb 99.19% <100.00%> (+1.53%) ⬆️
...pec/datadog/tracing/configuration/settings_spec.rb 100.00% <100.00%> (ø)
...ng/contrib/active_support/cache/instrumentation.rb 87.67% <0.00%> (+0.08%) ⬆️
lib/datadog/core/workers/polling.rb 100.00% <0.00%> (+3.44%) ⬆️
spec/support/http_helpers.rb 100.00% <0.00%> (+9.09%) ⬆️

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

@delner delner force-pushed the refactor/extract_tracing_config_settings branch from f09b28c to dc759cd Compare December 3, 2022 02:10
@delner delner requested a review from a team December 3, 2022 02:11
@delner delner merged commit 3424345 into master Dec 5, 2022
@delner delner deleted the refactor/extract_tracing_config_settings branch December 5, 2022 16:41
@github-actions github-actions bot added this to the 1.8.0 milestone Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Involves Datadog core libraries dev/internal Other internal work that does not need to be included in the changelog dev/refactor Involves refactoring existing components tracing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants