Refactor config into its own module #9426
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PerfectSlayer
added
comp: core
|
🎯 Code Coverage 🔗 Commit SHA: b40c033 | Docs | Was this helpful? Give us feedback! |
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 46 metrics, 13 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.54.0-SNAPSHOT~b40c033805, baseline=1.54.0-SNAPSHOT~8f4fb427ca
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.066 s) : 0, 1065572
Total [baseline] (8.66 s) : 0, 8660091
Agent [candidate] (1.065 s) : 0, 1065026
Total [candidate] (8.636 s) : 0, 8636325
section iast
Agent [baseline] (1.194 s) : 0, 1193951
Total [baseline] (9.328 s) : 0, 9327800
Agent [candidate] (1.191 s) : 0, 1190808
Total [candidate] (9.325 s) : 0, 9325382
gantt
title insecure-bank - break down per module: candidate=1.54.0-SNAPSHOT~b40c033805, baseline=1.54.0-SNAPSHOT~8f4fb427ca
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.461 ms) : 0, 1461
crashtracking [candidate] (1.465 ms) : 0, 1465
BytebuddyAgent [baseline] (735.925 ms) : 0, 735925
BytebuddyAgent [candidate] (735.016 ms) : 0, 735016
GlobalTracer [baseline] (252.878 ms) : 0, 252878
GlobalTracer [candidate] (253.202 ms) : 0, 253202
AppSec [baseline] (30.683 ms) : 0, 30683
AppSec [candidate] (30.725 ms) : 0, 30725
Debugger [baseline] (6.427 ms) : 0, 6427
Debugger [candidate] (6.409 ms) : 0, 6409
Remote Config [baseline] (690.06 µs) : 0, 690
Remote Config [candidate] (691.353 µs) : 0, 691
Telemetry [baseline] (16.464 ms) : 0, 16464
Telemetry [candidate] (16.371 ms) : 0, 16371
section iast
crashtracking [baseline] (1.477 ms) : 0, 1477
crashtracking [candidate] (1.463 ms) : 0, 1463
BytebuddyAgent [baseline] (855.298 ms) : 0, 855298
BytebuddyAgent [candidate] (852.721 ms) : 0, 852721
GlobalTracer [baseline] (249.578 ms) : 0, 249578
GlobalTracer [candidate] (244.171 ms) : 0, 244171
IAST [baseline] (27.167 ms) : 0, 27167
IAST [candidate] (31.264 ms) : 0, 31264
AppSec [baseline] (24.607 ms) : 0, 24607
AppSec [candidate] (25.409 ms) : 0, 25409
Debugger [baseline] (6.001 ms) : 0, 6001
Debugger [candidate] (6.05 ms) : 0, 6050
Remote Config [baseline] (602.492 µs) : 0, 602
Remote Config [candidate] (614.592 µs) : 0, 615
Telemetry [baseline] (8.177 ms) : 0, 8177
Telemetry [candidate] (8.188 ms) : 0, 8188
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.54.0-SNAPSHOT~b40c033805, baseline=1.54.0-SNAPSHOT~8f4fb427ca
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.069 s) : 0, 1069372
Total [baseline] (10.791 s) : 0, 10791274
Agent [candidate] (1.062 s) : 0, 1061954
Total [candidate] (10.83 s) : 0, 10830391
section appsec
Agent [baseline] (1.235 s) : 0, 1235486
Total [baseline] (11.11 s) : 0, 11109520
Agent [candidate] (1.247 s) : 0, 1246802
Total [candidate] (11.101 s) : 0, 11100876
section iast
Agent [baseline] (1.196 s) : 0, 1195781
Total [baseline] (11.033 s) : 0, 11033498
Agent [candidate] (1.204 s) : 0, 1203680
Total [candidate] (9.717 s) : 0, 9716829
section profiling
Agent [baseline] (1.221 s) : 0, 1220531
Total [baseline] (11.068 s) : 0, 11067522
Agent [candidate] (1.213 s) : 0, 1212676
Total [candidate] (11.058 s) : 0, 11057946
gantt
title petclinic - break down per module: candidate=1.54.0-SNAPSHOT~b40c033805, baseline=1.54.0-SNAPSHOT~8f4fb427ca
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.472 ms) : 0, 1472
crashtracking [candidate] (1.455 ms) : 0, 1455
BytebuddyAgent [baseline] (739.87 ms) : 0, 739870
BytebuddyAgent [candidate] (733.087 ms) : 0, 733087
GlobalTracer [baseline] (253.457 ms) : 0, 253457
GlobalTracer [candidate] (252.571 ms) : 0, 252571
AppSec [baseline] (30.587 ms) : 0, 30587
AppSec [candidate] (30.42 ms) : 0, 30420
Debugger [baseline] (6.425 ms) : 0, 6425
Debugger [candidate] (6.364 ms) : 0, 6364
Remote Config [baseline] (692.683 µs) : 0, 693
Remote Config [candidate] (690.209 µs) : 0, 690
Telemetry [baseline] (15.524 ms) : 0, 15524
Telemetry [candidate] (16.238 ms) : 0, 16238
section appsec
crashtracking [baseline] (1.457 ms) : 0, 1457
crashtracking [candidate] (1.467 ms) : 0, 1467
BytebuddyAgent [baseline] (755.741 ms) : 0, 755741
BytebuddyAgent [candidate] (763.876 ms) : 0, 763876
GlobalTracer [baseline] (245.924 ms) : 0, 245924
GlobalTracer [candidate] (248.037 ms) : 0, 248037
IAST [baseline] (23.749 ms) : 0, 23749
IAST [candidate] (24.084 ms) : 0, 24084
AppSec [baseline] (171.473 ms) : 0, 171473
AppSec [candidate] (172.626 ms) : 0, 172626
Debugger [baseline] (6.763 ms) : 0, 6763
Debugger [candidate] (6.087 ms) : 0, 6087
Remote Config [baseline] (645.254 µs) : 0, 645
Remote Config [candidate] (660.73 µs) : 0, 661
Telemetry [baseline] (8.496 ms) : 0, 8496
Telemetry [candidate] (8.594 ms) : 0, 8594
section iast
crashtracking [baseline] (1.461 ms) : 0, 1461
crashtracking [candidate] (1.476 ms) : 0, 1476
BytebuddyAgent [baseline] (855.844 ms) : 0, 855844
BytebuddyAgent [candidate] (864.307 ms) : 0, 864307
GlobalTracer [baseline] (247.796 ms) : 0, 247796
GlobalTracer [candidate] (248.228 ms) : 0, 248228
IAST [baseline] (28.982 ms) : 0, 28982
IAST [candidate] (29.582 ms) : 0, 29582
AppSec [baseline] (25.596 ms) : 0, 25596
AppSec [candidate] (24.319 ms) : 0, 24319
Debugger [baseline] (6.069 ms) : 0, 6069
Debugger [candidate] (6.042 ms) : 0, 6042
Remote Config [baseline] (607.727 µs) : 0, 608
Remote Config [candidate] (598.026 µs) : 0, 598
Telemetry [baseline] (8.217 ms) : 0, 8217
Telemetry [candidate] (8.001 ms) : 0, 8001
section profiling
crashtracking [baseline] (1.457 ms) : 0, 1457
crashtracking [candidate] (1.445 ms) : 0, 1445
BytebuddyAgent [baseline] (769.099 ms) : 0, 769099
BytebuddyAgent [candidate] (763.552 ms) : 0, 763552
GlobalTracer [baseline] (234.987 ms) : 0, 234987
GlobalTracer [candidate] (233.26 ms) : 0, 233260
AppSec [baseline] (30.808 ms) : 0, 30808
AppSec [candidate] (30.558 ms) : 0, 30558
Debugger [baseline] (9.018 ms) : 0, 9018
Debugger [candidate] (12.988 ms) : 0, 12988
Remote Config [baseline] (756.653 µs) : 0, 757
Remote Config [candidate] (734.017 µs) : 0, 734
Telemetry [baseline] (14.154 ms) : 0, 14154
Telemetry [candidate] (10.252 ms) : 0, 10252
ProfilingAgent [baseline] (108.929 ms) : 0, 108929
ProfilingAgent [candidate] (108.539 ms) : 0, 108539
Profiling [baseline] (109.592 ms) : 0, 109592
Profiling [candidate] (109.191 ms) : 0, 109191
LoadParameters
See matching parameters
SummaryFound 3 performance improvements and 1 performance regressions! Performance is the same for 8 metrics, 12 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.54.0-SNAPSHOT~b40c033805, baseline=1.54.0-SNAPSHOT~8f4fb427ca
dateFormat X
axisFormat %s
section baseline
no_agent (4.27 ms) : 4222, 4319
. : milestone, 4270,
iast (9.416 ms) : 9260, 9572
. : milestone, 9416,
iast_FULL (14.026 ms) : 13745, 14308
. : milestone, 14026,
iast_GLOBAL (10.985 ms) : 10777, 11194
. : milestone, 10985,
profiling (9.139 ms) : 8977, 9301
. : milestone, 9139,
tracing (8.212 ms) : 8093, 8331
. : milestone, 8212,
section candidate
no_agent (4.378 ms) : 4328, 4428
. : milestone, 4378,
iast (9.417 ms) : 9255, 9579
. : milestone, 9417,
iast_FULL (14.021 ms) : 13740, 14301
. : milestone, 14021,
iast_GLOBAL (10.896 ms) : 10701, 11092
. : milestone, 10896,
profiling (9.49 ms) : 9316, 9664
. : milestone, 9490,
tracing (7.658 ms) : 7541, 7774
. : milestone, 7658,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.54.0-SNAPSHOT~b40c033805, baseline=1.54.0-SNAPSHOT~8f4fb427ca
dateFormat X
axisFormat %s
section baseline
no_agent (37.164 ms) : 36864, 37465
. : milestone, 37164,
appsec (50.582 ms) : 50145, 51019
. : milestone, 50582,
code_origins (46.899 ms) : 46484, 47314
. : milestone, 46899,
iast (45.706 ms) : 45301, 46110
. : milestone, 45706,
profiling (47.068 ms) : 46599, 47536
. : milestone, 47068,
tracing (45.049 ms) : 44663, 45435
. : milestone, 45049,
section candidate
no_agent (37.557 ms) : 37253, 37860
. : milestone, 37557,
appsec (48.546 ms) : 48109, 48983
. : milestone, 48546,
code_origins (45.048 ms) : 44672, 45424
. : milestone, 45048,
iast (46.354 ms) : 45955, 46753
. : milestone, 46354,
profiling (48.939 ms) : 48448, 49430
. : milestone, 48939,
tracing (45.514 ms) : 45101, 45928
. : milestone, 45514,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 1 unstable metrics. Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.54.0-SNAPSHOT~b40c033805, baseline=1.54.0-SNAPSHOT~8f4fb427ca
dateFormat X
axisFormat %s
section baseline
no_agent (1.473 ms) : 1461, 1484
. : milestone, 1473,
appsec (3.678 ms) : 3461, 3896
. : milestone, 3678,
iast (2.2 ms) : 2137, 2263
. : milestone, 2200,
iast_GLOBAL (2.238 ms) : 2175, 2301
. : milestone, 2238,
profiling (2.039 ms) : 1988, 2089
. : milestone, 2039,
tracing (2.007 ms) : 1959, 2056
. : milestone, 2007,
section candidate
no_agent (1.474 ms) : 1462, 1485
. : milestone, 1474,
appsec (3.637 ms) : 3425, 3850
. : milestone, 3637,
iast (2.196 ms) : 2134, 2259
. : milestone, 2196,
iast_GLOBAL (2.229 ms) : 2166, 2291
. : milestone, 2229,
profiling (2.072 ms) : 2020, 2125
. : milestone, 2072,
tracing (2.024 ms) : 1975, 2073
. : milestone, 2024,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.54.0-SNAPSHOT~b40c033805, baseline=1.54.0-SNAPSHOT~8f4fb427ca
dateFormat X
axisFormat %s
section baseline
no_agent (14.956 s) : 14956000, 14956000
. : milestone, 14956000,
appsec (14.901 s) : 14901000, 14901000
. : milestone, 14901000,
iast (18.471 s) : 18471000, 18471000
. : milestone, 18471000,
iast_GLOBAL (17.964 s) : 17964000, 17964000
. : milestone, 17964000,
profiling (15.32 s) : 15320000, 15320000
. : milestone, 15320000,
tracing (15.219 s) : 15219000, 15219000
. : milestone, 15219000,
section candidate
no_agent (15.536 s) : 15536000, 15536000
. : milestone, 15536000,
appsec (14.821 s) : 14821000, 14821000
. : milestone, 14821000,
iast (18.229 s) : 18229000, 18229000
. : milestone, 18229000,
iast_GLOBAL (18.076 s) : 18076000, 18076000
. : milestone, 18076000,
profiling (15.446 s) : 15446000, 15446000
. : milestone, 15446000,
tracing (14.819 s) : 14819000, 14819000
. : milestone, 14819000,
|
PerfectSlayer
force-pushed
the
bbujon/config-module
branch
7 times, most recently
from
2cd71e0 to
50e5e23
Compare
PerfectSlayer
requested review from
daniel-mohedano,
bric3 and
manuel-alvarez-alvarez
and removed request for
a team
mhlidd
approved these changes
Sep 8, 2025
| import datadog.trace.civisibility.source.LinesResolver; | ||
| import datadog.trace.civisibility.source.SourcePathResolver; | ||
| import datadog.trace.util.Strings; | ||
| import datadog.trace.util.ConfigStrings; |
There was a problem hiding this comment.
Suggested change
| import datadog.trace.util.ConfigStrings; | |
| import static datadog.trace.util.ConfigStrings.propertyNameToSystemPropertyName; |
Nit: Replace all calls to the function below
| import datadog.trace.civisibility.git.CIProviderGitInfoBuilder | ||
| import datadog.trace.civisibility.git.tree.GitClient | ||
| import datadog.trace.util.Strings | ||
| import datadog.trace.util.ConfigStrings |
There was a problem hiding this comment.
Suggested change
| import datadog.trace.util.ConfigStrings | |
| import static datadog.trace.util.ConfigStrings.propertyNameToSystemPropertyName; |
Nit: replace all calls to the function below
dd-smoke-tests/gradle/src/test/groovy/datadog/smoketest/GradleDaemonSmokeTest.groovy
Outdated
Show resolved
Hide resolved
dd-smoke-tests/maven/src/test/groovy/datadog/smoketest/MavenSmokeTest.groovy
Outdated
Show resolved
Hide resolved
Comment on lines
+1232
to
+1249
| static { | ||
| // Bind telemetry collector to config module before initializing ConfigProvider | ||
| OtelEnvMetricCollectorProvider.register(OtelEnvMetricCollectorImpl.getInstance()); | ||
| } | ||
|
|
There was a problem hiding this comment.
For my understanding, is there a reason that you chose to register the instance here instead of when it is initialized in OtelEnvMetricCollectorImpl?
There was a problem hiding this comment.
instead of when it is initialized in OtelEnvMetricCollectorImpl
There is nobody to initialize it. The code that needs it can't have a dependency on it (to avoid circular dependency).
So the initialization should come from the :internal-api module.
bric3
approved these changes
Sep 11, 2025
bric3
reviewed
Sep 11, 2025
Comment on lines
+177
to
+180
| static { | ||
| // Bind telemetry collector to config module before initializing ConfigProvider | ||
| OtelEnvMetricCollectorProvider.register(OtelEnvMetricCollectorImpl.getInstance()); | ||
| } |
There was a problem hiding this comment.
question: For my understanding this registration code is called here and in Config class, why is that ?
There was a problem hiding this comment.
The two modules are tightly coupled together.
I inverted the dependency and needed somewhere for the parent module to register the service implementation.
I did it into the two main entry points as object lifecycle are screwed due to heavy use of static and singleton.
telemetry/src/main/java/datadog/telemetry/metric/OtelEnvMetricPeriodicAction.java
Show resolved
Hide resolved
PerfectSlayer
force-pushed
the
bbujon/config-module
branch
2 times, most recently
from
cc4709a to
89a6a02
Compare
Using `utils` for now as it has `:dd-trace-api` as dependency so can't be considered as a `:component`.
PerfectSlayer
force-pushed
the
bbujon/config-module
branch
from
89a6a02 to
b40c033
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What Does This Do
This PR refactor the config related code into its own module.
Motivation
This would improve readability, maintenance and ownership.
Additional Notes
Using
:utilsas destination for now as the new config module still has:dd-trace-apias dependency so can't be considered as a:component.The
ConfigandInstrumenterConfigare not good candidate to the refactoring as they are too tightly coupled to many other products.The
DynamicConfigcould still be a good candidate (coupled with sampling internal api).I used Dependency Inversion for the OTel metrics collector. It’s needed for the
OtelEnvironmentConfigSourcebut can’t be moved to the new config module as it’s heavily dependent on the telemetry code.We still need to :
:component:yamlinto this new module (only one class and one dependency, only used for stable config)Contributor Checklist
type:and (comp:orinst:) labels in addition to any usefull labelsclose,fixor any linking keywords when referencing an issue.Use
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]