ability to filter topic's config options when listing topics

[d1egoaz]

related to #1514

Sarama ListTopics returns by default config options that are not default, however, it also returns brokers static configs, the java command line tool only returns by defaults the dynamic topic config options, however it also have the option to return all the configuration. This PR tries to offer an option to allow to get the required topic's configuration options.

This PR allows to use a new function ListTopicsAndFilterConfigEntries (please some help with the naming), which allows to pass a different filter, so we can get whatever combination that we'd like:

a few examples:
all options: kafka-configs.sh --bootstrap-server localhost:9092 --entity-type topics --entity-name xxxtopic --describe -all

	filter := func(entry *ConfigEntry) bool {
		return true
	}
        ListTopicsAndFilterConfigEntries(filter)

only sensitive:

	filter := func(entry *ConfigEntry) bool {
		return entry.Sensitive
	}

what java returs by default: kafka-configs.sh --bootstrap-server localhost:9092 --entity-type topics --entity-name xxxtopic --describe

	filter := func(entry *ConfigEntry) bool {
		return entry.Source == SourceTopic
	}

I didn't want to modify the current expectation of ListTopics as some people might be depending on current results

ping @sladkoff as I see you worked on #1594

[dnwe]

I'm not sure about introducing a new exported func for this specialisation of the behaviour. From the code it seems that the intention was never to return default or sensitive values in the response, so I would probably suggest that we correct that and just announce it in the changelog as a "potentially breaking change" for people to be aware of. I think we'd still be in the spirit of semver as the API remained stable.

If we wanted to, we could introduce a new ListTopicsWithOptions that just takes a simple struct of bools to change the behavior includeDefault, includeInternal etc.? That would be more in-live with the Java AdminClient

As an aside admin.go is already somewhat divergent from what the Java AdminClient does for listTopics in returning anything more than just the names of topics that exist as a []string. I expect the describeTopics might also be somewhat different too

[d1egoaz]

thanks for the feedback @dnwe

The reason that I'm not too keen to change the current behaviour of ListTopics, is that people might have created tooling around the current results, and if we change it, there is no other provided API that they could use to get the previous results.

I also thought about having only some bools to returns some data, like you mentioned includeDefault, however, this "default" looks is different for some people, for some it could be what the server has configured (current sarama default), for me, it could also means what server and broker has configured, that's why I decided to just provide a function where user could create their own filter. I think this could be more powerful than having hardcoded expectations like includeServerDefaults, includeBrokerDefaults, includeDynamicBroker, includeDynamicTopic, etc.

And as you mention, it looks like this has diverged now too much from the java AdminClient, so, not sure we'd need to follow their way of passing options, that looks is limited.

[dnwe]

The reason that I'm not too keen to change the current behaviour of ListTopics, is that people might have created tooling around the current results

As mentioned though — the code clearly never intended to include Default values in the response https://github.com/Shopify/sarama/blob/65f0fec86aabe011db77ad641d31fddf14f3ca41/admin.go#L371-L375 so personally I think it would be fine to correct the behaviour to match that intention, as long we mention as such in the changelog

and if we change it, there is no other provided API that they could use to get the previous results.

Well ListTopics in Sarama is essentially just a convenience wrapper around a MetadataRequest and a bulk DescribeConfigsRequest, with a slice of ConfigResource for all the topics, which are all things a user can technically call themselves.

We currently have an exported func for the DescribeConfig in admin.go — although it would have been better if it was a variadic function (something to change in 2.x maybe?) so it could take a slice of ConfigResources rather than only taking a single one like it does:

https://github.com/Shopify/sarama/blob/65f0fec86aabe011db77ad641d31fddf14f3ca41/admin.go#L57-L64

However, if it was only the broker-provided default config for topics that we start omitting in the ListTopics response, those would be accessible with a single DescribeConfig with a ConfigResource for broker config.

[d1egoaz]

@dnwe hey Dominic,

tl;dr: Sarama is ok, this PR is not needed.

I made the wrong assumption here:
Sarama does return what kafka-topics.sh returns, and I wrongly used kafka-configs.sh for the comparison.

kafka-topics.sh:

kafka-topics.sh --bootstrap-server localhost:9092 --topic xxx --describe
Topic: xxx    PartitionCount: 2       ReplicationFactor: 3    Configs: message.downconversion.enable=true,follower.replication.throttled.replicas=*,segment.bytes=104857600,retention.ms=259200000,message.format.version=2.0-IV1,max.message.bytes=1048576,unclean.leader.election.enable=false,retention.bytes=1610612736,segment.ms=28800000
        Topic: xxx    Partition: 0    Leader: 3016    Replicas: 3016,1013,2024        Isr: 1013,2024,3016
        Topic: xxx    Partition: 1    Leader: 1017    Replicas: 1017,2013,3006        Isr: 1017,2013,3006

sarama-ListTopics:

sarama.TopicDetail: {NumPartitions:2 ReplicationFactor:3 ReplicaAssignment:map[0:[3016 1013 2024] 1:[1017 2013 3006]] ConfigEntries:map[follower.replication.throttled.replicas:0xc000f7c310 max.message.bytes:0xc000f7c4d0 message.downconversion.enable:0xc000f7c210 message.format.version:0xc000f7c410 retention.bytes:0xc000f7c690 retention.ms:0xc000f7c390 segment.bytes:0xc000f7c350 segment.ms:0xc000f7c710 unclean.leader.election.enable:0xc000f7c650]}

the issue was that I used kafka-configs.sh, which by default only returns the topic dynamic configuration, which differ when kafka-topics.sh returns.