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

Why are SASL Handshake errors not logged ? #2994

Closed
pierDipi opened this issue Sep 30, 2024 · 1 comment · Fixed by #2995
Closed

Why are SASL Handshake errors not logged ? #2994

pierDipi opened this issue Sep 30, 2024 · 1 comment · Fixed by #2995

Comments

@pierDipi
Copy link
Contributor

Is there a security concern that prevents Sarama from logging SASL handshake errors?

I'm trying to debug an issue and the logger is only logging Error while performing SASL handshake kafka-1:9093 since the actual error is not logged, it's not clear what went wrong:

sarama/broker.go

Lines 1243 to 1252 in 893978c

handshakeErr := b.sendInternal(handshakeRequest, prom)
if handshakeErr != nil {
Logger.Printf("Error while performing SASL handshake %s\n", b.addr)
return handshakeErr
}
handshakeErr = handleResponsePromise(handshakeRequest, handshakeResponse, prom, metricRegistry)
if handshakeErr != nil {
Logger.Printf("Error while performing SASL handshake %s\n", b.addr)
return handshakeErr
}

sarama/broker.go

Lines 260 to 275 in 893978c

if conf.Net.SASL.Enable && !useSaslV0 {
b.connErr = b.authenticateViaSASLv1()
if b.connErr != nil {
close(b.responses)
<-b.done
err = b.conn.Close()
if err == nil {
DebugLogger.Printf("Closed connection to broker %s\n", b.addr)
} else {
Logger.Printf("Error while closing connection to broker %s: %s\n", b.addr, err)
}
b.conn = nil
atomic.StoreInt32(&b.opened, 0)
return
}
}

@puellanivis
Copy link
Contributor

Huh… I’m not at all sure why these would want to be logged at Info at all. They definitely seem like something that should be logged at error. Huh… they came in during PR #2234 with no comments.

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 a pull request may close this issue.

2 participants