This is an automated email from the ASF dual-hosted git repository. royteeuwen pushed a commit to branch logstore-extensions in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-commons-log.git
commit 348a7e254df54474587a9162771dd0e56d4e2c33 Author: Roy Teeuwen <[email protected]> AuthorDate: Sun May 24 20:54:12 2026 +0200 SLING-13212: Add LogStoreListener that allows reading of the logs on a push-based API --- .../logback/internal/store/LogStoreAppender.java | 12 ++- .../log/logback/internal/store/LogStoreImpl.java | 21 +++++ .../logback/internal/store/LogStoreRegistrar.java | 92 +++++++++++++++++++++- .../sling/commons/log/logback/store/LogEntry.java | 10 +++ .../log/logback/store/LogStoreListener.java | 44 +++++++++++ .../commons/log/logback/store/package-info.java | 2 +- .../integration/ITLogStoreRegistrarLifecycle.java | 80 +++++++++++++++++++ .../internal/store/LogStoreAppenderTest.java | 24 ++++++ .../logback/internal/store/LogStoreImplTest.java | 55 ++++++++++++- 9 files changed, 328 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/apache/sling/commons/log/logback/internal/store/LogStoreAppender.java b/src/main/java/org/apache/sling/commons/log/logback/internal/store/LogStoreAppender.java index c22f5d4..c86b112 100644 --- a/src/main/java/org/apache/sling/commons/log/logback/internal/store/LogStoreAppender.java +++ b/src/main/java/org/apache/sling/commons/log/logback/internal/store/LogStoreAppender.java @@ -47,13 +47,16 @@ public class LogStoreAppender extends AppenderBase<ILoggingEvent> { return; } + IThrowableProxy throwableProxy = eventObject.getThrowableProxy(); store.append(new LogEntry( eventObject.getTimeStamp(), logLevel, eventObject.getLoggerName(), eventObject.getThreadName(), eventObject.getFormattedMessage(), - getThrowableText(eventObject), + throwableProxy != null ? throwableProxy.getClassName() : null, + throwableProxy != null ? throwableProxy.getMessage() : null, + throwableProxy != null ? formatThrowable(throwableProxy) : null, eventObject.getMDCPropertyMap())); } @@ -74,12 +77,7 @@ public class LogStoreAppender extends AppenderBase<ILoggingEvent> { } } - private String getThrowableText(ILoggingEvent eventObject) { - IThrowableProxy throwableProxy = eventObject.getThrowableProxy(); - if (throwableProxy == null) { - return null; - } - + private String formatThrowable(IThrowableProxy throwableProxy) { StringBuilder text = new StringBuilder(); appendThrowable(text, throwableProxy, null); return text.toString(); diff --git a/src/main/java/org/apache/sling/commons/log/logback/internal/store/LogStoreImpl.java b/src/main/java/org/apache/sling/commons/log/logback/internal/store/LogStoreImpl.java index d62f066..ecc4386 100644 --- a/src/main/java/org/apache/sling/commons/log/logback/internal/store/LogStoreImpl.java +++ b/src/main/java/org/apache/sling/commons/log/logback/internal/store/LogStoreImpl.java @@ -24,11 +24,14 @@ import java.util.Deque; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.concurrent.CopyOnWriteArraySet; import java.util.regex.Pattern; import org.apache.sling.commons.log.logback.store.LogEntry; import org.apache.sling.commons.log.logback.store.LogLevel; import org.apache.sling.commons.log.logback.store.LogStore; +import org.apache.sling.commons.log.logback.store.LogStoreListener; public class LogStoreImpl implements LogStore { @@ -36,6 +39,7 @@ public class LogStoreImpl implements LogStore { private final Object lock = new Object(); private final Deque<LogEntry> entries = new ArrayDeque<>(); + private final Set<LogStoreListener> listeners = new CopyOnWriteArraySet<>(); private int maxEntriesKept; public LogStoreImpl(int maxEntriesKept) { @@ -47,6 +51,23 @@ public class LogStoreImpl implements LogStore { entries.addLast(snapshot); trimToSize(); } + // Notify listeners outside the lock so a slow or re-entrant listener does + // not stall other producers waiting to append. + for (LogStoreListener listener : listeners) { + listener.onEntry(snapshot); + } + } + + public void addListener(LogStoreListener listener) { + if (listener != null) { + listeners.add(listener); + } + } + + public void removeListener(LogStoreListener listener) { + if (listener != null) { + listeners.remove(listener); + } } @Override diff --git a/src/main/java/org/apache/sling/commons/log/logback/internal/store/LogStoreRegistrar.java b/src/main/java/org/apache/sling/commons/log/logback/internal/store/LogStoreRegistrar.java index eb87229..418b303 100644 --- a/src/main/java/org/apache/sling/commons/log/logback/internal/store/LogStoreRegistrar.java +++ b/src/main/java/org/apache/sling/commons/log/logback/internal/store/LogStoreRegistrar.java @@ -20,32 +20,46 @@ package org.apache.sling.commons.log.logback.internal.store; import java.util.Dictionary; import java.util.Hashtable; +import java.util.Set; +import java.util.concurrent.CopyOnWriteArraySet; import ch.qos.logback.core.Appender; import org.apache.sling.commons.log.logback.internal.LogConstants; import org.apache.sling.commons.log.logback.store.LogStore; +import org.apache.sling.commons.log.logback.store.LogStoreListener; import org.jetbrains.annotations.Nullable; import org.osgi.framework.BundleContext; import org.osgi.framework.Constants; +import org.osgi.framework.ServiceReference; import org.osgi.framework.ServiceRegistration; import org.osgi.service.cm.ManagedService; import org.osgi.util.converter.Converters; +import org.osgi.util.tracker.ServiceTracker; +import org.osgi.util.tracker.ServiceTrackerCustomizer; public class LogStoreRegistrar { static final String PID = "org.apache.sling.commons.log.LogStore"; static final String PROP_MAX_ENTRIES = "maxEntries"; + static final String PROP_LOGGERS = "loggers"; + static final String[] DEFAULT_LOGGERS = {"ROOT"}; private ServiceRegistration<LogStore> storeRegistration; private ServiceRegistration<Appender> appenderRegistration; private ServiceRegistration<ManagedService> configRegistration; + private ServiceTracker<LogStoreListener, LogStoreListener> listenerTracker; + private final Set<LogStoreListener> knownListeners = new CopyOnWriteArraySet<>(); private BundleContext bundleContext; private LogStoreImpl store; private LogStoreAppender appender; + private String[] activeLoggers; public void start(BundleContext context) { this.bundleContext = context; + listenerTracker = new ServiceTracker<>(context, LogStoreListener.class, new ListenerTrackerCustomizer()); + listenerTracker.open(); + Dictionary<String, Object> configProps = new Hashtable<>(); configProps.put(Constants.SERVICE_VENDOR, LogConstants.ASF_SERVICE_VENDOR); configProps.put(Constants.SERVICE_DESCRIPTION, "Log Store Configurator"); @@ -60,6 +74,11 @@ public class LogStoreRegistrar { configRegistration.unregister(); configRegistration = null; } + if (listenerTracker != null) { + listenerTracker.close(); + listenerTracker = null; + } + knownListeners.clear(); bundleContext = null; } @@ -74,19 +93,31 @@ public class LogStoreRegistrar { .defaultValue(LogStoreImpl.DEFAULT_MAX_ENTRIES) .to(Integer.class); + String[] loggers = Converters.standardConverter() + .convert(properties.get(PROP_LOGGERS)) + .defaultValue(DEFAULT_LOGGERS) + .to(String[].class); + if (loggers == null || loggers.length == 0) { + loggers = DEFAULT_LOGGERS; + } + if (store == null) { - activate(maxEntries); + activate(maxEntries, loggers); } else { store.setMaxEntries(maxEntries); + applyLoggerConfig(loggers); } } - private void activate(int maxEntries) { + private void activate(int maxEntries, String[] loggers) { if (bundleContext == null || store != null) { return; } store = new LogStoreImpl(maxEntries); + for (LogStoreListener listener : knownListeners) { + store.addListener(listener); + } appender = new LogStoreAppender(store); Dictionary<String, Object> serviceProps = new Hashtable<>(); @@ -97,8 +128,33 @@ public class LogStoreRegistrar { Dictionary<String, Object> appenderProps = new Hashtable<>(); appenderProps.put(Constants.SERVICE_VENDOR, LogConstants.ASF_SERVICE_VENDOR); appenderProps.put(Constants.SERVICE_DESCRIPTION, "Log Store Appender"); - appenderProps.put("loggers", "ROOT"); + appenderProps.put(PROP_LOGGERS, loggers); appenderRegistration = bundleContext.registerService(Appender.class, appender, appenderProps); + activeLoggers = loggers; + } + + private void applyLoggerConfig(String[] loggers) { + if (appenderRegistration == null || equalLoggers(activeLoggers, loggers)) { + return; + } + Dictionary<String, Object> appenderProps = new Hashtable<>(); + appenderProps.put(Constants.SERVICE_VENDOR, LogConstants.ASF_SERVICE_VENDOR); + appenderProps.put(Constants.SERVICE_DESCRIPTION, "Log Store Appender"); + appenderProps.put(PROP_LOGGERS, loggers); + appenderRegistration.setProperties(appenderProps); + activeLoggers = loggers; + } + + private boolean equalLoggers(String[] a, String[] b) { + if (a == null || b == null || a.length != b.length) { + return false; + } + for (int i = 0; i < a.length; i++) { + if (!a[i].equals(b[i])) { + return false; + } + } + return true; } private void deactivate() { @@ -113,5 +169,35 @@ public class LogStoreRegistrar { appender = null; store = null; + activeLoggers = null; + } + + private class ListenerTrackerCustomizer implements ServiceTrackerCustomizer<LogStoreListener, LogStoreListener> { + + @Override + public LogStoreListener addingService(ServiceReference<LogStoreListener> reference) { + LogStoreListener listener = bundleContext.getService(reference); + if (listener != null) { + knownListeners.add(listener); + if (store != null) { + store.addListener(listener); + } + } + return listener; + } + + @Override + public void modifiedService(ServiceReference<LogStoreListener> reference, LogStoreListener service) { + // service properties changed; no internal state depends on them + } + + @Override + public void removedService(ServiceReference<LogStoreListener> reference, LogStoreListener service) { + knownListeners.remove(service); + if (store != null) { + store.removeListener(service); + } + bundleContext.ungetService(reference); + } } } diff --git a/src/main/java/org/apache/sling/commons/log/logback/store/LogEntry.java b/src/main/java/org/apache/sling/commons/log/logback/store/LogEntry.java index d7cc252..a576ddd 100644 --- a/src/main/java/org/apache/sling/commons/log/logback/store/LogEntry.java +++ b/src/main/java/org/apache/sling/commons/log/logback/store/LogEntry.java @@ -27,6 +27,14 @@ import java.util.Map; * * <p>Stores only the lightweight, stable parts of a log event so the log store * does not retain full logging event object graphs.</p> + * + * <p>The throwable that accompanies the event, if any, is decomposed into + * three optional fields. {@link #throwableClassName()} and {@link #throwableMessage()} + * are the binary name and detail message of the leaf {@code Throwable} for + * structured consumption (alerting, attribute mapping). {@link #throwableText()} + * is the formatted representation of the full throwable chain including any + * {@code cause} and suppressed throwables; it is suitable for verbatim display + * such as stack traces in log viewers.</p> */ public record LogEntry( long timeMillis, @@ -34,6 +42,8 @@ public record LogEntry( String loggerName, String threadName, String formattedMessage, + String throwableClassName, + String throwableMessage, String throwableText, Map<String, String> mdc) { diff --git a/src/main/java/org/apache/sling/commons/log/logback/store/LogStoreListener.java b/src/main/java/org/apache/sling/commons/log/logback/store/LogStoreListener.java new file mode 100644 index 0000000..25ed4f3 --- /dev/null +++ b/src/main/java/org/apache/sling/commons/log/logback/store/LogStoreListener.java @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.sling.commons.log.logback.store; + +import org.osgi.annotation.versioning.ConsumerType; + +/** + * Whiteboard-style callback for entries appended to the {@link LogStore}. + * + * <p>Listeners are notified synchronously on the logging thread after the + * entry has been stored, so implementations must not block or perform expensive + * work. They must also be careful not to log at levels the store captures, to + * avoid re-entrant notification.</p> + * + * <p>Listeners registered before the {@code LogStore} is configured begin + * receiving callbacks as soon as the store activates. Listeners that + * unregister stop receiving callbacks before the next entry is appended.</p> + */ +@ConsumerType +public interface LogStoreListener { + + /** + * Notification that a new {@link LogEntry} was appended to the store. + * + * @param entry the entry that was just appended; never {@code null} + */ + void onEntry(LogEntry entry); +} diff --git a/src/main/java/org/apache/sling/commons/log/logback/store/package-info.java b/src/main/java/org/apache/sling/commons/log/logback/store/package-info.java index 76ac0cf..c1522c7 100644 --- a/src/main/java/org/apache/sling/commons/log/logback/store/package-info.java +++ b/src/main/java/org/apache/sling/commons/log/logback/store/package-info.java @@ -23,7 +23,7 @@ * <p>The API is intentionally decoupled from slf4j and logback to prevent binding to those APIs * directly, which hinders evolution and is problematic when upgrading dependency versions.</p> */ -@Version("1.0.0") +@Version("1.1.0") package org.apache.sling.commons.log.logback.store; import org.osgi.annotation.versioning.Version; diff --git a/src/test/java/org/apache/sling/commons/log/logback/integration/ITLogStoreRegistrarLifecycle.java b/src/test/java/org/apache/sling/commons/log/logback/integration/ITLogStoreRegistrarLifecycle.java index 2e1a123..c09896a 100644 --- a/src/test/java/org/apache/sling/commons/log/logback/integration/ITLogStoreRegistrarLifecycle.java +++ b/src/test/java/org/apache/sling/commons/log/logback/integration/ITLogStoreRegistrarLifecycle.java @@ -20,21 +20,35 @@ package org.apache.sling.commons.log.logback.integration; import javax.inject.Inject; +import java.util.Arrays; import java.util.Dictionary; +import java.util.HashSet; import java.util.Hashtable; +import java.util.List; +import java.util.Set; +import java.util.concurrent.CopyOnWriteArrayList; import ch.qos.logback.core.Appender; +import org.apache.sling.commons.log.logback.store.LogEntry; import org.apache.sling.commons.log.logback.store.LogStore; +import org.apache.sling.commons.log.logback.store.LogStoreListener; import org.junit.Test; import org.junit.runner.RunWith; import org.ops4j.pax.exam.Option; import org.ops4j.pax.exam.junit.PaxExam; import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy; import org.ops4j.pax.exam.spi.reactors.PerClass; +import org.osgi.framework.ServiceReference; +import org.osgi.framework.ServiceRegistration; import org.osgi.service.cm.Configuration; import org.osgi.service.cm.ConfigurationAdmin; +import org.osgi.util.converter.Converters; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import static org.ops4j.pax.exam.CoreOptions.composite; import static org.ops4j.pax.exam.CoreOptions.mavenBundle; @@ -44,6 +58,7 @@ public class ITLogStoreRegistrarLifecycle extends LogTestBase { private static final String LOG_STORE_PID = "org.apache.sling.commons.log.LogStore"; private static final String MAX_ENTRIES = "maxEntries"; + private static final String LOGGERS = "loggers"; @Inject private ConfigurationAdmin ca; @@ -88,4 +103,69 @@ public class ITLogStoreRegistrarLifecycle extends LogTestBase { assertEquals(0, bundleContext.getServiceReferences(LogStore.class, null).size()); assertEquals(0, bundleContext.getServiceReferences(Appender.class, null).size()); } + + @Test + public void testLoggersConfiguration() throws Exception { + Configuration config = ca.getConfiguration(LOG_STORE_PID, null); + try { + Dictionary<String, Object> properties = new Hashtable<>(); + properties.put(MAX_ENTRIES, 5); + properties.put(LOGGERS, new String[] {"ROOT", "test.other"}); + config.update(properties); + delay(); + + assertEquals(Set.of("ROOT", "test.other"), readLoggersProperty()); + + properties.put(LOGGERS, new String[] {"ROOT"}); + config.update(properties); + delay(); + + assertEquals(Set.of("ROOT"), readLoggersProperty()); + } finally { + config.delete(); + delay(); + } + } + + @Test + public void testListenerReceivesAppendedEntry() throws Exception { + Configuration config = ca.getConfiguration(LOG_STORE_PID, null); + List<LogEntry> received = new CopyOnWriteArrayList<>(); + ServiceRegistration<LogStoreListener> listenerRegistration = + bundleContext.registerService(LogStoreListener.class, received::add, null); + + try { + Dictionary<String, Object> properties = new Hashtable<>(); + properties.put(MAX_ENTRIES, 5); + config.update(properties); + delay(); + + Logger logger = LoggerFactory.getLogger("test.listener.logger"); + logger.error("integration-marker"); + + // Allow the appender thread to fan the entry out to the listener. + for (int i = 0; + i < 20 && received.stream().noneMatch(e -> "integration-marker".equals(e.formattedMessage())); + i++) { + Thread.sleep(50); + } + + assertTrue( + "Listener did not receive log entry: " + received, + received.stream().anyMatch(e -> "integration-marker".equals(e.formattedMessage()))); + } finally { + listenerRegistration.unregister(); + config.delete(); + delay(); + } + } + + private Set<String> readLoggersProperty() throws Exception { + ServiceReference<?>[] refs = bundleContext.getServiceReferences(Appender.class.getName(), null); + assertNotNull("expected an Appender registration", refs); + assertEquals(1, refs.length); + Object property = refs[0].getProperty(LOGGERS); + return new HashSet<>( + Arrays.asList(Converters.standardConverter().convert(property).to(String[].class))); + } } diff --git a/src/test/java/org/apache/sling/commons/log/logback/internal/store/LogStoreAppenderTest.java b/src/test/java/org/apache/sling/commons/log/logback/internal/store/LogStoreAppenderTest.java index 7259351..1e0de15 100644 --- a/src/test/java/org/apache/sling/commons/log/logback/internal/store/LogStoreAppenderTest.java +++ b/src/test/java/org/apache/sling/commons/log/logback/internal/store/LogStoreAppenderTest.java @@ -31,6 +31,7 @@ import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; class LogStoreAppenderTest { @@ -60,9 +61,32 @@ class LogStoreAppenderTest { assertEquals("worker-1", logs.get(0).threadName()); assertEquals(LogLevel.ERROR, logs.get(0).level()); assertEquals(Map.of("requestId", "123"), logs.get(0).mdc()); + assertEquals("java.lang.RuntimeException", logs.get(0).throwableClassName()); + assertEquals("error", logs.get(0).throwableMessage()); assertNotNull(logs.get(0).throwableText()); } + @Test + void appenderLeavesThrowableFieldsNullWhenNoException() { + LogStoreImpl store = new LogStoreImpl(5); + LogStoreAppender appender = new LogStoreAppender(store); + + LoggerContext context = new LoggerContext(); + appender.setContext(context); + Logger logger = context.getLogger("test.logger"); + LoggingEvent event = new LoggingEvent(getClass().getName(), logger, Level.INFO, "no-exception", null, null); + event.setMDCPropertyMap(Map.of()); + event.setThreadName("worker-2"); + + appender.append(event); + + List<LogEntry> logs = store.getRecent(null, LogLevel.TRACE, 10); + assertEquals(1, logs.size()); + assertNull(logs.get(0).throwableClassName()); + assertNull(logs.get(0).throwableMessage()); + assertNull(logs.get(0).throwableText()); + } + @Test void appenderFormatsSuppressedCauseAndCommonFrames() { LogStoreImpl store = new LogStoreImpl(5); diff --git a/src/test/java/org/apache/sling/commons/log/logback/internal/store/LogStoreImplTest.java b/src/test/java/org/apache/sling/commons/log/logback/internal/store/LogStoreImplTest.java index 2d056bc..060e530 100644 --- a/src/test/java/org/apache/sling/commons/log/logback/internal/store/LogStoreImplTest.java +++ b/src/test/java/org/apache/sling/commons/log/logback/internal/store/LogStoreImplTest.java @@ -18,6 +18,7 @@ */ package org.apache.sling.commons.log.logback.internal.store; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.regex.Pattern; @@ -25,9 +26,12 @@ import java.util.stream.Collectors; import org.apache.sling.commons.log.logback.store.LogEntry; import org.apache.sling.commons.log.logback.store.LogLevel; +import org.apache.sling.commons.log.logback.store.LogStoreListener; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; class LogStoreImplTest { @@ -87,7 +91,56 @@ class LogStoreImplTest { logs.stream().map(LogEntry::formattedMessage).collect(Collectors.toList())); } + @Test + void notifiesListenersInRegistrationOrder() { + LogStoreImpl store = new LogStoreImpl(10); + List<LogEntry> received = new ArrayList<>(); + LogStoreListener listener = received::add; + + store.addListener(listener); + LogEntry entry1 = logEntry(1L, LogLevel.INFO, "first"); + LogEntry entry2 = logEntry(2L, LogLevel.INFO, "second"); + store.append(entry1); + store.append(entry2); + + assertEquals(2, received.size()); + assertSame(entry1, received.get(0)); + assertSame(entry2, received.get(1)); + } + + @Test + void stopsNotifyingAfterRemoveListener() { + LogStoreImpl store = new LogStoreImpl(10); + List<LogEntry> received = new ArrayList<>(); + LogStoreListener listener = received::add; + + store.addListener(listener); + store.append(logEntry(1L, LogLevel.INFO, "before")); + store.removeListener(listener); + store.append(logEntry(2L, LogLevel.INFO, "after")); + + assertEquals(1, received.size()); + assertEquals("before", received.get(0).formattedMessage()); + } + + @Test + void listenerExceptionDoesNotPreventStorage() { + LogStoreImpl store = new LogStoreImpl(10); + store.addListener(entry -> { + throw new RuntimeException("listener boom"); + }); + + try { + store.append(logEntry(1L, LogLevel.INFO, "stored")); + } catch (RuntimeException expected) { + // listener propagated; storage already happened before the throw + } + List<LogEntry> stored = store.getRecent(null, LogLevel.TRACE, 10); + assertEquals(1, stored.size()); + assertTrue("stored".equals(stored.get(0).formattedMessage())); + } + private LogEntry logEntry(long timeMillis, LogLevel level, String message) { - return new LogEntry(timeMillis, level, "logger", "thread", message, null, Map.of()); + return new LogEntry(timeMillis, level, "logger", "thread", message, null, null, null, Map.of()); } }
