You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "m1a2st (via GitHub)" <gi...@apache.org> on 2026/06/29 12:23:54 UTC
[PR] MINOR: Remove clients test-fixtures shadow JAR rewire hack [kafka]
m1a2st opened a new pull request, #22699:
URL: https://github.com/apache/kafka/pull/22699
See the discussion: https://github.com/apache/kafka/pull/22201#discussion_r3491163510
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Remove clients test-fixtures shadow JAR rewire hack [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #22699:
URL: https://github.com/apache/kafka/pull/22699#discussion_r3500402110
##########
clients/src/main/java/org/apache/kafka/common/telemetry/internals/SinglePointMetric.java:
##########
@@ -50,6 +51,60 @@ public Metric.Builder builder() {
return metricBuilder;
}
+ boolean hasSum() {
Review Comment:
Could you please add comments for those test-only methods?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Remove clients test-fixtures shadow JAR rewire hack [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #22699:
URL: https://github.com/apache/kafka/pull/22699#discussion_r3498413487
##########
clients/src/main/java/org/apache/kafka/common/telemetry/internals/ClientTelemetryReporter.java:
##########
@@ -725,7 +725,7 @@ private Optional<Builder<?>> createPushRequest(ClientTelemetrySubscription local
CompressionType compressionType = ClientTelemetryUtils.preferredCompressionType(localSubscription.acceptedCompressionTypes(), unsupportedCompressionTypes);
ByteBuffer compressedPayload;
try {
- compressedPayload = ClientTelemetryUtils.compress(payload, compressionType);
+ compressedPayload = ClientTelemetryUtils.compress(payload.toByteArray(), compressionType);
Review Comment:
It still makes an unnecessary deep copy. Memory prices have been freaking high lately
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Remove clients test-fixtures shadow JAR rewire hack [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #22699:
URL: https://github.com/apache/kafka/pull/22699#discussion_r3491837071
##########
clients/src/main/java/org/apache/kafka/common/telemetry/internals/ClientTelemetryReporter.java:
##########
@@ -725,7 +725,7 @@ private Optional<Builder<?>> createPushRequest(ClientTelemetrySubscription local
CompressionType compressionType = ClientTelemetryUtils.preferredCompressionType(localSubscription.acceptedCompressionTypes(), unsupportedCompressionTypes);
ByteBuffer compressedPayload;
try {
- compressedPayload = ClientTelemetryUtils.compress(payload, compressionType);
+ compressedPayload = ClientTelemetryUtils.compress(ByteBuffer.wrap(payload.toByteArray()), compressionType);
Review Comment:
We should avoid unnecessary memory copy here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Remove clients test-fixtures shadow JAR rewire hack [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #22699:
URL: https://github.com/apache/kafka/pull/22699#discussion_r3498683824
##########
clients/src/main/java/org/apache/kafka/common/telemetry/internals/ClientTelemetryReporter.java:
##########
@@ -725,7 +725,7 @@ private Optional<Builder<?>> createPushRequest(ClientTelemetrySubscription local
CompressionType compressionType = ClientTelemetryUtils.preferredCompressionType(localSubscription.acceptedCompressionTypes(), unsupportedCompressionTypes);
ByteBuffer compressedPayload;
try {
- compressedPayload = ClientTelemetryUtils.compress(payload, compressionType);
+ compressedPayload = ClientTelemetryUtils.compress(payload.toByteArray(), compressionType);
Review Comment:
Currently, `ClientTelemetryUtils.compress` is exposed for mocking and throwing an expected exception. We can mock `Compression#of` instead, and it works perfectly since `ClientTelemetryUtils.compress` uses it internally to create a Compression object.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Remove clients test-fixtures shadow JAR rewire hack [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on code in PR #22699:
URL: https://github.com/apache/kafka/pull/22699#discussion_r3499113450
##########
clients/src/main/java/org/apache/kafka/common/telemetry/internals/ClientTelemetryReporter.java:
##########
@@ -725,7 +725,7 @@ private Optional<Builder<?>> createPushRequest(ClientTelemetrySubscription local
CompressionType compressionType = ClientTelemetryUtils.preferredCompressionType(localSubscription.acceptedCompressionTypes(), unsupportedCompressionTypes);
ByteBuffer compressedPayload;
try {
- compressedPayload = ClientTelemetryUtils.compress(payload, compressionType);
+ compressedPayload = ClientTelemetryUtils.compress(payload.toByteArray(), compressionType);
Review Comment:
Thanks, addressed it!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: Remove clients test-fixtures shadow JAR rewire hack [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #22699:
URL: https://github.com/apache/kafka/pull/22699#discussion_r3500415962
##########
clients/src/main/java/org/apache/kafka/common/telemetry/internals/ClientTelemetryProvider.java:
##########
@@ -135,6 +136,19 @@ Resource resource() {
return resource;
}
+ /**
+ * The resource attributes as plain labels.
+ *
+ * @return resource attributes keyed by label name, preserving insertion order.
+ */
+ Map<String, String> resourceLabels() {
Review Comment:
this method could be reused by `updateLabels`
##########
clients/src/main/java/org/apache/kafka/common/telemetry/internals/ClientTelemetryProvider.java:
##########
@@ -135,6 +136,19 @@ Resource resource() {
return resource;
}
+ /**
+ * The resource attributes as plain labels.
+ *
+ * @return resource attributes keyed by label name, preserving insertion order.
+ */
+ Map<String, String> resourceLabels() {
Review Comment:
Should it be `synchronized`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org