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

Add KafkaMetricsRegistry #243

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add KafkaMetricsRegistry #243

wants to merge 2 commits into from

Conversation

Z1kkurat
Copy link
Contributor

This issue addresses #237.

Please read the updated README.md, it contains the reasoning behind the change and a full (I hope) description of the new API.

Let me know what you think about it.

@t3hnar
Copy link
Contributor

t3hnar commented Mar 8, 2023

hm, are we sure it should be module of smetrics and not skafka ?

@Z1kkurat
Copy link
Contributor Author

Z1kkurat commented Mar 8, 2023

hm, are we sure it should be module of smetrics and not skafka ?

@t3hnar This is a good question. On the one hand, I agree that this code doesn't depend on anything from smetrics and could be placed in skafka. On the other hand, the whole essence of it is collecting metrics, I don't think it makes much sense without having KafkaMetricsCollector to expose the collected metrics and it was created to overcome the limitations of KafkaMetricsCollector (being a Prometheus collector).

But I like the idea of moving it to skafka. I'll start working on it.

@Z1kkurat
Copy link
Contributor Author

Z1kkurat commented Mar 8, 2023

@t3hnar thanks for the idea, PR to skafka with the identical content: evolution-gaming/skafka#324

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