It's not huge. It's a few hours of dev, but then it will need to be tested on each of the major versions of Tomcat we support - 5, 6, and 7, which will take a few more hours. So probably 4-8 hours total.

(In reply to comment #2)
> It's not huge. It's a few hours of dev, but then it will need to be tested on
> each of the major versions of Tomcat we support - 5, 6, and 7, which will take
> a few more hours. So probably 4-8 hours total.

Ian - are you saying you recommend adding those three metric definitions to the descriptor (the ones that Larry put in the description of this issue?)

If so, I agree this should be easy. I'll assign this to me - but let me know if that's why you say it should be easy to implement. Because when I first read this, I thought it would not be trivial to do what Larry mentioned:

"The fix will be to make the impl of TomcatConnectorComponent smart enough to
discern whether or not an executor is configured, and if so, obtain the metrics
from the appropriate Executor mbean."

The way the plugin metric subsystem works, we'll attempt to collect ALL the metrics - so we'll still report -1 for those non-shared metrics. The user would just have to know to ignore those and to instead look at the Shared metrics when monitoring his systems. If this is Ok, then there isn't must we need to do other than add the new metrics and test with those different tomcat versions.

If there is something more we need to do, please comment.

After thinking about this, here's how I will implement this. This should be backwards compatible and will not require us to introduce three new metrics (and thus we won't have 6 metrics on each Tomcat server, where only 3 are used and 3 remain unused, per server). It will also allow the user to define their own shared executor name (there is nothing special about the name "tomcatThreadPool" so we can't hardcode that or assume that is what the user will use).

I will add a new plugin configuration property on the Connector resource:

               <c:simple-property
                  name="sharedExecutorName"
                  type="string"
                  description="If this Connector is configured to use a shared Executor for its thread pool, this is the name of that Executor. This is required to successfully collect threadpool metrics from a shared Executor used by the Connector. Leave this unset if the Connector is not using a shared Executor for its threadpool."
                  required="false" />

If this is not set, we do nothing special - we continue collecting the metrics from the original MBean as defined in the descriptor. This provides backwards compat - any old Tomcat servers already in inventory that is working, will continue to work.

If this is set, we will use a different MBean name, substituting the value of this property into the MBean object name, so we can collect the new metrics:

1) Catalina:type=ThreadPool,name=%handler%%address%-%port%:currentThreadsBusy
will be replaced with:
Catalina:type=Executor,name=<value of the new plugin config prop>:activeCount

2) Catalina:type=ThreadPool,name=%handler%%address%-%port%:currentThreadCount
will be replaced with:
Catalina:type=Executor,name=<value of the new plugin config prop>:poolSize

3) Catalina:type=ThreadPool,name=%handler%%address%-%port%:maxThreads
will be replaced with
Catalina:type=Executor,name=<value of the new plugin config prop>:maxThreads

git commit to master: 41eb05d5004470cbdaab4b2bc4ecfb082edaf8ce

there is one thing that we can still do - auto-discover the value for this new property during discovery.

I notice that the MBean for the Connector, e.g.: "Catalina:type=Connector,port=8080", has the attribute: "executorName".

If the connector is using a shared executor, this is the name of that Executor as defined in server.xml (i.e. its the value we want to store in this new plugin config prop).

If the connector is NOT using a shared executor, I see its value as "Internal" (at least on Tomcat 6 - we'd have to see if its this same Internal name for other versions).

In TomcatConnectorDiscoveryComponent, we could ask for the connector MBean's attribute value for "executorName" and pre-fill the plugin configuration with that value. This would allow those three metrics to begin getting collected ASAP without requiring an admin it manually go into the plugin config and setting its value in the GUI.

git commit to master d0707a4 

the new plugin config setting will be automatically set when appropriate - the discovery component will check to see if the Connector MBean has a executorName attribute that is something other than "Internal" - if it is, this is the executor name of the shared threadpool.

Clean JON 310 installation with tomcat 310 plugin jar, will return proper values for thread pool related metrics on Tomcat 6 Connectors when the connectors are configured to use an Executor. 

However, connectors for Tomcat 5 are not discovered at all. The following bugzilla has been raised to address this: 

https://bugzilla.redhat.com/show_bug.cgi?id=822861