Skip to content

Fix inaccessible fields capture #6883

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
merged 1 commit into from
Apr 4, 2024
Merged

Fix inaccessible fields capture #6883

merged 1 commit into from
Apr 4, 2024

Conversation

@jpbempel
Copy link
Member

@jpbempel jpbempel commented Apr 3, 2024 •
edited by atlassian bot
Loading

What Does This Do

To fix this we need to catch eventual exception thrown by accessing field values. For that we continue to try to access directly to field when possible because it will be faster, but when we detect fields are not accessible we use another method that try to access it through reflection but could catch exception if not and returns a CapturedValue instance filled with the reason of this exception.

Motivation

NoSuchFieldException can be thrown if we try to capture, through reflection, (static) fields from JDK classes or from another module.

Additional Notes

Jira ticket: DEBUG-2270

@jpbempel
Fix inaccessible fields capture
62a7002 
NoSuchFieldException can be thrown if we try to capture, through
reflection, (static) fields from JDK classes or from another module.
To fix this we need to catch eventual exception thrown by accessing
field values.
For that we continue to try to access directly to field when possible
because it will be faster, but when we detect fields are not
accessible we use another method that try to access it through
reflection but could catch exception if not and returns a
CapturedValue instance filled with the reason of this exception.
@jpbempel jpbempel requested a review from a team as a code owner April 3, 2024 17:44
@jpbempel jpbempel requested review from cimi and evanchooly and removed request for a team April 3, 2024 17:44
@pr-commenter
Copy link

pr-commenter bot commented Apr 3, 2024

Benchmarks

Startup

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master jpbempel/fix-field-collection
git_commit_date 1712162876 1712166192
git_commit_sha 396fcca 62a7002
release_version 1.32.0-SNAPSHOT~396fccabad 1.32.0-SNAPSHOT~62a7002f0d
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1712169511 1712169511
ci_job_id 477184295 477184295
ci_pipeline_id 31380067 31380067
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 49 metrics, 14 unstable metrics.

Startup time reports for insecure-bank 
gantt
    title insecure-bank - global startup overhead: candidate=1.32.0-SNAPSHOT~62a7002f0d, baseline=1.32.0-SNAPSHOT~396fccabad

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.081 s) : 0, 1081233
Total [baseline] (8.583 s) : 0, 8582527
Agent [candidate] (1.084 s) : 0, 1084011
Total [candidate] (8.567 s) : 0, 8567358
section iast
Agent [baseline] (1.215 s) : 0, 1214750
Total [baseline] (9.015 s) : 0, 9014676
Agent [candidate] (1.195 s) : 0, 1195028
Total [candidate] (9.009 s) : 0, 9009124
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.206 s) : 0, 1205744
Total [baseline] (8.975 s) : 0, 8975098
Agent [candidate] (1.215 s) : 0, 1214555
Total [candidate] (9.035 s) : 0, 9034784
section iast_TELEMETRY_OFF
Agent [baseline] (1.199 s) : 0, 1198706
Total [baseline] (9.038 s) : 0, 9037581
Agent [candidate] (1.2 s) : 0, 1199767
Total [candidate] (9.023 s) : 0, 9023292
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.081 s -
Agent iast 1.215 s 133.517 ms (12.3%)
Agent iast_HARDCODED_SECRET_DISABLED 1.206 s 124.511 ms (11.5%)
Agent iast_TELEMETRY_OFF 1.199 s 117.473 ms (10.9%)
Total tracing 8.583 s -
Total iast 9.015 s 432.149 ms (5.0%)
Total iast_HARDCODED_SECRET_DISABLED 8.975 s 392.572 ms (4.6%)
Total iast_TELEMETRY_OFF 9.038 s 455.054 ms (5.3%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.084 s -
Agent iast 1.195 s 111.017 ms (10.2%)
Agent iast_HARDCODED_SECRET_DISABLED 1.215 s 130.544 ms (12.0%)
Agent iast_TELEMETRY_OFF 1.2 s 115.755 ms (10.7%)
Total tracing 8.567 s -
Total iast 9.009 s 441.767 ms (5.2%)
Total iast_HARDCODED_SECRET_DISABLED 9.035 s 467.427 ms (5.5%)
Total iast_TELEMETRY_OFF 9.023 s 455.934 ms (5.3%) 
gantt
    title insecure-bank - break down per module: candidate=1.32.0-SNAPSHOT~62a7002f0d, baseline=1.32.0-SNAPSHOT~396fccabad

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (677.331 ms) : 0, 677331
BytebuddyAgent [candidate] (678.888 ms) : 0, 678888
GlobalTracer [baseline] (311.302 ms) : 0, 311302
GlobalTracer [candidate] (312.58 ms) : 0, 312580
AppSec [baseline] (49.752 ms) : 0, 49752
AppSec [candidate] (49.775 ms) : 0, 49775
Remote Config [baseline] (664.714 µs) : 0, 665
Remote Config [candidate] (668.272 µs) : 0, 668
Telemetry [baseline] (7.626 ms) : 0, 7626
Telemetry [candidate] (7.585 ms) : 0, 7585
section iast
BytebuddyAgent [baseline] (805.9 ms) : 0, 805900
BytebuddyAgent [candidate] (792.053 ms) : 0, 792053
GlobalTracer [baseline] (291.399 ms) : 0, 291399
GlobalTracer [candidate] (287.965 ms) : 0, 287965
AppSec [baseline] (50.919 ms) : 0, 50919
AppSec [candidate] (49.245 ms) : 0, 49245
IAST [baseline] (23.727 ms) : 0, 23727
IAST [candidate] (21.309 ms) : 0, 21309
Remote Config [baseline] (574.524 µs) : 0, 575
Remote Config [candidate] (581.48 µs) : 0, 581
Telemetry [baseline] (7.327 ms) : 0, 7327
Telemetry [candidate] (9.628 ms) : 0, 9628
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (799.316 ms) : 0, 799316
BytebuddyAgent [candidate] (805.843 ms) : 0, 805843
GlobalTracer [baseline] (289.363 ms) : 0, 289363
GlobalTracer [candidate] (292.302 ms) : 0, 292302
AppSec [baseline] (51.438 ms) : 0, 51438
AppSec [candidate] (50.529 ms) : 0, 50529
IAST [baseline] (22.961 ms) : 0, 22961
IAST [candidate] (23.061 ms) : 0, 23061
Remote Config [baseline] (593.888 µs) : 0, 594
Remote Config [candidate] (586.531 µs) : 0, 587
Telemetry [baseline] (7.493 ms) : 0, 7493
Telemetry [candidate] (7.359 ms) : 0, 7359
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (792.732 ms) : 0, 792732
BytebuddyAgent [candidate] (794.325 ms) : 0, 794325
GlobalTracer [baseline] (290.102 ms) : 0, 290102
GlobalTracer [candidate] (289.544 ms) : 0, 289544
AppSec [baseline] (49.682 ms) : 0, 49682
AppSec [candidate] (49.762 ms) : 0, 49762
IAST [baseline] (23.836 ms) : 0, 23836
IAST [candidate] (24.571 ms) : 0, 24571
Remote Config [baseline] (594.545 µs) : 0, 595
Remote Config [candidate] (579.927 µs) : 0, 580
Telemetry [baseline] (7.357 ms) : 0, 7357
Telemetry [candidate] (6.522 ms) : 0, 6522
Loading
Startup time reports for petclinic 
gantt
    title petclinic - global startup overhead: candidate=1.32.0-SNAPSHOT~62a7002f0d, baseline=1.32.0-SNAPSHOT~396fccabad

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.075 s) : 0, 1074592
Total [baseline] (10.465 s) : 0, 10465478
Agent [candidate] (1.075 s) : 0, 1075173
Total [candidate] (10.342 s) : 0, 10342377
section appsec
Agent [baseline] (1.204 s) : 0, 1204160
Total [baseline] (10.504 s) : 0, 10503658
Agent [candidate] (1.2 s) : 0, 1200064
Total [candidate] (10.468 s) : 0, 10468449
section iast
Agent [baseline] (1.2 s) : 0, 1199826
Total [baseline] (10.798 s) : 0, 10797525
Agent [candidate] (1.198 s) : 0, 1198087
Total [candidate] (10.8 s) : 0, 10800158
section profiling
Agent [baseline] (1.274 s) : 0, 1274007
Total [baseline] (10.653 s) : 0, 10652979
Agent [candidate] (1.266 s) : 0, 1265675
Total [candidate] (10.589 s) : 0, 10588895
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.075 s -
Agent appsec 1.204 s 129.568 ms (12.1%)
Agent iast 1.2 s 125.233 ms (11.7%)
Agent profiling 1.274 s 199.415 ms (18.6%)
Total tracing 10.465 s -
Total appsec 10.504 s 38.18 ms (0.4%)
Total iast 10.798 s 332.048 ms (3.2%)
Total profiling 10.653 s 187.502 ms (1.8%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.075 s -
Agent appsec 1.2 s 124.891 ms (11.6%)
Agent iast 1.198 s 122.914 ms (11.4%)
Agent profiling 1.266 s 190.502 ms (17.7%)
Total tracing 10.342 s -
Total appsec 10.468 s 126.072 ms (1.2%)
Total iast 10.8 s 457.781 ms (4.4%)
Total profiling 10.589 s 246.519 ms (2.4%) 
gantt
    title petclinic - break down per module: candidate=1.32.0-SNAPSHOT~62a7002f0d, baseline=1.32.0-SNAPSHOT~396fccabad

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (672.642 ms) : 0, 672642
BytebuddyAgent [candidate] (672.968 ms) : 0, 672968
GlobalTracer [baseline] (309.623 ms) : 0, 309623
GlobalTracer [candidate] (310.02 ms) : 0, 310020
AppSec [baseline] (49.589 ms) : 0, 49589
AppSec [candidate] (49.588 ms) : 0, 49588
Remote Config [baseline] (660.836 µs) : 0, 661
Remote Config [candidate] (661.735 µs) : 0, 662
Telemetry [baseline] (7.634 ms) : 0, 7634
Telemetry [candidate] (7.584 ms) : 0, 7584
section appsec
BytebuddyAgent [baseline] (699.465 ms) : 0, 699465
BytebuddyAgent [candidate] (696.034 ms) : 0, 696034
GlobalTracer [baseline] (292.938 ms) : 0, 292938
GlobalTracer [candidate] (292.538 ms) : 0, 292538
AppSec [baseline] (150.099 ms) : 0, 150099
AppSec [candidate] (149.52 ms) : 0, 149520
Remote Config [baseline] (613.674 µs) : 0, 614
Remote Config [candidate] (603.504 µs) : 0, 604
Telemetry [baseline] (7.487 ms) : 0, 7487
Telemetry [candidate] (8.099 ms) : 0, 8099
IAST [baseline] (18.916 ms) : 0, 18916
IAST [candidate] (18.822 ms) : 0, 18822
section iast
BytebuddyAgent [baseline] (795.782 ms) : 0, 795782
BytebuddyAgent [candidate] (793.474 ms) : 0, 793474
GlobalTracer [baseline] (288.025 ms) : 0, 288025
GlobalTracer [candidate] (288.392 ms) : 0, 288392
AppSec [baseline] (50.216 ms) : 0, 50216
AppSec [candidate] (51.844 ms) : 0, 51844
Remote Config [baseline] (579.345 µs) : 0, 579
Remote Config [candidate] (580.856 µs) : 0, 581
Telemetry [baseline] (6.566 ms) : 0, 6566
Telemetry [candidate] (8.093 ms) : 0, 8093
IAST [baseline] (24.223 ms) : 0, 24223
IAST [candidate] (21.282 ms) : 0, 21282
section profiling
ProfilingAgent [baseline] (96.198 ms) : 0, 96198
ProfilingAgent [candidate] (95.482 ms) : 0, 95482
BytebuddyAgent [baseline] (680.892 ms) : 0, 680892
BytebuddyAgent [candidate] (675.639 ms) : 0, 675639
GlobalTracer [baseline] (381.755 ms) : 0, 381755
GlobalTracer [candidate] (380.384 ms) : 0, 380384
AppSec [baseline] (50.427 ms) : 0, 50427
AppSec [candidate] (49.92 ms) : 0, 49920
Remote Config [baseline] (706.726 µs) : 0, 707
Remote Config [candidate] (699.819 µs) : 0, 700
Telemetry [baseline] (7.464 ms) : 0, 7464
Telemetry [candidate] (7.441 ms) : 0, 7441
Profiling [baseline] (96.223 ms) : 0, 96223
Profiling [candidate] (95.506 ms) : 0, 95506
Loading

Load

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
end_time 2024-04-03T18:10:47 2024-04-03T18:32:46
git_branch master jpbempel/fix-field-collection
git_commit_date 1712162876 1712166192
git_commit_sha 396fcca 62a7002
release_version 1.32.0-SNAPSHOT~396fccabad 1.32.0-SNAPSHOT~62a7002f0d
start_time 2024-04-03T18:10:34 2024-04-03T18:32:32
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1712169511 1712169511
ci_job_id 477184295 477184295
ci_pipeline_id 31380067 31380067
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 13 metrics, 15 unstable metrics.

Request duration reports for insecure-bank 
gantt
    title insecure-bank - request duration [CI 0.99] : candidate=1.32.0-SNAPSHOT~62a7002f0d, baseline=1.32.0-SNAPSHOT~396fccabad
    dateFormat X
    axisFormat %s
section baseline
no_agent (372.694 µs) : 353, 392
.   : milestone, 373,
iast (479.76 µs) : 458, 501
.   : milestone, 480,
iast_FULL (546.141 µs) : 525, 567
.   : milestone, 546,
iast_GLOBAL (503.069 µs) : 481, 525
.   : milestone, 503,
iast_HARDCODED_SECRET_DISABLED (483.553 µs) : 462, 506
.   : milestone, 484,
iast_INACTIVE (447.59 µs) : 427, 468
.   : milestone, 448,
iast_TELEMETRY_OFF (474.058 µs) : 453, 495
.   : milestone, 474,
tracing (450.981 µs) : 431, 471
.   : milestone, 451,
section candidate
no_agent (376.005 µs) : 356, 396
.   : milestone, 376,
iast (475.927 µs) : 455, 497
.   : milestone, 476,
iast_FULL (547.439 µs) : 526, 568
.   : milestone, 547,
iast_GLOBAL (498.726 µs) : 477, 520
.   : milestone, 499,
iast_HARDCODED_SECRET_DISABLED (492.775 µs) : 471, 514
.   : milestone, 493,
iast_INACTIVE (449.953 µs) : 429, 471
.   : milestone, 450,
iast_TELEMETRY_OFF (474.012 µs) : 454, 494
.   : milestone, 474,
tracing (450.808 µs) : 430, 472
.   : milestone, 451,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 372.694 µs [353.255 µs, 392.133 µs] -
iast 479.76 µs [458.385 µs, 501.134 µs] 107.066 µs (28.7%)
iast_FULL 546.141 µs [524.907 µs, 567.376 µs] 173.447 µs (46.5%)
iast_GLOBAL 503.069 µs [480.912 µs, 525.225 µs] 130.374 µs (35.0%)
iast_HARDCODED_SECRET_DISABLED 483.553 µs [461.565 µs, 505.541 µs] 110.859 µs (29.7%)
iast_INACTIVE 447.59 µs [427.285 µs, 467.895 µs] 74.895 µs (20.1%)
iast_TELEMETRY_OFF 474.058 µs [453.498 µs, 494.617 µs] 101.363 µs (27.2%)
tracing 450.981 µs [430.642 µs, 471.321 µs] 78.287 µs (21.0%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 376.005 µs [356.363 µs, 395.647 µs] -
iast 475.927 µs [455.153 µs, 496.701 µs] 99.921 µs (26.6%)
iast_FULL 547.439 µs [526.447 µs, 568.432 µs] 171.434 µs (45.6%)
iast_GLOBAL 498.726 µs [477.454 µs, 519.999 µs] 122.721 µs (32.6%)
iast_HARDCODED_SECRET_DISABLED 492.775 µs [471.306 µs, 514.244 µs] 116.77 µs (31.1%)
iast_INACTIVE 449.953 µs [429.387 µs, 470.52 µs] 73.948 µs (19.7%)
iast_TELEMETRY_OFF 474.012 µs [453.587 µs, 494.438 µs] 98.007 µs (26.1%)
tracing 450.808 µs [429.823 µs, 471.792 µs] 74.802 µs (19.9%)
Request duration reports for petclinic 
gantt
    title petclinic - request duration [CI 0.99] : candidate=1.32.0-SNAPSHOT~62a7002f0d, baseline=1.32.0-SNAPSHOT~396fccabad
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.345 ms) : 1326, 1364
.   : milestone, 1345,
appsec (1.734 ms) : 1710, 1757
.   : milestone, 1734,
appsec_no_iast (1.725 ms) : 1701, 1750
.   : milestone, 1725,
iast (1.503 ms) : 1480, 1526
.   : milestone, 1503,
profiling (1.491 ms) : 1467, 1516
.   : milestone, 1491,
tracing (1.48 ms) : 1455, 1504
.   : milestone, 1480,
section candidate
no_agent (1.369 ms) : 1350, 1388
.   : milestone, 1369,
appsec (1.713 ms) : 1689, 1738
.   : milestone, 1713,
appsec_no_iast (1.716 ms) : 1691, 1740
.   : milestone, 1716,
iast (1.505 ms) : 1482, 1528
.   : milestone, 1505,
profiling (1.51 ms) : 1486, 1534
.   : milestone, 1510,
tracing (1.499 ms) : 1474, 1525
.   : milestone, 1499,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.345 ms [1.326 ms, 1.364 ms] -
appsec 1.734 ms [1.71 ms, 1.757 ms] 388.666 µs (28.9%)
appsec_no_iast 1.725 ms [1.701 ms, 1.75 ms] 380.196 µs (28.3%)
iast 1.503 ms [1.48 ms, 1.526 ms] 157.881 µs (11.7%)
profiling 1.491 ms [1.467 ms, 1.516 ms] 146.417 µs (10.9%)
tracing 1.48 ms [1.455 ms, 1.504 ms] 134.554 µs (10.0%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.369 ms [1.35 ms, 1.388 ms] -
appsec 1.713 ms [1.689 ms, 1.738 ms] 344.081 µs (25.1%)
appsec_no_iast 1.716 ms [1.691 ms, 1.74 ms] 346.395 µs (25.3%)
iast 1.505 ms [1.482 ms, 1.528 ms] 135.748 µs (9.9%)
profiling 1.51 ms [1.486 ms, 1.534 ms] 140.754 µs (10.3%)
tracing 1.499 ms [1.474 ms, 1.525 ms] 130.333 µs (9.5%)

@jpbempel jpbempel added comp: debugger Dynamic Instrumentation type: bug Bug report and fix labels Apr 3, 2024
evanchooly
evanchooly approved these changes Apr 3, 2024
View reviewed changes
Copy link
Contributor

@evanchooly evanchooly left a comment

Choose a reason for hiding this comment

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

this looks good to me. a rather straightforward solution.

...debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java
int counter = 0;
for (FieldNode fieldNode : fieldsToCapture) {
insnList.add(new InsnNode(Opcodes.DUP));
// stack: [capturedcontext, capturedcontext, array, array]
Copy link
Contributor

@evanchooly evanchooly Apr 3, 2024

Choose a reason for hiding this comment

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

i like the tracking of the stack values. :)

@jpbempel jpbempel merged commit c1ae1bc into master Apr 4, 2024
@jpbempel jpbempel deleted the jpbempel/fix-field-collection branch April 4, 2024 07:09
@github-actions github-actions bot added this to the 1.32.0 milestone Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@evanchooly evanchooly evanchooly approved these changes

@cimi cimi Awaiting requested review from cimi cimi is a code owner automatically assigned from DataDog/debugger-java

Assignees

No one assigned

Labels

comp: debugger Dynamic Instrumentation type: bug Bug report and fix

Projects

None yet

Milestone

1.32.0

Development

Successfully merging this pull request may close these issues.

3 participants

@jpbempel @evanchooly