Skip to content

logging for auth errors in kafka client binding#1664

Merged
jfallows merged 2 commits intoaklivity:developfrom
ankitk-me:logging-kafka
Mar 27, 2026
Merged

logging for auth errors in kafka client binding#1664
jfallows merged 2 commits intoaklivity:developfrom
ankitk-me:logging-kafka

Conversation

@ankitk-me
Copy link
Contributor

Fixes #1662

@ankitk-me ankitk-me requested a review from jfallows March 25, 2026 15:59
Comment on lines +437 to +438
int errorCode,
String topic)
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of adding the extra parameter on the existing method and changing all the call sites to pass null, let's add an override with the same arguments as the original here, and default null for topic, then all the call sites that don't need topic won't need to change to pass null explicitly any more.

{
case ERROR_CLUSTER_AUTHORIZATION_FAILED -> event.clusterAuthorizationFailed(traceId, bindingId, apiKey, apiVersion);
case ERROR_UNSUPPORTED_VERSION -> event.apiVersionRejected(traceId, bindingId, apiKey, apiVersion);
case ERROR_TOPIC_AUTHORIZATION_FAILED -> event.topicAuthorizationFailed(traceId, bindingId, topic);
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we need apiKey and apiVersion here as well?
I think topic authorization failure can occur on multiple different API calls so we would need to disambiguate, agree?

Comment on lines +162 to +167
public void saslAuthenticationFailed(
long traceId,
long bindingId,
String identity,
String error)
{
Copy link
Contributor

Choose a reason for hiding this comment

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

If error is not required, then create an overload method same name but without error parameter and call this passing null for error.

@jfallows jfallows merged commit 7e2d917 into aklivity:develop Mar 27, 2026
38 of 40 checks passed
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.

Logging strategy for auth errors in kafka client binding

2 participants