Add a feature flag to disable extra calls to the DB in JDBC instrumentation #9774
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
This comment has been minimized.
This comment has been minimized.
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 54 metrics, 11 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.55.0-SNAPSHOT~fde17658d3, baseline=1.57.0-SNAPSHOT~4d1a38c992
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.061 s) : 0, 1060616
Total [baseline] (10.925 s) : 0, 10924826
Agent [candidate] (1.055 s) : 0, 1055049
Total [candidate] (10.883 s) : 0, 10882984
section appsec
Agent [baseline] (1.226 s) : 0, 1225958
Total [baseline] (10.889 s) : 0, 10889327
Agent [candidate] (1.232 s) : 0, 1231922
Total [candidate] (10.887 s) : 0, 10886863
section iast
Agent [baseline] (1.193 s) : 0, 1193384
Total [baseline] (11.203 s) : 0, 11203154
Agent [candidate] (1.202 s) : 0, 1202233
Total [candidate] (11.185 s) : 0, 11184872
section profiling
Agent [baseline] (1.201 s) : 0, 1201442
Total [baseline] (10.908 s) : 0, 10907909
Agent [candidate] (1.206 s) : 0, 1205629
Total [candidate] (10.943 s) : 0, 10942868
gantt
title petclinic - break down per module: candidate=1.55.0-SNAPSHOT~fde17658d3, baseline=1.57.0-SNAPSHOT~4d1a38c992
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.501 ms) : 0, 1501
crashtracking [candidate] (1.481 ms) : 0, 1481
BytebuddyAgent [baseline] (712.432 ms) : 0, 712432
BytebuddyAgent [candidate] (708.548 ms) : 0, 708548
GlobalTracer [baseline] (251.475 ms) : 0, 251475
GlobalTracer [candidate] (250.562 ms) : 0, 250562
AppSec [baseline] (32.292 ms) : 0, 32292
AppSec [candidate] (32.099 ms) : 0, 32099
Debugger [baseline] (6.499 ms) : 0, 6499
Debugger [candidate] (6.42 ms) : 0, 6420
Remote Config [baseline] (688.025 µs) : 0, 688
Remote Config [candidate] (668.666 µs) : 0, 669
Telemetry [baseline] (15.014 ms) : 0, 15014
Telemetry [candidate] (14.743 ms) : 0, 14743
Flare Poller [baseline] (5.662 ms) : 0, 5662
Flare Poller [candidate] (5.67 ms) : 0, 5670
section appsec
crashtracking [baseline] (1.489 ms) : 0, 1489
crashtracking [candidate] (1.48 ms) : 0, 1480
BytebuddyAgent [baseline] (730.077 ms) : 0, 730077
BytebuddyAgent [candidate] (733.132 ms) : 0, 733132
GlobalTracer [baseline] (241.178 ms) : 0, 241178
GlobalTracer [candidate] (242.853 ms) : 0, 242853
AppSec [baseline] (174.505 ms) : 0, 174505
AppSec [candidate] (175.383 ms) : 0, 175383
Debugger [baseline] (6.245 ms) : 0, 6245
Debugger [candidate] (6.346 ms) : 0, 6346
Remote Config [baseline] (703.831 µs) : 0, 704
Remote Config [candidate] (688.167 µs) : 0, 688
Telemetry [baseline] (8.07 ms) : 0, 8070
Telemetry [candidate] (8.347 ms) : 0, 8347
Flare Poller [baseline] (3.961 ms) : 0, 3961
Flare Poller [candidate] (4.012 ms) : 0, 4012
IAST [baseline] (24.789 ms) : 0, 24789
IAST [candidate] (24.602 ms) : 0, 24602
section iast
crashtracking [baseline] (1.494 ms) : 0, 1494
crashtracking [candidate] (1.486 ms) : 0, 1486
BytebuddyAgent [baseline] (831.598 ms) : 0, 831598
BytebuddyAgent [candidate] (839.162 ms) : 0, 839162
GlobalTracer [baseline] (238.61 ms) : 0, 238610
GlobalTracer [candidate] (239.052 ms) : 0, 239052
AppSec [baseline] (29.341 ms) : 0, 29341
AppSec [candidate] (31.573 ms) : 0, 31573
Debugger [baseline] (5.968 ms) : 0, 5968
Debugger [candidate] (6.117 ms) : 0, 6117
Remote Config [baseline] (674.716 µs) : 0, 675
Remote Config [candidate] (613.975 µs) : 0, 614
Telemetry [baseline] (7.972 ms) : 0, 7972
Telemetry [candidate] (8.034 ms) : 0, 8034
Flare Poller [baseline] (10.728 ms) : 0, 10728
Flare Poller [candidate] (10.819 ms) : 0, 10819
IAST [baseline] (32.148 ms) : 0, 32148
IAST [candidate] (30.335 ms) : 0, 30335
section profiling
crashtracking [baseline] (1.442 ms) : 0, 1442
crashtracking [candidate] (1.473 ms) : 0, 1473
BytebuddyAgent [baseline] (735.605 ms) : 0, 735605
BytebuddyAgent [candidate] (739.54 ms) : 0, 739540
GlobalTracer [baseline] (223.059 ms) : 0, 223059
GlobalTracer [candidate] (222.749 ms) : 0, 222749
AppSec [baseline] (32.143 ms) : 0, 32143
AppSec [candidate] (32.166 ms) : 0, 32166
Debugger [baseline] (7.532 ms) : 0, 7532
Debugger [candidate] (6.922 ms) : 0, 6922
Remote Config [baseline] (685.812 µs) : 0, 686
Remote Config [candidate] (2.149 ms) : 0, 2149
Telemetry [baseline] (15.477 ms) : 0, 15477
Telemetry [candidate] (15.034 ms) : 0, 15034
Flare Poller [baseline] (4.162 ms) : 0, 4162
Flare Poller [candidate] (4.206 ms) : 0, 4206
ProfilingAgent [baseline] (111.987 ms) : 0, 111987
ProfilingAgent [candidate] (111.574 ms) : 0, 111574
Profiling [baseline] (112.639 ms) : 0, 112639
Profiling [candidate] (112.222 ms) : 0, 112222
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.55.0-SNAPSHOT~fde17658d3, baseline=1.57.0-SNAPSHOT~4d1a38c992
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.058 s) : 0, 1058093
Total [baseline] (8.692 s) : 0, 8691758
Agent [candidate] (1.054 s) : 0, 1053525
Total [candidate] (8.725 s) : 0, 8724602
section iast
Agent [baseline] (1.201 s) : 0, 1200903
Total [baseline] (9.352 s) : 0, 9352496
Agent [candidate] (1.192 s) : 0, 1192091
Total [candidate] (9.336 s) : 0, 9335606
gantt
title insecure-bank - break down per module: candidate=1.55.0-SNAPSHOT~fde17658d3, baseline=1.57.0-SNAPSHOT~4d1a38c992
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.493 ms) : 0, 1493
crashtracking [candidate] (1.494 ms) : 0, 1494
BytebuddyAgent [baseline] (711.144 ms) : 0, 711144
BytebuddyAgent [candidate] (707.918 ms) : 0, 707918
GlobalTracer [baseline] (250.405 ms) : 0, 250405
GlobalTracer [candidate] (249.656 ms) : 0, 249656
AppSec [baseline] (32.241 ms) : 0, 32241
AppSec [candidate] (31.993 ms) : 0, 31993
Debugger [baseline] (6.433 ms) : 0, 6433
Debugger [candidate] (6.36 ms) : 0, 6360
Remote Config [baseline] (682.795 µs) : 0, 683
Remote Config [candidate] (685.154 µs) : 0, 685
Telemetry [baseline] (16.538 ms) : 0, 16538
Telemetry [candidate] (16.401 ms) : 0, 16401
Flare Poller [baseline] (4.138 ms) : 0, 4138
Flare Poller [candidate] (4.116 ms) : 0, 4116
section iast
crashtracking [baseline] (1.49 ms) : 0, 1490
crashtracking [candidate] (1.487 ms) : 0, 1487
BytebuddyAgent [baseline] (837.985 ms) : 0, 837985
BytebuddyAgent [candidate] (830.915 ms) : 0, 830915
GlobalTracer [baseline] (239.395 ms) : 0, 239395
GlobalTracer [candidate] (237.875 ms) : 0, 237875
AppSec [baseline] (30.524 ms) : 0, 30524
AppSec [candidate] (31.376 ms) : 0, 31376
Debugger [baseline] (6.057 ms) : 0, 6057
Debugger [candidate] (6.048 ms) : 0, 6048
Remote Config [baseline] (606.344 µs) : 0, 606
Remote Config [candidate] (603.101 µs) : 0, 603
Telemetry [baseline] (7.999 ms) : 0, 7999
Telemetry [candidate] (7.941 ms) : 0, 7941
Flare Poller [baseline] (10.77 ms) : 0, 10770
Flare Poller [candidate] (10.921 ms) : 0, 10921
IAST [baseline] (30.975 ms) : 0, 30975
IAST [candidate] (30.043 ms) : 0, 30043
LoadParameters
See matching parameters
SummaryFound 2 performance improvements and 0 performance regressions! Performance is the same for 16 metrics, 18 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.55.0-SNAPSHOT~fde17658d3, baseline=1.57.0-SNAPSHOT~4d1a38c992
dateFormat X
axisFormat %s
section baseline
no_agent (1.206 ms) : 1194, 1217
. : milestone, 1206,
iast (3.209 ms) : 3169, 3250
. : milestone, 3209,
iast_FULL (5.91 ms) : 5850, 5969
. : milestone, 5910,
iast_GLOBAL (3.697 ms) : 3635, 3759
. : milestone, 3697,
profiling (2.129 ms) : 2110, 2149
. : milestone, 2129,
tracing (1.836 ms) : 1820, 1852
. : milestone, 1836,
section candidate
no_agent (1.214 ms) : 1202, 1226
. : milestone, 1214,
iast (3.33 ms) : 3280, 3380
. : milestone, 3330,
iast_FULL (5.702 ms) : 5646, 5758
. : milestone, 5702,
iast_GLOBAL (3.538 ms) : 3486, 3590
. : milestone, 3538,
profiling (2.068 ms) : 2048, 2087
. : milestone, 2068,
tracing (1.81 ms) : 1795, 1825
. : milestone, 1810,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.55.0-SNAPSHOT~fde17658d3, baseline=1.57.0-SNAPSHOT~4d1a38c992
dateFormat X
axisFormat %s
section baseline
no_agent (18.164 ms) : 17978, 18350
. : milestone, 18164,
appsec (19.758 ms) : 19557, 19959
. : milestone, 19758,
code_origins (17.709 ms) : 17533, 17884
. : milestone, 17709,
iast (17.682 ms) : 17504, 17861
. : milestone, 17682,
profiling (18.588 ms) : 18402, 18774
. : milestone, 18588,
tracing (17.513 ms) : 17336, 17690
. : milestone, 17513,
section candidate
no_agent (19.014 ms) : 18823, 19206
. : milestone, 19014,
appsec (18.486 ms) : 18296, 18675
. : milestone, 18486,
code_origins (17.69 ms) : 17513, 17866
. : milestone, 17690,
iast (17.737 ms) : 17560, 17914
. : milestone, 17737,
profiling (18.45 ms) : 18268, 18631
. : milestone, 18450,
tracing (17.856 ms) : 17675, 18036
. : milestone, 17856,
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.55.0-SNAPSHOT~fde17658d3, baseline=1.57.0-SNAPSHOT~4d1a38c992
dateFormat X
axisFormat %s
section baseline
no_agent (14.721 s) : 14721000, 14721000
. : milestone, 14721000,
appsec (14.703 s) : 14703000, 14703000
. : milestone, 14703000,
iast (18.396 s) : 18396000, 18396000
. : milestone, 18396000,
iast_GLOBAL (17.933 s) : 17933000, 17933000
. : milestone, 17933000,
profiling (15.378 s) : 15378000, 15378000
. : milestone, 15378000,
tracing (14.664 s) : 14664000, 14664000
. : milestone, 14664000,
section candidate
no_agent (15.087 s) : 15087000, 15087000
. : milestone, 15087000,
appsec (14.714 s) : 14714000, 14714000
. : milestone, 14714000,
iast (18.677 s) : 18677000, 18677000
. : milestone, 18677000,
iast_GLOBAL (18.027 s) : 18027000, 18027000
. : milestone, 18027000,
profiling (14.448 s) : 14448000, 14448000
. : milestone, 14448000,
tracing (14.692 s) : 14692000, 14692000
. : milestone, 14692000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.55.0-SNAPSHOT~fde17658d3, baseline=1.57.0-SNAPSHOT~4d1a38c992
dateFormat X
axisFormat %s
section baseline
no_agent (1.478 ms) : 1467, 1490
. : milestone, 1478,
appsec (3.642 ms) : 3428, 3855
. : milestone, 3642,
iast (2.226 ms) : 2161, 2291
. : milestone, 2226,
iast_GLOBAL (2.273 ms) : 2208, 2338
. : milestone, 2273,
profiling (2.115 ms) : 2061, 2170
. : milestone, 2115,
tracing (2.076 ms) : 2025, 2127
. : milestone, 2076,
section candidate
no_agent (1.479 ms) : 1468, 1491
. : milestone, 1479,
appsec (3.691 ms) : 3474, 3909
. : milestone, 3691,
iast (2.227 ms) : 2162, 2291
. : milestone, 2227,
iast_GLOBAL (2.277 ms) : 2212, 2342
. : milestone, 2277,
profiling (2.485 ms) : 2323, 2647
. : milestone, 2485,
tracing (2.056 ms) : 2005, 2107
. : milestone, 2056,
|
internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy
Outdated
Show resolved
Hide resolved
| try { | ||
| clientInfo = connection.getClientInfo(); | ||
| } catch (final Throwable ex) { | ||
| // getClientInfo is likely not allowed, we can still extract info from the url alone |
Contributor
There was a problem hiding this comment.
Maybe make sense to put some debug/trace log? Just thinking out loud.
Contributor
Author
There was a problem hiding this comment.
you mean for investigation in the current case, or in general ?
Contributor
There was a problem hiding this comment.
I think it could help for investigation in the current case! I have no scale for how often this method is called though -- would adding debug statements here be "okay" in a production environment where we don't want to spam logs?
Contributor
Author
There was a problem hiding this comment.
added a log, we'll need to make sure this is not merged to main if we want to keep the feature flags for further investigations
Contributor
There was a problem hiding this comment.
Cool thanks! I'll add a do not merge tag for now to remind us
Contributor
Author
There was a problem hiding this comment.
removed the tag as we actually want to merge, and also I think a debug log is no issue here (I was initially spooked by the "let's not log" above)
Contributor
Author
|
Marking ready for review, as testing with the customer has shown that this helps mitigate the high resource usage they were seeing with their setup, so this feels like a good option to have around. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
49
to
53
| def clientInfo = new Properties() | ||
| clientInfo.setProperty("warehouse", "my-test-warehouse") // we'll check that property to know if clientInfo were used | ||
| def connection = Mock(Connection) { | ||
| getMetaData() >> (shouldFetchMetadata() ? metadata : { assert false }) | ||
| } |
There was a problem hiding this comment.
The regular-case test builds a clientInfo with a warehouse value but the Connection mock only stubs getMetaData() and never returns that clientInfo. JDBCDecorator.parseDBInfoFromConnection will call connection.getClientInfo(), which currently returns null, so the subsequent assertions for result.warehouse == "my-test-warehouse" fail whenever metadata fetching is enabled. Please stub getClientInfo() to return the constructed clientInfo so the test exercises the intended code path.
Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment.
this is both true and false, the setup was incorrect, but also TIL the implicit asserts inside a block are not treated as asserts, so the test was still passing (asserting nothing) 🤦
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
sarahchen6
approved these changes
Dec 2, 2025
mhlidd
reviewed
Dec 2, 2025
| "DD_TRACE_CUCUMBER_5_ANALYTICS_ENABLED": ["DD_CUCUMBER_5_ANALYTICS_ENABLED"], | ||
| "DD_TRACE_CUCUMBER_ANALYTICS_ENABLED": ["DD_CUCUMBER_ANALYTICS_ENABLED"], | ||
| "DD_TRACE_DATANUCLEUS_ANALYTICS_ENABLED": ["DD_DATANUCLEUS_ANALYTICS_ENABLED"], | ||
| "DD_TRACE_DB_METADATA_FETCHING_ON_CONNECT": [ |
Contributor
There was a problem hiding this comment.
The two newly added configs don't have aliases so they can directly go into the supportedConfigurations key instead
Comment on lines
+1675
to
+1676
| dbMetadataFetchingOnQuery = configProvider.getBoolean(DB_METADATA_FETCHING_ON_QUERY, true); | ||
| dbMetadataFetchingOnConnect = configProvider.getBoolean(DB_METADATA_FETCHING_ON_CONNECT, true); |
Contributor
There was a problem hiding this comment.
Nit: Should the default values be defined in a static final in ConfigDefaults.java for consistency with other configs?
| Config.get().isDbmTracePreparedStatements(); | ||
| public static final boolean DBM_ALWAYS_APPEND_SQL_COMMENT = | ||
| Config.get().isDbmAlwaysAppendSqlComment(); | ||
| public static final boolean FETCH_DB_METADATA_ON_CONNECT = |
Contributor
There was a problem hiding this comment.
❓ (For my understanding) What is the reason to have one flag be public and the other be private? Isn't best practice for fields to be private and accessed through public getters? 🤔
Contributor
PerfectSlayer
left a comment
PerfectSlayer
left a comment
There was a problem hiding this comment.
Additional Notes
missing tests for now
According the PR content, it seems this was properly addressed?
Do we still miss some more tests?
Comment on lines
+81
to
+82
| || !Boolean.parseBoolean( | ||
| props.getProperty("oracle.jdbc.useShardingDriverConnection")))) { |
Contributor
There was a problem hiding this comment.
🎯 suggestion: IIRC, this comes from user configuration? If so, parseBoolean() can throw an exception if the value can't be parsed properly, breaking the instrumentation.
Contributor
Author
There was a problem hiding this comment.
hmm, this is not my code, it was here before, and also looking at the implementation, I see it as ((s != null) && s.equalsIgnoreCase("true")) in Java 8, i.e. it shouldn't throw and just default to false
PerfectSlayer
changed the title
Contributor
Author
yes right I forgot to edit the description, I'll fix that |
sarahchen6
added a commit
that referenced
this pull request
Dec 10, 2025
…tation (#9774) (#10161) (cherry picked from commit ba2c056) Co-authored-by: Raphaël Vandon <[email protected]>
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
the JDBC instrumentation does extra calls to the DB to fetch the metadata and client info. In case this might cause issues, we want to be able to disable this behavior.
We'd still get some info to populate the DB tags from parsing the connection URL that's available without any extra call, but might be less representative of the truth than what we get from the metadata (for instance if the URL provided targets a cluster, the metdatada would give the exact instance we connected to)
Motivation
customer investigation
Additional Notes
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: APMS-16143