-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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.
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
Document and update integration test with the versions of Kafka Jaeger supports #3068
Comments
We have a couple of action items here. An initial suggestion could be:
About the nightly jobs: I think we could even consider moving some of the tests we have from the CI to the nightly build as well. Rinse and repeat for other storage mechanisms :-) |
Great topic :) Note that in the future Jaeger will use Kafka implementation from OTEL collector. It's a newer implementation that does not use deprecated sarama-cluster implementation. Therefore the OTELcollector might be a better place for the itest. |
Special thanks @Ashmita152. Excellent topic |
hi @Mayank77maruti - sure, give it a try. We have examples of testing different backend versions with Elasticsearch. |
Hi @Mayank77maruti, are you still working on this? If you don't have much time, I would like to give it a try. |
…6400) ## Which problem is this PR solving? - Resolves #3068 ## Description of the changes - Introduce old version kafka env - Update e2e CI script ## How was this change tested? - integration tests ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [ ] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` --------- Signed-off-by: zzzk1 <[email protected]>
@zzzk1 according to https://endoflife.date/apache-kafka the support for Kafka 2.x ended 2yrs ago. Your PR is still useful as it allows us to test different versions in the future, but I think it's wasteful to be running CI tests for 2.x at this point. |
…aegertracing#6400) ## Which problem is this PR solving? - Resolves jaegertracing#3068 ## Description of the changes - Introduce old version kafka env - Update e2e CI script ## How was this change tested? - integration tests ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [ ] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` --------- Signed-off-by: zzzk1 <[email protected]>
## Which problem is this PR solving? - Resolves jaegertracing#3068 - According to https://endoflife.date/apache-kafka the support for Kafka 2.x ended 2yrs ago. There is no reason to waste CI resources testing against it. - Our documentation only mentions 3.x https://www.jaegertracing.io/docs/2.1/kafka/ ## Description of the changes - Remove Kafka 2.x from the test matrix - Remove the corresponding docker-compose file ## How was this change tested? - CI --------- Signed-off-by: Yuri Shkuro <[email protected]>
Requirement - what kind of business use case are you trying to solve?
Update jaeger-v2 kafka integration tests to test with the two most recent versions of Kafka
Problem - what in Jaeger blocks you from solving the requirement?
Right now we don't mention anywhere in the doc the version of kafka we support. This has lead to multiple open issues: #3059 #2950 #2068
Although Sarama says "Sarama is a Go library for Apache Kafka 0.8, and up." but their functional tests only test for kafka v2.7.1 and v2.8.0.
The dependabot or other PR bump up the version of Sarama which may break integration with older version of kafka.
Proposal - what do you suggest to solve the problem or improve the existing situation?
Update kafka integration test for each kafka versions we support just like how we do for ES
Any open questions to address
This will increase the integration test time but I think we should do it to prevent unknown breaking changes.
The text was updated successfully, but these errors were encountered: