Bug 56382 - Add logging of deployment time
Add logging of deployment time
Status: RESOLVED FIXED
Product: Tomcat 7
Classification: Unclassified
Component: Catalina
trunk
All All
: P2 enhancement with 2 votes (vote)
: ---
Assigned To: Tomcat Developers Mailing List
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2014-04-10 12:49 UTC by Danila Galimov
Modified: 2014-04-24 13:02 UTC (History)
0 users



Attachments
Proposed patch (1.82 KB, patch)
2014-04-10 12:50 UTC, Danila Galimov
Details | Diff
Proposed patch - corrected (8.92 KB, patch)
2014-04-11 09:57 UTC, Danila Galimov
Details | Diff
Proposed patch - corrected 2 (5.01 KB, patch)
2014-04-11 09:59 UTC, Danila Galimov
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Danila Galimov 2014-04-10 12:49:45 UTC
Tomcat logs the start of the deployment directory, but doesn't log finish.
I've created the small patch, which logs when the particular deployment is finished, with the deployment time.

So output will be like this:

Apr 10, 2014 4:26:17 PM org.apache.catalina.startup.HostConfig deployDirectory
INFO: Deploying web application directory C:\AAA\Tools\apache-tomcat-7.0.52\webapps\aaa-admin

....

Apr 10, 2014 4:31:10 PM org.apache.catalina.startup.HostConfig deployDirectory
INFO: Deployment of web application directory C:\AAA\Tools\apache-tomcat-7.0.52\webapps\aaa-admin has finished in 292,516 ms.
Comment 1 Danila Galimov 2014-04-10 12:50:29 UTC
Created attachment 31506 [details]
Proposed patch
Comment 2 Konstantin Kolinko 2014-04-10 16:47:35 UTC
Comment on attachment 31506 [details]
Proposed patch

In reply to attachment 31506 [details]

>         if( log.isInfoEnabled() )
> +            startTime = System.currentTimeMillis();
>             log.info(sm.getString("hostConfig.deployDir",
>                     dir.getAbsolutePath()));

1. The above change breaks the code, because of missing '{' after an 'if' above.

> System.currentTimeMillis() - startTime

2. Boxing and unboxing have to be explicit, per our coding rules.

3. This covers only 'deployDirectory(..)' method. What about the others?
Comment 3 Danila Galimov 2014-04-11 09:56:50 UTC
(In reply to Konstantin Kolinko from comment #2)
> Comment on attachment 31506 [details]
> Proposed patch
> 
> In reply to attachment 31506 [details]
> 
> >         if( log.isInfoEnabled() )
> > +            startTime = System.currentTimeMillis();
> >             log.info(sm.getString("hostConfig.deployDir",
> >                     dir.getAbsolutePath()));
> 
> 1. The above change breaks the code, because of missing '{' after an 'if'
> above.
Fixed.
 
> > System.currentTimeMillis() - startTime
> 
> 2. Boxing and unboxing have to be explicit, per our coding rules.
Fixed.
 
> 3. This covers only 'deployDirectory(..)' method. What about the others?
Done for deployDirectory, deployWAR and deployDescriptor methods.
Comment 4 Danila Galimov 2014-04-11 09:57:12 UTC
Created attachment 31511 [details]
Proposed patch - corrected
Comment 5 Danila Galimov 2014-04-11 09:59:02 UTC
Created attachment 31512 [details]
Proposed patch - corrected 2
Comment 6 Danila Galimov 2014-04-17 10:18:31 UTC
Is it ok now?
Comment 7 Violeta Georgieva 2014-04-24 13:02:17 UTC
Thanks for the report and the patch. This has been fixed in trunk for 8.0.6 and in 7.0.x for 7.0.54 onwards.