Re: [DISCUSS] KIP-606: Add Metadata Context to MetricsReporter

2020-05-26 Thread Xavier Léauté
Based on feedback during code review we made some minor adjustments to the
KIP:
- clarify that connect group id labels are only included in distributed mode
- rename metadata() to contextLabels() in the MetricsContext interface
- add context label configuration properties to kafka brokers as well


On Sat, May 9, 2020 at 7:21 AM Randall Hauch  wrote:

> Thanks, Xavier. Looks great.
>
> On Fri, May 8, 2020 at 7:31 PM Xavier Léauté  wrote:
>
> > > This does seem very useful. A minor request would be to mention the new
> > > configs for Connect, Streams and clients, specifically that because
> they
> > > are optional they will not hinder upgrades, and because they are
> > namespaced
> > > are unlikely to clash or conflict with other configs from extensions.
> > >
> >
> > Thanks Randall I've updated the compatibility / migration section to
> > highlight this information.
> >
>


Re: [DISCUSS] KIP-606: Add Metadata Context to MetricsReporter

2020-05-22 Thread Thomas Becker
This looks useful, I think the only nit I would pick would be to name the 
MetricsReporter method contextChanged (past tense), which seems more 
conventional for methods like this.


On Tue, 2020-05-05 at 16:58 -0700, Xavier Léauté wrote:

[EXTERNAL EMAIL] Attention: This email was sent from outside TiVo. DO NOT CLICK 
any links or attachments unless you expected them.





Hi Everyone,


I've published a KIP to address some shortcoming of our current metrics

reporter interface. Would appreciate feedback.


https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FKAFKA%2FKIP-606%253A%2BAdd%2BMetadata%2BContext%2Bto%2BMetricsReporterdata=02%7C01%7CThomas.Becker%40tivo.com%7C0504b45e4eb648514a7b08d7f1503ce5%7Cd05b7c6912014c0db45d7f1dcc227e4d%7C1%7C1%7C637243199236640045sdata=ILYVMK6e%2BeirHq0ocz2f97x%2FF9yL5mHRNr8XMe7J7nc%3Dreserved=0


Thank you,

Xavier


--
[cid:d3a26b7d3693657e816e0ddd2739d3d3b0257f01.camel@tivo.com] Tommy Becker
Principal Engineer
Personalized Content Discovery
O +1 919.460.4747
tivo.com



This email and any attachments may contain confidential and privileged material 
for the sole use of the intended recipient. Any review, copying, or 
distribution of this email (or any attachments) by others is prohibited. If you 
are not the intended recipient, please contact the sender immediately and 
permanently delete this email and any attachments. No employee or agent of TiVo 
is authorized to conclude any binding agreement on behalf of TiVo by email. 
Binding agreements with TiVo may only be made by a signed written agreement.


Re: [DISCUSS] KIP-606: Add Metadata Context to MetricsReporter

2020-05-09 Thread Randall Hauch
Thanks, Xavier. Looks great.

On Fri, May 8, 2020 at 7:31 PM Xavier Léauté  wrote:

> > This does seem very useful. A minor request would be to mention the new
> > configs for Connect, Streams and clients, specifically that because they
> > are optional they will not hinder upgrades, and because they are
> namespaced
> > are unlikely to clash or conflict with other configs from extensions.
> >
>
> Thanks Randall I've updated the compatibility / migration section to
> highlight this information.
>


Re: [DISCUSS] KIP-606: Add Metadata Context to MetricsReporter

2020-05-08 Thread Xavier Léauté
> This does seem very useful. A minor request would be to mention the new
> configs for Connect, Streams and clients, specifically that because they
> are optional they will not hinder upgrades, and because they are namespaced
> are unlikely to clash or conflict with other configs from extensions.
>

Thanks Randall I've updated the compatibility / migration section to
highlight this information.


Re: [DISCUSS] KIP-606: Add Metadata Context to MetricsReporter

2020-05-07 Thread Randall Hauch
Thanks, Xavier.

This does seem very useful. A minor request would be to mention the new
configs for Connect, Streams and clients, specifically that because they
are optional they will not hinder upgrades, and because they are namespaced
are unlikely to clash or conflict with other configs from extensions.

Thanks, and best regards.

Randall

On Thu, May 7, 2020 at 5:38 PM Gwen Shapira  wrote:

> This would be useful, thank you :)
>
> On Tue, May 5, 2020 at 4:58 PM Xavier Léauté  wrote:
>
> > Hi Everyone,
> >
> > I've published a KIP to address some shortcoming of our current metrics
> > reporter interface. Would appreciate feedback.
> >
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-606%3A+Add+Metadata+Context+to+MetricsReporter
> >
> > Thank you,
> > Xavier
> >
>
>
> --
> Gwen Shapira
> Engineering Manager | Confluent
> 650.450.2760 | @gwenshap
> Follow us: Twitter | blog
>


Re: [DISCUSS] KIP-606: Add Metadata Context to MetricsReporter

2020-05-07 Thread Gwen Shapira
This would be useful, thank you :)

On Tue, May 5, 2020 at 4:58 PM Xavier Léauté  wrote:

> Hi Everyone,
>
> I've published a KIP to address some shortcoming of our current metrics
> reporter interface. Would appreciate feedback.
>
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-606%3A+Add+Metadata+Context+to+MetricsReporter
>
> Thank you,
> Xavier
>


-- 
Gwen Shapira
Engineering Manager | Confluent
650.450.2760 | @gwenshap
Follow us: Twitter | blog


[DISCUSS] KIP-606: Add Metadata Context to MetricsReporter

2020-05-05 Thread Xavier Léauté
Hi Everyone,

I've published a KIP to address some shortcoming of our current metrics
reporter interface. Would appreciate feedback.

https://cwiki.apache.org/confluence/display/KAFKA/KIP-606%3A+Add+Metadata+Context+to+MetricsReporter

Thank you,
Xavier