From dev-return-107449-archive-asf-public=cust-asf.ponee.io@kafka.apache.org Thu Sep 12 18:04:56 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id 840F4180651 for ; Thu, 12 Sep 2019 20:04:55 +0200 (CEST) Received: (qmail 8964 invoked by uid 500); 12 Sep 2019 18:04:54 -0000 Mailing-List: contact dev-help@kafka.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@kafka.apache.org Delivered-To: mailing list dev@kafka.apache.org Received: (qmail 8952 invoked by uid 99); 12 Sep 2019 18:04:53 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd2-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 12 Sep 2019 18:04:53 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd2-us-west.apache.org (ASF Mail Server at spamd2-us-west.apache.org) with ESMTP id 17A091A323A for ; Thu, 12 Sep 2019 18:04:53 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd2-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 1.803 X-Spam-Level: * X-Spam-Status: No, score=1.803 tagged_above=-999 required=6.31 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=2, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=disabled Authentication-Results: spamd2-us-west.apache.org (amavisd-new); dkim=pass (2048-bit key) header.d=confluent.io Received: from mx1-ec2-va.apache.org ([10.40.0.8]) by localhost (spamd2-us-west.apache.org [10.40.0.9]) (amavisd-new, port 10024) with ESMTP id zgD8vA2mRedj for ; Thu, 12 Sep 2019 18:04:47 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=209.85.210.66; helo=mail-ot1-f66.google.com; envelope-from=jason@confluent.io; receiver= Received: from mail-ot1-f66.google.com (mail-ot1-f66.google.com [209.85.210.66]) by mx1-ec2-va.apache.org (ASF Mail Server at mx1-ec2-va.apache.org) with ESMTPS id 87522BC509 for ; Thu, 12 Sep 2019 18:04:47 +0000 (UTC) Received: by mail-ot1-f66.google.com with SMTP id z26so17693513oto.1 for ; Thu, 12 Sep 2019 11:04:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=confluent.io; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=vzUvTNpoTl8UlvePYhdm+pFu92EbVGRKHvRbuc7dlyo=; b=Fyq2EQKqrvswBZKT3i1Pcd/yxUgMx5Gd8QN13WUQVjH3vj8r/nJ6u8dwO+efiTfCsE A44yf3L7/AwKDUEQksx4OIjG+6F2hNR8p4KKerGH5YPyL5Mkrxh8OtxnmDv4a69knlld sDYZILOh5VWp5ywZD4hyQguIfo2TOsqGuVbASagkwIy5m6EgR7jAjZJB/6ElpRcX4Orr k6fucGi0GLQ9ydRuzjEtfiDfu1PGC8xbj89Ek374AP0c7uIyDOc8orbb9pVaQiV7c2D1 8ARDti9RwWEbSblAtIKYrF2U6Y4Z5s2Nt8hATxYfIxjGlN1OvBuFNoKwOPGV3TR+KyWR 04YQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=vzUvTNpoTl8UlvePYhdm+pFu92EbVGRKHvRbuc7dlyo=; b=kTFvO1hndYf3xDT4i60XS2PtepxlNKv3ekTlOF5Rf6Y/OMprvNrnRoEtaw+mg9AO9o vVnnrO1TaveRo+C15fBNd0XEWpPXe8Wz10vUT+yiBBhwLhlLj6lp8qv7GxzRBgWrwLlN Cej/8+cFQjZYHxYHaRN8elTuVkLEh7mQ4fHWHIl02e6k2nKw944t1VgRQP6q9wPWa9de 9biD06yQVBkzuLY+sDSNNvWJvuMR6ZTDiuT938QvAkol19qeNz+eFvplzYzw5JXU9uLa 3tHvj3KRsp/tQae6rpX5n9Z7H908ytLTq54ELvD0f63Xt0QT39L/GJbKAcYWGzR935By HWiQ== X-Gm-Message-State: APjAAAVPYAmGtMqQ1gZAKegv6Y8PViQeNrDo+yLVCNSgQNVEtVtviW+j +Cb1H4w0GqLQar7A4iEMJ8LPtqOnFXjmwC8cMvP+W8DS X-Google-Smtp-Source: APXvYqw+eJKdajRANTiWggazv1fnMP0gMnjqjv5rnQj2ewAkz9T5RVWFnMqA7JdA7Rb0iAVSHb42V0f8n+R3pCVB2Jw= X-Received: by 2002:a05:6830:124f:: with SMTP id s15mr27951441otp.95.1568311485639; Thu, 12 Sep 2019 11:04:45 -0700 (PDT) MIME-Version: 1.0 References: <026a34fe-bd43-4811-8097-d4d3524f5501@www.fastmail.com> In-Reply-To: From: Jason Gustafson Date: Thu, 12 Sep 2019 11:04:34 -0700 Message-ID: Subject: Re: [DISCUSS] KIP-495: Dynamically Adjust Log Levels in Connect To: dev Content-Type: multipart/alternative; boundary="000000000000ffd4c205925ef814" --000000000000ffd4c205925ef814 Content-Type: text/plain; charset="UTF-8" Thanks Arjun. +1 On Thu, Sep 12, 2019 at 9:58 AM Gwen Shapira wrote: > The new REST API for logger management looks great to me. > > > On Thu, Sep 12, 2019 at 8:36 AM Arjun Satish > wrote: > > > > Bumping this thread. > > > > If there are no further comments, please add your votes here: > > https://www.mail-archive.com/dev@kafka.apache.org/msg100313.html > > > > Thanks in advance, > > Arjun > > > > On Fri, Sep 6, 2019 at 4:22 PM Arjun Satish > wrote: > > > > > Thanks a lot, Jason! Answers inline. I'll also modify the kip to make > > > these clear. > > > > > > On Fri, Sep 6, 2019 at 4:01 PM Jason Gustafson > wrote: > > > > > >> Hi Arjun, > > >> > > >> The updated KIP looks good. Just a couple questions: > > >> > > >> 1. Is the /admin endpoint on the normal listener by default? If not, > is > > >> there a way to have it use the same listener? > > >> > > > > > > Uses the normal listener by default. > > > > > > > > >> 2. Changes to logging configuration are not intended to be > persistent, is > > >> that right? Also, I assume changes only apply to the worker that > received > > >> the request? > > >> > > > > > > Changes will not be persistent and only apply to the worker that > received > > > the request. > > > > > > > > >> Thanks, > > >> Jason > > >> > > >> On Fri, Aug 30, 2019 at 1:25 AM Arjun Satish > > >> wrote: > > >> > > >> > OK. I didn't realize the plan was to deprecate and remove the JMX > > >> endpoint. > > >> > KIP-412 says brokers will continue to expose the JMX API. JMX was > > >> selected > > >> > so all components could follow the brokers. In light of this, I > think we > > >> > should simply aim for semantic equivalency across the different API > for > > >> > this functionality. > > >> > > > >> > REST is convenient for Connect. We can modify the KIP to have a > /admin > > >> > endpoint, and /admin/loggers specifically for getting/setting the > log > > >> > levels of different loggers. The /admin/loggers will only impact > loggers > > >> > running in the specific worker we target requests to, and upon > > >> restarting > > >> > the worker, these loggers will reset back to their original level. > > >> > > > >> > Since configuring the rest server already has multiple config keys, > I am > > >> > inclined to bundle this /admin endpoint on to the same listener, and > > >> > provide a single new config key that enables or disables the entire > > >> /admin > > >> > endpoint. This keeps the initial approach simple and doesn't require > > >> users > > >> > to configure/discover a new endpoint. > > >> > > > >> > If this works with you all, I can update the KIP. Please let me know > > >> what > > >> > you think. > > >> > > > >> > Thanks everyone. > > >> > > > >> > Best, > > >> > > > >> > On Thu, Aug 29, 2019 at 10:14 AM Colin McCabe > > >> wrote: > > >> > > > >> > > On Mon, Aug 26, 2019, at 14:03, Jason Gustafson wrote: > > >> > > > Hi Arjun, > > >> > > > > > >> > > > From a high level, I feel like we are making light of the JMX > api > > >> > because > > >> > > > it's convenient and the broker already has it. Personally I > would > > >> take > > >> > > the > > >> > > > broker out of the picture. The JMX endpoint is not something we > were > > >> > > happy > > >> > > > with, hence KIP-412. Ultimately I think we will deprecate and > > >> remove it > > >> > > and > > >> > > > there's no point trying to standardize on a deprecated > mechanism. > > >> > > Thinking > > >> > > > just about connect, we already have an HTTP endpoint. The > default > > >> > > position > > >> > > > should be to add new APIs to it rather than introducing other > > >> > mechanisms. > > >> > > > The fewer ways you have to interact with a system, the better, > > >> right? > > >> > > > > > >> > > > I think the main argument against a REST endpoint is basically > that > > >> > > > adjusting log levels is an administrative operation and connect > is > > >> > > lacking > > >> > > > an authorization framework to enforce administrative access. The > > >> same > > >> > > > argument applies to JMX, but it has the benefit that you can > specify > > >> > > > different credentials and it is easier to isolate since it is > > >> running > > >> > on > > >> > > a > > >> > > > separate port. As you suggested, I think the same benefits > could be > > >> > > > achieved by having a separate /admin endpoint which is exposed > > >> (perhaps > > >> > > > optionally) on another listener. This is a pretty standard > pattern. > > >> If > > >> > > > memory serves, dropwizard has something like this out of the > box. We > > >> > > should > > >> > > > think hard whether there are additional administrative > capabilities > > >> > that > > >> > > we > > >> > > > would ultimately need. The answer is probably yes, so unless we > > >> want to > > >> > > > double down on JMX, it might be worth thinking through the > > >> implications > > >> > > of > > >> > > > an admin endpoint now so that we're not left with odd > compatibility > > >> > > baggage > > >> > > > in the future. > > >> > > > > >> > > Hi Jason, > > >> > > > > >> > > I agree... I think Connect needs a REST admin API. There will > > >> probably > > >> > be > > >> > > a lot of other stuff that we'll want to add to it. > > >> > > > > >> > > best, > > >> > > Colin > > >> > > > > >> > > > > > >> > > > > >> > > > Thanks, > > >> > > > Jason > > >> > > > > > >> > > > > > >> > > > > > >> > > > > > >> > > > On Fri, Aug 23, 2019 at 5:38 PM Arjun Satish < > > >> arjun.satish@gmail.com> > > >> > > wrote: > > >> > > > > > >> > > > > Jason, > > >> > > > > > > >> > > > > Thanks for your comments! > > >> > > > > > > >> > > > > I understand the usability issues with JMX that you mention. > But > > >> it > > >> > was > > >> > > > > chosen for the following reasons: > > >> > > > > > > >> > > > > 1. Cross-cutting functionality across different components > (Kafka > > >> > > brokers, > > >> > > > > Connect workers and even with Streams jobs). If we go down the > > >> REST > > >> > > route, > > >> > > > > then brokers don't get this feature. > > >> > > > > 2. Adding this to existing REST servers adds the > whole-or-nothing > > >> > > problem. > > >> > > > > It's hard to disable an endpoint if the functionality is not > > >> desired > > >> > or > > >> > > > > needs to be protected from users (Connect doesn't have ACLs > which > > >> > makes > > >> > > > > this even harder to manage). Adding endpoints to different > > >> listeners > > >> > > makes > > >> > > > > configuring Connect harder (and it's already a hard problem > as it > > >> > is). > > >> > > A > > >> > > > > lot of the existing functionality there is driven around the > > >> > connector > > >> > > data > > >> > > > > model (connectors, plugins, their statuses and so on). Adding > an > > >> > > '/admin' > > >> > > > > endpoint may be a way to go, but that has tremendous > implications > > >> (we > > >> > > are > > >> > > > > effectively adding an administration endpoint similar to the > admin > > >> > one > > >> > > in > > >> > > > > brokers), and probably requires a KIP of its own with > discussions > > >> > > catered > > >> > > > > around just that. > > >> > > > > 3. JMX is currently AK's default way to report metrics and > perform > > >> > > other > > >> > > > > operations. Changing log levels is typically a system > level/admin > > >> > > > > operation, and fits better there, instead of REST APIs (which > is > > >> more > > >> > > user > > >> > > > > facing). > > >> > > > > > > >> > > > > Having said that, I'm happy to consider alternatives. JMX > seemed > > >> to > > >> > be > > >> > > the > > >> > > > > lowest hanging fruit. But if there are better ideas, we can > > >> consider > > >> > > them. > > >> > > > > At the end of the day, when we download and run Kafka, there > > >> should > > >> > be > > >> > > one > > >> > > > > way to achieve the same functionality among its components. > > >> > > > > > > >> > > > > Finally, I hope I didn't convey that we are > reverting/changing the > > >> > > changes > > >> > > > > made in KIP-412. The proposed changes would be an addition to > it. > > >> It > > >> > > will > > >> > > > > give brokers multiple ways of changing log levels. and there > is > > >> > still a > > >> > > > > consistent way of achieving cross component goals of the KIP. > > >> > > > > > > >> > > > > Best, > > >> > > > > > > >> > > > > > > >> > > > > On Fri, Aug 23, 2019 at 4:12 PM Jason Gustafson < > > >> jason@confluent.io> > > >> > > > > wrote: > > >> > > > > > > >> > > > > > Let me elaborate a little bit. We made the decision early > on for > > >> > > Connect > > >> > > > > to > > >> > > > > > use HTTP instead of Kafka's custom RPC protocol. In > exchange for > > >> > > losing > > >> > > > > > some hygienic consistency with Kafka, we took easier > integration > > >> > with > > >> > > > > > management tools. The scope of the connect REST APIs is > really > > >> > > managing > > >> > > > > the > > >> > > > > > connect cluster. It has endpoints for creating connectors, > > >> changing > > >> > > > > > configs, seeing their health, etc. Doesn't debugging fit in > with > > >> > > that? I > > >> > > > > am > > >> > > > > > not sure I see why we would treat this as an exceptional > case. > > >> > > > > > > > >> > > > > > I personally see JMX as a necessary evil in Kafka because > most > > >> > > metrics > > >> > > > > > agents have native support. But it is particularly painful > when > > >> it > > >> > > comes > > >> > > > > to > > >> > > > > > use as an RPC mechanism. This was the central motivation > behind > > >> > > KIP-412, > > >> > > > > > which makes it very odd to see a new proposal which suggests > > >> > > > > standardizing > > >> > > > > > on JMX for log level adjustment. I actually see this as > > >> something > > >> > > we'd > > >> > > > > want > > >> > > > > > to eventually turn off in Kafka. Now that we have a proper > API > > >> with > > >> > > > > support > > >> > > > > > in the AdminClient, we can deprecate and eventually remove > the > > >> JMX > > >> > > > > > endpoint. > > >> > > > > > > > >> > > > > > Thanks, > > >> > > > > > Jason > > >> > > > > > > > >> > > > > > On Fri, Aug 23, 2019 at 10:49 AM Jason Gustafson < > > >> > jason@confluent.io > > >> > > > > > >> > > > > > wrote: > > >> > > > > > > > >> > > > > > > Hi Arjun, > > >> > > > > > > > > >> > > > > > > Thanks for the KIP. Do we really need a JMX-based API? Is > > >> there > > >> > > > > literally > > >> > > > > > > anyone in the world that wants to use JMX if they don't > have > > >> to? > > >> > I > > >> > > > > > thought > > >> > > > > > > one of the major motivations of KIP-412 was how much of a > pain > > >> > JMX > > >> > > is. > > >> > > > > > > > > >> > > > > > > Thanks, > > >> > > > > > > Jason > > >> > > > > > > > > >> > > > > > > On Mon, Aug 19, 2019 at 5:28 PM Arjun Satish < > > >> > > arjun.satish@gmail.com> > > >> > > > > > > wrote: > > >> > > > > > > > > >> > > > > > >> Thanks, Konstantine. > > >> > > > > > >> > > >> > > > > > >> Updated the KIP with the restrictions around log4j and > added > > >> > > > > references > > >> > > > > > to > > >> > > > > > >> similar KIPs. > > >> > > > > > >> > > >> > > > > > >> Best, > > >> > > > > > >> > > >> > > > > > >> On Mon, Aug 19, 2019 at 3:20 PM Konstantine Karantasis < > > >> > > > > > >> konstantine@confluent.io> wrote: > > >> > > > > > >> > > >> > > > > > >> > Thanks Arjun, the example is useful! > > >> > > > > > >> > > > >> > > > > > >> > My point when I mentioned the restrictions around > log4j is > > >> > that > > >> > > this > > >> > > > > > is > > >> > > > > > >> > information is significant and IMO needs to be > included in > > >> the > > >> > > KIP. > > >> > > > > > >> > > > >> > > > > > >> > Speaking of its relevance to KIP-412, I think a > reference > > >> > would > > >> > > be > > >> > > > > > nice > > >> > > > > > >> > too. > > >> > > > > > >> > > > >> > > > > > >> > Konstantine > > >> > > > > > >> > > > >> > > > > > >> > > > >> > > > > > >> > > > >> > > > > > >> > On Thu, Aug 15, 2019 at 4:00 PM Arjun Satish < > > >> > > > > arjun.satish@gmail.com> > > >> > > > > > >> > wrote: > > >> > > > > > >> > > > >> > > > > > >> > > Hey Konstantine, > > >> > > > > > >> > > > > >> > > > > > >> > > Thanks for the feedback. > > >> > > > > > >> > > > > >> > > > > > >> > > re: the use of log4j, yes, the proposed changes will > only > > >> > > work if > > >> > > > > > >> log4j > > >> > > > > > >> > is > > >> > > > > > >> > > available in runtime. We will not add the mBean if > log4j > > >> is > > >> > > not > > >> > > > > > >> available > > >> > > > > > >> > > in classpath. If we change from log4j 1 to 2, that > would > > >> > > involve > > >> > > > > > >> another > > >> > > > > > >> > > KIP, and it would need to update the changes > proposed in > > >> > this > > >> > > KIP > > >> > > > > > and > > >> > > > > > >> > > others (KIP-412, for instance). > > >> > > > > > >> > > > > >> > > > > > >> > > re: use of Object types, I've changed it from > Boolean to > > >> the > > >> > > > > > primitive > > >> > > > > > >> > type > > >> > > > > > >> > > for setLogLevel. We are changing the signature of > the old > > >> > > method > > >> > > > > > this > > >> > > > > > >> > way, > > >> > > > > > >> > > but since it never returned null, this should be > fine. > > >> > > > > > >> > > > > >> > > > > > >> > > re: example usage, I've added some screenshot on how > this > > >> > > feature > > >> > > > > > >> would > > >> > > > > > >> > be > > >> > > > > > >> > > used with jconsole. > > >> > > > > > >> > > > > >> > > > > > >> > > Hope this works! > > >> > > > > > >> > > > > >> > > > > > >> > > Thanks very much, > > >> > > > > > >> > > Arjun > > >> > > > > > >> > > > > >> > > > > > >> > > On Wed, Aug 14, 2019 at 6:42 AM Konstantine > Karantasis < > > >> > > > > > >> > > konstantine@confluent.io> wrote: > > >> > > > > > >> > > > > >> > > > > > >> > > > And one thing I forgot is also related to Chris's > > >> comment > > >> > > > > above. I > > >> > > > > > >> > agree > > >> > > > > > >> > > > that an example on how a user is expected to set > the > > >> log > > >> > > level > > >> > > > > > (for > > >> > > > > > >> > > > instance to DEBUG) would be nice, even if it's > showing > > >> > only > > >> > > one > > >> > > > > > out > > >> > > > > > >> of > > >> > > > > > >> > > the > > >> > > > > > >> > > > many possible ways to achieve that. > > >> > > > > > >> > > > > > >> > > > > > >> > > > - Konstantine > > >> > > > > > >> > > > > > >> > > > > > >> > > > On Wed, Aug 14, 2019 at 4:38 PM Konstantine > Karantasis > > >> < > > >> > > > > > >> > > > konstantine@confluent.io> wrote: > > >> > > > > > >> > > > > > >> > > > > > >> > > > > > > >> > > > > > >> > > > > Thanks Arjun for tackling the need to support > this > > >> very > > >> > > useful > > >> > > > > > >> > feature. > > >> > > > > > >> > > > > > > >> > > > > > >> > > > > One thing I noticed while reading the KIP is > that I > > >> > would > > >> > > have > > >> > > > > > >> loved > > >> > > > > > >> > to > > >> > > > > > >> > > > > see more info regarding how this proposal > depends on > > >> the > > >> > > > > > >> underlying > > >> > > > > > >> > > > logging > > >> > > > > > >> > > > > APIs and implementations. For instance, my > > >> understanding > > >> > > is > > >> > > > > that > > >> > > > > > >> > slf4j > > >> > > > > > >> > > > can > > >> > > > > > >> > > > > not be leveraged and that the logging framework > > >> needs to > > >> > > be > > >> > > > > > >> pegged to > > >> > > > > > >> > > > log4j > > >> > > > > > >> > > > > explicitly (or another logging implementation). > > >> Correct > > >> > > me if > > >> > > > > > I'm > > >> > > > > > >> > > wrong, > > >> > > > > > >> > > > > but if such a dependency is introduced I believe > it's > > >> > > worth > > >> > > > > > >> > mentioning. > > >> > > > > > >> > > > > > > >> > > > > > >> > > > > Additionally, if the above is correct, there are > > >> > > differences > > >> > > > > in > > >> > > > > > >> > log4j's > > >> > > > > > >> > > > > APIs between version 1 and version 2. In version > 2, > > >> > > > > > >> Logger#setLevel > > >> > > > > > >> > > > method > > >> > > > > > >> > > > > has been removed from the Logger interface and in > > >> order > > >> > > to set > > >> > > > > > the > > >> > > > > > >> > log > > >> > > > > > >> > > > > level programmatically the Configurator class > needs > > >> to > > >> > > used, > > >> > > > > > >> which as > > >> > > > > > >> > > > > stated in the FAQ ( > > >> > > > > > >> > > > > > > >> > > > > > >> > > > >> > > > > > > > >> > > > > >> > https://logging.apache.org/log4j/2.x/faq.html#reconfig_level_from_code > > >> > > > > > >> > > ) > > >> > > > > > >> > > > > it's not part of log4j2's public API. Is this a > > >> > concern? I > > >> > > > > > believe > > >> > > > > > >> > that > > >> > > > > > >> > > > > even if these are implementation specific > details for > > >> > the > > >> > > > > > wrappers > > >> > > > > > >> > > > > introduced by this KIP (which to a certain extent > > >> they > > >> > > are), a > > >> > > > > > >> > mention > > >> > > > > > >> > > in > > >> > > > > > >> > > > > the KIP text and a few references would be > useful to > > >> > > > > understand > > >> > > > > > >> the > > >> > > > > > >> > > > changes > > >> > > > > > >> > > > > and the dependencies introduced by this proposal. > > >> > > > > > >> > > > > > > >> > > > > > >> > > > > And a few minor comments: > > >> > > > > > >> > > > > - Is there any specific reason that object types > were > > >> > > > > preferred > > >> > > > > > in > > >> > > > > > >> > the > > >> > > > > > >> > > > > proposed interface compared to primitive types? > My > > >> > > > > understanding > > >> > > > > > >> is > > >> > > > > > >> > > that > > >> > > > > > >> > > > > `null` is not expected as a return value. > > >> > > > > > >> > > > > - Related to the above, I think it'd be nice for > the > > >> > > javadoc > > >> > > > > to > > >> > > > > > >> > mention > > >> > > > > > >> > > > > when a parameter is not expected to be `null` > with an > > >> > > > > > appropriate > > >> > > > > > >> > > comment > > >> > > > > > >> > > > > (e.g. foo bar etc; may not be null) > > >> > > > > > >> > > > > > > >> > > > > > >> > > > > Cheers, > > >> > > > > > >> > > > > Konstantine > > >> > > > > > >> > > > > > > >> > > > > > >> > > > > On Tue, Aug 6, 2019 at 9:34 AM Cyrus Vafadari < > > >> > > > > > cyrus@confluent.io > > >> > > > > > >> > > > >> > > > > > >> > > > wrote: > > >> > > > > > >> > > > > > > >> > > > > > >> > > > >> This looks like a useful feature, the strategy > makes > > >> > > sense, > > >> > > > > and > > >> > > > > > >> the > > >> > > > > > >> > > KIP > > >> > > > > > >> > > > is > > >> > > > > > >> > > > >> thorough and nicely written. Thanks! > > >> > > > > > >> > > > >> > > >> > > > > > >> > > > >> Cyrus > > >> > > > > > >> > > > >> > > >> > > > > > >> > > > >> On Thu, Aug 1, 2019, 12:40 PM Chris Egerton < > > >> > > > > > chrise@confluent.io > > >> > > > > > >> > > > >> > > > > > >> > > > wrote: > > >> > > > > > >> > > > >> > > >> > > > > > >> > > > >> > Thanks Arjun! Looks good to me. > > >> > > > > > >> > > > >> > > > >> > > > > > >> > > > >> > On Thu, Aug 1, 2019 at 12:33 PM Arjun Satish < > > >> > > > > > >> > > arjun.satish@gmail.com> > > >> > > > > > >> > > > >> > wrote: > > >> > > > > > >> > > > >> > > > >> > > > > > >> > > > >> > > Thanks for the feedback, Chris! > > >> > > > > > >> > > > >> > > > > >> > > > > > >> > > > >> > > Yes, the example is pretty much how Connect > will > > >> > use > > >> > > the > > >> > > > > > new > > >> > > > > > >> > > > feature. > > >> > > > > > >> > > > >> > > Tweaked the section to make this more clear. > > >> > > > > > >> > > > >> > > > > >> > > > > > >> > > > >> > > Best, > > >> > > > > > >> > > > >> > > > > >> > > > > > >> > > > >> > > On Fri, Jul 26, 2019 at 11:52 AM Chris > Egerton < > > >> > > > > > >> > > chrise@confluent.io > > >> > > > > > >> > > > > > > >> > > > > > >> > > > >> > > wrote: > > >> > > > > > >> > > > >> > > > > >> > > > > > >> > > > >> > > > Hi Arjun, > > >> > > > > > >> > > > >> > > > > > >> > > > > > >> > > > >> > > > This looks great. The changes to public > > >> interface > > >> > > are > > >> > > > > > >> pretty > > >> > > > > > >> > > small > > >> > > > > > >> > > > >> and > > >> > > > > > >> > > > >> > > > moving the Log4jController class into the > > >> clients > > >> > > > > package > > >> > > > > > >> > seems > > >> > > > > > >> > > > like > > >> > > > > > >> > > > >> > the > > >> > > > > > >> > > > >> > > > right way to go. One question I have--it > looks > > >> > > like the > > >> > > > > > >> > purpose > > >> > > > > > >> > > of > > >> > > > > > >> > > > >> this > > >> > > > > > >> > > > >> > > KIP > > >> > > > > > >> > > > >> > > > is to enable dynamic setting of log > levels in > > >> the > > >> > > > > Connect > > >> > > > > > >> > > > framework, > > >> > > > > > >> > > > >> > but > > >> > > > > > >> > > > >> > > > it's not clear how the Connect framework > will > > >> use > > >> > > that > > >> > > > > > new > > >> > > > > > >> > > > utility. > > >> > > > > > >> > > > >> Is > > >> > > > > > >> > > > >> > > the > > >> > > > > > >> > > > >> > > > "Example Usage" section (which involves > > >> invoking > > >> > > the > > >> > > > > > >> utility > > >> > > > > > >> > > with > > >> > > > > > >> > > > a > > >> > > > > > >> > > > >> > > > namespace of "kafka.connect") actually > meant > > >> to > > >> > be > > >> > > part > > >> > > > > > of > > >> > > > > > >> the > > >> > > > > > >> > > > >> proposed > > >> > > > > > >> > > > >> > > > changes to public interface? > > >> > > > > > >> > > > >> > > > > > >> > > > > > >> > > > >> > > > Cheers, > > >> > > > > > >> > > > >> > > > > > >> > > > > > >> > > > >> > > > Chris > > >> > > > > > >> > > > >> > > > > > >> > > > > > >> > > > >> > > > On Mon, Jul 22, 2019 at 11:03 PM Arjun > Satish > > >> < > > >> > > > > > >> > > > >> arjun.satish@gmail.com> > > >> > > > > > >> > > > >> > > > wrote: > > >> > > > > > >> > > > >> > > > > > >> > > > > > >> > > > >> > > > > Hi everyone. > > >> > > > > > >> > > > >> > > > > > > >> > > > > > >> > > > >> > > > > I'd like to propose the following KIP to > > >> > > implement > > >> > > > > > >> changing > > >> > > > > > >> > > log > > >> > > > > > >> > > > >> > levels > > >> > > > > > >> > > > >> > > on > > >> > > > > > >> > > > >> > > > > the fly in Connect workers: > > >> > > > > > >> > > > >> > > > > > > >> > > > > > >> > > > >> > > > > > > >> > > > > > >> > > > >> > > > > > > >> > > > > > >> > > > >> > > > > > >> > > > > > >> > > > >> > > > > >> > > > > > >> > > > >> > > > >> > > > > > >> > > > >> > > >> > > > > > >> > > > > > >> > > > > > >> > > > > >> > > > > > >> > > > >> > > > > > >> > > >> > > > > > > > >> > > > > > > >> > > > > >> > > > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-495%3A+Dynamically+Adjust+Log+Levels+in+Connect > > >> > > > > > >> > > > >> > > > > > > >> > > > > > >> > > > >> > > > > Would like to hear your thoughts on > this. > > >> > > > > > >> > > > >> > > > > > > >> > > > > > >> > > > >> > > > > Thanks very much, > > >> > > > > > >> > > > >> > > > > Arjun > > >> > > > > > >> > > > >> > > > > > > >> > > > > > >> > > > >> > > > > > >> > > > > > >> > > > >> > > > > >> > > > > > >> > > > >> > > > >> > > > > > >> > > > >> > > >> > > > > > >> > > > > > > >> > > > > > >> > > > > > >> > > > > > >> > > > > >> > > > > > >> > > > >> > > > > > >> > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > > > --000000000000ffd4c205925ef814--