From caae8dcfed1462cb19c82f99087e6fe2ba3d407c Mon Sep 17 00:00:00 2001 From: Edward Rudd Date: Sat, 25 Oct 2008 04:25:56 +0000 Subject: implemented logging added better error messages for DB connections fix several segfaults --- utility/config.c | 23 +++++++++--------- utility/config.h | 12 ++++++---- utility/database.c | 24 ++++++++++++------- utility/logparse.c | 61 +++++++++++++++++++++++++++++------------------- utility/mod_log_sql.conf | 4 ++-- utility/shell.c | 52 ++++++++++++++++++++++++----------------- utility/util.c | 31 ++++++++++++++++++++---- utility/util.h | 2 ++ 8 files changed, 132 insertions(+), 77 deletions(-) diff --git a/utility/config.c b/utility/config.c index ccdc6e5..b1e7585 100644 --- a/utility/config.c +++ b/utility/config.c @@ -41,12 +41,10 @@ static apr_status_t config_set_loglevel(config_t *cfg, config_opt_t *opt, return APR_EINVAL; if (!strcasecmp(argv[1], "error")) { cfg->loglevel = LOGLEVEL_ERROR; - } else if (!strcasecmp(argv[1], "warn")) { - cfg->loglevel = LOGLEVEL_WARN; + } else if (!strcasecmp(argv[1], "notice")) { + cfg->loglevel = LOGLEVEL_NOTICE; } else if (!strcasecmp(argv[1], "debug")) { cfg->loglevel = LOGLEVEL_DEBUG; - } else if (!strcasecmp(argv[1], "quiet")) { - cfg->loglevel = LOGLEVEL_QUIET; } else { cfg->loglevel = LOGLEVEL_ERROR; } @@ -293,6 +291,8 @@ void config_init(apr_pool_t *p) config_add_option(p, "DryRun", "Don't perform any actual database changes", config_set_flag, (void *)APR_OFFSETOF(config_t, dryrun)); + config_add_option(p, "Dump", "Dump Configuration and quit", + config_set_flag, (void *)APR_OFFSETOF(config_t, dump)); config_add_option(p, "Config", "Dummy to handle config directive", config_set_dummy, NULL); config_add_option(p, "Summary", "Show the summary before exit?", @@ -306,7 +306,7 @@ config_t *config_create(apr_pool_t *p) apr_pool_create(&sp, p); cfg = apr_pcalloc(sp, sizeof(config_t)); cfg->pool = sp; - cfg->loglevel = LOGLEVEL_WARN; + cfg->loglevel = LOGLEVEL_ERROR; cfg->summary = 1; cfg->transactions = 1; cfg->input_files = apr_array_make(cfg->pool, 10, sizeof(char *)); @@ -320,15 +320,15 @@ apr_status_t config_check(config_t *cfg) { apr_status_t ret = APR_SUCCESS; if (!cfg->dbdriver || !cfg->dbparams) { - printf("Database configuration is missing\n"); + logging_log(cfg, LOGLEVEL_NOISE, "Database configuration is missing\n"); ret = APR_EINVAL; } if (!cfg->table) { - printf("No Log Table defined\n"); + logging_log(cfg, LOGLEVEL_NOISE, "No Log Table defined\n"); ret = APR_EINVAL; } if (apr_is_empty_array(cfg->output_fields)) { - printf("No Output Fields Defined\n"); + logging_log(cfg, LOGLEVEL_NOISE, "No Output Fields Defined\n"); ret = APR_EINVAL; } return ret; @@ -345,7 +345,7 @@ static int config_merge(void *rec, const char *key, const char *value) value }; opt->func(cfg, opt, 2, args); } else { - printf("Unhandled: %s\n", key); + logging_log(cfg, LOGLEVEL_NOISE, "Unhandled: %s\n", key); } return 1; } @@ -400,12 +400,13 @@ apr_status_t config_read(config_t *cfg, const char *filename, if (opt) { rv = opt->func(cfg, opt, targc, (const char **)targv); if (APR_STATUS_IS_EINVAL(rv)) { - printf("Config Error: Invalid Arguments for %s\n\t%s\n", + logging_log(cfg, LOGLEVEL_NOISE, + "Config Error: Invalid Arguments for %s\n\t%s\n", opt->name, opt->help); ret = rv; } } else { - printf("Unhandled: %s\n", targv[0]); + logging_log(cfg, LOGLEVEL_NOISE, "Unhandled: %s\n", targv[0]); } } } while (rv == APR_SUCCESS); diff --git a/utility/config.h b/utility/config.h index 611e9de..763ef5d 100644 --- a/utility/config.h +++ b/utility/config.h @@ -7,10 +7,11 @@ #include "ap_pcre.h" typedef enum { - LOGLEVEL_QUIET = 0, - LOGLEVEL_ERROR = 1, - LOGLEVEL_WARN = 2, - LOGLEVEL_DEBUG = 3, + LOGLEVEL_NOISE = 0, + LOGLEVEL_NONE, + LOGLEVEL_ERROR, + LOGLEVEL_NOTICE, + LOGLEVEL_DEBUG, } loglevel_e; typedef struct config_dbd_t config_dbd_t; @@ -25,6 +26,7 @@ struct config_t { loglevel_e loglevel; /** error_log */ apr_file_t *errorlog_fp; + apr_file_t *errorlog_fperr; apr_pool_t *errorlog_p; /** input directory of log files */ @@ -59,6 +61,8 @@ struct config_t { /** Dry Run */ int dryrun; + /** dump configuration only */ + int dump; /* Show the summary */ int summary; diff --git a/utility/database.c b/utility/database.c index 45022da..c4e4bc9 100644 --- a/utility/database.c +++ b/utility/database.c @@ -4,6 +4,7 @@ #include "apr_strings.h" #include "util.h" +#include "mysql/mysql.h" struct config_dbd_t { const apr_dbd_driver_t *driver; @@ -27,9 +28,10 @@ apr_status_t database_connect(config_t *cfg) } rv = apr_dbd_get_driver(cfg->pool, cfg->dbdriver, &(cfg->dbconn->driver)); if (rv) { + logging_log(cfg, LOGLEVEL_ERROR, - "Could not load database driver %s. Error %d", cfg->dbdriver, - rv); + "Could not load database driver %s. Error %s", cfg->dbdriver, + logging_strerror(rv)); return rv; } @@ -37,7 +39,7 @@ apr_status_t database_connect(config_t *cfg) &(cfg->dbconn->dbd)); if (rv) { logging_log(cfg, LOGLEVEL_ERROR, - "Could not connect to database. Error %d", rv); + "Could not connect to database. Error %s", logging_strerror(rv)); return rv; } @@ -55,7 +57,7 @@ static apr_dbd_prepared_t *database_prepare_insert(config_t *cfg, apr_pool_t *p) char *sql; int i, f; struct iovec *vec; - apr_dbd_prepared_t *stmt; + apr_dbd_prepared_t *stmt = NULL; int nfs = cfg->output_fields->nelts; config_output_field_t *ofields; @@ -87,14 +89,14 @@ static apr_dbd_prepared_t *database_prepare_insert(config_t *cfg, apr_pool_t *p) sql = apr_pstrcatv(p, vec, i+2, NULL); - printf("SQL: %s\n", sql); + logging_log(cfg, LOGLEVEL_DEBUG, "Generated SQL: %s", sql); rv = apr_dbd_prepare(cfg->dbconn->driver, cfg->pool, cfg->dbconn->dbd, sql, "INSERT", &stmt); if (rv) { - printf("DB Error: %s\n", apr_dbd_error(cfg->dbconn->driver, - cfg->dbconn->dbd, rv)); + logging_log(cfg, LOGLEVEL_ERROR, "Unable to Prepare SQL insert: %s", + apr_dbd_error(cfg->dbconn->driver, cfg->dbconn->dbd, rv)); return NULL; } return stmt; @@ -109,6 +111,10 @@ apr_status_t database_insert(config_t *cfg, apr_pool_t *p, apr_table_t *data) // Prepare statement if (!cfg->dbconn->stmt) { cfg->dbconn->stmt = database_prepare_insert(cfg, p); + if (!cfg->dbconn->stmt) { + logging_log(cfg, LOGLEVEL_NOISE, "Unable to prepare SQL statement"); + return APR_EINVAL; + } cfg->dbconn->args = apr_palloc(cfg->pool, nfs * sizeof(char *)); } for (f=0; fdbconn->driver, p, cfg->dbconn->dbd, &f, cfg->dbconn->stmt, nfs, cfg->dbconn->args); if (rv) { - printf("DB Error: %s\n", apr_dbd_error(cfg->dbconn->driver, - cfg->dbconn->dbd, rv)); + logging_log(cfg, LOGLEVEL_ERROR, "Unable to Insert SQL: %s", + apr_dbd_error(cfg->dbconn->driver, cfg->dbconn->dbd, rv)); return rv; } return APR_SUCCESS; diff --git a/utility/logparse.c b/utility/logparse.c index 4b5ab40..f4afb52 100644 --- a/utility/logparse.c +++ b/utility/logparse.c @@ -11,7 +11,6 @@ #include "ap_pcre.h" #include "database.h" - apr_hash_t *g_parser_funcs; static apr_status_t parser_func_regexmatch(apr_pool_t *p, config_t *cfg, @@ -20,23 +19,26 @@ static apr_status_t parser_func_regexmatch(apr_pool_t *p, config_t *cfg, struct { ap_regex_t *rx; const char *substr; - } *data; + }*data; ap_regmatch_t regm[AP_MAX_REG_MATCH]; // Check if a regular expression configured - if (!field->args[0]) return APR_EINVAL; + if (!field->args[0]) + return APR_EINVAL; if (!field->data) { // pre compile the regex data = apr_palloc(cfg->pool, sizeof(ap_regex_t)+sizeof(const char *)); data->rx = ap_pregcomp(cfg->pool, field->args[0], - AP_REG_EXTENDED|AP_REG_ICASE); + AP_REG_EXTENDED|AP_REG_ICASE); if (field->args[1]) { data->substr = field->args[1]; } else { data->substr = "$1"; } - if (!data->rx) return APR_EINVAL; + if (!data->rx) + return APR_EINVAL; field->data = data; - } else data = field->data; + } else + data = field->data; if (!ap_regexec(data->rx, value, AP_MAX_REG_MATCH, regm, 0)) { *ret = ap_pregsub(p, data->substr, value, AP_MAX_REG_MATCH, regm); @@ -66,7 +68,7 @@ static apr_status_t parser_func_machineid(apr_pool_t *p, config_t *cfg, config_output_field_t *field, const char *value, const char **ret) { if (cfg->machineid) { - *ret = apr_pstrdup(p,cfg->machineid); + *ret = apr_pstrdup(p, cfg->machineid); } else { *ret = field->def; } @@ -81,7 +83,7 @@ parser_func_t parser_get_func(const char *name) } static void parser_add_func(apr_pool_t *p, const char *const name, - parser_func_t func) + parser_func_t func) { if (!g_parser_funcs) { g_parser_funcs = apr_hash_make(p); @@ -103,6 +105,7 @@ void parser_find_logs(config_t *cfg) apr_finfo_t finfo; char **newp; + logging_log(cfg, LOGLEVEL_NOTICE, "Find Log files"); if (!cfg->input_dir) return; apr_pool_create(&tp, cfg->pool); @@ -113,7 +116,7 @@ void parser_find_logs(config_t *cfg) continue; newp = (char **)apr_array_push(cfg->input_files); apr_filepath_merge(newp, cfg->input_dir, finfo.name, - APR_FILEPATH_TRUENAME, cfg->pool); + APR_FILEPATH_TRUENAME, cfg->pool); } apr_dir_close(dir); } @@ -261,11 +264,12 @@ apr_status_t parse_logfile(config_t *cfg, const char *filename) apr_pool_create(&tp, cfg->pool); apr_pool_create(&targp, tp); + logging_log(cfg, LOGLEVEL_NOTICE, "Begin Parsing Log File '%s'", filename); + rv = apr_file_open(&file, filename, APR_FOPEN_READ | APR_BUFFERED, - APR_OS_DEFAULT, tp); + APR_OS_DEFAULT, tp); if (rv != APR_SUCCESS) { - logging_log(cfg, LOGLEVEL_ERROR, "Could not open %s",filename); - printf("Could not open %s\n", filename); + logging_log(cfg, LOGLEVEL_NOISE, "Could not open %s", filename); return rv; } @@ -280,44 +284,52 @@ apr_status_t parse_logfile(config_t *cfg, const char *filename) apr_pool_clear(targp); tokenize_logline(buff, &targv, targp, 0); targc = 0; - while (targv[targc]) targc++; + while (targv[targc]) + targc++; /** @todo Run Line Filters here */ rv = parse_processline(targp, cfg, targv, targc); if (rv != APR_SUCCESS) { int i; - printf("Line %d(%d): %s\n",line, targc, buff); + logging_log(cfg, LOGLEVEL_ERROR, "Line %d(%d): %s", line, + targc, buff); for (i = 0; targv[i]; i++) { - printf("Arg (%d): '%s'\n", i, targv[i]); + logging_log(cfg, LOGLEVEL_ERROR, "Arg (%d): '%s'", i, + targv[i]); } } } } while (rv == APR_SUCCESS); - printf("Total Lines: %d\n", line); apr_file_close(file); apr_pool_destroy(tp); + logging_log(cfg, LOGLEVEL_NOTICE, + "Finish Parsing Log File '%s'. Lines: %d", filename, line); + return APR_SUCCESS; } -apr_status_t parse_processline(apr_pool_t *ptemp, config_t *cfg, char **argv, int argc) +apr_status_t parse_processline(apr_pool_t *ptemp, config_t *cfg, char **argv, + int argc) { config_logformat_t *fmt; config_logformat_field_t *ifields; config_output_field_t *ofields; apr_table_t *datain; apr_table_t *dataout; - apr_status_t rv = APR_SUCCESS; + apr_status_t rv= APR_SUCCESS; int i; fmt = apr_hash_get(cfg->log_formats, cfg->logformat, APR_HASH_KEY_STRING); - if (!fmt) return APR_EINVAL; - if (fmt->fields->nelts != argc) return APR_EINVAL; + if (!fmt) + return APR_EINVAL; + if (fmt->fields->nelts != argc) + return APR_EINVAL; datain = apr_table_make(ptemp, fmt->fields->nelts); dataout = apr_table_make(ptemp, cfg->output_fields->nelts); ifields = (config_logformat_field_t *)fmt->fields->elts; for (i=0; ifields->nelts; i++) { - apr_table_setn(datain,ifields[i].name,argv[i]); + apr_table_setn(datain, ifields[i].name, argv[i]); } /** @todo Run Pre Filters here */ @@ -332,12 +344,13 @@ apr_status_t parse_processline(apr_pool_t *ptemp, config_t *cfg, char **argv, in continue; } if (!ofields[i].func) { - apr_table_setn(dataout,ofields[i].field, val); + apr_table_setn(dataout, ofields[i].field, val); } else { - const char *ret = NULL; + const char *ret= NULL; rv = ((parser_func_t)ofields[i].func)(ptemp, cfg, &ofields[i], val, &ret); - if (rv) return rv; + if (rv) + return rv; apr_table_setn(dataout, ofields[i].field, ret); } } diff --git a/utility/mod_log_sql.conf b/utility/mod_log_sql.conf index bd8496c..771f7c7 100644 --- a/utility/mod_log_sql.conf +++ b/utility/mod_log_sql.conf @@ -5,8 +5,8 @@ DBDParams "host=localhost;user=root;dbname=apache_log" Table access_log MachineID 7of9 UseTransactions on -LogLevel notice -DryRun on +LogLevel debug +DryRun off Summary on LogFormatConfig CLF remhost String diff --git a/utility/shell.c b/utility/shell.c index b289bce..e521916 100644 --- a/utility/shell.c +++ b/utility/shell.c @@ -17,9 +17,10 @@ const apr_getopt_option_t _opt_config[] = { {"transaction", 't', 1, "Use a Transaction (yes,no)"}, {"logformat", 'r', 1, "Use this logformat to parse files"}, {"file", 'f', 1, "Parse this single log file (input dir is NOT scanned)"}, - {"inputdir", 'd', 1, "Input Directory to look for log files"}, + {"inputdir", 'i', 1, "Input Directory to look for log files"}, {"config", 'c', 1, "Configuration file to use (default mod_log_sql.conf)"}, {"dryrun", 'n', 0, "Perform a dry run (do not actually alter the databse)"}, + {"dump", 'd', 0, "Dump the configuration after parsing and quit"}, {"loglevel", 'l', 1, "Log Level (deubg, warn, error)"}, {"summary", 's', 1, "Summary (yes,no)"}, {"help", 'h', 0, "Show Help"}, @@ -62,7 +63,7 @@ int main(int argc, const char *const argv[]) const char *opt_arg; apr_status_t rv; apr_table_t *args; - config_t *base; + config_t *cfg; apr_app_initialize(&argc, &argv, NULL); atexit(apr_terminate); @@ -84,7 +85,7 @@ int main(int argc, const char *const argv[]) apr_table_setn(args,"config",opt_arg); break; case 'd': - apr_table_setn(args,"inputdirectory",opt_arg); + apr_table_setn(args,"dump","yes"); break; case 'f': apr_table_setn(args,"inputfile",opt_arg); @@ -93,6 +94,9 @@ int main(int argc, const char *const argv[]) show_help(argv[0], _opt_config, stdout); exit(1); break; + case 'i': + apr_table_setn(args,"inputdirectory",opt_arg); + break; case 'l': apr_table_setn(args,"loglevel",opt_arg); break; @@ -128,45 +132,49 @@ int main(int argc, const char *const argv[]) parser_init(pool); config_init(pool); database_init(pool); - logging_init(pool); // Process configuration file - base = config_create(pool); - rv = config_read(base, apr_table_get(args,"Config"), args); + cfg = config_create(pool); + rv = config_read(cfg, apr_table_get(args,"Config"), args); apr_pool_destroy(ptemp); + // Initialize Log system AFTER we parse the configuration + logging_init(cfg); + if (APR_STATUS_IS_ENOENT(rv)) { - fprintf(stderr,"Could not load configuration file: %s\n",apr_table_get(args,"config")); + logging_log(cfg,LOGLEVEL_NOISE,"Could not load configuration file: %s",apr_table_get(args,"config")); } else if (rv) { exit(1); } - config_dump(base); + if (cfg->dump) { + config_dump(cfg); + exit(0); + } - if (config_check(base)) { - printf("Please correct the configuration\n"); + if (config_check(cfg)) { + logging_log(cfg,LOGLEVEL_NOISE, "Please correct the configuration"); exit(1); } // Find files and parse - parser_find_logs(base); - if (!base->dryrun) { - if ((rv = database_connect(base))) { - printf("Error Connecting to Database: %d\n",rv); + parser_find_logs(cfg); + if (!cfg->dryrun) { + if ((rv = database_connect(cfg))) { + logging_log(cfg,LOGLEVEL_NOISE, "Error Connecting to Database"); exit(1); } } - if (!apr_is_empty_array(base->input_files)) { + if (!apr_is_empty_array(cfg->input_files)) { char **filelist; int f, l; - filelist = (char **)base->input_files->elts; - for (f=0, l=base->input_files->nelts; f < l; f++) { - printf("Scanning %s\n",filelist[f]); - parse_logfile(base, filelist[f]); + filelist = (char **)cfg->input_files->elts; + for (f=0, l=cfg->input_files->nelts; f < l; f++) { + parse_logfile(cfg, filelist[f]); } } else { - printf("No input files\n"); + logging_log(cfg,LOGLEVEL_NOISE,"No log files found to parse"); } - if (!base->dryrun) { - database_disconnect(base); + if (!cfg->dryrun) { + database_disconnect(cfg); } /** @todo summary goes here */ return 0; diff --git a/utility/util.c b/utility/util.c index cb6898d..99bb046 100644 --- a/utility/util.c +++ b/utility/util.c @@ -33,15 +33,31 @@ void line_chomp(char *str) void logging_init(config_t *cfg) { + apr_status_t rv; + apr_pool_create(&cfg->errorlog_p, cfg->pool); + apr_file_open_stderr(&cfg->errorlog_fperr, cfg->pool); if (cfg->errorlog) { - apr_file_open(&cfg->errorlog_fp, cfg->errorlog, - APR_FOPEN_CREATE | APR_FOPEN_WRITE | APR_BUFFERED, + rv = apr_file_open(&cfg->errorlog_fp, cfg->errorlog, + APR_FOPEN_CREATE | APR_FOPEN_WRITE | APR_FOPEN_APPEND, APR_OS_DEFAULT, cfg->pool); - apr_pool_create(&cfg->errorlog_p, cfg->pool); + if (rv) { + printf("Error opening %s\n",cfg->errorlog); + cfg->loglevel = LOGLEVEL_NONE; + } + logging_log(cfg, LOGLEVEL_ERROR, "Log file Opened"); + } else { + cfg->loglevel = LOGLEVEL_NONE; + logging_log(cfg, LOGLEVEL_NOISE, "No Log file specified, disabled logging"); } } +const char *logging_strerror(apr_status_t rv) +{ + char buff[256]; + return apr_strerror(rv, buff, 256); +} + /** * @todo implement logging */ @@ -51,7 +67,7 @@ void logging_log(config_t *cfg, loglevel_e level, const char *fmt, ...) struct iovec vec[2]; apr_size_t blen; - if (!cfg->errorlog_fp || cfg->loglevel < level) return; + if (cfg->loglevel < level) return; va_start(ap, fmt); apr_pool_clear(cfg->errorlog_p); @@ -61,7 +77,12 @@ void logging_log(config_t *cfg, loglevel_e level, const char *fmt, ...) vec[1].iov_base = "\n"; vec[1].iov_len = 1; - apr_file_writev(cfg->errorlog_fp,vec,2,&blen); + if (level == LOGLEVEL_NOISE) { + apr_file_writev(cfg->errorlog_fperr,vec,2,&blen); + } + if (cfg->loglevel > LOGLEVEL_NONE) { + apr_file_writev(cfg->errorlog_fp,vec,2,&blen); + } va_end(ap); } diff --git a/utility/util.h b/utility/util.h index 9e3aed3..c67cf9c 100644 --- a/utility/util.h +++ b/utility/util.h @@ -17,4 +17,6 @@ void logging_init(config_t *cfg); void logging_log(config_t *cfg, loglevel_e level, const char *fmt, ...) __attribute__((format(printf, 3, 4))); +const char *logging_strerror(apr_status_t rv); + #endif /*UTIL_H_*/ -- cgit