kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Randall Hauch <rha...@gmail.com>
Subject Re: [DISCUSS] KIP-495: Dynamically Adjust Log Levels in Connect
Date Tue, 17 Sep 2019 14:23:15 GMT
Looks great. Thanks again, Arjun!

On Tue, Sep 17, 2019 at 1:17 AM Arjun Satish <arjun.satish@gmail.com> wrote:

> Answers inline
>
> On Mon, Sep 16, 2019 at 5:06 PM Randall Hauch <rhauch@gmail.com> wrote:
>
> > Thanks for the updates, Arjun. If possible, it'd be great to have the KIP
> > clarify a few things:
> >
> > 1) IIUC, the loggers returned by the GET methods are only those loggers
> > that have been activated/used/set in the JVM. If this is the case, this
> > should be specified.
> >
>
> The GET methods should return all initialized loggers (ancestors and the
> ones created by runtime classes).
>
>
> >
> > 2) It's possible to set a log level on an ancestor of other loggers, so
> we
> > should also specify whether or not ancestors are included in the GET
> > responses. Doing so would be helpful, but might not be feasible since two
> > different descendants might have different log levels.
> >
>
> The ancestors are also specified in the GET responses. Updated the KIP to
> highlight this.
>
>
> >
> > Otherwise this looks good!
> >
> > Best regards,
> >
> > Randall
> >
> > On Mon, Sep 16, 2019 at 4:15 AM Arjun Satish <arjun.satish@gmail.com>
> > wrote:
> >
> > > Good catch, Randall. Yes, we should be able to set the level of any
> > logger
> > > given its name. If this is an ancestor, then the levels of all child
> > > classes are updated. I updated the KIP to be more explicit about what
> > > loggers we can set, and how they affect child classes, if any.
> > >
> > > Let me know what you think.
> > >
> > > Best,
> > >
> > > On Thu, Sep 12, 2019 at 5:02 PM Randall Hauch <rhauch@gmail.com>
> wrote:
> > >
> > > > Thanks for the KIP, Arjun. It's going to be really nice to be able to
> > set
> > > > the log levels dynamically, especially through the REST API.
> > > >
> > > > However, I think it's not clear what behavior the KIP is actually
> > > proposing
> > > > with respect to which loggers are exposed (just those that are used,
> or
> > > > common ancestors) and the behavior when I change the log level on a
> > > > specific logger (is just that logger affected, or are descendants
> > > affected,
> > > > too).
> > > >
> > > > For example, in a Log4J configuration file we can set the logger for
> > > > packages (e.g., `org.apache.kafka`, `org.apache.kafka.connect`, etc.)
> > or
> > > > classes (e.g., `org.apache.kafka.connect.runtime.WorkerSinkTask`).
> > > Really,
> > > > those are just conventions, because if the code created a logger for
> > the
> > > > "kafka.connect.worker" context then we could set that, too. So by
> > > > convention, the loggers map to Kafka classes and packages.
> > > >
> > > > But it's unclear what behavior the KIP is proposing. Are the
> > intermediate
> > > > loggers such as all packages exposed as loggers? If so, if I set the
> > > logger
> > > > on `org.apache.kafka.connect.runtime`, will this set the log level
> for
> > > all
> > > > loggers below this?
> > > >
> > > > My concern is that if the behavior is (a) only concrete classes,
> and/or
> > > (b)
> > > > setting a log level for a specific logger sets only that logger, then
> > > this
> > > > deviates from what our users are familiar with when setting the log
> > > levels
> > > > in the Log4J configuration files, and would be a difficult user
> > > experience
> > > > if I have to set 30+ loggers rather than 1 or 2.
> > > >
> > > > On Thu, Sep 12, 2019 at 1:04 PM Jason Gustafson <jason@confluent.io>
> > > > wrote:
> > > >
> > > > > Thanks Arjun. +1
> > > > >
> > > > > On Thu, Sep 12, 2019 at 9:58 AM Gwen Shapira <gwen@confluent.io>
> > > wrote:
> > > > >
> > > > > > The new REST API for logger management looks great to me.
> > > > > >
> > > > > >
> > > > > > On Thu, Sep 12, 2019 at 8:36 AM Arjun Satish <
> > arjun.satish@gmail.com
> > > >
> > > > > > 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 <
> > > arjun.satish@gmail.com>
> > > > > > 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 <
> > > jason@confluent.io
> > > > >
> > > > > > 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 <
> > > > > arjun.satish@gmail.com>
> > > > > > > >> 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 <
> > > > cmccabe@apache.org
> > > > > >
> > > > > > > >> 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
> > > > > > > >> > > > > > >> > > > >> > > > >
> > > > > > > >> > > > > > >> > > > >> > > >
> > > > > > > >> > > > > > >> > > > >> > >
> > > > > > > >> > > > > > >> > > > >> >
> > > > > > > >> > > > > > >> > > > >>
> > > > > > > >> > > > > > >> > > > >
> > > > > > > >> > > > > > >> > > >
> > > > > > > >> > > > > > >> > >
> > > > > > > >> > > > > > >> >
> > > > > > > >> > > > > > >>
> > > > > > > >> > > > > > >
> > > > > > > >> > > > > >
> > > > > > > >> > > > >
> > > > > > > >> > > >
> > > > > > > >> > >
> > > > > > > >> >
> > > > > > > >>
> > > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message