[JENKINS-75310] Fix log recorder level updates - #27065
Conversation
|
Yay, your first pull request towards Jenkins core was created successfully! Thank you so much! |
There was a problem hiding this comment.
Pull request overview
Fixes an inconsistency where lowering a custom log recorder target’s verbosity (e.g., FINEST → INFO) could leave a stale explicit controller Logger level in place, causing Logger#getLevel() / isLoggable to disagree with the intended recorder configuration. The change introduces targeted controller-side level reconciliation after config submissions and adds regression tests covering shared/descendant/manual-level interactions.
Changes:
- Refactors
LogRecorder.Target#enable()to split controller-side conditional updates (enableLogger) from agent broadcasts. - Adds
LogRecorder#updateLogLevels(...)to clear stale explicit controller levels (when still matching the old recorder target) and reapply same-name/descendant targets in a safe order. - Adds new JUnit regression tests for less-verbose updates, shared targets, descendants, and manual intermediate levels.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
core/src/main/java/hudson/logging/LogRecorder.java |
Adds controller-side reconciliation logic for less-verbose target changes and refactors enable behavior to support it. |
test/src/test/java/hudson/logging/LogRecorderManagerTest.java |
Adds form-driven regression tests ensuring logger state updates correctly when recorder targets become less verbose, including overlap/descendant cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Hi @jenkinsci/core-pr-reviewers, this PR has been ready for review since July 10. I’ve addressed and resolved all Copilot feedback in |
Generated with core pull request: * jenkinsci/jenkins#27206 Includes pull requests: * jenkinsci/jenkins#27205 * jenkinsci/jenkins#27201 * jenkinsci/jenkins#27193 * jenkinsci/jenkins#27163 * jenkinsci/jenkins#27088 * jenkinsci/jenkins#27083 * jenkinsci/jenkins#27067 * jenkinsci/jenkins#27065 * jenkinsci/jenkins#27063 * jenkinsci/jenkins#27032 * jenkinsci/jenkins#26923 * jenkinsci/jenkins#26922 * jenkinsci/jenkins#26913 * jenkinsci/jenkins#26880 * jenkinsci/jenkins#26690 * jenkinsci/jenkins#26668 * jenkinsci/jenkins#26600 * jenkinsci/jenkins#26587 * jenkinsci/jenkins#11216 * jenkinsci/jenkins#10432 * jenkinsci/jenkins#8559
Fixes #16660
When a custom log recorder target was changed from a more verbose level to a less verbose level,
Target.enable()left the old explicit controller logger level in place. The new level was already loggable at the old setting, so the controller skipped the update even though the recorder configuration had changed.Keep the pre-submit targets long enough to identify these less-verbose changes. After the normal save, clear an explicit level only when it still matches the old recorder target, then reapply active same-name and descendant targets using the existing conditional behavior. This preserves overlapping recorder requirements and manual or inherited hierarchy levels. Agent broadcasts remain unchanged.
Testing done
FINESTremains installed after selecting a less verbose level.mvn -pl test -am -Dtest=hudson.logging.LogRecorderManagerTest -Dsurefire.failIfNoSpecifiedTests=false test(11 tests, 0 failures or errors)mvn -pl core spotless:checkScreenshots (UI changes only)
No UI changes.
Before
N/A
After
N/A
Proposed changelog entries
Proposed changelog category
/label bug
Proposed upgrade guidelines
N/A
Submitter checklist
@Restrictedor have@since TODOJavadocs, as appropriate.@Deprecated(since = "TODO")or@Deprecated(forRemoval = true, since = "TODO"), if applicable.evalto ease future introduction of Content Security Policy (CSP) directives (see documentation).Desired reviewers
N/A
Before the changes are marked as
ready-for-merge:Maintainer checklist
upgrade-guide-neededlabel is set and there is a Proposed upgrade guidelines section in the pull request title (see example).lts-candidateto be considered.