You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "cuishuang (via GitHub)" <gi...@apache.org> on 2026/09/05 18:00:34 UTC
[PR] KAFKA-21041: Allow equals signs in client metrics match patterns [kafka]
cuishuang opened a new pull request, #23371:
URL: https://github.com/apache/kafka/pull/23371
ClientMetricsConfigs.parseMatchingPatterns currently splits client match patterns on every '=' character. As a result, a valid pattern such as client_id=foo=bar is rejected because it produces more than two parts.
The method Javadoc specifies that the input should be split at the first occurrence of '='. This change uses split("=", 2), preserving additional equals signs in the regular expression.
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-21041: Allow equals signs in client metrics match patterns [kafka]
Posted by "cuishuang (via GitHub)" <gi...@apache.org>.
cuishuang commented on PR #23371:
URL: https://github.com/apache/kafka/pull/23371#issuecomment-5729982415
I rebased this PR onto the latest trunk, which includes the fix for the unrelated flaky ClientRebootstrapTest.testAdminRebootstrap failure (KAFKA-21014 / #23343). A fresh CI run is now in progress.
The previous requested change has also been addressed by adding a comment explaining why the pattern is split only at the first =. @AndrewJSchofield, could you please take another look when convenient? Thanks!
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-21041: Allow equals signs in client metrics match patterns [kafka]
Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on code in PR #23371:
URL: https://github.com/apache/kafka/pull/23371#discussion_r4153203656
##########
server/src/main/java/org/apache/kafka/server/metrics/ClientMetricsConfigs.java:
##########
@@ -195,7 +195,8 @@ public static Map<String, Pattern> parseMatchingPatterns(List<String> patterns)
Map<String, Pattern> patternsMap = new HashMap<>();
Review Comment:
I agree that RE2 is better and your proposed handling is good. If you can get another contributor to take it, that would be quicker. Otherwise, it will wait until KIP-1368 is voted and this code gets enhanced for the new capabilities.
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-21041: Allow equals signs in client metrics match patterns [kafka]
Posted by "cuishuang (via GitHub)" <gi...@apache.org>.
cuishuang commented on code in PR #23371:
URL: https://github.com/apache/kafka/pull/23371#discussion_r3978653211
##########
server/src/main/java/org/apache/kafka/server/metrics/ClientMetricsConfigs.java:
##########
@@ -195,7 +195,7 @@ public static Map<String, Pattern> parseMatchingPatterns(List<String> patterns)
Map<String, Pattern> patternsMap = new HashMap<>();
patterns.forEach(pattern -> {
- String[] nameValuePair = pattern.split("=");
+ String[] nameValuePair = pattern.split("=", 2);
Review Comment:
Thanks for the review. I added the requested comment above the modified line and pushed the update as a single commit.
Please review it again.
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-21041: Allow equals signs in client metrics match patterns [kafka]
Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on code in PR #23371:
URL: https://github.com/apache/kafka/pull/23371#discussion_r3951280396
##########
server/src/main/java/org/apache/kafka/server/metrics/ClientMetricsConfigs.java:
##########
@@ -195,7 +195,7 @@ public static Map<String, Pattern> parseMatchingPatterns(List<String> patterns)
Map<String, Pattern> patternsMap = new HashMap<>();
patterns.forEach(pattern -> {
- String[] nameValuePair = pattern.split("=");
+ String[] nameValuePair = pattern.split("=", 2);
Review Comment:
Please put a comment above the modified line explaining that the value can contain instances of =. Otherwise, people are going to be looking for the javadoc figuring out why it was changed.
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-21041: Allow equals signs in client metrics match patterns [kafka]
Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on code in PR #23371:
URL: https://github.com/apache/kafka/pull/23371#discussion_r4146331187
##########
server/src/main/java/org/apache/kafka/server/metrics/ClientMetricsConfigs.java:
##########
@@ -195,7 +195,8 @@ public static Map<String, Pattern> parseMatchingPatterns(List<String> patterns)
Map<String, Pattern> patternsMap = new HashMap<>();
Review Comment:
We could, or we could adopt RE2/J (which is now available in the broker since KIP-848 broker-side regex subscriptions). wdyt?
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-21041: Allow equals signs in client metrics match patterns [kafka]
Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield merged PR #23371:
URL: https://github.com/apache/kafka/pull/23371
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-21041: Allow equals signs in client metrics match patterns [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #23371:
URL: https://github.com/apache/kafka/pull/23371#discussion_r4144013837
##########
server/src/main/java/org/apache/kafka/server/metrics/ClientMetricsConfigs.java:
##########
@@ -195,7 +195,8 @@ public static Map<String, Pattern> parseMatchingPatterns(List<String> patterns)
Map<String, Pattern> patternsMap = new HashMap<>();
Review Comment:
KIP-714 states "The regular expressions are compiled and executed using Google RE2/J.", but the implementation uses java.util.regex. @AndrewJSchofield Should we revise KIP-714?
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-21041: Allow equals signs in client metrics match patterns [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #23371:
URL: https://github.com/apache/kafka/pull/23371#discussion_r4153467416
##########
server/src/main/java/org/apache/kafka/server/metrics/ClientMetricsConfigs.java:
##########
@@ -195,7 +195,8 @@ public static Map<String, Pattern> parseMatchingPatterns(List<String> patterns)
Map<String, Pattern> patternsMap = new HashMap<>();
Review Comment:
https://issues.apache.org/jira/browse/KAFKA-21195
I've opened the ticket and asked a contributor to take it over 😀
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-21041: Allow equals signs in client metrics match patterns [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #23371:
URL: https://github.com/apache/kafka/pull/23371#discussion_r4151416392
##########
server/src/main/java/org/apache/kafka/server/metrics/ClientMetricsConfigs.java:
##########
@@ -195,7 +195,8 @@ public static Map<String, Pattern> parseMatchingPatterns(List<String> patterns)
Map<String, Pattern> patternsMap = new HashMap<>();
Review Comment:
I prefer to use RE2 as well, but it is still a behavior change. If users stored a string incompatible with RE2 on the server, it can cause unexpected errors during a Kafka upgrade.
I bet the chance is almost zero, but it is Kafka. At least we need to catch the error and leave a warning log for users.
@AndrewJSchofield do you have free time to handle it? Or I could ask other contributors to take it over.
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org