Register config-inversion for InstrumenterConfig
#9953
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
InstrumenterConfig runs before Config and checks settings that control instrumentation. We skip the registration when doing native-image builds because we deliberately turn off non-critical features like telemetry for native-image. This change also simplifies ConfigInversionMetricCollectorProvider.get() for performance reasons, while moving debug logs to NoOpConfigInversionMetricCollector when the telemetry based implementation is not yet registered.
mhlidd
approved these changes
Nov 13, 2025
Contributor
mhlidd
left a comment
mhlidd
left a comment
There was a problem hiding this comment.
Few questions. Thanks for the changes!
| static { | ||
| // skip registration when building native-images as telemetry is not available | ||
| if (!Platform.isNativeImageBuilder()) { | ||
| ConfigInversionMetricCollectorProvider.register( |
Contributor
There was a problem hiding this comment.
❓ If we register the Collector in InstrumenterConfig, which gets initialized in Config.java, should we remove the registration that already exists in Config.java(ref)?
Contributor
There was a problem hiding this comment.
Also, out of curiosity, what is the added benefit of registering in InstrumenterConfig instead of just Config?
Contributor
Author
There was a problem hiding this comment.
See #9953 (comment) - without this registration config-inversion is not applied to InstrumenterConfig, which means new integrations don't cause any CI alerts about documenting them - for example the 'hikari' and 'dbcp2' integrations.
(This is because an instance of InstrumenterConfig is created before Config's static initializer runs.)
In the future as Config is broken apart there will hopefully be a better place to do this registration, but for now we have to do this in both InstrumenterConfig and Config to cover a couple of different user-cases.
Contributor
Author
There was a problem hiding this comment.
Also I intentionally left the existing registration in Config because a) it doesn't matter which runs first because the implementation is a singleton and b) if someone refactors this in the future then I think it's clearer to have both registrations in the code
Eventually it would be nice have a clear place to register this kind of thing once - but right now there is this intentional split-brain situation between InstrumenterConfig (controls instrumentation, baked into native images and not re-evaluated) and Config (controls non-instrumentation, re-evaluated in native imeges)
| @Override | ||
| public void setUndocumentedEnvVarMetric(String configName) { | ||
| // NOOP - do nothing | ||
| log.debug("Environment variable {} is undocumented", configName); |
Contributor
There was a problem hiding this comment.
What is the circumstance where we would expect to have a NOOP implementation used? I would expect none, and if that's the case, should we log an error message saying that the actual implementation should be registered? 🤔
Contributor
Author
There was a problem hiding this comment.
So this PR came about because I was looking at recent debug logs and noticed a bunch of these messages:
[dd.trace 2025-11-13 16:14:37:076 +0000] [main] DEBUG datadog.trace.api.telemetry.ConfigInversionMetricCollectorProvider - ConfigInversionMetricCollector has not been registered. Defaulting to NoOp implementation.
[dd.trace 2025-11-13 16:14:37:076 +0000] [main] DEBUG datadog.trace.api.telemetry.ConfigInversionMetricCollectorProvider - ConfigInversionMetricCollector has not been registered. Defaulting to NoOp implementation.
[dd.trace 2025-11-13 16:14:37:076 +0000] [main] DEBUG datadog.trace.api.telemetry.ConfigInversionMetricCollectorProvider - ConfigInversionMetricCollector has not been registered. Defaulting to NoOp implementation.
... 27 more duplicate log messages ...
I tracked this back to the missing initialization in InstrumenterConfig - but while fixing that I realized that if this happened again (i.e. the "no-op" implementation being used accidentally) then it would be better to have a log including the undocumented config key - as that would point to where the config was being looked up.
So I would argue we still need logging in the "no-op" implementation to help in the future, even if ideally that will never be called. I chose debug as that is less likely to cause disruption and is best suited for this kind of triaging. (I usually reserve use of warn/error for things that users need to know about and can fix themselves.)
Also there may well be situations (build/tooling) where we don't have telemetry, but this debug logging would be useful - especially as we often have debug on in CI.
Contributor
There was a problem hiding this comment.
Makes sense, thanks for the info!
|
🎯 Code Coverage 🔗 Commit SHA: 0e726e3 | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 57 metrics, 8 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.56.0-SNAPSHOT~0e726e3f8c, baseline=1.56.0-SNAPSHOT~640a4bd02b
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.096 s) : 0, 1096079
Total [baseline] (8.857 s) : 0, 8857497
Agent [candidate] (1.1 s) : 0, 1100230
Total [candidate] (8.83 s) : 0, 8829675
section iast
Agent [baseline] (1.243 s) : 0, 1242930
Total [baseline] (9.564 s) : 0, 9563614
Agent [candidate] (1.243 s) : 0, 1242940
Total [candidate] (9.538 s) : 0, 9538491
gantt
title insecure-bank - break down per module: candidate=1.56.0-SNAPSHOT~0e726e3f8c, baseline=1.56.0-SNAPSHOT~640a4bd02b
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.45 ms) : 0, 1450
crashtracking [candidate] (1.452 ms) : 0, 1452
BytebuddyAgent [baseline] (704.188 ms) : 0, 704188
BytebuddyAgent [candidate] (706.644 ms) : 0, 706644
GlobalTracer [baseline] (247.695 ms) : 0, 247695
GlobalTracer [candidate] (248.822 ms) : 0, 248822
AppSec [baseline] (32.245 ms) : 0, 32245
AppSec [candidate] (32.438 ms) : 0, 32438
Debugger [baseline] (63.33 ms) : 0, 63330
Debugger [candidate] (63.671 ms) : 0, 63671
Remote Config [baseline] (639.696 µs) : 0, 640
Remote Config [candidate] (639.312 µs) : 0, 639
Telemetry [baseline] (8.22 ms) : 0, 8220
Telemetry [candidate] (8.268 ms) : 0, 8268
Flare Poller [baseline] (3.706 ms) : 0, 3706
Flare Poller [candidate] (3.666 ms) : 0, 3666
section iast
crashtracking [baseline] (1.466 ms) : 0, 1466
crashtracking [candidate] (1.474 ms) : 0, 1474
BytebuddyAgent [baseline] (834.798 ms) : 0, 834798
BytebuddyAgent [candidate] (833.632 ms) : 0, 833632
GlobalTracer [baseline] (238.585 ms) : 0, 238585
GlobalTracer [candidate] (238.672 ms) : 0, 238672
AppSec [baseline] (33.929 ms) : 0, 33929
AppSec [candidate] (34.4 ms) : 0, 34400
Debugger [baseline] (60.155 ms) : 0, 60155
Debugger [candidate] (60.603 ms) : 0, 60603
Remote Config [baseline] (543.641 µs) : 0, 544
Remote Config [candidate] (541.846 µs) : 0, 542
Telemetry [baseline] (7.599 ms) : 0, 7599
Telemetry [candidate] (7.631 ms) : 0, 7631
Flare Poller [baseline] (3.396 ms) : 0, 3396
Flare Poller [candidate] (3.47 ms) : 0, 3470
IAST [baseline] (27.52 ms) : 0, 27520
IAST [candidate] (27.638 ms) : 0, 27638
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.56.0-SNAPSHOT~0e726e3f8c, baseline=1.56.0-SNAPSHOT~640a4bd02b
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.106 s) : 0, 1106392
Total [baseline] (10.759 s) : 0, 10759497
Agent [candidate] (1.106 s) : 0, 1106179
Total [candidate] (10.793 s) : 0, 10792518
section appsec
Agent [baseline] (1.28 s) : 0, 1279838
Total [baseline] (11.094 s) : 0, 11093913
Agent [candidate] (1.276 s) : 0, 1275992
Total [candidate] (11.084 s) : 0, 11083584
section iast
Agent [baseline] (1.255 s) : 0, 1254988
Total [baseline] (11.24 s) : 0, 11239935
Agent [candidate] (1.241 s) : 0, 1241209
Total [candidate] (11.287 s) : 0, 11287239
section profiling
Agent [baseline] (1.236 s) : 0, 1235864
Total [baseline] (11.121 s) : 0, 11120828
Agent [candidate] (1.242 s) : 0, 1242390
Total [candidate] (11.102 s) : 0, 11102296
gantt
title petclinic - break down per module: candidate=1.56.0-SNAPSHOT~0e726e3f8c, baseline=1.56.0-SNAPSHOT~640a4bd02b
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.462 ms) : 0, 1462
crashtracking [candidate] (1.463 ms) : 0, 1463
BytebuddyAgent [baseline] (712.082 ms) : 0, 712082
BytebuddyAgent [candidate] (710.825 ms) : 0, 710825
GlobalTracer [baseline] (249.092 ms) : 0, 249092
GlobalTracer [candidate] (249.793 ms) : 0, 249793
AppSec [baseline] (32.403 ms) : 0, 32403
AppSec [candidate] (32.451 ms) : 0, 32451
Debugger [baseline] (64.115 ms) : 0, 64115
Debugger [candidate] (64.249 ms) : 0, 64249
Remote Config [baseline] (635.918 µs) : 0, 636
Remote Config [candidate] (644.559 µs) : 0, 645
Telemetry [baseline] (8.122 ms) : 0, 8122
Telemetry [candidate] (8.243 ms) : 0, 8243
Flare Poller [baseline] (3.676 ms) : 0, 3676
Flare Poller [candidate] (3.725 ms) : 0, 3725
section appsec
crashtracking [baseline] (1.462 ms) : 0, 1462
crashtracking [candidate] (1.449 ms) : 0, 1449
BytebuddyAgent [baseline] (729.819 ms) : 0, 729819
BytebuddyAgent [candidate] (727.917 ms) : 0, 727917
GlobalTracer [baseline] (241.155 ms) : 0, 241155
GlobalTracer [candidate] (239.706 ms) : 0, 239706
AppSec [baseline] (174.008 ms) : 0, 174008
AppSec [candidate] (174.204 ms) : 0, 174204
Debugger [baseline] (60.846 ms) : 0, 60846
Debugger [candidate] (60.345 ms) : 0, 60345
Remote Config [baseline] (657.673 µs) : 0, 658
Remote Config [candidate] (693.592 µs) : 0, 694
Telemetry [baseline] (8.35 ms) : 0, 8350
Telemetry [candidate] (8.32 ms) : 0, 8320
Flare Poller [baseline] (3.973 ms) : 0, 3973
Flare Poller [candidate] (3.936 ms) : 0, 3936
IAST [baseline] (24.748 ms) : 0, 24748
IAST [candidate] (24.765 ms) : 0, 24765
section iast
crashtracking [baseline] (1.492 ms) : 0, 1492
crashtracking [candidate] (1.454 ms) : 0, 1454
BytebuddyAgent [baseline] (843.61 ms) : 0, 843610
BytebuddyAgent [candidate] (831.162 ms) : 0, 831162
GlobalTracer [baseline] (240.161 ms) : 0, 240161
GlobalTracer [candidate] (239.171 ms) : 0, 239171
AppSec [baseline] (29.653 ms) : 0, 29653
AppSec [candidate] (30.737 ms) : 0, 30737
Debugger [baseline] (61.044 ms) : 0, 61044
Debugger [candidate] (61.396 ms) : 0, 61396
Remote Config [baseline] (561.565 µs) : 0, 562
Remote Config [candidate] (573.324 µs) : 0, 573
Telemetry [baseline] (7.714 ms) : 0, 7714
Telemetry [candidate] (7.738 ms) : 0, 7738
Flare Poller [baseline] (3.51 ms) : 0, 3510
Flare Poller [candidate] (3.477 ms) : 0, 3477
IAST [baseline] (32.235 ms) : 0, 32235
IAST [candidate] (30.813 ms) : 0, 30813
section profiling
crashtracking [baseline] (1.439 ms) : 0, 1439
crashtracking [candidate] (1.44 ms) : 0, 1440
BytebuddyAgent [baseline] (734.409 ms) : 0, 734409
BytebuddyAgent [candidate] (738.67 ms) : 0, 738670
GlobalTracer [baseline] (223.277 ms) : 0, 223277
GlobalTracer [candidate] (224.556 ms) : 0, 224556
AppSec [baseline] (32.634 ms) : 0, 32634
AppSec [candidate] (32.694 ms) : 0, 32694
Debugger [baseline] (63.434 ms) : 0, 63434
Debugger [candidate] (63.992 ms) : 0, 63992
Remote Config [baseline] (641.337 µs) : 0, 641
Remote Config [candidate] (664.889 µs) : 0, 665
Telemetry [baseline] (8.024 ms) : 0, 8024
Telemetry [candidate] (8.144 ms) : 0, 8144
Flare Poller [baseline] (3.808 ms) : 0, 3808
Flare Poller [candidate] (3.868 ms) : 0, 3868
ProfilingAgent [baseline] (98.25 ms) : 0, 98250
ProfilingAgent [candidate] (97.857 ms) : 0, 97857
Profiling [baseline] (98.842 ms) : 0, 98842
Profiling [candidate] (98.449 ms) : 0, 98449
LoadParameters
See matching parameters
SummaryFound 3 performance improvements and 0 performance regressions! Performance is the same for 17 metrics, 16 unstable metrics.
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.56.0-SNAPSHOT~0e726e3f8c, baseline=1.56.0-SNAPSHOT~29d5c2d641
dateFormat X
axisFormat %s
section baseline
no_agent (18.376 ms) : 18187, 18565
. : milestone, 18376,
appsec (18.592 ms) : 18401, 18782
. : milestone, 18592,
code_origins (17.788 ms) : 17612, 17964
. : milestone, 17788,
iast (17.612 ms) : 17438, 17787
. : milestone, 17612,
profiling (19.711 ms) : 19512, 19910
. : milestone, 19711,
tracing (17.405 ms) : 17234, 17575
. : milestone, 17405,
section candidate
no_agent (19.132 ms) : 18937, 19327
. : milestone, 19132,
appsec (18.384 ms) : 18198, 18571
. : milestone, 18384,
code_origins (17.608 ms) : 17432, 17784
. : milestone, 17608,
iast (17.474 ms) : 17299, 17649
. : milestone, 17474,
profiling (18.535 ms) : 18346, 18723
. : milestone, 18535,
tracing (17.6 ms) : 17427, 17774
. : milestone, 17600,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.56.0-SNAPSHOT~0e726e3f8c, baseline=1.56.0-SNAPSHOT~29d5c2d641
dateFormat X
axisFormat %s
section baseline
no_agent (1.195 ms) : 1183, 1207
. : milestone, 1195,
iast (3.279 ms) : 3238, 3319
. : milestone, 3279,
iast_FULL (5.985 ms) : 5907, 6062
. : milestone, 5985,
iast_GLOBAL (3.675 ms) : 3626, 3725
. : milestone, 3675,
profiling (2.16 ms) : 2140, 2180
. : milestone, 2160,
tracing (1.829 ms) : 1814, 1845
. : milestone, 1829,
section candidate
no_agent (1.175 ms) : 1164, 1187
. : milestone, 1175,
iast (3.361 ms) : 3298, 3424
. : milestone, 3361,
iast_FULL (5.667 ms) : 5611, 5723
. : milestone, 5667,
iast_GLOBAL (3.622 ms) : 3560, 3683
. : milestone, 3622,
profiling (2.064 ms) : 2045, 2082
. : milestone, 2064,
tracing (1.815 ms) : 1799, 1830
. : milestone, 1815,
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 tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.56.0-SNAPSHOT~0e726e3f8c, baseline=1.56.0-SNAPSHOT~640a4bd02b
dateFormat X
axisFormat %s
section baseline
no_agent (1.474 ms) : 1463, 1486
. : milestone, 1474,
appsec (3.676 ms) : 3461, 3890
. : milestone, 3676,
iast (2.225 ms) : 2161, 2288
. : milestone, 2225,
iast_GLOBAL (2.267 ms) : 2203, 2331
. : milestone, 2267,
profiling (2.505 ms) : 2335, 2674
. : milestone, 2505,
tracing (2.026 ms) : 1976, 2075
. : milestone, 2026,
section candidate
no_agent (1.477 ms) : 1466, 1489
. : milestone, 1477,
appsec (3.706 ms) : 3487, 3925
. : milestone, 3706,
iast (2.211 ms) : 2148, 2275
. : milestone, 2211,
iast_GLOBAL (2.253 ms) : 2189, 2317
. : milestone, 2253,
profiling (2.062 ms) : 2011, 2114
. : milestone, 2062,
tracing (2.036 ms) : 1986, 2085
. : milestone, 2036,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.56.0-SNAPSHOT~0e726e3f8c, baseline=1.56.0-SNAPSHOT~640a4bd02b
dateFormat X
axisFormat %s
section baseline
no_agent (15.204 s) : 15204000, 15204000
. : milestone, 15204000,
appsec (14.675 s) : 14675000, 14675000
. : milestone, 14675000,
iast (18.655 s) : 18655000, 18655000
. : milestone, 18655000,
iast_GLOBAL (18.205 s) : 18205000, 18205000
. : milestone, 18205000,
profiling (15.139 s) : 15139000, 15139000
. : milestone, 15139000,
tracing (14.796 s) : 14796000, 14796000
. : milestone, 14796000,
section candidate
no_agent (15.409 s) : 15409000, 15409000
. : milestone, 15409000,
appsec (14.862 s) : 14862000, 14862000
. : milestone, 14862000,
iast (18.437 s) : 18437000, 18437000
. : milestone, 18437000,
iast_GLOBAL (17.987 s) : 17987000, 17987000
. : milestone, 17987000,
profiling (14.798 s) : 14798000, 14798000
. : milestone, 14798000,
tracing (14.793 s) : 14793000, 14793000
. : milestone, 14793000,
|
mcculls
deleted the
mcculls/register-cfg-inversion-for-instrumenter-cfg
branch
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.
... and update
metadata/supported-configurations.jsonwith a couple of missing integrations.Contributor Checklist
type:and (comp:orinst:) labels in addition to any useful labelsclose,fixor any linking keywords when referencing an issue.Use
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]