openzipkin-contrib / zipkin-storage-kafka

Kafka-based storage for Zipkin.
Apache License 2.0
37 stars 9 forks source link

Replace Confluent docker images to Zipkin-Kafka on IT #45

Closed jeqo closed 4 years ago

jeqo commented 5 years ago

Check if possible to replace default Kafka container based on Confluent:

https://github.com/openzipkin-contrib/zipkin-storage-kafka/blob/master/storage/src/test/java/zipkin2/storage/kafka/KafkaStorageIT.java#L63-L64

to zipkin-kafka. There need to be some tweak on the current image to inject right hostname and port, similar to

https://github.com/testcontainers/testcontainers-java/blob/master/modules/kafka/src/main/java/org/testcontainers/containers/KafkaContainer.java#L32-L51

jeqo commented 5 years ago

This will require to change how zipkin-kafka image pass variables to configuration and initialize process to be able to apply this. Here is how it is down with confluent images: https://github.com/testcontainers/testcontainers-java/blob/master/modules/kafka/src/main/java/org/testcontainers/containers/KafkaContainer.java#L32-L51

I've give a couple of attempts to implement this without success so far, and enabling this variable passing in our kafka image will require significant changes that I don't think is worth it just to be used under integration tests.

cc @adriancole

codefromthecrypt commented 5 years ago

I'd leave this open then. if you have an inventory of which parameters are needed specifically (might be different than all the ones passed in that link) it might help avoid some later time where we discover we also need them.

codefromthecrypt commented 5 years ago

fwiw the confluent cp-kafka image is licensed not apache, but seems ok? https://www.confluent.io/confluent-community-license

it is also nearly 600M with no shared layers with our stuff

confluentinc/cp-kafka   5.3.0               f9936a4fcbce        3 months ago        589MB

also it is hard to know if there is anything different about this build and normal kafka unless you have special knowledge.

None of the things mentioned are critical, just it would be nice to not have to think about them.

OneCricketeer commented 4 years ago

The wurstmeister or bitnami containers are much smaller, by the way, since they pull pure Apache sources, not the rest of Confluent Platform dependencies

jeqo commented 4 years ago

@OneCricketeer thanks for the tip!

I give some time to test further if zipkin's kafka container is enough and found FixedHostPortContainer, helping me to fix port mapping. Seems that openzipkin/zipkin-kafka is good enough to move forward #70 :)