Skip to content
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

feat(kafka): Set and send SASL extensions #2832

Merged
merged 4 commits into from
Sep 4, 2024
Merged

Conversation

gunnaraasen
Copy link
Contributor

Required checklist

  • Sample config files updated (both /etc folder and NewDemoConfig methods) (influxdb and plutonium)
  • openapi swagger.yml updated (if modified API) - link openapi PR
  • Signed CLA (if not already signed)

Description

Allows SASL extensions to be set via config. Also updates the samara dependency to use the new IBM source.

@gunnaraasen
Copy link
Contributor Author

Looks like there's a bug. But I'm having a hard time deciphering the test code.

=== RUN   TestServer_AlertHandlers
=== RUN   TestServer_AlertHandlers/alerta-0
=== RUN   TestServer_AlertHandlers/bigpanda-1
=== RUN   TestServer_AlertHandlers/discord-2
=== RUN   TestServer_AlertHandlers/exec-3
=== RUN   TestServer_AlertHandlers/hipchat-4
=== RUN   TestServer_AlertHandlers/kafka-5
2024/08/29 17:22:59 Initializing new client
2024/08/29 17:22:59 Successfully initialized new client
2024/08/29 17:22:59 client/metadata fetching metadata for [test] from broker [::]:39117
2024/08/29 17:22:59 Connected to broker at [::]:39117 (unregistered)
2024/08/29 17:22:59 client/metadata got error from broker -1 while fetching metadata: kafka: insufficient data to decode packet, more bytes expected
2024/08/29 17:22:59 Closed connection to broker [::]:39117
2024/08/29 17:22:59 client/metadata no available broker to send metadata request to
2024/08/29 17:22:59 client/brokers resurrecting 1 dead seed brokers
2024/08/29 17:22:59 client/metadata retrying after 250ms... (2 attempts remaining)
2024/08/29 17:22:59 client/metadata fetching metadata for [test] from broker [::]:39117
2024/08/29 17:22:59 Connected to broker at [::]:39117 (unregistered)
2024/08/29 17:22:59 client/metadata got error from broker -1 while fetching metadata: kafka: insufficient data to decode packet, more bytes expected
2024/08/29 17:22:59 Closed connection to broker [::]:39117
2024/08/29 17:22:59 client/metadata no available broker to send metadata request to
2024/08/29 17:22:59 client/brokers resurrecting 1 dead seed brokers
2024/08/29 17:22:59 client/metadata retrying after 250ms... (1 attempts remaining)
2024/08/29 17:22:59 client/metadata fetching metadata for [test] from broker [::]:39117
2024/08/29 17:22:59 Connected to broker at [::]:39117 (unregistered)
2024/08/29 17:22:59 client/metadata got error from broker -1 while fetching metadata: kafka: insufficient data to decode packet, more bytes expected
2024/08/29 17:22:59 Closed connection to broker [::]:39117
2024/08/29 17:22:59 client/metadata no available broker to send metadata request to
2024/08/29 17:22:59 client/brokers resurrecting 1 dead seed brokers
2024/08/29 17:23:00 client/metadata retrying after 250ms... (0 attempts remaining)
2024/08/29 17:23:00 client/metadata fetching metadata for [test] from broker [::]:39117
2024/08/29 17:23:00 Connected to broker at [::]:39117 (unregistered)
2024/08/29 17:23:00 client/metadata got error from broker -1 while fetching metadata: kafka: insufficient data to decode packet, more bytes expected
2024/08/29 17:23:00 Closed connection to broker [::]:39117
2024/08/29 17:23:00 client/metadata no available broker to send metadata request to
2024/08/29 17:23:00 client/brokers resurrecting 1 dead seed brokers
    server_test.go:11907: kafka: unexpected kafka messages -exp/+got:
          []kafkatest.Message(
        -       {
        -               {
        -                       Topic:     "test",
        -                       Partition: 3,
        -                       Key:       "id",
        -                       Message:   `{"id":"id","message":"message","details":"details","time":"1970-`...,
        -                       Time:      s"2024-08-29 17:24:44.491478142 +0000 UTC",
        -               },
        -       },
        +       nil,
          )
--- FAIL: TestServer_AlertHandlers (130.34s)
    --- PASS: TestServer_AlertHandlers/alerta-0 (5.04s)
    --- PASS: TestServer_AlertHandlers/bigpanda-1 (5.02s)
    --- PASS: TestServer_AlertHandlers/discord-2 (5.03s)
    --- PASS: TestServer_AlertHandlers/exec-3 (5.02s)
    --- PASS: TestServer_AlertHandlers/hipchat-4 (5.04s)
    --- FAIL: TestServer_AlertHandlers/kafka-5 (105.19s)

@bednar bednar requested review from vlastahajek and removed request for bednar September 2, 2024 09:05
Copy link
Contributor

@vlastahajek vlastahajek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test failed because of the upgraded Sarama package (I haven't investigated the exact problem). For simplicity, I suggest keeping the previous Sarama library version.

@vlastahajek vlastahajek requested a review from bednar September 3, 2024 15:52
@vlastahajek
Copy link
Contributor

I've fixed the PR. The Sarama version is set to 1.40.1, which is the last where the test works. I'm still investigating the exact problem causing the test fails.

@gunnaraasen, is there a specific reason why there should be Sarama version 1.41.3 you previously set?

Copy link
Contributor

@bednar bednar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 🚀

Following our discussion here: https://influxdata.slack.com/archives/C5BSZ026L/p1725387374078259, it is okay to use Sarama v1.40.1.

@bednar bednar removed the request for review from dgnorton September 4, 2024 11:06
@bednar bednar merged commit b40155f into master Sep 4, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants