Re: [PR] MINOR: Support to update Async Consumer group member label (KIP-714) [kafka]
mjsax merged PR #14946: URL: https://github.com/apache/kafka/pull/14946 -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINOR: Support to update Async Consumer group member label (KIP-714) [kafka]
apoorvmittal10 commented on PR #14946: URL: https://github.com/apache/kafka/pull/14946#issuecomment-1845174689 @mjsax @wcarlson5 Build passed with unrelated test failures. -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINOR: Support to update Async Consumer group member label (KIP-714) [kafka]
apoorvmittal10 commented on code in PR #14946: URL: https://github.com/apache/kafka/pull/14946#discussion_r1417877859 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/MembershipManagerImpl.java: ## @@ -321,6 +332,9 @@ public void onHeartbeatResponseReceived(ConsumerGroupHeartbeatResponseData respo this.memberEpoch = response.memberEpoch(); ConsumerGroupHeartbeatResponseData.Assignment assignment = response.assignment(); +clientTelemetryReporter.ifPresent(reporter -> reporter.updateMetricsLabels( +Collections.singletonMap(ClientTelemetryProvider.GROUP_MEMBER_ID, memberId))); Review Comment: @AndrewJSchofield I have added the check with comment, please see if it makes sense. -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINOR: Support to update Async Consumer group member label (KIP-714) [kafka]
apoorvmittal10 commented on code in PR #14946: URL: https://github.com/apache/kafka/pull/14946#discussion_r1417849266 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/MembershipManagerImpl.java: ## @@ -321,6 +332,9 @@ public void onHeartbeatResponseReceived(ConsumerGroupHeartbeatResponseData respo this.memberEpoch = response.memberEpoch(); ConsumerGroupHeartbeatResponseData.Assignment assignment = response.assignment(); +clientTelemetryReporter.ifPresent(reporter -> reporter.updateMetricsLabels( +Collections.singletonMap(ClientTelemetryProvider.GROUP_MEMBER_ID, memberId))); Review Comment: Rechecked the code and it seems the placement of update code is right but I can optimize with the check that there occurred a change in `memberId` prior updating it. Making the change. -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINOR: Support to update Async Consumer group member label (KIP-714) [kafka]
apoorvmittal10 commented on code in PR #14946: URL: https://github.com/apache/kafka/pull/14946#discussion_r1417815612 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/MembershipManagerImpl.java: ## @@ -321,6 +332,9 @@ public void onHeartbeatResponseReceived(ConsumerGroupHeartbeatResponseData respo this.memberEpoch = response.memberEpoch(); ConsumerGroupHeartbeatResponseData.Assignment assignment = response.assignment(); +clientTelemetryReporter.ifPresent(reporter -> reporter.updateMetricsLabels( +Collections.singletonMap(ClientTelemetryProvider.GROUP_MEMBER_ID, memberId))); Review Comment: Yeah, I needed that feedback and that's the reason I saw multiple updates happening for labels and optimized duplicate labels. However I think handling duplicates was anyways required but let me check what should be the right place where this update should be called. -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINOR: Support to update Async Consumer group member label (KIP-714) [kafka]
AndrewJSchofield commented on code in PR #14946: URL: https://github.com/apache/kafka/pull/14946#discussion_r1417760787 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/MembershipManagerImpl.java: ## @@ -321,6 +332,9 @@ public void onHeartbeatResponseReceived(ConsumerGroupHeartbeatResponseData respo this.memberEpoch = response.memberEpoch(); ConsumerGroupHeartbeatResponseData.Assignment assignment = response.assignment(); +clientTelemetryReporter.ifPresent(reporter -> reporter.updateMetricsLabels( +Collections.singletonMap(ClientTelemetryProvider.GROUP_MEMBER_ID, memberId))); Review Comment: This will work, but you are updating the metrics labels on every single heartbeat response. The member ID and member epoch are always sent. -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINOR: Support to update Async Consumer group member label (KIP-714) [kafka]
apoorvmittal10 commented on PR #14946: URL: https://github.com/apache/kafka/pull/14946#issuecomment-1843373904 @kirktrue @philipnee @AndrewJSchofield Can you please review the change and let me know if change is at right place for new consumer? cc: @mjsax @wcarlson5 -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
