Merge pull request #890 from Minecrell/slf4j

Allow plugins to use SLF4J for logging
This commit is contained in:
kashike 2017-09-23 12:57:11 -07:00 committed by GitHub
commit e5e5063a7d
7 changed files with 351 additions and 0 deletions

View file

@ -0,0 +1,44 @@
From 862e7c3bc002483f3d98c7f24ea0e06d6575a3eb Mon Sep 17 00:00:00 2001
From: Minecrell <dev@minecrell.net>
Date: Thu, 21 Sep 2017 16:14:13 +0200
Subject: [PATCH] Handle plugin prefixes in implementation logging
configuration
Currently, plugin prefixes are prepended to the log message in
the PluginLogger before passing the message to the underlying
logging framework. This is bad design because they need to be
stripped manually when using custom appenders to log messages
in a different format.
Additionally, it makes integration of alternative logging APIs hard
because all logging must go through the PluginLogger. Avoid using
PluginLogger and create a regular logger using the plugin name.
The implementation should handle plugin prefixes by displaying
logger names when appropriate.
diff --git a/src/main/java/org/bukkit/plugin/java/JavaPlugin.java b/src/main/java/org/bukkit/plugin/java/JavaPlugin.java
index 16b1eb37..0abad9ad 100644
--- a/src/main/java/org/bukkit/plugin/java/JavaPlugin.java
+++ b/src/main/java/org/bukkit/plugin/java/JavaPlugin.java
@@ -50,7 +50,7 @@ public abstract class JavaPlugin extends PluginBase {
private boolean naggable = true;
private FileConfiguration newConfig = null;
private File configFile = null;
- private PluginLogger logger = null;
+ private Logger logger = null; // Paper - PluginLogger -> Logger
public JavaPlugin() {
final ClassLoader classLoader = this.getClass().getClassLoader();
@@ -277,7 +277,8 @@ public abstract class JavaPlugin extends PluginBase {
this.dataFolder = dataFolder;
this.classLoader = classLoader;
this.configFile = new File(dataFolder, "config.yml");
- this.logger = new PluginLogger(this);
+ // Paper - Handle plugin prefix in implementation
+ this.logger = Logger.getLogger(description.getPrefix() != null ? description.getPrefix() : description.getName());
}
/**
--
2.14.1

View file

@ -0,0 +1,54 @@
From fd9c40f3f333eab91c410c02c20b7b9f7c64ebf2 Mon Sep 17 00:00:00 2001
From: Minecrell <dev@minecrell.net>
Date: Thu, 21 Sep 2017 16:33:12 +0200
Subject: [PATCH] Allow plugins to use SLF4J for logging
SLF4J is a commonly used abstraction for various logging frameworks
such as java.util.logging (JUL) or Log4j. Currently, plugins are
required to do all their logging using the provided JUL logger.
This is annoying for plugins that target multiple platforms or when
using libraries that log messages using SLF4J.
Expose SLF4J as optional logging API for plugins, so they can use
it without having to shade it in the plugin and going through
several layers of logging abstraction.
diff --git a/pom.xml b/pom.xml
index ad385f45..b83c6eb1 100644
--- a/pom.xml
+++ b/pom.xml
@@ -122,6 +122,14 @@
<scope>compile</scope>
</dependency>
+ <!-- Paper - Add SLF4J -->
+ <dependency>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-api</artifactId>
+ <version>1.7.25</version>
+ <scope>compile</scope>
+ </dependency>
+
<!-- testing -->
<dependency>
<groupId>junit</groupId>
diff --git a/src/main/java/org/bukkit/plugin/Plugin.java b/src/main/java/org/bukkit/plugin/Plugin.java
index c4e22c62..02670254 100644
--- a/src/main/java/org/bukkit/plugin/Plugin.java
+++ b/src/main/java/org/bukkit/plugin/Plugin.java
@@ -157,6 +157,12 @@ public interface Plugin extends TabExecutor {
*/
public Logger getLogger();
+ // Paper start - Add SLF4J logger
+ default org.slf4j.Logger getSLF4JLogger() {
+ return org.slf4j.LoggerFactory.getLogger(getLogger().getName());
+ }
+ // Paper end
+
/**
* Returns the name of the plugin.
* <p>
--
2.14.1

View file

@ -0,0 +1,63 @@
From a308504bd0a184f8e568e063143a3df4ec10fefb Mon Sep 17 00:00:00 2001
From: Minecrell <dev@minecrell.net>
Date: Thu, 21 Sep 2017 19:41:20 +0200
Subject: [PATCH] Add workaround for plugins modifying the parent of the plugin
logger
Essentials uses a custom logger name ("Essentials") instead of the
plugin logger. Log messages are redirected to the plugin logger by
setting the parent of the "Essentials" logger to the plugin logger.
With our changes, the plugin logger is now also called "Essentials",
resulting in an infinite loop. Make sure plugins can't change the
parent of the plugin logger to avoid this.
diff --git a/src/main/java/com/destroystokyo/paper/utils/PaperPluginLogger.java b/src/main/java/com/destroystokyo/paper/utils/PaperPluginLogger.java
new file mode 100644
index 00000000..1c93cf30
--- /dev/null
+++ b/src/main/java/com/destroystokyo/paper/utils/PaperPluginLogger.java
@@ -0,0 +1,27 @@
+package com.destroystokyo.paper.utils;
+
+import org.bukkit.plugin.PluginDescriptionFile;
+
+import java.util.logging.LogManager;
+import java.util.logging.Logger;
+
+/**
+ * Prevents plugins (e.g. Essentials) from changing the parent of the plugin logger.
+ */
+public class PaperPluginLogger extends Logger {
+
+ public PaperPluginLogger(PluginDescriptionFile description) {
+ super(description.getPrefix() != null ? description.getPrefix() : description.getName(), null);
+ LogManager.getLogManager().addLogger(this);
+ }
+
+ @Override
+ public void setParent(Logger parent) {
+ if (getParent() != null) {
+ warning("Ignoring attempt to change parent of plugin logger");
+ } else {
+ super.setParent(parent);
+ }
+ }
+
+}
diff --git a/src/main/java/org/bukkit/plugin/java/PluginClassLoader.java b/src/main/java/org/bukkit/plugin/java/PluginClassLoader.java
index b2cbf9e4..7a95239a 100644
--- a/src/main/java/org/bukkit/plugin/java/PluginClassLoader.java
+++ b/src/main/java/org/bukkit/plugin/java/PluginClassLoader.java
@@ -59,6 +59,8 @@ public final class PluginClassLoader extends URLClassLoader { // Spigot
this.dataFolder = dataFolder;
this.file = file;
+ new com.destroystokyo.paper.utils.PaperPluginLogger(description); // Paper - Register logger early
+
try {
Class<?> jarClass;
try {
--
2.14.1

View file

@ -0,0 +1,51 @@
From a7ced92f0d09b6e25e5c66dc24db2da45f3341f7 Mon Sep 17 00:00:00 2001
From: Minecrell <dev@minecrell.net>
Date: Mon, 18 Sep 2017 12:00:03 +0200
Subject: [PATCH] Use Log4j IOStreams to redirect System.out/err to logger
Log4j2 provides an optimized implementation of PrintStream that
redirects its output to a logger. Use it instead of a custom
implementation for minor performance improvements and some fixes.
With the old implementation, each call to System.print()
results in a separate line, even though it should not result in
a line break. Log4j's implementation handles it correctly.
diff --git a/pom.xml b/pom.xml
index 98972114d..aff997468 100644
--- a/pom.xml
+++ b/pom.xml
@@ -97,6 +97,13 @@
<scope>runtime</scope>
</dependency>
+ <!-- Paper - Add additional Log4J dependencies -->
+ <dependency>
+ <groupId>org.apache.logging.log4j</groupId>
+ <artifactId>log4j-iostreams</artifactId>
+ <version>2.8.1</version>
+ </dependency>
+
<!-- testing -->
<dependency>
<groupId>junit</groupId>
diff --git a/src/main/java/net/minecraft/server/DedicatedServer.java b/src/main/java/net/minecraft/server/DedicatedServer.java
index b3f1aa999..854455711 100644
--- a/src/main/java/net/minecraft/server/DedicatedServer.java
+++ b/src/main/java/net/minecraft/server/DedicatedServer.java
@@ -129,8 +129,10 @@ public class DedicatedServer extends MinecraftServer implements IMinecraftServer
*/
// Paper end
- System.setOut(new PrintStream(new LoggerOutputStream(logger, Level.INFO), true));
- System.setErr(new PrintStream(new LoggerOutputStream(logger, Level.WARN), true));
+ // Paper start - Use Log4j IOStreams
+ System.setOut(org.apache.logging.log4j.io.IoBuilder.forLogger(logger).setLevel(Level.INFO).buildPrintStream());
+ System.setErr(org.apache.logging.log4j.io.IoBuilder.forLogger(logger).setLevel(Level.WARN).buildPrintStream());
+ // Paper end
// CraftBukkit end
thread.setDaemon(true);
--
2.14.1

View file

@ -0,0 +1,74 @@
From c5fa4d29b7cf19517b522dc34ce0d3c3aec40b04 Mon Sep 17 00:00:00 2001
From: Minecrell <dev@minecrell.net>
Date: Thu, 21 Sep 2017 16:14:55 +0200
Subject: [PATCH] Handle plugin prefixes using Log4J configuration
Display logger name in the console for all loggers except the
root logger, Bukkit's logger ("Minecraft") and Minecraft loggers.
Since plugins now use the plugin name as logger name this will
restore the plugin prefixes without having to prepend them manually
to the log messages.
Logger prefixes are shown by default for all loggers except for
the root logger, the Minecraft/Mojang loggers and the Bukkit loggers.
This may cause additional prefixes to be disabled for plugins bypassing
the plugin logger.
diff --git a/pom.xml b/pom.xml
index aff997468..dfb006aa0 100644
--- a/pom.xml
+++ b/pom.xml
@@ -94,7 +94,7 @@
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-core</artifactId>
<version>2.8.1</version>
- <scope>runtime</scope>
+ <scope>compile</scope>
</dependency>
<!-- Paper - Add additional Log4J dependencies -->
diff --git a/src/main/java/org/spigotmc/SpigotConfig.java b/src/main/java/org/spigotmc/SpigotConfig.java
index 712fc1f9b..b5bfb15fa 100644
--- a/src/main/java/org/spigotmc/SpigotConfig.java
+++ b/src/main/java/org/spigotmc/SpigotConfig.java
@@ -282,7 +282,7 @@ public class SpigotConfig
private static void playerSample()
{
playerSample = getInt( "settings.sample-count", 12 );
- System.out.println( "Server Ping Player Sample Count: " + playerSample );
+ Bukkit.getLogger().log( Level.INFO, "Server Ping Player Sample Count: {0}", playerSample ); // Paper - Use logger
}
public static int playerShuffle;
diff --git a/src/main/resources/log4j2.xml b/src/main/resources/log4j2.xml
index 08b6bb7f9..9f8334376 100644
--- a/src/main/resources/log4j2.xml
+++ b/src/main/resources/log4j2.xml
@@ -2,10 +2,22 @@
<Configuration status="WARN">
<Appenders>
<TerminalConsole name="TerminalConsole">
- <PatternLayout pattern="%highlightError{[%d{HH:mm:ss} %level]: %minecraftFormatting{%msg}%n%xEx}" />
+ <PatternLayout>
+ <LoggerNamePatternSelector defaultPattern="%highlightError{[%d{HH:mm:ss} %level]: [%logger] %minecraftFormatting{%msg}%n%xEx}">
+ <!-- Log root, Minecraft, Mojang and Bukkit loggers without prefix -->
+ <PatternMatch key=",net.minecraft.,Minecraft,com.mojang."
+ pattern="%highlightError{[%d{HH:mm:ss} %level]: %minecraftFormatting{%msg}%n%xEx}" />
+ </LoggerNamePatternSelector>
+ </PatternLayout>
</TerminalConsole>
<RollingRandomAccessFile name="File" fileName="logs/latest.log" filePattern="logs/%d{yyyy-MM-dd}-%i.log.gz">
- <PatternLayout pattern="[%d{HH:mm:ss}] [%t/%level]: %minecraftFormatting{%msg}{strip}%n" />
+ <PatternLayout>
+ <LoggerNamePatternSelector defaultPattern="[%d{HH:mm:ss}] [%t/%level]: [%logger] %minecraftFormatting{%msg}{strip}%n">
+ <!-- Log root, Minecraft, Mojang and Bukkit loggers without prefix -->
+ <PatternMatch key=",net.minecraft.,Minecraft,com.mojang."
+ pattern="[%d{HH:mm:ss}] [%t/%level]: %minecraftFormatting{%msg}{strip}%n" />
+ </LoggerNamePatternSelector>
+ </PatternLayout>
<Policies>
<TimeBasedTriggeringPolicy />
<OnStartupTriggeringPolicy />
--
2.14.1

View file

@ -0,0 +1,26 @@
From e4b022439652ed4b044c10cadb7525671bcd54f3 Mon Sep 17 00:00:00 2001
From: Minecrell <dev@minecrell.net>
Date: Thu, 21 Sep 2017 16:33:35 +0200
Subject: [PATCH] Include Log4J2 SLF4J implementation
diff --git a/pom.xml b/pom.xml
index fa726851b..647b2c619 100644
--- a/pom.xml
+++ b/pom.xml
@@ -98,6 +98,12 @@
</dependency>
<!-- Paper - Add additional Log4J dependencies -->
+ <dependency>
+ <groupId>org.apache.logging.log4j</groupId>
+ <artifactId>log4j-slf4j-impl</artifactId>
+ <version>2.8.1</version>
+ <scope>runtime</scope>
+ </dependency>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-iostreams</artifactId>
--
2.14.1

View file

@ -0,0 +1,39 @@
From 8f6662b78aa50b814c65312928337c13e61d2de7 Mon Sep 17 00:00:00 2001
From: Minecrell <dev@minecrell.net>
Date: Sat, 23 Sep 2017 21:07:20 +0200
Subject: [PATCH] Disable logger prefix for various plugins bypassing the
plugin logger
Some plugins bypass the plugin logger and add the plugin prefix
manually to the log message. Since they use other logger names
(e.g. qualified class names) these would now also appear in the
log. Disable the logger prefix for these plugins so the messages
show up correctly.
diff --git a/src/main/resources/log4j2.xml b/src/main/resources/log4j2.xml
index 9f8334376..6711e6dff 100644
--- a/src/main/resources/log4j2.xml
+++ b/src/main/resources/log4j2.xml
@@ -5,7 +5,8 @@
<PatternLayout>
<LoggerNamePatternSelector defaultPattern="%highlightError{[%d{HH:mm:ss} %level]: [%logger] %minecraftFormatting{%msg}%n%xEx}">
<!-- Log root, Minecraft, Mojang and Bukkit loggers without prefix -->
- <PatternMatch key=",net.minecraft.,Minecraft,com.mojang."
+ <!-- Disable prefix for various plugins that bypass the plugin logger -->
+ <PatternMatch key=",net.minecraft.,Minecraft,com.mojang.,com.sk89q.,ru.tehkode.,Minecraft.AWE"
pattern="%highlightError{[%d{HH:mm:ss} %level]: %minecraftFormatting{%msg}%n%xEx}" />
</LoggerNamePatternSelector>
</PatternLayout>
@@ -14,7 +15,8 @@
<PatternLayout>
<LoggerNamePatternSelector defaultPattern="[%d{HH:mm:ss}] [%t/%level]: [%logger] %minecraftFormatting{%msg}{strip}%n">
<!-- Log root, Minecraft, Mojang and Bukkit loggers without prefix -->
- <PatternMatch key=",net.minecraft.,Minecraft,com.mojang."
+ <!-- Disable prefix for various plugins that bypass the plugin logger -->
+ <PatternMatch key=",net.minecraft.,Minecraft,com.mojang.,com.sk89q.,ru.tehkode.,Minecraft.AWE"
pattern="[%d{HH:mm:ss}] [%t/%level]: %minecraftFormatting{%msg}{strip}%n" />
</LoggerNamePatternSelector>
</PatternLayout>
--
2.14.1