nickbabcock/collectd-rust-plugin

Write a low-cost, ergonomic plugin for collectd

collectd-rust-plugin's Issues

Support Custom Chains

Collectd allows one to write custom filters / chains. See fc_register_match and fc_register_target in filter_chain.h. While I would like collectd_plugin to support chains, I don't have a use case so I haven't set aside any time to solve this issue.

It may be best to add a couple methods to PluginManager:

fn matchers(_config: Option<&[ConfigItem]>) -> Result<Vec<Matcher>, Error> {
    Vec::new()
}

fn targets(_config: Option<&[ConfigItem]>) -> Result<Vec<Target>, Error> {
    Vec::new()
}

Use macros for module registration

Just looking at the example on your README, it seems like you could benefit by hiding the module registration behind a macro. I did something similar for the plugins section of my Rust FFI guide, feel free to steal ideas from there 😄

So you could replace this:

#[no_mangle]
pub extern "C" fn module_register() {
    // The entry function for our plugin. Our registered read function will be called at
    // intervals defined by collectd
    let s = CString::new("myplugin").unwrap();
    unsafe {
        plugin_register_read(s.as_ptr(), Some(my_read));
    }
}

With something like this:

register_plugin!("myplugin", my_read);

Which may internally expand to:

#[no_mangle]
pub extern "C" fn module_register() {
  if let Ok(s) = CString::new("myplugin") {
    unsafe {
      plugin_register_read(s.as_ptr(), Some(my_read));
    }
  } 
}

Serde support for node values

The real write_graphite plugin uses this configuration

<Plugin write_graphite>
       <Node "example">
               Host "localhost"
               Port "20030"
       </Node>
</Plugin>

Where the name it is registered under is "example". To use serde for automatic deserialization of config items, the following structure would be preferred:

<Plugin write_graphite>
       <Node>
               Name "example"
               Host "localhost"
               Port "20030"
       </Node>
</Plugin>

Ideally the former structure would be serde supported, but I am not going to worry too much if not.

Add notification callback

Collectd exposes plugin_register_notification for plugins to register into. Those who want alert like capabilities built into this plugin should implement this 😄

Expose public error types

The following errors are not publicly exposed:

• ConfigItem::from uses ConfigError
• ValueList::rates uses CacheRateError
• ValueList::from uses ReceiveError
• ValueListBuilder::from uses SubmitError

These errors should be exported publicly so that users can have an additional recourse instead of propagating / logging

Catch Rust Panics

A properly written plugin will not panic (instead, it will return an Error). Still, mistakes happen and catching panics is the polite thing to do for collectd (collectd should still run if a rust plugin panics). I looked into panic::catch_unwind, but received the following error:

let read_panic = catch_unwind(|| plugin.read_values());
                 ^^^^^^^^^^^^ the type &mut std::boxed::Box<std::boxed::Box<plugins::Plugin>> may not be safely transferred across an unwind boundary

I'm not sure what the solution is here 🤔

Parsing value lists?

Hi, I'm new to collectd, so this might be completely wrong!

As far as I understand, collectd can submit collected data via MQTT for example. It does sovia the MQTT plugin, which serializes data into a value list.

So if I want to be able to read this data, I would need to parse this "value list" data structure into a rust type to be able to work with it conveniently. Is that right?

As far as I can see, this crate does not provide functionality for parsing value lists, only for building them. Would it be possible to add such functionality to this crate?

---

I understand if this is out of scope for this crate, of course!

Update for Collectd 5.8

Collectd 5.8 is out, which means the bindings may have to updated. I'll take a look at the latest collectd-dev package

Better Build System

Currently someone can specify multiple collectd versions as features: cargo build --features 'collectd-55 collectd-57' -p readme

The build system:

• Should know not to allow multiple mutually exclusive features
• Allow a cargo package to work

Remove failure dependency

While initially promising, failure seems to be too heavy-handed of a requirement to force onto the user. There seems to be some sort of consensus that failure should be used only for applications.

References:

• Do I really need failure/error-chain?
• Current state of error handling in Rust?
• Redefining Failure
• Goodbye failure

I'm not sure what would be the most appropriate replacement. Potentially adding a trait so that a plugin's errors are Debug + Display. RFC 2504 also proposes changes to the error trait, which may be a more convenient. Though, on the other hand, I can see a use case where if a plugin handles all errors (ie: specialized logging statements) that it would want to return an empty tuple (implements debug but not display) (ideally suited for the never type). Sounds hard to create an API that is one size fits all.

On Thread Safety

It's my understanding that collectd will send multiple requests to the write callback concurrently. I need to somehow force Plugin implementations to be (afaik) Sync. Someone should not be able to define:

struct MyPlugin<T: Write> {
   writer: BufWriter<T>
}

impl Plugin for MyPlugin {
}

As MyPlugin.writer is not thread safe