Add Integration Specific Handling for Config Inversion Linter#11074
Add Integration Specific Handling for Config Inversion Linter#11074
Conversation
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 62 metrics, 9 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.62.0-SNAPSHOT~d011080219, baseline=1.62.0-SNAPSHOT~4b85865643
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.062 s) : 0, 1061999
Total [baseline] (8.84 s) : 0, 8839622
Agent [candidate] (1.059 s) : 0, 1059140
Total [candidate] (8.842 s) : 0, 8842248
section iast
Agent [baseline] (1.224 s) : 0, 1223828
Total [baseline] (9.596 s) : 0, 9596206
Agent [candidate] (1.225 s) : 0, 1224833
Total [candidate] (9.586 s) : 0, 9586202
gantt
title insecure-bank - break down per module: candidate=1.62.0-SNAPSHOT~d011080219, baseline=1.62.0-SNAPSHOT~4b85865643
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.232 ms) : 0, 1232
crashtracking [candidate] (1.219 ms) : 0, 1219
BytebuddyAgent [baseline] (637.075 ms) : 0, 637075
BytebuddyAgent [candidate] (633.698 ms) : 0, 633698
AgentMeter [baseline] (29.598 ms) : 0, 29598
AgentMeter [candidate] (29.431 ms) : 0, 29431
GlobalTracer [baseline] (250.033 ms) : 0, 250033
GlobalTracer [candidate] (249.156 ms) : 0, 249156
AppSec [baseline] (32.4 ms) : 0, 32400
AppSec [candidate] (32.082 ms) : 0, 32082
Debugger [baseline] (59.749 ms) : 0, 59749
Debugger [candidate] (59.047 ms) : 0, 59047
Remote Config [baseline] (598.719 µs) : 0, 599
Remote Config [candidate] (594.776 µs) : 0, 595
Telemetry [baseline] (8.221 ms) : 0, 8221
Telemetry [candidate] (8.028 ms) : 0, 8028
Flare Poller [baseline] (6.836 ms) : 0, 6836
Flare Poller [candidate] (9.738 ms) : 0, 9738
section iast
crashtracking [baseline] (1.239 ms) : 0, 1239
crashtracking [candidate] (1.219 ms) : 0, 1219
BytebuddyAgent [baseline] (801.032 ms) : 0, 801032
BytebuddyAgent [candidate] (801.76 ms) : 0, 801760
AgentMeter [baseline] (11.403 ms) : 0, 11403
AgentMeter [candidate] (11.4 ms) : 0, 11400
GlobalTracer [baseline] (239.093 ms) : 0, 239093
GlobalTracer [candidate] (239.736 ms) : 0, 239736
AppSec [baseline] (31.089 ms) : 0, 31089
AppSec [candidate] (31.067 ms) : 0, 31067
Debugger [baseline] (62.175 ms) : 0, 62175
Debugger [candidate] (61.933 ms) : 0, 61933
Remote Config [baseline] (535.13 µs) : 0, 535
Remote Config [candidate] (534.624 µs) : 0, 535
Telemetry [baseline] (11.8 ms) : 0, 11800
Telemetry [candidate] (11.406 ms) : 0, 11406
Flare Poller [baseline] (3.475 ms) : 0, 3475
Flare Poller [candidate] (3.476 ms) : 0, 3476
IAST [baseline] (25.863 ms) : 0, 25863
IAST [candidate] (25.853 ms) : 0, 25853
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.62.0-SNAPSHOT~d011080219, baseline=1.62.0-SNAPSHOT~4b85865643
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.063 s) : 0, 1062607
Total [baseline] (11.16 s) : 0, 11159760
Agent [candidate] (1.066 s) : 0, 1065809
Total [candidate] (11.112 s) : 0, 11112340
section appsec
Agent [baseline] (1.247 s) : 0, 1247384
Total [baseline] (11.15 s) : 0, 11149724
Agent [candidate] (1.256 s) : 0, 1255874
Total [candidate] (11.039 s) : 0, 11038680
section iast
Agent [baseline] (1.226 s) : 0, 1225639
Total [baseline] (11.257 s) : 0, 11257018
Agent [candidate] (1.224 s) : 0, 1223958
Total [candidate] (11.254 s) : 0, 11254294
section profiling
Agent [baseline] (1.187 s) : 0, 1186826
Total [baseline] (11.106 s) : 0, 11105590
Agent [candidate] (1.186 s) : 0, 1186275
Total [candidate] (11.058 s) : 0, 11057544
gantt
title petclinic - break down per module: candidate=1.62.0-SNAPSHOT~d011080219, baseline=1.62.0-SNAPSHOT~4b85865643
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.245 ms) : 0, 1245
crashtracking [candidate] (1.225 ms) : 0, 1225
BytebuddyAgent [baseline] (636.77 ms) : 0, 636770
BytebuddyAgent [candidate] (638.759 ms) : 0, 638759
AgentMeter [baseline] (29.594 ms) : 0, 29594
AgentMeter [candidate] (29.683 ms) : 0, 29683
GlobalTracer [baseline] (248.704 ms) : 0, 248704
GlobalTracer [candidate] (250.604 ms) : 0, 250604
AppSec [baseline] (32.079 ms) : 0, 32079
AppSec [candidate] (32.39 ms) : 0, 32390
Debugger [baseline] (59.997 ms) : 0, 59997
Debugger [candidate] (60.491 ms) : 0, 60491
Remote Config [baseline] (599.814 µs) : 0, 600
Remote Config [candidate] (596.951 µs) : 0, 597
Telemetry [baseline] (8.097 ms) : 0, 8097
Telemetry [candidate] (8.171 ms) : 0, 8171
Flare Poller [baseline] (9.256 ms) : 0, 9256
Flare Poller [candidate] (7.552 ms) : 0, 7552
section appsec
crashtracking [baseline] (1.227 ms) : 0, 1227
crashtracking [candidate] (1.226 ms) : 0, 1226
BytebuddyAgent [baseline] (661.482 ms) : 0, 661482
BytebuddyAgent [candidate] (667.685 ms) : 0, 667685
AgentMeter [baseline] (12.08 ms) : 0, 12080
AgentMeter [candidate] (12.185 ms) : 0, 12185
GlobalTracer [baseline] (248.962 ms) : 0, 248962
GlobalTracer [candidate] (250.756 ms) : 0, 250756
AppSec [baseline] (184.196 ms) : 0, 184196
AppSec [candidate] (184.629 ms) : 0, 184629
Debugger [baseline] (65.913 ms) : 0, 65913
Debugger [candidate] (65.454 ms) : 0, 65454
Remote Config [baseline] (595.514 µs) : 0, 596
Remote Config [candidate] (593.855 µs) : 0, 594
Telemetry [baseline] (8.539 ms) : 0, 8539
Telemetry [candidate] (8.54 ms) : 0, 8540
Flare Poller [baseline] (3.534 ms) : 0, 3534
Flare Poller [candidate] (3.544 ms) : 0, 3544
IAST [baseline] (24.508 ms) : 0, 24508
IAST [candidate] (24.687 ms) : 0, 24687
section iast
crashtracking [baseline] (1.237 ms) : 0, 1237
crashtracking [candidate] (1.214 ms) : 0, 1214
BytebuddyAgent [baseline] (802.07 ms) : 0, 802070
BytebuddyAgent [candidate] (801.078 ms) : 0, 801078
AgentMeter [baseline] (11.398 ms) : 0, 11398
AgentMeter [candidate] (11.353 ms) : 0, 11353
GlobalTracer [baseline] (239.271 ms) : 0, 239271
GlobalTracer [candidate] (238.986 ms) : 0, 238986
AppSec [baseline] (31.796 ms) : 0, 31796
AppSec [candidate] (32.589 ms) : 0, 32589
Debugger [baseline] (59.974 ms) : 0, 59974
Debugger [candidate] (58.189 ms) : 0, 58189
Remote Config [baseline] (556.745 µs) : 0, 557
Remote Config [candidate] (514.798 µs) : 0, 515
Telemetry [baseline] (13.511 ms) : 0, 13511
Telemetry [candidate] (14.064 ms) : 0, 14064
Flare Poller [baseline] (3.473 ms) : 0, 3473
Flare Poller [candidate] (3.419 ms) : 0, 3419
IAST [baseline] (25.882 ms) : 0, 25882
IAST [candidate] (25.841 ms) : 0, 25841
section profiling
crashtracking [baseline] (1.176 ms) : 0, 1176
crashtracking [candidate] (1.189 ms) : 0, 1189
BytebuddyAgent [baseline] (692.792 ms) : 0, 692792
BytebuddyAgent [candidate] (692.072 ms) : 0, 692072
AgentMeter [baseline] (9.138 ms) : 0, 9138
AgentMeter [candidate] (9.145 ms) : 0, 9145
GlobalTracer [baseline] (207.685 ms) : 0, 207685
GlobalTracer [candidate] (207.164 ms) : 0, 207164
AppSec [baseline] (32.705 ms) : 0, 32705
AppSec [candidate] (32.65 ms) : 0, 32650
Debugger [baseline] (65.792 ms) : 0, 65792
Debugger [candidate] (65.766 ms) : 0, 65766
Remote Config [baseline] (568.27 µs) : 0, 568
Remote Config [candidate] (576.527 µs) : 0, 577
Telemetry [baseline] (7.828 ms) : 0, 7828
Telemetry [candidate] (7.959 ms) : 0, 7959
Flare Poller [baseline] (3.602 ms) : 0, 3602
Flare Poller [candidate] (3.649 ms) : 0, 3649
ProfilingAgent [baseline] (94.145 ms) : 0, 94145
ProfilingAgent [candidate] (94.861 ms) : 0, 94861
Profiling [baseline] (94.713 ms) : 0, 94713
Profiling [candidate] (95.435 ms) : 0, 95435
LoadParameters
See matching parameters
SummaryFound 1 performance improvements and 1 performance regressions! Performance is the same for 17 metrics, 17 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.62.0-SNAPSHOT~d011080219, baseline=1.62.0-SNAPSHOT~4b85865643
dateFormat X
axisFormat %s
section baseline
no_agent (1.24 ms) : 1228, 1252
. : milestone, 1240,
iast (3.361 ms) : 3315, 3406
. : milestone, 3361,
iast_FULL (5.923 ms) : 5864, 5982
. : milestone, 5923,
iast_GLOBAL (3.811 ms) : 3741, 3881
. : milestone, 3811,
profiling (2.312 ms) : 2291, 2333
. : milestone, 2312,
tracing (2.092 ms) : 2073, 2111
. : milestone, 2092,
section candidate
no_agent (1.261 ms) : 1248, 1273
. : milestone, 1261,
iast (3.399 ms) : 3348, 3450
. : milestone, 3399,
iast_FULL (6.26 ms) : 6195, 6324
. : milestone, 6260,
iast_GLOBAL (3.715 ms) : 3652, 3779
. : milestone, 3715,
profiling (2.287 ms) : 2266, 2307
. : milestone, 2287,
tracing (1.971 ms) : 1951, 1990
. : milestone, 1971,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.62.0-SNAPSHOT~d011080219, baseline=1.62.0-SNAPSHOT~4b85865643
dateFormat X
axisFormat %s
section baseline
no_agent (19.295 ms) : 19098, 19492
. : milestone, 19295,
appsec (19.2 ms) : 19002, 19398
. : milestone, 19200,
code_origins (17.751 ms) : 17574, 17929
. : milestone, 17751,
iast (18.174 ms) : 17995, 18354
. : milestone, 18174,
profiling (19.467 ms) : 19270, 19665
. : milestone, 19467,
tracing (18.491 ms) : 18305, 18678
. : milestone, 18491,
section candidate
no_agent (18.634 ms) : 18441, 18827
. : milestone, 18634,
appsec (18.735 ms) : 18544, 18927
. : milestone, 18735,
code_origins (17.884 ms) : 17709, 18060
. : milestone, 17884,
iast (18.234 ms) : 18056, 18412
. : milestone, 18234,
profiling (18.552 ms) : 18367, 18738
. : milestone, 18552,
tracing (17.854 ms) : 17679, 18029
. : milestone, 17854,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 1 unstable metrics. Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.62.0-SNAPSHOT~d011080219, baseline=1.62.0-SNAPSHOT~4b85865643
dateFormat X
axisFormat %s
section baseline
no_agent (1.488 ms) : 1476, 1499
. : milestone, 1488,
appsec (2.533 ms) : 2478, 2588
. : milestone, 2533,
iast (2.273 ms) : 2204, 2343
. : milestone, 2273,
iast_GLOBAL (2.312 ms) : 2242, 2382
. : milestone, 2312,
profiling (2.1 ms) : 2045, 2155
. : milestone, 2100,
tracing (2.081 ms) : 2027, 2134
. : milestone, 2081,
section candidate
no_agent (1.491 ms) : 1480, 1503
. : milestone, 1491,
appsec (3.817 ms) : 3595, 4040
. : milestone, 3817,
iast (2.265 ms) : 2196, 2334
. : milestone, 2265,
iast_GLOBAL (2.304 ms) : 2234, 2373
. : milestone, 2304,
profiling (2.096 ms) : 2041, 2151
. : milestone, 2096,
tracing (2.088 ms) : 2034, 2142
. : milestone, 2088,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.62.0-SNAPSHOT~d011080219, baseline=1.62.0-SNAPSHOT~4b85865643
dateFormat X
axisFormat %s
section baseline
no_agent (15.148 s) : 15148000, 15148000
. : milestone, 15148000,
appsec (15.054 s) : 15054000, 15054000
. : milestone, 15054000,
iast (18.036 s) : 18036000, 18036000
. : milestone, 18036000,
iast_GLOBAL (17.974 s) : 17974000, 17974000
. : milestone, 17974000,
profiling (14.933 s) : 14933000, 14933000
. : milestone, 14933000,
tracing (14.91 s) : 14910000, 14910000
. : milestone, 14910000,
section candidate
no_agent (15.436 s) : 15436000, 15436000
. : milestone, 15436000,
appsec (14.886 s) : 14886000, 14886000
. : milestone, 14886000,
iast (18.476 s) : 18476000, 18476000
. : milestone, 18476000,
iast_GLOBAL (17.928 s) : 17928000, 17928000
. : milestone, 17928000,
profiling (15.053 s) : 15053000, 15053000
. : milestone, 15053000,
tracing (14.955 s) : 14955000, 14955000
. : milestone, 14955000,
|
| * Registers a task that parses Java files in dd-java-agent/instrumentation/ and calls [checker] | ||
| * for each parsed CompilationUnit to collect violations. | ||
| */ | ||
| private fun registerInstrumentationCheckTask( |
There was a problem hiding this comment.
todo: In my opinion with #11066 is that this becomes a big file, and properly extracting responsibilities as classes, would help, here maybe creating an InstrumentationModuleConfigCheckTask would fit well. Given the checker is a lambda I wonder if this can be passed to the task as an input, but at the same time this can be expressed as anabstract "check" method that is implemented by inheritors of InstrumentationModuleConfigCheckTask.
| errorHeader: String, | ||
| errorMessage: String, | ||
| successMessage: String, | ||
| checker: MutableList<String>.(LoadedConfigFields, String, CompilationUnit) -> Unit |
There was a problem hiding this comment.
question: I believe the check signature is a bit confusing, while specifying the self (or this) can be very useful, In this instance I would rather opt for a regular function, maybe one that returns a new non mutable list.
Currently the checker lambdas seems to mutate the list. This reads weird, to have list.checker(...). Which is done in this statement
val violations = buildList {
checker(configFields, rel, cu) // actually invokes the checker lambda on the list of `buildList`, and add items in it
}
}
The name might be more transparent, e.g. e.g. collectPropertyViolations(...).
| val violations = buildList { | ||
| instrumentationFiles.files.forEach { file -> | ||
| val rel = repoRoot.relativize(file.toPath()).toString() | ||
| val cu: CompilationUnit = try { | ||
| StaticJavaParser.parse(file) | ||
| } catch (_: Exception) { | ||
| return@forEach | ||
| } | ||
| checker(configFields, rel, cu) | ||
| } | ||
| } |
There was a problem hiding this comment.
todo: In pseudo code I would do that this way
| val violations = buildList { | |
| instrumentationFiles.files.forEach { file -> | |
| val rel = repoRoot.relativize(file.toPath()).toString() | |
| val cu: CompilationUnit = try { | |
| StaticJavaParser.parse(file) | |
| } catch (_: Exception) { | |
| return@forEach | |
| } | |
| checker(configFields, rel, cu) | |
| } | |
| } | |
| val violations = instrumentationFiles.files.flatMap { file -> | |
| val rel = repoRoot.relativize(file.toPath()).toString() | |
| val cu: CompilationUnit = try { | |
| StaticJavaParser.parse(file) | |
| } catch (_: Exception) { | |
| return@forEach | |
| } | |
| collectPropertyViolations(configFields, rel, cu) | |
| } |
Here I suppose collectPropertyViolations(configFields, rel, cu) is returning a new List (non mutable)
| val context = "Integration '$name' (super arg)" | ||
| val location = "$rel:$line" | ||
|
|
||
| checkKeyAndAliases( |
There was a problem hiding this comment.
todo: to resonate with what I wrote earlier, in this case it's a bit hard to follow that checkKeyAndAliases acts as an extension functions of the current lambda's this (the Mutable<List>).
So I'd rather have a local list as well that is feeded by this method.
val cuViolations = mutableList()
for (nam,e in names) {
// ...
cuViolations.addAll(collectKeyAndAliasesViolations(...))
}
return cuViolations
Also I'd rename checkKeyAndAliases to something that indicates it's collecting violations.
What Does This Do
This PR adds Integration-specific handling for
ConfigInversionLinter. This falls into 2 sections.Instrumentation Classes:
These define configs by extending
InstrumenterModulewhich callsConfigProviderin it's constructor w/ theinstrumentationNamepassed in. This then created the typicalDD_TRACE_<INTEGRATION>_ENABLEDflow withDD_TRACE_INTEGRATION_<INTEGRATION>_ENABLEDandDD_INTEGRATION_<INTEGRATION>_ENABLEDas aliases. TheregisterCheckInstrumenterModuleConfigurationstask handles this scenario.Decorator Classes:
These define configs by extending
BaseDecoratorwhich callsConfigProviderin it's constructor w/ using the values returned from theinstrumentationNames()function. This creates theDD_TRACE_<INTEGRATION>_ANALYTICS_ENABLEDflow withDD_<INTEGRATION>_ANALYTICS_ENABLEDas an alias, andDD_TRACE_<INTEGRATION>_ANALYTICS_SAMPLE_RATEflow withDD_<INTEGRATION>_ANALYTICS_SAMPLE_RATEas an alias. TheregisterCheckDecoratorAnalyticsConfigurationstask handles this scenario.A shared helper
registerInstrumentationCheckTaskis used to loop through each source file indd-trace-java/instrumentationand call the respective tasks afterwards.This PR also adds the new Gradle task to the
config-inversion-linterCI job.Motivation
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]
Note: Once your PR is ready to merge, add it to the merge queue by commenting
/merge./merge -ccancels the queue request./merge -f --reason "reason"skips all merge queue checks; please use this judiciously, as some checks do not run at the PR-level. For more information, see this doc.