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

Tracing Dynamic Configuration #2848

Merged
merged 26 commits into from
Jul 26, 2023
Merged

Tracing Dynamic Configuration #2848

merged 26 commits into from
Jul 26, 2023

Conversation

marcotc
Copy link
Member

@marcotc marcotc commented May 15, 2023

Adds support for the ddtrace to receive APM_LIBRARY Dynamic Configuration, through the existing Remote Configuration channel, and applies these changes to the local application.

Dynamic Configuration has precedence over locally configured values.

Dynamic Configuration can be disabled at any point in time, restore the locally configured values.

This PR passes our new system-tests for Dynamic Configuration: DataDog/system-tests#1172

Notes

All components are safely reconfigured on-the-fly; ddtrace doesn't need to be reconfigured (Datadog.configure{}) for them to apply.

In fact, invoking Datadog.configure{} to apply Remote Configuration has several issues:

  • Dynamic Configuration today does not utilize The Update Framework's feature that allow us to know which values were changed, so we are forced to apply all values all the time, this is problematic because it would mean a Datadog.configure{} every 5 seconds. We could track the value changes ourselves, but now we are adding an unnecessary complexity layer on a system that has already solved it for us.
  • The Remote Configuration configuration work is in itself a Component, thus it gets shut down on Datadog.configure{}, which causes the reconfiguration process itself to be aborted (the active thread is killed) and ddtrace ends up in an inconsistent state.
  • Moving Datadog.configure{} to a separate execution context (e.g. another thread) opens room for race conditions, especially when the new Remote Configuration component initializes and tries to apply Remote Configuration values very quickly. I tried adding synchronization mechanisms, buy my tests were consistently deadlocking.

Moving Remote Configuration outside of Components is likely an option, but the complexity is quite large was we'd need a separate lifecycle management mechanism for it, and it depends on configuration values (e.g. agent.host, remote.enabled), which are naturally configured alongside the Component lifecycle management. I did not attempt this, as it seemed like its own project, but I think it's worth discussing.

@github-actions github-actions bot added core Involves Datadog core libraries tracing labels May 15, 2023
@marcotc marcotc force-pushed the tracing-remote-configuration branch from 2ad7381 to c4871d7 Compare June 19, 2023 21:32
@marcotc marcotc closed this Jun 19, 2023
@marcotc marcotc force-pushed the tracing-remote-configuration branch from c4871d7 to a734980 Compare June 19, 2023 21:34
@github-actions github-actions bot added dev/ci Involves CircleCI, GitHub Actions, or GitLab dev/github Github repository maintenance and automation dev/testing Involves testing processes (e.g. RSpec) docs Involves documentation release Official release and removed tracing core Involves Datadog core libraries labels Jun 19, 2023
@marcotc marcotc reopened this Jun 19, 2023
@marcotc marcotc added feature Involves a product feature and removed release Official release dev/github Github repository maintenance and automation dev/testing Involves testing processes (e.g. RSpec) dev/ci Involves CircleCI, GitHub Actions, or GitLab docs Involves documentation labels Jun 22, 2023
@marcotc marcotc force-pushed the tracing-remote-configuration branch from d74d5b1 to df3227d Compare July 20, 2023 22:16
@marcotc marcotc changed the base branch from master to rc-base July 20, 2023 22:16
@github-actions github-actions bot added appsec Application Security monitoring product core Involves Datadog core libraries integrations Involves tracing integrations tracing labels Jul 20, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jul 20, 2023

Codecov Report

Merging #2848 (afb8bee) into master (242c247) will decrease coverage by 0.01%.
The diff coverage is 97.92%.

@@            Coverage Diff             @@
##           master    #2848      +/-   ##
==========================================
- Coverage   98.08%   98.08%   -0.01%     
==========================================
  Files        1309     1316       +7     
  Lines       73439    73820     +381     
  Branches     3378     3385       +7     
==========================================
+ Hits        72031    72403     +372     
- Misses       1408     1417       +9     
Files Changed Coverage Δ
lib/datadog/core/transport/http/config.rb 83.72% <50.00%> (-0.54%) ⬇️
spec/support/tracer_helpers.rb 89.28% <50.00%> (-1.46%) ⬇️
spec/datadog/core/configuration/components_spec.rb 99.04% <88.23%> (-0.81%) ⬇️
...ib/datadog/tracing/configuration/dynamic/option.rb 95.65% <95.65%> (ø)
lib/datadog/tracing/remote.rb 97.29% <97.29%> (ø)
lib/datadog/core/configuration/components.rb 100.00% <100.00%> (ø)
lib/datadog/core/configuration/option.rb 99.23% <100.00%> (+<0.01%) ⬆️
lib/datadog/core/remote/client/capabilities.rb 100.00% <100.00%> (ø)
lib/datadog/core/remote/component.rb 94.11% <100.00%> (-0.33%) ⬇️
lib/datadog/tracing/component.rb 100.00% <100.00%> (ø)
... and 12 more

... and 2 files with indirect coverage changes

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

@github-actions github-actions bot removed the appsec Application Security monitoring product label Jul 20, 2023
@marcotc marcotc changed the title Tracing Remote Configuration Tracing Dynamic Configuration Jul 24, 2023
@marcotc marcotc changed the base branch from rc-base to master July 25, 2023 21:31
@marcotc marcotc force-pushed the tracing-remote-configuration branch from 746dea5 to afb8bee Compare July 26, 2023 19:32
@marcotc marcotc merged commit e5b8638 into master Jul 26, 2023
145 of 162 checks passed
@marcotc marcotc deleted the tracing-remote-configuration branch July 26, 2023 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Involves Datadog core libraries feature Involves a product feature tracing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants