Skip to content

Adding InitializationTelemetry - e.g. guard rails reporting #7287

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.

Sign up for GitHub

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

Jump to bottom
Merged
dougqh merged 63 commits into from
Aug 23, 2024
Merged

Adding InitializationTelemetry - e.g. guard rails reporting #7287

dougqh merged 63 commits into from
Aug 23, 2024

Conversation

@dougqh
Copy link
Contributor

@dougqh dougqh commented Jul 5, 2024 •
edited by atlassian bot
Loading

What Does This Do

Adding InitializationTelemetry for use by single step

Motivation

BootstrapInitializationTelemetry & InitializationTelemetry are used to report guard rail check failures via a helper process installed by single step

Additional Notes

Most of the real changes are in AgentBootstrap with a lot of attention towards working with restrictive SecurityManagers. With the most restrictive SecurityManagers access to env, properties, and process execution can be blocked.

To make dealing with SecurityExceptions less ugly, I've introduced a SystemUtils that helps with getProperty and getEnv.
But in many places, SecurityExceptions are still checked directly.

AgentBootstrap uses BootstrapInitializationTelemetry which records the gathered information into JSON buffers in memory and then sends to the helper process on finish.

For constructing the JSON that goes to the helper process, we don't want to introduce dependencies into the bootstrap. So I've implemented a small JsonBuffer class modeled loosely after GSON's JsonWriter.

To provide telemetry inside Agent, Agent is passed a BootstrapInitializationTelemetry by AgentBootstrap. The BootstrapInitializationTelemetry is wrapped by an InitializationTelemetry that uses MethodHandles to cross the ClassLoader divide.

APMLP-109

dougqh added 4 commits June 12, 2024 09:32
@dougqh
Relocating checks into try
e37b454 
Under an unreasonably strict SecurityManager, even reading core properties can be blocked, so moving checks into the try.
@dougqh
Refactoring if statement into multiple statements
e31388e 
Auto-injection telemetry requires different notifications for each case
@dougqh
Sketching out structure of initialization telemetry
a6e86c6 
Committing early, so I can share the branch
@dougqh
Adding "InitializationTelemetry"
65d71d7 
Adding InitializationTelemetry to the bootstrap

InitializationTelemetry is used to report guard rail check failures via a helper process installed by single step

Most of the real changes are in AgentBootstrap with a lot of attention towards working with restrictive SecurityManagers.  With the most restrictive SecurityManagers access to env & properties are blocked, so SecurityPermission & Throwable checks are usually liberally throughout.

For constructing the JSON that goes to the helper process, we don't want to introduce dependencies into the bootstrap.  So I've implemented a small JsonBuffer class modeled loosely after GSON's JsonWriter.
@dougqh dougqh requested a review from a team as a code owner July 5, 2024 20:24
@dougqh dougqh requested review from PerfectSlayer and ygree July 5, 2024 20:24
dougqh
dougqh commented Jul 5, 2024
View reviewed changes
dougqh
dougqh commented Jul 5, 2024
View reviewed changes
dougqh
dougqh commented Jul 5, 2024
View reviewed changes
dougqh
dougqh commented Jul 5, 2024
View reviewed changes
@dougqh dougqh marked this pull request as draft July 5, 2024 20:28
@pr-commenter
Copy link

pr-commenter bot commented Jul 5, 2024 •
edited
Loading

Benchmarks

Startup

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master dougqh/guardrails-telemetry
git_commit_date 1724422468 1724424575
git_commit_sha a4f73e2 e1e17fc
release_version 1.39.0-SNAPSHOT~a4f73e2ed0 1.39.0-SNAPSHOT~e1e17fca28
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1724426829 1724426829
ci_job_id 615401460 615401460
ci_pipeline_id 42691858 42691858
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
module Agent Agent
parent None None
variant iast iast

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 48 metrics, 15 unstable metrics.

Startup time reports for insecure-bank 
gantt
    title insecure-bank - global startup overhead: candidate=1.39.0-SNAPSHOT~e1e17fca28, baseline=1.39.0-SNAPSHOT~a4f73e2ed0

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.048 s) : 0, 1048480
Total [baseline] (8.514 s) : 0, 8514043
Agent [candidate] (1.045 s) : 0, 1045417
Total [candidate] (8.541 s) : 0, 8540898
section iast
Agent [baseline] (1.171 s) : 0, 1171235
Total [baseline] (8.961 s) : 0, 8960865
Agent [candidate] (1.176 s) : 0, 1175992
Total [candidate] (8.973 s) : 0, 8972714
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.178 s) : 0, 1178384
Total [baseline] (8.983 s) : 0, 8982520
Agent [candidate] (1.17 s) : 0, 1170064
Total [candidate] (8.929 s) : 0, 8929213
section iast_TELEMETRY_OFF
Agent [baseline] (1.176 s) : 0, 1176264
Total [baseline] (8.975 s) : 0, 8975043
Agent [candidate] (1.177 s) : 0, 1176895
Total [candidate] (9.0 s) : 0, 9000272
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.048 s -
Agent iast 1.171 s 122.755 ms (11.7%)
Agent iast_HARDCODED_SECRET_DISABLED 1.178 s 129.905 ms (12.4%)
Agent iast_TELEMETRY_OFF 1.176 s 127.784 ms (12.2%)
Total tracing 8.514 s -
Total iast 8.961 s 446.822 ms (5.2%)
Total iast_HARDCODED_SECRET_DISABLED 8.983 s 468.477 ms (5.5%)
Total iast_TELEMETRY_OFF 8.975 s 461.0 ms (5.4%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.045 s -
Agent iast 1.176 s 130.575 ms (12.5%)
Agent iast_HARDCODED_SECRET_DISABLED 1.17 s 124.647 ms (11.9%)
Agent iast_TELEMETRY_OFF 1.177 s 131.479 ms (12.6%)
Total tracing 8.541 s -
Total iast 8.973 s 431.816 ms (5.1%)
Total iast_HARDCODED_SECRET_DISABLED 8.929 s 388.315 ms (4.5%)
Total iast_TELEMETRY_OFF 9.0 s 459.374 ms (5.4%) 
gantt
    title insecure-bank - break down per module: candidate=1.39.0-SNAPSHOT~e1e17fca28, baseline=1.39.0-SNAPSHOT~a4f73e2ed0

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (669.346 ms) : 0, 669346
BytebuddyAgent [candidate] (666.905 ms) : 0, 666905
GlobalTracer [baseline] (305.893 ms) : 0, 305893
GlobalTracer [candidate] (305.693 ms) : 0, 305693
AppSec [baseline] (51.512 ms) : 0, 51512
AppSec [candidate] (51.303 ms) : 0, 51303
Remote Config [baseline] (673.054 µs) : 0, 673
Remote Config [candidate] (660.96 µs) : 0, 661
Telemetry [baseline] (7.608 ms) : 0, 7608
Telemetry [candidate] (7.471 ms) : 0, 7471
section iast
BytebuddyAgent [baseline] (778.347 ms) : 0, 778347
BytebuddyAgent [candidate] (780.597 ms) : 0, 780597
GlobalTracer [baseline] (295.415 ms) : 0, 295415
GlobalTracer [candidate] (296.917 ms) : 0, 296917
AppSec [baseline] (51.277 ms) : 0, 51277
AppSec [candidate] (49.24 ms) : 0, 49240
IAST [baseline] (22.594 ms) : 0, 22594
IAST [candidate] (27.243 ms) : 0, 27243
Remote Config [baseline] (599.418 µs) : 0, 599
Remote Config [candidate] (588.972 µs) : 0, 589
Telemetry [baseline] (9.566 ms) : 0, 9566
Telemetry [candidate] (7.919 ms) : 0, 7919
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (782.48 ms) : 0, 782480
BytebuddyAgent [candidate] (777.8 ms) : 0, 777800
GlobalTracer [baseline] (297.673 ms) : 0, 297673
GlobalTracer [candidate] (296.289 ms) : 0, 296289
AppSec [baseline] (50.047 ms) : 0, 50047
AppSec [candidate] (48.261 ms) : 0, 48261
IAST [baseline] (22.075 ms) : 0, 22075
IAST [candidate] (24.909 ms) : 0, 24909
Remote Config [baseline] (610.774 µs) : 0, 611
Remote Config [candidate] (598.671 µs) : 0, 599
Telemetry [baseline] (11.958 ms) : 0, 11958
Telemetry [candidate] (8.728 ms) : 0, 8728
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (781.797 ms) : 0, 781797
BytebuddyAgent [candidate] (780.698 ms) : 0, 780698
GlobalTracer [baseline] (298.824 ms) : 0, 298824
GlobalTracer [candidate] (298.18 ms) : 0, 298180
AppSec [baseline] (49.579 ms) : 0, 49579
AppSec [candidate] (52.02 ms) : 0, 52020
IAST [baseline] (23.216 ms) : 0, 23216
IAST [candidate] (23.201 ms) : 0, 23201
Remote Config [baseline] (586.7 µs) : 0, 587
Remote Config [candidate] (599.911 µs) : 0, 600
Telemetry [baseline] (8.72 ms) : 0, 8720
Telemetry [candidate] (8.641 ms) : 0, 8641
Loading
Startup time reports for petclinic 
gantt
    title petclinic - global startup overhead: candidate=1.39.0-SNAPSHOT~e1e17fca28, baseline=1.39.0-SNAPSHOT~a4f73e2ed0

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.056 s) : 0, 1055895
Total [baseline] (10.364 s) : 0, 10364411
Agent [candidate] (1.056 s) : 0, 1055750
Total [candidate] (10.404 s) : 0, 10404176
section appsec
Agent [baseline] (1.171 s) : 0, 1170596
Total [baseline] (10.421 s) : 0, 10421251
Agent [candidate] (1.172 s) : 0, 1172178
Total [candidate] (10.497 s) : 0, 10497078
section iast
Agent [baseline] (1.179 s) : 0, 1179248
Total [baseline] (10.842 s) : 0, 10841674
Agent [candidate] (1.181 s) : 0, 1181425
Total [candidate] (10.874 s) : 0, 10873508
section profiling
Agent [baseline] (1.261 s) : 0, 1260683
Total [baseline] (10.66 s) : 0, 10660170
Agent [candidate] (1.258 s) : 0, 1257614
Total [candidate] (10.666 s) : 0, 10665634
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.056 s -
Agent appsec 1.171 s 114.701 ms (10.9%)
Agent iast 1.179 s 123.353 ms (11.7%)
Agent profiling 1.261 s 204.788 ms (19.4%)
Total tracing 10.364 s -
Total appsec 10.421 s 56.84 ms (0.5%)
Total iast 10.842 s 477.263 ms (4.6%)
Total profiling 10.66 s 295.759 ms (2.9%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.056 s -
Agent appsec 1.172 s 116.428 ms (11.0%)
Agent iast 1.181 s 125.675 ms (11.9%)
Agent profiling 1.258 s 201.864 ms (19.1%)
Total tracing 10.404 s -
Total appsec 10.497 s 92.901 ms (0.9%)
Total iast 10.874 s 469.332 ms (4.5%)
Total profiling 10.666 s 261.457 ms (2.5%) 
gantt
    title petclinic - break down per module: candidate=1.39.0-SNAPSHOT~e1e17fca28, baseline=1.39.0-SNAPSHOT~a4f73e2ed0

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (674.17 ms) : 0, 674170
BytebuddyAgent [candidate] (674.01 ms) : 0, 674010
GlobalTracer [baseline] (308.362 ms) : 0, 308362
GlobalTracer [candidate] (308.659 ms) : 0, 308659
AppSec [baseline] (51.66 ms) : 0, 51660
AppSec [candidate] (51.472 ms) : 0, 51472
Remote Config [baseline] (669.678 µs) : 0, 670
Remote Config [candidate] (677.575 µs) : 0, 678
Telemetry [baseline] (7.518 ms) : 0, 7518
Telemetry [candidate] (7.402 ms) : 0, 7402
section appsec
BytebuddyAgent [baseline] (678.718 ms) : 0, 678718
BytebuddyAgent [candidate] (680.933 ms) : 0, 680933
GlobalTracer [baseline] (300.059 ms) : 0, 300059
GlobalTracer [candidate] (300.875 ms) : 0, 300875
AppSec [baseline] (157.098 ms) : 0, 157098
AppSec [candidate] (158.659 ms) : 0, 158659
Remote Config [baseline] (607.48 µs) : 0, 607
Remote Config [candidate] (623.333 µs) : 0, 623
Telemetry [baseline] (9.935 ms) : 0, 9935
Telemetry [candidate] (8.414 ms) : 0, 8414
IAST [baseline] (21.498 ms) : 0, 21498
IAST [candidate] (18.29 ms) : 0, 18290
section iast
BytebuddyAgent [baseline] (783.275 ms) : 0, 783275
BytebuddyAgent [candidate] (784.048 ms) : 0, 784048
GlobalTracer [baseline] (297.429 ms) : 0, 297429
GlobalTracer [candidate] (298.447 ms) : 0, 298447
AppSec [baseline] (49.982 ms) : 0, 49982
AppSec [candidate] (53.162 ms) : 0, 53162
Remote Config [baseline] (572.185 µs) : 0, 572
Remote Config [candidate] (594.997 µs) : 0, 595
Telemetry [baseline] (9.422 ms) : 0, 9422
Telemetry [candidate] (8.961 ms) : 0, 8961
IAST [baseline] (25.038 ms) : 0, 25038
IAST [candidate] (22.63 ms) : 0, 22630
section profiling
ProfilingAgent [baseline] (95.267 ms) : 0, 95267
ProfilingAgent [candidate] (97.177 ms) : 0, 97177
BytebuddyAgent [baseline] (673.612 ms) : 0, 673612
BytebuddyAgent [candidate] (668.706 ms) : 0, 668706
GlobalTracer [baseline] (392.794 ms) : 0, 392794
GlobalTracer [candidate] (393.162 ms) : 0, 393162
AppSec [baseline] (53.178 ms) : 0, 53178
AppSec [candidate] (52.892 ms) : 0, 52892
Remote Config [baseline] (678.729 µs) : 0, 679
Remote Config [candidate] (692.804 µs) : 0, 693
Telemetry [baseline] (7.503 ms) : 0, 7503
Telemetry [candidate] (7.423 ms) : 0, 7423
Profiling [baseline] (95.291 ms) : 0, 95291
Profiling [candidate] (97.201 ms) : 0, 97201
Loading

Load

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
end_time 2024-08-23T14:57:55 2024-08-23T15:04:45
git_branch master dougqh/guardrails-telemetry
git_commit_date 1724422468 1724424575
git_commit_sha a4f73e2 e1e17fc
release_version 1.39.0-SNAPSHOT~a4f73e2ed0 1.39.0-SNAPSHOT~e1e17fca28
start_time 2024-08-23T14:57:42 2024-08-23T15:04:31
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1724425830 1724425830
ci_job_id 615401461 615401461
ci_pipeline_id 42691858 42691858
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
variant iast iast

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 10 metrics, 18 unstable metrics.

Request duration reports for insecure-bank 
gantt
    title insecure-bank - request duration [CI 0.99] : candidate=1.39.0-SNAPSHOT~e1e17fca28, baseline=1.39.0-SNAPSHOT~a4f73e2ed0
    dateFormat X
    axisFormat %s
section baseline
no_agent (369.022 µs) : 349, 389
.   : milestone, 369,
iast (484.788 µs) : 462, 508
.   : milestone, 485,
iast_FULL (554.528 µs) : 533, 577
.   : milestone, 555,
iast_GLOBAL (506.438 µs) : 484, 529
.   : milestone, 506,
iast_HARDCODED_SECRET_DISABLED (494.913 µs) : 472, 518
.   : milestone, 495,
iast_INACTIVE (443.625 µs) : 423, 464
.   : milestone, 444,
iast_TELEMETRY_OFF (476.454 µs) : 454, 498
.   : milestone, 476,
tracing (445.267 µs) : 425, 466
.   : milestone, 445,
section candidate
no_agent (368.696 µs) : 349, 389
.   : milestone, 369,
iast (486.704 µs) : 464, 509
.   : milestone, 487,
iast_FULL (546.537 µs) : 525, 568
.   : milestone, 547,
iast_GLOBAL (521.291 µs) : 499, 544
.   : milestone, 521,
iast_HARDCODED_SECRET_DISABLED (480.521 µs) : 459, 502
.   : milestone, 481,
iast_INACTIVE (444.036 µs) : 423, 465
.   : milestone, 444,
iast_TELEMETRY_OFF (475.523 µs) : 453, 498
.   : milestone, 476,
tracing (439.437 µs) : 419, 460
.   : milestone, 439,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 369.022 µs [349.082 µs, 388.962 µs] -
iast 484.788 µs [462.055 µs, 507.521 µs] 115.766 µs (31.4%)
iast_FULL 554.528 µs [532.535 µs, 576.521 µs] 185.506 µs (50.3%)
iast_GLOBAL 506.438 µs [484.292 µs, 528.583 µs] 137.416 µs (37.2%)
iast_HARDCODED_SECRET_DISABLED 494.913 µs [472.055 µs, 517.77 µs] 125.891 µs (34.1%)
iast_INACTIVE 443.625 µs [423.078 µs, 464.172 µs] 74.603 µs (20.2%)
iast_TELEMETRY_OFF 476.454 µs [454.413 µs, 498.495 µs] 107.432 µs (29.1%)
tracing 445.267 µs [424.999 µs, 465.536 µs] 76.246 µs (20.7%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 368.696 µs [348.645 µs, 388.747 µs] -
iast 486.704 µs [464.222 µs, 509.186 µs] 118.007 µs (32.0%)
iast_FULL 546.537 µs [525.232 µs, 567.842 µs] 177.84 µs (48.2%)
iast_GLOBAL 521.291 µs [499.047 µs, 543.536 µs] 152.595 µs (41.4%)
iast_HARDCODED_SECRET_DISABLED 480.521 µs [459.059 µs, 501.983 µs] 111.824 µs (30.3%)
iast_INACTIVE 444.036 µs [423.103 µs, 464.97 µs] 75.34 µs (20.4%)
iast_TELEMETRY_OFF 475.523 µs [452.703 µs, 498.343 µs] 106.826 µs (29.0%)
tracing 439.437 µs [418.736 µs, 460.139 µs] 70.741 µs (19.2%)
Request duration reports for petclinic 
gantt
    title petclinic - request duration [CI 0.99] : candidate=1.39.0-SNAPSHOT~e1e17fca28, baseline=1.39.0-SNAPSHOT~a4f73e2ed0
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.343 ms) : 1324, 1362
.   : milestone, 1343,
appsec (1.715 ms) : 1690, 1739
.   : milestone, 1715,
appsec_no_iast (1.704 ms) : 1679, 1729
.   : milestone, 1704,
iast (1.48 ms) : 1457, 1503
.   : milestone, 1480,
profiling (1.539 ms) : 1513, 1565
.   : milestone, 1539,
tracing (1.476 ms) : 1452, 1500
.   : milestone, 1476,
section candidate
no_agent (1.353 ms) : 1332, 1373
.   : milestone, 1353,
appsec (1.74 ms) : 1716, 1763
.   : milestone, 1740,
appsec_no_iast (1.713 ms) : 1688, 1739
.   : milestone, 1713,
iast (1.485 ms) : 1462, 1508
.   : milestone, 1485,
profiling (1.53 ms) : 1505, 1555
.   : milestone, 1530,
tracing (1.466 ms) : 1442, 1490
.   : milestone, 1466,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.343 ms [1.324 ms, 1.362 ms] -
appsec 1.715 ms [1.69 ms, 1.739 ms] 371.953 µs (27.7%)
appsec_no_iast 1.704 ms [1.679 ms, 1.729 ms] 360.945 µs (26.9%)
iast 1.48 ms [1.457 ms, 1.503 ms] 137.133 µs (10.2%)
profiling 1.539 ms [1.513 ms, 1.565 ms] 196.117 µs (14.6%)
tracing 1.476 ms [1.452 ms, 1.5 ms] 133.284 µs (9.9%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.353 ms [1.332 ms, 1.373 ms] -
appsec 1.74 ms [1.716 ms, 1.763 ms] 387.15 µs (28.6%)
appsec_no_iast 1.713 ms [1.688 ms, 1.739 ms] 360.483 µs (26.7%)
iast 1.485 ms [1.462 ms, 1.508 ms] 132.617 µs (9.8%)
profiling 1.53 ms [1.505 ms, 1.555 ms] 177.686 µs (13.1%)
tracing 1.466 ms [1.442 ms, 1.49 ms] 113.272 µs (8.4%)

Dacapo

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master dougqh/guardrails-telemetry
git_commit_date 1724422468 1724424575
git_commit_sha a4f73e2 e1e17fc
release_version 1.39.0-SNAPSHOT~a4f73e2ed0 1.39.0-SNAPSHOT~e1e17fca28
See matching parameters
Baseline Candidate
application biojava biojava
ci_job_date 1724426342 1724426342
ci_job_id 615401462 615401462
ci_pipeline_id 42691858 42691858
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
variant appsec appsec

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 0 unstable metrics.

Execution time for biojava 
gantt
    title biojava - execution time [CI 0.99] : candidate=1.39.0-SNAPSHOT~e1e17fca28, baseline=1.39.0-SNAPSHOT~a4f73e2ed0
    dateFormat X
    axisFormat %s
section baseline
no_agent (15.447 s) : 15447000, 15447000
.   : milestone, 15447000,
appsec (14.919 s) : 14919000, 14919000
.   : milestone, 14919000,
iast (18.601 s) : 18601000, 18601000
.   : milestone, 18601000,
iast_GLOBAL (17.909 s) : 17909000, 17909000
.   : milestone, 17909000,
profiling (15.426 s) : 15426000, 15426000
.   : milestone, 15426000,
tracing (14.922 s) : 14922000, 14922000
.   : milestone, 14922000,
section candidate
no_agent (15.43 s) : 15430000, 15430000
.   : milestone, 15430000,
appsec (14.902 s) : 14902000, 14902000
.   : milestone, 14902000,
iast (18.993 s) : 18993000, 18993000
.   : milestone, 18993000,
iast_GLOBAL (17.733 s) : 17733000, 17733000
.   : milestone, 17733000,
profiling (15.105 s) : 15105000, 15105000
.   : milestone, 15105000,
tracing (14.968 s) : 14968000, 14968000
.   : milestone, 14968000,
Loading
  • baseline results
Variant Execution Time [CI 0.99] Δ no_agent
no_agent 15.447 s [15.447 s, 15.447 s] -
appsec 14.919 s [14.919 s, 14.919 s] -528.0 ms (-3.4%)
iast 18.601 s [18.601 s, 18.601 s] 3.154 s (20.4%)
iast_GLOBAL 17.909 s [17.909 s, 17.909 s] 2.462 s (15.9%)
profiling 15.426 s [15.426 s, 15.426 s] -21.0 ms (-0.1%)
tracing 14.922 s [14.922 s, 14.922 s] -525.0 ms (-3.4%)
  • candidate results
Variant Execution Time [CI 0.99] Δ no_agent
no_agent 15.43 s [15.43 s, 15.43 s] -
appsec 14.902 s [14.902 s, 14.902 s] -528.0 ms (-3.4%)
iast 18.993 s [18.993 s, 18.993 s] 3.563 s (23.1%)
iast_GLOBAL 17.733 s [17.733 s, 17.733 s] 2.303 s (14.9%)
profiling 15.105 s [15.105 s, 15.105 s] -325.0 ms (-2.1%)
tracing 14.968 s [14.968 s, 14.968 s] -462.0 ms (-3.0%)
Execution time for tomcat 
gantt
    title tomcat - execution time [CI 0.99] : candidate=1.39.0-SNAPSHOT~e1e17fca28, baseline=1.39.0-SNAPSHOT~a4f73e2ed0
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.466 ms) : 1455, 1478
.   : milestone, 1466,
appsec (2.225 ms) : 2190, 2260
.   : milestone, 2225,
iast (1.991 ms) : 1948, 2034
.   : milestone, 1991,
iast_GLOBAL (2.032 ms) : 1988, 2076
.   : milestone, 2032,
profiling (1.882 ms) : 1846, 1918
.   : milestone, 1882,
tracing (1.841 ms) : 1808, 1873
.   : milestone, 1841,
section candidate
no_agent (1.462 ms) : 1451, 1474
.   : milestone, 1462,
appsec (2.236 ms) : 2200, 2271
.   : milestone, 2236,
iast (1.99 ms) : 1947, 2033
.   : milestone, 1990,
iast_GLOBAL (2.033 ms) : 1989, 2077
.   : milestone, 2033,
profiling (1.864 ms) : 1830, 1899
.   : milestone, 1864,
tracing (1.847 ms) : 1814, 1880
.   : milestone, 1847,
Loading
  • baseline results
Variant Execution Time [CI 0.99] Δ no_agent
no_agent 1.466 ms [1.455 ms, 1.478 ms] -
appsec 2.225 ms [2.19 ms, 2.26 ms] 758.588 µs (51.7%)
iast 1.991 ms [1.948 ms, 2.034 ms] 524.42 µs (35.8%)
iast_GLOBAL 2.032 ms [1.988 ms, 2.076 ms] 565.444 µs (38.6%)
profiling 1.882 ms [1.846 ms, 1.918 ms] 415.682 µs (28.3%)
tracing 1.841 ms [1.808 ms, 1.873 ms] 374.229 µs (25.5%)
  • candidate results
Variant Execution Time [CI 0.99] Δ no_agent
no_agent 1.462 ms [1.451 ms, 1.474 ms] -
appsec 2.236 ms [2.2 ms, 2.271 ms] 773.233 µs (52.9%)
iast 1.99 ms [1.947 ms, 2.033 ms] 527.843 µs (36.1%)
iast_GLOBAL 2.033 ms [1.989 ms, 2.077 ms] 570.937 µs (39.0%)
profiling 1.864 ms [1.83 ms, 1.899 ms] 402.145 µs (27.5%)
tracing 1.847 ms [1.814 ms, 1.88 ms] 384.718 µs (26.3%)

dougqh added 17 commits July 9, 2024 13:34
@dougqh
spotless silliness
1ea0d0d
@dougqh
Fixing resource leak in IntegrationTestUtils
0f65b9f
@dougqh
Misc clean-up to improve test readability
122bf2b 
Adding methods that return File instead of URL
- avoids getting path then converting back to a File to delete

Added convenience method that takes File for classpath - avoids conversions
@dougqh
spotless
7cc2251
@dougqh
Introducing utilities to help with exception handling
b60ad48 
Added SystemUtils which has getPropertyOrDefault, tryGetProperty, getEnvOrDefault, tryGetEnv

These helpers handled SecurityException and return null or default value.

Similarly added tryGetVersion and getVersionOrDefault to AgentJar
@dougqh
Adding SecurityManager tests
22e5542 
Adding tests to make sure that AgentBootstrap doesn't fail even with a very strict SecurityManager

These tests are implemented via SecurityManagerCheck which is a simple main designed to be used with a javaagent and a TestSecurityManager

TestSecurityManager is a SecurityManager with a minimal set of permissions for the javaagent to run normally

TestSecurityManager has various sub-classes that restrict access to different things used by the javaagent.  The aim is to make that under no circumstance does the javaagent fail in a way that prevents the program from running normally.

More tests to follow
@dougqh
Added no network access test
d75864f
@dougqh
tryGetProperty -> tryGetEnv
b4623d6 
Fixing oversight in telemetry forwarder code - caught by adding more tests
@dougqh
spotless
2453d4d
@dougqh
Adding escaping of \
60c7981 
I neglected to escape \ previously.
@dougqh
Adding toString for ease of debugging + spotless
ca42c53
@dougqh
Adding nonProxyHosts
405ca3c
@dougqh
Starting to fully work
34e3781 
Rounded out InitializationTelemetryTest to cover...
- complete failure case
- normal start-up case
- different failure cases - both complete & incomplete
...including unable to send telemetry because the forwarder is blocked

Fixed problem with major failure inside Agent being improperly marked as complete
@dougqh
Pushing InitializationTelemetry into Agent & AgentCLI
c0245ae 
Pushing InitializationTelemetry down into Agent & AgentCLI
- introduced InitializationTelemetry next to Agent & AgentCLI
  - wraps the bootstrap initialization telemetry via MethodHandles
- changed InitializationTelemetry to BootstrapInitializationTelemetry to disambiguate
- altered (Bootstrap)InitializationTelemetry to track completion status
  - allows callers or InitializationTelemetry to indicate completeness without having to return status everywhere
  - removed prior returning of booleans in favor of passing (Bootstrap)InitializationTelemetry through to various init routines
@dougqh
Added a bit more header doc
6b5676a
@dougqh
Merge remote-tracking branch 'origin/master' into dougqh/guardrails-t…
9cc6da6 
…elemetry
@dougqh
none -> noneInstance
b5ae75a
PerfectSlayer
PerfectSlayer reviewed Aug 19, 2024
View reviewed changes
Copy link
Contributor

@PerfectSlayer PerfectSlayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is an issue with telemetry initialization

...ava-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/InitializationTelemetry.java
Comment on lines 17 to 21
if (bootstrapInitTelemetry == null) {
return new BootstrapProxy(bootstrapInitTelemetry);
} else {
return InitializationTelemetry.noneInstance();
}
Copy link
Contributor

@PerfectSlayer PerfectSlayer Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to invert the if cases. BootstrapProxy is always created with a null instance.

Copy link
Contributor Author

@dougqh dougqh Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch - I need to figure out a way to test this better, too.

...ava-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/InitializationTelemetry.java
*/
public abstract class InitializationTelemetry {
/** Returns a proxy around a BoostrapInitializationTelemetry object */
public static final InitializationTelemetry proxy(Object bootstrapInitTelemetry) {
Copy link
Contributor

@PerfectSlayer PerfectSlayer Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public static final InitializationTelemetry proxy(Object bootstrapInitTelemetry) {
public static InitializationTelemetry proxy(Object bootstrapInitTelemetry) {

Copy link
Contributor Author

@dougqh dougqh Aug 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to keep final almost everywhere, but I decided to remove the final where it is redundant because the class is final.

In this case, the class isn't final. So I'm keeping the final because I don't want someone to override this method on a child class.

...ava-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/InitializationTelemetry.java Outdated
}

/** Returns a singleton of the none InitializationTelemetry */
public static final InitializationTelemetry noneInstance() {
Copy link
Contributor

@PerfectSlayer PerfectSlayer Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public static final InitializationTelemetry noneInstance() {
public static InitializationTelemetry noneInstance() {

Copy link
Contributor

@PerfectSlayer PerfectSlayer Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually have noop rather than none.

Copy link
Contributor Author

@dougqh dougqh Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay

dd-java-agent/src/main/java/datadog/trace/bootstrap/AgentBootstrap.java Outdated
final BootstrapInitializationTelemetry initTelemetry,
final String agentArgs,
final Instrumentation inst)
throws IOException, URISyntaxException, ClassNotFoundException, NoSuchMethodException,
Copy link
Contributor

@PerfectSlayer PerfectSlayer Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ClassNotFoundException seems never thrown

Copy link
Contributor Author

@dougqh dougqh Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cleaned up the throws clause. I removed ClassNotFoundException and switched to ReflectiveOperationException.

I'm not really sure what I want this clause to be. I let the refactor define it, but I still feel it is a bit messy.

dd-java-agent/src/main/java/datadog/trace/bootstrap/BootstrapInitializationTelemetry.java Outdated

/** Telemetry class used to relay information about tracer activation. */
public abstract class BootstrapInitializationTelemetry {
public static final BootstrapInitializationTelemetry noneInstance() {
Copy link
Contributor

@PerfectSlayer PerfectSlayer Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public static final BootstrapInitializationTelemetry noneInstance() {
public static BootstrapInitializationTelemetry noneInstance() {

dd-java-agent/src/main/java/datadog/trace/bootstrap/SystemUtils.java Outdated
public class SystemUtils {
private SystemUtils() {}

public static final String tryGetEnv(String envVar) {
Copy link
Contributor

@PerfectSlayer PerfectSlayer Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public static final String tryGetEnv(String envVar) {
public static String tryGetEnv(String envVar) {

dd-java-agent/src/main/java/datadog/trace/bootstrap/SystemUtils.java Outdated
return getEnvOrDefault(envVar, null);
}

public static final String getEnvOrDefault(String envVar, String defaultValue) {
Copy link
Contributor

@PerfectSlayer PerfectSlayer Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public static final String getEnvOrDefault(String envVar, String defaultValue) {
public static String getEnvOrDefault(String envVar, String defaultValue) {

dd-java-agent/src/main/java/datadog/trace/bootstrap/SystemUtils.java Outdated
}
}

public static final String tryGetProperty(String property) {
Copy link
Contributor

@PerfectSlayer PerfectSlayer Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public static final String tryGetProperty(String property) {
public static String tryGetProperty(String property) {

dd-java-agent/src/main/java/datadog/trace/bootstrap/SystemUtils.java Outdated
}
}

public static final String getPropertyOrDefault(String property, String defaultValue) {
Copy link
Contributor

@PerfectSlayer PerfectSlayer Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public static final String getPropertyOrDefault(String property, String defaultValue) {
public static String getPropertyOrDefault(String property, String defaultValue) {

dougqh added 6 commits August 21, 2024 15:25
@dougqh
Refactor to introduce Result class
08ffeb7 
Introducing Result class to replace just returning exitCode
In subsequent change, Result class will also provide the forwarded JSON telemetry information
@dougqh
Fixing issue - forwarder wasn't flushing the JSON
982282f
@dougqh
Completing the integration tests
651b125 
InitializationTelemetryCheck now sets up a forwarder script that captures the output to a file

That output is then placed into the Result object in the new field telemetryJson

InitializationTelemetryTest has update to spot check the results of the Result.telemetryJson
@dougqh
Merge branch 'master' into dougqh/guardrails-telemetry
01dccb0
@dougqh
spotless
22f9192
@dougqh
Merge branch 'master' into dougqh/guardrails-telemetry
e1e17fc
@dougqh dougqh merged commit 14a80e9 into master Aug 23, 2024
@dougqh dougqh deleted the dougqh/guardrails-telemetry branch August 23, 2024 20:14
@github-actions github-actions bot added this to the 1.39.0 milestone Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcculls mcculls mcculls left review comments

@PerfectSlayer PerfectSlayer PerfectSlayer approved these changes

Assignees

No one assigned

Labels

comp: telemetry Telemetry

Projects

None yet

Milestone

1.39.0

Development

Successfully merging this pull request may close these issues.

5 participants

@dougqh @mcculls @PerfectSlayer @nikita-tkachenko-datadog