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: send ApiVersionsRequest on broker open #2028

Merged
merged 1 commit into from
Sep 16, 2021
Merged

Conversation

dnwe
Copy link
Collaborator

@dnwe dnwe commented Sep 15, 2021

Currently we don't use the actual response for anything, but lay the
groundwork by sending an ApiVersionsRequest on broker open when the
kafka version config has been set to 2.4.0.0 or newer. This is useful
because the clientSoftwareName and version will show up in the
server-side metrics.

kafka.server:clientSoftwareName=sarama,clientSoftwareVersion=1.30.0,listener=PLAINTEXT,networkProcessor=1,type=socket-server-metrics

Currently we don't use the actual response for anything, but lay the
groundwork by sending an ApiVersionsRequest on broker open when the
kafka version config has been set to 2.4.0.0 or newer. This is useful
because the clientSoftwareName and version will show up in the
server-side metrics.

```
kafka.server:clientSoftwareName=sarama,clientSoftwareVersion=1.30.0,listener=PLAINTEXT,networkProcessor=1,type=socket-server-metrics
```
@dnwe dnwe requested a review from bai as a code owner September 15, 2021 14:45
@bai bai merged commit abd562c into main Sep 16, 2021
@bai bai deleted the dnwe/send-apiversionsrequest branch September 16, 2021 04:38
// 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 conf.Version.IsAtLeast(V2_4_0_0) && conf.ApiVersionsRequest {
Copy link

@ijuma ijuma Oct 7, 2021

Choose a reason for hiding this comment

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

Why do we only send this if the version is at least 2.4.0? For older broker versions, the broker will send the response with an error that will be ignored and hence won't cause any harm. We just need to adjust the logging to avoid unnecessary noise depending on the version.

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

Successfully merging this pull request may close these issues.

3 participants