On Mon, Sep 25, 2017 at 3:35 AM, Mickael Maison <mickael.mai...@gmail.com> wrote:
> Hi Ewen, > > I understand your point of view and ideally we'd have one convention > for handling all user provided strings. This KIP reused the > sanitization mechanism we had in place for user principals. > > I think both ways have pros and cons but what I like about early > sanitization (as is currently) is that it's consistent. While > monitoring tools have to know about the sanitization, all metrics are > sanitized the same way before being passed to reporters and that > includes all "fields" in the metric name (client-id, user). > How is just passing the actual values to the reporters any less consistent? The "sanitization" process that was in place was really only for internal purposes, right? i.e. so that we'd have a path for ZK that ZK could handle? I think the real question is why JmxReporter is being special-cased? > > Moving the sanitization into JMXReporter is a publicly visible change > as it would affect the metrics we pass to other reporters. > How would this be any more publicly visible than the change already being made? In fact, since we haven't really specified what reporters should accept, if anything the change to the sanitized strings is more of a publicly visible change (you need to understand exactly what transformation is being applied) than the change I am suggesting (which could be considered a bug fix that now just fixes support for certain client IDs and only affects JMX metric names because of JMX limitations). -Ewen > > > > > On Fri, Sep 22, 2017 at 8:53 PM, Ewen Cheslack-Postava > <e...@confluent.io> wrote: > > Hi all, > > > > In working on the patch for KIP-196: Add metrics to Kafka Connect > > framework, we realized that we have worker and connector/task IDs that > are > > to be included in metrics and those don't currently have constraints on > > naming. I'd prefer to avoid adding naming restrictions or mangling names > > unnecessarily, and for users that define a custom metrics reporter the > name > > mangling may be unexpected since their metrics system may not have the > same > > limitations as JMX. > > > > The text of the KIP is pretty JMX specific and doesn't really define > where > > this mangling happens. Currently, it is being applied essentially as > early > > as possible. I would like to propose moving the name mangling into the > > JmxReporter itself so the only impact is on JMX metrics, but other > metrics > > reporters would see the original. In other words, leave system-specific > > name mangling up to that system. > > > > In the JmxReporter, the mangling could remain the same (though I think > the > > mangling for principals is an internal implementation detail, whereas > this > > name mangling is user-visible). The quota metrics on the broker would now > > be inconsistent with the others, but I think trying to be less > JMX-specific > > given that we support pluggable reporters is the right direction to go. > > > > I think in practice this has no publicly visible impact and definitely no > > compatibility concerns, it just moves where we're doing the JMX name > > mangling. However, since discussion about metric naming/character > > substitutions had happened here recently I wanted to raise it here and > make > > sure there would be agreement on this direction. > > > > (Long term I'd also like to get the required instantiation of JmxReporter > > removed as well, but that requires its own KIP.) > > > > Thanks, > > Ewen > > > > On Thu, Sep 14, 2017 at 2:09 AM, Tom Bentley <t.j.bent...@gmail.com> > wrote: > > > >> Hi Mickael, > >> > >> I was just wondering why the restriction was imposed for Java clients > the > >> first place, do you know? > >> > >> Cheers, > >> > >> Tom > >> > >> On 14 September 2017 at 09:16, Ismael Juma <ism...@juma.me.uk> wrote: > >> > >> > Thanks for the KIP Mickael. I suggest starting a vote. > >> > > >> > Ismael > >> > > >> > On Mon, Aug 21, 2017 at 2:51 PM, Mickael Maison < > >> mickael.mai...@gmail.com> > >> > wrote: > >> > > >> > > Hi all, > >> > > > >> > > I have created a KIP to cleanup the way client-ids are handled by > >> > > brokers and clients. > >> > > > >> > > Currently the Java clients have some restrictions on the client-ids > >> > > that are not enforced by the brokers. Using 3rd party clients, > >> > > client-ids containing any characters can be used causing some > strange > >> > > behaviours in the way brokers handle metrics and quotas. > >> > > > >> > > Feedback is appreciated. > >> > > > >> > > Thanks > >> > > > >> > > >> >
