Make CSI plugin collect too much classpaths #9985
Open
+341
−245
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bric3
added
type: enhancement
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 59 metrics, 6 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.56.0-SNAPSHOT~dea2a7c0f8, baseline=1.56.0-SNAPSHOT~5adec51856
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.099 s) : 0, 1098859
Total [baseline] (8.858 s) : 0, 8858025
Agent [candidate] (1.098 s) : 0, 1098115
Total [candidate] (8.846 s) : 0, 8846256
section iast
Agent [baseline] (1.236 s) : 0, 1235995
Total [baseline] (9.551 s) : 0, 9550967
Agent [candidate] (1.245 s) : 0, 1245443
Total [candidate] (9.553 s) : 0, 9552507
gantt
title insecure-bank - break down per module: candidate=1.56.0-SNAPSHOT~dea2a7c0f8, baseline=1.56.0-SNAPSHOT~5adec51856
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.475 ms) : 0, 1475
crashtracking [candidate] (1.46 ms) : 0, 1460
BytebuddyAgent [baseline] (707.285 ms) : 0, 707285
BytebuddyAgent [candidate] (704.691 ms) : 0, 704691
GlobalTracer [baseline] (247.279 ms) : 0, 247279
GlobalTracer [candidate] (248.793 ms) : 0, 248793
AppSec [baseline] (32.092 ms) : 0, 32092
AppSec [candidate] (32.205 ms) : 0, 32205
Debugger [baseline] (63.351 ms) : 0, 63351
Debugger [candidate] (63.447 ms) : 0, 63447
Remote Config [baseline] (643.977 µs) : 0, 644
Remote Config [candidate] (632.973 µs) : 0, 633
Telemetry [baseline] (8.066 ms) : 0, 8066
Telemetry [candidate] (8.271 ms) : 0, 8271
Flare Poller [baseline] (3.66 ms) : 0, 3660
Flare Poller [candidate] (3.758 ms) : 0, 3758
section iast
crashtracking [baseline] (1.456 ms) : 0, 1456
crashtracking [candidate] (1.469 ms) : 0, 1469
BytebuddyAgent [baseline] (828.007 ms) : 0, 828007
BytebuddyAgent [candidate] (835.338 ms) : 0, 835338
GlobalTracer [baseline] (238.065 ms) : 0, 238065
GlobalTracer [candidate] (239.441 ms) : 0, 239441
IAST [baseline] (28.423 ms) : 0, 28423
IAST [candidate] (30.355 ms) : 0, 30355
AppSec [baseline] (33.303 ms) : 0, 33303
AppSec [candidate] (31.586 ms) : 0, 31586
Debugger [baseline] (60.178 ms) : 0, 60178
Debugger [candidate] (60.522 ms) : 0, 60522
Remote Config [baseline] (543.735 µs) : 0, 544
Remote Config [candidate] (548.621 µs) : 0, 549
Telemetry [baseline] (7.606 ms) : 0, 7606
Telemetry [candidate] (7.695 ms) : 0, 7695
Flare Poller [baseline] (3.466 ms) : 0, 3466
Flare Poller [candidate] (3.449 ms) : 0, 3449
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.56.0-SNAPSHOT~dea2a7c0f8, baseline=1.56.0-SNAPSHOT~5adec51856
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.114 s) : 0, 1113870
Total [baseline] (10.79 s) : 0, 10789595
Agent [candidate] (1.106 s) : 0, 1106226
Total [candidate] (10.799 s) : 0, 10799262
section appsec
Agent [baseline] (1.28 s) : 0, 1280391
Total [baseline] (11.022 s) : 0, 11021993
Agent [candidate] (1.28 s) : 0, 1279905
Total [candidate] (11.013 s) : 0, 11013398
section iast
Agent [baseline] (1.237 s) : 0, 1236549
Total [baseline] (11.28 s) : 0, 11279655
Agent [candidate] (1.237 s) : 0, 1236756
Total [candidate] (11.235 s) : 0, 11235362
section profiling
Agent [baseline] (1.228 s) : 0, 1227966
Total [baseline] (11.058 s) : 0, 11058256
Agent [candidate] (1.235 s) : 0, 1235425
Total [candidate] (11.015 s) : 0, 11014517
gantt
title petclinic - break down per module: candidate=1.56.0-SNAPSHOT~dea2a7c0f8, baseline=1.56.0-SNAPSHOT~5adec51856
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.48 ms) : 0, 1480
crashtracking [candidate] (1.449 ms) : 0, 1449
BytebuddyAgent [baseline] (715.338 ms) : 0, 715338
BytebuddyAgent [candidate] (708.695 ms) : 0, 708695
GlobalTracer [baseline] (251.821 ms) : 0, 251821
GlobalTracer [candidate] (251.807 ms) : 0, 251807
AppSec [baseline] (32.682 ms) : 0, 32682
AppSec [candidate] (32.542 ms) : 0, 32542
Debugger [baseline] (64.67 ms) : 0, 64670
Debugger [candidate] (64.544 ms) : 0, 64544
Remote Config [baseline] (637.264 µs) : 0, 637
Remote Config [candidate] (640.961 µs) : 0, 641
Telemetry [baseline] (8.256 ms) : 0, 8256
Telemetry [candidate] (8.069 ms) : 0, 8069
Flare Poller [baseline] (3.718 ms) : 0, 3718
Flare Poller [candidate] (3.646 ms) : 0, 3646
section appsec
crashtracking [baseline] (1.46 ms) : 0, 1460
crashtracking [candidate] (1.446 ms) : 0, 1446
BytebuddyAgent [baseline] (730.326 ms) : 0, 730326
BytebuddyAgent [candidate] (729.522 ms) : 0, 729522
GlobalTracer [baseline] (240.968 ms) : 0, 240968
GlobalTracer [candidate] (240.366 ms) : 0, 240366
IAST [baseline] (24.793 ms) : 0, 24793
IAST [candidate] (24.63 ms) : 0, 24630
AppSec [baseline] (174.599 ms) : 0, 174599
AppSec [candidate] (174.706 ms) : 0, 174706
Debugger [baseline] (60.304 ms) : 0, 60304
Debugger [candidate] (61.384 ms) : 0, 61384
Remote Config [baseline] (689.457 µs) : 0, 689
Remote Config [candidate] (664.04 µs) : 0, 664
Telemetry [baseline] (8.382 ms) : 0, 8382
Telemetry [candidate] (8.363 ms) : 0, 8363
Flare Poller [baseline] (3.854 ms) : 0, 3854
Flare Poller [candidate] (3.872 ms) : 0, 3872
section iast
crashtracking [baseline] (1.446 ms) : 0, 1446
crashtracking [candidate] (1.447 ms) : 0, 1447
BytebuddyAgent [baseline] (828.705 ms) : 0, 828705
BytebuddyAgent [candidate] (828.26 ms) : 0, 828260
GlobalTracer [baseline] (237.693 ms) : 0, 237693
GlobalTracer [candidate] (238.166 ms) : 0, 238166
IAST [baseline] (27.404 ms) : 0, 27404
IAST [candidate] (29.164 ms) : 0, 29164
AppSec [baseline] (33.953 ms) : 0, 33953
AppSec [candidate] (32.384 ms) : 0, 32384
Debugger [baseline] (60.6 ms) : 0, 60600
Debugger [candidate] (60.862 ms) : 0, 60862
Remote Config [baseline] (542.312 µs) : 0, 542
Remote Config [candidate] (554.781 µs) : 0, 555
Telemetry [baseline] (7.732 ms) : 0, 7732
Telemetry [candidate] (7.591 ms) : 0, 7591
Flare Poller [baseline] (3.451 ms) : 0, 3451
Flare Poller [candidate] (3.415 ms) : 0, 3415
section profiling
crashtracking [baseline] (1.432 ms) : 0, 1432
crashtracking [candidate] (1.446 ms) : 0, 1446
BytebuddyAgent [baseline] (729.683 ms) : 0, 729683
BytebuddyAgent [candidate] (735.259 ms) : 0, 735259
GlobalTracer [baseline] (222.217 ms) : 0, 222217
GlobalTracer [candidate] (223.375 ms) : 0, 223375
AppSec [baseline] (32.027 ms) : 0, 32027
AppSec [candidate] (32.409 ms) : 0, 32409
Debugger [baseline] (63.129 ms) : 0, 63129
Debugger [candidate] (62.842 ms) : 0, 62842
Remote Config [baseline] (669.634 µs) : 0, 670
Remote Config [candidate] (665.52 µs) : 0, 666
Telemetry [baseline] (8.109 ms) : 0, 8109
Telemetry [candidate] (8.056 ms) : 0, 8056
Flare Poller [baseline] (3.773 ms) : 0, 3773
Flare Poller [candidate] (3.811 ms) : 0, 3811
ProfilingAgent [baseline] (97.362 ms) : 0, 97362
ProfilingAgent [candidate] (97.421 ms) : 0, 97421
Profiling [baseline] (97.944 ms) : 0, 97944
Profiling [candidate] (98.005 ms) : 0, 98005
LoadParameters
See matching parameters
SummaryFound 2 performance improvements and 1 performance regressions! Performance is the same for 18 metrics, 15 unstable metrics.
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.56.0-SNAPSHOT~dea2a7c0f8, baseline=1.56.0-SNAPSHOT~5adec51856
dateFormat X
axisFormat %s
section baseline
no_agent (18.048 ms) : 17864, 18232
. : milestone, 18048,
appsec (18.39 ms) : 18202, 18578
. : milestone, 18390,
code_origins (17.665 ms) : 17490, 17839
. : milestone, 17665,
iast (17.834 ms) : 17656, 18011
. : milestone, 17834,
profiling (18.64 ms) : 18452, 18827
. : milestone, 18640,
tracing (17.564 ms) : 17387, 17741
. : milestone, 17564,
section candidate
no_agent (17.123 ms) : 16956, 17291
. : milestone, 17123,
appsec (18.702 ms) : 18515, 18890
. : milestone, 18702,
code_origins (17.762 ms) : 17586, 17939
. : milestone, 17762,
iast (17.493 ms) : 17318, 17667
. : milestone, 17493,
profiling (19.63 ms) : 19432, 19828
. : milestone, 19630,
tracing (17.489 ms) : 17315, 17663
. : milestone, 17489,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.56.0-SNAPSHOT~dea2a7c0f8, baseline=1.56.0-SNAPSHOT~5adec51856
dateFormat X
axisFormat %s
section baseline
no_agent (1.187 ms) : 1175, 1198
. : milestone, 1187,
iast (3.262 ms) : 3220, 3303
. : milestone, 3262,
iast_FULL (6.03 ms) : 5969, 6092
. : milestone, 6030,
iast_GLOBAL (3.572 ms) : 3519, 3625
. : milestone, 3572,
profiling (2.112 ms) : 2093, 2130
. : milestone, 2112,
tracing (1.864 ms) : 1847, 1881
. : milestone, 1864,
section candidate
no_agent (1.202 ms) : 1190, 1214
. : milestone, 1202,
iast (3.271 ms) : 3229, 3313
. : milestone, 3271,
iast_FULL (5.812 ms) : 5737, 5887
. : milestone, 5812,
iast_GLOBAL (3.6 ms) : 3547, 3652
. : milestone, 3600,
profiling (2.071 ms) : 2054, 2089
. : milestone, 2071,
tracing (1.841 ms) : 1826, 1856
. : milestone, 1841,
DacapoParameters
See matching parameters
SummaryFound 1 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 0 unstable metrics.
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.56.0-SNAPSHOT~dea2a7c0f8, baseline=1.56.0-SNAPSHOT~5adec51856
dateFormat X
axisFormat %s
section baseline
no_agent (1.481 ms) : 1469, 1492
. : milestone, 1481,
appsec (3.711 ms) : 3495, 3927
. : milestone, 3711,
iast (2.234 ms) : 2169, 2300
. : milestone, 2234,
iast_GLOBAL (2.288 ms) : 2222, 2353
. : milestone, 2288,
profiling (2.082 ms) : 2029, 2135
. : milestone, 2082,
tracing (2.048 ms) : 1997, 2099
. : milestone, 2048,
section candidate
no_agent (1.489 ms) : 1477, 1500
. : milestone, 1489,
appsec (2.527 ms) : 2472, 2582
. : milestone, 2527,
iast (2.239 ms) : 2174, 2305
. : milestone, 2239,
iast_GLOBAL (2.28 ms) : 2214, 2345
. : milestone, 2280,
profiling (2.079 ms) : 2025, 2132
. : milestone, 2079,
tracing (2.059 ms) : 2008, 2110
. : milestone, 2059,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.56.0-SNAPSHOT~dea2a7c0f8, baseline=1.56.0-SNAPSHOT~5adec51856
dateFormat X
axisFormat %s
section baseline
no_agent (15.029 s) : 15029000, 15029000
. : milestone, 15029000,
appsec (14.539 s) : 14539000, 14539000
. : milestone, 14539000,
iast (18.702 s) : 18702000, 18702000
. : milestone, 18702000,
iast_GLOBAL (18.058 s) : 18058000, 18058000
. : milestone, 18058000,
profiling (14.848 s) : 14848000, 14848000
. : milestone, 14848000,
tracing (14.68 s) : 14680000, 14680000
. : milestone, 14680000,
section candidate
no_agent (15.341 s) : 15341000, 15341000
. : milestone, 15341000,
appsec (14.519 s) : 14519000, 14519000
. : milestone, 14519000,
iast (18.677 s) : 18677000, 18677000
. : milestone, 18677000,
iast_GLOBAL (18.036 s) : 18036000, 18036000
. : milestone, 18036000,
profiling (14.833 s) : 14833000, 14833000
. : milestone, 14833000,
tracing (14.626 s) : 14626000, 14626000
. : milestone, 14626000,
|
This comment has been minimized.
This comment has been minimized.
Base automatically changed from
bdu/do-not-include-latest-dep-classpath
to
master
November 18, 2025 12:57
…time configurations.
The 'instrument' plugin changes destination around for now, it needs to be depended on for the time being.
bric3
force-pushed
the
bdu/make-csi-consume-way-less
branch
from
8022770 to
f2f5205
Compare
This forced to apply patching in afterEvaluate, instead, it's easier and cleaner to add the csi output as dependency in the appropriate configuration.
…sformation to string
bric3
force-pushed
the
bdu/make-csi-consume-way-less
branch
2 times, most recently
from
365af7b to
16f82db
Compare
bric3
force-pushed
the
bdu/make-csi-consume-way-less
branch
from
16f82db to
dea2a7c
Compare
bric3
commented
Nov 19, 2025
Comment on lines
+77
to
+85
| /** | ||
| * The paths used to look for the call site instrumenter dependencies. | ||
| * | ||
| * The plugin includes by default **only** the `main` and `test` source sets, and their | ||
| * related compilation classpath. As we don't want other test configurations by default. | ||
| * | ||
| * However, it's possible to contribute additional paths to look for dependencies. | ||
| */ | ||
| val additionalPaths: ConfigurableFileCollection = objectFactory.fileCollection() |
Contributor
Author
There was a problem hiding this comment.
note: Before the csi plugin consumed all test configurations, which forced to resolved all jvm test suites configurations. Now that the csi plugin is now restricted to the standard main and test, a way is needed to provide additional paths to the call site instrumenter generator.
Contributor
Author
There was a problem hiding this comment.
This file was moved from datadog/gradle/plugin to datadog/gradle/plugin/csi
Contributor
Author
There was a problem hiding this comment.
Extracted from datadog/gradle/plugin/CallSiteInstrumentationPlugin.kt
Comment on lines
+49
to
+51
| configureSourceSets(project, csiExtension) | ||
| registerGenerateCallSiteTask(project, csiExtension, project.tasks.named<AbstractCompile>("compileJava")) | ||
| configureTestConfigurations(project, csiExtension) |
Contributor
Author
There was a problem hiding this comment.
note: Before this happened in project.afterEvaluate phase which is suboptimal. This was working because it was patching some task classpath very late, but prevented other avoidance tricks that Gradle could provide.
Instead to make other task aware of the csi output one has to use configurations, see configureTestConfigurations.
Comment on lines
+213
to
+228
| get() = project.configurations.matching { | ||
| // Includes all main* source sets, but only the test (as wee don;t want other ) | ||
| // * For main => runtimeClasspath, compileClasspath | ||
| // * For test => testRuntimeClasspath, testCompileClasspath | ||
| // * For other main* => "main_javaXXRuntimeClasspath", "main_javaXXCompileClasspath" | ||
|
|
||
| when (it.name) { | ||
| // Regular main and test source sets | ||
| RUNTIME_CLASSPATH_CONFIGURATION_NAME, | ||
| COMPILE_CLASSPATH_CONFIGURATION_NAME, | ||
| TEST_SOURCE_SET_NAME + RUNTIME_CLASSPATH_CONFIGURATION_NAME.capitalize(), | ||
| TEST_SOURCE_SET_NAME + COMPILE_CLASSPATH_CONFIGURATION_NAME.capitalize() -> true | ||
|
|
||
| else -> false | ||
| } | ||
| } |
Contributor
Author
There was a problem hiding this comment.
note: Non greedy configurations collection by default. Before it could collect configurations (i.e. files, output folder), from many unrelated compile and test tasks.
There was a problem hiding this comment.
Yep, before it was trying to catch em all. Did you observe any performance improvements during configuration or execution of gradle tasks with this change?
Comment on lines
-119
to
-127
| private fun newBuildFolder(project: Project, name: String): File { | ||
| val folder = project.layout.buildDirectory.dir(name).get().asFile | ||
| if (!folder.exists()) { | ||
| if (!folder.mkdirs()) { | ||
| throw GradleException("Cannot create folder $folder") | ||
| } | ||
| } | ||
| return folder | ||
| } |
Contributor
Author
There was a problem hiding this comment.
note: This is non necessary to create the folder, and also this is unnecessary to convert targetFolder to a String and recreate it from here.
Comment on lines
-145
to
-154
| project.tasks.withType(AbstractCompile::class.java).matching { | ||
| task -> task.name.startsWith("compileTest") | ||
| }.configureEach { | ||
| inputs.dir(extension.targetFolder) | ||
| classpath += project.files(targetFolder) | ||
| } | ||
| project.tasks.withType(Test::class.java).configureEach { | ||
| inputs.dir(extension.targetFolder) | ||
| classpath += project.files(targetFolder) | ||
| } |
Contributor
Author
There was a problem hiding this comment.
note: This works only if configureEach block is appended after everything else, i.e. in the project.afterEvaluate, which is suboptimal, this was changed to add the targetFolder as a dependency to the relevant jvm test suites.
|
|
||
| private fun createTasks(project: Project, extension: CallSiteInstrumentationExtension) { | ||
| registerGenerateCallSiteTask(project, extension, "compileJava") | ||
| val targetFolder = extension.targetFolder.get().asFile |
Contributor
Author
There was a problem hiding this comment.
note: It's usually non necessary to convert to a File, the project.files(...) understand many types. (Also many other API indicates they rely on the project.files(...) behavior it's indicated in their Javadoc)
bric3
commented
Nov 19, 2025
Comment on lines
+96
to
+105
| project.pluginManager.withPlugin("jvm-test-suite") { | ||
| project.extensions.getByType<TestingExtension>().suites.withType<JvmTestSuite>().configureEach { | ||
| project.logger.info("Configuring jvm test suite '{}' to use csiExtension.targetFolder", name) | ||
| dependencies { | ||
| compileOnly.add(project.files(csiExtension.targetFolder)) | ||
| runtimeOnly.add(project.files(csiExtension.targetFolder)) | ||
| } | ||
| } | ||
| } | ||
| } |
Contributor
Author
There was a problem hiding this comment.
note: This replaces the old logic that patched test compile and test task classpath. The new code reacts to any registered test suites at configuration time.
| public static URL toURL(final Path path) { | ||
| try { | ||
| return path.toUri().toURL(); | ||
| URL url = path.toUri().toURL(); |
| } | ||
|
|
||
| csi { | ||
| additionalPaths.from(classPathConfiguration) |
There was a problem hiding this comment.
This way to extend the classpath of the generator is way cleaner, nice!
There was a problem hiding this comment.
We should probably ignore this folder in git.
|
|
||
| // Remote Debug | ||
| if (project.providers.gradleProperty("debugCsiJar").isPresent) { | ||
| jvmArgs("-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=localhost:5005") |
| inputs.dir(csiExtension.srcFolder) | ||
| inputs.dir(csiExtension.rootFolder).optional() | ||
| inputs.file(pluginJarFile) | ||
| inputs.property("cis.suffix", csiExtension.suffix) |
There was a problem hiding this comment.
Good catch, didn't realize those should be part of the inputs
manuel-alvarez-alvarez
approved these changes
Nov 20, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What Does This Do
Refactors the call-site-instrumentation plugin that collected too much classpath, by querying tasks classpath, which may create various problems (like conflicting dependency resolution because they end-up in the same bucket).
Instead, this PR tries to make the csi plugin collect the minimum (
mainandtest). Also, instead of querying tasks, it gather classpath via project'sconfigurations. If something non standard is needed it makes it explicit via.This change exposed a bug in the way the csi plugin was setting up its type resolution classpath. It uses a
URLClassLoader, however since the collected path is much more restricted, some compiled types exists in folders, andURLClassLoaderrequires a directory URL to end by a slash (/) otherwise it assumes the URL points to a jar even if it is actually a directory, this resulted in types not found at generation time.Also, this plugins applies its changes during
project.afterEvaluate, this has an unfortunate effect of theafterEvaluateand required to patch the classpath of some test related tasks directly. This is replaced a more appropriate declaration of a dependency in the appropriate configuration.Other cleanups were added.
Motivation
Quality of life, correctness.
Follow-up
Related to
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any useful labelsclose,fixor any linking keywords when referencing an issue.Use
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]