Re: Per-module / per-dir loglevel configuration version 5

[Stefan Fritsch <sf...@sfritsch.de>]

On Friday 04 June 2010, Stefan Fritsch wrote:
> which are either pure bug fixes or pretty trivial. I will create a
> new  patch series without these soon, hopefully tomorrow.

The next iteration is at

http://people.apache.org/~sf/per-module-loglevel-v5/

I have included Rainer's and Joe's corrections (thanks again). Apart 
from that, there is only comment and docs changes, no code. But the 
set is now reduced to 13 patches. I guess I need more git-fu, this is 
still too much manual work :-|

Since there has been some positive comments about the changes, I will 
commit before the next alpha (my current plan is Sunday). But anyone 
is of course welcome to review them before then.

Cheers,
Stefan

Re: Per-module / per-dir loglevel configuration version 5

[Stefan Fritsch <sf...@sfritsch.de>]

On Saturday 05 June 2010, Rainer Jung wrote:
> It doesn't compile, two small problems, both in
> include/http_config.h:
 
> During the build I also noticed, that mod_lua has two lines, where
> it  doesn't use APLOG_MARK but instead directly passes two
> file/line args along.

Thanks. It seems I haven't tested recently without --enable-
maintainer-mode and I didn't have mod_lua enabled.

Cheers,
Stefan

Re: Per-module / per-dir loglevel configuration version 5

[Rainer Jung <ra...@kippdata.de>]

On 05.06.2010 21:44, Rainer Jung wrote:
> During the build I also noticed, that mod_lua has two lines, where it
> doesn't use APLOG_MARK but instead directly passes two file/line args
> along.

Patch along the lines of the mod_ssl solution:

http://people.apache.org/~rjung/patches/httpd-trunk-per-module-loglevel-v5-addon-2.patch

Rainer

Re: Per-module / per-dir loglevel configuration version 5

[Rainer Jung <ra...@kippdata.de>]

On 04.06.2010 23:57, Stefan Fritsch wrote:
> On Friday 04 June 2010, Stefan Fritsch wrote:
>> which are either pure bug fixes or pretty trivial. I will create a
>> new  patch series without these soon, hopefully tomorrow.
>
> The next iteration is at
>
> http://people.apache.org/~sf/per-module-loglevel-v5/

It doesn't compile, two small problems, both in include/http_config.h:

1) ap_get_module_loglevel still contains (s)->loglevel and 
(s)->module_loglevels instead of s->log.level and s->log.module_levels. 
Trivial to fix.

2) The do-loops in the macros ap_get_conn_module_loglevel() and 
ap_get_request_module_loglevel() break when injected into an expression 
in server/log.c lines 584f.

I used nested function macros to fix 2), because that way the logic 
seemed to be readable best. At the end the macros for server, conn and 
request used a common macro, so I renamed ap_get_module_loglevel() to 
ap_get_server_module_loglevel() and used ap_get_module_loglevel() for 
the common part. Because of the rename, another three files have small 
changes. If you don't like the rename, your welcome to choose some other 
name for the new macro :)

Patch on top of your v5 at:

http://people.apache.org/~rjung/patches/httpd-trunk-per-module-loglevel-v5-addon-1.patch

During the build I also noticed, that mod_lua has two lines, where it 
doesn't use APLOG_MARK but instead directly passes two file/line args 
along. We need to fix those like you did for mod_ssl:

diff --git a/modules/lua/lua_config.c b/modules/lua/lua_config.c
index dd77cd8..f4d6e96 100644
--- a/modules/lua/lua_config.c
+++ b/modules/lua/lua_config.c
@@ -174,7 +174,10 @@ static int cmd_log_at(lua_State *L, int level)
      lua_getinfo(L, "Sl", &dbg);

      msg = luaL_checkstring(L, 2);
-    ap_log_error(dbg.source, dbg.currentline, level, 0, cmd->server, msg);
+/* Needs fixing for per-module-loglevel */
+#if 0
+        ap_log_error(dbg.source, dbg.currentline, level, 0, 
cmd->server, msg);
+#endif
      return 0;
  }

diff --git a/modules/lua/lua_request.c b/modules/lua/lua_request.c
index a18c74d..06609c6 100644
--- a/modules/lua/lua_request.c
+++ b/modules/lua/lua_request.c
@@ -407,7 +407,10 @@ static int req_log_at(lua_State *L, int level)
      lua_getinfo(L, "Sl", &dbg);

      msg = luaL_checkstring(L, 2);
+/* Needs fixing for per-module-loglevel */
+#if 0
      ap_log_rerror(dbg.source, dbg.currentline, level, 0, r, msg);
+#endif
      return 0;
  }

> Since there has been some positive comments about the changes, I will
> commit before the next alpha (my current plan is Sunday). But anyone
> is of course welcome to review them before then.

With those changes the build runs successful and first tests look good 
(I can see new trace log entries and I can reconfigure loglevel with 
graceful restart). I agree you should go ahead. Congratulations, very 
useful!

Regards,

Rainer