-
Notifications
You must be signed in to change notification settings - Fork 122
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
Add Kafka msgpack support #440
Conversation
That's great addition, thanks, @zerosoul13 ! |
- kafka.go InitPrometheus function signature update on receiver and paramater as these are unused - msgpack.go update string compare on metric name from len(d.Name) to d.Name == "" as proposed by linter
Great, it's green now. It's simple enough to merge, but I would wait for day, maybe someone else want to review. |
hi @zerosoul13 , it seems the new protocol isn't handled in Kafka.worker? |
The missing line has been added. |
LGTM. What do you think, @bom-d-van ? @zerosoul13 - as far as I understand you didn't test it in prod, but you had similar patch implemented, as you discussed in #288 (comment) ? |
@deniszh I tested this patch on a test environment. I was able to consume from a specific Kafka partition without any issues |
hi yeah, it lgtm as well. thanks for the addition. |
Hello,
When I tried to integrate carbon-relay-ng through Kafka I found that the format produced by carbon-relay-ng is not compatible with go-carbon.
This commit adds support for msgpack format so that, if we (the users) decide to use carbon-relay-ng and Kafka, it works out of the box with go-carbon.
Related issues: