Use package prefix to direct search for helper dependencies in OSGi #5973

mcculls · 2023-10-02T23:05:34Z

Motivation

When tracing a library we often need to inject a helper type into its class-loader. Occasionally when a library is deployed across multiple OSGi bundles the helper being injected may need to refer to a super-type that isn't imported. This is usually because the bundle being injected doesn't directly use the super-type, it only uses sub-types exported from a different package. The super-type is in a separate bundle indirectly used by the original bundle, but its package is not accessible due to modularity rules.

When this happens the tracer must temporarily bypass these modularity rules to access that super-type. Previously the tracer did this by searching all direct bundle dependencies for the type. However in rare circumstances this can lead to loader constraint issues.

For example, an application might have a dynamically updated URLClassLoader that has the bundle class-loader as its parent, and the bundle might import a package from the system bundle. Both the system bundle and the dynamic loader have access to log4j2, but from different jar locations, and the app bundle doesn't import any log4j2 packages.

+----------------+
| system bundle  |        log4j2
+----------------+
        |
+----------------+
|   app bundle   |
+----------------+
        |
+----------------+
| dynamic loader |        log4j2
+----------------+

Assume we inject a log4j2 helper class into the dynamic loader for tracing purposes. That triggers a load request for a log4j2 interface type. The dynamic loader delegates this request to its parent, the app bundle class-loader, which would normally throw a ClassNotFoundException because it doesn't provide or import any log4j2 types. However our OSGi support will also check direct dependencies, and in this case it would find the log4j interface type in the system bundle. Unfortunately this is a different type to the one in the dynamic loader, if we returned this then we could break a loader constraint.

Note that we cannot easily tell that a downstream class-loader will find the class. Ideally we would like to defer our wider search until the entire multi-class-loader search has been exhausted, but that is not feasible. Similarly class-loader design makes it hard to pass information back to the original class-loader, because it immediately discards any parent CNF exceptions.

We therefore need to direct our search to only those bundles which the original bundle imports related packages from. In other words if the app bundle imported a log4j2 package from the system bundle, and the type being looked up was under a related package then we can be relatively confident about returning that type from the search. In the previous example, the original bundle doesn't import any log4j2 packages and therefore we wouldn't bother searching the system bundle for log4j2 types.

This modified search still satisfies the original injection issue while avoiding the above loader constraint issue.

pr-commenter · 2023-10-02T23:49:56Z

Benchmarks

Startup

Parameters

	Baseline	Candidate
commit	1.22.0-SNAPSHOT~7f4a3c3f59	1.22.0-SNAPSHOT~ba5a3304e7
config	baseline	candidate

See matching parameters

	Baseline	Candidate
application	insecure-bank	insecure-bank
module	Agent	Agent
parent	None	None
variant	appsec	appsec

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 62 cases.

Startup time reports for insecure-bank

gantt
    title insecure-bank - global startup overhead: candidate=1.22.0-SNAPSHOT~ba5a3304e7, baseline=1.22.0-SNAPSHOT~7f4a3c3f59

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.017 s) : 0, 1016845
Total [baseline] (8.666 s) : 0, 8665736
Agent [candidate] (1.012 s) : 0, 1011941
Total [candidate] (8.675 s) : 0, 8674523
section appsec
Agent [baseline] (1.108 s) : 0, 1108427
Total [baseline] (8.753 s) : 0, 8752932
Agent [candidate] (1.099 s) : 0, 1098681
Total [candidate] (8.773 s) : 0, 8773325
section iast
Agent [baseline] (1.119 s) : 0, 1118713
Total [baseline] (9.199 s) : 0, 9199043
Agent [candidate] (1.119 s) : 0, 1119284
Total [candidate] (9.226 s) : 0, 9226420
section profiling
Agent [baseline] (1.182 s) : 0, 1182150
Total [baseline] (8.901 s) : 0, 8900964
Agent [candidate] (1.193 s) : 0, 1193230
Total [candidate] (8.885 s) : 0, 8885093

baseline results

Module	Variant	Duration	Δ tracing
Agent	tracing	1.017 s	-
Agent	appsec	1.108 s	91.582 ms (9.0%)
Agent	iast	1.119 s	101.868 ms (10.0%)
Agent	profiling	1.182 s	165.305 ms (16.3%)
Total	tracing	8.666 s	-
Total	appsec	8.753 s	87.196 ms (1.0%)
Total	iast	9.199 s	533.307 ms (6.2%)
Total	profiling	8.901 s	235.228 ms (2.7%)

candidate results

Module	Variant	Duration	Δ tracing
Agent	tracing	1.012 s	-
Agent	appsec	1.099 s	86.74 ms (8.6%)
Agent	iast	1.119 s	107.344 ms (10.6%)
Agent	profiling	1.193 s	181.289 ms (17.9%)
Total	tracing	8.675 s	-
Total	appsec	8.773 s	98.802 ms (1.1%)
Total	iast	9.226 s	551.898 ms (6.4%)
Total	profiling	8.885 s	210.57 ms (2.4%)

gantt
    title insecure-bank - break down per module: candidate=1.22.0-SNAPSHOT~ba5a3304e7, baseline=1.22.0-SNAPSHOT~7f4a3c3f59

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (632.713 ms) : 0, 632713
BytebuddyAgent [candidate] (628.809 ms) : 0, 628809
GlobalTracer [baseline] (293.637 ms) : 0, 293637
GlobalTracer [candidate] (293.02 ms) : 0, 293020
AppSec [baseline] (49.282 ms) : 0, 49282
AppSec [candidate] (49.11 ms) : 0, 49110
Remote Config [baseline] (665.15 µs) : 0, 665
Remote Config [candidate] (691.434 µs) : 0, 691
Telemetry [baseline] (6.033 ms) : 0, 6033
Telemetry [candidate] (5.961 ms) : 0, 5961
section appsec
BytebuddyAgent [baseline] (634.224 ms) : 0, 634224
BytebuddyAgent [candidate] (628.776 ms) : 0, 628776
GlobalTracer [baseline] (294.852 ms) : 0, 294852
GlobalTracer [candidate] (292.199 ms) : 0, 292199
AppSec [baseline] (138.362 ms) : 0, 138362
AppSec [candidate] (137.238 ms) : 0, 137238
Remote Config [baseline] (642.313 µs) : 0, 642
Remote Config [candidate] (636.875 µs) : 0, 637
Telemetry [baseline] (5.807 ms) : 0, 5807
Telemetry [candidate] (5.719 ms) : 0, 5719
section iast
BytebuddyAgent [baseline] (741.375 ms) : 0, 741375
BytebuddyAgent [candidate] (741.249 ms) : 0, 741249
GlobalTracer [baseline] (276.819 ms) : 0, 276819
GlobalTracer [candidate] (276.863 ms) : 0, 276863
AppSec [baseline] (45.869 ms) : 0, 45869
AppSec [candidate] (46.308 ms) : 0, 46308
Remote Config [baseline] (545.04 µs) : 0, 545
Remote Config [candidate] (553.564 µs) : 0, 554
Telemetry [baseline] (5.72 ms) : 0, 5720
Telemetry [candidate] (5.751 ms) : 0, 5751
IAST [baseline] (14.182 ms) : 0, 14182
IAST [candidate] (14.352 ms) : 0, 14352
section profiling
BytebuddyAgent [baseline] (640.198 ms) : 0, 640198
BytebuddyAgent [candidate] (645.998 ms) : 0, 645998
GlobalTracer [baseline] (352.999 ms) : 0, 352999
GlobalTracer [candidate] (356.546 ms) : 0, 356546
AppSec [baseline] (49.144 ms) : 0, 49144
AppSec [candidate] (49.384 ms) : 0, 49384
Remote Config [baseline] (671.368 µs) : 0, 671
Remote Config [candidate] (680.578 µs) : 0, 681
Telemetry [baseline] (6.066 ms) : 0, 6066
Telemetry [candidate] (6.101 ms) : 0, 6101
ProfilingAgent [baseline] (79.919 ms) : 0, 79919
ProfilingAgent [candidate] (81.048 ms) : 0, 81048
Profiling [baseline] (79.943 ms) : 0, 79943
Profiling [candidate] (81.072 ms) : 0, 81072

Startup time reports for petclinic

gantt
    title petclinic - global startup overhead: candidate=1.22.0-SNAPSHOT~ba5a3304e7, baseline=1.22.0-SNAPSHOT~7f4a3c3f59

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.017 s) : 0, 1017434
Total [baseline] (9.225 s) : 0, 9225109
Agent [candidate] (1.012 s) : 0, 1012224
Total [candidate] (9.208 s) : 0, 9208417
section appsec
Agent [baseline] (1.096 s) : 0, 1096448
Total [baseline] (9.251 s) : 0, 9250546
Agent [candidate] (1.098 s) : 0, 1098302
Total [candidate] (9.249 s) : 0, 9249036
section iast
Agent [baseline] (1.125 s) : 0, 1124849
Total [baseline] (9.418 s) : 0, 9417542
Agent [candidate] (1.121 s) : 0, 1121233
Total [candidate] (9.401 s) : 0, 9401472
section profiling
Agent [baseline] (1.183 s) : 0, 1182761
Total [baseline] (9.506 s) : 0, 9505856
Agent [candidate] (1.189 s) : 0, 1189492
Total [candidate] (9.452 s) : 0, 9452269

baseline results

Module	Variant	Duration	Δ tracing
Agent	tracing	1.017 s	-
Agent	appsec	1.096 s	79.014 ms (7.8%)
Agent	iast	1.125 s	107.415 ms (10.6%)
Agent	profiling	1.183 s	165.328 ms (16.2%)
Total	tracing	9.225 s	-
Total	appsec	9.251 s	25.438 ms (0.3%)
Total	iast	9.418 s	192.433 ms (2.1%)
Total	profiling	9.506 s	280.747 ms (3.0%)

candidate results

Module	Variant	Duration	Δ tracing
Agent	tracing	1.012 s	-
Agent	appsec	1.098 s	86.078 ms (8.5%)
Agent	iast	1.121 s	109.009 ms (10.8%)
Agent	profiling	1.189 s	177.267 ms (17.5%)
Total	tracing	9.208 s	-
Total	appsec	9.249 s	40.619 ms (0.4%)
Total	iast	9.401 s	193.055 ms (2.1%)
Total	profiling	9.452 s	243.851 ms (2.6%)

gantt
    title petclinic - break down per module: candidate=1.22.0-SNAPSHOT~ba5a3304e7, baseline=1.22.0-SNAPSHOT~7f4a3c3f59

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (633.089 ms) : 0, 633089
BytebuddyAgent [candidate] (629.077 ms) : 0, 629077
GlobalTracer [baseline] (293.906 ms) : 0, 293906
GlobalTracer [candidate] (293.08 ms) : 0, 293080
AppSec [baseline] (49.181 ms) : 0, 49181
AppSec [candidate] (49.121 ms) : 0, 49121
Remote Config [baseline] (667.972 µs) : 0, 668
Remote Config [candidate] (655.164 µs) : 0, 655
Telemetry [baseline] (6.085 ms) : 0, 6085
Telemetry [candidate] (6.002 ms) : 0, 6002
section appsec
BytebuddyAgent [baseline] (627.61 ms) : 0, 627610
BytebuddyAgent [candidate] (628.288 ms) : 0, 628288
GlobalTracer [baseline] (290.986 ms) : 0, 290986
GlobalTracer [candidate] (291.903 ms) : 0, 291903
AppSec [baseline] (137.313 ms) : 0, 137313
AppSec [candidate] (137.448 ms) : 0, 137448
Remote Config [baseline] (638.033 µs) : 0, 638
Remote Config [candidate] (642.157 µs) : 0, 642
Telemetry [baseline] (5.724 ms) : 0, 5724
Telemetry [candidate] (5.728 ms) : 0, 5728
section iast
BytebuddyAgent [baseline] (744.931 ms) : 0, 744931
BytebuddyAgent [candidate] (742.171 ms) : 0, 742171
GlobalTracer [baseline] (278.184 ms) : 0, 278184
GlobalTracer [candidate] (277.741 ms) : 0, 277741
AppSec [baseline] (46.374 ms) : 0, 46374
AppSec [candidate] (46.474 ms) : 0, 46474
Remote Config [baseline] (562.028 µs) : 0, 562
Remote Config [candidate] (561.24 µs) : 0, 561
Telemetry [baseline] (5.812 ms) : 0, 5812
Telemetry [candidate] (5.788 ms) : 0, 5788
IAST [baseline] (14.577 ms) : 0, 14577
IAST [candidate] (14.343 ms) : 0, 14343
section profiling
BytebuddyAgent [baseline] (639.92 ms) : 0, 639920
BytebuddyAgent [candidate] (645.242 ms) : 0, 645242
GlobalTracer [baseline] (353.831 ms) : 0, 353831
GlobalTracer [candidate] (355.297 ms) : 0, 355297
AppSec [baseline] (48.931 ms) : 0, 48931
AppSec [candidate] (49.142 ms) : 0, 49142
Remote Config [baseline] (662.034 µs) : 0, 662
Remote Config [candidate] (670.705 µs) : 0, 671
Telemetry [baseline] (6.103 ms) : 0, 6103
Telemetry [candidate] (6.033 ms) : 0, 6033
ProfilingAgent [baseline] (80.317 ms) : 0, 80317
ProfilingAgent [candidate] (79.857 ms) : 0, 79857
Profiling [baseline] (80.343 ms) : 0, 80343
Profiling [candidate] (79.882 ms) : 0, 79882

Load

Parameters

	Baseline	Candidate
commit	1.22.0-SNAPSHOT~7f4a3c3f59	1.22.0-SNAPSHOT~ba5a3304e7
config	baseline	candidate
end_time	2023-10-04T09:41:58	2023-10-04T10:00:05
start_time	2023-10-04T09:41:39	2023-10-04T09:59:47

See matching parameters

	Baseline	Candidate
application	insecure-bank	insecure-bank
variant	appsec	appsec

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 24 cases.

Request duration reports for insecure-bank

gantt
    title insecure-bank - request duration [CI 0.99] : candidate=1.22.0-SNAPSHOT~ba5a3304e7, baseline=1.22.0-SNAPSHOT~7f4a3c3f59
    dateFormat X
    axisFormat %s
section baseline
no_agent (360.456 µs) : 341, 380
.   : milestone, 360,
appsec (701.649 µs) : 681, 722
.   : milestone, 702,
iast (462.768 µs) : 442, 483
.   : milestone, 463,
iast_FULL (513.847 µs) : 493, 534
.   : milestone, 514,
iast_INACTIVE (431.497 µs) : 411, 452
.   : milestone, 431,
profiling (439.077 µs) : 419, 459
.   : milestone, 439,
tracing (437.453 µs) : 416, 459
.   : milestone, 437,
section candidate
no_agent (369.218 µs) : 348, 390
.   : milestone, 369,
appsec (693.884 µs) : 673, 714
.   : milestone, 694,
iast (463.428 µs) : 443, 484
.   : milestone, 463,
iast_FULL (524.58 µs) : 504, 545
.   : milestone, 525,
iast_INACTIVE (435.439 µs) : 414, 456
.   : milestone, 435,
profiling (437.378 µs) : 416, 459
.   : milestone, 437,
tracing (430.646 µs) : 410, 451
.   : milestone, 431,

baseline results

Variant	Request duration [CI 0.99]	Δ no_agent
no_agent	360.456 µs [340.584 µs, 380.328 µs]	-
appsec	701.649 µs [681.04 µs, 722.259 µs]	341.193 µs (94.7%)
iast	462.768 µs [442.092 µs, 483.445 µs]	102.312 µs (28.4%)
iast_FULL	513.847 µs [493.323 µs, 534.371 µs]	153.391 µs (42.6%)
iast_INACTIVE	431.497 µs [410.949 µs, 452.046 µs]	71.041 µs (19.7%)
profiling	439.077 µs [418.725 µs, 459.429 µs]	78.621 µs (21.8%)
tracing	437.453 µs [416.192 µs, 458.713 µs]	76.997 µs (21.4%)

candidate results

Variant	Request duration [CI 0.99]	Δ no_agent
no_agent	369.218 µs [348.116 µs, 390.32 µs]	-
appsec	693.884 µs [673.425 µs, 714.343 µs]	324.667 µs (87.9%)
iast	463.428 µs [442.914 µs, 483.943 µs]	94.211 µs (25.5%)
iast_FULL	524.58 µs [503.833 µs, 545.328 µs]	155.363 µs (42.1%)
iast_INACTIVE	435.439 µs [414.379 µs, 456.499 µs]	66.221 µs (17.9%)
profiling	437.378 µs [416.058 µs, 458.697 µs]	68.16 µs (18.5%)
tracing	430.646 µs [410.274 µs, 451.019 µs]	61.429 µs (16.6%)

Request duration reports for petclinic

gantt
    title petclinic - request duration [CI 0.99] : candidate=1.22.0-SNAPSHOT~ba5a3304e7, baseline=1.22.0-SNAPSHOT~7f4a3c3f59
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.332 ms) : 1313, 1352
.   : milestone, 1332,
appsec (1.721 ms) : 1697, 1746
.   : milestone, 1721,
iast (1.454 ms) : 1430, 1478
.   : milestone, 1454,
profiling (1.466 ms) : 1441, 1490
.   : milestone, 1466,
tracing (1.464 ms) : 1440, 1489
.   : milestone, 1464,
section candidate
no_agent (1.337 ms) : 1318, 1356
.   : milestone, 1337,
appsec (1.678 ms) : 1653, 1702
.   : milestone, 1678,
iast (1.463 ms) : 1439, 1487
.   : milestone, 1463,
profiling (1.452 ms) : 1428, 1477
.   : milestone, 1452,
tracing (1.471 ms) : 1447, 1495
.   : milestone, 1471,

baseline results

Variant	Request duration [CI 0.99]	Δ no_agent
no_agent	1.332 ms [1.313 ms, 1.352 ms]	-
appsec	1.721 ms [1.697 ms, 1.746 ms]	388.819 µs (29.2%)
iast	1.454 ms [1.43 ms, 1.478 ms]	121.604 µs (9.1%)
profiling	1.466 ms [1.441 ms, 1.49 ms]	133.31 µs (10.0%)
tracing	1.464 ms [1.44 ms, 1.489 ms]	132.004 µs (9.9%)

candidate results

Variant	Request duration [CI 0.99]	Δ no_agent
no_agent	1.337 ms [1.318 ms, 1.356 ms]	-
appsec	1.678 ms [1.653 ms, 1.702 ms]	340.865 µs (25.5%)
iast	1.463 ms [1.439 ms, 1.487 ms]	126.19 µs (9.4%)
profiling	1.452 ms [1.428 ms, 1.477 ms]	115.239 µs (8.6%)
tracing	1.471 ms [1.447 ms, 1.495 ms]	133.793 µs (10.0%)

…package with a common prefix to the class being loaded.

bantonsson · 2023-10-06T06:49:27Z

...entation/osgi-4.3/src/main/java/datadog/trace/instrumentation/osgi43/BundleWiringHelper.java

+        return false; // no common package prefix
+      }
+      if (c == '.' && ++segmentsMatched >= 3) {
+        break; // three package segments matched, assume related


Three is a magic number
Yes it is, it's a magic number

mcculls added the inst: others All other instrumentations label Oct 2, 2023

mcculls changed the title ~~WIP~~ Use package prefix to direct search for helper dependencies in OSGi Oct 3, 2023

mcculls force-pushed the mcculls/limit-osgi-visibility-fix branch from 41217e9 to d9a6ff0 Compare October 3, 2023 10:39

mcculls marked this pull request as ready for review October 3, 2023 11:02

mcculls requested a review from a team as a code owner October 3, 2023 11:02

mcculls requested review from bantonsson and nayeem-kamal October 3, 2023 11:02

mcculls enabled auto-merge (squash) October 3, 2023 14:46

mcculls force-pushed the mcculls/limit-osgi-visibility-fix branch from d9a6ff0 to 7a3abce Compare October 3, 2023 21:33

Limit OSGi visibility fix to those direct dependencies that export a …

ba5a330

…package with a common prefix to the class being loaded.

mcculls force-pushed the mcculls/limit-osgi-visibility-fix branch from 7a3abce to ba5a330 Compare October 4, 2023 09:19

mcculls added this to the 1.22.0 milestone Oct 5, 2023

bantonsson approved these changes Oct 6, 2023

View reviewed changes

mcculls merged commit d58f4db into master Oct 6, 2023
69 checks passed

mcculls deleted the mcculls/limit-osgi-visibility-fix branch October 6, 2023 06:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use package prefix to direct search for helper dependencies in OSGi #5973

Use package prefix to direct search for helper dependencies in OSGi #5973

mcculls commented Oct 2, 2023 •

edited

Loading

pr-commenter bot commented Oct 2, 2023 •

edited

Loading

bantonsson Oct 6, 2023

Use package prefix to direct search for helper dependencies in OSGi #5973

Use package prefix to direct search for helper dependencies in OSGi #5973

Conversation

mcculls commented Oct 2, 2023 • edited Loading

Motivation

pr-commenter bot commented Oct 2, 2023 • edited Loading

Benchmarks

Startup

Parameters

Summary

Load

Parameters

Summary

bantonsson Oct 6, 2023

Choose a reason for hiding this comment

mcculls commented Oct 2, 2023 •

edited

Loading

pr-commenter bot commented Oct 2, 2023 •

edited

Loading