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.

Sign up for GitHub

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

Fix OTEL kafka receiver/ingester panic #2512

Merged
merged 1 commit into from
Sep 25, 2020
Merged

Fix OTEL kafka receiver/ingester panic #2512

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions pkg/kafka/consumer/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package consumer

import (
"io"
"time"

"github.com/Shopify/sarama"
cluster "github.com/bsm/sarama-cluster"
Expand Down Expand Up @@ -63,5 +64,10 @@ func (c *Configuration) NewConsumer(logger *zap.Logger) (Consumer, error) {
if err := c.AuthenticationConfig.SetConfiguration(&saramaConfig.Config, logger); err != nil {
return nil, err
}
// cluster.NewConfig() uses sarama.NewConfig() to create the config.
// However the Jaeger OTEL module pulls in newer samara version (from OTEL collector)
// that does not set saramaConfig.Consumer.Offsets.CommitInterval to its default value 1s.
// then the samara-cluster fails if the default interval is not 1s.
saramaConfig.Consumer.Offsets.CommitInterval = time.Second
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this check whether the value has already been set, for use in non-OTEL context? Or would it only ever be the default value?

Copy link
Member Author

Choose a reason for hiding this comment

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

One second is always the default value we never override it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Note that the whole kafka implementation will be replaced to OTEL one that does not use samara-cluster #2494

return cluster.NewConsumer(c.Brokers, c.GroupID, []string{c.Topic}, saramaConfig)
}