KAFKA-15830: Add telemetry API handling (KIP-714) by apoorvmittal10 · Pull Request #14767 · apache/kafka

apoorvmittal10 · 2023-11-15T17:50:23Z

The PR adds handling of telemetry APIs in KafkaAPIs.scala which calls the respective manager to handle the API calls. Also the telemetry plugin which if registered in configs get registered for exporting client metrics.

Committer Checklist (excluded from commit message)

Verify design and implementation
Verify test coverage and CI build status
Verify documentation (including upgrade notes)

apoorvmittal10 · 2023-11-15T18:17:19Z

Build depends on PR: #14699

junrao

@apoorvmittal10 : Thanks for the PR. Left a couple of comments.

apoorvmittal10 · 2023-11-30T08:24:31Z

@junrao @AndrewJSchofield I have added the filtering for telemetry keys if plugin is not configured, please if the changes can be re-reviewed. I have addressed other feedback as well.

AndrewJSchofield

lgtm

junrao

@apoorvmittal10 : Thanks for the update PR. A couple of more comments.

junrao

@apoorvmittal10 : Thanks for the updated PR. The code LGTM. Waiting for the tests to finish.

junrao · 2023-12-02T18:05:07Z

@apoorvmittal10 : Are the 59 test failures related, especially those ApiVersionsRequestTest?

apoorvmittal10 · 2023-12-03T08:43:38Z

@apoorvmittal10 : Are the 59 test failures related, especially those ApiVersionsRequestTest?

Hi @junrao, I should have realized that integration test in ApiVersionsRequestTest are legitimate before re-running the build. I have added fix in the PR where filtering needs to be applied while determining the keys returned as per config in integration tests.

Waiting for the build to finish, I ll check the tests.

apoorvmittal10 · 2023-12-03T16:40:59Z

@junrao The build passed on all JDK with flaky tests failure.

There are some consistent failures as well which are unrelated to the PR changes.

apoorvmittal10 · 2023-12-03T19:35:59Z

@junrao Below are the status of flaky tests. I have ran locally the flaky tests and can see them passed. I have triaged some of the failing tests, others have existing jira.

Existing: Build / JDK 17 and Scala 2.13 / testOffsetTranslationBehindReplicationFlow() – org.apache.kafka.connect.mirror.integration.MirrorConnectorsIntegrationExactlyOnceTest
40s: https://issues.apache.org/jira/browse/KAFKA-15197

Triaged: Build / JDK 17 and Scala 2.13 / testQuotaOverrideDelete(String).quorum=kraft – kafka.api.ClientIdQuotaTest
2s: https://issues.apache.org/jira/browse/KAFKA-15960

Triaged: Build / JDK 17 and Scala 2.13 / testTopicIdPersistsThroughControllerRestart() – kafka.controller.ControllerIntegrationTest
<1s: https://issues.apache.org/jira/browse/KAFKA-15961

Triaged: Build / JDK 17 and Scala 2.13 / testAlterPartitionVersion2KeepWorkingWhenControllerDowngradeToPre28IBP() – kafka.controller.ControllerIntegrationTest
<1s: https://issues.apache.org/jira/browse/KAFKA-15962

Triaged: Build / JDK 17 and Scala 2.13 / testBrokerHeartbeatDuringMigration [3] 3.6-IV0 – org.apache.kafka.controller.QuorumControllerTest
<1s: https://issues.apache.org/jira/browse/KAFKA-15963

Triaged: Build / JDK 17 and Scala 2.13 / testHighAvailabilityTaskAssignorLargeNumConsumers – org.apache.kafka.streams.processor.internals.StreamsAssignmentScaleTest
1m 35s: https://issues.apache.org/jira/browse/KAFKA-15964

Existing: Build / JDK 8 and Scala 2.12 / testSyncTopicConfigs() – org.apache.kafka.connect.mirror.integration.MirrorConnectorsIntegrationBaseTest
1m 49s: https://issues.apache.org/jira/browse/KAFKA-15945

Existing: Build / JDK 8 and Scala 2.12 / testReplicateSourceDefault() – org.apache.kafka.connect.mirror.integration.MirrorConnectorsIntegrationExactlyOnceTest
2m 5s: https://issues.apache.org/jira/browse/KAFKA-15927

Existing: Build / JDK 8 and Scala 2.12 / testWithGroupMetadata() – kafka.api.TransactionsBounceTest
28s: https://issues.apache.org/jira/browse/KAFKA-10251

Existing: Build / JDK 8 and Scala 2.12 / testWithGroupMetadata() – kafka.api.TransactionsBounceTest
21s: https://issues.apache.org/jira/browse/KAFKA-10251

Existing: Build / JDK 21 and Scala 2.13 / testMultiWorkerRestartOnlyConnector – org.apache.kafka.connect.integration.ConnectorRestartApiIntegrationTest: https://issues.apache.org/jira/browse/KAFKA-15675

Existing: Build / JDK 11 and Scala 2.13 / testWithGroupMetadata() – kafka.api.TransactionsBounceTest: https://issues.apache.org/jira/browse/KAFKA-10251

junrao · 2023-12-03T19:47:28Z

@apoorvmittal10 : Thanks for triaging the tests. There are a bunch of failures for BrokerRegistrationRequestTest. Is that being tracked?

apoorvmittal10 · 2023-12-03T19:56:05Z

@apoorvmittal10 : Thanks for triaging the tests. There are a bunch of failures for BrokerRegistrationRequestTest. Is that being tracked?

I have created jira to track that: https://issues.apache.org/jira/browse/KAFKA-15965

Can see jira already created for ConsistencyVectorIntegrationTest: https://issues.apache.org/jira/browse/KAFKA-15957

apoorvmittal10 · 2023-12-04T05:06:23Z

@junrao Please let me know if I need to address anything else or the PR is good to merge.

junrao · 2023-12-04T17:27:14Z

@apoorvmittal10 : Now that #14887 is merged. Triggering a rerun of the tests.

apoorvmittal10 · 2023-12-04T17:38:57Z

@apoorvmittal10 : Now that #14887 is merged. Triggering a rerun of the tests.

Thanks @junrao

apoorvmittal10 · 2023-12-04T17:42:12Z

@junrao I think the merge from upstream is required to get the changes in current branch. I have merged upstream/trunk.

junrao · 2023-12-04T23:58:48Z

@apoorvmittal10 : Thanks for triaging the tests. Merging the PR.

The PR adds handling of telemetry APIs in KafkaAPIs.scala which calls the respective manager to handle the API calls. Also the telemetry plugin which if registered in configs get registered for exporting client metrics. Reviewers: Jun Rao <junrao@gmail.com>

apoorvmittal10 commented Nov 15, 2023

View reviewed changes

Comment thread core/src/main/java/kafka/metrics/ClientMetricsReceiverPlugin.java

apoorvmittal10 commented Nov 15, 2023

View reviewed changes

Comment thread core/src/main/scala/kafka/server/KafkaApis.scala Outdated

mjsax added core Kafka Broker kip Requires or implements a KIP labels Nov 21, 2023

apoorvmittal10 commented Nov 22, 2023

View reviewed changes

Comment thread core/src/main/scala/kafka/server/DynamicBrokerConfig.scala

KAFKA-15830: Add telemetry API handling

5c4d371

apoorvmittal10 force-pushed the kip-714-ak-rpc branch from cae7b8c to 5c4d371 Compare November 24, 2023 12:52

apoorvmittal10 added 2 commits November 28, 2023 19:29

Merge remote-tracking branch 'upstream/trunk' into kip-714-ak-rpc

3cdb61c

Merge remote-tracking branch 'upstream/trunk' into kip-714-ak-rpc

97fe0d4

junrao reviewed Nov 29, 2023

View reviewed changes

Comment thread core/src/main/scala/kafka/server/KafkaApis.scala Outdated

Comment thread core/src/main/scala/kafka/server/KafkaApis.scala Outdated

apoorvmittal10 added 2 commits November 30, 2023 05:01

Corrected throttle code

7c8ce7a

Move telemetry enabled check to ApiVersion

38048a0

apoorvmittal10 requested review from AndrewJSchofield and junrao November 30, 2023 08:24

Removed test log

48073cd

AndrewJSchofield approved these changes Nov 30, 2023

View reviewed changes

junrao reviewed Nov 30, 2023

View reviewed changes

Comment thread core/src/main/scala/kafka/server/BrokerServer.scala

Comment thread core/src/main/scala/kafka/server/ApiVersionManager.scala

Addressing review comments

e35888b

apoorvmittal10 requested a review from junrao December 1, 2023 07:28

apoorvmittal10 added 2 commits December 1, 2023 12:48

Merge remote-tracking branch 'upstream/trunk' into kip-714-ak-rpc

1209423

Merge remote-tracking branch 'upstream/trunk' into kip-714-ak-rpc

4565b3e

junrao reviewed Dec 1, 2023

View reviewed changes

Merge remote-tracking branch 'upstream/trunk' into kip-714-ak-rpc

704b61e

apoorvmittal10 closed this Dec 2, 2023

apoorvmittal10 reopened this Dec 2, 2023

Correcting tests to filter conditional telemetry keys

55ff34c

Fixing tests

3b8406a

apoorvmittal10 requested a review from junrao December 4, 2023 10:07

junrao closed this Dec 4, 2023

junrao reopened this Dec 4, 2023

apoorvmittal10 added 2 commits December 4, 2023 17:40

Merge remote-tracking branch 'upstream/trunk' into kip-714-ak-rpc

40ac93a

Merge remote-tracking branch 'upstream/trunk' into kip-714-ak-rpc

2865524

junrao merged commit 463ed09 into apache:trunk Dec 5, 2023

junrao mentioned this pull request Dec 5, 2023

KAFKA-15831: KIP-1000 protocol and admin client #14811

Merged

3 tasks

apoorvmittal10 deleted the kip-714-ak-rpc branch January 5, 2024 12:47

Uh oh!

Conversation

apoorvmittal10 commented Nov 15, 2023

Committer Checklist (excluded from commit message)

Uh oh!

Uh oh!

Uh oh!

apoorvmittal10 commented Nov 15, 2023

Uh oh!

Uh oh!

junrao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

apoorvmittal10 commented Nov 30, 2023

Uh oh!

AndrewJSchofield left a comment

Choose a reason for hiding this comment

Uh oh!

junrao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

junrao left a comment

Choose a reason for hiding this comment

Uh oh!

junrao commented Dec 2, 2023

Uh oh!

apoorvmittal10 commented Dec 3, 2023

Uh oh!

apoorvmittal10 commented Dec 3, 2023

Uh oh!

apoorvmittal10 commented Dec 3, 2023

Uh oh!

junrao commented Dec 3, 2023

Uh oh!

apoorvmittal10 commented Dec 3, 2023

Uh oh!

apoorvmittal10 commented Dec 4, 2023

Uh oh!

junrao commented Dec 4, 2023

Uh oh!

apoorvmittal10 commented Dec 4, 2023

Uh oh!

apoorvmittal10 commented Dec 4, 2023

Uh oh!

junrao commented Dec 4, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants