Skip to content

Commit

Permalink
Logback configuration defined via JAVA_TOOL_OPTIONS is ignored (#9009)
Browse files Browse the repository at this point in the history
Previously, we only checked the classpath for logback configuration files, and when refreshing we ignored the `logback.configurationFile` setting.

This PR checks the filesystem if the config cannot be found on the classpath

It also adds `logback.configurationFile` as an optional property that points to the location of the config for when we refresh the configuration.

`logback.configurationFile` has precedence over the existing `logger.config` property
  • Loading branch information
timyates committed Mar 28, 2023
1 parent 9ca308f commit 0c1d486
Show file tree
Hide file tree
Showing 7 changed files with 160 additions and 6 deletions.
@@ -1,5 +1,5 @@
/*
* Copyright 2017-2020 original authors
* Copyright 2017-2023 original authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -23,6 +23,7 @@
import io.micronaut.core.annotation.Nullable;
import io.micronaut.logging.LogLevel;
import io.micronaut.logging.LoggingSystem;
import jakarta.inject.Inject;
import jakarta.inject.Singleton;
import org.slf4j.LoggerFactory;

Expand All @@ -41,8 +42,35 @@ public final class LogbackLoggingSystem implements LoggingSystem {

private final String logbackXmlLocation;

/**
* @deprecated Use {@link LogbackLoggingSystem#LogbackLoggingSystem(String, String)} instead
* @param logbackXmlLocation
*/
@Deprecated
public LogbackLoggingSystem(@Nullable @Property(name = "logger.config") String logbackXmlLocation) {
this.logbackXmlLocation = logbackXmlLocation != null ? logbackXmlLocation : DEFAULT_LOGBACK_LOCATION;
this(
System.getProperty("logback.configurationFile"),
logbackXmlLocation
);
}

/**
* @param logbackExternalConfigLocation The location of the logback configuration file set via logback properties
* @param logbackXmlLocation The location of the logback configuration file set via micronaut properties
* @since 3.8.8
*/
@Inject
public LogbackLoggingSystem(
@Nullable @Property(name = "logback.configurationFile") String logbackExternalConfigLocation,
@Nullable @Property(name = "logger.config") String logbackXmlLocation
) {
if (logbackExternalConfigLocation != null) {
this.logbackXmlLocation = logbackExternalConfigLocation;
} else if (logbackXmlLocation != null) {
this.logbackXmlLocation = logbackXmlLocation;
} else {
this.logbackXmlLocation = DEFAULT_LOGBACK_LOCATION;
}
}

@Override
Expand Down
27 changes: 23 additions & 4 deletions runtime/src/main/java/io/micronaut/logging/impl/LogbackUtils.java
Expand Up @@ -24,6 +24,8 @@
import io.micronaut.core.annotation.Nullable;
import io.micronaut.logging.LoggingSystemException;

import java.io.File;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.Iterator;
import java.util.ServiceLoader;
Expand All @@ -50,18 +52,35 @@ private LogbackUtils() {
public static void configure(@NonNull ClassLoader classLoader,
@NonNull LoggerContext context,
@NonNull String logbackXmlLocation) {
configure(context, logbackXmlLocation, () -> classLoader.getResource(logbackXmlLocation));
configure(context, logbackXmlLocation, () -> {
// Check classpath first
URL resource = classLoader.getResource(logbackXmlLocation);
if (resource != null) {
return resource;
}
// Check file system
File file = new File(logbackXmlLocation);
if (file.exists()) {
try {
resource = file.toURI().toURL();
} catch (MalformedURLException e) {

throw new LoggingSystemException("Error creating URL for off-classpath resource", e);
}
}
return resource;
});
}

/**
* Configures a Logger Context.
*
* <p>
* Searches fpr a custom {@link Configurator} via a service loader.
* If not present it configures the context with the resource.
*
* @param context Logger Context
* @param context Logger Context
* @param logbackXmlLocation the location of the xml logback config file
* @param resourceSupplier A resource for example logback.xml
* @param resourceSupplier A resource for example logback.xml
*/
private static void configure(
@NonNull LoggerContext context,
Expand Down
1 change: 1 addition & 0 deletions settings.gradle
Expand Up @@ -70,6 +70,7 @@ include "test-suite-graal"
include "test-suite-groovy"
include "test-suite-groovy"
include "test-suite-logback"
include "test-suite-logback-external-configuration"
include "test-utils"

// benchmarks
Expand Down
18 changes: 18 additions & 0 deletions test-suite-logback-external-configuration/build.gradle.kts
@@ -0,0 +1,18 @@
plugins {
id("io.micronaut.build.internal.convention-test-library")
}

dependencies {
testAnnotationProcessor(projects.injectJava)

testImplementation(libs.managed.micronaut.test.spock) {
exclude(group="io.micronaut", module="micronaut-aop")
}
testImplementation(projects.context)
testImplementation(projects.injectGroovy)
testImplementation(libs.managed.logback)
testImplementation(projects.management)
testImplementation(projects.httpClient)

testRuntimeOnly(projects.httpServerNetty)
}
@@ -0,0 +1,16 @@
<configuration>

<!-- This should be ignored, as we have programmatic configuration -->
<appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
<!-- encoders are assigned the type
ch.qos.logback.classic.encoder.PatternLayoutEncoder by default -->
<encoder>
<pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
</encoder>
</appender>

<root level="info">
<appender-ref ref="STDOUT" />
</root>
<logger name="external.logging" level="TRACE" />
</configuration>
@@ -0,0 +1,56 @@
package io.micronaut.logback

import ch.qos.logback.classic.Level
import ch.qos.logback.classic.Logger
import io.micronaut.context.ApplicationContext
import io.micronaut.runtime.server.EmbeddedServer
import org.slf4j.LoggerFactory
import spock.lang.See
import spock.lang.Specification
import spock.util.environment.RestoreSystemProperties

@See("https://logback.qos.ch/manual/configuration.html#auto_configuration")
class ExternalConfigurationSpec extends Specification {

@RestoreSystemProperties
def "should use the external configuration"() {
given:
System.setProperty("logback.configurationFile", "src/external/external-logback.xml")

when:
Logger fromXml = (Logger) LoggerFactory.getLogger("i.should.not.exist")
Logger external = (Logger) LoggerFactory.getLogger("external.logging")

then: 'logback.xml is ignored as we have set a configurationFile'
fromXml.level == null

and: 'external configuration is used'
external.level == Level.TRACE
}

@RestoreSystemProperties
def "should still use the external config if custom levels are defines"() {
given:
System.setProperty("logback.configurationFile", "src/external/external-logback.xml")

when:
def server = ApplicationContext.run(EmbeddedServer, [
"logger.levels.app.customisation": "DEBUG"
])
Logger fromXml = (Logger) LoggerFactory.getLogger("i.should.not.exist")
Logger custom = (Logger) LoggerFactory.getLogger("app.customisation")
Logger external = (Logger) LoggerFactory.getLogger("external.logging")

then: 'logback.xml is ignored as we have set a configurationFile'
fromXml.level == null

and: 'custom levels are still respected'
custom.level == Level.DEBUG

and: 'external configuration is used'
external.level == Level.TRACE

cleanup:
server.stop()
}
}
@@ -0,0 +1,16 @@
<configuration>

<!-- This should be ignored, as we have programmatic configuration -->
<appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
<!-- encoders are assigned the type
ch.qos.logback.classic.encoder.PatternLayoutEncoder by default -->
<encoder>
<pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
</encoder>
</appender>

<root level="info">
<appender-ref ref="STDOUT" />
</root>
<logger name="i.should.not.exist" level="TRACE" />
</configuration>

0 comments on commit 0c1d486

Please sign in to comment.