Skip to content

MINOR : Handle error for client telemetry push #19881

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

Merged

Conversation

k-raina
Copy link
Contributor

@k-raina k-raina commented Jun 2, 2025

Update catch to handle compression errors

Before :
image

After

Sent message: KR Message 376
[kafka-producer-network-thread | kr-kafka-producer] INFO
org.apache.kafka.common.telemetry.internals.ClientTelemetryReporter -
KR: Failed to compress telemetry payload for compression: zstd, sending
uncompressed data
Sent message: KR Message 377

@github-actions github-actions bot added triage PRs from the community clients small Small PRs labels Jun 2, 2025
@AndrewJSchofield AndrewJSchofield added ci-approved and removed triage PRs from the community labels Jun 2, 2025
Copy link
Member

@bbejeck bbejeck left a comment

Choose a reason for hiding this comment

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

LGTM - but I'll defer to @apoorvmittal10 for the final say

@chia7712
Copy link
Member

chia7712 commented Jun 2, 2025

@k-raina thanks for this contribution.

has analogous issue as NoClassDefFoundError is not caught correctly. Could you please fix it as well?

@github-actions github-actions bot added the core Kafka Broker label Jun 3, 2025
Copy link
Contributor

@apoorvmittal10 apoorvmittal10 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, 1 minor query.

Comment on lines 720 to 721
} catch (Throwable e) {
log.info("Failed to compress telemetry payload for compression: {}, sending uncompressed data", compressionType);
Copy link
Contributor

Choose a reason for hiding this comment

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

For log.info("Failed to compress telemetry payload for...., should it be in debug mode i.e. silent exception. Or we should still log it in INFO mode so application is aware of? What others think @AndrewJSchofield @bbejeck @chia7712 @k-raina?

Copy link
Member

Choose a reason for hiding this comment

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

agreed. Using info may caused the log to be flooded with messages. If that is acceptable exception, then using debug mode should be fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to debug logs 3ea7dbb

Copy link
Contributor

@apoorvmittal10 apoorvmittal10 left a comment

Choose a reason for hiding this comment

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

Approving as we should have this fix in 4.1.

@apoorvmittal10 apoorvmittal10 merged commit 82ea9d0 into apache:trunk Jun 3, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants