Conversation
…ame into dddbs and dddb SQL comment tags instead of the generic type string and null
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 64 metrics, 7 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.61.0-SNAPSHOT~877ba3d010, baseline=1.61.0-SNAPSHOT~1d0b64dbdb
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.061 s) : 0, 1061122
Total [baseline] (8.85 s) : 0, 8850353
Agent [candidate] (1.062 s) : 0, 1062045
Total [candidate] (8.825 s) : 0, 8825416
section iast
Agent [baseline] (1.223 s) : 0, 1222923
Total [baseline] (9.574 s) : 0, 9574173
Agent [candidate] (1.235 s) : 0, 1235325
Total [candidate] (9.535 s) : 0, 9534680
gantt
title insecure-bank - break down per module: candidate=1.61.0-SNAPSHOT~877ba3d010, baseline=1.61.0-SNAPSHOT~1d0b64dbdb
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.203 ms) : 0, 1203
crashtracking [candidate] (1.191 ms) : 0, 1191
BytebuddyAgent [baseline] (631.301 ms) : 0, 631301
BytebuddyAgent [candidate] (630.883 ms) : 0, 630883
AgentMeter [baseline] (28.943 ms) : 0, 28943
AgentMeter [candidate] (29.231 ms) : 0, 29231
GlobalTracer [baseline] (256.171 ms) : 0, 256171
GlobalTracer [candidate] (258.372 ms) : 0, 258372
AppSec [baseline] (31.415 ms) : 0, 31415
AppSec [candidate] (31.633 ms) : 0, 31633
Debugger [baseline] (58.522 ms) : 0, 58522
Debugger [candidate] (58.845 ms) : 0, 58845
Remote Config [baseline] (620.023 µs) : 0, 620
Remote Config [candidate] (610.119 µs) : 0, 610
Telemetry [baseline] (8.664 ms) : 0, 8664
Telemetry [candidate] (8.695 ms) : 0, 8695
Flare Poller [baseline] (8.106 ms) : 0, 8106
Flare Poller [candidate] (6.488 ms) : 0, 6488
section iast
crashtracking [baseline] (1.2 ms) : 0, 1200
crashtracking [candidate] (1.191 ms) : 0, 1191
BytebuddyAgent [baseline] (793.381 ms) : 0, 793381
BytebuddyAgent [candidate] (802.7 ms) : 0, 802700
AgentMeter [baseline] (11.294 ms) : 0, 11294
AgentMeter [candidate] (11.619 ms) : 0, 11619
GlobalTracer [baseline] (246.638 ms) : 0, 246638
GlobalTracer [candidate] (248.712 ms) : 0, 248712
AppSec [baseline] (27.223 ms) : 0, 27223
AppSec [candidate] (26.539 ms) : 0, 26539
Debugger [baseline] (61.685 ms) : 0, 61685
Debugger [candidate] (62.684 ms) : 0, 62684
Remote Config [baseline] (515.739 µs) : 0, 516
Remote Config [candidate] (520.038 µs) : 0, 520
Telemetry [baseline] (14.886 ms) : 0, 14886
Telemetry [candidate] (14.827 ms) : 0, 14827
Flare Poller [baseline] (4.913 ms) : 0, 4913
Flare Poller [candidate] (4.932 ms) : 0, 4932
IAST [baseline] (25.144 ms) : 0, 25144
IAST [candidate] (25.39 ms) : 0, 25390
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.61.0-SNAPSHOT~877ba3d010, baseline=1.61.0-SNAPSHOT~1d0b64dbdb
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.067 s) : 0, 1067487
Total [baseline] (11.042 s) : 0, 11042240
Agent [candidate] (1.066 s) : 0, 1065828
Total [candidate] (11.042 s) : 0, 11041640
section appsec
Agent [baseline] (1.244 s) : 0, 1244065
Total [baseline] (11.243 s) : 0, 11243084
Agent [candidate] (1.25 s) : 0, 1250267
Total [candidate] (11.191 s) : 0, 11190710
section iast
Agent [baseline] (1.228 s) : 0, 1228463
Total [baseline] (11.388 s) : 0, 11388101
Agent [candidate] (1.226 s) : 0, 1226491
Total [candidate] (11.392 s) : 0, 11392012
section profiling
Agent [baseline] (1.186 s) : 0, 1185622
Total [baseline] (11.003 s) : 0, 11002587
Agent [candidate] (1.181 s) : 0, 1181008
Total [candidate] (11.063 s) : 0, 11063125
gantt
title petclinic - break down per module: candidate=1.61.0-SNAPSHOT~877ba3d010, baseline=1.61.0-SNAPSHOT~1d0b64dbdb
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.209 ms) : 0, 1209
crashtracking [candidate] (1.192 ms) : 0, 1192
BytebuddyAgent [baseline] (633.211 ms) : 0, 633211
BytebuddyAgent [candidate] (633.729 ms) : 0, 633729
AgentMeter [baseline] (29.229 ms) : 0, 29229
AgentMeter [candidate] (29.283 ms) : 0, 29283
GlobalTracer [baseline] (258.672 ms) : 0, 258672
GlobalTracer [candidate] (258.897 ms) : 0, 258897
AppSec [baseline] (31.788 ms) : 0, 31788
AppSec [candidate] (31.724 ms) : 0, 31724
Debugger [baseline] (59.859 ms) : 0, 59859
Debugger [candidate] (59.746 ms) : 0, 59746
Remote Config [baseline] (629.82 µs) : 0, 630
Remote Config [candidate] (610.543 µs) : 0, 611
Telemetry [baseline] (8.753 ms) : 0, 8753
Telemetry [candidate] (8.68 ms) : 0, 8680
Flare Poller [baseline] (8.002 ms) : 0, 8002
Flare Poller [candidate] (5.752 ms) : 0, 5752
section appsec
crashtracking [baseline] (1.19 ms) : 0, 1190
crashtracking [candidate] (1.195 ms) : 0, 1195
BytebuddyAgent [baseline] (656.346 ms) : 0, 656346
BytebuddyAgent [candidate] (661.224 ms) : 0, 661224
AgentMeter [baseline] (12.046 ms) : 0, 12046
AgentMeter [candidate] (12.145 ms) : 0, 12145
GlobalTracer [baseline] (257.884 ms) : 0, 257884
GlobalTracer [candidate] (258.798 ms) : 0, 258798
AppSec [baseline] (177.439 ms) : 0, 177439
AppSec [candidate] (177.428 ms) : 0, 177428
Debugger [baseline] (65.877 ms) : 0, 65877
Debugger [candidate] (65.839 ms) : 0, 65839
Remote Config [baseline] (567.923 µs) : 0, 568
Remote Config [candidate] (579.293 µs) : 0, 579
Telemetry [baseline] (8.949 ms) : 0, 8949
Telemetry [candidate] (9.007 ms) : 0, 9007
Flare Poller [baseline] (3.652 ms) : 0, 3652
Flare Poller [candidate] (3.628 ms) : 0, 3628
IAST [baseline] (23.998 ms) : 0, 23998
IAST [candidate] (24.128 ms) : 0, 24128
section iast
crashtracking [baseline] (1.187 ms) : 0, 1187
crashtracking [candidate] (1.195 ms) : 0, 1195
BytebuddyAgent [baseline] (796.352 ms) : 0, 796352
BytebuddyAgent [candidate] (795.404 ms) : 0, 795404
AgentMeter [baseline] (11.324 ms) : 0, 11324
AgentMeter [candidate] (11.32 ms) : 0, 11320
GlobalTracer [baseline] (247.761 ms) : 0, 247761
GlobalTracer [candidate] (247.34 ms) : 0, 247340
AppSec [baseline] (26.529 ms) : 0, 26529
AppSec [candidate] (26.456 ms) : 0, 26456
Debugger [baseline] (64.43 ms) : 0, 64430
Debugger [candidate] (64.205 ms) : 0, 64205
Remote Config [baseline] (520.503 µs) : 0, 521
Remote Config [candidate] (508.538 µs) : 0, 509
Telemetry [baseline] (14.368 ms) : 0, 14368
Telemetry [candidate] (14.234 ms) : 0, 14234
Flare Poller [baseline] (4.813 ms) : 0, 4813
Flare Poller [candidate] (4.762 ms) : 0, 4762
IAST [baseline] (25.184 ms) : 0, 25184
IAST [candidate] (25.171 ms) : 0, 25171
section profiling
ProfilingAgent [baseline] (93.712 ms) : 0, 93712
ProfilingAgent [candidate] (93.857 ms) : 0, 93857
crashtracking [baseline] (1.173 ms) : 0, 1173
crashtracking [candidate] (1.15 ms) : 0, 1150
BytebuddyAgent [baseline] (685.061 ms) : 0, 685061
BytebuddyAgent [candidate] (680.47 ms) : 0, 680470
AgentMeter [baseline] (8.646 ms) : 0, 8646
AgentMeter [candidate] (8.701 ms) : 0, 8701
GlobalTracer [baseline] (216.14 ms) : 0, 216140
GlobalTracer [candidate] (216.101 ms) : 0, 216101
AppSec [baseline] (32.101 ms) : 0, 32101
AppSec [candidate] (32.139 ms) : 0, 32139
Debugger [baseline] (63.868 ms) : 0, 63868
Debugger [candidate] (64.057 ms) : 0, 64057
Remote Config [baseline] (598.383 µs) : 0, 598
Remote Config [candidate] (595.956 µs) : 0, 596
Telemetry [baseline] (9.756 ms) : 0, 9756
Telemetry [candidate] (9.784 ms) : 0, 9784
Flare Poller [baseline] (3.511 ms) : 0, 3511
Flare Poller [candidate] (3.491 ms) : 0, 3491
Profiling [baseline] (94.282 ms) : 0, 94282
Profiling [candidate] (94.426 ms) : 0, 94426
LoadParameters
See matching parameters
SummaryFound 5 performance improvements and 2 performance regressions! Performance is the same for 12 metrics, 17 unstable metrics.
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.61.0-SNAPSHOT~877ba3d010, baseline=1.61.0-SNAPSHOT~1d0b64dbdb
dateFormat X
axisFormat %s
section baseline
no_agent (19.364 ms) : 19164, 19563
. : milestone, 19364,
appsec (20.132 ms) : 19929, 20336
. : milestone, 20132,
code_origins (18.932 ms) : 18737, 19127
. : milestone, 18932,
iast (17.709 ms) : 17533, 17886
. : milestone, 17709,
profiling (18.514 ms) : 18325, 18703
. : milestone, 18514,
tracing (17.565 ms) : 17391, 17739
. : milestone, 17565,
section candidate
no_agent (18.103 ms) : 17914, 18292
. : milestone, 18103,
appsec (18.817 ms) : 18626, 19008
. : milestone, 18817,
code_origins (17.736 ms) : 17562, 17910
. : milestone, 17736,
iast (17.828 ms) : 17648, 18007
. : milestone, 17828,
profiling (18.914 ms) : 18719, 19109
. : milestone, 18914,
tracing (18.02 ms) : 17838, 18202
. : milestone, 18020,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.61.0-SNAPSHOT~877ba3d010, baseline=1.61.0-SNAPSHOT~1d0b64dbdb
dateFormat X
axisFormat %s
section baseline
no_agent (1.173 ms) : 1162, 1185
. : milestone, 1173,
iast (3.171 ms) : 3128, 3214
. : milestone, 3171,
iast_FULL (5.685 ms) : 5629, 5741
. : milestone, 5685,
iast_GLOBAL (3.381 ms) : 3327, 3435
. : milestone, 3381,
profiling (2.018 ms) : 1999, 2037
. : milestone, 2018,
tracing (1.758 ms) : 1744, 1773
. : milestone, 1758,
section candidate
no_agent (1.217 ms) : 1204, 1229
. : milestone, 1217,
iast (3.254 ms) : 3206, 3302
. : milestone, 3254,
iast_FULL (5.96 ms) : 5900, 6021
. : milestone, 5960,
iast_GLOBAL (3.593 ms) : 3542, 3643
. : milestone, 3593,
profiling (2.03 ms) : 2010, 2050
. : milestone, 2030,
tracing (1.778 ms) : 1763, 1793
. : milestone, 1778,
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.61.0-SNAPSHOT~877ba3d010, baseline=1.61.0-SNAPSHOT~1d0b64dbdb
dateFormat X
axisFormat %s
section baseline
no_agent (1.472 ms) : 1461, 1484
. : milestone, 1472,
appsec (2.528 ms) : 2472, 2583
. : milestone, 2528,
iast (2.259 ms) : 2190, 2328
. : milestone, 2259,
iast_GLOBAL (2.296 ms) : 2227, 2366
. : milestone, 2296,
profiling (2.085 ms) : 2030, 2140
. : milestone, 2085,
tracing (2.075 ms) : 2021, 2128
. : milestone, 2075,
section candidate
no_agent (1.483 ms) : 1471, 1494
. : milestone, 1483,
appsec (3.732 ms) : 3517, 3946
. : milestone, 3732,
iast (2.255 ms) : 2186, 2324
. : milestone, 2255,
iast_GLOBAL (2.295 ms) : 2226, 2365
. : milestone, 2295,
profiling (2.088 ms) : 2033, 2143
. : milestone, 2088,
tracing (2.058 ms) : 2005, 2111
. : milestone, 2058,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.61.0-SNAPSHOT~877ba3d010, baseline=1.61.0-SNAPSHOT~1d0b64dbdb
dateFormat X
axisFormat %s
section baseline
no_agent (14.902 s) : 14902000, 14902000
. : milestone, 14902000,
appsec (14.953 s) : 14953000, 14953000
. : milestone, 14953000,
iast (18.26 s) : 18260000, 18260000
. : milestone, 18260000,
iast_GLOBAL (17.729 s) : 17729000, 17729000
. : milestone, 17729000,
profiling (14.808 s) : 14808000, 14808000
. : milestone, 14808000,
tracing (15.082 s) : 15082000, 15082000
. : milestone, 15082000,
section candidate
no_agent (15.303 s) : 15303000, 15303000
. : milestone, 15303000,
appsec (14.863 s) : 14863000, 14863000
. : milestone, 14863000,
iast (18.115 s) : 18115000, 18115000
. : milestone, 18115000,
iast_GLOBAL (17.554 s) : 17554000, 17554000
. : milestone, 17554000,
profiling (14.662 s) : 14662000, 14662000
. : milestone, 14662000,
tracing (15.249 s) : 15249000, 15249000
. : milestone, 15249000,
|
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
...ent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java
Outdated
Show resolved
Hide resolved
...entation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java
Show resolved
Hide resolved
dd-java-agent/instrumentation/jdbc/src/test/groovy/OracleInjectionForkedTest.groovy
Show resolved
Hide resolved
dd-java-agent/instrumentation/jdbc/src/test/groovy/OracleInjectionForkedTest.groovy
Outdated
Show resolved
Hide resolved
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f04c0f9bf
ℹ️ 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".
| def adminConn = java.sql.DriverManager.getConnection(adminUrl, "system", oracle.getPassword()) | ||
| def rs = adminConn.createStatement().executeQuery( | ||
| "SELECT sql_fulltext FROM v\$sql " + | ||
| "WHERE sql_fulltext LIKE '%1729%' AND sql_fulltext LIKE '%dddbs%' " + |
There was a problem hiding this comment.
Exclude the admin lookup SQL from the v$sql match
This lookup can return the verifier query itself instead of the instrumented markerQuery, because the lookup SQL text contains both literals ('1729' and 'dddbs') used in its own LIKE filters and Oracle records executed statements in v$sql; with ROWNUM = 1, assertions may pass even if the target query was not injected correctly. Add a predicate that uniquely identifies the target statement (or excludes the lookup statement) so the test actually validates the intended SQL text.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
updated with adding like predicate on the comment
What Does This Do
Fix Oracle DBM trace correlation by injecting the database instance name into dddbs and dddb SQL comment tags instead of the generic type string and null.
Motivation
https://datadoghq.atlassian.net/browse/SDBM-2434
On the APM service page, Oracle-backed services show broken DBM trace correlation:
Checked the oracle integration and backend pipeline are correct. The Java tracer is injecting the wrong values.
Root cause
Two bugs in the SQL comment injection path:
Bug 1 — dddbs='oracle' instead of the instance name:
getDbService() called dbService(type, instance) which, when split-by-instance=false (the default), ignores the
instance name and returns the type-based cache entry ("oracle").
Bug 2 — dddb always absent:
Both injection sites passed dbInfo.getDb() for the dddb tag. For Oracle, the JDBC URL parser sets instance only —
getDb() is always null — so dddb was silently skipped by SQLCommenter.
Fix
type-based naming cache. Uses getInstance() (not the dbInstance() fallback to getDb()) so databases like SQL Server
that carry identity in getDb() are unaffected.
db).
span.getServiceName()); dddb now uses DECORATE.getDbInstance(dbInfo) (was dbInfo.getDb()).
Result
Oracle connections now inject:
/ddps='my-service',dddbs='freepdb1',ddh='localhost',dddb='freepdb1'/
instead of:
/ddps='my-service',dddbs='oracle',ddh='localhost'/
Testing
correct comment format against a mock Oracle connection.
container (Testcontainers), runs a query, then queries v$sql as system to verify the exact SQL text Oracle received
contains dddbs='freepdb1' and dddb='freepdb1' and does not contain dddbs='oracle'.
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: https://datadoghq.atlassian.net/browse/SDBM-2434
Note: Once your PR is ready to merge, add it to the merge queue by commenting
/merge./merge -ccancels the queue request./merge -f --reason "reason"skips all merge queue checks; please use this judiciously, as some checks do not run at the PR-level. For more information, see this doc.