kafkaex/kafka_ex

Add kafka headers to messages, producer and consumer

matreyes opened this issue · 8 comments

matreyes commented

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

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

jbruggem commented

@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

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

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

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.

  • 👍1
  • 👀1
habutre commented

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

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

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