kafkaex / kafka_ex

Kafka client library for Elixir
MIT License
596 stars 162 forks source link

Add kafka headers to messages, producer and consumer #397

Closed matreyes closed 1 year ago

matreyes commented 4 years ago

Kafka protocol >= 0.11 allows to send arbitrary headers [{key, value}] along with the message payload. This is very useful for metadata as schema version, origin, etc.

habutre commented 4 years ago

I am interested in work on this, please let me know how can I help. It also involves the kayrock client, right?

jbruggem commented 4 years ago

@habutre PRs are welcome of course :-). What kind of information do you need to get started ? Some pointers in the code to know where to get started ?

habutre commented 4 years ago

The main point would be if supporting headers should be first implemented on Kayrock and back ported to kafka_ex or the other way around.

I will check the points in the code where changes will be done but if there is a well-know points where this change will affects would be handy

dantswain commented 4 years ago

Kayrock already has support for headers in Kayrock.RecordBatch.Record - https://github.com/dantswain/kayrock/blob/master/lib/kayrock/record_batch.ex

It looks like it’s expecting headers to be a binary value and that gets serialized directly. I can do a little digging later today.

dantswain commented 4 years ago

OK I was half-wrong. Kayrock has the headers field for the v2 message format but it does not appear to properly serialize/deserialize the values. So you'd need to support that in Kayrock first.

The message spec for headers is here: https://kafka.apache.org/documentation/#recordheader

If you need help with that, feel free to reach out. I've been quite busy but I can find time.

habutre commented 4 years ago

Hey @dantswain and all here in order to validate the support for record headers on Kayrock I have pushed a small branch with a test case and a temporary change on deps to point KafkaEx to Kayrock master

I don't know exactly what would be the best approach here to validate Kayrock's integration and if that single test case is enough to validation purposes. but I figure out that record headers will not be a seamless integration from compatibility point of view.

I have checked other integration tests and couldn't find any similar test using RecordBatch.Record

Locally all tests passes, sometimes in the second or third run.

yingyingtang-brex commented 2 years ago

Hi! As we have the above fix merged on Nov 8, 2021, is there a plan to have a new release including this fix?

habutre commented 1 year ago

@joshuawscott according to the latest release this issue can be closed, right?