kafka-rest icon indicating copy to clipboard operation
kafka-rest copied to clipboard

Add feature to request consumer group information

Open Ledostuff opened this issue 7 years ago • 35 comments

Add feature to request consumer group information(start, end offsets, lags, consumer-instance-id, consumer-instance-ip) Added rest paths:

  1. /groups - returns consumer groups list
[{"groupId":"perf-consumer-46894","coordinator":{"host":"{some_host_name}","port":9496}}]
  1. /groups/{groupId}/topics - returns topics for wich group is subscribed
[{"name":"1"}]
  1. /groups/{groupId}/topics/{topic} - returns consumer group information(start, end offsets, lags, consumer-instance-id, consumer-instance-ip...) restricted by spesified topic
{"topicPartitionList":[{"consumerId":"consumer-1-88792db6-99a2-4064-aad2-38be12b32e88","consumerIp":"/{some_ip}","topicName":"1","partitionId":0,"currentOffset":15338734,"lag":113812,"endOffset":15452546},{"consumerId":"consumer-1-88792db6-99a2-4064-aad2-38be12b32e88","consumerIp":"/{some_ip}","topicName":"1","partitionId":1,"currentOffset":15753823,"lag":136160,"endOffset":15889983},{"consumerId":"consumer-1-88792db6-99a2-4064-aad2-38be12b32e88","consumerIp":"/{some_ip}","topicName":"1","partitionId":2,"currentOffset":15649419,"lag":133052,"endOffset":15782471}],"topicPartitionCount":3,"coordinator":{"host":"{coordinator_host_name}","port":9496}}
  1. /groups/{groupId}/partitions - returns consumer group information(start, end offsets, lags, consumer-instance-id, consumer-instance-ip...) for all topics for wich group is subscribed
{"topicPartitionList":[{"consumerId":"consumer-1-88792db6-99a2-4064-aad2-38be12b32e88","consumerIp":"/{some_ip}","topicName":"1","partitionId":0,"currentOffset":15338734,"lag":113812,"endOffset":15452546},{"consumerId":"consumer-1-88792db6-99a2-4064-aad2-38be12b32e88","consumerIp":"/{some_ip}","topicName":"1","partitionId":1,"currentOffset":15753823,"lag":136160,"endOffset":15889983},{"consumerId":"consumer-1-88792db6-99a2-4064-aad2-38be12b32e88","consumerIp":"/{some_ip}","topicName":"1","partitionId":2,"currentOffset":15649419,"lag":133052,"endOffset":15782471}],"topicPartitionCount":3,"coordinator":{"host":"{coordinator_host_name}","port":9496}}

Add unit and integration tests

Ledostuff avatar Jun 04 '18 05:06 Ledostuff

It looks like @Ledostuff hasn't signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence. Wikipedia

You can read and sign our full Contributor License Agreement here.

Once you've signed reply with [clabot:check] to prove it.

Appreciation of efforts,

clabot

ghost avatar Jun 04 '18 05:06 ghost

[clabot:check]

Ledostuff avatar Jun 04 '18 07:06 Ledostuff

@confluentinc It looks like @Ledostuff just signed our Contributor License Agreement. :+1:

Always at your service,

clabot

ghost avatar Jun 04 '18 07:06 ghost

@Ledostuff PR build is failing. Could you check?

gAmUssA avatar Sep 13 '18 03:09 gAmUssA

The build failed:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.6.1:testCompile (default-testCompile) on project kafka-rest: Compilation failure
[ERROR] /home/jenkins/workspace/entinc-pr_kafka-rest_PR-435-42EK4ZKFX7WUDVKYQNLWCJBUA3J5IBRM4CNHID3FE32PTTTFMBRQ/kafka-rest/src/test/java/io/confluent/kafkarest/TestUtils.java:[263,34] cannot find symbol
[ERROR]   symbol:   method createConsumerProperties(java.lang.String,java.lang.String,java.lang.String,int)
[ERROR]   location: class kafka.utils.TestUtils
[ERROR] -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <goals> -rf :kafka-rest

cmccabe avatar Sep 13 '18 18:09 cmccabe

This PR shouldn't need to depend on ZooKeeper. We should be able to use the public AdminClient.

What's the security model here? How do we decide who can and cannot access admin APIs?

cmccabe avatar Sep 13 '18 18:09 cmccabe

@gAmUssA Thank you for review. I`ll fix build and push changes asap.

Ledostuff avatar Sep 14 '18 05:09 Ledostuff

@cmccabe Thank you for review. I`ll fix build and push changes asap. Zookeeper needs to get broker information. I mean security protocol in EndPoint class. Do you mean I should move method from AdminClientWrapper to another place? Could you explain in detail your sentence about security model?

Ledostuff avatar Sep 14 '18 06:09 Ledostuff

Zookeeper needs to get broker information. I mean security protocol in EndPoint class.

Why is this information needed for REST proxy? If it is needed, then we should at least think about providing it through the AdminClient.

Could you explain in detail your sentence about security model?

This PR seems to give rest-proxy users a lot of powerful abilities to access administrative APIs. I'm not sure that rest-proxy is the right place to manage the Kafka cluster.

If we did want to do this we would need to figure out how to give certain users the ability to use the admin APIs and block everyone else. For example, users should not be able to list groups unless they have DESCRIBE permission on the group, or DESCRIBE permission on the Cluster resource.

To be honest, I'm not sure this is the right direction to go-- at least not without a lot more discussion.

cmccabe avatar Sep 14 '18 19:09 cmccabe

@cmccabe Thank you for the answer.

This PR seems to give rest-proxy users a lot of powerful abilities to access administrative APIs. I'm not sure that rest-proxy is the right place to manage the Kafka cluster.

We are only show information about groups who consumes messges from clusters topics. We don`t try to change settings or do any other work with cluster.

If we did want to do this we would need to figure out how to give certain users the ability to use the admin APIs and block everyone else. For example, users should not be able to list groups unless they have DESCRIBE permission on the group, or DESCRIBE permission on the Cluster resource.

I don't see any problem about that. Because we allow users to consume messages through the rest-proxy. But why do we can't show information about consumer groups? Even if we allow users DESCRIBE permission on groups or DESCRIBE permission on the Cluster resource . I'm open for discussion this PR)

Why is this information needed for REST proxy? If it is needed, then we should at least think about providing it through the AdminClient.

I didn't found similar method in the API of AdminClient. If you show me alternative method to get information about brokers in cluster with security protocol, I'll repair code in this PR gladly.)

Ledostuff avatar Sep 19 '18 09:09 Ledostuff

Hey @Ledostuff, thanks for the PR!

I haven't had time to give this a proper look yet but I wanted to ask - do we need ZooKeeper to access the cluster information only ( {"brokerId":0,"endPointList":[{"protocol":"SSL","host":"127.0.0.1","port":9092}]}) ? In regards to that, I feel it would be best if we do not re-introduce a ZooKeeper dependancy in the proxy.

The consumer information should be fetchable through the adminClient only, correct? To me, it makes sense to expose consumer information. I think endpoints 2., 3., 4. and 5. will be useful.

There is a question of whether that information should be limited to consumers managed by this proxy instance or consumers in the Kafka Cluster. As @cmccabe said, permissions are a valid concern:

For example, users should not be able to list groups unless they have DESCRIBE permission on the group, or DESCRIBE permission on the Cluster resource. As far as I know, in the current model we allow a consumer to be created and join any arbitrary group present in the Kafka cluster. That means exposing consumer group information for the whole cluster would be consistent with the security model already present.

stanislavkozlovski avatar Nov 14 '18 11:11 stanislavkozlovski

Hi @stanislavkozlovski,

"in the current model we allow a consumer to be created and join any arbitrary group present"

No, we don't. This is only valid for the case where authentication and acl's are off OR we elevate kafka-rest principal's resource permissions like READ/DESCRIBE on cluster/group/topic, depending on the operation type we want. We are free to choose which ACLs to give to kafka-rest's principal, should consumer groups be exposed or not. That is, users are free to choose their own security model.

users should not be able to list groups unless they have DESCRIBE permission on the group, or DESCRIBE permission on the Cluster resource.

That's right, unless we provide DESCRIBE permission on the Cluster resource.

Viewing consumer group offsets or listing consumer groups is just a feature that can be enabled or disabled, according to a security model adapted by the user.

rootex- avatar Nov 22 '18 12:11 rootex-

Hi @rootex-,

That's true. I agree with exposing consumer group information.

stanislavkozlovski avatar Nov 23 '18 15:11 stanislavkozlovski

Not to be annoying, but... is there still interest in this? I'd welcome having a list of consumer groups available via REST proxy.

analytik avatar Sep 20 '19 07:09 analytik

Not to be annoying, but... is there still interest in this? I'd welcome having a list of consumer groups available via REST proxy.

Yes, there is still a huge interest in this feature. So much useful work done and no one have ever reviewed this stuff. We are also sick and tired of endless patching Kafka Rest in Sberbank.

Could we merge this pull request?

rootex- avatar Sep 20 '19 12:09 rootex-

Hello there! Please review this PR. Thanks)

Ledostuff avatar Sep 25 '19 10:09 Ledostuff

Hi @stanislavkozlovski, Please, review my changes. Sorry for the delay)

Ledostuff avatar Sep 25 '19 11:09 Ledostuff

@stanislavkozlovski , All notes are fixed. Will you be able review my PR again? Thanks)

Ledostuff avatar Oct 04 '19 03:10 Ledostuff

Yes, I will try to review it fully this week

stanislavkozlovski avatar Oct 07 '19 16:10 stanislavkozlovski

@stanislavkozlovski I've just fixed all notes. Review this PR again, please. Do you agree with my response about duplication consumerId and host for every topic-partition subscription information?

Ledostuff avatar Oct 10 '19 09:10 Ledostuff

I want to discuss the data model and what we return a bit more in depth. Would you mind commenting on this PR each new endpoint and an example of what it returns?

stanislavkozlovski avatar Oct 11 '19 22:10 stanislavkozlovski

I want to discuss the data model and what we return a bit more in depth. Would you mind commenting on this PR each new endpoint and an example of what it returns?

Hi, @stanislavkozlovski. I have already done it - link. Or do you mean something else?

Ledostuff avatar Oct 14 '19 06:10 Ledostuff

@stanislavkozlovski, @rigelbm Please review may changes. To my mind all specified notes are fixed.

Ledostuff avatar Nov 01 '19 05:11 Ledostuff

Hello guys. Could anybody review my changes?

Ledostuff avatar Nov 11 '19 08:11 Ledostuff

I will do a second pass today. Sorry for the delay.

rigelbm avatar Nov 11 '19 08:11 rigelbm

I will do a second pass today. Sorry for the delay.

Guys, do you have any notes to my PR?

Ledostuff avatar Nov 21 '19 08:11 Ledostuff

Guys, we need this update, please review 🙏🏼

rootex- avatar Nov 21 '19 14:11 rootex-

retest, please

Ledostuff avatar Nov 22 '19 05:11 Ledostuff

@stanislavkozlovski Have you seen my comments to your notes?

Ledostuff avatar Dec 03 '19 04:12 Ledostuff

Helooooo) Any suggestions?

Ledostuff avatar Dec 05 '19 05:12 Ledostuff