You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2016/09/21 13:04:51 UTC

[Bug 60161] New: RewriteValve: Add more logging support similar to mod-rewrite

https://bz.apache.org/bugzilla/show_bug.cgi?id=60161

            Bug ID: 60161
           Summary: RewriteValve: Add more logging support similar to
                    mod-rewrite
           Product: Tomcat 8
           Version: 8.0.35
          Hardware: PC
                OS: Mac OS X 10.1
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: Catalina
          Assignee: dev@tomcat.apache.org
          Reporter: santhanapreethi28@gmail.com

Created attachment 34287
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34287&action=edit
Patch for providing detailed logging in RewriteValve

The logging provided for RewriteValve is minimal. httpd provides detailed
logging support with configurable log levels. Also It would be useful while
debugging, if the time taken for rewrite is logged. I've added a patch
replicating apache mod_rewrite behaviour.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 60161] RewriteValve: Add more logging support similar to mod-rewrite

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60161

--- Comment #4 from Remy Maucherat <re...@apache.org> ---
If there are more than one rewrite valves used, it doesn't sound like a good
idea.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 60161] RewriteValve: Add more logging support similar to mod-rewrite

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60161

Remy Maucherat <re...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|WONTFIX                     |FIXED

--- Comment #12 from Remy Maucherat <re...@apache.org> ---
Although I didn't get feedback, I decided to include the change (with some
fixes). So this is now "fixed".

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 60161] RewriteValve: Add more logging support similar to mod-rewrite

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60161

Santhana Preethi <sa...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |santhanapreethi28@gmail.com

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 60161] RewriteValve: Add more logging support similar to mod-rewrite

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60161

--- Comment #11 from Santhana Preethi <sa...@gmail.com> ---
Any update regarding writing RewriteValve logs based on Remy Maucherat's patch?

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 60161] RewriteValve: Add more logging support similar to mod-rewrite

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60161

--- Comment #10 from Santhana Preethi <sa...@gmail.com> ---
(In reply to Remy Maucherat from comment #9)
> 
> Since the issue is about configuring the valve logging, I have a possible
> patch for that. It allows creating subcategories for the container logger,
> and exploits the ValveBase.containerLog field to use that other logger.

The proposed patch does allow adjusting the log level of ONLY the RewriteValve.
It is definitely better than the current situation. 

(In reply to Remy Maucherat from comment #8)
>
> I also don't see what the processing time is used for in a sub component,
> this screams like a feature from 15 years ago [use a debugger instead].

Often, Url Rewriting involves complex regex matching. Logging the processing
time for the rewrite will be useful for debugging any slow requests. Also,
httpd already logs the rules applied for every URI at a higher log level (3 and
above).

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 60161] RewriteValve: Add more logging support similar to mod-rewrite

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60161

--- Comment #8 from Remy Maucherat <re...@apache.org> ---
(In reply to Santhana Preethi from comment #6)
> > Now so? I would think this might be the one good part of this proposed
> > patch, because it makes it easier to adjust the log level of *only* the
> > RewriteValve. 
> 
> As Christopher pointed out, this would make it easier to configure log level
> for RewriteValve. 

That's his opinion and I happen to disagree with it. You can configure the log
level of the container.
I also don't see what the processing time is used for in a sub component, this
screams like a feature from 15 years ago [use a debugger instead].

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 60161] RewriteValve: Add more logging support similar to mod-rewrite

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60161

--- Comment #6 from Santhana Preethi <sa...@gmail.com> ---
(In reply to Mark Thomas from comment #2)

> The switch from the container's logger to a dedicated Rewrite logger is
> problematic too.

I don't think writing all rewrite logs to container's logger is a good idea. In
the current situation, adjusting container's logger level to DEBUG is required
to write rewrite logs. 

(In reply to Christopher Schultz from comment #3)

> Now so? I would think this might be the one good part of this proposed
> patch, because it makes it easier to adjust the log level of *only* the
> RewriteValve. 

As Christopher pointed out, this would make it easier to configure log level
for RewriteValve. 

(In reply to Mark Thomas from comment #2)

> There might be a case for recording the time taken to rewrite the URL but a)
> it should only be collected and logged when debug logging is enabled and b)
> should use nanoTime.

I've reworked the patch with log levels DEBUG,TRACE and nanoTime. I've
replicated httpd mod_rewrite logging behaviour where level 2 logs only the
rewrite information and level 3 also logs all the rules that were applied
before the rewrite happened.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 60161] RewriteValve: Add more logging support similar to mod-rewrite

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60161

Remy Maucherat <re...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #34290|0                           |1
        is obsolete|                            |

--- Comment #9 from Remy Maucherat <re...@apache.org> ---
Created attachment 34293
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34293&action=edit
Valve logging change

Since the issue is about configuring the valve logging, I have a possible patch
for that. It allows creating subcategories for the container logger, and
exploits the ValveBase.containerLog field to use that other logger.
Not sure if it's really a good idea though.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 60161] RewriteValve: Add more logging support similar to mod-rewrite

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60161

--- Comment #2 from Mark Thomas <ma...@apache.org> ---
The switch from the container's logger to a dedicated Rewrite logger is
problematic too.

There might be a case for recording the time taken to rewrite the URL but a) it
should only be collected and logged when debug logging is enabled and b) should
use nanoTime.

As a general rule, please keep the coding style of any patches consistent with
the code being patched.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 60161] RewriteValve: Add more logging support similar to mod-rewrite

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60161

Santhana Preethi <sa...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #34287|0                           |1
        is obsolete|                            |

--- Comment #7 from Santhana Preethi <sa...@gmail.com> ---
Created attachment 34290
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34290&action=edit
Modified patch for Rewrite Logging

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 60161] RewriteValve: Add more logging support similar to mod-rewrite

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60161

--- Comment #3 from Christopher Schultz <ch...@christopherschultz.net> ---
(In reply to Mark Thomas from comment #2)
> The switch from the container's logger to a dedicated Rewrite logger is
> problematic too.

Now so? I would think this might be the one good part of this proposed patch,
because it makes it easier to adjust the log level of *only* the RewriteValve.
Honestly, that should have been the only thing necessary, here. Everything else
would have been configuration of the log as per usual.

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 60161] RewriteValve: Add more logging support similar to mod-rewrite

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60161

Remy Maucherat <re...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |WONTFIX
             Status|NEW                         |RESOLVED

--- Comment #1 from Remy Maucherat <re...@apache.org> ---
This patch is not very good. Changing the log level from debug to info is not
acceptable (debug is more than appropriate given what this is, and there's also
going to be a large performance impact ...), and that's what most of the patch
is doing in addition to logging statistics that don't look very useful (given
the precision of currentTimeMillis).

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 60161] RewriteValve: Add more logging support similar to mod-rewrite

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60161

--- Comment #5 from Christopher Schultz <ch...@christopherschultz.net> ---
That's a good point, Rémy, but it's no worse than the current situation where
the rewrite logs all go to the container's logger.

I think there's still some useful work that can be done, here.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org