[collectd] Tomcat plugin

[Florian Forster]

Hi again :)

On Tue, Dec 18, 2007 at 02:03:20PM +0100, Sebastian Harl wrote:
> Just a small nitpick about coding style:

Sorry again if we appear picky here, but it's my firm conviction that
code should be easy to read, not (necessarily) easy write.

I have some questions regarding the data:
- What do the garbage collector statistics mean? Is `count' the number
  of times the GC has been run? What is GC `time'? The time spend
  collecting garbage? Why should these two values be bundled together?
- What's this connector business? Possibly that's obvious to someone who
  knows how Tomcat works, but I'm afraid I have no idea ;) I'm afraid
  the Tomcat documentation is a bit short in this respect ;)
  <http://tomcat.apache.org/tomcat-5.5-doc/connectors.html>

I've also made some changes to your code and put that in a separate
branch of the repository. The changed file is attached, please use it as
a base for further changes. My changes include:
- Fixed the changing indentation. There were parts indented with one
  tab, two spaces and four spaces. I changed it to have two-space
  indentation everywhere.
- Removed the `Instance' setting from _within_ the block. This removes
  this ambiguity:
  <Plugin tomcat>
    <Instance "foo">
      Instance "bar"
      ...
    </Instance>
  </Plugin>
  Would that instance be named `foo' or `bar'?
- Removed some stacking depth with the following reformulation:
  Instead of
    for (...)
    {
      if (foo)
      {
        /* lot and lot of lines, including more for-loops
	 * and if-blocks */
      }
    }
  rather use
    for (...)
    {
      if (!foo)
        continue;
      /* the other lines, outside the if-block now */
    }
- I broke up the types, like `gc' with the data sources `time' and
  `count' is not `gc_time' and `gc_count'.
- I've changed all long lines so that no lines has more than 78
  characters.

If any of these changes broke the plugin for good, please let me know -
I couldn't test the code myself. Due to the changed types you will have
to adapt your `types.db' before testing, though.

> > 		if (ti->url == NULL)
> > 			return (-1);
> 
> Is it really appropriate to return an error in this case?

Yes, but I changed it so the error occurs during configuration.

So, thank you very much for the plugin Justo :) It'd be great if you
could test my changes and if you could double check what I put into the
manpage at some point (right now it's undocumented).

Regards,
-octo
-- 
Florian octo Forster
Hacker in training
GnuPG: 0x91523C3D
http://verplant.org/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: tomcat.c
Type: text/x-csrc
Size: 18580 bytes
Desc: not available
Url : http://mailman.verplant.org/pipermail/collectd/attachments/20071219/fc29d920/attachment.c 
-------------- 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/20071219/fc29d920/attachment.pgp