-
Notifications
You must be signed in to change notification settings - Fork 312
Migrate Querying of Environment Variables to to ConfigHelper
#9620
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
base: mhlidd/config_inversion_base
Are you sure you want to change the base?
Migrate Querying of Environment Variables to to ConfigHelper
#9620
Conversation
Debugger benchmarksParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 8 metrics, 7 unstable metrics. See unchanged results
Request duration reports for reportsgantt
title reports - request duration [CI 0.99] : candidate=None, baseline=None
dateFormat X
axisFormat %s
section baseline
noprobe (326.351 µs) : 290, 362
. : milestone, 326,
basic (281.143 µs) : 275, 288
. : milestone, 281,
loop (8.951 ms) : 8946, 8956
. : milestone, 8951,
section candidate
noprobe (321.941 µs) : 278, 365
. : milestone, 322,
basic (280.261 µs) : 274, 286
. : milestone, 280,
loop (8.97 ms) : 8966, 8975
. : milestone, 8970,
|
🎯 Code Coverage 🔗 Commit SHA: 2401cfd | Docs | Was this helpful? Give us feedback! |
BenchmarksStartupParameters
See matching parameters
SummaryFound 5 performance improvements and 0 performance regressions! Performance is the same for 49 metrics, 5 unstable metrics.
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.54.0-SNAPSHOT~2401cfd900a, baseline=1.54.0-SNAPSHOT~6549d20548
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.024 s) : 0, 1023690
Total [baseline] (8.659 s) : 0, 8659458
Agent [candidate] (1.017 s) : 0, 1017375
Total [candidate] (8.641 s) : 0, 8640709
section iast
Agent [baseline] (1.147 s) : 0, 1147187
Total [baseline] (9.215 s) : 0, 9215287
Agent [candidate] (1.167 s) : 0, 1167287
Total [candidate] (9.321 s) : 0, 9320577
gantt
title insecure-bank - break down per module: candidate=1.54.0-SNAPSHOT~2401cfd900a, baseline=1.54.0-SNAPSHOT~6549d20548
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.475 ms) : 0, 1475
crashtracking [candidate] (1.469 ms) : 0, 1469
BytebuddyAgent [baseline] (697.889 ms) : 0, 697889
BytebuddyAgent [candidate] (694.14 ms) : 0, 694140
GlobalTracer [baseline] (243.379 ms) : 0, 243379
GlobalTracer [candidate] (248.151 ms) : 0, 248151
AppSec [baseline] (32.579 ms) : 0, 32579
AppSec [candidate] (30.741 ms) : 0, 30741
Debugger [baseline] (6.325 ms) : 0, 6325
Debugger [candidate] (6.337 ms) : 0, 6337
Remote Config [baseline] (678.091 µs) : 0, 678
Remote Config [candidate] (674.277 µs) : 0, 674
Telemetry [baseline] (9.298 ms) : 0, 9298
Telemetry [candidate] (14.573 ms) : 0, 14573
Flare Poller [baseline] (10.827 ms) : 0, 10827
section iast
crashtracking [baseline] (1.466 ms) : 0, 1466
crashtracking [candidate] (1.469 ms) : 0, 1469
BytebuddyAgent [baseline] (811.267 ms) : 0, 811267
BytebuddyAgent [candidate] (829.895 ms) : 0, 829895
GlobalTracer [baseline] (232.474 ms) : 0, 232474
GlobalTracer [candidate] (239.665 ms) : 0, 239665
AppSec [baseline] (35.235 ms) : 0, 35235
AppSec [candidate] (32.209 ms) : 0, 32209
Debugger [baseline] (6.063 ms) : 0, 6063
Debugger [candidate] (6.071 ms) : 0, 6071
Remote Config [baseline] (585.236 µs) : 0, 585
Remote Config [candidate] (584.262 µs) : 0, 584
Telemetry [baseline] (8.359 ms) : 0, 8359
Telemetry [candidate] (8.103 ms) : 0, 8103
Flare Poller [baseline] (4.201 ms) : 0, 4201
IAST [baseline] (26.225 ms) : 0, 26225
IAST [candidate] (26.257 ms) : 0, 26257
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.54.0-SNAPSHOT~2401cfd900a, baseline=1.54.0-SNAPSHOT~6549d20548
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.028 s) : 0, 1027846
Total [baseline] (10.788 s) : 0, 10787727
Agent [candidate] (1.017 s) : 0, 1016694
Total [candidate] (10.759 s) : 0, 10759472
section appsec
Agent [baseline] (1.195 s) : 0, 1195342
Total [baseline] (11.103 s) : 0, 11102977
Agent [candidate] (1.194 s) : 0, 1194433
Total [candidate] (10.906 s) : 0, 10905811
section iast
Agent [baseline] (1.152 s) : 0, 1152210
Total [baseline] (11.034 s) : 0, 11033532
Agent [candidate] (1.152 s) : 0, 1151522
Total [candidate] (11.11 s) : 0, 11110343
section profiling
Agent [baseline] (1.162 s) : 0, 1161768
Total [baseline] (11.006 s) : 0, 11005735
Agent [candidate] (1.161 s) : 0, 1161393
Total [candidate] (11.04 s) : 0, 11039823
gantt
title petclinic - break down per module: candidate=1.54.0-SNAPSHOT~2401cfd900a, baseline=1.54.0-SNAPSHOT~6549d20548
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.472 ms) : 0, 1472
crashtracking [candidate] (1.453 ms) : 0, 1453
BytebuddyAgent [baseline] (700.119 ms) : 0, 700119
BytebuddyAgent [candidate] (693.096 ms) : 0, 693096
GlobalTracer [baseline] (244.705 ms) : 0, 244705
GlobalTracer [candidate] (247.94 ms) : 0, 247940
AppSec [baseline] (32.86 ms) : 0, 32860
AppSec [candidate] (30.878 ms) : 0, 30878
Debugger [baseline] (6.443 ms) : 0, 6443
Debugger [candidate] (6.379 ms) : 0, 6379
Remote Config [baseline] (682.717 µs) : 0, 683
Remote Config [candidate] (680.815 µs) : 0, 681
Telemetry [baseline] (9.39 ms) : 0, 9390
Telemetry [candidate] (14.413 ms) : 0, 14413
Flare Poller [baseline] (10.882 ms) : 0, 10882
section appsec
crashtracking [baseline] (1.481 ms) : 0, 1481
crashtracking [candidate] (1.465 ms) : 0, 1465
BytebuddyAgent [baseline] (718.033 ms) : 0, 718033
BytebuddyAgent [candidate] (719.404 ms) : 0, 719404
GlobalTracer [baseline] (235.961 ms) : 0, 235961
GlobalTracer [candidate] (240.174 ms) : 0, 240174
AppSec [baseline] (173.075 ms) : 0, 173075
AppSec [candidate] (172.577 ms) : 0, 172577
Debugger [baseline] (6.082 ms) : 0, 6082
Debugger [candidate] (5.967 ms) : 0, 5967
Remote Config [baseline] (648.685 µs) : 0, 649
Remote Config [candidate] (641.691 µs) : 0, 642
Telemetry [baseline] (9.314 ms) : 0, 9314
Telemetry [candidate] (8.345 ms) : 0, 8345
Flare Poller [baseline] (4.781 ms) : 0, 4781
IAST [baseline] (24.778 ms) : 0, 24778
IAST [candidate] (24.702 ms) : 0, 24702
section iast
crashtracking [baseline] (1.466 ms) : 0, 1466
crashtracking [candidate] (1.452 ms) : 0, 1452
BytebuddyAgent [baseline] (815.266 ms) : 0, 815266
BytebuddyAgent [candidate] (816.579 ms) : 0, 816579
GlobalTracer [baseline] (233.113 ms) : 0, 233113
GlobalTracer [candidate] (238.173 ms) : 0, 238173
AppSec [baseline] (35.36 ms) : 0, 35360
AppSec [candidate] (33.658 ms) : 0, 33658
Debugger [baseline] (6.089 ms) : 0, 6089
Debugger [candidate] (6.047 ms) : 0, 6047
Remote Config [baseline] (590.95 µs) : 0, 591
Remote Config [candidate] (581.108 µs) : 0, 581
Telemetry [baseline] (8.498 ms) : 0, 8498
Telemetry [candidate] (8.025 ms) : 0, 8025
Flare Poller [baseline] (4.249 ms) : 0, 4249
IAST [baseline] (26.327 ms) : 0, 26327
IAST [candidate] (25.751 ms) : 0, 25751
section profiling
crashtracking [baseline] (1.425 ms) : 0, 1425
crashtracking [candidate] (1.418 ms) : 0, 1418
BytebuddyAgent [baseline] (721.841 ms) : 0, 721841
BytebuddyAgent [candidate] (721.745 ms) : 0, 721745
GlobalTracer [baseline] (218.646 ms) : 0, 218646
GlobalTracer [candidate] (222.404 ms) : 0, 222404
AppSec [baseline] (32.518 ms) : 0, 32518
AppSec [candidate] (31.291 ms) : 0, 31291
Debugger [baseline] (6.45 ms) : 0, 6450
Debugger [candidate] (6.566 ms) : 0, 6566
Remote Config [baseline] (747.952 µs) : 0, 748
Remote Config [candidate] (720.829 µs) : 0, 721
Telemetry [baseline] (16.665 ms) : 0, 16665
Telemetry [candidate] (16.138 ms) : 0, 16138
Flare Poller [baseline] (4.101 ms) : 0, 4101
ProfilingAgent [baseline] (107.01 ms) : 0, 107010
ProfilingAgent [candidate] (107.19 ms) : 0, 107190
Profiling [baseline] (107.661 ms) : 0, 107661
Profiling [candidate] (107.811 ms) : 0, 107811
LoadParameters
See matching parameters
SummaryFound 3 performance improvements and 0 performance regressions! Performance is the same for 9 metrics, 12 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.54.0-SNAPSHOT~2401cfd900a, baseline=1.54.0-SNAPSHOT~6549d20548
dateFormat X
axisFormat %s
section baseline
no_agent (4.415 ms) : 4359, 4471
. : milestone, 4415,
iast (10.021 ms) : 9852, 10190
. : milestone, 10021,
iast_FULL (14.394 ms) : 14102, 14685
. : milestone, 14394,
iast_GLOBAL (11.08 ms) : 10878, 11281
. : milestone, 11080,
profiling (9.341 ms) : 9195, 9487
. : milestone, 9341,
tracing (7.738 ms) : 7616, 7860
. : milestone, 7738,
section candidate
no_agent (4.386 ms) : 4334, 4439
. : milestone, 4386,
iast (9.815 ms) : 9654, 9976
. : milestone, 9815,
iast_FULL (14.54 ms) : 14250, 14829
. : milestone, 14540,
iast_GLOBAL (11.071 ms) : 10872, 11270
. : milestone, 11071,
profiling (9.062 ms) : 8921, 9204
. : milestone, 9062,
tracing (7.493 ms) : 7383, 7603
. : milestone, 7493,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.54.0-SNAPSHOT~2401cfd900a, baseline=1.54.0-SNAPSHOT~6549d20548
dateFormat X
axisFormat %s
section baseline
no_agent (37.633 ms) : 37329, 37936
. : milestone, 37633,
appsec (50.347 ms) : 49896, 50798
. : milestone, 50347,
code_origins (43.372 ms) : 43009, 43735
. : milestone, 43372,
iast (43.781 ms) : 43417, 44145
. : milestone, 43781,
profiling (50.158 ms) : 49683, 50633
. : milestone, 50158,
tracing (44.087 ms) : 43717, 44457
. : milestone, 44087,
section candidate
no_agent (36.152 ms) : 35860, 36444
. : milestone, 36152,
appsec (46.941 ms) : 46533, 47348
. : milestone, 46941,
code_origins (43.955 ms) : 43583, 44327
. : milestone, 43955,
iast (43.8 ms) : 43409, 44190
. : milestone, 43800,
profiling (47.502 ms) : 47009, 47995
. : milestone, 47502,
tracing (44.266 ms) : 43873, 44660
. : milestone, 44266,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 10 metrics, 2 unstable metrics. Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.54.0-SNAPSHOT~2401cfd900a, baseline=1.54.0-SNAPSHOT~6549d20548
dateFormat X
axisFormat %s
section baseline
no_agent (14.863 s) : 14863000, 14863000
. : milestone, 14863000,
appsec (15.251 s) : 15251000, 15251000
. : milestone, 15251000,
iast (18.453 s) : 18453000, 18453000
. : milestone, 18453000,
iast_GLOBAL (17.755 s) : 17755000, 17755000
. : milestone, 17755000,
profiling (15.462 s) : 15462000, 15462000
. : milestone, 15462000,
tracing (15.083 s) : 15083000, 15083000
. : milestone, 15083000,
section candidate
no_agent (15.326 s) : 15326000, 15326000
. : milestone, 15326000,
appsec (15.4 s) : 15400000, 15400000
. : milestone, 15400000,
iast (18.612 s) : 18612000, 18612000
. : milestone, 18612000,
iast_GLOBAL (18.165 s) : 18165000, 18165000
. : milestone, 18165000,
profiling (14.902 s) : 14902000, 14902000
. : milestone, 14902000,
tracing (15.249 s) : 15249000, 15249000
. : milestone, 15249000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.54.0-SNAPSHOT~2401cfd900a, baseline=1.54.0-SNAPSHOT~6549d20548
dateFormat X
axisFormat %s
section baseline
no_agent (1.475 ms) : 1464, 1486
. : milestone, 1475,
appsec (3.727 ms) : 3508, 3947
. : milestone, 3727,
iast (2.21 ms) : 2146, 2274
. : milestone, 2210,
iast_GLOBAL (2.25 ms) : 2185, 2314
. : milestone, 2250,
profiling (2.066 ms) : 2014, 2117
. : milestone, 2066,
tracing (2.031 ms) : 1982, 2081
. : milestone, 2031,
section candidate
no_agent (1.477 ms) : 1465, 1488
. : milestone, 1477,
appsec (3.719 ms) : 3501, 3937
. : milestone, 3719,
iast (2.194 ms) : 2132, 2257
. : milestone, 2194,
iast_GLOBAL (2.238 ms) : 2175, 2301
. : milestone, 2238,
profiling (2.492 ms) : 2322, 2663
. : milestone, 2492,
tracing (2.022 ms) : 1973, 2071
. : milestone, 2022,
|
ConfigHelper
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So far it looks good. I left some comments along the review.
🎯 suggestion: The more I read usage of ConfigHelper
, the more I wonder if you should not introduce static helper methods as syntactic sugar like: ConfigHelper.env(String key) : String
and ConfigHelper.env() : Map<String, String>
.
public static String env(String key) {
return get().getEnvironmentVariable(key);
}
This would replace calls like:
ConfigHelper.get().getEnvironmentVariable("TMPDIR")
and .map(ConfigHelper.get()::getEnvironmentVariable)
to
ConfigHelper.env("TMPDIR")
and map(ConfigHelper::env)
WDYT?
components/environment/src/main/java/datadog/environment/EnvironmentVariables.java
Show resolved
Hide resolved
dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Constants.java
Outdated
Show resolved
Hide resolved
...entation-testing/src/main/java/datadog/trace/agent/test/BootstrapClasspathSetupListener.java
Outdated
Show resolved
Hide resolved
...-utils/src/main/java/datadog/trace/api/telemetry/ConfigInversionMetricCollectorProvider.java
Outdated
Show resolved
Hide resolved
I think you mean I should introduce it 😄. But yes I agree this makes sense and is a lot more clear. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you mean I should introduce it 😄. But yes I agree this makes sense and is a lot more clear.
Yes, sorry, it was a poor translation from my French 😓
👏 praise: Thanks for the follow up changes! Everything looks great now ✨
What Does This Do
This PR builds off of the Config Inversion implementation to migrate all querying of environment variables to utilize
ConfigHelper
instead of directly invoking the Environment component. Linter rules to restrict the direct invocation of the Environment component or undocumented String constants for environment variables (e.g.DD_ENV_VAR
) are now enforced.Classes that are excluded from this migration are bootstrap-related classes since there would be issues with the Logger being initialized too early. Furthermore, there are few configurations used in bootstrap and they are rarely modified.
Additionally, this PR introduces a NoOp implementation of the
ConfigInversionMetricCollector
to handle exceptions during build-time where theConfigHelper
is invoked but actual implementation has not been registered.This PR also adds the environment variables that have been introduced since the initial commit of the
metadata/supported-configurations.json
file.Motivation
Additional Notes
Contributor Checklist
type:
and (comp:
orinst:
) labels in addition to any useful labelsclose
,fix
or any linking keywords when referencing an issue.Use
solves
instead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]