Closed jutley closed 5 years ago
braedon
commented
5 years ago @jutley re. the logging level changes, how about we change the struct_error ones to warnings (on the assumption they aren't messages we care about), but leave the unsupported version logs as errors. If Kafka is producing messages with a version we don't understand, the exporter is probably not going to work correctly, and the exporter needs to be updated.
Does that make sense for you?
jutley
commented
5 years ago In reality, we don't have a lot of consumers, just some very buggy consumers that commit at an excessive rate. I'm not sure whether it's really appropriate to refuse to upgrade the Kafka client for this issue. I imagine at some point it'll cause issues. Unfortunately it is creating issues for us, and since there is no way to parallelize this exporter we're kind of stuck.
I think your suggestions regarding logging make sense.
braedon
commented
5 years ago I'd be more comfortable not holding back kafka-python (unless other users are having similar consumption rate issues), if you're happy to deal with it on your side. Can you just install 1.4.4 specifically yourself?
If so, how about I create a separate PR with the logging and dead code changes, and we leave the kafka-python requirement as in for now? Let me know 😄
jutley
commented
5 years ago Cleaned up in #35
This PR makes a few changes.
Primarily, it updates the dependency on kafka-python to use 1.4.4 in order to fix #30.
I removed some unreachable code, which was introduced when refactoring the logic to remove data for deleted consumer groups.
Logs from parsing failures are changed from ERROR to WARNING, which seems appropriate since these failures are not the fault of the exporter. It just encountered bad data. We should be able to let real errors through and hide these messages by changing the log level.