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.

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

Fix inner class signature matching #6270

Merged
merged 1 commit into from
Nov 24, 2023
Merged

Conversation

jpbempel
Copy link
Member

@jpbempel jpbempel commented Nov 23, 2023

What Does This Do

the dollar character in inner class is interpreted as regex and replacement special character. Need to be escaped.

Motivation

Supporting inner classes in probe method signature

Additional Notes

Jira ticket: DEBUG-1620

the dollar character in inner class is interpreted as regex and
replacement special character. Need to be escaped.
@jpbempel jpbempel added type: bug comp: debugger Dynamic Instrumentation labels Nov 23, 2023
@jpbempel jpbempel requested a review from a team as a code owner November 23, 2023 14:34
@jpbempel jpbempel requested review from cimi and removed request for a team November 23, 2023 14:34
@pr-commenter
Copy link

pr-commenter bot commented Nov 23, 2023

Benchmarks

Startup

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master jpbempel/fix-inner-class-as-arg
git_commit_date 1700735892 1700749909
git_commit_sha 48850c7 1d2939f
release_version 1.25.0-SNAPSHOT~48850c78e8 1.25.0-SNAPSHOT~1d2939fd1e
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1700752594 1700752594
ci_job_id 375657477 375657477
ci_pipeline_id 23758502 23758502
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 47 metrics, 7 unstable metrics.

Startup time reports for petclinic
gantt
    title petclinic - global startup overhead: candidate=1.25.0-SNAPSHOT~1d2939fd1e, baseline=1.25.0-SNAPSHOT~48850c78e8

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.033 s) : 0, 1032819
Total [baseline] (9.428 s) : 0, 9427678
Agent [candidate] (1.032 s) : 0, 1032170
Total [candidate] (9.328 s) : 0, 9328275
section appsec
Agent [baseline] (1.125 s) : 0, 1124512
Total [baseline] (9.436 s) : 0, 9436093
Agent [candidate] (1.123 s) : 0, 1122677
Total [candidate] (9.407 s) : 0, 9407493
section iast
Agent [baseline] (1.153 s) : 0, 1153471
Total [baseline] (9.569 s) : 0, 9569453
Agent [candidate] (1.165 s) : 0, 1165122
Total [candidate] (9.52 s) : 0, 9519544
section profiling
Agent [baseline] (1.218 s) : 0, 1218272
Total [baseline] (9.586 s) : 0, 9586230
Agent [candidate] (1.217 s) : 0, 1217269
Total [candidate] (9.62 s) : 0, 9620055
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.033 s -
Agent appsec 1.125 s 91.693 ms (8.9%)
Agent iast 1.153 s 120.652 ms (11.7%)
Agent profiling 1.218 s 185.453 ms (18.0%)
Total tracing 9.428 s -
Total appsec 9.436 s 8.416 ms (0.1%)
Total iast 9.569 s 141.776 ms (1.5%)
Total profiling 9.586 s 158.552 ms (1.7%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.032 s -
Agent appsec 1.123 s 90.506 ms (8.8%)
Agent iast 1.165 s 132.952 ms (12.9%)
Agent profiling 1.217 s 185.099 ms (17.9%)
Total tracing 9.328 s -
Total appsec 9.407 s 79.218 ms (0.8%)
Total iast 9.52 s 191.269 ms (2.1%)
Total profiling 9.62 s 291.78 ms (3.1%)
gantt
    title petclinic - break down per module: candidate=1.25.0-SNAPSHOT~1d2939fd1e, baseline=1.25.0-SNAPSHOT~48850c78e8

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (643.467 ms) : 0, 643467
BytebuddyAgent [candidate] (644.059 ms) : 0, 644059
GlobalTracer [baseline] (298.854 ms) : 0, 298854
GlobalTracer [candidate] (297.648 ms) : 0, 297648
AppSec [baseline] (48.294 ms) : 0, 48294
AppSec [candidate] (48.283 ms) : 0, 48283
Remote Config [baseline] (678.971 µs) : 0, 679
Remote Config [candidate] (681.396 µs) : 0, 681
Telemetry [baseline] (7.238 ms) : 0, 7238
Telemetry [candidate] (7.365 ms) : 0, 7365
section appsec
BytebuddyAgent [baseline] (646.866 ms) : 0, 646866
BytebuddyAgent [candidate] (644.173 ms) : 0, 644173
GlobalTracer [baseline] (298.841 ms) : 0, 298841
GlobalTracer [candidate] (299.798 ms) : 0, 299798
AppSec [baseline] (136.889 ms) : 0, 136889
AppSec [candidate] (137.02 ms) : 0, 137020
Remote Config [baseline] (656.069 µs) : 0, 656
Remote Config [candidate] (650.54 µs) : 0, 651
Telemetry [baseline] (6.819 ms) : 0, 6819
Telemetry [candidate] (6.846 ms) : 0, 6846
section iast
BytebuddyAgent [baseline] (766.64 ms) : 0, 766640
BytebuddyAgent [candidate] (775.235 ms) : 0, 775235
GlobalTracer [baseline] (278.911 ms) : 0, 278911
GlobalTracer [candidate] (281.239 ms) : 0, 281239
AppSec [baseline] (48.097 ms) : 0, 48097
AppSec [candidate] (50.922 ms) : 0, 50922
IAST [baseline] (14.816 ms) : 0, 14816
IAST [candidate] (14.32 ms) : 0, 14320
Remote Config [baseline] (573.952 µs) : 0, 574
Remote Config [candidate] (592.803 µs) : 0, 593
Telemetry [baseline] (10.145 ms) : 0, 10145
Telemetry [candidate] (8.079 ms) : 0, 8079
section profiling
BytebuddyAgent [baseline] (653.014 ms) : 0, 653014
BytebuddyAgent [candidate] (652.283 ms) : 0, 652283
GlobalTracer [baseline] (367.387 ms) : 0, 367387
GlobalTracer [candidate] (366.733 ms) : 0, 366733
AppSec [baseline] (48.421 ms) : 0, 48421
AppSec [candidate] (48.272 ms) : 0, 48272
Remote Config [baseline] (692.819 µs) : 0, 693
Remote Config [candidate] (713.539 µs) : 0, 714
Telemetry [baseline] (7.32 ms) : 0, 7320
Telemetry [candidate] (7.466 ms) : 0, 7466
ProfilingAgent [baseline] (87.554 ms) : 0, 87554
ProfilingAgent [candidate] (88.065 ms) : 0, 88065
Profiling [baseline] (87.577 ms) : 0, 87577
Profiling [candidate] (88.088 ms) : 0, 88088
Loading
Startup time reports for insecure-bank
gantt
    title insecure-bank - global startup overhead: candidate=1.25.0-SNAPSHOT~1d2939fd1e, baseline=1.25.0-SNAPSHOT~48850c78e8

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.038 s) : 0, 1038301
Total [baseline] (8.709 s) : 0, 8708867
Agent [candidate] (1.031 s) : 0, 1030854
Total [candidate] (8.701 s) : 0, 8700744
section iast
Agent [baseline] (1.149 s) : 0, 1149170
Total [baseline] (9.282 s) : 0, 9281652
Agent [candidate] (1.146 s) : 0, 1146071
Total [candidate] (9.213 s) : 0, 9213308
section iast_TELEMETRY_OFF
Agent [baseline] (1.148 s) : 0, 1148460
Total [baseline] (9.263 s) : 0, 9262508
Agent [candidate] (1.151 s) : 0, 1151228
Total [candidate] (9.248 s) : 0, 9247886
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.038 s -
Agent iast 1.149 s 110.869 ms (10.7%)
Agent iast_TELEMETRY_OFF 1.148 s 110.159 ms (10.6%)
Total tracing 8.709 s -
Total iast 9.282 s 572.785 ms (6.6%)
Total iast_TELEMETRY_OFF 9.263 s 553.642 ms (6.4%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.031 s -
Agent iast 1.146 s 115.217 ms (11.2%)
Agent iast_TELEMETRY_OFF 1.151 s 120.374 ms (11.7%)
Total tracing 8.701 s -
Total iast 9.213 s 512.564 ms (5.9%)
Total iast_TELEMETRY_OFF 9.248 s 547.142 ms (6.3%)
gantt
    title insecure-bank - break down per module: candidate=1.25.0-SNAPSHOT~1d2939fd1e, baseline=1.25.0-SNAPSHOT~48850c78e8

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (648.088 ms) : 0, 648088
BytebuddyAgent [candidate] (643.265 ms) : 0, 643265
GlobalTracer [baseline] (299.446 ms) : 0, 299446
GlobalTracer [candidate] (297.621 ms) : 0, 297621
AppSec [baseline] (48.262 ms) : 0, 48262
AppSec [candidate] (47.863 ms) : 0, 47863
Remote Config [baseline] (674.677 µs) : 0, 675
Remote Config [candidate] (674.115 µs) : 0, 674
Telemetry [baseline] (7.319 ms) : 0, 7319
Telemetry [candidate] (7.222 ms) : 0, 7222
section iast
BytebuddyAgent [baseline] (763.117 ms) : 0, 763117
BytebuddyAgent [candidate] (761.614 ms) : 0, 761614
GlobalTracer [baseline] (277.392 ms) : 0, 277392
GlobalTracer [candidate] (276.7 ms) : 0, 276700
AppSec [baseline] (48.04 ms) : 0, 48040
AppSec [candidate] (46.688 ms) : 0, 46688
IAST [baseline] (15.514 ms) : 0, 15514
IAST [candidate] (17.557 ms) : 0, 17557
Remote Config [baseline] (582.173 µs) : 0, 582
Remote Config [candidate] (578.212 µs) : 0, 578
Telemetry [baseline] (10.283 ms) : 0, 10283
Telemetry [candidate] (8.775 ms) : 0, 8775
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (761.962 ms) : 0, 761962
BytebuddyAgent [candidate] (762.462 ms) : 0, 762462
GlobalTracer [baseline] (279.557 ms) : 0, 279557
GlobalTracer [candidate] (280.36 ms) : 0, 280360
AppSec [baseline] (46.945 ms) : 0, 46945
AppSec [candidate] (47.366 ms) : 0, 47366
IAST [baseline] (18.587 ms) : 0, 18587
IAST [candidate] (18.805 ms) : 0, 18805
Remote Config [baseline] (560.534 µs) : 0, 561
Remote Config [candidate] (569.619 µs) : 0, 570
Telemetry [baseline] (6.356 ms) : 0, 6356
Telemetry [candidate] (7.079 ms) : 0, 7079
Loading

Load

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
end_time 2023-11-23T14:55:50 2023-11-23T15:12:22
git_branch master jpbempel/fix-inner-class-as-arg
git_commit_date 1700735892 1700749909
git_commit_sha 48850c7 1d2939f
release_version 1.25.0-SNAPSHOT~48850c78e8 1.25.0-SNAPSHOT~1d2939fd1e
start_time 2023-11-23T14:55:37 2023-11-23T15:12:09
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1700752594 1700752594
ci_job_id 375657477 375657477
ci_pipeline_id 23758502 23758502
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 9 metrics, 13 unstable metrics.

Request duration reports for petclinic
gantt
    title petclinic - request duration [CI 0.99] : candidate=1.25.0-SNAPSHOT~1d2939fd1e, baseline=1.25.0-SNAPSHOT~48850c78e8
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.351 ms) : 1332, 1370
.   : milestone, 1351,
appsec (1.759 ms) : 1734, 1784
.   : milestone, 1759,
iast (1.558 ms) : 1533, 1582
.   : milestone, 1558,
profiling (1.507 ms) : 1481, 1532
.   : milestone, 1507,
tracing (1.521 ms) : 1496, 1546
.   : milestone, 1521,
section candidate
no_agent (1.352 ms) : 1333, 1371
.   : milestone, 1352,
appsec (1.763 ms) : 1738, 1788
.   : milestone, 1763,
iast (1.542 ms) : 1517, 1566
.   : milestone, 1542,
profiling (1.511 ms) : 1486, 1537
.   : milestone, 1511,
tracing (1.482 ms) : 1457, 1508
.   : milestone, 1482,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.351 ms [1.332 ms, 1.37 ms] -
appsec 1.759 ms [1.734 ms, 1.784 ms] 408.196 µs (30.2%)
iast 1.558 ms [1.533 ms, 1.582 ms] 206.836 µs (15.3%)
profiling 1.507 ms [1.481 ms, 1.532 ms] 155.995 µs (11.5%)
tracing 1.521 ms [1.496 ms, 1.546 ms] 170.063 µs (12.6%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.352 ms [1.333 ms, 1.371 ms] -
appsec 1.763 ms [1.738 ms, 1.788 ms] 410.784 µs (30.4%)
iast 1.542 ms [1.517 ms, 1.566 ms] 189.625 µs (14.0%)
profiling 1.511 ms [1.486 ms, 1.537 ms] 159.146 µs (11.8%)
tracing 1.482 ms [1.457 ms, 1.508 ms] 130.05 µs (9.6%)
Request duration reports for insecure-bank
gantt
    title insecure-bank - request duration [CI 0.99] : candidate=1.25.0-SNAPSHOT~1d2939fd1e, baseline=1.25.0-SNAPSHOT~48850c78e8
    dateFormat X
    axisFormat %s
section baseline
no_agent (365.382 µs) : 345, 386
.   : milestone, 365,
iast (474.453 µs) : 454, 495
.   : milestone, 474,
iast_FULL (535.573 µs) : 515, 556
.   : milestone, 536,
iast_INACTIVE (441.611 µs) : 421, 462
.   : milestone, 442,
iast_TELEMETRY_OFF (476.298 µs) : 455, 497
.   : milestone, 476,
tracing (449.605 µs) : 429, 470
.   : milestone, 450,
section candidate
no_agent (370.98 µs) : 351, 390
.   : milestone, 371,
iast (481.502 µs) : 461, 502
.   : milestone, 482,
iast_FULL (537.066 µs) : 517, 557
.   : milestone, 537,
iast_INACTIVE (448.682 µs) : 428, 470
.   : milestone, 449,
iast_TELEMETRY_OFF (474.385 µs) : 453, 496
.   : milestone, 474,
tracing (443.068 µs) : 422, 464
.   : milestone, 443,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 365.382 µs [344.935 µs, 385.829 µs] -
iast 474.453 µs [454.167 µs, 494.74 µs] 109.071 µs (29.9%)
iast_FULL 535.573 µs [515.196 µs, 555.951 µs] 170.191 µs (46.6%)
iast_INACTIVE 441.611 µs [421.024 µs, 462.198 µs] 76.229 µs (20.9%)
iast_TELEMETRY_OFF 476.298 µs [455.128 µs, 497.468 µs] 110.916 µs (30.4%)
tracing 449.605 µs [428.837 µs, 470.374 µs] 84.223 µs (23.1%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 370.98 µs [351.473 µs, 390.487 µs] -
iast 481.502 µs [460.966 µs, 502.039 µs] 110.522 µs (29.8%)
iast_FULL 537.066 µs [516.86 µs, 557.272 µs] 166.086 µs (44.8%)
iast_INACTIVE 448.682 µs [427.534 µs, 469.83 µs] 77.702 µs (20.9%)
iast_TELEMETRY_OFF 474.385 µs [452.887 µs, 495.882 µs] 103.405 µs (27.9%)
tracing 443.068 µs [422.312 µs, 463.824 µs] 72.088 µs (19.4%)

@@ -164,12 +164,19 @@ private static String removeFQN(String targetSignature) {
}
String result = targetSignature;
for (String className : classes) {
Pattern classNamePattern = Pattern.compile("L" + className + ";");
result = classNamePattern.matcher(result).replaceAll("L" + simplify(className) + ";");
Pattern classNamePattern = Pattern.compile("L" + escapeClassName(className) + ";");
Copy link
Contributor

Choose a reason for hiding this comment

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

why are we using regex here? it look for me like a simple string search and replace?

Copy link
Member Author

Choose a reason for hiding this comment

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

the method String.replaceAll does exactly (and use regex) the same thing but for reason this call is forbidden by the a CI tool configured on this repo

@@ -78,18 +80,30 @@ public void linesRoundTrip() throws IOException {
@Test
public void methodMatching() {
Where where = new Where("String", "substring", "(int,int)", new String[0], null);
Copy link
Contributor

Choose a reason for hiding this comment

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

the signature doesn't have return value? we only match based on args?

Copy link
Member Author

Choose a reason for hiding this comment

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

return value is optional because it does not help to distinguish 2 overloaded ones for example.
In Java you couldn't differentiate 2 overloaded method by their return type. Only by args

  int process() {}
  void process() {}
  
  process(); // which one is called?

new Where(
"Inner",
"innerMethod",
"(com.datadog.debugger.probe.Outer$Inner)",
Copy link
Contributor

Choose a reason for hiding this comment

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

how does symdb export those inner classes? we export the class symbol as com.datadog.debugger.probe.Outer$Inner?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, because it is gave by ASM Type class for us

@jpbempel jpbempel merged commit 1bb2ee9 into master Nov 24, 2023
73 checks passed
@jpbempel jpbempel deleted the jpbempel/fix-inner-class-as-arg branch November 24, 2023 06:41
@github-actions github-actions bot added this to the 1.25.0 milestone Nov 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: debugger Dynamic Instrumentation type: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants