-
Notifications
You must be signed in to change notification settings - Fork 312
WIP: chore(sampling): track knuth sampling in distributed trace #9518
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
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
This comment has been minimized.
This comment has been minimized.
BenchmarksStartupParameters
See matching parameters
SummaryFound 1 performance improvements and 8 performance regressions! Performance is the same for 40 metrics, 10 unstable metrics.
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.54.0-SNAPSHOT~ad8190f8ca, baseline=1.54.0-SNAPSHOT~61c2d97420
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.005 s) : 0, 1004602
Total [baseline] (8.687 s) : 0, 8687065
Agent [candidate] (1.019 s) : 0, 1018830
Total [candidate] (8.635 s) : 0, 8634794
section iast
Agent [baseline] (1.155 s) : 0, 1154628
Total [baseline] (9.257 s) : 0, 9256742
Agent [candidate] (1.155 s) : 0, 1154882
Total [candidate] (9.413 s) : 0, 9412743
gantt
title insecure-bank - break down per module: candidate=1.54.0-SNAPSHOT~ad8190f8ca, baseline=1.54.0-SNAPSHOT~61c2d97420
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.459 ms) : 0, 1459
crashtracking [candidate] (1.452 ms) : 0, 1452
BytebuddyAgent [baseline] (687.677 ms) : 0, 687677
BytebuddyAgent [candidate] (688.319 ms) : 0, 688319
GlobalTracer [baseline] (247.256 ms) : 0, 247256
GlobalTracer [candidate] (257.876 ms) : 0, 257876
AppSec [baseline] (31.111 ms) : 0, 31111
AppSec [candidate] (31.134 ms) : 0, 31134
Debugger [baseline] (6.37 ms) : 0, 6370
Debugger [candidate] (6.323 ms) : 0, 6323
Remote Config [baseline] (695.835 µs) : 0, 696
Remote Config [candidate] (664.147 µs) : 0, 664
Telemetry [baseline] (8.997 ms) : 0, 8997
Telemetry [candidate] (12.083 ms) : 0, 12083
section iast
crashtracking [baseline] (1.473 ms) : 0, 1473
crashtracking [candidate] (1.46 ms) : 0, 1460
BytebuddyAgent [baseline] (817.494 ms) : 0, 817494
BytebuddyAgent [candidate] (809.557 ms) : 0, 809557
GlobalTracer [baseline] (238.8 ms) : 0, 238800
GlobalTracer [candidate] (247.779 ms) : 0, 247779
IAST [baseline] (26.395 ms) : 0, 26395
IAST [candidate] (28.681 ms) : 0, 28681
AppSec [baseline] (34.167 ms) : 0, 34167
AppSec [candidate] (30.912 ms) : 0, 30912
Debugger [baseline] (6.055 ms) : 0, 6055
Debugger [candidate] (6.166 ms) : 0, 6166
Remote Config [baseline] (592.081 µs) : 0, 592
Remote Config [candidate] (607.479 µs) : 0, 607
Telemetry [baseline] (8.374 ms) : 0, 8374
Telemetry [candidate] (8.571 ms) : 0, 8571
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.54.0-SNAPSHOT~ad8190f8ca, baseline=1.54.0-SNAPSHOT~61c2d97420
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.012 s) : 0, 1011908
Total [baseline] (10.755 s) : 0, 10755145
Agent [candidate] (1.022 s) : 0, 1021757
Total [candidate] (10.637 s) : 0, 10636653
section appsec
Agent [baseline] (1.184 s) : 0, 1184398
Total [baseline] (11.08 s) : 0, 11080058
Agent [candidate] (1.189 s) : 0, 1189375
Total [candidate] (10.916 s) : 0, 10916335
section iast
Agent [baseline] (1.146 s) : 0, 1146072
Total [baseline] (10.999 s) : 0, 10998823
Agent [candidate] (1.168 s) : 0, 1168143
Total [candidate] (11.097 s) : 0, 11096928
section profiling
Agent [baseline] (1.152 s) : 0, 1152001
Total [baseline] (10.965 s) : 0, 10965282
Agent [candidate] (1.172 s) : 0, 1172389
Total [candidate] (11.029 s) : 0, 11029216
gantt
title petclinic - break down per module: candidate=1.54.0-SNAPSHOT~ad8190f8ca, baseline=1.54.0-SNAPSHOT~61c2d97420
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.468 ms) : 0, 1468
crashtracking [candidate] (1.454 ms) : 0, 1454
BytebuddyAgent [baseline] (692.684 ms) : 0, 692684
BytebuddyAgent [candidate] (689.431 ms) : 0, 689431
GlobalTracer [baseline] (248.948 ms) : 0, 248948
GlobalTracer [candidate] (258.305 ms) : 0, 258305
AppSec [baseline] (31.413 ms) : 0, 31413
AppSec [candidate] (31.182 ms) : 0, 31182
Debugger [baseline] (6.492 ms) : 0, 6492
Debugger [candidate] (6.363 ms) : 0, 6363
Remote Config [baseline] (706.816 µs) : 0, 707
Remote Config [candidate] (679.466 µs) : 0, 679
Telemetry [baseline] (9.085 ms) : 0, 9085
Telemetry [candidate] (13.285 ms) : 0, 13285
section appsec
crashtracking [baseline] (1.461 ms) : 0, 1461
crashtracking [candidate] (1.452 ms) : 0, 1452
BytebuddyAgent [baseline] (710.074 ms) : 0, 710074
BytebuddyAgent [candidate] (712.628 ms) : 0, 712628
GlobalTracer [baseline] (239.101 ms) : 0, 239101
GlobalTracer [candidate] (251.155 ms) : 0, 251155
IAST [baseline] (24.625 ms) : 0, 24625
IAST [candidate] (24.75 ms) : 0, 24750
AppSec [baseline] (171.572 ms) : 0, 171572
AppSec [candidate] (163.216 ms) : 0, 163216
Debugger [baseline] (7.528 ms) : 0, 7528
Debugger [candidate] (6.028 ms) : 0, 6028
Remote Config [baseline] (631.063 µs) : 0, 631
Remote Config [candidate] (642.547 µs) : 0, 643
Telemetry [baseline] (8.367 ms) : 0, 8367
Telemetry [candidate] (8.527 ms) : 0, 8527
section iast
crashtracking [baseline] (1.456 ms) : 0, 1456
crashtracking [candidate] (1.471 ms) : 0, 1471
BytebuddyAgent [baseline] (810.627 ms) : 0, 810627
BytebuddyAgent [candidate] (819.426 ms) : 0, 819426
GlobalTracer [baseline] (237.335 ms) : 0, 237335
GlobalTracer [candidate] (250.403 ms) : 0, 250403
IAST [baseline] (26.313 ms) : 0, 26313
IAST [candidate] (27.766 ms) : 0, 27766
AppSec [baseline] (34.064 ms) : 0, 34064
AppSec [candidate] (28.371 ms) : 0, 28371
Debugger [baseline] (6.02 ms) : 0, 6020
Debugger [candidate] (6.27 ms) : 0, 6270
Remote Config [baseline] (587.132 µs) : 0, 587
Remote Config [candidate] (612.683 µs) : 0, 613
Telemetry [baseline] (8.314 ms) : 0, 8314
Telemetry [candidate] (8.365 ms) : 0, 8365
section profiling
crashtracking [baseline] (1.45 ms) : 0, 1450
crashtracking [candidate] (1.441 ms) : 0, 1441
BytebuddyAgent [baseline] (718.558 ms) : 0, 718558
BytebuddyAgent [candidate] (719.405 ms) : 0, 719405
GlobalTracer [baseline] (223.686 ms) : 0, 223686
GlobalTracer [candidate] (234.669 ms) : 0, 234669
AppSec [baseline] (31.299 ms) : 0, 31299
AppSec [candidate] (31.421 ms) : 0, 31421
Debugger [baseline] (7.292 ms) : 0, 7292
Debugger [candidate] (8.197 ms) : 0, 8197
Remote Config [baseline] (717.646 µs) : 0, 718
Remote Config [candidate] (687.739 µs) : 0, 688
Telemetry [baseline] (15.607 ms) : 0, 15607
Telemetry [candidate] (15.001 ms) : 0, 15001
ProfilingAgent [baseline] (101.14 ms) : 0, 101140
ProfilingAgent [candidate] (109.109 ms) : 0, 109109
Profiling [baseline] (101.753 ms) : 0, 101753
Profiling [candidate] (109.843 ms) : 0, 109843
LoadParameters
See matching parameters
SummaryFound 3 performance improvements and 3 performance regressions! Performance is the same for 6 metrics, 12 unstable metrics.
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.54.0-SNAPSHOT~ad8190f8ca, baseline=1.54.0-SNAPSHOT~61c2d97420
dateFormat X
axisFormat %s
section baseline
no_agent (37.55 ms) : 37245, 37855
. : milestone, 37550,
appsec (48.519 ms) : 48094, 48943
. : milestone, 48519,
code_origins (45.21 ms) : 44827, 45593
. : milestone, 45210,
iast (44.989 ms) : 44602, 45376
. : milestone, 44989,
profiling (46.852 ms) : 46449, 47254
. : milestone, 46852,
tracing (46.089 ms) : 45699, 46478
. : milestone, 46089,
section candidate
no_agent (37.217 ms) : 36909, 37524
. : milestone, 37217,
appsec (50.69 ms) : 50229, 51150
. : milestone, 50690,
code_origins (43.34 ms) : 42991, 43688
. : milestone, 43340,
iast (43.347 ms) : 42958, 43735
. : milestone, 43347,
profiling (48.534 ms) : 48062, 49006
. : milestone, 48534,
tracing (44.019 ms) : 43642, 44395
. : milestone, 44019,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.54.0-SNAPSHOT~ad8190f8ca, baseline=1.54.0-SNAPSHOT~61c2d97420
dateFormat X
axisFormat %s
section baseline
no_agent (4.408 ms) : 4359, 4457
. : milestone, 4408,
iast (9.403 ms) : 9247, 9559
. : milestone, 9403,
iast_FULL (14.554 ms) : 14255, 14852
. : milestone, 14554,
iast_GLOBAL (11.319 ms) : 11116, 11522
. : milestone, 11319,
profiling (8.828 ms) : 8689, 8968
. : milestone, 8828,
tracing (7.925 ms) : 7810, 8041
. : milestone, 7925,
section candidate
no_agent (4.283 ms) : 4231, 4336
. : milestone, 4283,
iast (9.889 ms) : 9722, 10056
. : milestone, 9889,
iast_FULL (14.079 ms) : 13796, 14363
. : milestone, 14079,
iast_GLOBAL (11.208 ms) : 11002, 11415
. : milestone, 11208,
profiling (9.151 ms) : 9007, 9296
. : milestone, 9151,
tracing (7.763 ms) : 7645, 7881
. : milestone, 7763,
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 biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.54.0-SNAPSHOT~ad8190f8ca, baseline=1.54.0-SNAPSHOT~61c2d97420
dateFormat X
axisFormat %s
section baseline
no_agent (14.68 s) : 14680000, 14680000
. : milestone, 14680000,
appsec (15.005 s) : 15005000, 15005000
. : milestone, 15005000,
iast (18.912 s) : 18912000, 18912000
. : milestone, 18912000,
iast_GLOBAL (17.632 s) : 17632000, 17632000
. : milestone, 17632000,
profiling (15.194 s) : 15194000, 15194000
. : milestone, 15194000,
tracing (14.963 s) : 14963000, 14963000
. : milestone, 14963000,
section candidate
no_agent (14.953 s) : 14953000, 14953000
. : milestone, 14953000,
appsec (15.218 s) : 15218000, 15218000
. : milestone, 15218000,
iast (18.424 s) : 18424000, 18424000
. : milestone, 18424000,
iast_GLOBAL (18.22 s) : 18220000, 18220000
. : milestone, 18220000,
profiling (15.386 s) : 15386000, 15386000
. : milestone, 15386000,
tracing (15.063 s) : 15063000, 15063000
. : milestone, 15063000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.54.0-SNAPSHOT~ad8190f8ca, baseline=1.54.0-SNAPSHOT~61c2d97420
dateFormat X
axisFormat %s
section baseline
no_agent (1.479 ms) : 1467, 1491
. : milestone, 1479,
appsec (3.738 ms) : 3521, 3956
. : milestone, 3738,
iast (2.208 ms) : 2144, 2271
. : milestone, 2208,
iast_GLOBAL (2.252 ms) : 2188, 2316
. : milestone, 2252,
profiling (2.038 ms) : 1988, 2088
. : milestone, 2038,
tracing (2.011 ms) : 1962, 2061
. : milestone, 2011,
section candidate
no_agent (1.478 ms) : 1467, 1490
. : milestone, 1478,
appsec (3.717 ms) : 3502, 3932
. : milestone, 3717,
iast (2.208 ms) : 2145, 2271
. : milestone, 2208,
iast_GLOBAL (2.247 ms) : 2184, 2310
. : milestone, 2247,
profiling (2.07 ms) : 2017, 2122
. : milestone, 2070,
tracing (2.029 ms) : 1979, 2078
. : milestone, 2029,
|
return span.getTag("env", ""); | ||
} | ||
|
||
private String formatKnuthSamplingRate(double rate) { |
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.
Should centralize this logic. Currently we define this function in two places
|
||
private String formatKnuthSamplingRate(double rate) { | ||
// Format to up to 6 decimal places, removing trailing zeros | ||
return DECIMAL_FORMAT.format(rate); |
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.
Quick heads-up that java.text.DecimalFormat.format(...)
is not thread-safe: https://docs.oracle.com/javase/8/docs/api/java/text/DecimalFormat.html#synchronization
But adding synchronization here would kill performance....
One option could be to write our own simple formatter, since we just want to truncate the value - for example, if we assume the value is between 0 and 1, a naive solution might be:
private static String example(double value) {
if (value <= 0) {
return "0";
} else if (value >= 1) {
return "1";
} else {
return "0." + (int) (value * 1_000_000 + 0.5);
}
}
( you'd need to write tests to verify this behaves as expected - but you'd need to do that anyway :)
Also note you don't need to do the formatting every time the sampling rate is applied because each RateSampler
is deterministic: https://github.com/DataDog/dd-trace-java/blob/master/dd-trace-core/src/main/java/datadog/trace/common/sampling/DeterministicSampler.java#L41
In other words, the sample rate for each individual RateSampler
is fixed at construct time - so you could add a String getKnuthSampleRate()
or similar method to RateSampler
and then do the formatting once in the DeterministicSampler
constructor and save it as a String
field alongside the rate
, at which point setting the tag just involves calling getKnuthSampleRate()
to get the pre-formatted string.
What Does This Do
Motivation
Additional Notes
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: [PROJ-IDENT]