ctubbsii
commented
1 year ago I don't think this is a good idea to do. Here are my reasons:
The re-logging that 1.x did was because we were tightly coupled to the log4j implementation and had no way to easily disambiguate between logs produced by the monitor and logs received by the monitor, using that old implementation. In the new implementation, these aren't even log4j objects anymore, but a serialized JSON message that is expected at the REST endpoint. Changing that implementation was necessary because of the numerous vulnerabilities the socket appender and log object serialization code that log4j previously provided in order to support serializing and sending log objects. If we were to try to convert these JSON messages back into log messages, we would either have to pretend to be the original logger in order to get them to print correctly, or we'd have to just relog them as a new logger, with a very messy message without the ability to nicely print the stack traces, or we would have to tightly couple ourselves to log4j again to use their serialization, which I'm not even sure is supported anymore, because of the aforementioned vulnerabilities. And, I'm not sure I'd want to couple ourselves to log4j code like that anyway, because of the work we've done to make the logging more extensible through slf4j.
Even if we were to go back and try to reproduce this behavior, it is not a great way to do log aggregation. In principle, the view on the monitor is for convenience. It is not a good substitute for proper log aggregation. It's not efficient, and I don't even think I'd recommend having it configured at all (for security reasons... because we are sending potentially sensitive internal logs to a web service that has no authentication), if it weren't for the fact that existing users expect it (and accept or mitigate the risks for doing so).
Doing this would result in worse results than using an appender that is actually designed to do log aggregation, like one that sends to an rsyslog service or similar. A quick Google search resulted in several tutorials on how to use log4j2 with rsyslog, or centralized log4j2 aggregation in general. Since there are already solutions available to users, which are better in many ways than what we would do, I think it's better that we defer to users to customize their logging, and we merely provide the current basic implementation, and support users through the use of customizable logging libraries (like continuing to use slf4j).
The monitor in the 1.x series would take the log messages being passed to it from the tservers etc through it own log4j. This would allows us to capture that set of messages in a single monitor log. Since the updates in the 2.x series, this is no longer the case. It is very benificial for us to capture the aggregation of tserver messages being sent to the monitor in one log. All that is required is to relog the messages in the monitor. Perhaps the logger class can be TserverMessage, GcMessage, ManagerMessage, etc. to allow us to pipe those to separate logs if we wish.