You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Randy Terbush <ra...@zyzzyva.com> on 1997/07/12 15:39:44 UTC

Re: [PATCH] loglevels

> Although I like the idea of different error logging levels, one
> thing that has always bothered me with the concept (in general)
> is that what is "information" and what is "debugging" is murky.
> What I liked about how we did it in Apache was anything logged in
> error.log was important enough to make note of and we didn't
> have to bother with which "level" it was.

Having 8 log levels is admittedly questionable and I could easily 
be convinced to reduce those options. However, the server is 
unnecessarily verbose about some things. For example, every time a 
browser doesn't wait to download an entire page. On busy servers, 
entries like this are being logged every second. From an admin 
viewpoint, I would like to be able to turn off every error about 
not being able to find a file. That is usually not my problem. 
Being able to drop a .htaccess entry into a directory and see DEBUG 
level error output would be very useful IMO.

Replacing existing functions could be the most difficult part of 
this from what I have found. We probably need to agree on a loose 
rule of where we use different levels. My approach to this is has 
been to maintain the same format for output:

log_reason would be replaced by ap_log(APLOG_INFO
log_unixerr would be replaced by ap_log(APLOG_CRIT
log_printf would be replaced by ap_log(APLOG_ERR (but varies)
log_error would be replaced by ap_log(APLOG_ERR 

Looking through the code, this is a reasonable way to start, but it 
seems there will need to be considerable judgments made along the 
way.

As Dean suggested, I think it would also be nice to make the 
necessary changes to allow us to get rid of fprintf() usage as well 
and use this for all error logging.

My biggest concern is how to replace the old functions and maintain 
some backward compatibility for these routines. I suppose that 
could be done by just ignoring the missing request_rec and 
defaulting the level to _DEBUG. It would just not be controllable 
if older modules don't use the new function.

I like the suggestion about choosing a different logging facility. 
I had considered making YACD, but this option would allow us not to.

> 
> Still, it seems that this makes sense and is really needed.
> 
> I really like the syslog addition although I'd like to see the
> facility part selectable. Maybe something like syslog-local1
> to use LOG_LOCAL1 for ErrorLog might be nice.
> 
> Randy Terbush wrote:
> > 
> > New attempt below. I'm tired and this is lightly tested, so be 
> > nice. Biggest difference from the previous attempt is that I have 
> > added support for error logging via syslog if ErrorLog is set to 
> > 'syslog' in the config files. Level definitions follows syslog as 
> > well. I have also left the old log routines untouched thus far.
> > 
> > Problem: What do you suggest that we do about compatibility for the 
> > old routines? Since ap_log() is configurable on a per directory 
> > basis, I need a request_rec *. Is there a way to get at the 
> > per_dir_config from a server_rec that I am forgetting about?
> > 
> > I've included only one example in the patch where a log_reason is 
> > replaced with ap_log().
> > 
> > Comments please.
> > 
> > 
> > Index: http_core.c
> > ===================================================================
> > RCS file: /export/home/cvs/apache/src/http_core.c,v
> > retrieving revision 1.92
> > diff -c -c -r1.92 http_core.c
> > *** http_core.c	1997/07/08 04:45:27	1.92
> > --- http_core.c	1997/07/12 06:41:08
> > ***************
> > *** 107,112 ****
> > --- 107,114 ----
> >       conf->limit_nproc = NULL;
> >   #endif
> >   
> > +     conf->loglevel = DEFAULT_LOGLEVEL;
> > +     
> >       conf->sec = make_array (a, 2, sizeof(void *));
> >   
> >       return (void *)conf;
> > ***************
> > *** 1197,1202 ****
> > --- 1199,1232 ----
> >       return NULL;
> >   }
> >   
> > + const char *set_loglevel (cmd_parms *cmd, core_dir_config *conf, const char *arg) 
> > + {
> > +     char *str;
> > +     
> > +     if ((str = getword_conf(cmd->pool, &arg))) {
> > + 	if (!strcasecmp(str, "emerg"))
> > + 	    conf->loglevel = APLOG_EMERG;
> > + 	else if (!strcasecmp(str, "alert"))
> > + 	    conf->loglevel = APLOG_ALERT;
> > + 	else if (!strcasecmp(str, "crit"))
> > + 	    conf->loglevel = APLOG_CRIT;
> > +     	else if (!strcasecmp(str, "error"))
> > + 	    conf->loglevel = APLOG_ERR;
> > + 	else if (!strcasecmp(str, "warn"))
> > + 	    conf->loglevel = APLOG_WARNING;
> > +     	else if (!strcasecmp(str, "notice"))
> > + 	    conf->loglevel = APLOG_NOTICE;
> > + 	else if (!strcasecmp(str, "info"))
> > + 	    conf->loglevel = APLOG_INFO;
> > + 	else if (!strcasecmp(str, "debug"))
> > + 	    conf->loglevel = APLOG_DEBUG;
> > +     }
> > +     else
> > + 	return "LogLevel requires level keyword";
> > +     
> > +     return NULL;
> > + }
> > +     
> >   /* Note --- ErrorDocument will now work from .htaccess files.  
> >    * The AllowOverride of Fileinfo allows webmasters to turn it off
> >    */
> > ***************
> > *** 1321,1326 ****
> > --- 1351,1357 ----
> >   { "ThreadsPerChild", set_threads, NULL, RSRC_CONF, TAKE1, "Number of threads a child creates" },
> >   { "ExcessRequestsPerChild", set_excess_requests, NULL, RSRC_CONF, TAKE1, "Maximum number of requests a particular child serves after it is ready to die." },
> >   { "ListenBacklog", set_listenbacklog, NULL, RSRC_CONF, TAKE1, "maximum length of the queue of pending connections, as used by listen(2)" },
> > + { "LogLevel", set_loglevel, (void*)XtOffsetOf(core_dir_config, loglevel), OR_ALL, TAKE1, "set level of verbosity in error logging" },
> >   { NULL },
> >   };
> >   
> > ***************
> > *** 1382,1390 ****
> >       if (r->method_number == M_PUT) return METHOD_NOT_ALLOWED;
> >   
> >       if (r->finfo.st_mode == 0 || (r->path_info && *r->path_info)) {
> > ! 	log_reason("File does not exist",
> > ! 	    r->path_info ? pstrcat(r->pool, r->filename, r->path_info, NULL)
> > ! 		: r->filename, r);
> >   	return NOT_FOUND;
> >       }
> >       if (r->method_number != M_GET) return METHOD_NOT_ALLOWED;
> > --- 1413,1421 ----
> >       if (r->method_number == M_PUT) return METHOD_NOT_ALLOWED;
> >   
> >       if (r->finfo.st_mode == 0 || (r->path_info && *r->path_info)) {
> > ! 	ap_log(APLOG_INFO, r, NULL,
> > ! 	       r->path_info ? pstrcat(r->pool, r->filename, r->path_info, NULL)
> > ! 	       : r->filename, "File does not exist");
> >   	return NOT_FOUND;
> >       }
> >       if (r->method_number != M_GET) return METHOD_NOT_ALLOWED;
> > Index: http_core.h
> > ===================================================================
> > RCS file: /export/home/cvs/apache/src/http_core.h,v
> > retrieving revision 1.21
> > diff -c -c -r1.21 http_core.h
> > *** http_core.h	1997/06/29 17:53:04	1.21
> > --- http_core.h	1997/07/12 06:41:08
> > ***************
> > *** 179,184 ****
> > --- 179,187 ----
> >       struct rlimit *limit_nproc;
> >   #endif
> >   
> > +     /* logging options */
> > +     int loglevel;
> > +     
> >       /* Access control */
> >       array_header *sec;
> >       regex_t *r;
> > Index: http_log.c
> > ===================================================================
> > RCS file: /export/home/cvs/apache/src/http_log.c,v
> > retrieving revision 1.18
> > diff -c -c -r1.18 http_log.c
> > *** http_log.c	1997/06/29 19:19:35	1.18
> > --- http_log.c	1997/07/12 06:41:10
> > ***************
> > *** 58,69 ****
> > --- 58,71 ----
> >    */
> >   
> >   
> > + #define CORE_PRIVATE
> >   #include "httpd.h"
> >   #include "http_config.h"
> >   #include "http_core.h"
> >   #include "http_log.h"
> >   
> >   #include <stdarg.h>
> > + #include <syslog.h>
> >   
> >   static int
> >   error_log_child (void *cmd)
> > ***************
> > *** 144,149 ****
> > --- 146,231 ----
> >           dup2(fileno(s->error_log),STDERR_FILENO);
> >   }
> >   
> > + void ap_log(int level, const request_rec *r, const char *routine,
> > + 	    const char *file, const char *fmt, ...)
> > + {
> > +     int logfile;
> > +     core_dir_config *conf;
> > +     va_list args;
> > +     const char *time;
> > +     char errstr[MAX_STRING_LEN];
> > +     char varstr[MAX_STRING_LEN];
> > +     
> > + 
> > +     conf = get_module_config(r->per_dir_config, &core_module);
> > + 
> > +     if (level <= conf->loglevel)
> > + 	return;
> > + 	
> > +     logfile = strcasecmp(r->server->error_fname, "syslog");
> > + 
> > +     if (logfile)
> > + 	time = get_time();
> > +     
> > +     switch (level) {
> > +     case APLOG_EMERG:
> > +     case APLOG_DEBUG:
> > +     case APLOG_CRIT:
> > +     case APLOG_ALERT:
> > + 	if (file != NULL)
> > + 	    if (logfile)
> > + 		ap_snprintf(errstr, sizeof(errstr), "[%s] %s: %s: %s",
> > + 			    time, routine, file, strerror(errno));
> > + 	    else
> > + 		ap_snprintf(errstr, sizeof(errstr), "%s: %s: %s",
> > + 			    routine, file, strerror(errno));
> > + 		
> > + 	else
> > + 	    if (logfile)
> > + 		ap_snprintf(errstr, sizeof(errstr), "[%s] %s: %s",
> > + 			    time, routine, strerror(errno));
> > + 	    else
> > + 		ap_snprintf(errstr, sizeof(errstr), "%s: %s",
> > + 			    routine, strerror(errno));
> > + 	break;
> > +     case APLOG_INFO:
> > + 	if (logfile)
> > + 	    ap_snprintf(errstr, sizeof(errstr),
> > + 			"[%s] access to %s failed for %s", time, file,
> > + 			get_remote_host(r->connection, r->per_dir_config, REMOTE_NAME));
> > + 	else
> > + 	    ap_snprintf(errstr, sizeof(errstr),
> > + 			"access to %s failed for %s", file,
> > + 			get_remote_host(r->connection, r->per_dir_config, REMOTE_NAME));
> > + 	    
> > + 	break;
> > +     case APLOG_ERR:
> > +     case APLOG_WARNING:
> > +     case APLOG_NOTICE:
> > + 	if (logfile)
> > + 	    ap_snprintf(errstr, sizeof(errstr), "[%s]", time);
> > + 	break;
> > +     }
> > +     
> > +     
> > +     va_start(args, fmt);
> > + 
> > +     if (logfile) {
> > + 	fprintf(r->server->error_log, "%s\n[%s]", errstr, time);
> > + 	vfprintf(r->server->error_log, fmt, args);
> > + 	fflush(r->server->error_log);
> > +     }
> > +     else {
> > + 	openlog("httpd", LOG_NDELAY|LOG_CONS|LOG_PID, LOG_LOCAL7);
> > + 	if (errstr)
> > + 	    syslog(conf->loglevel, "%s", errstr);
> > + 
> > + 	vsyslog(conf->loglevel, fmt, args);
> > +     }
> > +     
> > +     va_end(args);
> > + }
> > + 
> >   void log_pid(pool *p, char *pid_fname) {
> >       FILE *pid_file;
> >   
> > ***************
> > *** 163,170 ****
> >       fflush(s->error_log);
> >   }
> >   
> > ! void
> > ! log_unixerr(const char *routine, const char *file, const char *msg,
> >   	    server_rec *s)
> >   {
> >       const char *p, *q;
> > --- 245,252 ----
> >       fflush(s->error_log);
> >   }
> >   
> > ! 
> > ! void log_unixerr(const char *routine, const char *file, const char *msg,
> >   	    server_rec *s)
> >   {
> >       const char *p, *q;
> > ***************
> > *** 182,189 ****
> >       fflush(err);
> >   }
> >   
> > ! void
> > ! log_printf(const server_rec *s, const char *fmt, ...)
> >   {
> >       va_list args;
> >       
> > --- 264,270 ----
> >       fflush(err);
> >   }
> >   
> > ! void log_printf(const server_rec *s, const char *fmt, ...)
> >   {
> >       va_list args;
> >       
> > Index: http_log.h
> > ===================================================================
> > RCS file: /export/home/cvs/apache/src/http_log.h,v
> > retrieving revision 1.7
> > diff -c -c -r1.7 http_log.h
> > *** http_log.h	1997/01/01 18:10:19	1.7
> > --- http_log.h	1997/07/12 06:41:12
> > ***************
> > *** 50,55 ****
> > --- 50,65 ----
> >    *
> >    */
> >   
> > + #define DEFAULT_LOGLEVEL	3
> > + #define	APLOG_EMERG	0	/* system is unusable */
> > + #define	APLOG_ALERT	1	/* action must be taken immediately */
> > + #define	APLOG_CRIT	2	/* critical conditions */
> > + #define	APLOG_ERR	3	/* error conditions */
> > + #define	APLOG_WARNING	4	/* warning conditions */
> > + #define	APLOG_NOTICE	5	/* normal but significant condition */
> > + #define	APLOG_INFO	6	/* informational */
> > + #define	APLOG_DEBUG	7	/* debug-level messages */
> > + 
> >   void open_logs (server_rec *, pool *p);
> >   void error_log2stderr (server_rec *);     
> >   
> > ***************
> > *** 59,62 ****
> > --- 69,75 ----
> >   			const char *msg, server_rec *s);
> >   void log_printf(const server_rec *s, const char *fmt, ...);
> >   void log_reason(const char *reason, const char *fname, request_rec *r);
> > + void ap_log(int level, const request_rec *r, const char *routine,
> > + 	    const char *file, const char *fmt, ...);
> > + 
> >   
> > 
> > 
> > 
> 
> 
> -- 
> ====================================================================
>       Jim Jagielski            |       jaguNET Access Services
>      jim@jaguNET.com           |       http://www.jaguNET.com/
>             "Look at me! I'm wearing a cardboard belt!"