search

HomeAdvisor / Kafdrop

Kafka UI and Monitoring Tool
Apache License 2.0
398 stars 163 forks source link

Add Avro message deserialization capability #33

Closed lcary closed 5 years ago

lcary commented 5 years ago

This pull request has the following features:

  1. Adds a new dropdown to the message viewer screen, which allows users to select a message format (e.g. "AVRO").
  2. Adds an AvroMessageDeserializer class to display Avro-serialized messages, which are indented for readability using GSON.
  3. Allows user to configure Schema Registry URL via configuration settings (optional unless Avro message viewing is being used).
  4. Allows user to configure an optional, default message format via configuration settings.
  5. Bumps the library dependency on kafka-clients to version 0.10.2.2.

This change has been tested for viewing both normal and Avro-serialized Kafka messages, and should be backward-compatible with the existing Kafdrop use cases.

See the below screenshot for an example of the new "Message Format" dropdown and Avro-deserialized messages:

screen shot 2018-12-12 at 11 05 17 pm

See the README changes for new configuration settings in CLI form.

lcary commented 5 years ago

Hello @dhayha, what's required to get this pull request merged in? I'm trying to upstream our changes so that we don't have to maintain a fork. It should be compatible with the existing use cases, since the schema registry / avro deserialization capability is controlled via an optional flag. I can make any style/test/etc adjustments, just say the word.

dhayha commented 5 years ago

Sorry for taking so long to get back to you. We definitely appreciate the contribution!

The main concern I have is that the MessageInspector now has to know whether the message uses Avro or JSON. One way to abstract this concept would be to define a MessageDeserializer interface that could be used by the MessageInspector to deserialize messages using whatever format necessary. If this could be auto-detected, great! However my instinct says that we'd need some kind of control on the UI (e.g. radio buttons, drop down) to allow the user to select what type of message format to use (maybe with a configurable default). Then, based on the user's choice, instead of passing in a schema URL, the controller passes in a MessageDeserializer instance

lcary commented 5 years ago

David, thanks for the feedback. I’ll move the deserialization logic to a MessageDeserializer interface and then I will investigate if auto-detection is feasible.

On Tue, Nov 27, 2018 at 10:26 AM David Hay notifications@github.com wrote:

Sorry for taking so long to get back to you. We definitely appreciate the contribution!

The main concern I have is that the MessageInspector now has to know whether the message uses Avro or JSON. One way to abstract this concept would be to define a MessageDeserializer interface that could be used by the MessageInspector to deserialize messages using whatever format necessary. If this could be auto-detected, great! However my instinct says that we'd need some kind of control on the UI (e.g. radio buttons, drop down) to allow the user to select what type of message format to use (maybe with a configurable default). Then, based on the user's choice, instead of passing in a schema URL, the controller passes in a MessageDeserializer instance

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/HomeAdvisor/Kafdrop/pull/33#issuecomment-442099401, or mute the thread https://github.com/notifications/unsubscribe-auth/AGdAcNUgo43V3QFWjhqRqOa57xUf8JiUks5uzVnAgaJpZM4Yeu-f .

lcary commented 5 years ago

@dhayha the changes you suggested have been incorporated into this pull request (see: https://github.com/HomeAdvisor/Kafdrop/pull/33/commits/4c079edccbada1cb9cb34e458cb1726a5cd40d7c). Could you please take another look when you have some time?