[jira] [Commented] (KAFKA-3881) Remove the replacing logic from "." to "_" in Fetcher

[Tom Bentley (Jira)]

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

Tom Bentley commented on KAFKA-3881:


[~kirktrue] the difficult is that metrics are a public API of Kafka. People 
will have built things on top of the existing metrics (e.g. dash boards, 
alerting etc) which will break if we go changing metric names. So either the 
change needs to happen in a major release, or it needs to be configurable and 
opt-in. In either case it will require a KIP.

> Remove the replacing logic from "." to "_" in Fetcher
> -
>
> Key: KAFKA-3881
> URL: https://issues.apache.org/jira/browse/KAFKA-3881
> Project: Kafka
>  Issue Type: Bug
>  Components: consumer, metrics
>Reporter: Guozhang Wang
>Assignee: Tom Bentley
>Priority: Major
>  Labels: newbie, patch-available
>
> The logic of replacing "." to "_" in metrics names / tags was originally 
> introduced in the core package's metrics since Graphite treats "." as 
> hierarchy separators (see KAFKA-1902); for the client metrics, it is supposed 
> that the GraphiteReported should take care of this itself rather than letting 
> Kafka metrics to special handle for it. In addition, right now only consumer 
> Fetcher had replace, but producer Sender does not have it actually.
> So we should consider removing this logic in the consumer Fetcher's metrics 
> package. NOTE that this is a public API backward incompatible change.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

[jira] [Commented] (KAFKA-3881) Remove the replacing logic from "." to "_" in Fetcher

[Kirk True (Jira)]

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

Kirk True commented on KAFKA-3881:
--

Two questions:
 # Is there any reason the PR wasn't merged?
 # Will making this change now break users who've coded this discrepancy into 
their applications/tools?

> Remove the replacing logic from "." to "_" in Fetcher
> -
>
> Key: KAFKA-3881
> URL: https://issues.apache.org/jira/browse/KAFKA-3881
> Project: Kafka
>  Issue Type: Bug
>  Components: consumer, metrics
>Reporter: Guozhang Wang
>Assignee: Tom Bentley
>Priority: Major
>  Labels: newbie, patch-available
>
> The logic of replacing "." to "_" in metrics names / tags was originally 
> introduced in the core package's metrics since Graphite treats "." as 
> hierarchy separators (see KAFKA-1902); for the client metrics, it is supposed 
> that the GraphiteReported should take care of this itself rather than letting 
> Kafka metrics to special handle for it. In addition, right now only consumer 
> Fetcher had replace, but producer Sender does not have it actually.
> So we should consider removing this logic in the consumer Fetcher's metrics 
> package. NOTE that this is a public API backward incompatible change.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

[jira] [Commented] (KAFKA-3881) Remove the replacing logic from "." to "_" in Fetcher

[Kirk True (Jira)]

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

Kirk True commented on KAFKA-3881:
--

We have a user running into this issue. It appears that only the 
fetcher-related metrics are the metrics that preemptively convert {{.}} to 
{{{}_{}}}.

My initial research lead me down the path to the fetcher metric names, which 
looks to jive with the direction of [~tombentley] and [~dbrinegar]'s solutions 
in their respective pull requests.

> Remove the replacing logic from "." to "_" in Fetcher
> -
>
> Key: KAFKA-3881
> URL: https://issues.apache.org/jira/browse/KAFKA-3881
> Project: Kafka
>  Issue Type: Bug
>  Components: consumer, metrics
>Reporter: Guozhang Wang
>Assignee: Tom Bentley
>Priority: Major
>  Labels: newbie, patch-available
>
> The logic of replacing "." to "_" in metrics names / tags was originally 
> introduced in the core package's metrics since Graphite treats "." as 
> hierarchy separators (see KAFKA-1902); for the client metrics, it is supposed 
> that the GraphiteReported should take care of this itself rather than letting 
> Kafka metrics to special handle for it. In addition, right now only consumer 
> Fetcher had replace, but producer Sender does not have it actually.
> So we should consider removing this logic in the consumer Fetcher's metrics 
> package. NOTE that this is a public API backward incompatible change.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

[jira] [Commented] (KAFKA-3881) Remove the replacing logic from "." to "_" in Fetcher

[Andrey Polyakov (Jira)]

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

Andrey Polyakov commented on KAFKA-3881:


+1 to this ticket and David's PR.

We have broken Prometheus dashboards for some Kafka client applications because 
of the inconsistency in metric names or labels due to the sanitation happening 
in some places and not others. Agree that only Graphite-specific components 
should be doing the lossy transform, and the raw data be available to systems 
that don't have this issue.

> Remove the replacing logic from "." to "_" in Fetcher
> -
>
> Key: KAFKA-3881
> URL: https://issues.apache.org/jira/browse/KAFKA-3881
> Project: Kafka
>  Issue Type: Bug
>  Components: consumer, metrics
>Reporter: Guozhang Wang
>Assignee: Tom Bentley
>Priority: Major
>  Labels: newbie, patch-available
>
> The logic of replacing "." to "_" in metrics names / tags was originally 
> introduced in the core package's metrics since Graphite treats "." as 
> hierarchy separators (see KAFKA-1902); for the client metrics, it is supposed 
> that the GraphiteReported should take care of this itself rather than letting 
> Kafka metrics to special handle for it. In addition, right now only consumer 
> Fetcher had replace, but producer Sender does not have it actually.
> So we should consider removing this logic in the consumer Fetcher's metrics 
> package. NOTE that this is a public API backward incompatible change.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

[jira] [Commented] (KAFKA-3881) Remove the replacing logic from "." to "_" in Fetcher

[ASF GitHub Bot (Jira)]

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

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

tombentley commented on pull request #3407: KAFKA-3881: Remove the replacing 
logic from "." to "_" in Fetcher
URL: https://github.com/apache/kafka/pull/3407
 
 
   
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Remove the replacing logic from "." to "_" in Fetcher
> -
>
> Key: KAFKA-3881
> URL: https://issues.apache.org/jira/browse/KAFKA-3881
> Project: Kafka
>  Issue Type: Bug
>  Components: consumer, metrics
>Reporter: Guozhang Wang
>Assignee: Tom Bentley
>Priority: Major
>  Labels: newbie, patch-available
>
> The logic of replacing "." to "_" in metrics names / tags was originally 
> introduced in the core package's metrics since Graphite treats "." as 
> hierarchy separators (see KAFKA-1902); for the client metrics, it is supposed 
> that the GraphiteReported should take care of this itself rather than letting 
> Kafka metrics to special handle for it. In addition, right now only consumer 
> Fetcher had replace, but producer Sender does not have it actually.
> So we should consider removing this logic in the consumer Fetcher's metrics 
> package. NOTE that this is a public API backward incompatible change.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

[jira] [Commented] (KAFKA-3881) Remove the replacing logic from "." to "_" in Fetcher

[ASF GitHub Bot (JIRA)]

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

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

dbrinegar commented on pull request #6420: KAFKA-3881: use plain topic tag in 
Fetcher metrics
URL: https://github.com/apache/kafka/pull/6420
 
 
   Reverts topic.replace conversion of dots to underbars in topic values for 
Fetcher metrics, so topic values are consistent across metrics.
   
   The original `topic.replace` code was introduced in 
https://github.com/apache/kafka/pull/939 following a pattern where the topic 
value would be prefixed with "topic-" and dots would be converted to underbars. 
This pattern was then partially backed out, leaving the dots conversion by 
mistake.
   
   Note that all other metric collections use the topic value as-is without 
conversion, so this change should align topic values from Fetcher with other 
metric topic values. The failure to align only happens when the topic value has 
a dot, so is understandably hard to find otherwise.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> Remove the replacing logic from "." to "_" in Fetcher
> -
>
> Key: KAFKA-3881
> URL: https://issues.apache.org/jira/browse/KAFKA-3881
> Project: Kafka
>  Issue Type: Bug
>  Components: consumer, metrics
>Reporter: Guozhang Wang
>Assignee: Tom Bentley
>Priority: Major
>  Labels: newbie, patch-available
>
> The logic of replacing "." to "_" in metrics names / tags was originally 
> introduced in the core package's metrics since Graphite treats "." as 
> hierarchy separators (see KAFKA-1902); for the client metrics, it is supposed 
> that the GraphiteReported should take care of this itself rather than letting 
> Kafka metrics to special handle for it. In addition, right now only consumer 
> Fetcher had replace, but producer Sender does not have it actually.
> So we should consider removing this logic in the consumer Fetcher's metrics 
> package. NOTE that this is a public API backward incompatible change.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (KAFKA-3881) Remove the replacing logic from "." to "_" in Fetcher

[David Brinegar (JIRA)]

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

David Brinegar commented on KAFKA-3881:
---

There are now a couple of other lines in Fetcher.java that need the replace() 
removed.

> Remove the replacing logic from "." to "_" in Fetcher
> -
>
> Key: KAFKA-3881
> URL: https://issues.apache.org/jira/browse/KAFKA-3881
> Project: Kafka
>  Issue Type: Bug
>  Components: consumer, metrics
>Reporter: Guozhang Wang
>Assignee: Tom Bentley
>Priority: Major
>  Labels: newbie, patch-available
>
> The logic of replacing "." to "_" in metrics names / tags was originally 
> introduced in the core package's metrics since Graphite treats "." as 
> hierarchy separators (see KAFKA-1902); for the client metrics, it is supposed 
> that the GraphiteReported should take care of this itself rather than letting 
> Kafka metrics to special handle for it. In addition, right now only consumer 
> Fetcher had replace, but producer Sender does not have it actually.
> So we should consider removing this logic in the consumer Fetcher's metrics 
> package. NOTE that this is a public API backward incompatible change.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (KAFKA-3881) Remove the replacing logic from "." to "_" in Fetcher

[David Brinegar (JIRA)]

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

David Brinegar commented on KAFKA-3881:
---

We recently came across this bug noticing the discrepancy in fetcher metrics 
with everything else, if you have a topic with a dot in the name. It looks like 
the original code in [https://github.com/apache/kafka/pull/939] was following a 
pattern to prefix and normalize tags, but then backed out the prefix while 
leaving the normalization. So it would seem a mistake. In any case, other topic 
tags throughout the client do not do this conversion of dot to underbar.

One can look at Sender.java
 
{code:java}
 Map metricTags = Collections.singletonMap("topic", topic);   
{code}
Partition.scala
{code:java}
private val tags = Map("topic" -> topic, "partition" -> partitionId.toString)
{code}
Log.scala
{code:java}
Map("topic" -> topicPartition.topic, "partition" -> 
topicPartition.partition.toString) ++ maybeFutureTag{code}
and so on, it would seem Fetcher.java is alone in this hard coded conversion, 
while all the other metric collectors use the plain topic string as-is.
 

> Remove the replacing logic from "." to "_" in Fetcher
> -
>
> Key: KAFKA-3881
> URL: https://issues.apache.org/jira/browse/KAFKA-3881
> Project: Kafka
>  Issue Type: Bug
>  Components: consumer, metrics
>Reporter: Guozhang Wang
>Assignee: Tom Bentley
>Priority: Major
>  Labels: newbie, patch-available
>
> The logic of replacing "." to "_" in metrics names / tags was originally 
> introduced in the core package's metrics since Graphite treats "." as 
> hierarchy separators (see KAFKA-1902); for the client metrics, it is supposed 
> that the GraphiteReported should take care of this itself rather than letting 
> Kafka metrics to special handle for it. In addition, right now only consumer 
> Fetcher had replace, but producer Sender does not have it actually.
> So we should consider removing this logic in the consumer Fetcher's metrics 
> package. NOTE that this is a public API backward incompatible change.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (KAFKA-3881) Remove the replacing logic from "." to "_" in Fetcher

[raghavan chockalingam (JIRA)]

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

raghavan chockalingam commented on KAFKA-3881:
--

This pull request is open since June 2017. Is this requirement still valid?

I believe you meant the {{MetricName}} should be agnostic of the name format 
that a particular metric platform / repository might require such as Graphite. 
An implementation of {{MetricsReporter}} should be able to convert 
{{MetricName}} appropriately so that the metric is reported as intended in a 
metric platform.

> Remove the replacing logic from "." to "_" in Fetcher
> -
>
> Key: KAFKA-3881
> URL: https://issues.apache.org/jira/browse/KAFKA-3881
> Project: Kafka
>  Issue Type: Bug
>  Components: consumer, metrics
>Reporter: Guozhang Wang
>Assignee: Tom Bentley
>Priority: Major
>  Labels: newbie, patch-available
>
> The logic of replacing "." to "_" in metrics names / tags was originally 
> introduced in the core package's metrics since Graphite treats "." as 
> hierarchy separators (see KAFKA-1902); for the client metrics, it is supposed 
> that the GraphiteReported should take care of this itself rather than letting 
> Kafka metrics to special handle for it. In addition, right now only consumer 
> Fetcher had replace, but producer Sender does not have it actually.
> So we should consider removing this logic in the consumer Fetcher's metrics 
> package. NOTE that this is a public API backward incompatible change.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (KAFKA-3881) Remove the replacing logic from "." to "_" in Fetcher

[ASF GitHub Bot (JIRA)]

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

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

GitHub user tombentley opened a pull request:

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

KAFKA-3881: Remove the replacing logic from "." to "_" in Fetcher



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

$ git pull https://github.com/tombentley/kafka KAFKA-3881

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

https://github.com/apache/kafka/pull/3407.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 #3407


commit be0de3a22b30648e2c811ee1442145702024af7a
Author: Tom Bentley 
Date:   2017-06-22T10:28:58Z

KAFKA-3881: Remove the replacing logic from "." to "_" in Fetcher




> Remove the replacing logic from "." to "_" in Fetcher
> -
>
> Key: KAFKA-3881
> URL: https://issues.apache.org/jira/browse/KAFKA-3881
> Project: Kafka
>  Issue Type: Bug
>  Components: consumer, metrics
>Reporter: Guozhang Wang
>  Labels: newbie, patch-available
>
> The logic of replacing "." to "_" in metrics names / tags was originally 
> introduced in the core package's metrics since Graphite treats "." as 
> hierarchy separators (see KAFKA-1902); for the client metrics, it is supposed 
> that the GraphiteReported should take care of this itself rather than letting 
> Kafka metrics to special handle for it. In addition, right now only consumer 
> Fetcher had replace, but producer Sender does not have it actually.
> So we should consider removing this logic in the consumer Fetcher's metrics 
> package. NOTE that this is a public API backward incompatible change.



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