Upgrade github.com/Shopify/sarama v1.33.0 to github.com/IBM/sarama v1.40.0 #4294

axfor · 2023-03-11T16:12:18Z

Bump github.com/Shopify/sarama v1.33.0 to github.com/IBM/sarama v1.40.0
Remove sarama-cluster dependencies
Use the Sarama native consumer group，see: Implement a higher-level consumer group IBM/sarama#1099

Deprecation notice of sarama-cluster：

yurishkuro · 2023-03-12T07:17:32Z

This looks a lot more than a version bump, please describe the changes.

axfor · 2023-03-12T14:34:29Z

This looks a lot more than a version bump, please describe the changes.

Bump github.com/Shopify/sarama from v1.33.0 to v1.38.1
Remove sarama-cluster dependencies
Use the Sarama native consumer group，see: Implement a higher-level consumer group IBM/sarama#1099

Deprecation notice of sarama-cluster：

pkg/kafka/consumer/config.go

cmd/ingester/app/consumer/consumer.go

pkg/kafka/consumer/config.go

plugin/storage/integration/kafka_test.go

yurishkuro · 2023-03-25T17:44:50Z

Note: merging from main invalidates previous CI runs, but unit tests there are failing due to this change. https://github.com/jaegertracing/jaeger/actions/runs/4492589597/jobs/7912198192

axfor · 2023-03-26T08:50:15Z

Note: merging from main invalidates previous CI runs, but unit tests there are failing due to this change. https://github.com/jaegertracing/jaeger/actions/runs/4492589597/jobs/7912198192

Based on the main branch, I used git merge --squash upgrade-sarama-v1.38.1 to merge all the commits of my branch

See axfor#8 :

codecov · 2023-03-26T18:33:25Z

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.03 🎉

Comparison is base (2c1bf07) 97.04% compared to head (77acc07) 97.08%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4294      +/-   ##
==========================================
+ Coverage   97.04%   97.08%   +0.03%     
==========================================
  Files         301      301              
  Lines       17857    17944      +87     
==========================================
+ Hits        17330    17421      +91     
+ Misses        422      419       -3     
+ Partials      105      104       -1

Flag	Coverage Δ
unittests	`97.08% <100.00%> (+0.03%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
cmd/ingester/app/consumer/message.go	`100.00% <ø> (ø)`
plugin/storage/kafka/factory.go	`100.00% <ø> (ø)`
plugin/storage/kafka/options.go	`89.38% <ø> (ø)`
plugin/storage/kafka/writer.go	`100.00% <ø> (ø)`
cmd/ingester/app/consumer/consumer.go	`98.31% <100.00%> (+1.34%)`	⬆️
cmd/ingester/app/consumer/consumer_metrics.go	`100.00% <100.00%> (ø)`
cmd/ingester/app/consumer/deadlock_detector.go	`100.00% <100.00%> (ø)`
cmd/ingester/app/consumer/offset/manager.go	`100.00% <100.00%> (ø)`
cmd/ingester/app/consumer/processor_factory.go	`100.00% <100.00%> (ø)`

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

axfor · 2023-04-02T14:49:24Z

Codecov Report

Patch coverage: 2.70% and project coverage change: -0.82 ⚠️

Comparison is base (ea55beb) 97.10% compared to head (39cbfea) 96.29%.

❗ Current head 39cbfea differs from pull request most recent head 555aff3. Consider uploading reports for the commit 555aff3 to get more accurate results

Additional details and impacted files
☔ View full report in Codecov by Sentry. 📢 Do you have feedback about the report comment? Let us know in this issue.

I added more unit tests

axfor · 2023-04-07T18:21:28Z

@yurishkuro @albertteoh
I added more unit tests and please re-review ! Thank you

cmd/ingester/app/consumer/consumer.go

cmd/ingester/app/consumer/processor_factory.go

cmd/ingester/app/consumer/consumer.go

cmd/ingester/app/consumer/consumer_test.go

pkg/kafka/consumer/config.go

axfor · 2023-04-16T02:38:57Z

Codecov Report

Patch coverage: 2.70% and project coverage change: -0.82 ⚠️

Comparison is base (ea55beb) 97.10% compared to head (39cbfea) 96.29%.

❗ Current head 39cbfea differs from pull request most recent head 555aff3. Consider uploading reports for the commit 555aff3 to get more accurate results

Additional details and impacted files
☔ View full report in Codecov by Sentry. 📢 Do you have feedback about the report comment? Let us know in this issue.

I added more unit tests

Patch code coverage up to 100%

refer to my repository：

https://app.codecov.io/github/jaegertracing/jaeger/commit/d26207e598c39806a6bff7689bf0e71347c3718a

axfor · 2023-04-18T17:37:15Z

Merge main to axfor:upgrade-sarama-v1.38.1 and resolve go.mod conflicts

cmd/ingester/app/consumer/consumer_test.go

cmd/ingester/app/consumer/consumer.go

albertteoh · 2023-04-19T11:39:58Z

cmd/ingester/app/consumer/consumer_test.go

+// NewTestConfig returns a config meant to be used by tests.
+// Due to inconsistencies with the request versions the clients send using the default Kafka version
+// and the response versions our mocks use, we default to the minimum Kafka version in most tests
+func NewTestConfig() *sarama.Config {


I think you may have missed this and others in this file.

pkg/kafka/consumer/config.go

cmd/ingester/app/consumer/processor_factory_test.go

cmd/ingester/app/consumer/consumer.go

cmd/ingester/app/consumer/consumer_test.go

cmd/ingester/app/consumer/consumer.go

cmd/ingester/app/consumer/offset/manager.go

cmd/ingester/app/consumer/consumer.go

cmd/ingester/app/consumer/consumer_test.go

cmd/ingester/app/consumer/consumer.go

cmd/ingester/app/consumer/consumer_test.go

Signed-off-by: axfor <aixiaoxiang2009@hotmail.com>

yurishkuro · 2023-07-26T16:49:29Z

@axfor thanks for the work. Are you interested in completing it per this ticket's scope #4591 ?

yurishkuro · 2024-08-04T18:44:44Z

Thanks for contributing, but I am going to close this. It's too risky of a change to apply without significant testing, and we are going to deprecate all of this code anyway once jaeger-v2 goes out.

axfor requested a review from a team as a code owner March 11, 2023 16:12

axfor requested a review from albertteoh March 11, 2023 16:12

axfor force-pushed the upgrade-sarama-v1.38.1 branch 2 times, most recently from 52af9be to 52f7aac Compare March 11, 2023 16:25

axfor force-pushed the upgrade-sarama-v1.38.1 branch from 7788caa to 3a82b7c Compare March 12, 2023 09:12

yurishkuro reviewed Mar 12, 2023

View reviewed changes

pkg/kafka/consumer/config.go Outdated Show resolved Hide resolved

cmd/ingester/app/consumer/consumer.go Show resolved Hide resolved

yurishkuro reviewed Mar 18, 2023

View reviewed changes

cmd/ingester/app/consumer/consumer.go Outdated Show resolved Hide resolved

yurishkuro reviewed Mar 19, 2023

View reviewed changes

cmd/ingester/app/consumer/consumer.go Outdated Show resolved Hide resolved

yurishkuro reviewed Mar 19, 2023

View reviewed changes

pkg/kafka/consumer/config.go Outdated Show resolved Hide resolved

yurishkuro reviewed Mar 19, 2023

View reviewed changes

pkg/kafka/consumer/config.go Outdated Show resolved Hide resolved

yurishkuro reviewed Mar 19, 2023

View reviewed changes

plugin/storage/integration/kafka_test.go Outdated Show resolved Hide resolved

axfor force-pushed the upgrade-sarama-v1.38.1 branch 2 times, most recently from d428632 to 39cbfea Compare March 26, 2023 08:29

axfor force-pushed the upgrade-sarama-v1.38.1 branch from fe83bc1 to 555aff3 Compare April 2, 2023 14:18

axfor requested a review from yurishkuro April 3, 2023 06:23

axfor force-pushed the upgrade-sarama-v1.38.1 branch 3 times, most recently from 380ec42 to b76ebe0 Compare April 7, 2023 18:05

axfor force-pushed the upgrade-sarama-v1.38.1 branch from b76ebe0 to 7445687 Compare April 9, 2023 08:46

albertteoh reviewed Apr 10, 2023

View reviewed changes

axfor force-pushed the upgrade-sarama-v1.38.1 branch 3 times, most recently from 532f13a to 1bbe8d1 Compare April 15, 2023 16:50

axfor force-pushed the upgrade-sarama-v1.38.1 branch from cc4f731 to d26207e Compare April 16, 2023 02:01

axfor force-pushed the upgrade-sarama-v1.38.1 branch from d26207e to 5c302b7 Compare April 18, 2023 17:16

albertteoh reviewed Apr 19, 2023

View reviewed changes

axfor force-pushed the upgrade-sarama-v1.38.1 branch 2 times, most recently from 47430ea to 131da26 Compare April 22, 2023 11:08

albertteoh reviewed Apr 25, 2023

View reviewed changes

axfor force-pushed the upgrade-sarama-v1.38.1 branch 6 times, most recently from 7ff2980 to 1eba14a Compare April 30, 2023 08:59

albertteoh reviewed Apr 30, 2023

View reviewed changes

axfor force-pushed the upgrade-sarama-v1.38.1 branch from 9ab9320 to efa562d Compare May 1, 2023 03:39

albertteoh reviewed May 9, 2023

View reviewed changes

axfor force-pushed the upgrade-sarama-v1.38.1 branch from 7113fe6 to ef59997 Compare June 23, 2023 10:10

yurishkuro mentioned this pull request Jul 18, 2023

Upgrade Kafka consumer to IBM/sarama driver #4591

Closed

5 tasks

axfor force-pushed the upgrade-sarama-v1.38.1 branch from ef59997 to 91e6303 Compare July 22, 2023 07:57

axfor changed the title ~~Bump github.com/Shopify/sarama from v1.33.0 to v1.38.1~~ Bump github.com/Shopify/sarama v1.33.0 to github.com/IBM/sarama v1.40.0 Jul 22, 2023

Bump github.com/Shopify/sarama v1.33.0 to github.com/IBM/sarama v1.40.0

c3dd2cb

Signed-off-by: axfor <aixiaoxiang2009@hotmail.com>

axfor force-pushed the upgrade-sarama-v1.38.1 branch from 91e6303 to c3dd2cb Compare July 22, 2023 08:03

axfor added 2 commits July 22, 2023 16:29

fix missing topic field

95d97e0

Signed-off-by: axfor <aixiaoxiang2009@hotmail.com>

fixed topic parameter value assignment

77acc07

Signed-off-by: axfor <aixiaoxiang2009@hotmail.com>

axfor changed the title ~~Bump github.com/Shopify/sarama v1.33.0 to github.com/IBM/sarama v1.40.0~~ Upgrade github.com/Shopify/sarama v1.33.0 to github.com/IBM/sarama v1.40.0 Jul 22, 2023

yurishkuro closed this Aug 4, 2024

yurishkuro added the fixed-in-v2 label Aug 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade github.com/Shopify/sarama v1.33.0 to github.com/IBM/sarama v1.40.0 #4294

Upgrade github.com/Shopify/sarama v1.33.0 to github.com/IBM/sarama v1.40.0 #4294

axfor commented Mar 11, 2023 •

edited

Loading

yurishkuro commented Mar 12, 2023

axfor commented Mar 12, 2023

yurishkuro commented Mar 25, 2023

axfor commented Mar 26, 2023 •

edited

Loading

codecov bot commented Mar 26, 2023 •

edited

Loading

axfor commented Apr 2, 2023

Codecov Report

axfor commented Apr 7, 2023 •

edited

Loading

axfor commented Apr 16, 2023 •

edited

Loading

Codecov Report

axfor commented Apr 18, 2023

albertteoh Apr 19, 2023

yurishkuro commented Jul 26, 2023

yurishkuro commented Aug 4, 2024

Upgrade github.com/Shopify/sarama v1.33.0 to github.com/IBM/sarama v1.40.0 #4294

Upgrade github.com/Shopify/sarama v1.33.0 to github.com/IBM/sarama v1.40.0 #4294

Conversation

axfor commented Mar 11, 2023 • edited Loading

yurishkuro commented Mar 12, 2023

axfor commented Mar 12, 2023

yurishkuro commented Mar 25, 2023

axfor commented Mar 26, 2023 • edited Loading

codecov bot commented Mar 26, 2023 • edited Loading

Codecov Report

axfor commented Apr 2, 2023

Codecov Report

axfor commented Apr 7, 2023 • edited Loading

axfor commented Apr 16, 2023 • edited Loading

Codecov Report

axfor commented Apr 18, 2023

albertteoh Apr 19, 2023

Choose a reason for hiding this comment

yurishkuro commented Jul 26, 2023

yurishkuro commented Aug 4, 2024

axfor commented Mar 11, 2023 •

edited

Loading

axfor commented Mar 26, 2023 •

edited

Loading

codecov bot commented Mar 26, 2023 •

edited

Loading

axfor commented Apr 7, 2023 •

edited

Loading

axfor commented Apr 16, 2023 •

edited

Loading