Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace colons in group ID with dash #291

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

connor-thanx
Copy link

@connor-thanx connor-thanx commented Jun 17, 2022

Hi. I have recent been encountering an issue when using Racecar with AWS MSK. Whenever I use a consumer class which is not namespaced (e.g. MyFunnyConsumer), everything works fine, however, if I use a consumer class which is namespace, (e.g. MyFunny::Consumer), I no longer am able to see Cloudwatch metrics for that consumer group.

From some testing it seems like it is due to the : in the group ID. I see some logic exists to format the group ID, however it seems limited to handling camel case. In my example here, the MyFunny::Consumer class is transformed into a group ID of my-funny::consumer.

# MyFunnyConsumer => my-funny-consumer
consumer_class.name.gsub(/[a-z][A-Z]/) { |str| "#{str[0]}-#{str[1]}" }.downcase,

Now, I know my use case here, and the issue I am seeing, is limited to a proprietary Kafka implementation. However, I believe colons may be illegal characters for Kafka group IDs. I have struggled to find canonical documentation around this, but I have come across some related documentation:

In the Consumer group ID property, specify the ID of the consumer group to which this consumer belongs. This ID can be up to 255 characters in length, and can include the following characters: a-z, A-Z, 0-9, . (dot), _ (underscore), and - (dash).

From: https://www.ibm.com/docs/en/app-connect/11.0.0?topic=enterprise-consuming-messages-from-kafka-topics

It's possible cause errors thrown in kafka broker when defining client id with invalid characters. The invalid characters I have tested are ?:,".

From: SOHU-Co/kafka-node#394

I can understand if there may be something I'm overlooking, or that the current behaviour is the desired behaviour - but I thought I'd open a quick PR in case this is valid.

@leonmaia
Copy link
Collaborator

leonmaia commented Jul 14, 2022

Thanks for the PR @connor-thanx! We noticed recently that we also have some consumers with group_ids containing ::. We need to remember that if we go ahead with this change, we will change the group id of existing consumers, meaning that the Kafka Broker will consider the existing consumers as different ones when deployed.

We need to put this as an optional change, maybe create a new configuration to control this behavior, defaulting to the current one if not set. WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants