Skip to content
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

Refactor Java concurrency instrumentations #7101

Merged
merged 5 commits into from
Jul 9, 2024
Merged

Refactor Java concurrency instrumentations #7101

merged 5 commits into from
Jul 9, 2024

Conversation

PerfectSlayer
Copy link
Contributor

What Does This Do

This PR refactors JDK concurrency instrumentations by:

  • Creating dedicated package per instrumented features (instrumentations and tests),
  • Unifying instrumentation names (no breaking change, only moving to common constants),
  • Moving Java classes to java source file folder

Motivation

Try to split instrumentations by concern when they are not tangled to each other.

Additional Notes

Jira ticket: [PROJ-IDENT]

@PerfectSlayer PerfectSlayer added tag: no release notes Changes to exclude from release notes type: refactoring inst: java Core Java language instrumentation labels May 30, 2024
@PerfectSlayer PerfectSlayer requested a review from a team as a code owner May 30, 2024 16:19
@pr-commenter
Copy link

pr-commenter bot commented May 31, 2024 •
edited
Loading

Benchmarks

Startup

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master bbujon/loom
git_commit_date 1720097486 1720100320
git_commit_sha b792b2d 5e7a05d
release_version 1.38.0-SNAPSHOT~b792b2d352 1.36.0-SNAPSHOT~5e7a05dfbb
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1720103403 1720103403
ci_job_id 562600864 562600864
ci_pipeline_id 38379911 38379911
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 1 performance regressions! Performance is the same for 49 metrics, 13 unstable metrics.

scenario Δ mean execution_time candidate mean execution_time baseline mean execution_time
scenario:startup:petclinic:profiling:Remote Config worse
[+31.784µs; +78.916µs] or [+4.650%; +11.547%]		 738.811µs 683.461µs
Startup time reports for insecure-bank 
gantt
    title insecure-bank - global startup overhead: candidate=1.36.0-SNAPSHOT~5e7a05dfbb, baseline=1.38.0-SNAPSHOT~b792b2d352

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.063 s) : 0, 1062719
Total [baseline] (8.511 s) : 0, 8510880
Agent [candidate] (1.057 s) : 0, 1057327
Total [candidate] (8.492 s) : 0, 8491670
section iast
Agent [baseline] (1.184 s) : 0, 1184302
Total [baseline] (8.96 s) : 0, 8959524
Agent [candidate] (1.163 s) : 0, 1162895
Total [candidate] (8.952 s) : 0, 8952383
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.171 s) : 0, 1170797
Total [baseline] (8.916 s) : 0, 8915556
Agent [candidate] (1.164 s) : 0, 1164185
Total [candidate] (8.961 s) : 0, 8961290
section iast_TELEMETRY_OFF
Agent [baseline] (1.167 s) : 0, 1167276
Total [baseline] (8.95 s) : 0, 8950077
Agent [candidate] (1.175 s) : 0, 1174921
Total [candidate] (8.972 s) : 0, 8972320
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.063 s -
Agent iast 1.184 s 121.583 ms (11.4%)
Agent iast_HARDCODED_SECRET_DISABLED 1.171 s 108.078 ms (10.2%)
Agent iast_TELEMETRY_OFF 1.167 s 104.557 ms (9.8%)
Total tracing 8.511 s -
Total iast 8.96 s 448.643 ms (5.3%)
Total iast_HARDCODED_SECRET_DISABLED 8.916 s 404.676 ms (4.8%)
Total iast_TELEMETRY_OFF 8.95 s 439.197 ms (5.2%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.057 s -
Agent iast 1.163 s 105.568 ms (10.0%)
Agent iast_HARDCODED_SECRET_DISABLED 1.164 s 106.858 ms (10.1%)
Agent iast_TELEMETRY_OFF 1.175 s 117.594 ms (11.1%)
Total tracing 8.492 s -
Total iast 8.952 s 460.713 ms (5.4%)
Total iast_HARDCODED_SECRET_DISABLED 8.961 s 469.62 ms (5.5%)
Total iast_TELEMETRY_OFF 8.972 s 480.65 ms (5.7%) 
gantt
    title insecure-bank - break down per module: candidate=1.36.0-SNAPSHOT~5e7a05dfbb, baseline=1.38.0-SNAPSHOT~b792b2d352

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (665.07 ms) : 0, 665070
BytebuddyAgent [candidate] (661.719 ms) : 0, 661719
GlobalTracer [baseline] (305.038 ms) : 0, 305038
GlobalTracer [candidate] (302.695 ms) : 0, 302695
AppSec [baseline] (49.688 ms) : 0, 49688
AppSec [candidate] (49.994 ms) : 0, 49994
Logs Intake [candidate] (405.383 µs) : 0, 405
Remote Config [baseline] (676.782 µs) : 0, 677
Remote Config [candidate] (666.758 µs) : 0, 667
Telemetry [baseline] (7.686 ms) : 0, 7686
Telemetry [candidate] (7.507 ms) : 0, 7507
section iast
BytebuddyAgent [baseline] (788.357 ms) : 0, 788357
BytebuddyAgent [candidate] (775.311 ms) : 0, 775311
GlobalTracer [baseline] (299.487 ms) : 0, 299487
GlobalTracer [candidate] (292.192 ms) : 0, 292192
AppSec [baseline] (47.359 ms) : 0, 47359
AppSec [candidate] (47.165 ms) : 0, 47165
IAST [baseline] (27.87 ms) : 0, 27870
IAST [candidate] (25.595 ms) : 0, 25595
Logs Intake [candidate] (304.681 µs) : 0, 305
Remote Config [baseline] (568.056 µs) : 0, 568
Remote Config [candidate] (591.714 µs) : 0, 592
Telemetry [baseline] (7.085 ms) : 0, 7085
Telemetry [candidate] (8.503 ms) : 0, 8503
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (779.533 ms) : 0, 779533
BytebuddyAgent [candidate] (776.318 ms) : 0, 776318
GlobalTracer [baseline] (295.53 ms) : 0, 295530
GlobalTracer [candidate] (292.775 ms) : 0, 292775
AppSec [baseline] (47.003 ms) : 0, 47003
AppSec [candidate] (46.378 ms) : 0, 46378
IAST [baseline] (27.599 ms) : 0, 27599
IAST [candidate] (25.378 ms) : 0, 25378
Logs Intake [candidate] (296.655 µs) : 0, 297
Remote Config [baseline] (569.378 µs) : 0, 569
Remote Config [candidate] (574.971 µs) : 0, 575
Telemetry [baseline] (6.981 ms) : 0, 6981
Telemetry [candidate] (9.17 ms) : 0, 9170
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (778.041 ms) : 0, 778041
BytebuddyAgent [candidate] (783.782 ms) : 0, 783782
GlobalTracer [baseline] (295.03 ms) : 0, 295030
GlobalTracer [candidate] (296.044 ms) : 0, 296044
AppSec [baseline] (46.571 ms) : 0, 46571
AppSec [candidate] (47.555 ms) : 0, 47555
IAST [baseline] (26.639 ms) : 0, 26639
IAST [candidate] (24.8 ms) : 0, 24800
Logs Intake [candidate] (305.145 µs) : 0, 305
Remote Config [baseline] (596.822 µs) : 0, 597
Remote Config [candidate] (587.016 µs) : 0, 587
Telemetry [baseline] (6.853 ms) : 0, 6853
Telemetry [candidate] (8.383 ms) : 0, 8383
Loading
Startup time reports for petclinic 
gantt
    title petclinic - global startup overhead: candidate=1.36.0-SNAPSHOT~5e7a05dfbb, baseline=1.38.0-SNAPSHOT~b792b2d352

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.065 s) : 0, 1064793
Total [baseline] (10.398 s) : 0, 10398129
Agent [candidate] (1.057 s) : 0, 1056513
Total [candidate] (10.344 s) : 0, 10343854
section appsec
Agent [baseline] (1.186 s) : 0, 1186252
Total [baseline] (10.488 s) : 0, 10488023
Agent [candidate] (1.173 s) : 0, 1172647
Total [candidate] (10.409 s) : 0, 10409216
section iast
Agent [baseline] (1.169 s) : 0, 1168541
Total [baseline] (10.644 s) : 0, 10644237
Agent [candidate] (1.165 s) : 0, 1165003
Total [candidate] (10.668 s) : 0, 10667819
section profiling
Agent [baseline] (1.265 s) : 0, 1265362
Total [baseline] (10.603 s) : 0, 10603020
Agent [candidate] (1.257 s) : 0, 1257458
Total [candidate] (10.584 s) : 0, 10583791
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.065 s -
Agent appsec 1.186 s 121.459 ms (11.4%)
Agent iast 1.169 s 103.748 ms (9.7%)
Agent profiling 1.265 s 200.568 ms (18.8%)
Total tracing 10.398 s -
Total appsec 10.488 s 89.894 ms (0.9%)
Total iast 10.644 s 246.108 ms (2.4%)
Total profiling 10.603 s 204.891 ms (2.0%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.057 s -
Agent appsec 1.173 s 116.133 ms (11.0%)
Agent iast 1.165 s 108.49 ms (10.3%)
Agent profiling 1.257 s 200.945 ms (19.0%)
Total tracing 10.344 s -
Total appsec 10.409 s 65.362 ms (0.6%)
Total iast 10.668 s 323.965 ms (3.1%)
Total profiling 10.584 s 239.937 ms (2.3%) 
gantt
    title petclinic - break down per module: candidate=1.36.0-SNAPSHOT~5e7a05dfbb, baseline=1.38.0-SNAPSHOT~b792b2d352

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (666.73 ms) : 0, 666730
BytebuddyAgent [candidate] (661.669 ms) : 0, 661669
GlobalTracer [baseline] (305.447 ms) : 0, 305447
GlobalTracer [candidate] (302.243 ms) : 0, 302243
AppSec [baseline] (49.626 ms) : 0, 49626
AppSec [candidate] (49.755 ms) : 0, 49755
Logs Intake [candidate] (394.121 µs) : 0, 394
Remote Config [baseline] (723.232 µs) : 0, 723
Remote Config [candidate] (668.744 µs) : 0, 669
Telemetry [baseline] (7.64 ms) : 0, 7640
Telemetry [candidate] (7.471 ms) : 0, 7471
section appsec
BytebuddyAgent [baseline] (679.031 ms) : 0, 679031
BytebuddyAgent [candidate] (670.963 ms) : 0, 670963
GlobalTracer [baseline] (299.034 ms) : 0, 299034
GlobalTracer [candidate] (295.385 ms) : 0, 295385
AppSec [baseline] (153.463 ms) : 0, 153463
AppSec [candidate] (152.012 ms) : 0, 152012
IAST [baseline] (20.744 ms) : 0, 20744
IAST [candidate] (20.809 ms) : 0, 20809
Logs Intake [candidate] (329.194 µs) : 0, 329
Remote Config [baseline] (631.213 µs) : 0, 631
Remote Config [candidate] (623.872 µs) : 0, 624
Telemetry [baseline] (8.867 ms) : 0, 8867
Telemetry [candidate] (8.7 ms) : 0, 8700
section iast
BytebuddyAgent [baseline] (778.318 ms) : 0, 778318
BytebuddyAgent [candidate] (776.199 ms) : 0, 776199
GlobalTracer [baseline] (295.474 ms) : 0, 295474
GlobalTracer [candidate] (292.461 ms) : 0, 292461
AppSec [baseline] (46.603 ms) : 0, 46603
AppSec [candidate] (46.843 ms) : 0, 46843
IAST [baseline] (26.499 ms) : 0, 26499
IAST [candidate] (27.741 ms) : 0, 27741
Logs Intake [candidate] (299.478 µs) : 0, 299
Remote Config [baseline] (566.14 µs) : 0, 566
Remote Config [candidate] (576.813 µs) : 0, 577
Telemetry [baseline] (7.609 ms) : 0, 7609
Telemetry [candidate] (7.647 ms) : 0, 7647
section profiling
BytebuddyAgent [baseline] (663.5 ms) : 0, 663500
BytebuddyAgent [candidate] (660.118 ms) : 0, 660118
GlobalTracer [baseline] (388.854 ms) : 0, 388854
GlobalTracer [candidate] (385.417 ms) : 0, 385417
AppSec [baseline] (51.318 ms) : 0, 51318
AppSec [candidate] (50.692 ms) : 0, 50692
Logs Intake [candidate] (326.445 µs) : 0, 326
Remote Config [baseline] (683.461 µs) : 0, 683
Remote Config [candidate] (738.811 µs) : 0, 739
Telemetry [baseline] (7.334 ms) : 0, 7334
Telemetry [candidate] (7.361 ms) : 0, 7361
ProfilingAgent [baseline] (96.492 ms) : 0, 96492
ProfilingAgent [candidate] (96.284 ms) : 0, 96284
Profiling [baseline] (96.516 ms) : 0, 96516
Profiling [candidate] (96.309 ms) : 0, 96309
Loading

Load

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
end_time 2024-07-04T13:59:02 2024-07-04T14:05:52
git_branch master bbujon/loom
git_commit_date 1720097486 1720100320
git_commit_sha b792b2d 5e7a05d
release_version 1.38.0-SNAPSHOT~b792b2d352 1.36.0-SNAPSHOT~5e7a05dfbb
start_time 2024-07-04T13:58:48 2024-07-04T14:05:38
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1720102296 1720102296
ci_job_id 562600865 562600865
ci_pipeline_id 38379911 38379911
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 12 metrics, 16 unstable metrics.

Request duration reports for petclinic 
gantt
    title petclinic - request duration [CI 0.99] : candidate=1.36.0-SNAPSHOT~5e7a05dfbb, baseline=1.38.0-SNAPSHOT~b792b2d352
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.333 ms) : 1314, 1353
.   : milestone, 1333,
appsec (1.717 ms) : 1693, 1742
.   : milestone, 1717,
appsec_no_iast (1.723 ms) : 1698, 1747
.   : milestone, 1723,
iast (1.477 ms) : 1455, 1499
.   : milestone, 1477,
profiling (1.553 ms) : 1528, 1579
.   : milestone, 1553,
tracing (1.464 ms) : 1440, 1489
.   : milestone, 1464,
section candidate
no_agent (1.347 ms) : 1327, 1368
.   : milestone, 1347,
appsec (1.729 ms) : 1705, 1753
.   : milestone, 1729,
appsec_no_iast (1.704 ms) : 1679, 1729
.   : milestone, 1704,
iast (1.495 ms) : 1473, 1518
.   : milestone, 1495,
profiling (1.513 ms) : 1489, 1537
.   : milestone, 1513,
tracing (1.469 ms) : 1445, 1493
.   : milestone, 1469,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.333 ms [1.314 ms, 1.353 ms] -
appsec 1.717 ms [1.693 ms, 1.742 ms] 384.062 µs (28.8%)
appsec_no_iast 1.723 ms [1.698 ms, 1.747 ms] 389.143 µs (29.2%)
iast 1.477 ms [1.455 ms, 1.499 ms] 143.728 µs (10.8%)
profiling 1.553 ms [1.528 ms, 1.579 ms] 219.961 µs (16.5%)
tracing 1.464 ms [1.44 ms, 1.489 ms] 131.081 µs (9.8%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.347 ms [1.327 ms, 1.368 ms] -
appsec 1.729 ms [1.705 ms, 1.753 ms] 381.382 µs (28.3%)
appsec_no_iast 1.704 ms [1.679 ms, 1.729 ms] 356.564 µs (26.5%)
iast 1.495 ms [1.473 ms, 1.518 ms] 147.799 µs (11.0%)
profiling 1.513 ms [1.489 ms, 1.537 ms] 165.673 µs (12.3%)
tracing 1.469 ms [1.445 ms, 1.493 ms] 121.652 µs (9.0%)
Request duration reports for insecure-bank 
gantt
    title insecure-bank - request duration [CI 0.99] : candidate=1.36.0-SNAPSHOT~5e7a05dfbb, baseline=1.38.0-SNAPSHOT~b792b2d352
    dateFormat X
    axisFormat %s
section baseline
no_agent (366.589 µs) : 347, 386
.   : milestone, 367,
iast (485.411 µs) : 464, 507
.   : milestone, 485,
iast_FULL (550.451 µs) : 529, 571
.   : milestone, 550,
iast_GLOBAL (521.243 µs) : 498, 545
.   : milestone, 521,
iast_HARDCODED_SECRET_DISABLED (479.383 µs) : 459, 500
.   : milestone, 479,
iast_INACTIVE (453.662 µs) : 432, 475
.   : milestone, 454,
iast_TELEMETRY_OFF (474.85 µs) : 453, 497
.   : milestone, 475,
tracing (440.67 µs) : 420, 461
.   : milestone, 441,
section candidate
no_agent (366.215 µs) : 346, 386
.   : milestone, 366,
iast (484.436 µs) : 463, 506
.   : milestone, 484,
iast_FULL (549.29 µs) : 528, 570
.   : milestone, 549,
iast_GLOBAL (514.331 µs) : 493, 536
.   : milestone, 514,
iast_HARDCODED_SECRET_DISABLED (483.203 µs) : 462, 504
.   : milestone, 483,
iast_INACTIVE (446.152 µs) : 426, 467
.   : milestone, 446,
iast_TELEMETRY_OFF (468.678 µs) : 448, 490
.   : milestone, 469,
tracing (439.049 µs) : 419, 459
.   : milestone, 439,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 366.589 µs [347.184 µs, 385.993 µs] -
iast 485.411 µs [464.113 µs, 506.709 µs] 118.822 µs (32.4%)
iast_FULL 550.451 µs [529.461 µs, 571.441 µs] 183.862 µs (50.2%)
iast_GLOBAL 521.243 µs [497.937 µs, 544.549 µs] 154.654 µs (42.2%)
iast_HARDCODED_SECRET_DISABLED 479.383 µs [458.583 µs, 500.183 µs] 112.795 µs (30.8%)
iast_INACTIVE 453.662 µs [432.068 µs, 475.255 µs] 87.073 µs (23.8%)
iast_TELEMETRY_OFF 474.85 µs [452.78 µs, 496.921 µs] 108.262 µs (29.5%)
tracing 440.67 µs [420.078 µs, 461.261 µs] 74.081 µs (20.2%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 366.215 µs [346.17 µs, 386.259 µs] -
iast 484.436 µs [463.132 µs, 505.74 µs] 118.222 µs (32.3%)
iast_FULL 549.29 µs [528.358 µs, 570.221 µs] 183.075 µs (50.0%)
iast_GLOBAL 514.331 µs [492.803 µs, 535.859 µs] 148.116 µs (40.4%)
iast_HARDCODED_SECRET_DISABLED 483.203 µs [462.367 µs, 504.04 µs] 116.989 µs (31.9%)
iast_INACTIVE 446.152 µs [425.508 µs, 466.795 µs] 79.937 µs (21.8%)
iast_TELEMETRY_OFF 468.678 µs [447.797 µs, 489.56 µs] 102.464 µs (28.0%)
tracing 439.049 µs [419.198 µs, 458.9 µs] 72.834 µs (19.9%)

Dacapo

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master bbujon/loom
git_commit_date 1720097486 1720100320
git_commit_sha b792b2d 5e7a05d
release_version 1.38.0-SNAPSHOT~b792b2d352 1.36.0-SNAPSHOT~5e7a05dfbb
See matching parameters
Baseline Candidate
application biojava biojava
ci_job_date 1720102801 1720102801
ci_job_id 562600866 562600866
ci_pipeline_id 38379911 38379911
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 11 metrics, 1 unstable metrics.

Execution time for biojava 
gantt
    title biojava - execution time [CI 0.99] : candidate=1.36.0-SNAPSHOT~5e7a05dfbb, baseline=1.38.0-SNAPSHOT~b792b2d352
    dateFormat X
    axisFormat %s
section baseline
no_agent (14.973 s) : 14973000, 14973000
.   : milestone, 14973000,
appsec (15.375 s) : 15375000, 15375000
.   : milestone, 15375000,
iast (18.721 s) : 18721000, 18721000
.   : milestone, 18721000,
iast_GLOBAL (17.682 s) : 17682000, 17682000
.   : milestone, 17682000,
profiling (15.205 s) : 15205000, 15205000
.   : milestone, 15205000,
tracing (14.824 s) : 14824000, 14824000
.   : milestone, 14824000,
section candidate
no_agent (15.442 s) : 15442000, 15442000
.   : milestone, 15442000,
appsec (14.851 s) : 14851000, 14851000
.   : milestone, 14851000,
iast (19.004 s) : 19004000, 19004000
.   : milestone, 19004000,
iast_GLOBAL (18.098 s) : 18098000, 18098000
.   : milestone, 18098000,
profiling (15.601 s) : 15601000, 15601000
.   : milestone, 15601000,
tracing (15.107 s) : 15107000, 15107000
.   : milestone, 15107000,
Loading
  • baseline results
Variant Execution Time [CI 0.99] Δ no_agent
no_agent 14.973 s [14.973 s, 14.973 s] -
appsec 15.375 s [15.375 s, 15.375 s] 402.0 ms (2.7%)
iast 18.721 s [18.721 s, 18.721 s] 3.748 s (25.0%)
iast_GLOBAL 17.682 s [17.682 s, 17.682 s] 2.709 s (18.1%)
profiling 15.205 s [15.205 s, 15.205 s] 232.0 ms (1.5%)
tracing 14.824 s [14.824 s, 14.824 s] -149.0 ms (-1.0%)
  • candidate results
Variant Execution Time [CI 0.99] Δ no_agent
no_agent 15.442 s [15.442 s, 15.442 s] -
appsec 14.851 s [14.851 s, 14.851 s] -591.0 ms (-3.8%)
iast 19.004 s [19.004 s, 19.004 s] 3.562 s (23.1%)
iast_GLOBAL 18.098 s [18.098 s, 18.098 s] 2.656 s (17.2%)
profiling 15.601 s [15.601 s, 15.601 s] 159.0 ms (1.0%)
tracing 15.107 s [15.107 s, 15.107 s] -335.0 ms (-2.2%)
Execution time for tomcat 
gantt
    title tomcat - execution time [CI 0.99] : candidate=1.36.0-SNAPSHOT~5e7a05dfbb, baseline=1.38.0-SNAPSHOT~b792b2d352
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.455 ms) : 1444, 1467
.   : milestone, 1455,
appsec (2.208 ms) : 2174, 2242
.   : milestone, 2208,
iast (1.965 ms) : 1924, 2007
.   : milestone, 1965,
iast_GLOBAL (2.006 ms) : 1965, 2047
.   : milestone, 2006,
profiling (2.319 ms) : 2141, 2496
.   : milestone, 2319,
tracing (1.83 ms) : 1797, 1862
.   : milestone, 1830,
section candidate
no_agent (1.455 ms) : 1444, 1466
.   : milestone, 1455,
appsec (2.181 ms) : 2148, 2213
.   : milestone, 2181,
iast (1.937 ms) : 1897, 1976
.   : milestone, 1937,
iast_GLOBAL (1.973 ms) : 1934, 2012
.   : milestone, 1973,
profiling (1.83 ms) : 1797, 1863
.   : milestone, 1830,
tracing (1.804 ms) : 1774, 1835
.   : milestone, 1804,
Loading
  • baseline results
Variant Execution Time [CI 0.99] Δ no_agent
no_agent 1.455 ms [1.444 ms, 1.467 ms] -
appsec 2.208 ms [2.174 ms, 2.242 ms] 752.755 µs (51.7%)
iast 1.965 ms [1.924 ms, 2.007 ms] 509.966 µs (35.0%)
iast_GLOBAL 2.006 ms [1.965 ms, 2.047 ms] 550.74 µs (37.8%)
profiling 2.319 ms [2.141 ms, 2.496 ms] 863.338 µs (59.3%)
tracing 1.83 ms [1.797 ms, 1.862 ms] 374.27 µs (25.7%)
  • candidate results
Variant Execution Time [CI 0.99] Δ no_agent
no_agent 1.455 ms [1.444 ms, 1.466 ms] -
appsec 2.181 ms [2.148 ms, 2.213 ms] 725.864 µs (49.9%)
iast 1.937 ms [1.897 ms, 1.976 ms] 481.534 µs (33.1%)
iast_GLOBAL 1.973 ms [1.934 ms, 2.012 ms] 518.231 µs (35.6%)
profiling 1.83 ms [1.797 ms, 1.863 ms] 375.121 µs (25.8%)
tracing 1.804 ms [1.774 ms, 1.835 ms] 349.425 µs (24.0%)

@PerfectSlayer PerfectSlayer requested review from a team, mcculls and nayeem-kamal June 11, 2024 11:40
@PerfectSlayer
Copy link
Contributor Author

Rebased branch to avoid conflict

mcculls
mcculls reviewed Jun 28, 2024
View reviewed changes
...va/datadog/trace/instrumentation/java/concurrent/AsyncPropagatingDisableInstrumentation.java Outdated
@@ -27,7 +28,7 @@ public final class AsyncPropagatingDisableInstrumentation extends InstrumenterMo
implements Instrumenter.CanShortcutTypeMatching {

public AsyncPropagatingDisableInstrumentation() {
super("java_concurrent");
super(EXEC_NAME);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not a fan of adding a dependency to AbstractExecutorInstrumentation just to share a string constant.

If we have to share these string constants then I would rather put them in a separate class, such as ExecNames or similar. Although TBH when it comes to instrumentations I actually prefer to see the real string, rather than have it hidden behind a constant which I then have to look up.

Note that the compiler will inline string constants, so the class holding them isn't actually used at runtime.

mcculls
mcculls reviewed Jun 28, 2024
View reviewed changes
Copy link
Contributor

@mcculls mcculls left a comment

Choose a reason for hiding this comment

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

I'd like to see the shared instrumentation names moved to a class like ExecInstrumentationNames or ExecNames before this is merged.

I think that would be cleaner, rather than increasing the dependency between instrumentation classes just to nominally share a string constant (when it will get inlined either way.)

Once that is done this can be merged

@PerfectSlayer
Copy link
Contributor Author

I'd like to see the shared instrumentation names moved to a class like ExecInstrumentationNames or ExecNames before this is merged.

Done. I introduced a dedicated class with the few common instrumentation names.

I think that would be cleaner, rather than increasing the dependency between instrumentation classes just to nominally share a string constant (when it will get inlined either way.)

Totally agree. My original idea was to reuse string constant to avoid typo and increase consistency across instrumentation names and aliases (to help with deactivation and issue investigation), not to increase dependencies between instrumentations.

@PerfectSlayer PerfectSlayer merged commit 9e7aa67 into master Jul 9, 2024
82 checks passed
@PerfectSlayer PerfectSlayer deleted the bbujon/loom branch July 9, 2024 08:31
@github-actions github-actions bot added this to the 1.38.0 milestone Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcculls mcculls mcculls approved these changes

@nayeem-kamal nayeem-kamal Awaiting requested review from nayeem-kamal nayeem-kamal was automatically assigned from DataDog/apm-java

Assignees
No one assigned
Labels
inst: java Core Java language instrumentation tag: no release notes Changes to exclude from release notes type: refactoring
Projects
None yet
Milestone
1.38.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@PerfectSlayer @mcculls