From c065f60771221579ab88f73b350950da78d548e8 Mon Sep 17 00:00:00 2001 From: om7057 Date: Sun, 9 Aug 2026 17:45:34 +0530 Subject: [PATCH] fix: reset OtelJulHandler circuit breaker on reconfiguration 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 #1291 --- .../opentelemetry/init/OtelJulHandler.java | 19 +++- .../init/OtelJulHandlerTest.java | 106 ++++++++++++++++++ 2 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 src/test/java/io/jenkins/plugins/opentelemetry/init/OtelJulHandlerTest.java diff --git a/src/main/java/io/jenkins/plugins/opentelemetry/init/OtelJulHandler.java b/src/main/java/io/jenkins/plugins/opentelemetry/init/OtelJulHandler.java index efb34aa80..8c002c003 100644 --- a/src/main/java/io/jenkins/plugins/opentelemetry/init/OtelJulHandler.java +++ b/src/main/java/io/jenkins/plugins/opentelemetry/init/OtelJulHandler.java @@ -15,6 +15,7 @@ import io.opentelemetry.api.logs.LoggerProvider; import io.opentelemetry.api.logs.Severity; import io.opentelemetry.context.Context; +import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; import io.opentelemetry.semconv.ExceptionAttributes; import io.opentelemetry.semconv.incubating.ThreadIncubatingAttributes; import java.io.PrintWriter; @@ -55,9 +56,11 @@ public OtelJulHandler() { } /** - * Circuit breaker + * Circuit breaker. Latches to {@code true} on the first emit failure and is cleared by + * {@link #afterConfiguration(ConfigProperties)} so a reconfigure (or a transient endpoint outage that + * resolves before the next reconfigure) does not disable log export for the remaining life of the JVM. */ - private boolean disabled = false; + private volatile boolean disabled = false; /** * Map the {@link LogRecord} data model onto the {@link io.opentelemetry.api.logs.LogRecordBuilder}. Unmapped fields include: @@ -162,6 +165,18 @@ public void flush() {} @Override public void close() throws SecurityException {} + /** + * Re-fetch the logger provider and clear the circuit breaker so a reconfigure (JCasC reload, endpoint + * recovery, etc.) resumes log export even if a prior emit failure had disabled it. + */ + @Override + public void afterConfiguration(ConfigProperties configProperties) { + this.loggerProvider = openTelemetry.getLogsBridge(); + this.captureExperimentalAttributes = configProperties.getBoolean( + "otel.instrumentation.java-util-logging.experimental-log-attributes", false); + this.disabled = false; + } + @PostConstruct public void postConstruct() { this.loggerProvider = openTelemetry.getLogsBridge(); diff --git a/src/test/java/io/jenkins/plugins/opentelemetry/init/OtelJulHandlerTest.java b/src/test/java/io/jenkins/plugins/opentelemetry/init/OtelJulHandlerTest.java new file mode 100644 index 000000000..e767d56d5 --- /dev/null +++ b/src/test/java/io/jenkins/plugins/opentelemetry/init/OtelJulHandlerTest.java @@ -0,0 +1,106 @@ +/* + * Copyright The Original Author or Authors + * SPDX-License-Identifier: Apache-2.0 + */ +package io.jenkins.plugins.opentelemetry.init; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import io.jenkins.plugins.opentelemetry.api.ReconfigurableOpenTelemetry; +import io.opentelemetry.api.logs.LogRecordBuilder; +import io.opentelemetry.api.logs.Logger; +import io.opentelemetry.api.logs.LoggerProvider; +import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties; +import java.lang.reflect.Field; +import java.util.Collections; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.logging.Level; +import java.util.logging.LogRecord; +import org.junit.jupiter.api.Test; +import org.mockito.Answers; + +/** + * Regression tests for issue #1291: + * {@link OtelJulHandler} used to permanently disable log export after a single emit failure, with no code + * path that ever reset the circuit breaker. + */ +class OtelJulHandlerTest { + + @Test + void handlerNeverRecoversWithoutAfterConfiguration() throws Exception { + AtomicInteger emitAttempts = new AtomicInteger(); + OtelJulHandler handler = newHandler(emitAttempts); + + // 1st record: emit() throws, the circuit breaker latches disabled = true. + handler.publish(new LogRecord(Level.INFO, "first record - endpoint transiently down")); + // 2nd record: the endpoint has recovered, but the handler short-circuits on `disabled`. + handler.publish(new LogRecord(Level.INFO, "second record - endpoint back up")); + + assertEquals( + 1, + emitAttempts.get(), + "without a reset path, emit() is attempted only once and every subsequent log is dropped"); + assertTrue(getDisabled(handler), "disabled must latch true after the first emit failure"); + } + + @Test + void afterConfigurationResetsTheCircuitBreaker() throws Exception { + AtomicInteger emitAttempts = new AtomicInteger(); + OtelJulHandler handler = newHandler(emitAttempts); + + handler.publish(new LogRecord(Level.INFO, "first record - endpoint transiently down")); + assertTrue(getDisabled(handler), "disabled must latch true after the first emit failure"); + + handler.afterConfiguration(DefaultConfigProperties.createFromMap(Collections.emptyMap())); + assertTrue(!getDisabled(handler), "afterConfiguration() must clear the circuit breaker"); + + handler.publish(new LogRecord(Level.INFO, "second record - after reconfiguration")); + assertEquals( + 2, + emitAttempts.get(), + "after afterConfiguration() resets the breaker, publish() must attempt emit() again"); + } + + private static OtelJulHandler newHandler(AtomicInteger emitAttempts) throws Exception { + LogRecordBuilder builder = mock(LogRecordBuilder.class, Answers.RETURNS_SELF); + doAnswer(invocation -> { + if (emitAttempts.incrementAndGet() == 1) { + throw new IllegalStateException("simulated transient OTLP emit failure"); + } + return null; + }) + .when(builder) + .emit(); + + Logger otelLogger = mock(Logger.class); + when(otelLogger.logRecordBuilder()).thenReturn(builder); + LoggerProvider provider = mock(LoggerProvider.class); + when(provider.get(anyString())).thenReturn(otelLogger); + + OtelJulHandler handler = new OtelJulHandler(); + setField(handler, "loggerProvider", provider); + + ReconfigurableOpenTelemetry openTelemetry = mock(ReconfigurableOpenTelemetry.class); + when(openTelemetry.getLogsBridge()).thenReturn(provider); + setField(handler, "openTelemetry", openTelemetry); + + return handler; + } + + private static void setField(OtelJulHandler handler, String fieldName, Object value) throws Exception { + Field field = OtelJulHandler.class.getDeclaredField(fieldName); + field.setAccessible(true); + field.set(handler, value); + } + + private static boolean getDisabled(OtelJulHandler handler) throws Exception { + Field field = OtelJulHandler.class.getDeclaredField("disabled"); + field.setAccessible(true); + return (boolean) field.get(handler); + } +}