Commit f3dcf109 authored by Phillip Webb's avatar Phillip Webb

Merge pull request #18710 from wycm

* pr/18710:
  Polish "Optimize logger calls"
  Optimize logger calls

Closes gh-18710
parents 744dcd94 597baf97
......@@ -43,6 +43,7 @@ import org.springframework.boot.autoconfigure.jdbc.DataSourceAutoConfiguration;
import org.springframework.boot.jdbc.DataSourceUnwrapper;
import org.springframework.boot.jdbc.metadata.DataSourcePoolMetadataProvider;
import org.springframework.context.annotation.Configuration;
import org.springframework.core.log.LogMessage;
import org.springframework.util.StringUtils;
/**
......@@ -124,7 +125,7 @@ public class DataSourcePoolMetricsAutoConfiguration {
hikari.setMetricsTrackerFactory(new MicrometerMetricsTrackerFactory(this.registry));
}
catch (Exception ex) {
logger.warn("Failed to bind Hikari metrics: " + ex.getMessage());
logger.warn(LogMessage.format("Failed to bind Hikari metrics: %s", ex.getMessage()));
}
}
}
......
......@@ -25,6 +25,7 @@ import org.springframework.boot.actuate.health.AbstractHealthIndicator;
import org.springframework.boot.actuate.health.Health;
import org.springframework.boot.actuate.health.HealthIndicator;
import org.springframework.boot.actuate.health.Status;
import org.springframework.core.log.LogMessage;
import org.springframework.util.unit.DataSize;
/**
......@@ -62,7 +63,7 @@ public class DiskSpaceHealthIndicator extends AbstractHealthIndicator {
builder.up();
}
else {
logger.warn(String.format("Free disk space below threshold. Available: %d bytes (threshold: %s)",
logger.warn(LogMessage.format("Free disk space below threshold. Available: %d bytes (threshold: %s)",
diskFreeInBytes, this.threshold));
builder.down();
}
......
......@@ -41,6 +41,7 @@ import org.springframework.context.ApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.i18n.LocaleContextHolder;
import org.springframework.core.log.LogMessage;
import org.springframework.web.servlet.view.UrlBasedViewResolver;
import org.springframework.web.servlet.view.groovy.GroovyMarkupConfig;
import org.springframework.web.servlet.view.groovy.GroovyMarkupConfigurer;
......@@ -84,9 +85,10 @@ public class GroovyTemplateAutoConfiguration {
if (this.properties.isCheckTemplateLocation() && !isUsingGroovyAllJar()) {
TemplateLocation location = new TemplateLocation(this.properties.getResourceLoaderPath());
if (!location.exists(this.applicationContext)) {
logger.warn("Cannot find template location: " + location
+ " (please add some templates, check your Groovy "
+ "configuration, or set spring.groovy.template.check-template-location=false)");
logger.warn(LogMessage.format(
"Cannot find template location: %s (please add some templates, check your Groovy "
+ "configuration, or set spring.groovy.template.check-template-location=false)",
location));
}
}
}
......
......@@ -25,6 +25,7 @@ import org.springframework.beans.factory.InitializingBean;
import org.springframework.beans.factory.ObjectProvider;
import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationListener;
import org.springframework.core.log.LogMessage;
/**
* Bean to handle {@link DataSource} initialization by running {@literal schema-*.sql} on
......@@ -76,7 +77,8 @@ class DataSourceInitializerInvoker implements ApplicationListener<DataSourceSche
}
}
catch (IllegalStateException ex) {
logger.warn("Could not send event to complete DataSource initialization (" + ex.getMessage() + ")");
logger.warn(LogMessage.format("Could not send event to complete DataSource initialization (%s)",
ex.getMessage()));
}
}
......
......@@ -63,7 +63,7 @@ public class MustacheAutoConfiguration {
public void checkTemplateLocationExists() {
if (this.mustache.isCheckTemplateLocation()) {
TemplateLocation location = new TemplateLocation(this.mustache.getPrefix());
if (!location.exists(this.applicationContext)) {
if (!location.exists(this.applicationContext) && logger.isWarnEnabled()) {
logger.warn("Cannot find template location: " + location
+ " (please add some templates, check your Mustache configuration, or set spring.mustache."
+ "check-template-location=false)");
......
......@@ -34,6 +34,7 @@ import org.springframework.boot.web.reactive.error.ErrorWebExceptionHandler;
import org.springframework.context.ApplicationContext;
import org.springframework.core.NestedExceptionUtils;
import org.springframework.core.io.Resource;
import org.springframework.core.log.LogMessage;
import org.springframework.http.HttpLogging;
import org.springframework.http.HttpStatus;
import org.springframework.http.codec.HttpMessageReader;
......@@ -288,8 +289,8 @@ public abstract class AbstractErrorWebExceptionHandler implements ErrorWebExcept
}
if (HttpStatus.resolve(response.rawStatusCode()) != null
&& response.statusCode().equals(HttpStatus.INTERNAL_SERVER_ERROR)) {
logger.error(request.exchange().getLogPrefix() + "500 Server Error for " + formatRequest(request),
throwable);
logger.error(LogMessage.of(() -> String.format("%s 500 Server Error for %s",
request.exchange().getLogPrefix(), formatRequest(request))), throwable);
}
}
......
......@@ -21,6 +21,7 @@ import org.apache.commons.logging.LogFactory;
import org.springframework.beans.factory.InitializingBean;
import org.springframework.boot.devtools.livereload.LiveReloadServer;
import org.springframework.core.log.LogMessage;
/**
* Manages an optional {@link LiveReloadServer}. The {@link LiveReloadServer} may
......@@ -54,7 +55,7 @@ public class OptionalLiveReloadServer implements InitializingBean {
if (!this.server.isStarted()) {
this.server.start();
}
logger.info("LiveReload server is running on port " + this.server.getPort());
logger.info(LogMessage.format("LiveReload server is running on port %s", this.server.getPort()));
}
catch (Exception ex) {
logger.warn("Unable to start LiveReload server");
......
......@@ -48,6 +48,7 @@ import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Conditional;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Import;
import org.springframework.core.log.LogMessage;
import org.springframework.http.server.ServerHttpRequest;
/**
......@@ -126,7 +127,7 @@ public class RemoteDevToolsAutoConfiguration {
RemoteDevToolsProperties remote = properties.getRemote();
String servletContextPath = (servlet.getContextPath() != null) ? servlet.getContextPath() : "";
String url = servletContextPath + remote.getContextPath() + "/restart";
logger.warn("Listening for remote restart updates on " + url);
logger.warn(LogMessage.format("Listening for remote restart updates on %s", url));
Handler handler = new HttpRestartServerHandler(server);
return new UrlHandlerMapper(url, handler);
}
......
......@@ -26,6 +26,7 @@ import java.util.List;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.core.log.LogMessage;
import org.springframework.util.ResourceUtils;
/**
......@@ -58,8 +59,8 @@ public class ClassPathFolders implements Iterable<File> {
this.folders.add(ResourceUtils.getFile(url));
}
catch (Exception ex) {
logger.warn("Unable to get classpath URL " + url);
logger.trace("Unable to get classpath URL " + url, ex);
logger.warn(LogMessage.format("Unable to get classpath URL %s", url));
logger.trace(LogMessage.format("Unable to get classpath URL ", url), ex);
}
}
}
......
......@@ -32,6 +32,7 @@ import org.springframework.core.annotation.Order;
import org.springframework.core.env.ConfigurableEnvironment;
import org.springframework.core.env.Environment;
import org.springframework.core.env.MapPropertySource;
import org.springframework.core.log.LogMessage;
import org.springframework.util.ClassUtils;
/**
......@@ -80,12 +81,14 @@ public class DevToolsPropertyDefaultsPostProcessor implements EnvironmentPostPro
public void postProcessEnvironment(ConfigurableEnvironment environment, SpringApplication application) {
if (DevToolsEnablementDeducer.shouldEnable(Thread.currentThread()) && isLocalApplication(environment)) {
if (canAddProperties(environment)) {
logger.info("Devtools property defaults active! Set '" + ENABLED + "' to 'false' to disable");
logger.info(LogMessage.format("Devtools property defaults active! Set '%s' to 'false' to disable",
ENABLED));
environment.getPropertySources().addLast(new MapPropertySource("devtools", PROPERTIES));
}
if (isWebApplication(environment) && !environment.containsProperty(WEB_LOGGING)) {
logger.info("For additional web related logging consider setting the '" + WEB_LOGGING
+ "' property to 'DEBUG'");
logger.info(LogMessage.format(
"For additional web related logging consider setting the '%s' property to 'DEBUG'",
WEB_LOGGING));
}
}
}
......
......@@ -38,6 +38,7 @@ import org.springframework.boot.devtools.restart.classloader.ClassLoaderFile;
import org.springframework.boot.devtools.restart.classloader.ClassLoaderFile.Kind;
import org.springframework.boot.devtools.restart.classloader.ClassLoaderFiles;
import org.springframework.context.ApplicationListener;
import org.springframework.core.log.LogMessage;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
......@@ -114,8 +115,8 @@ public class ClassPathChangeUploader implements ApplicationListener<ClassPathCha
return;
}
catch (SocketException ex) {
logger.warn("A failure occurred when uploading to " + this.uri
+ ". Upload will be retried in 2 seconds");
logger.warn(LogMessage.format(
"A failure occurred when uploading to %s. Upload will be retried in 2 seconds", this.uri));
logger.debug("Upload failure", ex);
Thread.sleep(2000);
}
......@@ -129,7 +130,7 @@ public class ClassPathChangeUploader implements ApplicationListener<ClassPathCha
private void logUpload(ClassLoaderFiles classLoaderFiles) {
int size = classLoaderFiles.size();
logger.info("Uploaded " + size + " class " + ((size != 1) ? "resources" : "resource"));
logger.info(LogMessage.format("Uploaded %s class %s", size, (size != 1) ? "resources" : "resource"));
}
private byte[] serialize(ClassLoaderFiles classLoaderFiles) throws IOException {
......
......@@ -53,6 +53,7 @@ import org.springframework.context.ApplicationListener;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.support.PropertySourcesPlaceholderConfigurer;
import org.springframework.core.log.LogMessage;
import org.springframework.http.client.ClientHttpRequestFactory;
import org.springframework.http.client.ClientHttpRequestInterceptor;
import org.springframework.http.client.InterceptingClientHttpRequestFactory;
......@@ -119,8 +120,9 @@ public class RemoteClientConfiguration implements InitializingBean {
logger.warn("Remote restart is disabled.");
}
if (!this.remoteUrl.startsWith("https://")) {
logger.warn("The connection to " + this.remoteUrl
+ " is insecure. You should use a URL starting with 'https://'.");
logger.warn(LogMessage.format(
"The connection to %s is insecure. You should use a URL starting with 'https://'.",
this.remoteUrl));
}
}
......
......@@ -37,6 +37,7 @@ import org.apache.commons.logging.Log;
import org.springframework.boot.devtools.logger.DevToolsLogFactory;
import org.springframework.boot.devtools.settings.DevToolsSettings;
import org.springframework.core.log.LogMessage;
import org.springframework.util.StringUtils;
/**
......@@ -171,9 +172,9 @@ final class ChangeableUrls implements Iterable<URL> {
}
}
if (!nonExistentEntries.isEmpty()) {
logger.info("The Class-Path manifest attribute in " + jarFile.getName()
logger.info(LogMessage.of(() -> "The Class-Path manifest attribute in " + jarFile.getName()
+ " referenced one or more files that do not exist: "
+ StringUtils.collectionToCommaDelimitedString(nonExistentEntries));
+ StringUtils.collectionToCommaDelimitedString(nonExistentEntries)));
}
return urls;
}
......
......@@ -26,6 +26,7 @@ import org.springframework.boot.context.event.ApplicationStartingEvent;
import org.springframework.context.ApplicationEvent;
import org.springframework.context.ApplicationListener;
import org.springframework.core.Ordered;
import org.springframework.core.log.LogMessage;
/**
* {@link ApplicationListener} to initialize the {@link Restarter}.
......@@ -73,7 +74,8 @@ public class RestartApplicationListener implements ApplicationListener<Applicati
Restarter.initialize(args, false, initializer, restartOnInitialize);
}
else {
logger.info("Restart disabled due to System property '" + ENABLED_PROPERTY + "' being set to false");
logger.info(LogMessage.format("Restart disabled due to System property '%s' being set to false",
ENABLED_PROPERTY));
Restarter.disable();
}
}
......
......@@ -35,6 +35,7 @@ import org.apache.commons.logging.LogFactory;
import org.springframework.boot.devtools.tunnel.payload.HttpTunnelPayload;
import org.springframework.boot.devtools.tunnel.payload.HttpTunnelPayloadForwarder;
import org.springframework.core.log.LogMessage;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
import org.springframework.http.client.ClientHttpRequest;
......@@ -92,7 +93,7 @@ public class HttpTunnelConnection implements TunnelConnection {
@Override
public TunnelChannel open(WritableByteChannel incomingChannel, Closeable closeable) throws Exception {
logger.trace("Opening HTTP tunnel to " + this.uri);
logger.trace(LogMessage.format("Opening HTTP tunnel to %s", this.uri));
return new TunnelChannel(incomingChannel, closeable);
}
......@@ -152,7 +153,8 @@ public class HttpTunnelConnection implements TunnelConnection {
}
catch (IOException ex) {
if (ex instanceof ConnectException) {
logger.warn("Failed to connect to remote application at " + HttpTunnelConnection.this.uri);
logger.warn(LogMessage.format("Failed to connect to remote application at %s",
HttpTunnelConnection.this.uri));
}
else {
logger.trace("Unexpected connection error", ex);
......
......@@ -30,6 +30,7 @@ import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.beans.factory.SmartInitializingSingleton;
import org.springframework.core.log.LogMessage;
import org.springframework.util.Assert;
/**
......@@ -88,7 +89,7 @@ public class TunnelClient implements SmartInitializingSingleton {
ServerSocketChannel serverSocketChannel = ServerSocketChannel.open();
serverSocketChannel.socket().bind(new InetSocketAddress(this.listenPort));
int port = serverSocketChannel.socket().getLocalPort();
logger.trace("Listening for TCP traffic to tunnel on port " + port);
logger.trace(LogMessage.format("Listening for TCP traffic to tunnel on port %s", port));
this.serverThread = new ServerThread(serverSocketChannel);
this.serverThread.start();
return port;
......@@ -144,7 +145,8 @@ public class TunnelClient implements SmartInitializingSingleton {
}
public void close() throws IOException {
logger.trace("Closing tunnel client on port " + this.serverSocketChannel.socket().getLocalPort());
logger.trace(LogMessage.format("Closing tunnel client on port %s",
this.serverSocketChannel.socket().getLocalPort()));
this.serverSocketChannel.close();
this.acceptConnections = false;
interrupt();
......
......@@ -28,6 +28,7 @@ import java.nio.channels.SocketChannel;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.core.log.LogMessage;
import org.springframework.util.Assert;
/**
......@@ -54,7 +55,7 @@ public class SocketTargetServerConnection implements TargetServerConnection {
@Override
public ByteChannel open(int socketTimeout) throws IOException {
SocketAddress address = new InetSocketAddress(this.portProvider.getPort());
logger.trace("Opening tunnel connection to target server on " + address);
logger.trace(LogMessage.format("Opening tunnel connection to target server on %s", address));
SocketChannel channel = SocketChannel.open(address);
channel.socket().setSoTimeout(socketTimeout);
return new TimeoutAwareChannel(channel);
......
......@@ -42,6 +42,7 @@ import org.springframework.boot.ansi.AnsiElement;
import org.springframework.boot.ansi.AnsiOutput;
import org.springframework.core.env.Environment;
import org.springframework.core.io.Resource;
import org.springframework.core.log.LogMessage;
import org.springframework.util.Assert;
/**
......@@ -78,8 +79,8 @@ public class ImageBanner implements Banner {
printBanner(environment, out);
}
catch (Throwable ex) {
logger.warn("Image banner not printable: " + this.image + " (" + ex.getClass() + ": '" + ex.getMessage()
+ "')");
logger.warn(LogMessage.format("Image banner not printable: %s (%s: '%s')", this.image, ex.getClass(),
ex.getMessage()));
logger.debug("Image banner printing failure", ex);
}
finally {
......
......@@ -35,6 +35,7 @@ import org.springframework.core.env.MutablePropertySources;
import org.springframework.core.env.PropertyResolver;
import org.springframework.core.env.PropertySourcesPropertyResolver;
import org.springframework.core.io.Resource;
import org.springframework.core.log.LogMessage;
import org.springframework.util.Assert;
import org.springframework.util.StreamUtils;
......@@ -70,9 +71,8 @@ public class ResourceBanner implements Banner {
out.println(banner);
}
catch (Exception ex) {
logger.warn(
"Banner not printable: " + this.resource + " (" + ex.getClass() + ": '" + ex.getMessage() + "')",
ex);
logger.warn(LogMessage.format("Banner not printable: %s (%s: '%s')", this.resource, ex.getClass(),
ex.getMessage()), ex);
}
}
......
......@@ -112,16 +112,19 @@ class StartupInfoLogger {
long startTime = System.currentTimeMillis();
append(message, "on ", () -> InetAddress.getLocalHost().getHostName());
long resolveTime = System.currentTimeMillis() - startTime;
if (resolveTime > HOST_NAME_RESOLVE_THRESHOLD && logger.isWarnEnabled()) {
StringBuilder warning = new StringBuilder();
warning.append("InetAddress.getLocalHost().getHostName() took ");
warning.append(resolveTime);
warning.append(" milliseconds to respond.");
warning.append(" Please verify your network configuration");
if (System.getProperty("os.name").toLowerCase().contains("mac")) {
warning.append(" (macOS machines may need to add entries to /etc/hosts)");
}
logger.warn(warning.append("."));
if (resolveTime > HOST_NAME_RESOLVE_THRESHOLD) {
logger.warn(LogMessage.of(() -> {
StringBuilder warning = new StringBuilder();
warning.append("InetAddress.getLocalHost().getHostName() took ");
warning.append(resolveTime);
warning.append(" milliseconds to respond.");
warning.append(" Please verify your network configuration");
if (System.getProperty("os.name").toLowerCase().contains("mac")) {
warning.append(" (macOS machines may need to add entries to /etc/hosts)");
}
warning.append(".");
return warning;
}));
}
}
......
......@@ -64,12 +64,14 @@ public class FileEncodingApplicationListener
String encoding = System.getProperty("file.encoding");
String desired = environment.getProperty("spring.mandatory-file-encoding");
if (encoding != null && !desired.equalsIgnoreCase(encoding)) {
logger.error("System property 'file.encoding' is currently '" + encoding + "'. It should be '" + desired
+ "' (as defined in 'spring.mandatoryFileEncoding').");
logger.error("Environment variable LANG is '" + System.getenv("LANG")
+ "'. You could use a locale setting that matches encoding='" + desired + "'.");
logger.error("Environment variable LC_ALL is '" + System.getenv("LC_ALL")
+ "'. You could use a locale setting that matches encoding='" + desired + "'.");
if (logger.isErrorEnabled()) {
logger.error("System property 'file.encoding' is currently '" + encoding + "'. It should be '" + desired
+ "' (as defined in 'spring.mandatoryFileEncoding').");
logger.error("Environment variable LANG is '" + System.getenv("LANG")
+ "'. You could use a locale setting that matches encoding='" + desired + "'.");
logger.error("Environment variable LC_ALL is '" + System.getenv("LC_ALL")
+ "'. You could use a locale setting that matches encoding='" + desired + "'.");
}
throw new IllegalStateException("The Java Virtual Machine has not been configured to use the "
+ "desired default character encoding (" + desired + ").");
}
......
......@@ -50,6 +50,7 @@ import org.springframework.core.Ordered;
import org.springframework.core.ResolvableType;
import org.springframework.core.env.ConfigurableEnvironment;
import org.springframework.core.env.Environment;
import org.springframework.core.log.LogMessage;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
import org.springframework.util.ResourceUtils;
......@@ -416,7 +417,7 @@ public class LoggingApplicationListener implements GenericApplicationListener {
system.setLogLevel(name, level);
}
catch (RuntimeException ex) {
this.logger.error("Cannot set level '" + level + "' for '" + name + "'");
this.logger.error(LogMessage.format("Cannot set level '%s' for '%s'", level, name));
}
};
}
......
......@@ -77,7 +77,7 @@ final class FailureAnalyzers implements SpringBootExceptionReporter {
analyzers.add((FailureAnalyzer) constructor.newInstance());
}
catch (Throwable ex) {
logger.trace("Failed to load " + analyzerName, ex);
logger.trace(LogMessage.format("Failed to load %s", analyzerName), ex);
}
}
AnnotationAwareOrderComparator.sort(analyzers);
......
......@@ -25,6 +25,7 @@ import org.apache.commons.logging.LogFactory;
import org.springframework.boot.system.SystemProperties;
import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationListener;
import org.springframework.core.log.LogMessage;
import org.springframework.util.Assert;
import org.springframework.util.FileCopyUtils;
import org.springframework.util.StringUtils;
......@@ -91,7 +92,7 @@ public class WebServerPortFileWriter implements ApplicationListener<WebServerIni
portFile.deleteOnExit();
}
catch (Exception ex) {
logger.warn(String.format("Cannot create port file %s", this.file));
logger.warn(LogMessage.format("Cannot create port file %s", this.file));
}
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment