Skip to content

RFC: collectd: enable a bunch of plugins (by adding more deps)#2951

Closed
bjornfor wants to merge 5 commits intoNixOS:masterfrom
bjornfor:collectd-extra-plugins
Closed

RFC: collectd: enable a bunch of plugins (by adding more deps)#2951
bjornfor wants to merge 5 commits intoNixOS:masterfrom
bjornfor:collectd-extra-plugins

Conversation

@bjornfor
Copy link
Copy Markdown
Contributor

Increases the closure size from 129 MiB to 546 MiB.


Actually, I'm unsure of whether this should be committed or not, so I'm making this PR as a RFC. There are so many plugins (more than I've now enabled) that I'm wondering if the better/more sustainable solution is to tell users to just use lib.overrideDerivation in configuration.nix... Thoughts?

@jwiegley
Copy link
Copy Markdown
Contributor

I'd like to see a collectPackages subspace, perhaps, which allows users to install the plugins they want and then activate them if they are installed (for example, the way that emacsPackages can be added to the system). Using overrideDerivation is another option, but a bit more manual. However, since this seems like more of a NixOS issue, I'll defer to those who use that platform.

@bjornfor
Copy link
Copy Markdown
Contributor Author

That'd be a good idea for external plugins. Currently, I'm only enabling internal plugins... so there is nothing that can be split into something like "collectdPackages" (yet). Look at the patch and you'll see.

I wonder if multiple-outputs can help with this. But all plugins install to "lib/collectd/", so unless there are ways to manually define a filter to split out each plugin/library into its own "package", I guess not :-/

@peti
Copy link
Copy Markdown
Member

peti commented Jul 7, 2014

IMHO, a reasonable approach is to enable everything by default, but to give users an opportunity to override certain features to null if they want a smaller build.

@wmertens
Copy link
Copy Markdown
Contributor

wmertens commented Jul 7, 2014

what I find hard to grasp is the fact that even the standard set takes up
129MB... what is going into these plugins? Pretty insane...

@bjornfor
Copy link
Copy Markdown
Contributor Author

bjornfor commented Jul 7, 2014

@wmertens: 129 MiB is the size of the whole closure. The collectd store path itself is only 2 MiB. With these extra plugins, collectd grows to only 2.6 MiB, but pulls in lots of new dependencies... (I guess many of them are already installed on a typical system, so the increased size is probably not that noticeable).

I guess I'll commit this then.

@wmertens
Copy link
Copy Markdown
Contributor

wmertens commented Jul 7, 2014

Ah, in that case 👍 from me, although an example on how to override the dependencies so they are found might be appreciated by future Nix newbies.

@bjornfor
Copy link
Copy Markdown
Contributor Author

bjornfor commented Jul 7, 2014

@wmertens: Did you mean "so they are not found"? If so, just use .override on the derivation, like this: collectd = pkgs.collectd.override { optional-dep1 = null; }. The nixos manual talks about .override in this chapter: http://nixos.org/nixos/manual/#sec-customising-packages.

@wmertens
Copy link
Copy Markdown
Contributor

wmertens commented Jul 7, 2014

Ah ok, the foo ? null syntax was confusing me. Is that even needed, if you override like you propose?

@bjornfor
Copy link
Copy Markdown
Contributor Author

bjornfor commented Jul 7, 2014

@wmertens: Correct, it isn't (strictly) needed. "foo ? null" just provides a default value (null) if there is no foo attribute available when evaluating the expression. To me, "foo ? null" is just a hint to the user that this input is optional.

@wmertens
Copy link
Copy Markdown
Contributor

wmertens commented Jul 7, 2014

Right... I suppose, as a user, simply reading that in a comment is better
than a hint 😉

I was reading the diff assuming that the top level composition had some
setup where the dependencies weren't passed...

@ocharles
Copy link
Copy Markdown
Contributor

I'm 👍 to 3772d1e, but it looks like it won't get the PostgreSQL or Redis plugins working, which are the two I'm interested in :)

@bjornfor
Copy link
Copy Markdown
Contributor Author

@ocharles: I'm going through the plugin list again and will try to enable more "low-hanging fruits". I'll try to get PostgreSQL and Redis in.

libcredis is a client library for Redis (key-value database).

(libcredis is an optional dependency of collectd.)

Homepage: https://code.google.com/p/credis/
@bjornfor
Copy link
Copy Markdown
Contributor Author

@ocharles: postgresql and redis plugins are now in. The redis plugin was free (< 1 MiB), but postgresql pulled in 60 MiB. Still worth it. Pushing soon.

@ocharles
Copy link
Copy Markdown
Contributor

I'm fine with that - especially with a tool of this nature (space is cheap, my time doing sysops is not!)

bjornfor added 4 commits July 14, 2014 23:32
Add patch to fix linking against libpthread (from Fedora).

https://bugzilla.redhat.com/show_bug.cgi?id=1037707
https://bugs.launchpad.net/libmemcached/+bug/1281907

This fixes (upcoming) libmemcached use in collectd, which currently
doesn't work because of link failure caused by missing "pthread_once"
symbol.
And a more precise license (BSD => 3-clause BSD).
These are plugins that are internal to collectd; they are bundled in the
source archive. Without this, many of the collectd plugins advertised on
their homepage don't actually work, so I think we should add this.

This change increases the closure size from 129 MiB to 621 MiB. The size
of the collectd store path itself goes from 2 MiB to 2.9 MiB.

Users wanting a smaller closure size can set some (or all) of the
optional inputs to "null" using ".override".

List of new plugins:

  amqp
  apache
  ascent
  bind
  curl
  curl_json
  curl_xml
  dbi
  dns
  iptables
  libvirt
  lvm
  memcachec
  modbus
  mysql
  nginx
  notify_desktop
  pinba
  ping
  postgresql
  redis
  rrdcached
  rrdtool
  sensors
  sigrok
  varnish
  write_http
  write_redis
  write_riemann
Don't repeat package name in description, add maintainer, ...
@bjornfor
Copy link
Copy Markdown
Contributor Author

Pushed to master.

@bjornfor bjornfor closed this Jul 15, 2014
@bjornfor bjornfor deleted the collectd-extra-plugins branch July 23, 2014 21:07
@nbp nbp mentioned this pull request Feb 26, 2017
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants