[jira] [Commented] (KAFKA-6011) AppInfoParser should only use metrics API and should not register JMX mbeans directly

2017-10-05 Thread Rajini Sivaram (JIRA)

[ 
https://issues.apache.org/jira/browse/KAFKA-6011?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16192956#comment-16192956
 ] 

Rajini Sivaram commented on KAFKA-6011:
---

[~ewencp] Yes, It is odd to have both {{client-id=}} and {{id=}}, but the old 
{{id=}} ones that are directly registered with JMX are being deprecated (at 
least we have mentioned the deprecation in upgrade notes), so eventually we 
will just have the client-id metrics. I am not sure anyone actually uses 
different versions within a JVM, but it feels better to retain that flexibility 
and avoid registering global metrics. It also keeps all the metrics registered 
by a client together with consistent tags and allows these metrics to be 
created and deleted easily without checking for other clients.

> AppInfoParser should only use metrics API and should not register JMX mbeans 
> directly
> -
>
> Key: KAFKA-6011
> URL: https://issues.apache.org/jira/browse/KAFKA-6011
> Project: Kafka
>  Issue Type: Bug
>  Components: metrics
>Reporter: Ewen Cheslack-Postava
>Priority: Minor
>
> AppInfoParser collects info about the app ID, version, and commit ID and logs 
> them + exposes corresponding metrics. For some reason we ended up with the 
> app ID metric being registered directly to JMX while the version and commit 
> ID use the metrics API. This means the app ID would not be accessible to 
> custom metrics reporter.
> This isn't a huge loss as this is probably a rarely used metric, but we 
> should really only be using the metrics API. Only using the metrics API would 
> also reduce and centralize the places we need to do name mangling to handle 
> characters that might not be valid for metrics.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (KAFKA-6011) AppInfoParser should only use metrics API and should not register JMX mbeans directly

2017-10-04 Thread Ewen Cheslack-Postava (JIRA)

[ 
https://issues.apache.org/jira/browse/KAFKA-6011?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16192105#comment-16192105
 ] 

Ewen Cheslack-Postava commented on KAFKA-6011:
--

[~rsivaram] Fair point, I see that we have app-info for both client-id= and id= 
which seems a bit silly. We'd need to deprecate to to remove one of them. For 
connect we're currently registering by worker ID which is not just a client ID, 
so this would actually be a different bean. But really it just seems like 
overkill to even record this for every client. I guess with multiple 
classloaders you could end up with different versions in the same process, I 
guess maybe that's the motivation for not having a single global metric?

> AppInfoParser should only use metrics API and should not register JMX mbeans 
> directly
> -
>
> Key: KAFKA-6011
> URL: https://issues.apache.org/jira/browse/KAFKA-6011
> Project: Kafka
>  Issue Type: Bug
>  Components: metrics
>Reporter: Ewen Cheslack-Postava
>Priority: Minor
>
> AppInfoParser collects info about the app ID, version, and commit ID and logs 
> them + exposes corresponding metrics. For some reason we ended up with the 
> app ID metric being registered directly to JMX while the version and commit 
> ID use the metrics API. This means the app ID would not be accessible to 
> custom metrics reporter.
> This isn't a huge loss as this is probably a rarely used metric, but we 
> should really only be using the metrics API. Only using the metrics API would 
> also reduce and centralize the places we need to do name mangling to handle 
> characters that might not be valid for metrics.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (KAFKA-6011) AppInfoParser should only use metrics API and should not register JMX mbeans directly

2017-10-04 Thread Rajini Sivaram (JIRA)

[ 
https://issues.apache.org/jira/browse/KAFKA-6011?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16191994#comment-16191994
 ] 

Rajini Sivaram commented on KAFKA-6011:
---

The id that is passed to {{AppInfoParser}} by KafkaProducer, KafkaConsumer and 
KafkaAdminClient is the client-id which is set as a tag for commit-id and 
version metrics. Do we really need to duplicate this as a separate metric?

> AppInfoParser should only use metrics API and should not register JMX mbeans 
> directly
> -
>
> Key: KAFKA-6011
> URL: https://issues.apache.org/jira/browse/KAFKA-6011
> Project: Kafka
>  Issue Type: Bug
>  Components: metrics
>Reporter: Ewen Cheslack-Postava
>Priority: Minor
>
> AppInfoParser collects info about the app ID, version, and commit ID and logs 
> them + exposes corresponding metrics. For some reason we ended up with the 
> app ID metric being registered directly to JMX while the version and commit 
> ID use the metrics API. This means the app ID would not be accessible to 
> custom metrics reporter.
> This isn't a huge loss as this is probably a rarely used metric, but we 
> should really only be using the metrics API. Only using the metrics API would 
> also reduce and centralize the places we need to do name mangling to handle 
> characters that might not be valid for metrics.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (KAFKA-6011) AppInfoParser should only use metrics API and should not register JMX mbeans directly

2017-10-04 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/KAFKA-6011?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16191965#comment-16191965
 ] 

ASF GitHub Bot commented on KAFKA-6011:
---

GitHub user tedyu opened a pull request:

https://github.com/apache/kafka/pull/4019

KAFKA-6011 AppInfoParser should only use metrics API and should not 
register JMX mbeans directly

Added app ID to metrics API.

The JMX can be dropped post 1.0.0

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tedyu/kafka trunk

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/4019.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #4019


commit 28b6a5868327fe827d57328c03e086eb3bb5c19c
Author: tedyu 
Date:   2017-10-04T20:10:22Z

KAFKA-6011 AppInfoParser should only use metrics API and should not 
register JMX mbeans directly




> AppInfoParser should only use metrics API and should not register JMX mbeans 
> directly
> -
>
> Key: KAFKA-6011
> URL: https://issues.apache.org/jira/browse/KAFKA-6011
> Project: Kafka
>  Issue Type: Bug
>  Components: metrics
>Reporter: Ewen Cheslack-Postava
>Priority: Minor
>
> AppInfoParser collects info about the app ID, version, and commit ID and logs 
> them + exposes corresponding metrics. For some reason we ended up with the 
> app ID metric being registered directly to JMX while the version and commit 
> ID use the metrics API. This means the app ID would not be accessible to 
> custom metrics reporter.
> This isn't a huge loss as this is probably a rarely used metric, but we 
> should really only be using the metrics API. Only using the metrics API would 
> also reduce and centralize the places we need to do name mangling to handle 
> characters that might not be valid for metrics.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


[jira] [Commented] (KAFKA-6011) AppInfoParser should only use metrics API and should not register JMX mbeans directly

2017-10-04 Thread Ismael Juma (JIRA)

[ 
https://issues.apache.org/jira/browse/KAFKA-6011?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16191938#comment-16191938
 ] 

Ismael Juma commented on KAFKA-6011:


I agree that it was a mistake to use JMX directly although I don't know the 
history behind that (before my time). Since we wanted to have the version and 
commit id available for metrics reporters, we added that and deprecated the 
ones registered directly via JMX (via an upgrade note, not sure if there's a 
better way). Seems like we app id was missed, I agree that it would be good to 
have as well. cc [~rsivaram]

> AppInfoParser should only use metrics API and should not register JMX mbeans 
> directly
> -
>
> Key: KAFKA-6011
> URL: https://issues.apache.org/jira/browse/KAFKA-6011
> Project: Kafka
>  Issue Type: Bug
>  Components: metrics
>Reporter: Ewen Cheslack-Postava
>Priority: Minor
>
> AppInfoParser collects info about the app ID, version, and commit ID and logs 
> them + exposes corresponding metrics. For some reason we ended up with the 
> app ID metric being registered directly to JMX while the version and commit 
> ID use the metrics API. This means the app ID would not be accessible to 
> custom metrics reporter.
> This isn't a huge loss as this is probably a rarely used metric, but we 
> should really only be using the metrics API. Only using the metrics API would 
> also reduce and centralize the places we need to do name mangling to handle 
> characters that might not be valid for metrics.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)