Hi Randall, Thanks for pointing this out. You're quite right about the behaviour of the LoggingResource, and I've updated the KIP with your suggested wording.
However, looking at it has made me realise that while KIP-676 means the logger levels are now hierarchical there's still an inconsistency between how levels are set in Kafka Connect and how it works in the broker. In log4j you can configure foo.baz=DEBUG and then foo=INFO and debug messages from foo.baz.Y will continue to be logged because setting the parent doesn't override all descendents (the level is inherited). As you know, in Kafka Connect, the way the log setting works is to find all the descendent loggers of foo and apply the given level to them, so setting foo.baz=DEBUG and then foo=INFO means foo.baz.Y debug messages will not appear. Obviously that behavior for Connect is explicitly stated in KIP-495, but I can't help but feel that the KIP-676 changes not addressing this is a lost opportunity. It's also worth bearing in mind that KIP-653[1] is (hopefully) going to happen for Kafka 3.0. So I wonder if perhaps the pragmatic thing to do would be to: 1. Revert the changes for KIP-676 for Kafka 2.8 2. Pass another KIP, to be implemented for Kafka 3.0, which makes all the Kafka APIs consistent in both respecting the hierarchy and also in what updating a logger level means. I don't have a particularly strong preference either way, but it seems better, from a users PoV, if all these logging changes happened in a major release and achieved consistency across components going forward. Thoughts? Kind regards, Tom [1]: https://cwiki.apache.org/confluence/display/KAFKA/KIP-653%3A+Upgrade+log4j+to+log4j2 On Thu, Jan 21, 2021 at 9:17 PM Randall Hauch <rha...@gmail.com> wrote: > Tom, et al., > > I'm really late to the party and mistakenly thought the scope of this KIP > included only the broker. But I now see in the KIP-676 [1] text the > following claim: > > Kafka exposes a number of APIs for describing and changing logger levels: > > > > * The Kafka broker exposes the DescribeConfigs RPC with the BROKER_LOGGER > > config resource. > > * Broker logger levels can also be configured using the > > Log4jControllerMBean MBean, exposed through JMX as > > kafka:type=kafka.Log4jController. > > * Kafka Connect exposes the /admin/loggers REST API endpoint for > > describing and changing logger levels. > > > > When accessing a logger's level these APIs do not respect the logger > > hierarchy. > > Instead, if the logger's level is not explicitly set the level of the > root > > logger is used, even when an intermediate logger is configured with a > > different level. > > > > Regarding Connect, the third bullet is accurate: Kafka > Connect's `/admin/loggers/` REST API introduced via KIP-495 [2] does > describe and change logger levels. > > But the first sentence after the bullets is inaccurate, because per KIP-495 > the Kafka Connect `/admin/loggers/` REST API does respect the hierarchy. In > fact, this case is explicitly called out in KIP-495: > > Setting the log level of an ancestor (for example, > > `org.apache.kafka.connect` as opposed to a classname) will update the > > levels of all child classes. > > > > and there are even unit tests in Connect for that case [3]. > > Can we modify this sentence in KIP-676 to reflect this? One way to minimize > the changes to the already-approved KIP is to change: > > > When accessing a logger's level these APIs do not respect the logger > > hierarchy. > > to > > > When accessing a logger's level the first two of these APIs do not > respect > > the logger hierarchy. > > > Thoughts? > > Randall > > [1] > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-676%3A+Respect+logging+hierarchy > [2] > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-495%3A+Dynamically+Adjust+Log+Levels+in+Connect > [3] > > https://github.com/apache/kafka/blob/trunk/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/resources/LoggingResourceTest.java#L118-L124 > > On Thu, Oct 8, 2020 at 9:56 AM Dongjin Lee <dong...@apache.org> wrote: > > > Hi Tom, > > > > I also agree that the current behavior is clearly wrong and I think it > was > > mistakenly omitted in the KIP-412 discussion process. The current > > implementation does not reflect log4j's logger hierarchy. > > > > Regards, > > Dongjin > > > > On Thu, Oct 8, 2020 at 1:27 AM John Roesler <vvcep...@apache.org> wrote: > > > > > Ah, thanks Tom, > > > > > > My only concern was that we might silently start logging a > > > lot more or less after the upgrade, but if the logging > > > behavior won't change at all, then the concern is moot. > > > > > > Since the KIP is only to make the APIs return an accurrate > > > representation of the actual log level, I have no concerns > > > at all. > > > > > > Thanks, > > > -John > > > > > > On Wed, 2020-10-07 at 17:00 +0100, Tom Bentley wrote: > > > > Hi John, > > > > > > > > You're right, but note that this affects the level the broker/connect > > > > worker was _reporting_ for that logger, not the level at which the > > logger > > > > was actually logging, which would be TRACE both before and after > > > upgrading. > > > > > > > > I've added more of an explanation to the KIP, since it wasn't very > > clear. > > > > > > > > Thanks for taking a look. > > > > > > > > Tom > > > > > > > > On Wed, Oct 7, 2020 at 4:29 PM John Roesler <vvcep...@apache.org> > > wrote: > > > > > > > > > Thanks for this KIP Tom, > > > > > > > > > > Just to clarify the impact: In your KIP you described a > > > > > situation in which the root logger is configured at INFO, an > > > > > "kafka.foo" is configured at TRACE, and then "kafka.foo.bar" > > > > > is resolved to INFO. > > > > > > > > > > Assuming this goes into 3.0, would it be the case that if I > > > > > had the above configuration, after upgrade, "kafka.foo.bar" > > > > > would just switch from INFO to TRACE on its own? > > > > > > > > > > It seems like it must, since it's not configured explicitly, > > > > > and we are changing the inheritance rule from "inherit > > > > > directly from root" to "inherit from the closest configured > > > > > ancestor in the hierarchy". > > > > > > > > > > Am I thinking about this right? > > > > > > > > > > Thanks, > > > > > -John > > > > > > > > > > On Wed, 2020-10-07 at 15:42 +0100, Tom Bentley wrote: > > > > > > Hi all, > > > > > > > > > > > > I would like to start discussion on a small KIP which seeks to > > > rectify an > > > > > > inconsistency between how Kafka reports logger levels and how > > logger > > > > > > configuration is inherited hierarchically in log4j. > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-676%3A+Respect+logging+hierarchy > > > > > > < > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-676%3A+Respect+logging+hierarchy?moved=true > > > > > > > > > > > > If you have a few minutes to have a look I'd be grateful for any > > > > > feedback. > > > > > > Many thanks, > > > > > > > > > > > > Tom > > > > > > > > > > -- > > *Dongjin Lee* > > > > *A hitchhiker in the mathematical world.* > > > > > > > > > > *github: <http://goog_969573159/>github.com/dongjinleekr > > <https://github.com/dongjinleekr>keybase: > https://keybase.io/dongjinleekr > > <https://keybase.io/dongjinleekr>linkedin: > kr.linkedin.com/in/dongjinleekr > > <https://kr.linkedin.com/in/dongjinleekr>speakerdeck: > > speakerdeck.com/dongjin > > <https://speakerdeck.com/dongjin>* > > >
