diff --git a/javaagent-internal-logging-application/src/main/java/io/opentelemetry/javaagent/logging/application/ApplicationLoggerFactory.java b/javaagent-internal-logging-application/src/main/java/io/opentelemetry/javaagent/logging/application/ApplicationLoggerFactory.java index 1050ad04311e..efb4a989a2d6 100644 --- a/javaagent-internal-logging-application/src/main/java/io/opentelemetry/javaagent/logging/application/ApplicationLoggerFactory.java +++ b/javaagent-internal-logging-application/src/main/java/io/opentelemetry/javaagent/logging/application/ApplicationLoggerFactory.java @@ -9,13 +9,14 @@ import io.opentelemetry.javaagent.bootstrap.logging.ApplicationLoggerBridge; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; import javax.annotation.Nullable; final class ApplicationLoggerFactory extends ApplicationLoggerBridge implements InternalLogger.Factory { - private final AtomicBoolean installed = new AtomicBoolean(); + private final AtomicReference> installedFactoryClass = + new AtomicReference<>(); @Nullable private volatile InternalLogger.Factory actual = null; private final ConcurrentMap inMemoryLoggers = new ConcurrentHashMap<>(); @@ -28,8 +29,12 @@ final class ApplicationLoggerFactory extends ApplicationLoggerBridge @Override protected void install(InternalLogger.Factory applicationLoggerFactory) { + Class incomingClass = applicationLoggerFactory.getClass(); // just use the first bridge that gets discovered and ignore the rest - if (!installed.compareAndSet(false, true)) { + if (!installedFactoryClass.compareAndSet(null, incomingClass)) { + if (incomingClass.equals(installedFactoryClass.get())) { + return; + } applicationLoggerFactory .create(ApplicationLoggerBridge.class.getName()) .log( diff --git a/javaagent-internal-logging-application/src/test/java/io/opentelemetry/javaagent/logging/application/ApplicationLoggerFactoryTest.java b/javaagent-internal-logging-application/src/test/java/io/opentelemetry/javaagent/logging/application/ApplicationLoggerFactoryTest.java index 417631a896fe..49eb61e443cc 100644 --- a/javaagent-internal-logging-application/src/test/java/io/opentelemetry/javaagent/logging/application/ApplicationLoggerFactoryTest.java +++ b/javaagent-internal-logging-application/src/test/java/io/opentelemetry/javaagent/logging/application/ApplicationLoggerFactoryTest.java @@ -12,6 +12,7 @@ import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -53,25 +54,49 @@ void shouldNotDuplicateLoggers() { @Test void shouldOnlyInstallTheFirstBridge() { + InternalLogger.Factory factory1 = mock(FirstFactory.class); + InternalLogger.Factory factory2 = mock(SecondFactory.class); + when(logStore.currentSize()).thenReturn(1, 0, 0); - when(applicationLoggerBridge.create(any())).thenReturn(applicationLogger); + when(factory2.create(any())).thenReturn(applicationLogger); - underTest.install(applicationLoggerBridge); + underTest.install(factory1); verify(logStore, times(3)).currentSize(); - verify(logStore).flush(applicationLoggerBridge); - verify(logStore).setApplicationLoggerFactory(applicationLoggerBridge); + verify(logStore).flush(factory1); + verify(logStore).setApplicationLoggerFactory(factory1); verify(logStore).freeMemory(); - underTest.install(applicationLoggerBridge); + underTest.install(factory2); // verify logged warning - verify(applicationLoggerBridge).create(ApplicationLoggerBridge.class.getName()); + verify(factory2).create(ApplicationLoggerBridge.class.getName()); verify(applicationLogger).log(eq(WARN), anyString(), isNull()); verifyNoMoreInteractions(logStore); } + @Test + void shouldIgnoreSubsequentInstallationsOfSameClass() { + InternalLogger.Factory factory1 = mock(FirstFactory.class); + InternalLogger.Factory factory2 = mock(FirstFactory.class); + + when(logStore.currentSize()).thenReturn(1, 0, 0); + + underTest.install(factory1); + + verify(logStore, times(3)).currentSize(); + verify(logStore).flush(factory1); + verify(logStore).setApplicationLoggerFactory(factory1); + verify(logStore).freeMemory(); + + underTest.install(factory2); + + // verify no warning logged and no factory interaction for second install + verify(factory2, never()).create(anyString()); + verifyNoMoreInteractions(logStore); + } + @Test void shouldReplaceLoggerAfterTheBridgeIsInstalled() { InternalLogger beforeInstall = underTest.create("logger"); @@ -90,4 +115,18 @@ void shouldReplaceLoggerAfterTheBridgeIsInstalled() { verify(applicationLogger).log(INFO, "after", null); verify(logStore, never()).write(InMemoryLog.create("logger", INFO, "after", null)); } + + static class FirstFactory implements InternalLogger.Factory { + @Override + public InternalLogger create(String name) { + return null; + } + } + + static class SecondFactory implements InternalLogger.Factory { + @Override + public InternalLogger create(String name) { + return null; + } + } }