-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Metrics java port #637
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
Metrics java port #637
Conversation
kafka/metrics/kafka_metric.py
Outdated
| return self._config | ||
|
|
||
| def _set_config(self, config): | ||
| with self._lock: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
simple variable assignment is generally considered atomic in python. can we drop the lock here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will remove the lock entirely from KafkaMetric and leave a note that the java implementation uses one.
b749402 to
e2b340c
Compare
There is no straight translation for the JMX reporter into python, so I'll do something else in a separate commit.
This adds the parent metrics instance to kafka consumer, which will eventually be used to instrument everything under consumer. To start I ported the java consumer coordinator metrics.
|
Assuming tests pass, I think this is ready to go. It provides a complete port of the java client's metric system, minus the JMX reporter. I haven't done much of the instrumentation, but have instrumented fetch requests and the consumer coordinator as a first pass example. My plan is to ship this with the next release, and then soon thereafter work on matching the java client's instrumentation. Important: This doesn't publicly expose the metrics we're collecting just yet. Because the java client relies on JMX as the default built-in reporter, we need to make a design decision about how to handle that in kafka-python. I think a simple dictionary based approach would make sense, but would like to mess around with that some more before committing to a public interface. Towards this end I've made DictReporter that can export a snapshot of all stats at the current time and their values. That said, users can easily provide their own reporters through configuration to expose the metrics, just like in the java implementation. I've updated the documentation on #424 and #505 both add some metric instrumentation and provide a couple ways of retrieving those metrics - one exposes a dictionary of metrics, and the other has a handler system. This PR provides the same reporter interface the java client provides, which I think is similar to the handler system in #505. My intent for the DictReporter is to make the default instance of it available through a public API similar to #424. I don't know if the java implementation has the same instrumentation as the two mentioned PRs, but I would plan to mirror the java instrumentation fairly closely, but there's no reason we couldn't add our own instrumentation if it made sense. |
|
Will be interesting to watch https://issues.apache.org/jira/browse/KAFKA-3377 to see if a REST metrics interface gets added to Kafka. |
Ports the java metrics functionality, but only has one example instrumentation to keep size down.
The only thing that isn't a straight port is the DictReporter, which I made in place of the JMX reporter since there is no obvious equivalent in python. I don't have a good feel yet for how that will be used, so I would consider it a rough draft towards the standard python reporter.
One thing to keep an eye on is performance hits by the use of locks.