This is an automated email from the ASF dual-hosted git repository. rombert pushed a commit to branch issue/remove-logback-from-apis in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-mcp-server-contributions.git
commit 44daa94cf6aadc867b76beef52c7cfcff8dd0cec Author: Robert Munteanu <[email protected]> AuthorDate: Tue May 12 16:19:57 2026 +0200 fix: stop exposing logback APIs in LogTool support classes --- .../server/impl/contribs/LogToolContribution.java | 55 ++++++++++++---------- .../server/impl/contribs/internal/LogSnapshot.java | 46 ++++++++++++++++-- .../contribs/internal/StructuredLogBuffer.java | 19 ++++++-- .../internal/StructuredLogBufferAppender.java | 9 ++-- .../impl/contribs/internal/LogSnapshotTest.java} | 24 +++------- .../internal/StructuredLogBufferAppenderTest.java | 2 +- .../contribs/internal/StructuredLogBufferTest.java | 20 ++++---- 7 files changed, 109 insertions(+), 66 deletions(-) diff --git a/src/main/java/org/apache/sling/mcp/server/impl/contribs/LogToolContribution.java b/src/main/java/org/apache/sling/mcp/server/impl/contribs/LogToolContribution.java index 0ec2db3..bbd11c9 100644 --- a/src/main/java/org/apache/sling/mcp/server/impl/contribs/LogToolContribution.java +++ b/src/main/java/org/apache/sling/mcp/server/impl/contribs/LogToolContribution.java @@ -24,8 +24,8 @@ import java.util.List; import java.util.Map; import java.util.regex.Pattern; import java.util.regex.PatternSyntaxException; +import java.util.stream.Collectors; -import ch.qos.logback.classic.Level; import io.modelcontextprotocol.json.McpJsonMapperSupplier; import io.modelcontextprotocol.server.McpStatelessServerFeatures.SyncToolSpecification; import io.modelcontextprotocol.spec.McpSchema.CallToolResult; @@ -66,8 +66,8 @@ public class LogToolContribution implements McpServerContribution { }, "logLevel" : { "type" : "string", - "description" : "Minimum log level to return. Options: ERROR, WARN, INFO, DEBUG, TRACE. Defaults to ERROR.", - "enum" : ["ERROR", "WARN", "INFO", "DEBUG", "TRACE"] + "description" : "Minimum log level to return. Options: %s. Defaults to %s.", + "enum" : %s }, "maxEntries" : { "type" : "integer", @@ -77,13 +77,17 @@ public class LogToolContribution implements McpServerContribution { } } } - """; + """.formatted( + validLogLevelValuesAsCsv(), + LogSnapshot.getHighestLogLevelName(), + validLogLevelValuesAsJsonSchemaEnum()); return List.of(new SyncToolSpecification( Tool.builder() .name("logs") .description("Retrieve logs with optional filtering. " - + "Supports filtering by regex pattern, log level (ERROR, WARN, INFO, DEBUG, TRACE), " + + "Supports filtering by regex pattern, log level (" + validLogLevelValuesAsCsv() + + "), " + "and maximum number of entries. Returns most recent logs first.") .inputSchema(jsonMapper.get(), schema) .build(), @@ -99,16 +103,16 @@ public class LogToolContribution implements McpServerContribution { maxEntries = Math.min(maxEntries, 1000); // Cap at 1000 } - Level minLogLevel = Level.ERROR; + String minLogLevel = LogSnapshot.getHighestLogLevelName(); if (logLevelStr != null && !logLevelStr.isEmpty()) { - minLogLevel = parseLogLevel(logLevelStr); - if (minLogLevel == null) { + if (!LogSnapshot.isValidLogLevel(logLevelStr)) { return CallToolResult.builder() - .addTextContent("Invalid log level: " + logLevelStr - + ". Valid options are: ERROR, WARN, INFO, DEBUG, TRACE") + .addTextContent("Invalid log level: " + logLevelStr + ". Valid options are: " + + String.join(", ", LogSnapshot.getValidLogLevelNames())) .isError(true) .build(); } + minLogLevel = logLevelStr; } // Compile regex pattern if provided @@ -134,23 +138,12 @@ public class LogToolContribution implements McpServerContribution { })); } - private Level parseLogLevel(String levelStr) { - return switch (levelStr.toUpperCase()) { - case "ERROR" -> Level.ERROR; - case "WARN", "WARNING" -> Level.WARN; - case "INFO" -> Level.INFO; - case "DEBUG" -> Level.DEBUG; - case "TRACE" -> Level.TRACE; - default -> null; - }; - } - - private String formatLogs(List<LogSnapshot> logs, String regexPattern, Level minLogLevel, int maxEntries) { + private String formatLogs(List<LogSnapshot> logs, String regexPattern, String minLogLevel, int maxEntries) { StringBuilder result = new StringBuilder(); result.append("=== Log Entries ===\n\n"); result.append("Filter Settings:\n"); - result.append(" - Log Level: ").append(getLogLevelName(minLogLevel)).append(" and higher severity\n"); + result.append(" - Log LogLevel: ").append(minLogLevel).append(" and higher severity\n"); result.append(" - Regex Pattern: ") .append(regexPattern != null ? regexPattern : "(none)") .append("\n"); @@ -179,7 +172,7 @@ public class LogToolContribution implements McpServerContribution { private void formatLogEntry(LogSnapshot entry, int index, StringBuilder result) { result.append("[").append(index).append("] "); result.append(DATE_FORMAT.format(new Date(entry.timeMillis()))); - result.append(" [").append(getLogLevelName(entry.level())).append("] "); + result.append(" [").append(entry.level()).append("] "); result.append("[") .append(entry.loggerName() != null ? entry.loggerName() : "(unknown logger)") .append("] "); @@ -222,7 +215,17 @@ public class LogToolContribution implements McpServerContribution { return result.toString(); } - private String getLogLevelName(Level level) { - return level != null ? level.levelStr : "UNKNOWN"; + private String validLogLevelValuesAsCsv() { + return String.join(", ", LogSnapshot.getValidLogLevelNames()); + } + + private String validLogLevelValuesAsJsonSchemaEnum() { + StringBuilder result = new StringBuilder(); + result.append("[ "); + result.append(LogSnapshot.getValidLogLevelNames().stream() + .map(name -> '"' + name + '"') + .collect(Collectors.joining(", "))); + result.append(" ]"); + return result.toString(); } } diff --git a/src/main/java/org/apache/sling/mcp/server/impl/contribs/internal/LogSnapshot.java b/src/main/java/org/apache/sling/mcp/server/impl/contribs/internal/LogSnapshot.java index 80dce2f..6dc4cb4 100644 --- a/src/main/java/org/apache/sling/mcp/server/impl/contribs/internal/LogSnapshot.java +++ b/src/main/java/org/apache/sling/mcp/server/impl/contribs/internal/LogSnapshot.java @@ -18,25 +18,63 @@ */ package org.apache.sling.mcp.server.impl.contribs.internal; +import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.Map; -import ch.qos.logback.classic.Level; - /** * Stores only the lightweight, stable parts of a log event so the in-memory buffer - * does not retain full {@code ILoggingEvent} object graphs. + * does not retain full logging event object graphs. */ public record LogSnapshot( long timeMillis, - Level level, + LogLevel level, // avoid binding to logback or a specific version of slf4j String loggerName, String threadName, String formattedMessage, String throwableText, Map<String, String> mdc) { + public static boolean isValidLogLevel(String logLevelName) { + try { + LogLevel.valueOf(logLevelName); + return true; + } catch (IllegalArgumentException e) { + return false; + } + } + + public static List<String> getValidLogLevelNames() { + return Arrays.stream(LogLevel.values()).map(Enum::toString).toList(); + } + + public static String getHighestLogLevelName() { + return LogLevel.values()[LogLevel.values().length - 1].toString(); + } + public LogSnapshot { mdc = mdc == null ? Collections.emptyMap() : Collections.unmodifiableMap(mdc); } + + enum LogLevel { + TRACE, + DEBUG, + INFO, + WARN, + ERROR; + + boolean isGreaterOrEqual(LogLevel minLevel) { + return ordinal() >= minLevel.ordinal(); + } + + public boolean isValid(String logLevelName) { + try { + LogLevel.valueOf(logLevelName); + return true; + } catch (IllegalArgumentException e) { + return false; + } + } + } } diff --git a/src/main/java/org/apache/sling/mcp/server/impl/contribs/internal/StructuredLogBuffer.java b/src/main/java/org/apache/sling/mcp/server/impl/contribs/internal/StructuredLogBuffer.java index 80c64fe..9a9ba53 100644 --- a/src/main/java/org/apache/sling/mcp/server/impl/contribs/internal/StructuredLogBuffer.java +++ b/src/main/java/org/apache/sling/mcp/server/impl/contribs/internal/StructuredLogBuffer.java @@ -24,7 +24,7 @@ import java.util.Deque; import java.util.List; import java.util.regex.Pattern; -import ch.qos.logback.classic.Level; +import org.apache.sling.mcp.server.impl.contribs.internal.LogSnapshot.LogLevel; public class StructuredLogBuffer { @@ -43,14 +43,21 @@ public class StructuredLogBuffer { } } - public List<LogSnapshot> getRecent(Pattern pattern, Level minLevel, int maxEntries) { + public List<LogSnapshot> getRecent(Pattern pattern, String minLevel, int maxEntries) { + + if (!LogSnapshot.isValidLogLevel(minLevel)) { + throw new IllegalArgumentException("Invalid log level: " + minLevel); + } + + LogLevel minLogLevel = LogLevel.valueOf(minLevel); + synchronized (lock) { List<LogSnapshot> matches = new ArrayList<>(); int remaining = Math.max(1, maxEntries); for (var iterator = entries.descendingIterator(); iterator.hasNext() && remaining > 0; ) { LogSnapshot snapshot = iterator.next(); - if (!matches(snapshot, pattern, minLevel)) { + if (!matches(snapshot, pattern, minLogLevel)) { continue; } matches.add(snapshot); @@ -61,12 +68,14 @@ public class StructuredLogBuffer { } } - private boolean matches(LogSnapshot snapshot, Pattern pattern, Level minLevel) { + private boolean matches(LogSnapshot snapshot, Pattern pattern, LogLevel minLevel) { + if (snapshot.level().isGreaterOrEqual(minLevel)) { if (pattern == null) { return true; } - return matchesField(pattern, snapshot.level() != null ? snapshot.level().levelStr : null) + return matchesField( + pattern, snapshot.level() != null ? snapshot.level().toString() : null) || matchesField(pattern, snapshot.loggerName()) || matchesField(pattern, snapshot.threadName()) || matchesField(pattern, snapshot.formattedMessage()) diff --git a/src/main/java/org/apache/sling/mcp/server/impl/contribs/internal/StructuredLogBufferAppender.java b/src/main/java/org/apache/sling/mcp/server/impl/contribs/internal/StructuredLogBufferAppender.java index 95a02db..244f297 100644 --- a/src/main/java/org/apache/sling/mcp/server/impl/contribs/internal/StructuredLogBufferAppender.java +++ b/src/main/java/org/apache/sling/mcp/server/impl/contribs/internal/StructuredLogBufferAppender.java @@ -29,6 +29,7 @@ import ch.qos.logback.classic.spi.IThrowableProxy; import ch.qos.logback.classic.spi.StackTraceElementProxy; import ch.qos.logback.core.Appender; import ch.qos.logback.core.AppenderBase; +import org.apache.sling.mcp.server.impl.contribs.internal.LogSnapshot.LogLevel; import org.osgi.service.component.annotations.Activate; import org.osgi.service.component.annotations.Component; import org.osgi.service.metatype.annotations.AttributeDefinition; @@ -48,7 +49,7 @@ public class StructuredLogBufferAppender extends AppenderBase<ILoggingEvent> { // Forward compatibility with logback 1.5+, where IThrowableProxy may expose getOverridingMessage(). private static final MethodHandle GET_OVERRIDING_MESSAGE = findGetOverridingMessage(); - @ObjectClassDefinition(name = "Apache Sling MCP Structured Log Buffer") + @ObjectClassDefinition(name = "Apache Sling Structured Log Buffer") public @interface Configuration { @AttributeDefinition(name = "Max entries") @@ -60,7 +61,7 @@ public class StructuredLogBufferAppender extends AppenderBase<ILoggingEvent> { @Activate public StructuredLogBufferAppender(Configuration configuration) { buffer = new StructuredLogBuffer(configuration.maxEntries()); - setName("mcp-structured-log-buffer"); + setName("structured-log-buffer"); } public StructuredLogBuffer getBuffer() { @@ -73,9 +74,11 @@ public class StructuredLogBufferAppender extends AppenderBase<ILoggingEvent> { return; } + LogLevel logLevel = LogLevel.valueOf(eventObject.getLevel().levelStr); + buffer.append(new LogSnapshot( eventObject.getTimeStamp(), - eventObject.getLevel(), + logLevel, eventObject.getLoggerName(), eventObject.getThreadName(), eventObject.getFormattedMessage(), diff --git a/src/main/java/org/apache/sling/mcp/server/impl/contribs/internal/LogSnapshot.java b/src/test/java/org/apache/sling/mcp/server/impl/contribs/internal/LogSnapshotTest.java similarity index 60% copy from src/main/java/org/apache/sling/mcp/server/impl/contribs/internal/LogSnapshot.java copy to src/test/java/org/apache/sling/mcp/server/impl/contribs/internal/LogSnapshotTest.java index 80dce2f..8c494b5 100644 --- a/src/main/java/org/apache/sling/mcp/server/impl/contribs/internal/LogSnapshot.java +++ b/src/test/java/org/apache/sling/mcp/server/impl/contribs/internal/LogSnapshotTest.java @@ -18,25 +18,15 @@ */ package org.apache.sling.mcp.server.impl.contribs.internal; -import java.util.Collections; -import java.util.Map; +import org.apache.sling.mcp.server.impl.contribs.internal.LogSnapshot.LogLevel; +import org.junit.jupiter.api.Test; -import ch.qos.logback.classic.Level; +import static org.junit.jupiter.api.Assertions.*; -/** - * Stores only the lightweight, stable parts of a log event so the in-memory buffer - * does not retain full {@code ILoggingEvent} object graphs. - */ -public record LogSnapshot( - long timeMillis, - Level level, - String loggerName, - String threadName, - String formattedMessage, - String throwableText, - Map<String, String> mdc) { +class LogSnapshotTest { - public LogSnapshot { - mdc = mdc == null ? Collections.emptyMap() : Collections.unmodifiableMap(mdc); + @Test + void logLevelComparison() { + assertTrue(LogSnapshot.LogLevel.INFO.isGreaterOrEqual(LogLevel.TRACE)); } } diff --git a/src/test/java/org/apache/sling/mcp/server/impl/contribs/internal/StructuredLogBufferAppenderTest.java b/src/test/java/org/apache/sling/mcp/server/impl/contribs/internal/StructuredLogBufferAppenderTest.java index c9064e7..1d34628 100644 --- a/src/test/java/org/apache/sling/mcp/server/impl/contribs/internal/StructuredLogBufferAppenderTest.java +++ b/src/test/java/org/apache/sling/mcp/server/impl/contribs/internal/StructuredLogBufferAppenderTest.java @@ -47,7 +47,7 @@ class StructuredLogBufferAppenderTest { appender.append(event); - List<LogSnapshot> logs = appender.getBuffer().getRecent(null, Level.TRACE, 10); + List<LogSnapshot> logs = appender.getBuffer().getRecent(null, "TRACE", 10); assertEquals(1, logs.size()); assertEquals("message", logs.get(0).formattedMessage()); assertEquals("worker-1", logs.get(0).threadName()); diff --git a/src/test/java/org/apache/sling/mcp/server/impl/contribs/internal/StructuredLogBufferTest.java b/src/test/java/org/apache/sling/mcp/server/impl/contribs/internal/StructuredLogBufferTest.java index d92efce..8557801 100644 --- a/src/test/java/org/apache/sling/mcp/server/impl/contribs/internal/StructuredLogBufferTest.java +++ b/src/test/java/org/apache/sling/mcp/server/impl/contribs/internal/StructuredLogBufferTest.java @@ -22,7 +22,7 @@ import java.util.List; import java.util.Map; import java.util.regex.Pattern; -import ch.qos.logback.classic.Level; +import org.apache.sling.mcp.server.impl.contribs.internal.LogSnapshot.LogLevel; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -33,11 +33,11 @@ class StructuredLogBufferTest { void keepsOnlyNewestEntriesWithinCapacity() { StructuredLogBuffer buffer = new StructuredLogBuffer(2); - buffer.append(snapshot(1L, Level.INFO, "first")); - buffer.append(snapshot(2L, Level.INFO, "second")); - buffer.append(snapshot(3L, Level.INFO, "third")); + buffer.append(snapshot(1L, LogLevel.INFO, "first")); + buffer.append(snapshot(2L, LogLevel.INFO, "second")); + buffer.append(snapshot(3L, LogLevel.INFO, "third")); - List<LogSnapshot> logs = buffer.getRecent(null, Level.TRACE, 10); + List<LogSnapshot> logs = buffer.getRecent(null, "TRACE", 10); assertEquals( List.of("third", "second"), logs.stream().map(LogSnapshot::formattedMessage).toList()); @@ -47,18 +47,18 @@ class StructuredLogBufferTest { void filtersByLevelAndRegex() { StructuredLogBuffer buffer = new StructuredLogBuffer(10); - buffer.append(snapshot(1L, Level.DEBUG, "debug trace")); - buffer.append(snapshot(2L, Level.INFO, "first user ok")); - buffer.append(snapshot(3L, Level.ERROR, "first user failure")); + buffer.append(snapshot(1L, LogLevel.DEBUG, "debug trace")); + buffer.append(snapshot(2L, LogLevel.INFO, "first user ok")); + buffer.append(snapshot(3L, LogLevel.ERROR, "first user failure")); - List<LogSnapshot> logs = buffer.getRecent(Pattern.compile("first", Pattern.CASE_INSENSITIVE), Level.INFO, 10); + List<LogSnapshot> logs = buffer.getRecent(Pattern.compile("first", Pattern.CASE_INSENSITIVE), "INFO", 10); assertEquals( List.of("first user failure", "first user ok"), logs.stream().map(LogSnapshot::formattedMessage).toList()); } - private LogSnapshot snapshot(long timeMillis, Level level, String message) { + private LogSnapshot snapshot(long timeMillis, LogLevel level, String message) { return new LogSnapshot(timeMillis, level, "logger", "thread", message, null, Map.of()); } }
