-
Notifications
You must be signed in to change notification settings - Fork 313
Container hash tags propagation #9282
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
base: master
Are you sure you want to change the base?
Conversation
… propagation feature flag is enabled
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 45 metrics, 14 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.52.0-SNAPSHOT~2fcea15ec8, baseline=1.53.0-SNAPSHOT~27cd7f43d9
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.048 s) : 0, 1048323
Total [baseline] (10.799 s) : 0, 10799118
Agent [candidate] (1.054 s) : 0, 1053566
Total [candidate] (10.749 s) : 0, 10749440
section appsec
Agent [baseline] (1.228 s) : 0, 1227523
Total [baseline] (10.883 s) : 0, 10882852
Agent [candidate] (1.225 s) : 0, 1225151
Total [candidate] (10.822 s) : 0, 10822447
section iast
Agent [baseline] (1.19 s) : 0, 1189961
Total [baseline] (11.015 s) : 0, 11014952
Agent [candidate] (1.18 s) : 0, 1180203
Total [candidate] (10.89 s) : 0, 10889598
section profiling
Agent [baseline] (1.197 s) : 0, 1196920
Total [baseline] (10.86 s) : 0, 10859708
Agent [candidate] (1.192 s) : 0, 1191958
Total [candidate] (10.894 s) : 0, 10893614
gantt
title petclinic - break down per module: candidate=1.52.0-SNAPSHOT~2fcea15ec8, baseline=1.53.0-SNAPSHOT~27cd7f43d9
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.454 ms) : 0, 1454
crashtracking [candidate] (1.472 ms) : 0, 1472
BytebuddyAgent [baseline] (733.26 ms) : 0, 733260
BytebuddyAgent [candidate] (737.492 ms) : 0, 737492
GlobalTracer [baseline] (243.919 ms) : 0, 243919
GlobalTracer [candidate] (242.773 ms) : 0, 242773
AppSec [baseline] (30.182 ms) : 0, 30182
AppSec [candidate] (30.192 ms) : 0, 30192
Debugger [baseline] (6.058 ms) : 0, 6058
Debugger [candidate] (6.121 ms) : 0, 6121
Remote Config [baseline] (671.85 µs) : 0, 672
Remote Config [candidate] (688.474 µs) : 0, 688
Telemetry [baseline] (11.664 ms) : 0, 11664
Telemetry [candidate] (13.556 ms) : 0, 13556
section appsec
crashtracking [baseline] (1.454 ms) : 0, 1454
crashtracking [candidate] (1.449 ms) : 0, 1449
BytebuddyAgent [baseline] (757.777 ms) : 0, 757777
BytebuddyAgent [candidate] (757.836 ms) : 0, 757836
GlobalTracer [baseline] (236.177 ms) : 0, 236177
GlobalTracer [candidate] (234.901 ms) : 0, 234901
AppSec [baseline] (170.814 ms) : 0, 170814
AppSec [candidate] (169.907 ms) : 0, 169907
Debugger [baseline] (6.536 ms) : 0, 6536
Debugger [candidate] (6.478 ms) : 0, 6478
Remote Config [baseline] (647.88 µs) : 0, 648
Remote Config [candidate] (616.877 µs) : 0, 617
Telemetry [baseline] (9.249 ms) : 0, 9249
Telemetry [candidate] (9.275 ms) : 0, 9275
IAST [baseline] (23.737 ms) : 0, 23737
IAST [candidate] (23.521 ms) : 0, 23521
section iast
crashtracking [baseline] (1.469 ms) : 0, 1469
crashtracking [candidate] (1.446 ms) : 0, 1446
BytebuddyAgent [baseline] (859.882 ms) : 0, 859882
BytebuddyAgent [candidate] (852.678 ms) : 0, 852678
GlobalTracer [baseline] (234.321 ms) : 0, 234321
GlobalTracer [candidate] (232.556 ms) : 0, 232556
AppSec [baseline] (26.124 ms) : 0, 26124
AppSec [candidate] (26.811 ms) : 0, 26811
Debugger [baseline] (6.681 ms) : 0, 6681
Debugger [candidate] (5.744 ms) : 0, 5744
Remote Config [baseline] (611.669 µs) : 0, 612
Remote Config [candidate] (602.478 µs) : 0, 602
Telemetry [baseline] (8.362 ms) : 0, 8362
Telemetry [candidate] (8.243 ms) : 0, 8243
IAST [baseline] (31.236 ms) : 0, 31236
IAST [candidate] (31.012 ms) : 0, 31012
section profiling
ProfilingAgent [baseline] (107.923 ms) : 0, 107923
ProfilingAgent [candidate] (107.756 ms) : 0, 107756
crashtracking [baseline] (1.418 ms) : 0, 1418
crashtracking [candidate] (1.415 ms) : 0, 1415
BytebuddyAgent [baseline] (762.544 ms) : 0, 762544
BytebuddyAgent [candidate] (760.153 ms) : 0, 760153
GlobalTracer [baseline] (222.072 ms) : 0, 222072
GlobalTracer [candidate] (220.361 ms) : 0, 220361
AppSec [baseline] (29.94 ms) : 0, 29940
AppSec [candidate] (30.305 ms) : 0, 30305
Debugger [baseline] (6.272 ms) : 0, 6272
Debugger [candidate] (7.028 ms) : 0, 7028
Remote Config [baseline] (736.398 µs) : 0, 736
Remote Config [candidate] (704.34 µs) : 0, 704
Telemetry [baseline] (16.35 ms) : 0, 16350
Telemetry [candidate] (14.759 ms) : 0, 14759
Profiling [baseline] (108.569 ms) : 0, 108569
Profiling [candidate] (108.413 ms) : 0, 108413
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.52.0-SNAPSHOT~2fcea15ec8, baseline=1.53.0-SNAPSHOT~27cd7f43d9
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.059 s) : 0, 1058780
Total [baseline] (8.658 s) : 0, 8657881
Agent [candidate] (1.053 s) : 0, 1053151
Total [candidate] (8.654 s) : 0, 8653972
section iast
Agent [baseline] (1.198 s) : 0, 1198289
Total [baseline] (9.332 s) : 0, 9331798
Agent [candidate] (1.183 s) : 0, 1183094
Total [candidate] (9.363 s) : 0, 9363359
gantt
title insecure-bank - break down per module: candidate=1.52.0-SNAPSHOT~2fcea15ec8, baseline=1.53.0-SNAPSHOT~27cd7f43d9
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.465 ms) : 0, 1465
crashtracking [candidate] (1.464 ms) : 0, 1464
BytebuddyAgent [baseline] (740.084 ms) : 0, 740084
BytebuddyAgent [candidate] (737.691 ms) : 0, 737691
GlobalTracer [baseline] (244.88 ms) : 0, 244880
GlobalTracer [candidate] (242.768 ms) : 0, 242768
AppSec [baseline] (30.636 ms) : 0, 30636
AppSec [candidate] (30.311 ms) : 0, 30311
Debugger [baseline] (6.108 ms) : 0, 6108
Debugger [candidate] (6.116 ms) : 0, 6116
Remote Config [baseline] (678.85 µs) : 0, 679
Remote Config [candidate] (667.994 µs) : 0, 668
Telemetry [baseline] (13.791 ms) : 0, 13791
Telemetry [candidate] (12.972 ms) : 0, 12972
section iast
crashtracking [baseline] (1.48 ms) : 0, 1480
crashtracking [candidate] (1.464 ms) : 0, 1464
BytebuddyAgent [baseline] (865.907 ms) : 0, 865907
BytebuddyAgent [candidate] (856.43 ms) : 0, 856430
GlobalTracer [baseline] (235.696 ms) : 0, 235696
GlobalTracer [candidate] (231.632 ms) : 0, 231632
AppSec [baseline] (28.851 ms) : 0, 28851
AppSec [candidate] (25.099 ms) : 0, 25099
Debugger [baseline] (5.818 ms) : 0, 5818
Debugger [candidate] (5.757 ms) : 0, 5757
Remote Config [baseline] (617.531 µs) : 0, 618
Remote Config [candidate] (611.068 µs) : 0, 611
Telemetry [baseline] (8.486 ms) : 0, 8486
Telemetry [candidate] (8.239 ms) : 0, 8239
IAST [baseline] (30.217 ms) : 0, 30217
IAST [candidate] (32.868 ms) : 0, 32868
LoadParameters
See matching parameters
SummaryFound 2 performance improvements and 3 performance regressions! Performance is the same for 7 metrics, 12 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.52.0-SNAPSHOT~2fcea15ec8, baseline=1.53.0-SNAPSHOT~27cd7f43d9
dateFormat X
axisFormat %s
section baseline
no_agent (4.424 ms) : 4369, 4479
. : milestone, 4424,
iast (9.013 ms) : 8870, 9157
. : milestone, 9013,
iast_FULL (14.329 ms) : 14047, 14610
. : milestone, 14329,
iast_GLOBAL (9.595 ms) : 9431, 9758
. : milestone, 9595,
profiling (8.759 ms) : 8625, 8894
. : milestone, 8759,
tracing (8.042 ms) : 7917, 8167
. : milestone, 8042,
section candidate
no_agent (4.282 ms) : 4233, 4331
. : milestone, 4282,
iast (9.195 ms) : 9047, 9343
. : milestone, 9195,
iast_FULL (13.644 ms) : 13377, 13910
. : milestone, 13644,
iast_GLOBAL (10.042 ms) : 9870, 10213
. : milestone, 10042,
profiling (8.598 ms) : 8467, 8728
. : milestone, 8598,
tracing (7.575 ms) : 7458, 7692
. : milestone, 7575,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.52.0-SNAPSHOT~2fcea15ec8, baseline=1.53.0-SNAPSHOT~27cd7f43d9
dateFormat X
axisFormat %s
section baseline
no_agent (36.398 ms) : 36105, 36691
. : milestone, 36398,
appsec (46.882 ms) : 46474, 47291
. : milestone, 46882,
code_origins (47.054 ms) : 46655, 47452
. : milestone, 47054,
iast (43.167 ms) : 42789, 43546
. : milestone, 43167,
profiling (47.49 ms) : 47043, 47937
. : milestone, 47490,
tracing (43.637 ms) : 43284, 43990
. : milestone, 43637,
section candidate
no_agent (37.852 ms) : 37549, 38156
. : milestone, 37852,
appsec (46.389 ms) : 45968, 46810
. : milestone, 46389,
code_origins (47.263 ms) : 46843, 47683
. : milestone, 47263,
iast (45.259 ms) : 44858, 45661
. : milestone, 45259,
profiling (48.675 ms) : 48223, 49128
. : milestone, 48675,
tracing (43.228 ms) : 42865, 43592
. : milestone, 43228,
DacapoParameters
See matching parameters
SummaryFound 1 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 0 unstable metrics.
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.52.0-SNAPSHOT~2fcea15ec8, baseline=1.53.0-SNAPSHOT~27cd7f43d9
dateFormat X
axisFormat %s
section baseline
no_agent (15.223 s) : 15223000, 15223000
. : milestone, 15223000,
appsec (14.845 s) : 14845000, 14845000
. : milestone, 14845000,
iast (18.342 s) : 18342000, 18342000
. : milestone, 18342000,
iast_GLOBAL (18.152 s) : 18152000, 18152000
. : milestone, 18152000,
profiling (15.786 s) : 15786000, 15786000
. : milestone, 15786000,
tracing (14.999 s) : 14999000, 14999000
. : milestone, 14999000,
section candidate
no_agent (15.527 s) : 15527000, 15527000
. : milestone, 15527000,
appsec (15.018 s) : 15018000, 15018000
. : milestone, 15018000,
iast (18.303 s) : 18303000, 18303000
. : milestone, 18303000,
iast_GLOBAL (18.21 s) : 18210000, 18210000
. : milestone, 18210000,
profiling (15.59 s) : 15590000, 15590000
. : milestone, 15590000,
tracing (15.056 s) : 15056000, 15056000
. : milestone, 15056000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.52.0-SNAPSHOT~2fcea15ec8, baseline=1.53.0-SNAPSHOT~27cd7f43d9
dateFormat X
axisFormat %s
section baseline
no_agent (1.482 ms) : 1471, 1494
. : milestone, 1482,
appsec (3.705 ms) : 3488, 3922
. : milestone, 3705,
iast (2.213 ms) : 2150, 2275
. : milestone, 2213,
iast_GLOBAL (2.247 ms) : 2183, 2310
. : milestone, 2247,
profiling (2.071 ms) : 2018, 2123
. : milestone, 2071,
tracing (2.024 ms) : 1975, 2073
. : milestone, 2024,
section candidate
no_agent (1.482 ms) : 1470, 1493
. : milestone, 1482,
appsec (2.474 ms) : 2421, 2528
. : milestone, 2474,
iast (2.205 ms) : 2142, 2268
. : milestone, 2205,
iast_GLOBAL (2.244 ms) : 2181, 2308
. : milestone, 2244,
profiling (2.062 ms) : 2010, 2113
. : milestone, 2062,
tracing (2.032 ms) : 1983, 2081
. : milestone, 2032,
|
Extract getBaseHash to be available for SQLCommenter TBD: fix SQLCommenterTest TBD: add DB_DBM_INJECT_SERVICE_HASH_ENABLED
Code coverage: total 57.30%, base diff -0.15%, patch 74.00% (view details) This comment will be updated automatically if new data arrives.🔗 Commit SHA: 2fcea15 | Docs | Was this helpful? Give us feedback! |
Clean up hasDDComment.
if (sql == null || sql.isEmpty()) { | ||
return sql; | ||
} | ||
if (hasDDComment(sql, appendComment)) { |
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.
Move this check to after the appendComment decision is made. Otherwise, it won't work when preferAppend differs from appendComment. This can result in the comment being added twice. Tests were added to ensure this behavior is confirmed.
if (log.isDebugEnabled()) { | ||
log.debug("exception thrown while encoding sql comment %s", e); | ||
} | ||
String encodedValue; |
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.
Do the encoding first to handle a possible encoding exception properly; otherwise, in case of an exception, it will inject a key with an unclosed quote.
if (firstWord.equalsIgnoreCase("call")) { | ||
appendComment = true; | ||
} | ||
|
||
// Append the comment in the case of a pg_hint_plan extension | ||
if (dbType.startsWith("postgres") && containsPgHint(sql)) { | ||
if (dbType.startsWith("postgres") && sql.contains("/*+")) { |
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.
Fix the bug to ensure the hint is detected at the beginning. The original check was sql.indexOf("/*+") > 0
. Remove containsPgHint for simplicity. Tests were added to ensure this behavior is confirmed.
if ((!(sql.endsWith(CLOSE_COMMENT)) && appendComment) | ||
|| ((!(sql.startsWith(OPEN_COMMENT))) && !appendComment)) { | ||
return false; | ||
} | ||
// else check to see if it's a DBM trace sql comment | ||
int startIdx = 2; |
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.
Replace the magic constant 2
.
} | ||
int startComment = appendComment ? startIdx : sql.length(); | ||
boolean found = false; | ||
if (startComment > 2) { |
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.
This condition is useless because it's never true.
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
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.
nice, lgtm
if (DataStreamsTags.baseHash != 0) { | ||
this.hash = DataStreamsTags.baseHash; | ||
} | ||
this.hash = BaseHash.getBaseHash(); |
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.
@piochelepiotr I expected baseHash (novice DSM mental model)to be sent twice: pathHash computation and DSM pipeline payload, why is it used only here and this.hash is overwritten? Doesn't it end up with loss of informaiton for DSM backend
String newContainerTagsHash = response.header(DATADOG_CONTAINER_TAGS_HASH); | ||
if (newContainerTagsHash != null) { | ||
ContainerInfo containerInfo = ContainerInfo.get(); | ||
if (!newContainerTagsHash.equals(containerInfo.getContainerTagsHash())) { |
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.
The container hash comparison and update are not atomic. Between checking equality and calling recalcBaseHash
, I believe that another thread could potentially update the hash.
Maybe we could synchronize the update block by doing something like:
if (newContainerTagsHash != null) {
ContainerInfo containerInfo = ContainerInfo.get();
synchronized (containerInfo) {
String currentHash = containerInfo.getContainerTagsHash();
if (!newContainerTagsHash.equals(currentHash)) {
containerInfo.setContainerTagsHash(newContainerTagsHash);
BaseHash.recalcBaseHash(newContainerTagsHash);
}
}
}
if (primaryTag != null) { | ||
builder.append(primaryTag); | ||
} | ||
if (processTags != null) { |
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.
When processTags
is not null but containerTagsHash
is null, we still append to builder, is this intentional? Maybe we could split it up and do a proper null check:
if (processTags != null) {
builder.append(processTags);
}
if (containerTagsHash != null && !containerTagsHash.isEmpty()) {
builder.append(containerTagsHash);
}
What Does This Do
containerTagsHash
(received from the Agent) inbaseHash
calculationbaseHash
into SQLbaseHash
inDataStreamsTags
Motivation
Backpropagation of container tags. The agent calculates the hash, which is then injected into DBM queries and the DSM pathway context.
Additional Notes
The agent sends the containerTagsHash in the response header. The Java Tracer stores it in the ContainerInfo.
Contributor Checklist
type:
and (comp:
orinst:
) labels in addition to any usefull labelsclose
,fix
or any linking keywords when referencing an issue.Use
solves
instead, and assign the PR milestone to the issueJira ticket: OLDAIDM-700