Hi Andrew,
Thanks for the review.
AS6: Add section about deprecating ListClientMetricsResourcesOptions.
AS7: Add section about replacing ApiKeys.LIST_CLIENT_METRICS_RESOURCES
with ApiKeys.LIST_CONFIG_RESOURCES.
Thanks for the reminder.
Regards,
PoAn
> On Apr 12, 2025, at 12:10 AM, Andrew Schofield
> <andrew_schofield_j...@outlook.com> wrote:
>
> Hi PoAn,
> I have re-read the KIP and have 2 final comments before I vote.
>
> AS6: You should deprecate ListClientMetricsResourceOptions.
>
> AS7: I anticipate that ApiKeys.LIST_CLIENT_METRICS_RESOURCES
> will be replaced by ApiKeys.LIST_CONFIG_RESOURCES.
>
> Thanks,
> Andrew
> ________________________________________
> From: Chia-Ping Tsai <chia7...@gmail.com>
> Sent: 04 April 2025 11:03
> To: dev@kafka.apache.org <dev@kafka.apache.org>
> Subject: Re: [DISCUSS] KIP-1142: Allow to list non-existent group which has
> dynamic config
>
>> chia_2: We can do the best to improve user experience. For v1 request, if
> the admin client receives
> UNSUPPORTED_VERSION error and the request body only contains client metric
> resource type, the admin client
> can fall back to use v0 request. In other cases, such as when the resource
> type is empty or includes different resource
> types, it returns the error to user.
>
> It sounds good!
>
> PoAn Yang <yangp...@gmail.com> 於 2025年4月4日 週五 下午5:53寫道：
>
>> Hi Chia-Ping,
>>
>> chia_2: We can do the best to improve user experience. For v1 request, if
>> the admin client receives
>> UNSUPPORTED_VERSION error and the request body only contains client metric
>> resource type, the admin client
>> can fall back to use v0 request. In other cases, such as when the resource
>> type is empty or includes different resource
>> types, it returns the error to user.
>>
>> Thanks,
>> PoAn
>>
>>> On Apr 4, 2025, at 4:00 PM, PoAn Yang <yangp...@gmail.com> wrote:
>>>
>>> Hi Andrew and Chia-Ping,
>>>
>>> Thanks for the review.
>>>
>>> AS5: My mistake. Remove the specific named entity. The “—describe
>> —entity-type groups” lists active and non-active
>>> groups which having configuration resources.
>>>
>>>> It seems that the new ListConfigResources RPC would be able
>>>> to list all config resources, regardless of whether the groups
>>>> are active or not.
>>> Yes, I added new function Admin#listConfigResources to use v1
>> ListConfigResources RPC to return all configuration
>>> resources even if the groups are non-active.
>>>
>>> chia_0: Updated `listConfigs` to `listConfigResources`.
>>>
>>> chia_1: Yes, v0 only returns the ClientMetricsResources. Add related
>> information to protocol files and RPC Compatibility
>>> in KIP.
>>>
>>> chia_2: If a broker only supports v0 request, the admin client should
>> send v0 request. I don’t prefer to rollback v1 request to v0 when the
>> request fails due to version match, because the meaning is different. The
>> v0 only lists client metrics
>>> resources. The v1 lists all configuration resources. If we ignore the
>> error and return client metrics resources which may make users
>> misunderstand. I will update ListClientMetricsResourcesRequest.java to set
>> oldestAllowedVersion and
>>> latestAllowedVersionas as 0. For ListConfigResourcesRequest.java, it
>> sets as 1.
>>>
>>> chia_3: Removed than from Public Interfaces section.
>>>
>>> chia_4: Added it to ListConfigResourceRequest.json.
>>>
>>> Thanks,
>>> PoAn
>>>
>>>> On Apr 4, 2025, at 1:55 PM, Chia-Ping Tsai <chia7...@apache.org> wrote:
>>>>
>>>> hi PoAn
>>>>
>>>> thanks for this kip. A couple of questions are listed below.
>>>>
>>>> chia_0:
>>>>
>>>> Should `listConfigs` be renamed to `listConfigResources` to match the
>> returned type?
>>>>
>>>> chia_1:
>>>>
>>>> v0 returns only the ClientMetricsResource, right? If so, could you
>> please add docs to the protocol file and KIP
>>>>
>>>> chia_2:
>>>>
>>>> Should we rollback to use v0 when the request fails due to version
>> match? I point that because users are suggested to use new method instead
>> of listClientMetricsResources, and hence they may encounter version error
>> if not all nodes are up-to-date.
>>>>
>>>> chia_3:
>>>>
>>>> `ListClientMetricsResourcesRequest` and
>> `ListClientMetricsResourcesResponse` are not public APIs, so it seems to me
>> it is unnecessary to "deprecate" them
>>>>
>>>> chia_4:
>>>>
>>>> could you please add "empty means all sources are returned" to
>> ListConfigResourcesRequest.json?
>>>>
>>>> Best,
>>>> Chia-Ping
>>>>
>>>>
>>>> On 2025/04/02 02:10:31 PoAn Yang wrote:
>>>>> Hi all,
>>>>>
>>>>> If there is no further discussion, I will start vote thread for
>> KIP-1142 tomorrow.
>>>>>
>>>>> Thanks,
>>>>> PoAn
>>>>>
>>>>>> On Mar 16, 2025, at 5:23 PM, PoAn Yang <yangp...@gmail.com> wrote:
>>>>>>
>>>>>> Hi Andrew,
>>>>>>
>>>>>> Thanks for the review.
>>>>>>
>>>>>> AS1: The kafka-client-metrics.sh —describe —name for a non-existent
>> client-metric shows default configuration.
>>>>>> The behavior is inconsistent with kafka-configs.sh in this KIP. I add
>> this case to proposed changes as well. Thanks.
>>>>>>
>>>>>> AS2: Thanks for the great suggestion. We have an API DescribeConfig
>> to get all kinds of configuration.
>>>>>> It’s better to have a API to list all types of configuration. I
>> change ListClientMetricsResources to ListConfigResources
>>>>>> and remove change for ListGroups. For backward compatibility, we also
>> need to make sure v0 ListConfigResources
>>>>>> can work as ListClientMetricsResources.
>>>>>>
>>>>>> AS3: Without change for ListGroupsResponse, we don’t need to handle
>> state and type fields in this KIP.
>>>>>>
>>>>>> AS4: Remove change for ListGroupsResponse.
>>>>>>
>>>>>> Thanks,
>>>>>> PoAn
>>>>>>
>>>>>>> On Mar 14, 2025, at 6:37 PM, Andrew Schofield <
>> andrew_schofield_j...@outlook.com> wrote:
>>>>>>>
>>>>>>> Hi PoAn,
>>>>>>> Thanks for the KIP. It would certainly be good to fix this slightly
>> odd behaviour in Kafka configs.
>>>>>>>
>>>>>>> AS1: I'd like to confirm that the KIP also alters the behaviour of
>> kafka-client-metrics.sh --describe
>>>>>>> for the case where the resource does not exist. You do show this in
>> an example, but I wanted to
>>>>>>> confirm that this reflects a behaviour change that this KIP will
>> enact.
>>>>>>>
>>>>>>> AS2: I wonder whether there's an alternative way to do this in the
>> protocol. In the way that
>>>>>>> you have specified it, if ListGroups specified
>> IncludeNonExistentGroupWithDynamicConfig,
>>>>>>> the returned list of groups comes from two sources - the groups in
>> the group coordinator
>>>>>>> and the group config resources in the metadata.
>>>>>>>
>>>>>>> I wonder whether actually you could rename the
>> ListClientMetricsResources RPC to
>>>>>>> ListConfigResources, bump it to version 1 with a schema like this:
>>>>>>>
>>>>>>> {
>>>>>>> "apiKey": 74,
>>>>>>> "type": "request",
>>>>>>> "listeners": ["broker"],
>>>>>>> "name": "ListConfigResourcesRequest",
>>>>>>> "validVersions": "0-1",
>>>>>>> "flexibleVersions": "0+",
>>>>>>> "fields": [
>>>>>>> { "name": "ResourceType", "type": "int8", "versions": "1+",
>> "mapKey": true,
>>>>>>> "about": "The resource type." }
>>>>>>> ]
>>>>>>> }
>>>>>>>
>>>>>>> For v0, the resource type is client metrics.
>>>>>>>
>>>>>>> Then, the response would be:
>>>>>>>
>>>>>>> {
>>>>>>> "apiKey": 74,
>>>>>>> "type": "response",
>>>>>>> "name": "ListConfigResourcesResponse",
>>>>>>> "validVersions": "0-1",
>>>>>>> "flexibleVersions": "0+",
>>>>>>> "fields": [
>>>>>>> { "name": "ThrottleTimeMs", "type": "int32", "versions": "0+",
>>>>>>> "about": "The duration in milliseconds for which the request
>> was throttled due to a quota violation, or zero if the request did not
>> violate any quota." },
>>>>>>> { "name": "ErrorCode", "type": "int16", "versions": "0+",
>>>>>>> "about": "The error code, or 0 if there was no error." },
>>>>>>> { "name": "ConfigResources", "type": "[]ConfigResource",
>> "versions": "0+",
>>>>>>> "about": "Each resource in the response.", "fields": [
>>>>>>> { "name": "Name", "type": "string", "versions": "0+",
>>>>>>> "about": "The resource name." }
>>>>>>> ]}
>>>>>>> ]
>>>>>>> }
>>>>>>>
>>>>>>> Then, this could list the group config resources (which is not the
>> same as listing the existing groups):
>>>>>>>
>>>>>>>> bin/kafka-configs.sh --describe --entity-type groups
>>>>>>>
>>>>>>> And perhaps, this could list the config for the existing groups
>> because it uses ListGroups to get the list of groups:
>>>>>>>
>>>>>>>> bin/kafka-configs.sh --describe --entity-type groups --effective
>>>>>>>
>>>>>>> The advantage to having ListConfigResources is that it would apply
>> to future resources too without needing
>>>>>>> additional RPCs. It also lets you distinguish between what's
>> configuration and what's live.
>>>>>>>
>>>>>>> Just a random idea for your consideration.
>>>>>>>
>>>>>>> AS3: A group which does not exist but is only represented by a
>> configuration resource has
>>>>>>> neither a type nor a state. What values will be used for these
>> fields in the ListGroupsResponse.
>>>>>>>
>>>>>>> AS4: The comment in ListGroupsResponse for v6 is incorrect. v6 adds
>> the
>>>>>>> NonExistentGroupWithDynamicConfig field.
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Andrew
>>>>>>>
>>>>>>> ________________________________________
>>>>>>> From: PoAn Yang <yangp...@gmail.com>
>>>>>>> Sent: 12 March 2025 16:00
>>>>>>> To: dev@kafka.apache.org <dev@kafka.apache.org>
>>>>>>> Subject: [DISCUSS] KIP-1142: Allow to list non-existent group which
>> has dynamic config
>>>>>>>
>>>>>>> Hi all,
>>>>>>>
>>>>>>> I would like to start a discussion thread on KIP-1142.
>>>>>>>
>>>>>>> Currently, kafka-configs.sh requires explicit group names to
>> describe configurations, which limits flexibility in managing dynamically
>> configured groups. This KIP proposes extending the ListGroups API to
>> include groups that do not exist in the coordinator but have dynamic
>> configurations.
>>>>>>>
>>>>>>> Please take a look and feel free to share any thoughts.
>>>>>>>
>>>>>>>
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1142%3A+Allow+to+list+non-existent+group+which+has+dynamic+config
>>>>>>>
>>>>>>> Thanks,
>>>>>>> PoAn
>>>>>>
>>>>>
>>>>>
>>>
>>
>>
