Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

order-dependent configuration failure with plugin.library.paths monitoring-interceptor #225

Closed
dtheodor opened this issue Sep 3, 2018 · 4 comments
Labels

Comments

@dtheodor
Copy link
Contributor

dtheodor commented Sep 3, 2018

Any plugin-specific configuration properties fail to be set if the plugin has not been registered with the property "plugin.library.paths", with an error that looks like this

panic: No such configuration property: "confluent.monitoring.interceptor.icdebug"

However, there's no way to specify ordering of configuration properties through a ConfigMap, which is just a map with keys that are returned in random order. When instantiating a client, there's a loop over the map's keys, and each of them is set through C.rd_kafka_conf_set which will fail immediately for any unrecognized property

func configConvertAnyconf(m ConfigMap, anyconf rdkAnyconf) (err error) {
for k, v := range m {

That means that about 50% of the time I am starting my program and I set any confluent.monitoring.interceptor.* property the program crashes with panic: No such configuration property because the confluent.monitoring.interceptor.* came before the "plugin.library.paths": "monitoring-interceptor" property during the iteration over the config keys.

Running librdkafka 0.11.5 and confluent-kafka-go version v0.11.4

@edenhill edenhill added the bug label Sep 3, 2018
@rnpridgeon
Copy link

Thanks for reporting this @dtheodor, we actually ran into this issue with the python client recently as well.

To fix this the property plugin.library.paths is intercepted and processed prior to handling any additional properties. I can get a fix in for this sometime this week or if you'd like you are more than welcome to submit a PR yourself.

Let me know,
Ryan

@dtheodor
Copy link
Contributor Author

dtheodor commented Sep 3, 2018

I can do a PR. Thinking of extending the following snippet with something like this

func configConvertAnyconf(m ConfigMap, anyconf rdkAnyconf) (err error) {
for k, v := range m {
switch v.(type) {

func configConvertAnyconf(m ConfigMap, anyconf rdkAnyconf) (err error) {
  // set the plugin.library.paths first
  paths, exist := m["plugin.library.paths"]
  if exist {
    anyconfSet(anyconf, "plugin.library.paths", paths)
    delete(m, "plugin.library.paths") // not sure if this is needed
  } 
  for k, v := range m {
    // .. the config property loop unchanged

@dtheodor
Copy link
Contributor Author

dtheodor commented Sep 3, 2018

Also an idea is not to impose ordering in the client libraries, but fix this in librdkakfa, by exposing some "finalize configuration" function that will collect all config values set through rd_kafka_conf_set, validate them, and apply any side effects in the right order

@rnpridgeon
Copy link

I had a similar discussion with @edenhill while fixing the Python client. In the end it was decided that the client bindings should handle this.

That said @edenhill is a very reasonable man so you may be able to convince him otherwise. I recommend opening an issue for discussion within the librdkafka repo. Go ahead and submit a PR for the client binding though so we can get this fixed sooner rather than later. We can always back the fix out later if you can convince Magnus to solve this in librdkafka.

edenhill pushed a commit that referenced this issue Sep 4, 2018
…dtheodor)

* Make sure plugins are set before other configuration options

Any plugin-specific configuration can only be set after the plugin
has itself been set, so make sure plugins are set first

* add confluent-librdkafka-plugins to travis

* skip test if monitoring-interceptor shared library is missing

* travis: only install confluent-librdkafka-plugins on linux

* update error description
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants