[KIP-848]: Add testing changes and describe consumer group changes for KIP-848 #329

PratRanj07 · 2025-06-06T08:16:57Z

This PR intends to make changes to run Performance Tests on Semaphore with the newly introduced Consumer Group Protocol (KIP 848).

Copilot

Pull Request Overview

Adds support for running performance tests with the new Consumer Group Protocol (KIP-848) alongside the classic protocol by extending test helpers, consumer configuration, and CI setup.

Introduces TEST_CONSUMER_GROUP_PROTOCOL env var and helper functions to toggle protocol behavior in tests.
Updates createConsumer and internal config mapping to validate and strip classic-only settings when using the new protocol.
Extends Docker Compose files and Semaphore CI pipeline to run perf tests under both classic and consumer protocols.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
test/promisified/testhelpers.js	Adds `testConsumerGroupProtocol*` helpers and adjusts `createConsumer` to apply/remove settings based on protocol
test/docker/kraft/server.properties	Configures KRaft broker with consumer group protocol settings
test/docker/docker-compose-kraft.yml	Provides a Docker Compose definition for the KRaft broker used in consumer-protocol tests
lib/kafkajs/_consumer.js	Extends `#kafkaJSToConsumerConfig` to accept an `isClassicProtocol` flag and enforce validations
.semaphore/semaphore.yml	Adds two Semaphore CI jobs running perf tests for classic and consumer protocols

Comments suppressed due to low confidence (3)

test/promisified/testhelpers.js:18

Add a JSDoc or inline comment explaining that TEST_CONSUMER_GROUP_PROTOCOL should be set to either 'classic' or 'consumer', and what the helper returns in each case.

function testConsumerGroupProtocol() {

lib/kafkajs/_consumer.js:515

For non-classic protocols, no default partition.assignment.strategy is set. If this is intentional, consider adding a comment to clarify fallback behavior or explicitly document the absence of a default strategy.

} else if (isClassicProtocol) {

.semaphore/semaphore.yml:170

This line and the following commands lack a '+' prefix in the diff, so they won’t be added under the new consumer-protocol job. Ensure the NODE_OPTIONS, working directory change, and npm install commands are included in the patch.

              - export NODE_OPTIONS='--max-old-space-size=1536'

Copilot · 2025-06-06T08:19:57Z

test/promisified/testhelpers.js

@@ -129,6 +155,7 @@ class SequentialPromises {
 }

 module.exports = {
+    testConsumerGroupProtocolClassic,


[nitpick] Since testConsumerGroupProtocol is defined but not exported, either export it for clarity or rename it (e.g., _testConsumerGroupProtocol) to indicate it’s intended as a private helper.

… groups

* Added new fields in DescribeConsumerGroup * error debug * test changes * test enabled

milindl

Majority of changes look good. I tested locally with AK 4 also. Some comments to address here and here.
Please add a CHANGELOG.md entry

milindl · 2025-06-30T04:49:09Z

lib/kafkajs/_consumer.js

@@ -605,8 +621,10 @@ class Consumer {
  }

  #finalizedConfig() {
+    const protocol = this.#userConfig['group.protocol'];
+    const isClassicProtocol = protocol === undefined || protocol === 'classic';


this comparison should be case insensitive

test/promisified/admin/describe_groups.spec.js

milindl · 2025-06-30T04:51:57Z

test/promisified/consumer/incrementalRebalance.spec.js

@@ -12,7 +13,9 @@ describe('Consumer > incremental rebalance', () => {

    const consumerConfig = {
        groupId,
-        partitionAssigners: [PartitionAssigners.cooperativeSticky],
+        ...(testConsumerGroupProtocolClassic() && {


this syntax is quite hard to read, do it with an if block instead later

milindl · 2025-07-01T04:31:15Z

examples/kafkajs/admin/describe-groups.js

@@ -1,5 +1,5 @@
 // require('kafkajs') is replaced with require('@confluentinc/kafka-javascript').KafkaJS.
-const { Kafka, ConsumerGroupStates } = require('@confluentinc/kafka-javascript').KafkaJS;
+const { Kafka, ConsumerGroupStates, ConsumerGroupTypes } = require('@confluentinc/kafka-javascript').KafkaJS;


nit: since we're not actually using it, why have it here?

milindl · 2025-07-01T05:30:34Z

test/promisified/admin/describe_groups.spec.js

@@ -81,11 +82,12 @@ describe('Admin > describeGroups', () => {
        expect(describeGroupsResult.groups[0]).toEqual(
            expect.objectContaining({
                groupId,
-                protocol: 'roundrobin',
-                partitionAssignor: 'roundrobin',
+                protocol: expect.any(String),


is it possible to decide the actual value beforehand, stored it in a variable and replace it with 'expectedProtocol' or something like that?

milindl · 2025-07-01T11:38:20Z

CHANGELOG.md

+## Enhancements
+
+1. References librdkafka v2.11.0. Refer to the [librdkafka v2.11.0 release notes](https://github.com/confluentinc/librdkafka/releases/tag/v2.11.0) for more information.
+2. [KIP-848] `describeGroups()` now supports KIP-848 introduced `consumer` groups. Two new fields for consumer group type and target assignment have also been added. Type defines whether this group is a `classic` or `consumer` group. Target assignment is only valid for the `consumer` protocol and its defaults to nil.


its defaults to nil

change to

"it defaults to being undefined"

sonarqube-confluent · 2025-07-01T12:00:42Z

62.50% Coverage on New Code (is less than 80.00%)

Analysis Details

2 Issues

0 Bugs
0 Vulnerabilities
2 Code Smells

Coverage and Duplications

62.50% Coverage (47.90% Estimated after merge)
No duplication information (2.00% Estimated after merge)

Project ID: confluent-kafka-javascript

View in SonarQube

Copilot AI review requested due to automatic review settings June 6, 2025 08:16

PratRanj07 requested review from a team as code owners June 6, 2025 08:16

Copilot AI reviewed Jun 6, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

PratRanj07 added 12 commits June 30, 2025 10:39

Run perf tests for consumer protocol

7e534b0

changed docker location for Promisified tests

821058b

Run promisified tests with the new protocol

9bb1418

Added kafka_jaas.conf

598fc26

Remove forbidden properties from KAFAKJS configs too

3b609c0

Skip for partitionAssigners as well

663a602

Increase test timeout

125a548

increase sleep timeout and expect any strings in protocol in describe…

33eb38f

… groups

increase test timeout

8c51e8e

set session timeout in server properties

e624c99

set session timeout 10 secs

5767f5a

Skip oauthbreaker tests

6b868fa

airlock-confluentinc bot force-pushed the dev_kip848Testing branch from 334c1d2 to 6b868fa Compare June 30, 2025 05:10

This comment has been minimized.

Sign in to view

Skip all oauthbreaker_cb tests

5b05c28

This comment has been minimized.

Sign in to view

docker logging

e18c470

This comment has been minimized.

Sign in to view

increase timeout to 60

2a6b7d8

This comment has been minimized.

Sign in to view

pin versions

41a30c5

This comment has been minimized.

Sign in to view

PratRanj07 added 2 commits June 30, 2025 13:29

Remove logging and decrease sleep time

dee9afd

Change coverage file name

8db15b5

This comment has been minimized.

Sign in to view

[KIP-848]: Structure change for DescribeConsumerGroups (#327)

4bd627d

* Added new fields in DescribeConsumerGroup * error debug * test changes * test enabled

This comment has been minimized.

Sign in to view

milindl requested changes Jul 1, 2025

View reviewed changes

PratRanj07 added 4 commits July 1, 2025 15:21

requested changes

7a0e1cc

Test fix

3d89848

Test fix

c7d2ba0

Changelog and test fix

8ceb546

This comment has been minimized.

Sign in to view

milindl approved these changes Jul 1, 2025

View reviewed changes

Changelog change

704d1df

PratRanj07 changed the title ~~[KIP-848]: Run perf tests for consumer protocol~~ [KIP-848]: Add testing changes and describe consumer group changes for KIP-848 Jul 1, 2025

milindl approved these changes Jul 1, 2025

View reviewed changes

PratRanj07 merged commit 206347c into master Jul 1, 2025
1 of 2 checks passed

PratRanj07 deleted the dev_kip848Testing branch July 1, 2025 11:59

[KIP-848]: Add testing changes and describe consumer group changes for KIP-848 #329

[KIP-848]: Add testing changes and describe consumer group changes for KIP-848 #329

Uh oh!

Conversation

PratRanj07 commented Jun 6, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

milindl left a comment

Choose a reason for hiding this comment

Uh oh!

milindl Jun 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

milindl Jun 30, 2025

Choose a reason for hiding this comment

Uh oh!

milindl Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

milindl Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

milindl Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonarqube-confluent bot commented Jul 1, 2025

Analysis Details

2 Issues

Coverage and Duplications

Uh oh!

Uh oh!