Re: error log providers, multiple vhosts, mod_syslog

2013-11-07 Thread Joe Orton
On Thu, Oct 17, 2013 at 12:33:50PM +, Plüm, Rüdiger, Vodafone Group wrote:
> Hmm. This points out another issue when using an error log provider for the 
> main server log:
> We lose everything that the server or other programs like CGI-scripts write 
> to the stderr FD as it
> is simply written to /dev/null. Don't we need to have a separate process in 
> this case that
> like a piped logger reads from the reading end of the "stderr pipe" and 
> writes it
> via ap_server_conf->errorlog_provider->writer to the log?

Jumping in here...

Actually there should be few cases where modules write to stderr, no?  
I'm not sure we should even consider "writes to stderr get logged" a 
valid part of the module API.  "If you're not using ap_log_*error, 
you're out of luck" is not totally unreasonable.

mod_cgi does intercept stderr and logs it properly via ap_log_rerror, 
and mod_cgid does stderr logging differently anyway.

One case which is common is the use of apr_procattr_child_errfn_set() to 
write exec() errors to stderr after forking a child.  But perhaps we 
could and should provide a single common implementation of that which 
does something more useful.

With "ErrorLog syslog" in current 2.x (and 1.3?), stderr is /dev/null 
already, so the status quo is that we lose stuff, though that is kind of 
ugly.

Regards, Joe


Re: error log providers, multiple vhosts, mod_syslog

2013-11-07 Thread Jan Kaluža

On 11/07/2013 11:11 AM, Joe Orton wrote:

On Thu, Oct 17, 2013 at 12:33:50PM +, Plüm, Rüdiger, Vodafone Group wrote:

Hmm. This points out another issue when using an error log provider for the 
main server log:
We lose everything that the server or other programs like CGI-scripts write to 
the stderr FD as it
is simply written to /dev/null. Don't we need to have a separate process in 
this case that
like a piped logger reads from the reading end of the "stderr pipe" and writes 
it
via ap_server_conf->errorlog_provider->writer to the log?


Jumping in here...

Actually there should be few cases where modules write to stderr, no?
I'm not sure we should even consider "writes to stderr get logged" a
valid part of the module API.  "If you're not using ap_log_*error,
you're out of luck" is not totally unreasonable.

mod_cgi does intercept stderr and logs it properly via ap_log_rerror,
and mod_cgid does stderr logging differently anyway.

One case which is common is the use of apr_procattr_child_errfn_set() to
write exec() errors to stderr after forking a child.  But perhaps we
could and should provide a single common implementation of that which
does something more useful.

With "ErrorLog syslog" in current 2.x (and 1.3?), stderr is /dev/null
already, so the status quo is that we lose stuff, though that is kind of
ugly.


Reading this I think I could commit my httpd-trunk-stderr-provider.patch 
from older email in this thread. It does not fix stderr logging, but it 
fixes httpd-trunk problem where messages with NULL server_rec* are not 
logged.


I'm attaching better version of that patch here (it fixes potential 
crash pointed out by Rüdiger when both logf and error_log_provider are 
NULL).


If anyone is against it or has better idea how to fix logging of 
messages with NULL server_rec* when log providers are used, tell me.



Regards, Joe



Regards,
Jan Kaluza

Index: server/log.c
===
--- server/log.c	(revision 1539566)
+++ server/log.c	(working copy)
@@ -437,9 +437,12 @@
 #define NULL_DEVICE "/dev/null"
 #endif
 
-if (replace_stderr && freopen(NULL_DEVICE, "w", stderr) == NULL) {
-ap_log_error(APLOG_MARK, APLOG_CRIT, errno, s_main, APLOGNO(00093)
- "unable to replace stderr with %s", NULL_DEVICE);
+if (replace_stderr) {
+if (freopen(NULL_DEVICE, "w", stderr) == NULL) {
+ap_log_error(APLOG_MARK, APLOG_CRIT, errno, s_main, APLOGNO(00093)
+"unable to replace stderr with %s", NULL_DEVICE);
+}
+stderr_log = NULL;
 }
 
 for (virt = s_main->next; virt; virt = virt->next) {
@@ -1034,6 +1037,8 @@
 const request_rec *rmain = NULL;
 core_server_config *sconf = NULL;
 ap_errorlog_info info;
+ap_errorlog_provider *errorlog_provider = NULL;
+void *errorlog_provider_handle = NULL;
 
 /* do we need to log once-per-req or once-per-conn info? */
 int log_conn_info = 0, log_req_info = 0;
@@ -1060,6 +1065,10 @@
 #endif
 
 logf = stderr_log;
+if (!logf && ap_server_conf && ap_server_conf->errorlog_provider) {
+errorlog_provider = ap_server_conf->errorlog_provider;
+errorlog_provider_handle = ap_server_conf->errorlog_provider_handle;
+}
 }
 else {
 int configured_level = r ? ap_get_request_module_loglevel(r, module_index):
@@ -1078,6 +1087,9 @@
 logf = s->error_log;
 }
 
+errorlog_provider = s->errorlog_provider;
+errorlog_provider_handle = s->errorlog_provider_handle;
+
 /* the faked server_rec from mod_cgid does not have s->module_config */
 if (s->module_config) {
 sconf = ap_get_core_module_config(s->module_config);
@@ -1106,6 +1118,14 @@
 }
 }
 
+if (!logf && !errorlog_provider) {
+/* There is no file to send the log message to (or it is
+ * redirected to /dev/null and therefore any formating done below
+ * would be lost anyway) and there is no log provider available, so
+ * we just return here. */
+return;
+}
+
 info.s = s;
 info.c = c;
 info.pool  = pool;
@@ -1191,7 +1211,7 @@
 continue;
 }
 
-if (logf || (s->errorlog_provider->flags &
+if (logf || (errorlog_provider->flags &
 AP_ERRORLOG_PROVIDER_ADD_EOL_STR)) {
 /* Truncate for the terminator (as apr_snprintf does) */
 if (len > MAX_STRING_LEN - sizeof(APR_EOL_STR)) {
@@ -1205,8 +1225,8 @@
 write_logline(errstr, len, logf, level_and_mask);
 }
 else {
-s->errorlog_provider->writer(&info, s->errorlog_provider_handle,
- errstr, len);
+errorlog_provider->writer(&info, errorlog_provider_handle,
+  errstr, len);
 }
 
 if (done) {

RE: error log providers, multiple vhosts, mod_syslog

2013-11-07 Thread Plüm , Rüdiger , Vodafone Group
Seems fine.

Regards

Rüdiger

> -Original Message-
> From: Jan Kaluža 
> Sent: Donnerstag, 7. November 2013 13:09
> To: dev@httpd.apache.org
> Subject: Re: error log providers, multiple vhosts, mod_syslog
> 
> On 11/07/2013 11:11 AM, Joe Orton wrote:
> > On Thu, Oct 17, 2013 at 12:33:50PM +, Plüm, Rüdiger, Vodafone Group
> wrote:
> >> Hmm. This points out another issue when using an error log provider for
> the main server log:
> >> We lose everything that the server or other programs like CGI-scripts
> write to the stderr FD as it
> >> is simply written to /dev/null. Don't we need to have a separate
> process in this case that
> >> like a piped logger reads from the reading end of the "stderr pipe" and
> writes it
> >> via ap_server_conf->errorlog_provider->writer to the log?
> >
> > Jumping in here...
> >
> > Actually there should be few cases where modules write to stderr, no?
> > I'm not sure we should even consider "writes to stderr get logged" a
> > valid part of the module API.  "If you're not using ap_log_*error,
> > you're out of luck" is not totally unreasonable.
> >
> > mod_cgi does intercept stderr and logs it properly via ap_log_rerror,
> > and mod_cgid does stderr logging differently anyway.
> >
> > One case which is common is the use of apr_procattr_child_errfn_set() to
> > write exec() errors to stderr after forking a child.  But perhaps we
> > could and should provide a single common implementation of that which
> > does something more useful.
> >
> > With "ErrorLog syslog" in current 2.x (and 1.3?), stderr is /dev/null
> > already, so the status quo is that we lose stuff, though that is kind of
> > ugly.
> 
> Reading this I think I could commit my httpd-trunk-stderr-provider.patch
> from older email in this thread. It does not fix stderr logging, but it
> fixes httpd-trunk problem where messages with NULL server_rec* are not
> logged.
> 
> I'm attaching better version of that patch here (it fixes potential
> crash pointed out by Rüdiger when both logf and error_log_provider are
> NULL).
> 
> If anyone is against it or has better idea how to fix logging of
> messages with NULL server_rec* when log providers are used, tell me.
> 
> > Regards, Joe
> >
> 
> Regards,
> Jan Kaluza



Re: error log providers, multiple vhosts, mod_syslog

2013-11-07 Thread Jeff Trawick
On Thu, Nov 7, 2013 at 7:09 AM, Jan Kaluža  wrote:

> On 11/07/2013 11:11 AM, Joe Orton wrote:
>
>> On Thu, Oct 17, 2013 at 12:33:50PM +, Plüm, Rüdiger, Vodafone Group
>> wrote:
>>
>>> Hmm. This points out another issue when using an error log provider for
>>> the main server log:
>>> We lose everything that the server or other programs like CGI-scripts
>>> write to the stderr FD as it
>>> is simply written to /dev/null. Don't we need to have a separate process
>>> in this case that
>>> like a piped logger reads from the reading end of the "stderr pipe" and
>>> writes it
>>> via ap_server_conf->errorlog_provider->writer to the log?
>>>
>>
>> Jumping in here...
>>
>> Actually there should be few cases where modules write to stderr, no?
>> I'm not sure we should even consider "writes to stderr get logged" a
>> valid part of the module API.  "If you're not using ap_log_*error,
>> you're out of luck" is not totally unreasonable.
>>
>> mod_cgi does intercept stderr and logs it properly via ap_log_rerror,
>> and mod_cgid does stderr logging differently anyway.
>>
>> One case which is common is the use of apr_procattr_child_errfn_set() to
>> write exec() errors to stderr after forking a child.  But perhaps we
>> could and should provide a single common implementation of that which
>> does something more useful.
>>
>> With "ErrorLog syslog" in current 2.x (and 1.3?), stderr is /dev/null
>> already, so the status quo is that we lose stuff, though that is kind of
>> ugly.
>>
>
> Reading this I think I could commit my httpd-trunk-stderr-provider.patch
> from older email in this thread. It does not fix stderr logging, but it
> fixes httpd-trunk problem where messages with NULL server_rec* are not
> logged.
>
> I'm attaching better version of that patch here (it fixes potential crash
> pointed out by Rüdiger when both logf and error_log_provider are NULL).
>
> If anyone is against it or has better idea how to fix logging of messages
> with NULL server_rec* when log providers are used, tell me.
>

Is the observation about NULL server_rec for modules that weren't updated
properly to pass ap_server_conf when no server_rec was available, or for
some window where ap_server_conf is NULL but server_rec is needed?


>  Regards, Joe
>>
>>
> Regards,
> Jan Kaluza
>
>


-- 
Born in Roswell... married an alien...
http://emptyhammock.com/


Re: error log providers, multiple vhosts, mod_syslog

2013-11-07 Thread Jan Kaluza

On 11/07/2013 07:07 PM, Jeff Trawick wrote:

On Thu, Nov 7, 2013 at 7:09 AM, Jan Kaluža mailto:jkal...@redhat.com>> wrote:

On 11/07/2013 11:11 AM, Joe Orton wrote:

On Thu, Oct 17, 2013 at 12:33:50PM +, Plüm, Rüdiger,
Vodafone Group wrote:

Hmm. This points out another issue when using an error log
provider for the main server log:
We lose everything that the server or other programs like
CGI-scripts write to the stderr FD as it
is simply written to /dev/null. Don't we need to have a
separate process in this case that
like a piped logger reads from the reading end of the
"stderr pipe" and writes it
via ap_server_conf->errorlog___provider->writer to the log?


Jumping in here...

Actually there should be few cases where modules write to
stderr, no?
I'm not sure we should even consider "writes to stderr get logged" a
valid part of the module API.  "If you're not using ap_log_*error,
you're out of luck" is not totally unreasonable.

mod_cgi does intercept stderr and logs it properly via
ap_log_rerror,
and mod_cgid does stderr logging differently anyway.

One case which is common is the use of
apr_procattr_child_errfn_set() to
write exec() errors to stderr after forking a child.  But perhaps we
could and should provide a single common implementation of that
which
does something more useful.

With "ErrorLog syslog" in current 2.x (and 1.3?), stderr is
/dev/null
already, so the status quo is that we lose stuff, though that is
kind of
ugly.


Reading this I think I could commit my
httpd-trunk-stderr-provider.__patch from older email in this thread.
It does not fix stderr logging, but it fixes httpd-trunk problem
where messages with NULL server_rec* are not logged.

I'm attaching better version of that patch here (it fixes potential
crash pointed out by Rüdiger when both logf and error_log_provider
are NULL).

If anyone is against it or has better idea how to fix logging of
messages with NULL server_rec* when log providers are used, tell me.


Is the observation about NULL server_rec for modules that weren't
updated properly to pass ap_server_conf when no server_rec was
available, or for some window where ap_server_conf is NULL but
server_rec is needed?


I've originally observed that right in log.c in open_error_log(). 
ap_log_error calls there have NULL server_rec and in case something goes 
wrong during vhost logger initialization, the error messages about that 
are lost in case you use error log provider in ap_server_conf. That's 
what I'm trying to fix by that patch.


I haven't checked if there are other places where NULL server_rec is 
used, but I would say it's pretty common during httpd startup.


Jan Kaluza



Regards, Joe


Regards,
Jan Kaluza




--
Born in Roswell... married an alien...
http://emptyhammock.com/