- Fix null collectd values causing a panic
- Fix 32-bit compilation errors
- Prefer
SeqCstordering in abundance of caution - Update dependencies:
- Bindgen from 58.1 to 69.1
- Bitflags from 1.0 to 2.4
- env_logger from 0.8 to 0.10
- Submit and receive metadata values through
ValueListBuilder::metadataandValueList::metarespectively - Only collectd 5.7+ is supported (ie: 5.4 and 5.5 API versions have been dropped). This means that
COLLECTD_VERSIONhas a default fallback and is no longer required. - Collectd 5.12 now supported
- Some error structs migrated to named fields and away from tuple structs
env_loggerupdated to 0.8
- Add
PluginManager::shutdownto clean up resources allocated inPluginManager::initialize - Expose additional public error types:
- ConfigError
- CacheRateError
- ReceiveError
- SubmitError
- Disable additional logging features by default. The only env_logger feature related to filtering (what collectd-plugin enables) is the regex feature, so that feature can be enabled through the
regex_log_filterfeature. The rest of disabled to keep dependencies to a minimum - Allow autodetection of collectd version to work with 5.10 and 5.11
- Update bindgen requirement from 0.51.0 to 0.53.1
- Support collectd-5.9 through 5.7 interface (so
COLLECTD_VERSION=5.7can bind to collectd-5.9) - Upgrade to env_logger 0.7 from 0.6
- Bump to 2018 edition of rust
- Bump minimum required rust to 1.33
- Add additional include path when linking against a custom collectd version
- Bindgen updated from 0.47.0 to 0.51.0
- Compile on non-x86 platforms
- Add
COLLECTD_PATHenvironment variable for detecting collectd version from collectd's source directory (most useful with thebindgenfeature). - Output panic info into collectd logs
Big release with a couple backwards incompatible changes. Let's break it down.
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 for libraries.
References:
- Do I really need failure/error-chain?
- Current state of error handling in Rust?
- Redefining Failure
- Goodbye failure
All failure::Error should be replaced with Box<error::Error>. A change in the type signature may seem scary but the work entailed to migrate should be minimal. For instance, here is the (condensed) diff for migrating the readme example to the new format.
- extern crate failure;
- use failure::Error;
+ use std::error;
- fn plugins(_config: Option<&[ConfigItem]>) -> Result<PluginRegistration, Error> {
+ fn plugins(_config: Option<&[ConfigItem]>) -> Result<PluginRegistration, Box<error::Error>> {
- fn read_values(&self) -> Result<(), Error> {
+ fn read_values(&self) -> Result<(), Box<error::Error>> {Removing the failure crate cut compile times and the dependency tree in half.
It is currently undefined behavior to unwind from Rust code into foreign code, so [catching panics] is useful when Rust is called from another language (normally C). This can run arbitrary Rust code, capturing a panic and allowing a graceful handling of the error. [source]
Panics can be caused by silly mistakes like
let a = Instant::now();
// ...
a.duration_since(Instant::now()); // will panic!This would cause a SIGABRT in collectd, and no one likes a crashing program.
By forcing Plugin to be UnwindSafe + RefUnwindSafe, we can catch panics before they cross the ffi border. Instead of crashing, collectd will log an error. Most plugins should already be UnwindSafe + RefUnwindSafe.
Collectd plugin is doubling down on Rust's native logging. Now, if registered, all errors (even if they originated in between collectd and the plugin), will use prefer Rust's logging. If not registered, errors are instead sent directly to collectd and may lack context.
Given the following plugin that alternates between panicking and returning an error:
#[derive(Default)]
struct MyErrorManager;
#[derive(Default)]
struct MyErrorPlugin {
state: AtomicBool,
}
impl PluginManager for MyErrorPlugin {
fn name() -> &'static str {
"myerror"
}
fn plugins(_config: Option<&[ConfigItem]>) -> Result<PluginRegistration, Box<error::Error>> {
CollectdLoggerBuilder::new()
.prefix_plugin::<Self>()
.filter_level(LevelFilter::Info)
.try_init()
.expect("really the only thing that should create a logger");
Ok(PluginRegistration::Single(Box::new(
MyErrorPlugin::default(),
)))
}
}
impl Plugin for MyErrorPlugin {
fn capabilities(&self) -> PluginCapabilities {
PluginCapabilities::READ
}
fn read_values(&self) -> Result<(), Box<error::Error>> {
if self.state.fetch_xor(true, Ordering::SeqCst) {
panic!("Oh dear what is wrong!?")
} else {
Err(failure::err_msg("bailing"))?
}
}
}One will find the following lines in the log:
myerror: collectd_plugin::api::logger: read error: plugin encountered an error; bailing
read-function of plugin `myerror' failed
myerror: collectd_plugin::api::logger: read error: plugin panicked
Nested causes will be semi-colon joined to be printed on a single line.
Collectd will back off exponentially on failing plugins.
- Migrate most of
collectd_plugin!macro internally. This reduces the number lines in the macro from 300 to 30 - All of the internal logic used in
collectd_plugin!is no longer exposed as a public API, so this drastically curtails the public API. If part of the API was removed that you depended upon, open an issue to discuss. Do not depend on anything in theinternalmodule - Guard against null messages panicking logging plugins (though null messages should never happen)
- Add: serde config deserialize newtype structs
- Add: serde config deserialize unit variant enums (like
logcrateLogLevel) - Improve error message on build failure to suggest providing the
COLLECTD_VERSIONenvironment variable if missing - Simplify rust logging delegation by cutting out
Cursorin preference of writing aVec<u8>directly. Could yield a small performance increase for log heavy workloads. - Update env_logger from 0.5 to 0.6 (no behavioral changes)
- Fix double free segfault on shutdown for plugins that register more than on callback (often times write + flush).
collectd-rust-plugindelegates to a plugin's drop implementation on shutdown to make sure all resources are cleaned up. The previous behavior would have collectd calling a plugin's drop implementation for each callback register. Dropping the same plugin twice is undefined behavior and would often segfault. Collectd recommends only registering the drop function once to avoid a double free scenario.collectd-rust-pluginnow understands that when multiple callbacks are desired, to only execute drop on one of them.
- Fix segfault on plugins that implement flush when given a null identifier. The proper behavior now includes a check to see if the identifier is
NULL(and convert it to anOptionappropriately) before interpretting it as astr.
- Compatibility with collectd 5.8. Collectd 5.8 changed the signature of
hostname_gso it can no longer be submitted as a host value. We allowed it and resulted in collectd interpretting garbage. The fix is to switch to submitting empty arrays for collectd 5.7+, which will default tohostname_ginternally.
This is a breaking change that is going to affect everyone. It's a bummer, but hopefully by the end of this blurb, you will be convinced that the breaking change is worth it.
For background, collectd has the right to call any of the Plugin methods in parallel and concurrently. Previously, a Plugin only needed to be Sync (a plugin is safe to have its references shared between threads), but this allowed for undefined behavior with code that isn't thread safe. The compiler only allowed it as transcended unsafe (ffi) boundaries. We can do better. Send is now required (so a plugin can be transferred across thread boundaries). We don't control collectd and how they manage our Plugin, so if collectd offloads our plugin to another thread, the plugin functionality must remain the same.
Also all Plugin methods now force interior mutability by changing &mut self to &self to ensure thread safety in conjunction with the Sync + Send requirement. For instance, Vec<u8> is Sync + Send but one can't Vec::push across multiple threads, so synchronization primitives will now be required if mutability is desired.
Everything is more clear with examples. Here is the error one will receive if they attempt to mutate something that is not thread safe (this code was previously allowed)
#[derive(Debug, Default)]
pub struct MyPlugin {
names: HashSet<String>,
}
impl Plugin for MyPlugin {
fn read_values(&self) -> Result<(), Error> {
self.names.insert(String::from("A"));
}
}The compiler error:
error[E0596]: cannot borrow field `self.names` of immutable binding as mutable
--> src/plugin.rs:102:9
|
97 | fn read_values(&self) -> Result<(), Error> {
| ----- use `&mut self` here to make mutable
...
102 | self.names.insert(String::from("A"));
| ^^^^^^^^^^ cannot mutably borrow field of immutable binding
The fix is to use a synchronization primitive like a Mutex, which will allow one to mutate the inner data by taking an exclusive lock on the data.
#[derive(Debug, Default)]
pub struct MyPlugin {
names: Mutex<HashSet<String>>,
}
impl Plugin for MyPlugin {
fn read_values(&self) -> Result<(), Error> {
let mut n = self.names.lock().unwrap();
n.insert(String::from("A"));
}
}Now if collectd happens to call read_values in parallel, there is no way we would stumble into undefined behavior, as the mutex ensures that thread B waits until thread A is done with the data (names in this instance). Mutex may not be right for you, so make sure employ correct synchronization primitives
As a final demostration of why the changes were necessary the following should not compile:
#[derive(Debug, Default)]
pub struct MyPlugin {
names: RefCell<HashSet<String>>,
}
impl Plugin for MyPlugin {
fn read_values(&self) -> Result<(), Error> {
let mut n = self.names.borrow_mut();
n.insert(String::from("A"));
}
}RefCell does not implement Sync. From the Rust book
only for use in single-threaded scenarios and will give you a compile-time error if you try using it in a multithreaded context
But it did previously compile! This hints that we were not setting up enough context for the compiler to ensure code is threadsafe.
Thank you to the Rust FFI Guide for inspiration on this bugfix.
Many plugins that write out collectd values (write_graphite, write_tsdb, write_mongodb) contain an option called StoreRates, which is described with:
Controls whether DERIVE and COUNTER metrics are converted to a rate before sending
If one is creating a plugin that writes, it is also a good idea to expose a StoreRates configuration option, so that users familiar with StoreRates behavior can migrate seamlessly -- else users will receive a shock when they see accumulated values (eg: total number of bytes sent on an interface instead of bytes per second). Prior to this release it was too cumbersome for one to reasonably calculate rates. This all changes with the availability of ValueList::rates, which will delegate to collectd's uc_get_rate
fn write_values(&self, list: ValueList) -> Result<(), Error> {
// if the user configured `StoreRates` then, as needed, convert the given values to rates
let values = if self.store_rates {
list.rates()
} else {
Ok(::std::borrow::Cow::Borrowed(&list.values))
}?;
// do something with values
}The reason why rates returns a Cow<Vec<_>> is because if all values are already Value::Gauge then there is no work to be done (or allocations needed), so it provides an optimisation.
- Update bindgen requirement from 0.42.0 to 0.43.0
- Update failure requirement from 0.1.2 to 0.1.3
- Implement
Value::is_nanto determine if the inner value is not a number. - Implement
Serializefor collectd Value. Note this serialization cannot be completed roundtrip (eg: serialize a Value and then deserialize it), as the inner value is directly serialized without additional type information. This is intended for writers outputting values for csv, postgres, etc.
I know it's a little unheard for two release two minor versions to be so close to each other, but an important integration has been added. Users can opt in to have log statements forwarded to collectd's logger.
Here is the recommended way to log:
Before:
let line = format!("collectd logging!");
collectd_log(LogLevel::Info, &line);After:
info!("collectd logging!");To opt into this feature, utilize the CollectdLoggerBuilder to register the logger in a PluginManager::plugins.
#[derive(Default)]
struct MyPlugin;
impl PluginManager for MyPlugin {
fn name() -> &'static str {
"myplugin"
}
fn plugins(_config: Option<&[ConfigItem]>) -> Result<PluginRegistration, Error> {
CollectdLoggerBuilder::new()
.prefix_plugin::<Self>()
.filter_level(LevelFilter::Info)
.try_init()
.expect("really the only thing that should create a logger");
unimplemented!()
}
}CollectdLoggerBuilder will look and feel very similar to env_logger.
The motivation for this feature came when I realized that in one of my collectd plugins, a dependency was logging an error but since no logger was setup, the message was discarded.
What's happening to collectd_log? Nothing right now. While it still has some uses, most should prefer using rust's native logging for a performance, ergonomic, and debugging win. If one needs to circumvent a potential rust logging filter, collectd_log is available.
- Globalize module paths found in
collectd_plugin!macro. Previously the macro only worked if thePluginManagerwas defined in the same module ascollectd_plugin!usage (or if one included the necessary imports used internally). This inflexibility was not conducive to organizing larger collectd plugins. - Update bindgen from 0.41.0 to 0.42.0
- Bump failure dependency to 0.1.2
- Enable
serdefeature by default, as most plugins will have some sort of configuration, and the recommended course of action is to enable the serde feature. Instead of forcing users to hop through another step on their way to writing a collectd plugin, makeserdefeature enabled by default.
No functionality changed in this release -- more like cleanup for those who received clippy warnings using collectd-plugin or like it when a library remove unsafe usages!
PluginCapabilitiestakesselfby value instead of by reference (clippy lint)collectd_plugin!macro no longer referencescfg(collectd57), which while set in the collectd-plugin's build.rs doesn't affect the downstream user. The only reason whycollectd_plugin!referencedcfg(collectd57)was to switch up mutability of a parameter that changed in Collectd 5.7. Since the mutability of the parameter does not change the behavior, a swath of code was eliminated and the clippy lint ignored.- Shrank the surface area of
unsafecode:- Global static mutable boolean replaced with
AtomicBoolean. This necessitated a move to require a minimum rust version of 1.24.0. - Prefer pointer casts instead of
transmute - Remove
u32toLogLevelviatransmuteinstead there is aLogLevel::try_from - Instead of wrapping functions in
unsafe, wrap the one or two statements that need unsafe.
- Global static mutable boolean replaced with
Another attempt to have documentation displayed correctly.
- Update documentation. Since 0.5.0, the
COLLECTD_VERSIONenvironment variable needs to be supplied, or cargo heuristically determines the installed collectd package. In the case where neither condition applies, the build will fail. When docs.rs generates the documentation, it failed both conditions, so the docs didn't build. The fix was to tweakCargo.tomlto provide the needed arguments in docs.rs metadata.
Breaking Change: Replace collectd cargo features with env variable. The COLLECTD_VERSION environment variable takes precedence, but if missing, collectd_plugin will attempt to autodetect the version by executing collectd -h. By going this route, we can ensure several invariants at build time:
- That the package builds. Previously, the
defaultfeature set would fail compilation with not intuitive error message. Now one has to supply the version or rely on autodetection. - That one can't combine features, like specifying
collectd-54andcollectd-57at the same time. They are now mutually exclusive. - I can now execute
cargo packagewithout running with--no-verifyascargo packagedoesn't allow feature selection (somewhat understandably). - Create a compliant
examples/directory for examples!
Valid COLLECTD_VERSION variables:
5.45.55.7
- Add serde deserialization for
LogLevel - Add
collectd_log_raw!macro for lower level log formatting usingprintfformatting
- Fix conversion from datetime to cdtime. This will fix those who set the time
in a
ValueListBuilderand receive a "uc_update: Value too old: name =" error in the logs
- Errors now have all their causes concatenated (semicolon delimited) when logged instead of just the head cause
- Overhead of submitting values via
ValueListBuilderreduced in half to ~100ns
- (Breaking change) rename
RecvValueListtoValueList - Export
ValueReportas part of API - Avoid allocations for logging plugins
- Force
Pluginimplementations to implementSync - Add a example
write_graphiteplugin
- Reduce pre-computed bindings with whitelisted types
- Improve serde deserialization of multi-keys
- Change deserialization return type from an alias of
ResulttoDeResult
- (Breaking change): Switch
collectd_plugin!away from lazy_static mutex - Preliminary Serde support for deserializing collectd configs
- Update
ValueListBuilderto accept static string references instead of just owned strings to reduce unnecessary allocations - Update
ValueListBuilderto take a slice of values to submit instead of a vector - Add several example plugins to the repo
- Add plugin hook for plugin initialization
- Add plugin hook for plugin log
- Add plugin hook for plugin write
This is the initial release of this library on crates.io as collectd-plugin