On Sun, 2007-03-11 at 03:04 +0000, Bryan Henderson wrote:
You can find the results in tomorrow's snapshot, however I'd appreciate if you could check my judgement and see if I did something wrong.
In main.c, there are four messages generated at NOTICE level that I think are really INFO:
syslog-ng starting up SIGHUP received, reloading configuration syslog-ng shutting down
I would disagree here, startup/shutdown/config reload messages are quite significant.
Termination requested via signal, terminating
That's ok, the shutting down message is generated, the reason why it got terminated is less important.
The reason these aren't worthy of notice is that the system administrator already knows about these events; he ordered them. Something worthy of notice is something that happens independently.
While almost all the error reports are issued at ERROR level, I found these that seem to be the same kind of errors, but have the lower NOTICE level:
cfg_lex.c: unknown parse flag
fixed.
cfg_grammar.c: The value specified for time_sleep is too large.
time_sleep is maximized to 500msec in this case, but syslog-ng goes on. It is worth fixing up the configuration, but does not qualify as an error.
And here's one that's CRITICAL:
macros.c: Internal error, unknown macro referenced
this is an internal error, and might just be converted to a failed assertion. It should really never happen. I've converted it to g_assert_not_reached().
Isn't this just another error? If a result of this is that Syslog fails completely, it would be worth an ALERT, but then the message should mention that Syslog failed completely. CRITICAL is just for when an entire major system is in imminent danger of collapse and needs immediate attention. I think a system could continue useful work for quite a while without Syslog service.
I've completely removed the message, it is a situation that should never be triggered. It is worth a crash if it is. (there are other cases with asserts like this in the code).
In afsocket.c, "Number of allowed concurrent connections exceeded" is an ERROR. If this is what I think it is, it's not a case of Syslog being broken, but nonetheless something that might need attention, so I would call it WARNING or NOTICE.
I think that's worth an error as it means that an application, trying to send a log message failed to do so. It means that the system is losing messages.
And in affile.c, "Destination file is too old, removing" is to me business as usual, not something someone is likely to want to respond to, so I would make it INFO.
Sure, this is not very important, syslog-ng removes the file that was requested using overwrite_if_older(). I made this INFO and clarified the text. Here's the patch I've just commited: --- orig/src/affile.c +++ mod/src/affile.c @@ -242,8 +242,9 @@ affile_dw_init(LogPipe *s, GlobalConfig stat(self->filename->str, &st) == 0 && st.st_mtime < time(NULL) - self->owner->overwrite_if_older) { - msg_notice("Destination file is too old, removing", + msg_info("Destination file is older than overwrite_if_older(), overwriting", evt_tag_str("filename", self->filename->str), + evt_tag_int("overwrite_if_older", self->owner->overwrite_if_older), NULL); unlink(self->filename->str); } --- orig/src/cfg-lex.l +++ mod/src/cfg-lex.l @@ -284,7 +284,7 @@ lookup_parse_flag(char *flag) return LRO_NOPARSE; if (strcmp(flag, "kernel") == 0) return LRO_KERNEL; - msg_notice("Unknown parse flag", evt_tag_str("flag", flag), NULL); + msg_error("Unknown parse flag", evt_tag_str("flag", flag), NULL); return 0; } --- orig/src/macros.c +++ mod/src/macros.c @@ -441,8 +441,8 @@ log_macro_expand(GString *result, gint i break; } default: - msg_fatal("Internal error, unknown macro referenced;", NULL); - return FALSE; + g_assert_not_reached(); + break; } return TRUE; } --- orig/src/messages.h +++ mod/src/messages.h @@ -33,6 +33,7 @@ extern int verbose_flag; #define msg_fatal(desc, tag1, tags...) msg_event_send(msg_event_create(EVT_PRI_CRIT, desc, tag1, ##tags )) #define msg_error(desc, tag1, tags...) msg_event_send(msg_event_create(EVT_PRI_ERR, desc, tag1, ##tags )) #define msg_notice(desc, tag1, tags...) msg_event_send(msg_event_create(EVT_PRI_NOTICE, desc, tag1, ##tags )) +#define msg_info(desc, tag1, tags...) msg_event_send(msg_event_create(EVT_PRI_INFO, desc, tag1, ##tags )) #define msg_verbose(desc, tag1, tags...) \ do { \ -- Bazsi