You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Glenn <gs...@gluelogic.com> on 2003/06/25 01:24:39 UTC

[PATCH] ErrorLogsWithVhost for Apache 1.3.28

This adds a directive ErrorLogsWithVhost that extends the error log format
by adding server name and port to entries in the error logs.
Proof of concept.  Are there better ways to do this than using a global?

Cheers,
Glenn

diff -ru apache_1.3.27/src/include/http_conf_globals.h apache_1.3.27.new/src/include/http_conf_globals.h
--- apache_1.3.27/src/include/http_conf_globals.h	2002-09-27 13:40:24.000000000 -0400
+++ apache_1.3.27.new/src/include/http_conf_globals.h	2003-05-28 01:21:42.000000000 -0400
@@ -90,6 +90,7 @@
 extern API_VAR_EXPORT int ap_daemons_limit;
 extern API_VAR_EXPORT int ap_suexec_enabled;
 extern API_VAR_EXPORT int ap_listenbacklog;
+extern int ap_error_logs_with_vhost;
 #ifdef SO_ACCEPTFILTER
 extern int ap_acceptfilter;
 #endif
diff -ru apache_1.3.27/src/main/http_core.c apache_1.3.27.new/src/main/http_core.c
--- apache_1.3.27/src/main/http_core.c	2002-09-30 12:35:21.000000000 -0400
+++ apache_1.3.27.new/src/main/http_core.c	2003-05-28 01:23:07.000000000 -0400
@@ -2063,6 +2063,18 @@
     return NULL;
 }
 
+static const char *set_error_logs_with_vhost(cmd_parms *cmd, void *dummy)
+{
+    const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
+    if (err != NULL) {
+        return err;
+    }
+
+    ap_error_logs_with_vhost = 1;
+    return NULL;
+}
+
+
 static const char *set_send_buffer_size(cmd_parms *cmd, void *dummy, char *arg)
 {
     int s = atoi(arg);
@@ -3308,6 +3320,8 @@
 { "ErrorLog", set_server_string_slot,
   (void *)XtOffsetOf (server_rec, error_fname), RSRC_CONF, TAKE1,
   "The filename of the error log" },
+{ "ErrorLogsWithVhost", set_error_logs_with_vhost, NULL, RSRC_CONF, NO_ARGS,
+  "Include the virtual host and port, if available, in error log entries" },
 { "PidFile", set_pidfile, NULL, RSRC_CONF, TAKE1,
     "A file for logging the server process ID"},
 { "ScoreBoardFile", set_scoreboard, NULL, RSRC_CONF, TAKE1,
diff -ru apache_1.3.27/src/main/http_log.c apache_1.3.27.new/src/main/http_log.c
--- apache_1.3.27/src/main/http_log.c	2002-05-19 00:55:38.000000000 -0400
+++ apache_1.3.27.new/src/main/http_log.c	2003-05-28 01:24:38.000000000 -0400
@@ -388,8 +388,15 @@
 	 * quad is the most secure, which is why I'm implementing it
 	 * first. -djg
 	 */
-	len += ap_snprintf(errstr + len, sizeof(errstr) - len,
-		"[client %s] ", r->connection->remote_ip);
+	if (ap_error_logs_with_vhost && r->server) {
+	    len += ap_snprintf(errstr + len, sizeof(errstr) - len,
+		    "[client %s] [%s:%d] ", r->connection->remote_ip,
+		    r->server->server_hostname, r->server->port);
+	}
+	else {
+	    len += ap_snprintf(errstr + len, sizeof(errstr) - len,
+		    "[client %s] ", r->connection->remote_ip);
+	}
     }
     if (!(level & APLOG_NOERRNO)
 	&& (save_errno != 0)
diff -ru apache_1.3.27/src/main/http_main.c apache_1.3.27.new/src/main/http_main.c
--- apache_1.3.27/src/main/http_main.c	2002-09-27 13:40:24.000000000 -0400
+++ apache_1.3.27.new/src/main/http_main.c	2003-05-28 01:22:09.000000000 -0400
@@ -257,6 +257,7 @@
 API_VAR_EXPORT time_t ap_restart_time=0;
 API_VAR_EXPORT int ap_suexec_enabled = 0;
 API_VAR_EXPORT int ap_listenbacklog=0;
+int ap_error_logs_with_vhost=0;
 
 struct accept_mutex_methods_s {
     void (*child_init)(pool *p);

Re: [PATCH] ErrorLogsWithVhost for Apache 1.3.28

Posted by Astrid Keßler <ke...@kess-net.de>.
>> What about a second parameter to ErrorLog?

> That's an interesting idea.
> I can change ErrorLog to a TAKE12 to take an optional parameter so as not
> to break existing configs.  Is this method preferred to a second directive?

I can't tell, this is preferred or this is not preferred, espacially not
for 1.3. It's only my personal opinion not to create a new directive for
every small behavior option. So if you ask me: yes I would preferr this.
:) But maybe there are other opinions ...

 Kess

Re: [PATCH] ErrorLogsWithVhost for Apache 1.3.28

Posted by Glenn <gs...@gluelogic.com>.
On Fri, Jun 27, 2003 at 07:21:09PM -0700, Andrew Ho wrote:
> It would be great to have a configuration directive to separate out stderr
> messages from messages generated by Apache modules. It's often desirable
> to parse error logs to generate statistics on 404s, etc. and stderr output
> makes this more difficult.

I agree with Joshua that the access log is a much better place from which
to generate statistics.

> This would also help the case where you have 100 VirtualHosts, and you
> want to use something like ErrorLogsWithVhost to use a single error_log.
> You'd want to redirect stderr to a different place for each VirtualHost
> (or else you couldn't see which VirtualHost stderr output came from).

I'm guessing that you are referring to stderr from CGI scripts, which
can be voluminous.

I believe that Apache 2.x reads in CGI stderr and formats it consistently
into the ErrorLog, so if ErrorLogsWithVhost or some facimile thereof
eventually makes it into Apache 2.x, then you'll be able to robustly
split out a single error log, including CGI stderr output.

This is not the case in Apache 1.3 where CGI stderr goes directly to the
error log (on unix-like systems because of file descriptor inheritance).
Thinking aloud, it might be an interesting project (wish I had time) to
create a directive along the lines of ScriptLog that would work with
mod_cgi.  After mod_cgi fork'ed, the child could open a vhost-specific
CGI stderr log and then could dup it to stderr before exec'ing the CGI.
This might even work well for mass vhosting.

Cheers,
Glenn

Re: [PATCH] ErrorLogsWithVhost for Apache 1.3.28

Posted by Glenn <gs...@gluelogic.com>.
On Tue, Jul 08, 2003 at 12:41:09AM -0400, Glenn wrote:
> With the talk about a minor MMN bump, I put together this patch which
> adds a flag at the end of server_rec.  This also changes ErrorLog to
> a TAKE12, with an optional style of "default" or "vhosts", where the
> "vhosts" includes the server name and port in the error log entries.
> The TAKE12 maintains backwards compatibility to existing config files.
> 
> Comments appreciated on the method(s) that would most likely get this
> accepted into 1.3.28 or 1.3.29.  (global flag, server_rec addition,
> other ...)  Thanks!

I reworked this patch (from July!) for 1.3.28.  Comments appreciated.

Cheers,
Glenn


diff -ru apache_1.3.28/src/include/httpd.h apache_1.3.28.new/src/include/httpd.h
--- apache_1.3.28/src/include/httpd.h	2003-07-16 16:20:26.000000000 -0400
+++ apache_1.3.28.new/src/include/httpd.h	2003-09-04 06:25:26.000000000 -0400
@@ -967,8 +967,13 @@
     int limit_req_line;      /* limit on size of the HTTP request line    */
     int limit_req_fieldsize; /* limit on size of any request header field */
     int limit_req_fields;    /* limit on number of request header fields  */
+
+    int error_format_style;  /* format style of error log entries */
 };
 
+/* error format styles for log entry layout */
+enum { ERRORLOG_DEFAULT_STYLE = 0, ERRORLOG_WITH_VHOSTS = 1 };
+
 /* These are more like real hosts than virtual hosts */
 struct listen_rec {
     listen_rec *next;
Only in apache_1.3.28.new/src/include: httpd.h.orig
diff -ru apache_1.3.28/src/main/http_core.c apache_1.3.28.new/src/main/http_core.c
--- apache_1.3.28/src/main/http_core.c	2003-07-07 09:02:28.000000000 -0400
+++ apache_1.3.28.new/src/main/http_core.c	2003-09-04 06:25:26.000000000 -0400
@@ -2076,6 +2076,35 @@
     return NULL;
 }
 
+static const char *set_error_log(cmd_parms *cmd, void *dummy,
+				 char *arg1, char *arg2)
+{
+    const char *err = ap_check_cmd_context(cmd,
+					   NOT_IN_DIR_LOC_FILE|NOT_IN_LIMIT);
+    if (err != NULL) {
+	return err;
+    }
+
+    if (arg2 == NULL) {
+	cmd->server->error_format_style = ERRORLOG_DEFAULT_STYLE;
+	cmd->server->error_fname = arg1;
+    }
+    else {
+	cmd->server->error_fname = arg2;
+	if (strcmp(arg1, "vhosts") == 0) {
+	    cmd->server->error_format_style = ERRORLOG_WITH_VHOSTS;
+	}
+	else if (strcmp(arg1, "default") == 0) {
+	    cmd->server->error_format_style = ERRORLOG_DEFAULT_STYLE;
+	}
+	else {
+	    return "invalid ErrorLog syntax";
+	}
+    }
+
+    return NULL;
+}
+
 static const char *set_send_buffer_size(cmd_parms *cmd, void *dummy, char *arg)
 {
     int s = atoi(arg);
@@ -3450,9 +3479,8 @@
   "En-/disable server signature (on|off|email)" },
 { "ServerRoot", set_server_root, NULL, RSRC_CONF, TAKE1,
   "Common directory of server-related files (logs, confs, etc.)" },
-{ "ErrorLog", set_server_string_slot,
-  (void *)XtOffsetOf (server_rec, error_fname), RSRC_CONF, TAKE1,
-  "The filename of the error log" },
+{ "ErrorLog", set_error_log, NULL, RSRC_CONF, TAKE12,
+  "The log style tag (default|vhosts) and filename of the error log" },
 { "PidFile", set_pidfile, NULL, RSRC_CONF, TAKE1,
     "A file for logging the server process ID"},
 { "ScoreBoardFile", set_scoreboard, NULL, RSRC_CONF, TAKE1,
Only in apache_1.3.28.new/src/main: http_core.c.orig
diff -ru apache_1.3.28/src/main/http_log.c apache_1.3.28.new/src/main/http_log.c
--- apache_1.3.28/src/main/http_log.c	2003-02-03 12:13:21.000000000 -0500
+++ apache_1.3.28.new/src/main/http_log.c	2003-09-04 06:26:43.000000000 -0400
@@ -388,8 +388,15 @@
 	 * quad is the most secure, which is why I'm implementing it
 	 * first. -djg
 	 */
-	len += ap_snprintf(errstr + len, sizeof(errstr) - len,
-		"[client %s] ", r->connection->remote_ip);
+	if (r->server->error_format_style == ERRORLOG_WITH_VHOSTS) {
+	    len += ap_snprintf(errstr + len, sizeof(errstr) - len,
+		    "[client %s] [%s:%d] ", r->connection->remote_ip,
+		    r->server->server_hostname, r->server->port);
+	}
+	else { /* r->error_format_style == ERRORLOG_DEFAULT_STYLE */
+	    len += ap_snprintf(errstr + len, sizeof(errstr) - len,
+		    "[client %s] ", r->connection->remote_ip);
+	}
     }
     if (!(level & APLOG_NOERRNO)
 	&& (save_errno != 0)

Re: [PATCH] ErrorLogsWithVhost for Apache 1.3.28

Posted by Glenn <gs...@gluelogic.com>.
On Thu, Jun 26, 2003 at 11:18:41AM -0400, Glenn wrote:
> On Thu, Jun 26, 2003 at 09:01:21AM +0200, Astrid Ke?ler wrote:
> > What about a second parameter to ErrorLog?
> 
> That's an interesting idea.
> I can change ErrorLog to a TAKE12 to take an optional parameter so as not
> to break existing configs.  Is this method preferred to a second directive?
> 
> I am not suggesting supporting everything in LogFormat; only a few key
> words.  Initially, there would only be two key words for error log format,
> default and withvhost, or something like that.
> 
> I'd still rather set a global variable for withvhosts than to extend any
> existing structures, but would like to know if there are other cleaner
> methods.

With the talk about a minor MMN bump, I put together this patch which
adds a flag at the end of server_rec.  This also changes ErrorLog to
a TAKE12, with an optional style of "default" or "vhosts", where the
"vhosts" includes the server name and port in the error log entries.
The TAKE12 maintains backwards compatibility to existing config files.

Comments appreciated on the method(s) that would most likely get this
accepted into 1.3.28 or 1.3.29.  (global flag, server_rec addition,
other ...)  Thanks!

-Glenn


diff -ru apache_1.3.27/src/include/httpd.h apache_1.3.27.new/src/include/httpd.h
--- apache_1.3.27/src/include/httpd.h	2002-09-30 12:35:21.000000000 -0400
+++ apache_1.3.27.new/src/include/httpd.h	2003-07-07 19:12:08.000000000 -0400
@@ -966,8 +966,13 @@
     int limit_req_line;      /* limit on size of the HTTP request line    */
     int limit_req_fieldsize; /* limit on size of any request header field */
     int limit_req_fields;    /* limit on number of request header fields  */
+
+    int error_format_style;  /* format style of error log entries */
 };
 
+/* error format styles for log entry layout */
+enum { ERRORLOG_DEFAULT_STYLE = 0, ERRORLOG_WITH_VHOSTS = 1 };
+
 /* These are more like real hosts than virtual hosts */
 struct listen_rec {
     listen_rec *next;
diff -ru apache_1.3.27/src/main/http_core.c apache_1.3.27.new/src/main/http_core.c
--- apache_1.3.27/src/main/http_core.c	2002-09-30 12:35:21.000000000 -0400
+++ apache_1.3.27.new/src/main/http_core.c	2003-07-07 19:38:11.000000000 -0400
@@ -2063,6 +2063,35 @@
     return NULL;
 }
 
+static const char *set_error_log(cmd_parms *cmd, void *dummy,
+				 char *arg1, char *arg2)
+{
+    const char *err = ap_check_cmd_context(cmd,
+					   NOT_IN_DIR_LOC_FILE|NOT_IN_LIMIT);
+    if (err != NULL) {
+	return err;
+    }
+
+    if (arg2 == NULL) {
+	cmd->server->error_format_style = ERRORLOG_DEFAULT_STYLE;
+	cmd->server->error_fname = arg1;
+    }
+    else {
+	cmd->server->error_fname = arg2;
+	if (strcmp(arg1, "vhosts") == 0) {
+	    cmd->server->error_format_style = ERRORLOG_WITH_VHOSTS;
+	}
+	else if (strcmp(arg1, "default") == 0) {
+	    cmd->server->error_format_style = ERRORLOG_DEFAULT_STYLE;
+	}
+	else {
+	    return "invalid ErrorLog syntax";
+	}
+    }
+
+    return NULL;
+}
+
 static const char *set_send_buffer_size(cmd_parms *cmd, void *dummy, char *arg)
 {
     int s = atoi(arg);
@@ -3305,9 +3334,8 @@
   "En-/disable server signature (on|off|email)" },
 { "ServerRoot", set_server_root, NULL, RSRC_CONF, TAKE1,
   "Common directory of server-related files (logs, confs, etc.)" },
-{ "ErrorLog", set_server_string_slot,
-  (void *)XtOffsetOf (server_rec, error_fname), RSRC_CONF, TAKE1,
-  "The filename of the error log" },
+{ "ErrorLog", set_error_log, NULL, RSRC_CONF, TAKE12,
+  "The log style tag (default|vhosts) and filename of the error log" },
 { "PidFile", set_pidfile, NULL, RSRC_CONF, TAKE1,
     "A file for logging the server process ID"},
 { "ScoreBoardFile", set_scoreboard, NULL, RSRC_CONF, TAKE1,
diff -ru apache_1.3.27/src/main/http_log.c apache_1.3.27.new/src/main/http_log.c
--- apache_1.3.27/src/main/http_log.c	2002-05-19 00:55:38.000000000 -0400
+++ apache_1.3.27.new/src/main/http_log.c	2003-07-07 19:43:44.000000000 -0400
@@ -388,8 +388,15 @@
 	 * quad is the most secure, which is why I'm implementing it
 	 * first. -djg
 	 */
-	len += ap_snprintf(errstr + len, sizeof(errstr) - len,
-		"[client %s] ", r->connection->remote_ip);
+	if (r->server && r->server->error_format_style == ERRORLOG_WITH_VHOSTS){
+	    len += ap_snprintf(errstr + len, sizeof(errstr) - len,
+		    "[client %s] [%s:%d] ", r->connection->remote_ip,
+		    r->server->server_hostname, r->server->port);
+	}
+	else { /* no r->server or error_format_style == ERRORLOG_DEFAULT_STYLE*/
+	    len += ap_snprintf(errstr + len, sizeof(errstr) - len,
+		    "[client %s] ", r->connection->remote_ip);
+	}
     }
     if (!(level & APLOG_NOERRNO)
 	&& (save_errno != 0)

Re: [PATCH] ErrorLogsWithVhost for Apache 1.3.28

Posted by Joshua Slive <jo...@slive.ca>.
On Fri, 27 Jun 2003, Andrew Ho wrote:
> It would be great to have a configuration directive to separate out stderr
> messages from messages generated by Apache modules. It's often desirable
> to parse error logs to generate statistics on 404s, etc. and stderr output
> makes this more difficult.

I don't see a very good justification for this.  This type of
request-correlated information can just as easily be obtained from
examining status codes in the access log.

Joshua.

Re: [PATCH] ErrorLogsWithVhost for Apache 1.3.28

Posted by Andrew Ho <an...@tellme.com>.
Hello,

Just to add to this discussion--

It would be great to have a configuration directive to separate out stderr
messages from messages generated by Apache modules. It's often desirable
to parse error logs to generate statistics on 404s, etc. and stderr output
makes this more difficult.

This would also help the case where you have 100 VirtualHosts, and you
want to use something like ErrorLogsWithVhost to use a single error_log.
You'd want to redirect stderr to a different place for each VirtualHost
(or else you couldn't see which VirtualHost stderr output came from).

Humbly,

Andrew

----------------------------------------------------------------------
Andrew Ho               http://www.tellme.com/       andrew@tellme.com
Engineer                    1-800-555-TELL          Voice 650-930-9062
Tellme Networks, Inc.                                 Fax 650-930-9101
----------------------------------------------------------------------


Re: [PATCH] ErrorLogsWithVhost for Apache 1.3.28

Posted by Glenn <gs...@gluelogic.com>.
On Thu, Jun 26, 2003 at 09:01:21AM +0200, Astrid Ke?ler wrote:
> > On Thu, Jun 26, 2003 at 12:04:31AM +0200, Astrid Ke?ler wrote:
> >> > This adds a directive ErrorLogsWithVhost that extends the error log format
> >> > by adding server name and port to entries in the error logs.
> >>
> >> This can also be done with LogFormat using %v and %p. What is
> >> wrong with these log formats?
> 
> > "LogFormat" works well with "CustomLog" and "TransferLog" in mod_log_config,
> > but "ErrorLog" is in the core and does not support "LogFormat".
> 
> You are right. My fault, I didn't read carefully enough. As Joshua I'm
> +1 for the concept. But I'm not sure if we need a separate directive.
> What about a second parameter to ErrorLog?

That's an interesting idea.
I can change ErrorLog to a TAKE12 to take an optional parameter so as not
to break existing configs.  Is this method preferred to a second directive?

I am not suggesting supporting everything in LogFormat; only a few key
words.  Initially, there would only be two key words for error log format,
default and withvhost, or something like that.

I'd still rather set a global variable for withvhosts than to extend any
existing structures, but would like to know if there are other cleaner
methods.

Thanks.
Glenn

Re: [PATCH] ErrorLogsWithVhost for Apache 1.3.28

Posted by Astrid Keßler <ke...@kess-net.de>.
> On Thu, Jun 26, 2003 at 12:04:31AM +0200, Astrid Ke?ler wrote:
>> > This adds a directive ErrorLogsWithVhost that extends the error log format
>> > by adding server name and port to entries in the error logs.
>>
>> This can also be done with LogFormat using %v and %p. What is
>> wrong with these log formats?

> "LogFormat" works well with "CustomLog" and "TransferLog" in mod_log_config,
> but "ErrorLog" is in the core and does not support "LogFormat".

You are right. My fault, I didn't read carefully enough. As Joshua I'm
+1 for the concept. But I'm not sure if we need a separate directive.
What about a second parameter to ErrorLog?

 Kess

Re: [PATCH] ErrorLogsWithVhost for Apache 1.3.28

Posted by Joshua Slive <jo...@slive.ca>.
On Thu, 26 Jun 2003, Astrid Keßler wrote:

> > This adds a directive ErrorLogsWithVhost that extends the error log format
> > by adding server name and port to entries in the error logs.
>
> This can also be done with LogFormat using %v and %p. What is
> wrong with these log formats?

LogFormat only works for access logs, not for error logs.

By the way, I'm +1 on the concept of having the possibility of including
this info in the error log, but I haven't reviewed the patch.

Joshua.

Re: [PATCH] ErrorLogsWithVhost for Apache 1.3.28

Posted by Glenn <gs...@gluelogic.com>.
On Thu, Jun 26, 2003 at 12:04:31AM +0200, Astrid Ke?ler wrote:
> > This adds a directive ErrorLogsWithVhost that extends the error log format
> > by adding server name and port to entries in the error logs.
> 
> This can also be done with LogFormat using %v and %p. What is
> wrong with these log formats?

"LogFormat" works well with "CustomLog" and "TransferLog" in mod_log_config,
but "ErrorLog" is in the core and does not support "LogFormat".

Cheers,
Glenn

Re: [PATCH] ErrorLogsWithVhost for Apache 1.3.28

Posted by Astrid Keßler <ke...@kess-net.de>.
> This adds a directive ErrorLogsWithVhost that extends the error log format
> by adding server name and port to entries in the error logs.

This can also be done with LogFormat using %v and %p. What is
wrong with these log formats?

Kess