Add peer service tag in dbm sql commenter #7913
Open
+67
−7
Conversation
This file contains 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
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
|
|
||
| // Both Postgres and MySQL are unhappy with anything before CALL in a stored procedure | ||
| // invocation but they seem ok with it after so we force append mode | ||
| if (firstWord.equalsIgnoreCase("call")) { |
There was a problem hiding this comment.
🟠 Code Quality Violation
Suggested change
| if (firstWord.equalsIgnoreCase("call")) { | |
| if ("call".equalsIgnoreCase(firstWord)) { |
the literal should be first in String comparisons (...read more)
One should always prioritize using a string literal as the first arguments in any string comparison. This approach serves as a preventive measure against NullPointerExceptions because when the second argument is null, instead of encountering an exception, the comparisons will simply yield false results.
BenchmarksStartupParameters
See matching parameters
SummaryFound 11 performance improvements and 13 performance regressions! Performance is the same for 29 metrics, 10 unstable metrics.
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.43.0-SNAPSHOT~69b16cc7af, baseline=1.46.0-SNAPSHOT~0b239ae5cc
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.06 s) : 0, 1059614
Total [baseline] (8.608 s) : 0, 8607764
Agent [candidate] (1.102 s) : 0, 1101847
Total [candidate] (8.68 s) : 0, 8680139
section iast
Agent [baseline] (1.185 s) : 0, 1185301
Total [baseline] (9.209 s) : 0, 9208795
Agent [candidate] (1.215 s) : 0, 1215132
Total [candidate] (9.177 s) : 0, 9176585
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.183 s) : 0, 1182735
Total [baseline] (9.146 s) : 0, 9145674
Agent [candidate] (1.226 s) : 0, 1226263
Total [candidate] (9.194 s) : 0, 9194040
section iast_TELEMETRY_OFF
Agent [baseline] (1.185 s) : 0, 1185169
Total [baseline] (9.215 s) : 0, 9215241
Agent [candidate] (1.217 s) : 0, 1216966
Total [candidate] (9.204 s) : 0, 9204101
gantt
title insecure-bank - break down per module: candidate=1.43.0-SNAPSHOT~69b16cc7af, baseline=1.46.0-SNAPSHOT~0b239ae5cc
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (718.111 ms) : 0, 718111
BytebuddyAgent [candidate] (702.099 ms) : 0, 702099
GlobalTracer [baseline] (256.721 ms) : 0, 256721
GlobalTracer [candidate] (320.812 ms) : 0, 320812
AppSec [baseline] (56.632 ms) : 0, 56632
AppSec [candidate] (54.916 ms) : 0, 54916
Remote Config [baseline] (740.767 µs) : 0, 741
Remote Config [candidate] (702.23 µs) : 0, 702
Telemetry [baseline] (12.373 ms) : 0, 12373
Telemetry [candidate] (9.389 ms) : 0, 9389
section iast
BytebuddyAgent [baseline] (833.928 ms) : 0, 833928
BytebuddyAgent [candidate] (808.28 ms) : 0, 808280
GlobalTracer [baseline] (247.059 ms) : 0, 247059
GlobalTracer [candidate] (306.398 ms) : 0, 306398
AppSec [baseline] (58.128 ms) : 0, 58128
AppSec [candidate] (57.19 ms) : 0, 57190
IAST [baseline] (21.574 ms) : 0, 21574
IAST [candidate] (21.43 ms) : 0, 21430
Remote Config [baseline] (681.515 µs) : 0, 682
Remote Config [candidate] (630.143 µs) : 0, 630
Telemetry [baseline] (8.884 ms) : 0, 8884
Telemetry [candidate] (7.454 ms) : 0, 7454
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (831.855 ms) : 0, 831855
BytebuddyAgent [candidate] (815.816 ms) : 0, 815816
GlobalTracer [baseline] (246.183 ms) : 0, 246183
GlobalTracer [candidate] (309.127 ms) : 0, 309127
AppSec [baseline] (58.532 ms) : 0, 58532
AppSec [candidate] (58.403 ms) : 0, 58403
IAST [baseline] (21.632 ms) : 0, 21632
IAST [candidate] (20.814 ms) : 0, 20814
Remote Config [baseline] (686.611 µs) : 0, 687
Remote Config [candidate] (626.77 µs) : 0, 627
Telemetry [baseline] (8.825 ms) : 0, 8825
Telemetry [candidate] (7.606 ms) : 0, 7606
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (833.066 ms) : 0, 833066
BytebuddyAgent [candidate] (808.985 ms) : 0, 808985
GlobalTracer [baseline] (247.499 ms) : 0, 247499
GlobalTracer [candidate] (307.394 ms) : 0, 307394
AppSec [baseline] (58.496 ms) : 0, 58496
AppSec [candidate] (57.932 ms) : 0, 57932
IAST [baseline] (21.374 ms) : 0, 21374
IAST [candidate] (20.713 ms) : 0, 20713
Remote Config [baseline] (687.731 µs) : 0, 688
Remote Config [candidate] (654.983 µs) : 0, 655
Telemetry [baseline] (8.904 ms) : 0, 8904
Telemetry [candidate] (7.472 ms) : 0, 7472
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.43.0-SNAPSHOT~69b16cc7af, baseline=1.46.0-SNAPSHOT~0b239ae5cc
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.053 s) : 0, 1052980
Total [baseline] (10.426 s) : 0, 10425630
Agent [candidate] (1.09 s) : 0, 1089586
Total [candidate] (10.397 s) : 0, 10396934
section appsec
Agent [baseline] (1.189 s) : 0, 1188562
Total [baseline] (10.69 s) : 0, 10689663
Agent [candidate] (1.226 s) : 0, 1226156
Total [candidate] (10.748 s) : 0, 10747528
section iast
Agent [baseline] (1.199 s) : 0, 1199138
Total [baseline] (11.005 s) : 0, 11004636
Agent [candidate] (1.216 s) : 0, 1215601
Total [candidate] (10.907 s) : 0, 10907075
section profiling
Agent [baseline] (1.254 s) : 0, 1253542
Total [baseline] (10.742 s) : 0, 10741895
Agent [candidate] (1.299 s) : 0, 1299295
Total [candidate] (10.794 s) : 0, 10794124
gantt
title petclinic - break down per module: candidate=1.43.0-SNAPSHOT~69b16cc7af, baseline=1.46.0-SNAPSHOT~0b239ae5cc
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (711.616 ms) : 0, 711616
BytebuddyAgent [candidate] (693.038 ms) : 0, 693038
GlobalTracer [baseline] (255.187 ms) : 0, 255187
GlobalTracer [candidate] (317.633 ms) : 0, 317633
AppSec [baseline] (55.469 ms) : 0, 55469
AppSec [candidate] (54.378 ms) : 0, 54378
Remote Config [baseline] (745.916 µs) : 0, 746
Remote Config [candidate] (686.25 µs) : 0, 686
Telemetry [baseline] (14.99 ms) : 0, 14990
Telemetry [candidate] (10.137 ms) : 0, 10137
section appsec
BytebuddyAgent [baseline] (732.382 ms) : 0, 732382
BytebuddyAgent [candidate] (713.285 ms) : 0, 713285
GlobalTracer [baseline] (252.084 ms) : 0, 252084
GlobalTracer [candidate] (314.596 ms) : 0, 314596
AppSec [baseline] (170.641 ms) : 0, 170641
AppSec [candidate] (165.963 ms) : 0, 165963
Remote Config [baseline] (669.735 µs) : 0, 670
Remote Config [candidate] (633.193 µs) : 0, 633
Telemetry [baseline] (8.135 ms) : 0, 8135
Telemetry [candidate] (7.807 ms) : 0, 7807
IAST [baseline] (19.316 ms) : 0, 19316
IAST [candidate] (20.207 ms) : 0, 20207
section iast
BytebuddyAgent [baseline] (845.3 ms) : 0, 845300
BytebuddyAgent [candidate] (808.532 ms) : 0, 808532
GlobalTracer [baseline] (248.524 ms) : 0, 248524
GlobalTracer [candidate] (306.619 ms) : 0, 306619
AppSec [baseline] (58.606 ms) : 0, 58606
AppSec [candidate] (56.906 ms) : 0, 56906
Remote Config [baseline] (686.368 µs) : 0, 686
Remote Config [candidate] (634.496 µs) : 0, 634
Telemetry [baseline] (8.88 ms) : 0, 8880
Telemetry [candidate] (7.482 ms) : 0, 7482
IAST [baseline] (21.772 ms) : 0, 21772
IAST [candidate] (21.687 ms) : 0, 21687
section profiling
ProfilingAgent [baseline] (95.165 ms) : 0, 95165
ProfilingAgent [candidate] (94.894 ms) : 0, 94894
BytebuddyAgent [baseline] (703.562 ms) : 0, 703562
BytebuddyAgent [candidate] (692.016 ms) : 0, 692016
GlobalTracer [baseline] (348.625 ms) : 0, 348625
GlobalTracer [candidate] (405.253 ms) : 0, 405253
AppSec [baseline] (54.77 ms) : 0, 54770
AppSec [candidate] (55.526 ms) : 0, 55526
Remote Config [baseline] (692.354 µs) : 0, 692
Remote Config [candidate] (688.111 µs) : 0, 688
Telemetry [baseline] (8.746 ms) : 0, 8746
Telemetry [candidate] (11.587 ms) : 0, 11587
Profiling [baseline] (95.188 ms) : 0, 95188
Profiling [candidate] (94.917 ms) : 0, 94917
LoadParameters
See matching parameters
SummaryFound 2 performance improvements and 0 performance regressions! Performance is the same for 10 metrics, 16 unstable metrics.
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.43.0-SNAPSHOT~69b16cc7af, baseline=1.46.0-SNAPSHOT~0b239ae5cc
dateFormat X
axisFormat %s
section baseline
no_agent (1.34 ms) : 1320, 1359
. : milestone, 1340,
appsec (1.734 ms) : 1711, 1758
. : milestone, 1734,
appsec_no_iast (1.762 ms) : 1738, 1786
. : milestone, 1762,
iast (1.499 ms) : 1475, 1522
. : milestone, 1499,
profiling (1.518 ms) : 1494, 1541
. : milestone, 1518,
tracing (1.474 ms) : 1449, 1500
. : milestone, 1474,
section candidate
no_agent (1.361 ms) : 1341, 1382
. : milestone, 1361,
appsec (1.758 ms) : 1734, 1782
. : milestone, 1758,
appsec_no_iast (1.748 ms) : 1722, 1773
. : milestone, 1748,
iast (1.521 ms) : 1498, 1543
. : milestone, 1521,
profiling (1.512 ms) : 1487, 1536
. : milestone, 1512,
tracing (1.502 ms) : 1476, 1527
. : milestone, 1502,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.43.0-SNAPSHOT~69b16cc7af, baseline=1.46.0-SNAPSHOT~0b239ae5cc
dateFormat X
axisFormat %s
section baseline
no_agent (376.051 µs) : 356, 396
. : milestone, 376,
iast (508.845 µs) : 487, 531
. : milestone, 509,
iast_FULL (746.601 µs) : 725, 769
. : milestone, 747,
iast_GLOBAL (565.258 µs) : 541, 589
. : milestone, 565,
iast_HARDCODED_SECRET_DISABLED (508.252 µs) : 486, 530
. : milestone, 508,
iast_INACTIVE (459.53 µs) : 438, 481
. : milestone, 460,
iast_TELEMETRY_OFF (496.294 µs) : 474, 519
. : milestone, 496,
tracing (451.205 µs) : 431, 472
. : milestone, 451,
section candidate
no_agent (378.785 µs) : 358, 399
. : milestone, 379,
iast (493.254 µs) : 472, 515
. : milestone, 493,
iast_FULL (650.064 µs) : 628, 672
. : milestone, 650,
iast_GLOBAL (525.417 µs) : 504, 547
. : milestone, 525,
iast_HARDCODED_SECRET_DISABLED (493.921 µs) : 473, 515
. : milestone, 494,
iast_INACTIVE (464.707 µs) : 444, 486
. : milestone, 465,
iast_TELEMETRY_OFF (482.135 µs) : 461, 503
. : milestone, 482,
tracing (449.597 µs) : 429, 470
. : milestone, 450,
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.43.0-SNAPSHOT~69b16cc7af, baseline=1.46.0-SNAPSHOT~0b239ae5cc
dateFormat X
axisFormat %s
section baseline
no_agent (1.464 ms) : 1453, 1475
. : milestone, 1464,
appsec (2.352 ms) : 2309, 2395
. : milestone, 2352,
iast (2.095 ms) : 2041, 2149
. : milestone, 2095,
iast_GLOBAL (2.137 ms) : 2082, 2191
. : milestone, 2137,
profiling (2.442 ms) : 2259, 2625
. : milestone, 2442,
tracing (1.937 ms) : 1896, 1979
. : milestone, 1937,
section candidate
no_agent (1.468 ms) : 1457, 1480
. : milestone, 1468,
appsec (2.32 ms) : 2279, 2361
. : milestone, 2320,
iast (2.071 ms) : 2019, 2122
. : milestone, 2071,
iast_GLOBAL (2.109 ms) : 2057, 2161
. : milestone, 2109,
profiling (1.943 ms) : 1901, 1986
. : milestone, 1943,
tracing (1.913 ms) : 1873, 1953
. : milestone, 1913,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.43.0-SNAPSHOT~69b16cc7af, baseline=1.46.0-SNAPSHOT~0b239ae5cc
dateFormat X
axisFormat %s
section baseline
no_agent (15.399 s) : 15399000, 15399000
. : milestone, 15399000,
appsec (14.993 s) : 14993000, 14993000
. : milestone, 14993000,
iast (18.828 s) : 18828000, 18828000
. : milestone, 18828000,
iast_GLOBAL (17.977 s) : 17977000, 17977000
. : milestone, 17977000,
profiling (15.189 s) : 15189000, 15189000
. : milestone, 15189000,
tracing (15.006 s) : 15006000, 15006000
. : milestone, 15006000,
section candidate
no_agent (15.455 s) : 15455000, 15455000
. : milestone, 15455000,
appsec (15.133 s) : 15133000, 15133000
. : milestone, 15133000,
iast (18.352 s) : 18352000, 18352000
. : milestone, 18352000,
iast_GLOBAL (17.858 s) : 17858000, 17858000
. : milestone, 17858000,
profiling (15.23 s) : 15230000, 15230000
. : milestone, 15230000,
tracing (14.906 s) : 14906000, 14906000
. : milestone, 14906000,
|
|
Changed behavior according to discussion with Will. |
| @@ -15,6 +19,7 @@ public class SQLCommenter { | |||
| private static final String DATABASE_SERVICE = encode("dddbs"); | |||
| private static final String DD_HOSTNAME = encode("ddh"); | |||
| private static final String DD_DB_NAME = encode("dddb"); | |||
| private static final String DD_DB_INSTANCE = "ddprs"; | |||
There was a problem hiding this comment.
I would rename this to dd_peer_service or something
| @@ -113,6 +126,7 @@ public static String inject( | |||
| dbService, | |||
| hostname, | |||
| dbName, | |||
| peerServiceString, | |||
There was a problem hiding this comment.
We could probably just call this peerService
| } | ||
| } | ||
| } | ||
| sqlWithComment == expected |
There was a problem hiding this comment.
This is already checked in the then: block.
| if (injectTrace) { | ||
| sqlWithComment = SQLCommenter.inject(query, dbService, dbType, host, dbName, traceParent, true, appendComment) | ||
| } else { | ||
| if (appendComment) { |
There was a problem hiding this comment.
This if branching structure is a bit hard to read. I think you could just do else if(appendComment) and then just a final else rather than having nested if statements like this
| sqlWithComment == expected | ||
|
|
||
| where: | ||
| query | ddService | ddEnv | dbService | dbType | host | dbName | ddVersion | injectTrace | appendComment | traceParent | peerService | expected |
There was a problem hiding this comment.
we can probably update this to only have the cases that include ddprs since that is what we want to test here. the other cases should already be tested in other tests so no need to repeat it.
| final String dbService, | ||
| final String hostname, | ||
| final String dbName, | ||
| final String dbInstance, |
There was a problem hiding this comment.
lets rename this to peerService for cleanliness
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
Adds a peer.service annotation to the SQL Commenter for DBM tracing.
Motivation
We want to add tags in line with the peer service model
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any usefull labelsclose,fixor any linking keywords when referencing an issue.Use
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]