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.

Sign up for GitHub

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

Jump to bottom

Support deserializing binary protobuf encoded message keys #1699

Closed
jdechicchis opened this issue Mar 4, 2022 · 8 comments · Fixed by #1729
Closed

Support deserializing binary protobuf encoded message keys #1699

jdechicchis opened this issue Mar 4, 2022 · 8 comments · Fixed by #1729
Assignees
@jdechicchis
Labels
scope/backend status/accepted An issue which has passed triage and has been accepted type/enhancement En enhancement to an already existing feature
Milestone
0.4

Comments

@jdechicchis
Copy link
Contributor

First off, Kafka UI is great, thanks for maintaining it!

I noticed this TODO in ProtobufFileRecordSerDe:

//TODO: currently we assume that keys for this serde are always string - need to discuss if it is ok

I think it would be nice to provide the ability to deserialize binary protobuf encoded message keys in addition to message values. To support this, I think a new configuration value such as Map<String, String> protobufMessageNameByTopicForKey could be added (open to suggestions here). In terms of how a message key is deserialized when using ProtobufFileRecordSerDe I feel this precedence order would make sense:

  • If the protobuf message name is found in protobufMessageNameByTopicForKey deserialize the message key using a descriptor.
  • Else, deserialize the message key as a string.

Happy to make a PR for this (I've been playing around with the code and doesn't seem like it would be terribly difficult to add), but wanted to discuss if adding such a configuration makes sense
@jdechicchis jdechicchis added status/triage Issues pending maintainers triage type/feature A new feature labels Mar 4, 2022
@github-actions
Copy link

github-actions bot commented Mar 4, 2022

Hello there jdechicchis! 👋

Thank you and congratulations 🎉 for opening your very first issue in this project! 💖

In case you want to claim this issue, please comment down below! We will try to get back to you as soon as we can. 👀

@Haarolean
Copy link
Contributor

Hey, thanks for reaching out! Glad you like it.

Let's discuss and we'll see about the PR.

@iliax, what do you think?

@iliax
Copy link
Contributor

iliax commented Mar 8, 2022

I am ok with this. Will be happy to review PR, @jdechicchis

@Haarolean Haarolean assigned jdechicchis and unassigned iliax Mar 8, 2022
@Haarolean Haarolean added scope/backend status/accepted An issue which has passed triage and has been accepted type/enhancement En enhancement to an already existing feature and removed status/triage Issues pending maintainers triage type/feature A new feature labels Mar 8, 2022
@Haarolean Haarolean added this to the 0.5 milestone Mar 8, 2022
jdechicchis added a commit to jdechicchis/kafka-ui that referenced this issue Mar 14, 2022
@jdechicchis
Add protobufMessageNameForKeyByTopic option to config. Message keys are
bb0637e 
deserialized using a protobuf schema if the config is set. Otherwise
message keys are treated as strings.

Closes provectus#1699
@jdechicchis jdechicchis mentioned this issue Mar 14, 2022
Merged
13 tasks
@jdechicchis
Copy link
Contributor Author

@iliax @Haarolean Made a PR #1729. Would appreciate a review. I noticed that there didn't seem to be much documentation on how to use protobuf files with Kafka UI (maybe I missed it). I'm happy to document this change better if desired

@Haarolean
Copy link
Contributor

@jdechicchis hey, yeah, unfortunately there's not much documentation for this. Would appreciate any documentation improvements.

@jdechicchis
Copy link
Contributor Author

@Haarolean Happy to add some docs in this PR (or a separate one if you prefer). I was thinking of adding a documentation/guides/Protobuf.md. Does that sound good to you?

@Haarolean
Copy link
Contributor

@jdechicchis would be cool! The same PR is fine, the path is okay as well.

@jdechicchis
Copy link
Contributor Author

Updated the PR with some docs

@Haarolean Haarolean modified the milestones: 0.5, 0.4 Apr 5, 2022
Haarolean added a commit that referenced this issue Apr 5, 2022
@jdechicchis @Haarolean
Implement deserializing binary protobuf encoded message keys (#1729)
1796fd5 
* Add protobufMessageNameForKeyByTopic option to config. Message keys are
deserialized using a protobuf schema if the config is set. Otherwise
message keys are treated as strings.

Closes #1699

* Add documentation around kafkaui's protobuf support

* Add protobufMessageNameForKey config option

* Update README with info about default types

* Imeplement support for protobufMessageNameForKeyByTopic

* fallback to FALLBACK_FORMATTER

* Add ability to publish message with protobuf key

* Change log levels to debug and add @nullable annotations

* Attempt at fixing documentation workflow

Co-authored-by: Ilya Kuramshin <ilia-2k@rambler.ru>
Co-authored-by: Roman Zabaluev <rzabaluev@provectus.com>
Co-authored-by: Roman Zabaluev <github@haarolean.dev>
@Haarolean Haarolean changed the title Add configuration option to deserialize binary protobuf encoded message keys Support deserializing binary protobuf encoded message keys Apr 22, 2022
javalover123 pushed a commit to javalover123/kafka-ui that referenced this issue Dec 7, 2022
@jdechicchis @Haarolean
Implement deserializing binary protobuf encoded message keys (provect…
e8999cf 
…us#1729)

* Add protobufMessageNameForKeyByTopic option to config. Message keys are
deserialized using a protobuf schema if the config is set. Otherwise
message keys are treated as strings.

Closes provectus#1699

* Add documentation around kafkaui's protobuf support

* Add protobufMessageNameForKey config option

* Update README with info about default types

* Imeplement support for protobufMessageNameForKeyByTopic

* fallback to FALLBACK_FORMATTER

* Add ability to publish message with protobuf key

* Change log levels to debug and add @nullable annotations

* Attempt at fixing documentation workflow

Co-authored-by: Ilya Kuramshin <ilia-2k@rambler.ru>
Co-authored-by: Roman Zabaluev <rzabaluev@provectus.com>
Co-authored-by: Roman Zabaluev <github@haarolean.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jdechicchis jdechicchis

Labels
scope/backend status/accepted An issue which has passed triage and has been accepted type/enhancement En enhancement to an already existing feature
Projects
None yet
Milestone
0.4
Development

Successfully merging a pull request may close this issue.

Address issue #1699 (config option to deserialize proto message keys) jdechicchis/kafka-ui
3 participants
@iliax @Haarolean @jdechicchis