Refactor muzzle tasks to dedicated classes, make Muzzletask cacheable #9544
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
Note, this only caches the muzzle assertion, tasks are being still registered depending on what is brought back from maven. I.e. if a new dependency is discovered a new task will be registered and as such will be executed.
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
bric3
changed the title
bric3
added
tag: no release notes
This comment has been minimized.
This comment has been minimized.
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 47 metrics, 12 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.54.0-SNAPSHOT~f0d55c8e95, baseline=1.54.0-SNAPSHOT~2d42e7c7f3
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.064 s) : 0, 1063760
Total [baseline] (10.739 s) : 0, 10739254
Agent [candidate] (1.064 s) : 0, 1063818
Total [candidate] (10.817 s) : 0, 10817276
section appsec
Agent [baseline] (1.243 s) : 0, 1242923
Total [baseline] (11.074 s) : 0, 11074302
Agent [candidate] (1.235 s) : 0, 1235401
Total [candidate] (11.013 s) : 0, 11013425
section iast
Agent [baseline] (1.19 s) : 0, 1190002
Total [baseline] (11.056 s) : 0, 11055978
Agent [candidate] (1.23 s) : 0, 1229834
Total [candidate] (9.532 s) : 0, 9532006
section profiling
Agent [baseline] (1.21 s) : 0, 1210051
Total [baseline] (11.083 s) : 0, 11083228
Agent [candidate] (1.221 s) : 0, 1220555
Total [candidate] (11.048 s) : 0, 11048081
gantt
title petclinic - break down per module: candidate=1.54.0-SNAPSHOT~f0d55c8e95, baseline=1.54.0-SNAPSHOT~2d42e7c7f3
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.458 ms) : 0, 1458
crashtracking [candidate] (1.458 ms) : 0, 1458
BytebuddyAgent [baseline] (734.14 ms) : 0, 734140
BytebuddyAgent [candidate] (735.29 ms) : 0, 735290
GlobalTracer [baseline] (253.018 ms) : 0, 253018
GlobalTracer [candidate] (252.947 ms) : 0, 252947
AppSec [baseline] (30.767 ms) : 0, 30767
AppSec [candidate] (30.558 ms) : 0, 30558
Debugger [baseline] (6.43 ms) : 0, 6430
Debugger [candidate] (6.381 ms) : 0, 6381
Remote Config [baseline] (702.709 µs) : 0, 703
Remote Config [candidate] (684.855 µs) : 0, 685
Telemetry [baseline] (16.223 ms) : 0, 16223
Telemetry [candidate] (15.377 ms) : 0, 15377
section appsec
crashtracking [baseline] (1.47 ms) : 0, 1470
crashtracking [candidate] (1.453 ms) : 0, 1453
BytebuddyAgent [baseline] (762.502 ms) : 0, 762502
BytebuddyAgent [candidate] (756.197 ms) : 0, 756197
GlobalTracer [baseline] (247.216 ms) : 0, 247216
GlobalTracer [candidate] (245.823 ms) : 0, 245823
IAST [baseline] (24.075 ms) : 0, 24075
IAST [candidate] (23.822 ms) : 0, 23822
AppSec [baseline] (171.17 ms) : 0, 171170
AppSec [candidate] (171.066 ms) : 0, 171066
Debugger [baseline] (6.031 ms) : 0, 6031
Debugger [candidate] (6.014 ms) : 0, 6014
Remote Config [baseline] (634.544 µs) : 0, 635
Remote Config [candidate] (628.566 µs) : 0, 629
Telemetry [baseline] (8.532 ms) : 0, 8532
Telemetry [candidate] (9.208 ms) : 0, 9208
section iast
crashtracking [baseline] (1.463 ms) : 0, 1463
crashtracking [candidate] (1.485 ms) : 0, 1485
BytebuddyAgent [baseline] (852.323 ms) : 0, 852323
BytebuddyAgent [candidate] (882.386 ms) : 0, 882386
GlobalTracer [baseline] (246.048 ms) : 0, 246048
GlobalTracer [candidate] (250.903 ms) : 0, 250903
IAST [baseline] (28.153 ms) : 0, 28153
IAST [candidate] (31.859 ms) : 0, 31859
AppSec [baseline] (26.303 ms) : 0, 26303
AppSec [candidate] (26.066 ms) : 0, 26066
Debugger [baseline] (6.062 ms) : 0, 6062
Debugger [candidate] (6.378 ms) : 0, 6378
Remote Config [baseline] (589.98 µs) : 0, 590
Remote Config [candidate] (709.153 µs) : 0, 709
Telemetry [baseline] (8.107 ms) : 0, 8107
Telemetry [candidate] (8.677 ms) : 0, 8677
section profiling
crashtracking [baseline] (1.435 ms) : 0, 1435
crashtracking [candidate] (1.44 ms) : 0, 1440
BytebuddyAgent [baseline] (762.423 ms) : 0, 762423
BytebuddyAgent [candidate] (769.631 ms) : 0, 769631
GlobalTracer [baseline] (233.033 ms) : 0, 233033
GlobalTracer [candidate] (235.463 ms) : 0, 235463
AppSec [baseline] (31.295 ms) : 0, 31295
AppSec [candidate] (30.883 ms) : 0, 30883
Debugger [baseline] (10.644 ms) : 0, 10644
Debugger [candidate] (7.557 ms) : 0, 7557
Remote Config [baseline] (1.565 ms) : 0, 1565
Remote Config [candidate] (715.643 µs) : 0, 716
Telemetry [baseline] (10.982 ms) : 0, 10982
Telemetry [candidate] (15.642 ms) : 0, 15642
ProfilingAgent [baseline] (107.597 ms) : 0, 107597
ProfilingAgent [candidate] (107.977 ms) : 0, 107977
Profiling [baseline] (108.227 ms) : 0, 108227
Profiling [candidate] (108.62 ms) : 0, 108620
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.54.0-SNAPSHOT~f0d55c8e95, baseline=1.54.0-SNAPSHOT~2d42e7c7f3
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.071 s) : 0, 1070881
Total [baseline] (8.658 s) : 0, 8657585
Agent [candidate] (1.064 s) : 0, 1063521
Total [candidate] (8.639 s) : 0, 8639244
section iast
Agent [baseline] (1.19 s) : 0, 1190349
Total [baseline] (9.329 s) : 0, 9328742
Agent [candidate] (1.197 s) : 0, 1196813
Total [candidate] (9.332 s) : 0, 9332306
gantt
title insecure-bank - break down per module: candidate=1.54.0-SNAPSHOT~f0d55c8e95, baseline=1.54.0-SNAPSHOT~2d42e7c7f3
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.469 ms) : 0, 1469
crashtracking [candidate] (1.454 ms) : 0, 1454
BytebuddyAgent [baseline] (739.469 ms) : 0, 739469
BytebuddyAgent [candidate] (734.524 ms) : 0, 734524
GlobalTracer [baseline] (254.394 ms) : 0, 254394
GlobalTracer [candidate] (252.586 ms) : 0, 252586
AppSec [baseline] (30.93 ms) : 0, 30930
AppSec [candidate] (30.629 ms) : 0, 30629
Debugger [baseline] (6.494 ms) : 0, 6494
Debugger [candidate] (6.384 ms) : 0, 6384
Remote Config [baseline] (703.028 µs) : 0, 703
Remote Config [candidate] (695.821 µs) : 0, 696
Telemetry [baseline] (16.31 ms) : 0, 16310
Telemetry [candidate] (16.282 ms) : 0, 16282
section iast
crashtracking [baseline] (1.452 ms) : 0, 1452
crashtracking [candidate] (1.467 ms) : 0, 1467
BytebuddyAgent [baseline] (852.099 ms) : 0, 852099
BytebuddyAgent [candidate] (858.441 ms) : 0, 858441
GlobalTracer [baseline] (245.131 ms) : 0, 245131
GlobalTracer [candidate] (247.892 ms) : 0, 247892
IAST [baseline] (30.432 ms) : 0, 30432
IAST [candidate] (27.122 ms) : 0, 27122
AppSec [baseline] (25.485 ms) : 0, 25485
AppSec [candidate] (26.148 ms) : 0, 26148
Debugger [baseline] (6.078 ms) : 0, 6078
Debugger [candidate] (6.02 ms) : 0, 6020
Remote Config [baseline] (596.552 µs) : 0, 597
Remote Config [candidate] (586.327 µs) : 0, 586
Telemetry [baseline] (8.136 ms) : 0, 8136
Telemetry [candidate] (8.103 ms) : 0, 8103
LoadParameters
See matching parameters
SummaryFound 2 performance improvements and 1 performance regressions! Performance is the same for 9 metrics, 12 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.54.0-SNAPSHOT~f0d55c8e95, baseline=1.54.0-SNAPSHOT~2d42e7c7f3
dateFormat X
axisFormat %s
section baseline
no_agent (4.312 ms) : 4265, 4360
. : milestone, 4312,
iast (9.627 ms) : 9470, 9785
. : milestone, 9627,
iast_FULL (14.005 ms) : 13721, 14289
. : milestone, 14005,
iast_GLOBAL (10.333 ms) : 10148, 10517
. : milestone, 10333,
profiling (9.134 ms) : 8980, 9288
. : milestone, 9134,
tracing (7.695 ms) : 7573, 7818
. : milestone, 7695,
section candidate
no_agent (4.508 ms) : 4457, 4559
. : milestone, 4508,
iast (9.504 ms) : 9347, 9660
. : milestone, 9504,
iast_FULL (13.948 ms) : 13678, 14218
. : milestone, 13948,
iast_GLOBAL (10.341 ms) : 10155, 10527
. : milestone, 10341,
profiling (8.737 ms) : 8588, 8887
. : milestone, 8737,
tracing (7.405 ms) : 7297, 7512
. : milestone, 7405,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.54.0-SNAPSHOT~f0d55c8e95, baseline=1.54.0-SNAPSHOT~2d42e7c7f3
dateFormat X
axisFormat %s
section baseline
no_agent (37.901 ms) : 37592, 38209
. : milestone, 37901,
appsec (48.978 ms) : 48540, 49416
. : milestone, 48978,
code_origins (45.251 ms) : 44849, 45653
. : milestone, 45251,
iast (43.973 ms) : 43594, 44351
. : milestone, 43973,
profiling (49.112 ms) : 48663, 49561
. : milestone, 49112,
tracing (44.548 ms) : 44158, 44939
. : milestone, 44548,
section candidate
no_agent (37.66 ms) : 37360, 37960
. : milestone, 37660,
appsec (50.048 ms) : 49599, 50496
. : milestone, 50048,
code_origins (46.341 ms) : 45959, 46723
. : milestone, 46341,
iast (44.771 ms) : 44393, 45150
. : milestone, 44771,
profiling (49.491 ms) : 49007, 49974
. : milestone, 49491,
tracing (43.845 ms) : 43463, 44228
. : milestone, 43845,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 10 metrics, 2 unstable metrics. Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.54.0-SNAPSHOT~f0d55c8e95, baseline=1.54.0-SNAPSHOT~2d42e7c7f3
dateFormat X
axisFormat %s
section baseline
no_agent (1.47 ms) : 1459, 1482
. : milestone, 1470,
appsec (2.5 ms) : 2446, 2553
. : milestone, 2500,
iast (2.196 ms) : 2133, 2259
. : milestone, 2196,
iast_GLOBAL (2.232 ms) : 2169, 2295
. : milestone, 2232,
profiling (2.476 ms) : 2312, 2640
. : milestone, 2476,
tracing (2.022 ms) : 1973, 2072
. : milestone, 2022,
section candidate
no_agent (1.48 ms) : 1468, 1492
. : milestone, 1480,
appsec (3.69 ms) : 3474, 3906
. : milestone, 3690,
iast (2.19 ms) : 2127, 2252
. : milestone, 2190,
iast_GLOBAL (2.239 ms) : 2176, 2302
. : milestone, 2239,
profiling (2.055 ms) : 2003, 2107
. : milestone, 2055,
tracing (2.023 ms) : 1974, 2072
. : milestone, 2023,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.54.0-SNAPSHOT~f0d55c8e95, baseline=1.54.0-SNAPSHOT~2d42e7c7f3
dateFormat X
axisFormat %s
section baseline
no_agent (14.82 s) : 14820000, 14820000
. : milestone, 14820000,
appsec (15.102 s) : 15102000, 15102000
. : milestone, 15102000,
iast (18.464 s) : 18464000, 18464000
. : milestone, 18464000,
iast_GLOBAL (18.113 s) : 18113000, 18113000
. : milestone, 18113000,
profiling (15.978 s) : 15978000, 15978000
. : milestone, 15978000,
tracing (14.831 s) : 14831000, 14831000
. : milestone, 14831000,
section candidate
no_agent (15.296 s) : 15296000, 15296000
. : milestone, 15296000,
appsec (15.037 s) : 15037000, 15037000
. : milestone, 15037000,
iast (18.718 s) : 18718000, 18718000
. : milestone, 18718000,
iast_GLOBAL (18.107 s) : 18107000, 18107000
. : milestone, 18107000,
profiling (15.876 s) : 15876000, 15876000
. : milestone, 15876000,
tracing (15.223 s) : 15223000, 15223000
. : milestone, 15223000,
|
|
|
||
| abstract class AbstractMuzzleReportTask : AbstractMuzzleTask() { | ||
| internal fun dumpVersionsToCsv(versions: SortedMap<String, TestedArtifact>) { | ||
| val filename = project.path.replaceFirst("^:".toRegex(), "").replace(":", "_") |
There was a problem hiding this comment.
Would it be better to use substring instead of replace by regexp?
Looks like it could be a utility function to create valid file name.
There was a problem hiding this comment.
Note, this was ported from groovy in the previous PR. That said this look like it can be replaced by a simple substring.
Comment on lines
10
to
12
| val versionsFile = project.rootProject.layout.buildDirectory.file("${MUZZLE_DEPS_RESULTS}/$filename.csv") | ||
| with(project.file(versionsFile)) { | ||
| parentFile.mkdirs() |
There was a problem hiding this comment.
Maybe we can have resultDir and resulsFile as separte varaibles? Code would be a bit more readable.
Also it is better to use ${filename}.csv for readability.
There was a problem hiding this comment.
Mmmh, I'll see to improve here.
There was a problem hiding this comment.
Note in kotlin this is a weak warning to use braces if there's no code inside.
Maybe I'll declare the versionsFile as a task output 🤔.
buildSrc/src/main/kotlin/datadog/gradle/plugin/muzzle/tasks/MuzzleEndTask.kt
Outdated
Show resolved
Hide resolved
| * A pass or fail directive for a single dependency. | ||
| */ | ||
| open class MuzzleDirective { | ||
| open class MuzzleDirective : Serializable { |
There was a problem hiding this comment.
Just curious, why Serializable?
There was a problem hiding this comment.
This is for cacheability, without it there's a failure when serializing this task input.
buildSrc/src/main/kotlin/datadog/gradle/plugin/muzzle/MuzzlePlugin.kt
Outdated
Show resolved
Hide resolved
Comment on lines
+13
to
+14
| val endTimeMs = System.currentTimeMillis() | ||
| val seconds = (endTimeMs - startTimeMs.get()).toDouble() / 1000.0 |
There was a problem hiding this comment.
No much sense in endTimeMs?
There was a problem hiding this comment.
I slightly prefer to have a val.
| val endTimeMs = System.currentTimeMillis() | ||
| val seconds = (endTimeMs - startTimeMs.get()).toDouble() / 1000.0 | ||
| val name = "${project.path}:muzzle" | ||
| val dirname = name.replaceFirst("^:".toRegex(), "").replace(":", "_") |
There was a problem hiding this comment.
Looks like it could be a utility function to create valid file name.
There was a problem hiding this comment.
Indeed! The original code in groovy had some duplication. I barely inlined the utils methods.
I'll see what I can do.
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
This PR fix this wrong definition of tasks by creating proper types for each task.
Also, this refactoring was an opportunity to properly declare inputs and output and make the
MuzzleTaskcacheable.Follow-up to
MuzzlePlugin#9315Note
This only caches the muzzle assertion task, if the input don't change, tasks are being still registered depending on what is brought back from maven. That means that if a new dependency is discovered, a new task will be registered, and as such, it will appear in the task graph and executed.
Note the up-to-date when run a second time
Note that muzzle plugin actually fetches from maven the versions as asked in the directives
...
...
Also notice here the tasks for some versions that were not executed in an earlier run.
The
printReferenceswas also made cacheable.Motivation
Previous MuzzlePlugin was using
MuzzleTaskfor every muzzle tasks, however it wired services that were not related to every task. Since tasks were different, the plugin ran different action viadoLastand a call to some functions. This is a bad and confusing use of the Gradle API.Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any usefull labelsclose,fixor any linking keywords when referencing an issue.Use
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]