[collectd] Varnish plugin

[Florian Forster]

Hi Jérôme,

On Tue, Jun 01, 2010 at 04:07:58PM +0200, Jerome Renard wrote:
> > * The only argument passed to "VSL_OpenStats" is a NULL pointer
> >   called  "varnish_instance_name". Would it be reasonable to let the
> >   user  configure something else here and use NULL as a default?
> 
> Yeah I already thought about that and I was not sure what to do here.
> Even though it is in theory possible to run multiple varnishd
> instances on the same host, in practice it is quite common to have
> only one instance per host.

I was just curious. If having only one instance is the usual case,
going with that is alright. But in my experience, if it is possible to
have multiple instances, *someone* will come up with a reason to have
multiple instances and will want data about all of them ;)

So being able to configure multiple instances would definitely be nice
to have, but if you have better things to do that's no problem either –
let whoever needs it do the work ;)

> But maybe a relevant alternative would be to provide a configuration
> variable for this, and if it is empty, then set varnish_instance_name
> to NULL and thus get statistics for the current instance.

Should you want to look into that, here's how I'd do it:

* Register a simple config callback function. Take a look at
  "interface_config" in src/interface.c for an example.
* For every "Instance" (or whatever you chose to name it) option add the
  correcponding "value" string to a (module-)global "char **".
* In the read function, do:
  if the global "char **" is not NULL:
    iterate over all instances and record statistics in turn
  else
    call VSL_OpenStats with NULL

> > * The member (value_list_t *)->values_len is of type "int". You
> > assign  to it a "size_t" without cast, which may result in warnings
> > (and hence  problems) with some optimization settings.
> 
> Ok I'll see how I can fix that,

A simple cast should do the trick.

> could you please give me the optimization settings you use to get such
> warnings ? So I can reproduce the issue locallu and fix it :)

Sorry, but I didn't compile the code yet :/ When using GCC, "-O0" and a
x86_64 machine you should get a warning due to assigning a 64bit value
to a 32bit integer … But then again, I am hardly ever able to reproduce
such stuff with GCC ;)

> On a practical side, would you prefer I fork the project on github so
> you can pull my changes when you think everything is OK for you ? Or
> do you prefer getting patches through email ?

Either way works for me. Pulling from a public repository is a tad
easier than applying the patch from a mail, but I value the publicity of
a public mailing list, too. So just pick what *you* prefer ;)

Regards,
—octo
-- 
Florian octo Forster
Hacker in training
GnuPG: 0x91523C3D
http://verplant.org/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://mailman.verplant.org/pipermail/collectd/attachments/20100601/1147f00c/attachment.pgp