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

Include software version in requests in order to collect metrics for the client on brokers #2756

Open
omersiar opened this issue Dec 22, 2023 · 6 comments

Comments

@omersiar
Copy link

Hi,

Brokers can track of client versions and their metrics can be collected from JMX. We discovered almost every library set their software version but not the sarama.

https://docs.confluent.io/platform/current/kafka/monitoring.html#clientsoftwarename-clientsoftwareversion

@omersiar
Copy link
Author

omersiar commented Dec 25, 2023

Maybe related to issue below, since sarama incorporates version.go

golang/go#50603

@omersiar
Copy link
Author

If I understand correctly, if boolean returned true it may not reflect the dependent package

franz-go implemented like this:
https://github.com/twmb/franz-go/blob/master/pkg/kgo/config.go#L418-L430

@dnwe
Copy link
Collaborator

dnwe commented Feb 11, 2024

@omersiar Sarama does send its software name and version to Kafka as long as you are using sarama.Version = V2_4_0_0 or newer in your config and haven't disabled it via https://github.com/IBM/sarama/blob/5f63a84f47c39bf08a1c276f1f6b5f1d754e9fc3/config.go#L485-L489C2

sarama/broker.go

Lines 176 to 201 in 5f63a84

usingApiVersionsRequests := conf.Version.IsAtLeast(V2_4_0_0) && conf.ApiVersionsRequest
b.lock.Lock()
if b.metricRegistry == nil {
b.metricRegistry = newCleanupRegistry(conf.MetricRegistry)
}
go withRecover(func() {
defer func() {
b.lock.Unlock()
// Send an ApiVersionsRequest to identify the client (KIP-511).
// Ideally Sarama would use the response to control protocol versions,
// but for now just fire-and-forget just to send
if usingApiVersionsRequests {
_, err = b.ApiVersions(&ApiVersionsRequest{
Version: 3,
ClientSoftwareName: defaultClientSoftwareName,
ClientSoftwareVersion: version(),
})
if err != nil {
Logger.Printf("Error while sending ApiVersionsRequest to broker %s: %s\n", b.addr, err)
}
}
}()

@ijuma
Copy link

ijuma commented Mar 3, 2024

It's suboptimal that this isn't sent by default though. Can we send API versions by default and ignore the error if the broker doesn't understand it?

This comment was marked as outdated.

@github-actions github-actions bot added the stale Issues and pull requests without any recent activity label Jun 2, 2024
@dnwe dnwe removed the stale Issues and pull requests without any recent activity label Jun 3, 2024

This comment was marked as outdated.

@github-actions github-actions bot added the stale Issues and pull requests without any recent activity label Sep 1, 2024
@dnwe dnwe removed the stale Issues and pull requests without any recent activity label Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants