[collectd] Tomcat plugin

[Justo Alonso]

On Dec 19, 2007 1:35 PM, Florian Forster <octo at verplant.org> wrote:

> Hi again :)

Hi Florian !

>
> 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.
>

It's ok ... I  haven't develop  anything in  the last years .. only my
scripts, and sysadmin smalls program .... sorry ... ;-)

>
> 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?


Yes, it'is. I put it on one type to have only one rrrd file for gc type, but
you can put it on separate files if you want.

About it ... in the first message I send you a collectd servlet. If you
don't want stats about garbage collector then you can use the manager/status
servlet that is bundled with tomcat.


> - 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>


A tomcat connector is a port & protocol method to connect to tomcat. On the
tomcat instance you can define a connector on a 8080 port with HTTP
procotol, another 8009 with AJP protocol, another on port 80443 with HTTPS
protocol, etc ..... all stats on Mbeans are under the connector instances
...


> 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:


I read the file and all your changes are ok for me... Now is easier  to read
and safer ... good job !

Only ... on the tomcat_shutdown function, I forgot change
plugin_unregister_config to plugin_unregister_complex_config ....


>
> - 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).


You are welcome !! ... thanks for you and sebastian, you make a fantastic
job ! ;-)

I see the web git repository and I can't find this branch to see the man
page ... can you send me a copy for read it ?? ;-)

I'm testing your version ... I hope tomorrow send you my comments ...

thanks !
j


>
>
> Regards,
> -octo
> --
> Florian octo Forster
> Hacker in training
> GnuPG: 0x91523C3D
> http://verplant.org/
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.6 (GNU/Linux)
>
> iD8DBQFHaRAKOIGYkRnzlcMRAsh2AKCcONc9Vc3a24e+FJw/LizkYlsIVgCgvnn1
> ZbnWNEUbVEL6su4gsMFDy1k=
> =f47m
> -----END PGP SIGNATURE-----
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mailman.verplant.org/pipermail/collectd/attachments/20071220/791c1704/attachment.htm