fix: reset OtelJulHandler circuit breaker on reconfiguration - #1295
Open
om7057 wants to merge 1 commit into
Open
fix: reset OtelJulHandler circuit breaker on reconfiguration#1295om7057 wants to merge 1 commit into
om7057 wants to merge 1 commit into
Conversation
OtelJulHandler latches a disabled flag on the first RuntimeException thrown while emitting a log record, and nothing ever cleared it. A single transient failure, such as the OTLP endpoint being briefly unreachable, silently and permanently stopped all controller and pipeline log export for the remaining life of the JVM, with traces unaffected since they use a separate exporter, which masked the outage. The class already implements OpenTelemetryLifecycleListener but did not override afterConfiguration, so neither a JCasC reload nor any other SDK reconfiguration ever re-fetched the logger provider or reset the breaker. Override afterConfiguration to re-fetch the logger provider and clear disabled, so a reconfigure recovers a previously tripped handler. Fixes jenkinsci#1291
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
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.
Fixes #1291
OtelJulHandlerinstalls a circuit breaker on its JUL to OTLP log bridge: the firstRuntimeExceptionthrown from the emit path setsdisabled = true, andpublish()short-circuits on that flag from then on. Nothing in the class ever set it back tofalse. A single transient failure, for example the OTLP endpoint being briefly unavailable, or a reconfigure/shutdown race, permanently stops all controller and pipeline log export for the remaining life of the JVM. Traces are unaffected since they go through a separate exporter, which masks the outage.The class already implements
OpenTelemetryLifecycleListenerbut did not overrideafterConfiguration(ConfigProperties), so a JCasC reload or any other SDK reconfiguration never re-fetched the logger provider or reset the breaker. The only thing that previously cleared it was a brand new handler instance, meaning a JVM restart or plugin reload.This change overrides
afterConfigurationto re-fetch the logger provider fromopenTelemetry.getLogsBridge()and cleardisabled, so a reconfigure recovers a previously tripped handler. Thedisabledfield is also madevolatilesincepublish()can run on arbitrary logging threads whileafterConfiguration()runs on the reconfiguration thread.Testing done
Added
OtelJulHandlerTestwith two tests:handlerNeverRecoversWithoutAfterConfiguration: reproduces the original bug, a single emit failure permanently disables the handler and every subsequentpublish()is a no-op.afterConfigurationResetsTheCircuitBreaker: asserts callingafterConfiguration()clears the breaker and a subsequentpublish()attempts to emit again.Ran the full existing test suite locally on JDK 21 (matching this repo's Jenkinsfile): 238 tests run, 0 failures, 0 errors, 1 skipped (pre-existing skip, unrelated to this change). Also verified
./mvnw spotless:checkand./mvnw spotbugs:checkpass clean.Submitter checklist