[jira] [Commented] (SAMZA-350) Allow dynamic log level toggling

["Chris Riccomini (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SAMZA-350?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14079034#comment-14079034 ] 

Chris Riccomini commented on SAMZA-350:
---------------------------------------

One thing that bugs me about the TaskLifecycleListener approach is that it has to be turned on. The two things that I'd like to accomplish are:

# I'd rather have my Samza job automatically turn on the Log4J JMX MBean if I'm using log4j.
# I don't want to introduce log4j as a dependency in samza-core if we can avoid it.

For (2), I know a lot of folks use Log4J, and it's pretty benign as pervasive low-level libraries go, but for hygiene purposes I'd at least like to *try* and do the right thing.

Another potential implementation for this would be to use Java's built-in ServiceLoader. We could define a SamzaService API with start(), and stop() methods. We could then write a Log4JService that sets up a JMX MBean that would allow us to get/set the log level. Both the SamzaContainer and the SamzaAppMaster would have to start and stop all SamzaServices. This implementation would allow us to keep log4j dependencies out of samza-core, while still allowing us to automatically have the Log4J JMX MBean available to us simply by putting the samza-log4j jar on the classpath.

This service API could potentially be useful for other things as well. Some things that could be converted over to SamzaServices are JmxServer, and JvmMetrics, both of which we hard code to start right now.

One trade-off here is that this introduces a whole new way to do wiring with a Samza job. Whereas we've traditionally just done config-based wiring with Class.forName, we now have this other odd-ball way to wire up SamzaServices using ServiceLoader, where we're essentially magically traversing the package space looking for classes to instantiate automagically.

> Allow dynamic log level toggling
> --------------------------------
>
>                 Key: SAMZA-350
>                 URL: https://issues.apache.org/jira/browse/SAMZA-350
>             Project: Samza
>          Issue Type: Bug
>          Components: container
>    Affects Versions: 0.7.0
>            Reporter: Chris Riccomini
>
> Samza uses SLF4J for logging. We do not expose any mechanism to dynamically change the log level at runtime. I'm not even sure if SLF4J can do this because the log level settings are usually implementation-specific.
> Log4J does allow this via the setLevel API. By default in Log4J 2, JMX is already enabled. The hello-samza app shows how to use Log4J 1.2.*, and we use Log4J 1.2.* at LinkedIn.
> We should figure out how to implement this. Ideally, we'd do it at the SLF4J level so we don't break the log-implementation-independence that we get with SLF4J. Unfortunately, it doesn't seem that SLF4J supports this. This leaves us with log-implementation-specific support.
> I propose in this ticket that we just write Log4J 1.2.* specific implementation that calls:
> {code}
> Logger.getRootLogger().setLevel(...);
> {code}
> The second question is how to implement the RPC. There are many ways. We could use the ConfigLog (SAMZA-348), some other control stream, an HTTP endpoint in the containers, or JMX.
> My preference is to use JMX for this. It's nice and simple. The ConfigLog approach seems appealing, but it would either require containers to restart (not ideal if you're trying to debug an issue as it's happening) or would require all containers to listen to the ConfigLog topic (seems a big commitment if we want to keep containers light-weight).
> The third question is where this code should live. This code should be run in all containers and the YARN AM. I am favor having this in its own module (samza-log4j) just so we don't have to introduce a Log4J dependency to samza-core, which we've thus far been able to avoid. A reasonable location for this seems to be in a TaskLifecycleListener, which could setup the MBean in the beforeInit method. Unfortunately, the YARN AM does not use TaskLifecycleListener. We could write a SamzaAppMasterLifecycleListener interface, and implement the Log4J MBean for both.
> There might also be a better place for this to live. An argument could be made that this should be part of Samza-proper, and that we should just switch to Log4J. Another argument could be made that we need a better lifecycle interface, or some generic plugin thingy to let us run this kind of logic.



--
This message was sent by Atlassian JIRA
(v6.2#6252)