Bug 707349

Created attachment 500678 [details]
issue 1)

Created attachment 500679 [details]
issue 2)

*** Bug 833059 has been marked as a duplicate of this bug. ***

Here's my server.xml snippets that are relevent for looking at APR Connectors and using shared executors with Connectors (note, I used Tomcat 7 on Windows since it already had the JNI native wrappers pre-built out of box):

  # BZ change - SSLEngine was changed from on to "off"
  <Listener className="org.apache.catalina.core.AprLifecycleListener" SSLEngine="off" />
...
    # BZ change - uncommented this thread pool executor for use in the 8081 connector
    <!--The connectors can use a shared executor, you can define one or more named thread pools-->
    <Executor name="tomcatThreadPool" namePrefix="catalina-exec-"
        maxThreads="150" minSpareThreads="4"/>
...
    # BZ change - the port 8080 was unchanged but the 8081 was added to use a shared executor
    <Connector port="8080" protocol="HTTP/1.1"
               connectionTimeout="20000"
               redirectPort="8443" />

    <!-- A "Connector" using the shared thread pool-->
    <Connector executor="tomcatThreadPool"
               port="8081" protocol="HTTP/1.1"
               connectionTimeout="20000"
               redirectPort="8444" />

    # BZ change - SSLEnabled="false" rather than "true" - no need to use SSL for this test
    <!-- Define a SSL HTTP/1.1 Connector on port 8443
         This connector uses the JSSE configuration, when using APR, the
         connector should be using the OpenSSL style configuration
         described in the APR documentation -->
    <Connector port="8443" protocol="HTTP/1.1" SSLEnabled="false"
               maxThreads="150" scheme="https" secure="true"
               clientAuth="false" sslProtocol="TLS" />

I see these MBeans:

   Catalina:type=GlobalRequestProcessor,name="ajp-apr-8009"
   Catalina:type=GlobalRequestProcessor,name="http-apr-8080"
   ...and others...

Notice the literal double quotes wrapping the name attribute value. Our parsing code chokes on this (see the constructor for TomcatConnectorDiscoveryComponent.ConfigInfo).

Without APR configured (comment out the APR lifecycle listener in server.xml) you will see these global request proc MBeans:

   Catalina:type=GlobalRequestProcessor,name="ajp-bio-8009"
   Catalina:type=GlobalRequestProcessor,name="http-bio-8443"
   ... and others...

Notice "-bio" (and from the APR enabled Tomcat) "-apr" in the object names now. This appears to be new in Tomcat 7. Our parsing code in that ConfigInfo constructor needs to take this into account as well.

When specifying "address" attribute on the Connectors in server.xml, MBean names are like:

   Catalina:type=GlobalRequestProcessor,name="http-bio-192.168.1.10-8080"

When using APR connectors with "address", the same kind of thing is used:

   Catalina:type=GlobalRequestProcessor,name="ajp-apr-192.168.1.10-8009"

This is true (uses an IP address) if address is a hostname, too (Tomcat seems to do a DNS lookup and uses the IP for the hostname in the MBean ObjectName).

However, with "address" in the Connector, the Connector MBean ObjectName uses quotes, too:

   Catalina:type=Connector,port=8009,address="192.168.1.10"

I think this is not expected in the current Tomcat plugin parser code, either.

Another warning in the agent log that indicates a problem in another area (this is re: configuration of a connector)

2012-06-19 15:33:32,168 WARN  [ConfigurationManager.threadpool-1] (rhq.core.pc.configuration.ConfigurationCheckExecutor)- Plugin Error: Invalid Tomcat Connector resource configuration returned by Tomcat plugin - Required property 'emptySessionPath' has a null value in Configuration[id=0]

i think i found out why the wars are always down.

Catalina:j2eeType=WebModule,J2EEApplication=none,J2EEServer=none,name=//localhost/docs

is an example of an MBean ObjectName - this is still valid. But we are looking for an attribute called "state". I do not see an attribute with that name, but I do see "stateName" whose value is the string "STARTED" (no quotes).

Created attachment 593063 [details]
prototype patch to fix some things

attaching bz707349.patch as a prototype to fixing SOME things but not all. This doesn't take into account the new "-apr" or "-bio" strings in the name attribute of the ObjectName. But it does seem to fix the problem where there are now quotes in Tomcat 7's MBean name.

This patch is really only here for documentation purposes to see where in the code things need to be fixed for some of these issues. We can incorporate this patch in a real fix if appropriate.

I completed the update for Tomcat 7.
Changes:
Connector - Storing the name from discovery and using the full name (with quotes in 7), not rebuilding for each request
Cache Discovery - Try to query for 'path', if 0 returns, try with 'context'
War - Changed all availability items to stateName and using String instead of int

I don't have any datasources deployed, so I don't know if there were any changes.  I tested against Tomcat 6 and 7.  Let me know how you want me to get the code submitted.

I put in a datasource and found that there is a change there as well.  Almost identical to the Cache changes:
Datasource Discovery - Try to query for 'path', if 0 returns, try with 'context'

(In reply to comment #8)
> I completed the update for Tomcat 7.
> Changes:
> Connector - Storing the name from discovery and using the full name (with
> quotes in 7), not rebuilding for each request
> Cache Discovery - Try to query for 'path', if 0 returns, try with 'context'
> War - Changed all availability items to stateName and using String instead
> of int
> 
> I don't have any datasources deployed, so I don't know if there were any
> changes.  I tested against Tomcat 6 and 7.  Let me know how you want me to
> get the code submitted.

Ken - please rebase the patch and any of your fixes on the current master branch, then attach the new patch, which includes your fixes, to this BZ. I'll peer review and push if appropriate. Thanks -John

Created attachment 624151 [details]
jopr-tomcat-plugin

Comments in previous post.

Thanks,
Ken Rumer

(In reply to comment #11)
> Created attachment 624151 [details]
> jopr-tomcat-plugin
> 
> Comments in previous post.

Ken - it appears this attachment includes everything - all tomcat plugin source, not just your change diffs. Did you mean to attach the source code for the entire tomcat plugin here?

As RHQ community work is performed in git - you'd just need to git rebase on the current master branch, then use git to produce a patch diff (preferrably via git format-patch) which should include the changes being proposed. Otherwise, it would force someone to go through the entire source attachment to discern what you did or did not change and what those changes actually were. This is what we want to use git for. Thanks.

Created attachment 627712 [details]
Tomcat 7 support

Here is the commit patch.

(In reply to comment #13)
> Created attachment 627712 [details]
> Tomcat 7 support
> 
> Here is the commit patch.

I created a remote branch bug/707349, rebased off of current master branch, and applied this patch to it. The git commit is e7d48240474fba87f1a3c4118de4618fd2c8b32d

We'll need someone to peer review and test the patch before we cherry pick over to the master branch.

(note: for some reason "git am" could not apply the patch, so I applied it via "git apply" thus losing the author tag. It also told me there are tabs in the code - I applied it anyway, but please supply patches in the future that do not have any tabs in the source. The RHQ Eclipse project has the code formatting settings in it to help)

Related message sent to rhq-devel mailing list:

https://lists.fedorahosted.org/pipermail/rhq-devel/2012-October/002227.html

Here's the content:

---
There is a community contribution to the Tomcat plugin:

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

I created a branch off of master yesterday and put the patch in it:

http://git.fedorahosted.org/cgit/rhq/rhq.git/log/?h=bug/707349

In order to be merged into master and get into the next RHQ release, we need someone to peer review this and test this. Is anyone available?

We do not have any unit tests, but I was asked to get some. I'll ask the submitter to see if a unit test(s) is possible (this might be one of those that is hard to test today because it will probably require integration tests with a running Tomcat instance).
---

Ken, is there unit test(s) you can create to verify these changes?

I will try, but i can't get anything done for a couple of days.

I tested it with Tomcat 7 (current build), and it works properly. The branch can be merged.

the patch is git commit: e7d48240474fba87f1a3c4118de4618fd2c8b32d

I merged this into master: 99a1543f266710c7136844ae269ff944dad3f289

adding a link which documents the qualification of this plugin 

https://tcms.engineering.redhat.com/plan/536/ews-plugin#testruns

Seems that metric collection fails for Tomcat 7 for both connectors and web applications. 

Also, it is unclear on how this commit impacts Tomcat 5 and Tomcat 6.

I would strongly encourage that in the future, large chunks of work such as this be done as individual bugs/tasks instead of one BZ representing unrelated changes. However, considering this commit is tied to this BZ only, I have no choice but to put this entire thing back on dev for further work to "tomcat plugin: get support for Tomcat 7 fully working."

Also a duplicate of https://bugzilla.redhat.com/show_bug.cgi?id=921194

Marking as closed/won't fix. This bug is too broad and although some work had been done in an attempt to get Tomcat 7 from EWS working with the managed plug-in, it was unsuccessful and had sense been broken out into separate tasks.

The most relevant:

    Bug 921261
    Bug 921194


If additional work is needed to get the managed plug-in working with Tomcat 7 from EWS, please create new BZs for appropriate tacking int he EWS BZ.