Repository navigation
Skip unnecessary Muzzle checking - #12800
sarahchen6 wants to merge 1 commit into
Conversation
sarahchen6
added
type: feature
This comment has been minimized.
This comment has been minimized.
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![datadog-prod-us1-5[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
Explicit module checks bypass aggregate exclusions, while compile-only requests skip dependency-version planning. These behaviors were assessed through static inspection; tests were not executed.
🤖 Bits Code Review · Commit 8a0fe5e · @DataDog review to ask questions
| val isRelevantForProject = taskProjectPath.isEmpty() || taskProjectPath == project.path | ||
| val isAggregate = taskNameOnly.equals("runMuzzle", ignoreCase = true) | ||
| val isRelevantForProject = taskProjectPath.isEmpty() || taskProjectPath == project.path || | ||
| (isAggregate && project.path.startsWith("$taskProjectPath:")) |
There was a problem hiding this comment.
suggestion: Cover both absolute and relative paths in the nested-aggregate test; dd-java-agent:runMuzzle currently reports success without running the descendant checks.
In MuzzlePluginFunctionalTest, parameterize the existing test:
@ParameterizedTest
@ValueSource(strings = [":dd-java-agent:runMuzzle", "dd-java-agent:runMuzzle"])
fun `nested aggregate plans descendant instrumentation checks`(taskName: String) {Use run(taskName) and retain the generated check's SUCCESS assertion. Here, the relative taskProjectPath cannot match the absolute project.path. This limitation predates the PR.
There was a problem hiding this comment.
FYI, this block turns out to be useless and removed in #12831, so you can dismiss it.
| val session = MuzzleMavenRepoUtils.newRepositorySystemSession(system) | ||
| val taskPlanner = MuzzleTaskPlanner.from(system, session) | ||
| project.afterEvaluate { | ||
| if (relevantTasks.all { it.substringAfterLast(":").equals("runMuzzle", ignoreCase = true) } && |
There was a problem hiding this comment.
issue: Hmm this still couples muzzle plugin to ci-jobs ("runMuzzle" is registered in ci-jobs).
There was a problem hiding this comment.
I stumbled by chance to a feature of Gradle that allows to register and connect task very late, when Gradle assembled the task execution graph, which happens after project evaluation but before execution.
This looks like the exact thing needed. And as a byproduct (that I actually wanted for a while) it removes afterEvaluate. This even allows to get rid of the hasRelevantTasks / relevantTasks.
This branch has not been deployed
3 participants
What Does This Do
Avoid the work of Muzzle dependency version checks when they're unneeded, e.g. compile-only requests or modules outside the CI slot. The changes in this PR keep explicitly requested module checks, i.e. running Muzzle checks on a module specifically, and includes child modules when running a project's
runMuzzletask.Motivation
Additional Notes
I had Codex compare these changes to
master. It ran an experiment with a 30-module fixture selecting one CI module and registered checks and version configs fell from 120 to 4 while still preserving the module's 20-task graph. The median config + graph time fell from 117 to 68ms (42% improvement) for that request and 126 to 106ms (16% improvement) forcompileMuzzle(which registered 0 instead of 4 checks).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]