This is an automated email from the ASF dual-hosted git repository.
jbonofre pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/karaf.git
The following commit(s) were added to refs/heads/main by this push:
new 703fc1b1d5 Fix LogServiceLogback to find root logger correctly and add
support for basic variable substitution (#2172)
703fc1b1d5 is described below
commit 703fc1b1d5b9427a6aca9ae83038147fb00aeee8
Author: JB Onofré <[email protected]>
AuthorDate: Mon Dec 1 09:39:11 2025 +0100
Fix LogServiceLogback to find root logger correctly and add support for
basic variable substitution (#2172)
Co-authored-by: Antti Nevala <[email protected]>
---
.../core/internal/LogServiceLogbackXmlImpl.java | 100 ++++++++++++--
.../core/internal/LogServiceLog4j2XmlImplTest.java | 102 +++++++--------
.../core/internal/LogServiceLogbackXmlTest.java | 144 ++++++++++++++++++---
log/src/test/resources/logback.xml | 28 ++++
4 files changed, 291 insertions(+), 83 deletions(-)
diff --git
a/log/src/main/java/org/apache/karaf/log/core/internal/LogServiceLogbackXmlImpl.java
b/log/src/main/java/org/apache/karaf/log/core/internal/LogServiceLogbackXmlImpl.java
index ad4924aad2..89f41aec9a 100644
---
a/log/src/main/java/org/apache/karaf/log/core/internal/LogServiceLogbackXmlImpl.java
+++
b/log/src/main/java/org/apache/karaf/log/core/internal/LogServiceLogbackXmlImpl.java
@@ -39,17 +39,19 @@ import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardOpenOption;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Map;
-import java.util.TreeMap;
+import java.util.*;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
public class LogServiceLogbackXmlImpl implements LogServiceInternal {
private static final String ELEMENT_ROOT = "root";
private static final String ELEMENT_LOGGER = "logger";
+ private static final String ELEMENT_PROPERTY = "property";
+ private static final String ELEMENT_VARIABLE = "variable";
private static final String ATTRIBUTE_NAME = "name";
private static final String ATTRIBUTE_LEVEL = "level";
+ private static final String ATTRIBUTE_VALUE = "value";
private static final String ELEMENT_CONFIGURATION = "configuration";
private final Path path;
@@ -61,13 +63,15 @@ public class LogServiceLogbackXmlImpl implements
LogServiceInternal {
public Map<String, String> getLevel(String logger) {
try {
Document doc = loadConfig(path);
+ Map<String, String> properties = getProperties(doc);
Map<String, Element> loggers = getLoggers(doc);
Map<String, String> levels = new TreeMap<>();
for (Map.Entry<String, Element> e : loggers.entrySet()) {
+
String level = e.getValue().getAttribute(ATTRIBUTE_LEVEL);
- if (level != null && !level.isEmpty()) {
- levels.put(e.getKey(), level);
+ if (!level.isEmpty()) {
+ levels.put(e.getKey(), resolveValue(level, properties, new
Properties(System.getProperties()), new HashMap<>(System.getenv())));
}
}
@@ -93,6 +97,40 @@ public class LogServiceLogbackXmlImpl implements
LogServiceInternal {
}
}
+ static String resolveValue(String value, Map<String, String> properties,
Properties systemProperties, Map<String, String> envVariables) {
+ // Pattern to match ${variable:-default}
+ // At this moment only basic substitution is supported
+ // i.e D${my.param:-EBUG} is not supported
+ Pattern pattern = Pattern.compile("\\$\\{(.+?)(?::-(.+?))?}");
+ Matcher matcher = pattern.matcher(value);
+
+ if (matcher.matches()) {
+ String variable = matcher.group(1);
+ String defaultValue = matcher.group(2);
+ // Remove found property to prevent cyclic loops
+ // Check properties
+ String resolved = properties.remove(variable);
+ if (resolved == null) {
+ // Check system properties
+ resolved = systemProperties.getProperty(variable);
+ systemProperties.remove(variable);
+ }
+ if (resolved == null) {
+ // Check environment variables
+ resolved = envVariables.remove(variable);
+ }
+
+ if (resolved != null) {
+ // Check resolved variable again to susbstitute
+ return resolveValue(resolved, properties, systemProperties,
envVariables);
+ } else {
+ return defaultValue;
+ }
+ } else {
+ return value;
+ }
+ }
+
public void setLevel(String logger, String level) {
try {
Document doc = loadConfig(path);
@@ -146,13 +184,13 @@ public class LogServiceLogbackXmlImpl implements
LogServiceInternal {
static void insertIndented(Element parent, Element element) {
NodeList taggedElements = parent.getElementsByTagName("*");
//only use direct descendants of parent element to insert next to
- ArrayList <Node> childElements = new ArrayList<Node>();
+ ArrayList <Node> childElements = new ArrayList<>();
for (int i = 0;i < taggedElements.getLength(); i++ ){
if(taggedElements.item(i).getParentNode().equals(parent)){
childElements.add(taggedElements.item(i));
}
}
- Node insertAfter = childElements.size() > 0 ?
childElements.get(childElements.size() - 1) : null;
+ Node insertAfter = !childElements.isEmpty() ?
childElements.get(childElements.size() - 1) : null;
if (insertAfter != null) {
if (insertAfter.getPreviousSibling() != null &&
insertAfter.getPreviousSibling().getNodeType() == Node.TEXT_NODE) {
String indent =
insertAfter.getPreviousSibling().getTextContent();
@@ -185,7 +223,7 @@ public class LogServiceLogbackXmlImpl implements
LogServiceInternal {
indent += "\t";
}
}
- if (parent.getFirstChild() != null &&
parent.getPreviousSibling().getNodeType() == Node.TEXT_NODE) {
+ if (parent.getFirstChild() != null) {
parent.removeChild(parent.getFirstChild());
}
} else {
@@ -250,17 +288,53 @@ public class LogServiceLogbackXmlImpl implements
LogServiceInternal {
Node n = loggersList.item(i);
if (n instanceof Element) {
Element e = (Element) n;
- if (ELEMENT_ROOT.equals(e.getLocalName())) {
- loggers.put(ROOT_LOGGER, e);
- } else if (ELEMENT_LOGGER.equals(e.getLocalName())) {
+ if (ELEMENT_LOGGER.equals(e.getLocalName())) {
String name = e.getAttribute(ATTRIBUTE_NAME);
- if (name != null) {
+ if (!name.isEmpty()) {
loggers.put(name, e);
}
}
}
}
+ // Handle root separately
+ Node n = docE.getElementsByTagName(ELEMENT_ROOT).item(0);
+ if (n instanceof Element) {
+ Element e = (Element) n;
+ if (ELEMENT_ROOT.equals(e.getLocalName())) {
+ loggers.put(ROOT_LOGGER, e);
+ }
+ }
return loggers;
}
+ private Map<String, String> getProperties(Document doc) {
+ Map<String, String> properties = new TreeMap<>();
+ Element docE = doc.getDocumentElement();
+ if (!ELEMENT_CONFIGURATION.equals(docE.getLocalName())) {
+ throw new IllegalArgumentException("Xml root document should be "
+ ELEMENT_CONFIGURATION);
+ }
+ NodeList propertyList = docE.getElementsByTagName(ELEMENT_PROPERTY);
+ extractProperties(propertyList, ELEMENT_PROPERTY, properties);
+ NodeList variableList = docE.getElementsByTagName(ELEMENT_VARIABLE);
+ extractProperties(variableList, ELEMENT_VARIABLE, properties);
+ return properties;
+ }
+
+ private static void extractProperties(NodeList propertyList, String
elementName, Map<String, String> properties) {
+ for (int i = 0; i < propertyList.getLength(); i++) {
+ Node n = propertyList.item(i);
+ if (n instanceof Element) {
+ Element e = (Element) n;
+ if (elementName.equals(e.getLocalName())) {
+ String name = e.getAttribute(ATTRIBUTE_NAME);
+ String value = e.getAttribute(ATTRIBUTE_VALUE);
+ if (!name.isEmpty()) {
+ properties.put(name, value);
+ }
+ }
+ }
+ }
+ }
+
+
}
diff --git
a/log/src/test/java/org/apache/karaf/log/core/internal/LogServiceLog4j2XmlImplTest.java
b/log/src/test/java/org/apache/karaf/log/core/internal/LogServiceLog4j2XmlImplTest.java
index ce6a770e79..1b0815cdf7 100644
---
a/log/src/test/java/org/apache/karaf/log/core/internal/LogServiceLog4j2XmlImplTest.java
+++
b/log/src/test/java/org/apache/karaf/log/core/internal/LogServiceLog4j2XmlImplTest.java
@@ -34,99 +34,99 @@ public class LogServiceLog4j2XmlImplTest {
@Test
public void testInsertIndentedTabs() throws Exception {
- String xml = "<Configuration>\n" +
- "\t<Loggers>\n" +
- "\t</Loggers>\n" +
+ String xml = "<Configuration>" + System.lineSeparator() +
+ "\t<Loggers>" + System.lineSeparator() +
+ "\t</Loggers>" + System.lineSeparator() +
"</Configuration>";
String out = insertIndented(xml, false);
assertEquals(
- "<Configuration>\n" +
- "\t<Loggers>\n" +
- "\t\t<Logger/>\n" +
- "\t</Loggers>\n" +
+ "<Configuration>" + System.lineSeparator() +
+ "\t<Loggers>" + System.lineSeparator() +
+ "\t\t<Logger/>" + System.lineSeparator() +
+ "\t</Loggers>" + System.lineSeparator() +
"</Configuration>", out);
out = insertIndented(xml, true);
assertEquals(
- "<Configuration>\n" +
- "\t<Loggers>\n" +
- "\t\t<Logger/>\n" +
- "\t</Loggers>\n" +
+ "<Configuration>" + System.lineSeparator() +
+ "\t<Loggers>" + System.lineSeparator() +
+ "\t\t<Logger/>" + System.lineSeparator() +
+ "\t</Loggers>" + System.lineSeparator() +
"</Configuration>", out);
}
@Test
public void testInsertIndentedSpaces() throws Exception {
- String xml = "<Configuration>\n" +
- " <Loggers>\n" +
- " </Loggers>\n" +
+ String xml = "<Configuration>" + System.lineSeparator() +
+ " <Loggers>" + System.lineSeparator() +
+ " </Loggers>" + System.lineSeparator() +
"</Configuration>";
String out = insertIndented(xml, false);
assertEquals(
- "<Configuration>\n" +
- " <Loggers>\n" +
- " <Logger/>\n" +
- " </Loggers>\n" +
+ "<Configuration>" + System.lineSeparator() +
+ " <Loggers>" + System.lineSeparator() +
+ " <Logger/>" + System.lineSeparator() +
+ " </Loggers>" + System.lineSeparator() +
"</Configuration>", out);
out = insertIndented(xml, true);
assertEquals(
- "<Configuration>\n" +
- " <Loggers>\n" +
- " <Logger/>\n" +
- " </Loggers>\n" +
+ "<Configuration>" + System.lineSeparator() +
+ " <Loggers>" + System.lineSeparator() +
+ " <Logger/>" + System.lineSeparator() +
+ " </Loggers>" + System.lineSeparator() +
"</Configuration>", out);
}
@Test
public void testInsertIndentedTabsWithRoot() throws Exception {
- String xml = "<Configuration>\n" +
- "\t<Loggers>\n" +
- "\t\t<Root/>\n" +
- "\t</Loggers>\n" +
+ String xml = "<Configuration>" + System.lineSeparator() +
+ "\t<Loggers>" + System.lineSeparator() +
+ "\t\t<Root/>" + System.lineSeparator() +
+ "\t</Loggers>" + System.lineSeparator() +
"</Configuration>";
String out = insertIndented(xml, false);
assertEquals(
- "<Configuration>\n" +
- "\t<Loggers>\n" +
- "\t\t<Root/>\n" +
- "\t\t<Logger/>\n" +
- "\t</Loggers>\n" +
+ "<Configuration>" + System.lineSeparator() +
+ "\t<Loggers>" + System.lineSeparator() +
+ "\t\t<Root/>" + System.lineSeparator() +
+ "\t\t<Logger/>" + System.lineSeparator() +
+ "\t</Loggers>" + System.lineSeparator() +
"</Configuration>", out);
out = insertIndented(xml, true);
assertEquals(
- "<Configuration>\n" +
- "\t<Loggers>\n" +
- "\t\t<Logger/>\n" +
- "\t\t<Root/>\n" +
- "\t</Loggers>\n" +
+ "<Configuration>" + System.lineSeparator() +
+ "\t<Loggers>" + System.lineSeparator() +
+ "\t\t<Logger/>" + System.lineSeparator() +
+ "\t\t<Root/>" + System.lineSeparator() +
+ "\t</Loggers>" + System.lineSeparator() +
"</Configuration>", out);
}
@Test
public void testInsertIndentedSpacesWithRoot() throws Exception {
- String xml = "<Configuration>\n" +
- " <Loggers>\n" +
- " <Root/>\n" +
- " </Loggers>\n" +
+ String xml = "<Configuration>" + System.lineSeparator() +
+ " <Loggers>" + System.lineSeparator() +
+ " <Root/>" + System.lineSeparator() +
+ " </Loggers>" + System.lineSeparator() +
"</Configuration>";
-
+
String out = insertIndented(xml, false);
assertEquals(
- "<Configuration>\n" +
- " <Loggers>\n" +
- " <Root/>\n" +
- " <Logger/>\n" +
- " </Loggers>\n" +
+ "<Configuration>" + System.lineSeparator() +
+ " <Loggers>" + System.lineSeparator() +
+ " <Root/>" + System.lineSeparator() +
+ " <Logger/>" + System.lineSeparator() +
+ " </Loggers>" + System.lineSeparator() +
"</Configuration>", out);
out = insertIndented(xml, true);
assertEquals(
- "<Configuration>\n" +
- " <Loggers>\n" +
- " <Logger/>\n" +
- " <Root/>\n" +
- " </Loggers>\n" +
+ "<Configuration>" + System.lineSeparator() +
+ " <Loggers>" + System.lineSeparator() +
+ " <Logger/>" + System.lineSeparator() +
+ " <Root/>" + System.lineSeparator() +
+ " </Loggers>" + System.lineSeparator() +
"</Configuration>", out);
}
diff --git
a/log/src/test/java/org/apache/karaf/log/core/internal/LogServiceLogbackXmlTest.java
b/log/src/test/java/org/apache/karaf/log/core/internal/LogServiceLogbackXmlTest.java
index 70b62571a6..e3d8a537a1 100644
---
a/log/src/test/java/org/apache/karaf/log/core/internal/LogServiceLogbackXmlTest.java
+++
b/log/src/test/java/org/apache/karaf/log/core/internal/LogServiceLogbackXmlTest.java
@@ -16,6 +16,7 @@
*/
package org.apache.karaf.log.core.internal;
+import org.junit.BeforeClass;
import org.junit.Test;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
@@ -27,63 +28,85 @@ import javax.xml.transform.dom.DOMSource;
import javax.xml.transform.stream.StreamResult;
import java.io.ByteArrayInputStream;
import java.io.StringWriter;
+import java.net.URISyntaxException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Properties;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
public class LogServiceLogbackXmlTest {
+ public static final String LOG_LEVEL_TOKEN = "log.level";
+
+ private static String file;
+ @BeforeClass
+ public static void initClass() {
+ Path p;
+ try {
+ p =
Paths.get(LogServiceLogbackXmlImpl.class.getResource("/logback.xml").toURI());
+ } catch (URISyntaxException e) {
+ throw new RuntimeException(e);
+ }
+ file = p.toString();
+ }
+
@Test
public void testInsertIndentedTabs() throws Exception {
- String xml = "<configuration>\n" +
+ String xml = "<configuration>" + System.lineSeparator() +
"</configuration>";
String out = insertIndented(xml);
assertEquals(
- "<configuration>\n" +
- "\t<logger/>\n" +
+ "<configuration>" + System.lineSeparator() +
+ "\t<logger/>" + System.lineSeparator() +
"</configuration>", out);
}
@Test
public void testInsertIndentedSpaces() throws Exception {
//this one tests with one logger already added, because with no
loggers there is no indentation to decide by and the function will choose tab
- String xml = "<configuration>\n" +
- " <logger/>\n" +
+ String xml = "<configuration>" + System.lineSeparator() +
+ " <logger/>" + System.lineSeparator() +
"</configuration>";
String out = insertIndented(xml);
assertEquals(
- "<configuration>\n" +
- " <logger/>\n" +
- " <logger/>\n" +
+ "<configuration>" + System.lineSeparator() +
+ " <logger/>" + System.lineSeparator() +
+ " <logger/>" + System.lineSeparator() +
"</configuration>", out);
}
@Test
public void testInsertIndentedTabsWithRoot() throws Exception {
- String xml = "<configuration>\n" +
- "\t<root/>\n" +
+ String xml = "<configuration>" + System.lineSeparator() +
+ "\t<root/>" + System.lineSeparator() +
"</configuration>";
String out = insertIndented(xml);
assertEquals(
- "<configuration>\n" +
- "\t<root/>\n" +
- "\t<logger/>\n" +
+ "<configuration>" + System.lineSeparator() +
+ "\t<root/>" + System.lineSeparator() +
+ "\t<logger/>" + System.lineSeparator() +
"</configuration>", out);
}
@Test
public void testInsertIndentedSpacesWithRoot() throws Exception {
- String xml = "<configuration>\n" +
- " <root/>\n" +
+ String xml = "<configuration>" + System.lineSeparator() +
+ " <root/>" + System.lineSeparator() +
"</configuration>";
String out = insertIndented(xml);
assertEquals(
- "<configuration>\n" +
- " <root/>\n" +
- " <logger/>\n" +
+ "<configuration>" + System.lineSeparator() +
+ " <root/>" + System.lineSeparator() +
+ " <logger/>" + System.lineSeparator() +
"</configuration>", out);
}
@@ -91,7 +114,7 @@ public class LogServiceLogbackXmlTest {
Document doc = LogServiceLog4j2XmlImpl.loadConfig(null, new
ByteArrayInputStream(xml.getBytes()));
Element element = doc.createElement("logger");
LogServiceLogbackXmlImpl.insertIndented(
- (Element) doc.getDocumentElement(),
+ doc.getDocumentElement(),
element);
try (StringWriter os = new StringWriter()) {
TransformerFactory tFactory = TransformerFactory.newInstance();
@@ -101,4 +124,87 @@ public class LogServiceLogbackXmlTest {
return os.toString();
}
}
+
+ @Test
+ public void testBasicValue() {
+ Properties systemProps = new Properties();
+ String resolved = LogServiceLogbackXmlImpl.resolveValue("DEBUG",
Collections.emptyMap(), systemProps, Collections.emptyMap());
+ assertEquals("DEBUG", resolved);
+ }
+
+ @Test
+ public void testPropertySubstitution() {
+ Map<String, String> properties = new HashMap<>();
+ properties.put(LOG_LEVEL_TOKEN, "WARN");
+ String resolved =
LogServiceLogbackXmlImpl.resolveValue("${log.level:-DEBUG}", properties, new
Properties(), Collections.emptyMap());
+ assertEquals("WARN", resolved);
+ }
+
+ @Test
+ public void testSystemPropertiesSubstitution() {
+ Properties systemProps = new Properties();
+ systemProps.put(LOG_LEVEL_TOKEN, "WARN");
+ String resolved =
LogServiceLogbackXmlImpl.resolveValue("${log.level:-DEBUG}",
Collections.emptyMap(), systemProps, Collections.emptyMap());
+ assertEquals("WARN", resolved);
+ }
+
+ @Test
+ public void testEnvVariableSubstitution() {
+ Map<String, String> env = new HashMap<>();
+ env.put(LOG_LEVEL_TOKEN, "WARN");
+ String resolved =
LogServiceLogbackXmlImpl.resolveValue("${log.level:-DEBUG}",
Collections.emptyMap(), new Properties(), env);
+ assertEquals("WARN", resolved);
+ }
+
+ @Test
+ public void testPropertyWinsEnvSubstitution() {
+ Map<String, String> props = new HashMap<>();
+ props.put(LOG_LEVEL_TOKEN, "DEBUG");
+ Map<String, String> env = new HashMap<>();
+ env.put(LOG_LEVEL_TOKEN, "WARN");
+ String resolved =
LogServiceLogbackXmlImpl.resolveValue("${log.level}", props , new Properties(),
env);
+ assertEquals("DEBUG", resolved);
+ }
+
+ @Test
+ public void testRootLogLevel() {
+ LogServiceLogbackXmlImpl logService = getLogService();
+ assertEquals("WARN",
logService.getLevel(LogServiceInternal.ROOT_LOGGER).get(LogServiceInternal.ROOT_LOGGER));
+ }
+
+ @Test
+ public void testPropertyLogLevel() {
+ String logger = "debugPropertyLogger";
+ LogServiceLogbackXmlImpl logService = getLogService();
+ assertEquals("DEBUG", logService.getLevel(logger).get(logger));
+ }
+
+ @Test
+ public void testSystemPropertyLogLevel() {
+ String logger = "systemPropertyLogger";
+ System.setProperty("LOG_LEVEL", "DEBUG");
+ LogServiceLogbackXmlImpl logService = getLogService();
+ assertEquals("DEBUG", logService.getLevel(logger).get(logger));
+ System.clearProperty("LOG_LEVEL");
+ }
+
+ @Test
+ public void testDefaultValueLogLevel() {
+ String logger = "defaultValueLogger";
+ LogServiceLogbackXmlImpl logService = getLogService();
+ assertEquals("DEBUG", logService.getLevel(logger).get(logger));
+ }
+
+ @Test
+ public void testAllLoggerLogLevel() {
+ LogServiceLogbackXmlImpl logService = getLogService();
+ Map<String, String> levels =
logService.getLevel(LogServiceInternal.ALL_LOGGER);
+ assertEquals(4, levels.size());
+ assertTrue(levels.containsKey(LogServiceInternal.ROOT_LOGGER));
+ }
+
+ private LogServiceLogbackXmlImpl getLogService() {
+ return new LogServiceLogbackXmlImpl(file);
+ }
+
}
diff --git a/log/src/test/resources/logback.xml
b/log/src/test/resources/logback.xml
new file mode 100644
index 0000000000..fe3dfe991f
--- /dev/null
+++ b/log/src/test/resources/logback.xml
@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<!--
+
+ 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.
+-->
+<configuration>
+ <property name="system.log.level" scope="context" value="${LOG_LEVEL}"/>
+ <property name="debug.log.level" scope="context" value="DEBUG"/>
+
+ <root level="WARN"/>
+
+ <logger name="debugPropertyLogger" level="${debug.log.level}"/>
+ <logger name="systemPropertyLogger" level="${system.log.level}"/>
+ <logger name="defaultValueLogger" level="${not.existing.property:-DEBUG}"/>
+</configuration>
\ No newline at end of file
