-
Notifications
You must be signed in to change notification settings - Fork 316
fix JDBC's SQLCommenter not taking into account semicolons #9915
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
Conversation
|
🎯 Code Coverage 🔗 Commit SHA: f16df98 | 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 59 metrics, 6 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.56.0-SNAPSHOT~f16df987c1, baseline=1.56.0-SNAPSHOT~c17d0e83ec
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.101 s) : 0, 1101117
Total [baseline] (8.851 s) : 0, 8851193
Agent [candidate] (1.106 s) : 0, 1105860
Total [candidate] (8.832 s) : 0, 8832218
section iast
Agent [baseline] (1.24 s) : 0, 1240266
Total [baseline] (9.551 s) : 0, 9551280
Agent [candidate] (1.242 s) : 0, 1241833
Total [candidate] (9.556 s) : 0, 9556244
gantt
title insecure-bank - break down per module: candidate=1.56.0-SNAPSHOT~f16df987c1, baseline=1.56.0-SNAPSHOT~c17d0e83ec
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.45 ms) : 0, 1450
crashtracking [candidate] (1.454 ms) : 0, 1454
BytebuddyAgent [baseline] (704.476 ms) : 0, 704476
BytebuddyAgent [candidate] (707.026 ms) : 0, 707026
GlobalTracer [baseline] (247.754 ms) : 0, 247754
GlobalTracer [candidate] (248.948 ms) : 0, 248948
AppSec [baseline] (32.21 ms) : 0, 32210
AppSec [candidate] (32.437 ms) : 0, 32437
Debugger [baseline] (67.994 ms) : 0, 67994
Debugger [candidate] (68.598 ms) : 0, 68598
Remote Config [baseline] (634.97 µs) : 0, 635
Remote Config [candidate] (624.238 µs) : 0, 624
Telemetry [baseline] (8.16 ms) : 0, 8160
Telemetry [candidate] (8.233 ms) : 0, 8233
Flare Poller [baseline] (3.693 ms) : 0, 3693
Flare Poller [candidate] (3.751 ms) : 0, 3751
section iast
crashtracking [baseline] (1.464 ms) : 0, 1464
crashtracking [candidate] (1.471 ms) : 0, 1471
BytebuddyAgent [baseline] (828.44 ms) : 0, 828440
BytebuddyAgent [candidate] (829.298 ms) : 0, 829298
GlobalTracer [baseline] (237.796 ms) : 0, 237796
GlobalTracer [candidate] (237.884 ms) : 0, 237884
AppSec [baseline] (31.408 ms) : 0, 31408
AppSec [candidate] (33.087 ms) : 0, 33087
Debugger [baseline] (64.692 ms) : 0, 64692
Debugger [candidate] (65.223 ms) : 0, 65223
Remote Config [baseline] (547.541 µs) : 0, 548
Remote Config [candidate] (554.717 µs) : 0, 555
Telemetry [baseline] (7.651 ms) : 0, 7651
Telemetry [candidate] (7.652 ms) : 0, 7652
Flare Poller [baseline] (3.516 ms) : 0, 3516
Flare Poller [candidate] (3.583 ms) : 0, 3583
IAST [baseline] (29.984 ms) : 0, 29984
IAST [candidate] (28.333 ms) : 0, 28333
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.56.0-SNAPSHOT~f16df987c1, baseline=1.56.0-SNAPSHOT~c17d0e83ec
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.109 s) : 0, 1109118
Total [baseline] (10.816 s) : 0, 10816182
Agent [candidate] (1.106 s) : 0, 1106418
Total [candidate] (10.781 s) : 0, 10780727
section appsec
Agent [baseline] (1.287 s) : 0, 1286561
Total [baseline] (11.127 s) : 0, 11126670
Agent [candidate] (1.286 s) : 0, 1285547
Total [candidate] (11.212 s) : 0, 11211930
section iast
Agent [baseline] (1.262 s) : 0, 1261829
Total [baseline] (11.365 s) : 0, 11364914
Agent [candidate] (1.252 s) : 0, 1251811
Total [candidate] (11.364 s) : 0, 11364065
section profiling
Agent [baseline] (1.236 s) : 0, 1236077
Total [baseline] (11.071 s) : 0, 11071117
Agent [candidate] (1.237 s) : 0, 1236854
Total [candidate] (11.084 s) : 0, 11083905
gantt
title petclinic - break down per module: candidate=1.56.0-SNAPSHOT~f16df987c1, baseline=1.56.0-SNAPSHOT~c17d0e83ec
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.467 ms) : 0, 1467
crashtracking [candidate] (1.456 ms) : 0, 1456
BytebuddyAgent [baseline] (708.144 ms) : 0, 708144
BytebuddyAgent [candidate] (706.237 ms) : 0, 706237
GlobalTracer [baseline] (250.691 ms) : 0, 250691
GlobalTracer [candidate] (249.347 ms) : 0, 249347
AppSec [baseline] (32.556 ms) : 0, 32556
AppSec [candidate] (32.436 ms) : 0, 32436
Debugger [baseline] (69.059 ms) : 0, 69059
Debugger [candidate] (69.338 ms) : 0, 69338
Remote Config [baseline] (635.346 µs) : 0, 635
Remote Config [candidate] (705.136 µs) : 0, 705
Telemetry [baseline] (8.082 ms) : 0, 8082
Telemetry [candidate] (8.281 ms) : 0, 8281
Flare Poller [baseline] (3.703 ms) : 0, 3703
Flare Poller [candidate] (3.788 ms) : 0, 3788
section appsec
crashtracking [baseline] (1.471 ms) : 0, 1471
crashtracking [candidate] (1.463 ms) : 0, 1463
BytebuddyAgent [baseline] (735.007 ms) : 0, 735007
BytebuddyAgent [candidate] (731.349 ms) : 0, 731349
GlobalTracer [baseline] (241.359 ms) : 0, 241359
GlobalTracer [candidate] (242.858 ms) : 0, 242858
AppSec [baseline] (174.854 ms) : 0, 174854
AppSec [candidate] (174.563 ms) : 0, 174563
Debugger [baseline] (61.098 ms) : 0, 61098
Debugger [candidate] (62.553 ms) : 0, 62553
Remote Config [baseline] (641.123 µs) : 0, 641
Remote Config [candidate] (674.202 µs) : 0, 674
Telemetry [baseline] (8.472 ms) : 0, 8472
Telemetry [candidate] (8.372 ms) : 0, 8372
Flare Poller [baseline] (3.868 ms) : 0, 3868
Flare Poller [candidate] (3.933 ms) : 0, 3933
IAST [baseline] (24.865 ms) : 0, 24865
IAST [candidate] (25.004 ms) : 0, 25004
section iast
crashtracking [baseline] (1.488 ms) : 0, 1488
crashtracking [candidate] (1.464 ms) : 0, 1464
BytebuddyAgent [baseline] (844.285 ms) : 0, 844285
BytebuddyAgent [candidate] (835.736 ms) : 0, 835736
GlobalTracer [baseline] (240.592 ms) : 0, 240592
GlobalTracer [candidate] (239.854 ms) : 0, 239854
AppSec [baseline] (28.758 ms) : 0, 28758
AppSec [candidate] (33.459 ms) : 0, 33459
Debugger [baseline] (66.606 ms) : 0, 66606
Debugger [candidate] (66.115 ms) : 0, 66115
Remote Config [baseline] (570.713 µs) : 0, 571
Remote Config [candidate] (547.548 µs) : 0, 548
Telemetry [baseline] (7.831 ms) : 0, 7831
Telemetry [candidate] (7.673 ms) : 0, 7673
Flare Poller [baseline] (3.616 ms) : 0, 3616
Flare Poller [candidate] (3.554 ms) : 0, 3554
IAST [baseline] (33.082 ms) : 0, 33082
IAST [candidate] (28.552 ms) : 0, 28552
section profiling
ProfilingAgent [baseline] (97.063 ms) : 0, 97063
ProfilingAgent [candidate] (97.027 ms) : 0, 97027
crashtracking [baseline] (1.436 ms) : 0, 1436
crashtracking [candidate] (1.436 ms) : 0, 1436
BytebuddyAgent [baseline] (731.367 ms) : 0, 731367
BytebuddyAgent [candidate] (732.356 ms) : 0, 732356
GlobalTracer [baseline] (223.047 ms) : 0, 223047
GlobalTracer [candidate] (222.765 ms) : 0, 222765
AppSec [baseline] (32.257 ms) : 0, 32257
AppSec [candidate] (32.407 ms) : 0, 32407
Debugger [baseline] (68.664 ms) : 0, 68664
Debugger [candidate] (68.471 ms) : 0, 68471
Remote Config [baseline] (660.534 µs) : 0, 661
Remote Config [candidate] (667.783 µs) : 0, 668
Telemetry [baseline] (8.035 ms) : 0, 8035
Telemetry [candidate] (8.001 ms) : 0, 8001
Flare Poller [baseline] (3.83 ms) : 0, 3830
Flare Poller [candidate] (3.801 ms) : 0, 3801
Profiling [baseline] (97.636 ms) : 0, 97636
Profiling [candidate] (97.6 ms) : 0, 97600
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 insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.56.0-SNAPSHOT~f16df987c1, baseline=1.56.0-SNAPSHOT~c17d0e83ec
dateFormat X
axisFormat %s
section baseline
no_agent (1.198 ms) : 1186, 1210
. : milestone, 1198,
iast (3.309 ms) : 3206, 3412
. : milestone, 3309,
iast_FULL (6.109 ms) : 6046, 6173
. : milestone, 6109,
iast_GLOBAL (3.574 ms) : 3518, 3629
. : milestone, 3574,
profiling (2.169 ms) : 2149, 2189
. : milestone, 2169,
tracing (1.769 ms) : 1754, 1783
. : milestone, 1769,
section candidate
no_agent (1.181 ms) : 1169, 1192
. : milestone, 1181,
iast (3.342 ms) : 3270, 3415
. : milestone, 3342,
iast_FULL (5.693 ms) : 5637, 5749
. : milestone, 5693,
iast_GLOBAL (3.627 ms) : 3575, 3679
. : milestone, 3627,
profiling (2.216 ms) : 2193, 2238
. : milestone, 2216,
tracing (1.754 ms) : 1740, 1769
. : milestone, 1754,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.56.0-SNAPSHOT~f16df987c1, baseline=1.56.0-SNAPSHOT~c17d0e83ec
dateFormat X
axisFormat %s
section baseline
no_agent (18.435 ms) : 18243, 18627
. : milestone, 18435,
appsec (18.464 ms) : 18278, 18650
. : milestone, 18464,
code_origins (17.716 ms) : 17543, 17889
. : milestone, 17716,
iast (17.579 ms) : 17403, 17755
. : milestone, 17579,
profiling (18.577 ms) : 18393, 18761
. : milestone, 18577,
tracing (17.562 ms) : 17387, 17736
. : milestone, 17562,
section candidate
no_agent (17.181 ms) : 17009, 17354
. : milestone, 17181,
appsec (18.459 ms) : 18273, 18644
. : milestone, 18459,
code_origins (17.786 ms) : 17606, 17965
. : milestone, 17786,
iast (17.596 ms) : 17422, 17769
. : milestone, 17596,
profiling (18.553 ms) : 18364, 18743
. : milestone, 18553,
tracing (17.55 ms) : 17373, 17727
. : milestone, 17550,
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.56.0-SNAPSHOT~f16df987c1, baseline=1.56.0-SNAPSHOT~c17d0e83ec
dateFormat X
axisFormat %s
section baseline
no_agent (1.474 ms) : 1463, 1486
. : milestone, 1474,
appsec (3.704 ms) : 3485, 3922
. : milestone, 3704,
iast (2.214 ms) : 2150, 2279
. : milestone, 2214,
iast_GLOBAL (2.263 ms) : 2198, 2328
. : milestone, 2263,
profiling (2.052 ms) : 2000, 2104
. : milestone, 2052,
tracing (2.04 ms) : 1990, 2091
. : milestone, 2040,
section candidate
no_agent (1.47 ms) : 1458, 1481
. : milestone, 1470,
appsec (3.645 ms) : 3433, 3857
. : milestone, 3645,
iast (2.217 ms) : 2153, 2282
. : milestone, 2217,
iast_GLOBAL (2.263 ms) : 2198, 2328
. : milestone, 2263,
profiling (2.089 ms) : 2034, 2144
. : milestone, 2089,
tracing (2.042 ms) : 1991, 2092
. : milestone, 2042,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.56.0-SNAPSHOT~f16df987c1, baseline=1.56.0-SNAPSHOT~c17d0e83ec
dateFormat X
axisFormat %s
section baseline
no_agent (15.595 s) : 15595000, 15595000
. : milestone, 15595000,
appsec (14.553 s) : 14553000, 14553000
. : milestone, 14553000,
iast (18.42 s) : 18420000, 18420000
. : milestone, 18420000,
iast_GLOBAL (17.731 s) : 17731000, 17731000
. : milestone, 17731000,
profiling (14.785 s) : 14785000, 14785000
. : milestone, 14785000,
tracing (14.863 s) : 14863000, 14863000
. : milestone, 14863000,
section candidate
no_agent (15.507 s) : 15507000, 15507000
. : milestone, 15507000,
appsec (14.864 s) : 14864000, 14864000
. : milestone, 14864000,
iast (18.482 s) : 18482000, 18482000
. : milestone, 18482000,
iast_GLOBAL (17.924 s) : 17924000, 17924000
. : milestone, 17924000,
profiling (15.184 s) : 15184000, 15184000
. : milestone, 15184000,
tracing (14.514 s) : 14514000, 14514000
. : milestone, 14514000,
|
...gent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/SQLCommenter.java
Outdated
Show resolved
Hide resolved
82b9e9c to
a180774
Compare
...gent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/SQLCommenter.java
Outdated
Show resolved
Hide resolved
...gent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/SQLCommenter.java
Outdated
Show resolved
Hide resolved
...gent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/SQLCommenter.java
Outdated
Show resolved
Hide resolved
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.
LGTM
After adding the new always append comment feature in #9798, a customer found a bug that was present in our SQL Commenter since a long time. Basically, JDBC always split queries to send to the DB on the semicolon. If a comment is after a semicolon, JDBC will still split the queries, and return multiple ResultSet. This provoked exceptions like this one: ``` org.postgresql.util.PSQLException: Multiple ResultSets were returned by the query ``` SDBM-2100
a597794 to
f16df98
Compare
Motivation
After adding the new always append comment feature in #9798, a customer found a bug that was present in our SQL Commenter since a long time. Basically, JDBC always split queries to send to the DB on the semicolon. If a comment is after a semicolon, JDBC will still split the queries, and return multiple ResultSet. This provoked exceptions like this one:
What Does This Do
The SQL Commenter now correctly handles queries that ends with a semicolon. Instead of injecting the comment at the end, it will inject the comment before the semicolon.
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: SDBM-2100