chirs241097 / jline4mcdsrv

Command history, auto completion and syntax highlighting for every fabric server.
MIT License
24 stars 4 forks source link

[Feature Request] Better Log4j Config compatibility #25

Closed unilock closed 4 months ago

unilock commented 4 months ago

I use Better Log4j Config on my heavily modded server to adjust logging parameters to make issues easier to debug, and to filter out log spam from various mods.

Unfortunately, JLine4MCDSrv seems to completely override / reset the changes Better Log4j Config makes to the Log4j config.

Would it be possible to have some sort of compatibility between the two mods?

Fourmisain commented 4 months ago

You don't need a mod for that, you can just start your server with your Log4j configuration like that: -Dlog4j.configurationFile=/path/to/your/log4j2.xml (and -Dlog4j2.formatMsgNoLookups=true -Dlog4j.skipJansi=false while you're at it) and change the logPattern inside jline4mcdsrv.toml accordingly.

You can find the log config vanilla uses for 1.21 inside logging-1.2.7.jar. Here's a slightly modified version, matching jline4mcdsrv's default (colors plus shortened logger name):

<?xml version="1.0" encoding="UTF-8"?>
<Configuration status="WARN">
    <Appenders>
        <Console name="SysOut" target="SYSTEM_OUT">
            <PatternLayout pattern="%style{[%d{HH:mm:ss}]}{blue} %highlight{[%t/%level]}{FATAL=red, ERROR=red, WARN=yellow, INFO=green, DEBUG=green, TRACE=blue} %style{(%logger{1})}{cyan} %highlight{%msg{nolookups}%n}{FATAL=red, ERROR=red, WARN=normal, INFO=normal, DEBUG=normal, TRACE=normal}" />
        </Console>
        <RollingRandomAccessFile name="File" fileName="logs/latest.log" filePattern="logs/%d{yyyy-MM-dd}-%i.log.gz">
            <PatternLayout pattern="[%d{HH:mm:ss}] [%t/%level] (%logger): %msg{nolookups}%n" />
            <Policies>
                <TimeBasedTriggeringPolicy />
                <OnStartupTriggeringPolicy />
            </Policies>
        </RollingRandomAccessFile>
    </Appenders>
    <Loggers>
        <Root level="info">
            <filters>
                <MarkerFilter marker="NETWORK_PACKETS" onMatch="DENY" onMismatch="NEUTRAL" />
            </filters>
            <AppenderRef ref="SysOut"/>
            <AppenderRef ref="File"/>
        </Root>
    </Loggers>
</Configuration>

Change the SysOut and File patterns however you like, just make sure SysOut is consistent with jline4mcdsrv.toml. If you want to filter certain loggers, you need the full logger name (%logger, above config writes it to the log files) and something like this:

    <Loggers>
        <!-- deny all logging -->
        <Logger name="FullLoggerName" level="off" additivity="false"></Logger>

        <!-- deny info level logging -->
        <Logger name="AnotherLoggerName">
            <Filters>
            <LevelRangeFilter minLevel="INFO" maxLevel="INFO" onMatch="DENY" onMismatch="NEUTRAL" />
            </Filters>
        </Logger>

       <Root ...

Issue with compatiblity is that JLine4MCDSrv wants to add colors, and for that it needs to change the log pattern. Other than that, nothing should really change, filters, appenders etc should all be unaffected, so I'm not sure what you mean by "completely override(n)".

Maybe I can add a way for JLine4MCDSrv to not override the pattern, but copy it from SysOut by setting logPattern to "" or something (afaik Toml doesn't support null) for slightly better compatibility, though I'm not sure if it'll really help.

unilock commented 4 months ago

You don't need a mod for that, you can just start your server with your Log4j configuration like that: ...

I'm aware, but it's easier to ship my custom Log4j config with a modpack with the mod.

My Log4j config looks like this: ```xml ```

Note the RegexFilters.

I guess the problem is that JLine4MCDSrv doesn't use the SysOut appender at all? Would it be possible to use a Log4j config file to filter JLine4MCDSrv's appender?

Maybe I can add a way for JLine4MCDSrv to not override the pattern, but copy it from SysOut by setting logPattern to "" or something (afaik Toml doesn't support null) for slightly better compatibility, though I'm not sure if it'll really help.

If it were possible to have JLine4MCDSrv copy all of the configuration from SysOut except the logPattern, that might also be helpful.

unilock commented 4 months ago

Is there a way to retain JLine4MCDSrv's command auto-complete feature without having it replace the SysOut appender as well? That way, I could replace the logging pattern with a colored one myself, while still retaining JLine4MCDSrv's ability to auto-complete commands.

Fourmisain commented 4 months ago

I guess the problem is that JLine4MCDSrv doesn't use the SysOut appender at all?

Yes, it replaces it with it's own appender, to actually be able use JLine - now I see what you mean by (some) stuff being overridden.

Would it be possible to use a Log4j config file to filter JLine4MCDSrv's appender?

I'm not sure if that's possible - and it's probably isn't the way to go. My understanding of log configs is that they are a blueprints that define how the whole logging structure is to be constructed. E.g. the appender type Console is an actual class registered as a plugin, built through a plugin builder in this case. While jline4mcdsrv's custom JLineAppender could be registered as a plugin too and used in the config like <JLineAppender name="JLine" ..., it would be constructed way too early, before even the MinecraftServer is instanciated (which the appender needs for command completion and syntax highlighting).

If it were possible to have JLine4MCDSrv copy all of the configuration from SysOut except the logPattern, that might also be helpful.

Yeah, that's what I was thinking too, gonna see if that's doable.

...

And it is! For filters at least.

There's some other stuff, like ConsoleAppender specific attributes, the target attribute (whether to print to SYSTEM_OUT or SYSTEM_ERR), and a properties array, which I think represent <Property>s inside an appender, all of which sounds pretty esoteric or hard to support, so I'll just copy the filters for now.

Ran into an issue with StackDeobfuscator, since it actually removes all appenders, but was able to still get and apply the filter. Unsure if what StackDeobfuscator does has an effect on how the appenders themselves work, but it doesn't appear so...?

Is there a way to retain JLine4MCDSrv's command auto-complete feature without having it replace the SysOut appender as well?

No, that is the whole reason why it is being replaced. You can't have the SysOut appender write to the console while JLine tries to manage the inputs, it needs full control over everything, hence SysOut has to go.

Just released a new version that copies the filters, give it a try!

unilock commented 4 months ago

Ah, now I remember the problem I was having with Better Log4j Config alongside JLine4MCDSrv - somehow, the latter becomes configured to display all log messages (including debug and trace messages), which makes the log rather difficult to read.

Example of the server console when both mods are installed ``` [15:58:36] [Server thread/DEBUG] (Quilt Loader/Mixin) #lithium:lithium.mixins.json:chunk.entity_class_groups.TypeFilterableListMixin from mod lithium: Class version 61 required is higher than the class version supported by the current version of Mixin (JAVA_16 supports class version 60) [15:58:36] [Server thread/TRACE] (Quilt Loader/Mixin) Added class metadata for me/jellysquid/mods/lithium/common/world/chunk/ClassGroupFilterableList to metadata cache [15:58:36] [Server thread/TRACE] (Quilt Loader/Mixin) Added class metadata for it/unimi/dsi/fastutil/objects/Reference2ReferenceArrayMap to metadata cache [15:58:36] [Server thread/TRACE] (Quilt Loader/Mixin) Added class metadata for me/jellysquid/mods/lithium/common/entity/EntityClassGroup to metadata cache [15:58:36] [Server thread/TRACE] (Quilt Loader/Mixin) Added class metadata for it/unimi/dsi/fastutil/objects/ReferenceLinkedOpenHashSet to metadata cache [15:58:36] [Server thread/TRACE] (Quilt Loader/Mixin) Added class metadata for it/unimi/dsi/fastutil/objects/AbstractReferenceSortedSet to metadata cache [15:58:36] [Server thread/TRACE] (Quilt Loader/Mixin) Added class metadata for it/unimi/dsi/fastutil/objects/ReferenceSortedSet to metadata cache [15:58:36] [Server thread/DEBUG] (Quilt Loader/Mixin) Mixing collections.entity_by_type.TypeFilterableListMixin from #lithium:lithium.mixins.json into net.minecraft.class_3509 [15:58:36] [Server thread/DEBUG] (Quilt Loader/Mixin) #lithium:lithium.mixins.json:collections.entity_by_type.TypeFilterableListMixin from mod lithium: Class version 61 required is higher than the class version supported by the current version of Mixin (JAVA_16 supports class version 60) [15:58:36] [Server thread/DEBUG] (Quilt Loader/Mixin) Mixing collections.entity_filtering.TypeFilterableListMixin from #lithium:lithium.mixins.json into net.minecraft.class_3509 [15:58:36] [Server thread/DEBUG] (Quilt Loader/Mixin) #lithium:lithium.mixins.json:collections.entity_filtering.TypeFilterableListMixin from mod lithium: Class version 61 required is higher than the class version supported by the current version of Mixin (JAVA_16 supports class version 60) [15:58:36] [Server thread/DEBUG] (Quilt Loader/Mixin) Mixing block.hopper.TypeFilterableListMixin from #lithium:lithium.mixins.json into net.minecraft.class_3509 ```

Any idea what the problem could be?

Fourmisain commented 4 months ago

Uhh, no idea really. 🤔 I guess I need to look into how Better Log4j Config works exactly and do some testing with it, will do that tomorrow.

And just to make sure, these messages appear after "Starting jline4mcdsrv", right? Because everything before that should be completely unaffected.

Fourmisain commented 4 months ago

It's a configuration issue with the way Better Log4j Config changes the log levels:

        <Logger level="${sys:fabric.log.level:-info}" name="net.minecraft"/>
        <Root level="all">
            <AppenderRef ref="DebugFile" level="${sys:fabric.log.debug.level:-debug}"/>
            <AppenderRef ref="SysOut" level="${sys:fabric.log.level:-info}"/>
            <AppenderRef ref="LatestFile" level="${sys:fabric.log.level:-info}"/>
            <AppenderRef ref="ServerGuiConsole" level="${sys:fabric.log.level:-info}"/>
        </Root>

Normally (in vanilla) the Root logger has info level logging and the AppenderRefs don't set a level. Since jline4mcdsrv adds the JLine appender directly under the root logger, it'll log everything from the root logger, which will be everything with Better Log4j Config. I changed it copy the level of the SysOut AppenderRef (and the filters too, because of course Log4j allows you to have them there too... 😅)

And everything appears to work now! New version is up!

unilock commented 4 months ago

It works! Thank you so much!