Further changes on the way to 1.17, big one was the segfault fix.

2 files changed, 160 insertions, 153 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 24e56a1..8f53074 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,4 +1,4 @@
-$Id: CHANGELOG,v 1.11 2002/05/14 21:47:14 helios Exp $
+$Id: CHANGELOG,v 1.12 2002/05/16 21:35:12 helios Exp $
 
 
 TODO:
@@ -8,7 +8,8 @@ TODO:
 * check for mandatory conf directives / syntax quit if not
 * merge server config into vh config
 * port to Apache 2.x
-
+* does determining table name in massvirtual mode upon every request
+  cause performance degradation?  If so fix.
 
 CHANGES:
 
@@ -32,6 +33,10 @@ CHANGES:
   really dumb, and people have requested this change.
 * Better checking in the extract_cookie routine.  Before, it could
   segfault if a person specified "c" but didn't define MySQLWhichCookie.
+* Some code reorg/renaming for clarity and to support the new direction
+  of database inspecificity.  More to come.
+* Simplified error messages.
+* Table creation now uses safe_mysql_query and checks the result.
 
 
 1.16:
diff --git a/mod_log_sql.c b/mod_log_sql.c
index 84c6115..6726163 100644
--- a/mod_log_sql.c
+++ b/mod_log_sql.c
@@ -1,4 +1,4 @@
-/* $Id: mod_log_sql.c,v 1.12 2002/05/14 21:47:15 helios Exp $ */
+/* $Id: mod_log_sql.c,v 1.13 2002/05/16 21:35:12 helios Exp $ */
 
 /* --------*
  * DEFINES *
@@ -76,7 +76,7 @@ typedef struct {
         char *transfer_log_format;
         char *preserve_file;
         char *cookie_name;
-} log_mysql_state;
+} log_sql_state;
 
 
 /* -----------------*
@@ -322,7 +322,7 @@ static const char *extract_cookie(request_rec *r, char *a)
         char *isvalid;
         char *cookiebuf;
     
-        log_mysql_state *cls = get_module_config(r->server->module_config, &mysql_log_module);
+        log_sql_state *cls = get_module_config(r->server->module_config, &mysql_log_module);
         
         if (cls->cookie_name != NULL) {
                 #ifdef DEBUG
@@ -423,13 +423,13 @@ static const char *extract_unique_id(request_rec *r, char *a)
 
 
 
-struct log_mysql_item_list {
+struct log_sql_item_list {
           char ch;                                              /* its letter code */
           item_key_func func;                   /* its extraction function */
           const char *sql_field_name;   /* its column in SQL */
           int want_orig_default;                /* if it requires the original request prior to internal redirection */
           int string_contents;                  /* if it returns a string */
-    } log_mysql_item_keys[] = {
+    } log_sql_item_keys[] = {
 
         {   'A', extract_agent,             "agent",            1, 1    },
     {   'b', extract_bytes_sent,        "bytes_sent",       0, 0    },
@@ -553,11 +553,11 @@ const char *extract_table(void *data, const char *key, const char *val)
 void preserve_entry(request_rec *r, const char *query)
 {
         FILE *fp;
-        log_mysql_state *cls = get_module_config(r->server->module_config, &mysql_log_module);
+        log_sql_state *cls = get_module_config(r->server->module_config, &mysql_log_module);
         
         fp = pfopen(r->pool, cls->preserve_file, "a");
         if (fp == NULL)
-                ap_log_error(APLOG_MARK,ERRLEVEL,r->server,"attempted append of local offline file but failed.");
+                ap_log_error(APLOG_MARK,ERRLEVEL,r->server,"mod_log_sql: attempted append of local preserve file but failed.");
         else {
                 fprintf(fp,"%s;\n", query);
                 pfclose(r->pool, fp);
@@ -565,7 +565,7 @@ void preserve_entry(request_rec *r, const char *query)
 }
 
 /*-----------------------------------------------------*/
-/* safe_mysql_query: perform a database insert with    */
+/* safe_mysql_query: perform a database query with     */
 /* a degree of safety and error checking.              */
 /*                                                     */
 /* Parms:   request record, SQL insert statement       */
@@ -577,7 +577,6 @@ int safe_mysql_query(request_rec *r, const char *query)
         int retval;
         struct timespec delay, remainder;
         int ret;
-        char *str;
         void (*handler) (int);
         
 
@@ -585,53 +584,54 @@ int safe_mysql_query(request_rec *r, const char *query)
         handler = signal(SIGPIPE, SIG_IGN);      
 
         /* First attempt for the query */
-        retval = mysql_query(mysql_log, query);
-
+        if (mysql_log != NULL) 
+                retval = mysql_query(mysql_log, query);
+        else
+                return 1;
+        
         /* If we ran the query and it returned an error, try to be graceful.
-         * We only reach this point if the module thinks it has a valid connection to the
-         * database (i.e. mysql_log is non-null).  But that connection could go sour
-         * at any time, hence the check. */
+         * (The module thought it had a valid mysql_log connection but the query
+         * might have failed, so we have to check.)
+         */
         if ( retval != 0 ) 
     {
-                        log_mysql_state *cls = get_module_config(r->server->module_config, &mysql_log_module);
+                log_sql_state *cls = get_module_config(r->server->module_config, &mysql_log_module);
                 
-                        /* Something went wrong, so start by trying to restart the db link. */
-                    ap_log_error(APLOG_MARK,ERRLEVEL,r->server,"attempting reconnect because API said: %s", mysql_error(mysql_log));
-
-                        mysql_log = NULL;
-                        open_logdb_link();
-
-                if (mysql_log == NULL) {         /* still unable to link */
-                        signal(SIGPIPE, handler);
-                        ap_log_error(APLOG_MARK,ERRLEVEL,r->server,"httpd child reconnect failed, unable to reach database. SQL logging stopped until an httpd child regains a db connection.");
-                                ap_log_error(APLOG_MARK,ERRLEVEL,r->server,"log entries are being preserved in %s", cls->preserve_file);
-                                preserve_entry(r, query);
-                        return retval;
-                } else {
-                        ap_log_error(APLOG_MARK,ERRLEVEL,r->server,"reconnect successful.");
-                        }
-
-                    /* Attempt a single re-try... First sleep for a tiny amount of time. */
-                delay.tv_sec = 0;
-                delay.tv_nsec = 250000000;  /* max is 999999999 (nine nines) */
-                ret = nanosleep(&delay, &remainder);
-                if (ret && errno != EINTR)
-                                ap_log_error(APLOG_MARK,ERRLEVEL,r->server,"nanosleep unsuccessful.");
-
-                /* Now make our second attempt */
-                    retval = mysql_query(mysql_log,query);
-
-                        /* If this one also failed, log that and append to our local offline file */
-                    if ( retval != 0 )
-                    {
-                        str = ap_pstrcat(r->pool, "delayed insert attempt failed, API said: ", MYSQL_ERROR(mysql_log), NULL);
-                        ap_log_error(APLOG_MARK,ERRLEVEL,r->server,str);
+                /* Something went wrong, so start by trying to restart the db link. */
+                ap_log_error(APLOG_MARK,ERRLEVEL,r->server,"mod_log_sql: first attempt failed, API said: %s", MYSQL_ERROR(mysql_log));
+
+                mysql_log = NULL;
+                open_logdb_link();
+
+        if (mysql_log == NULL) {         /* still unable to link */
+                signal(SIGPIPE, handler);
+                ap_log_error(APLOG_MARK,ERRLEVEL,r->server,"mod_log_sql: reconnect failed, unable to reach database. SQL logging stopped until child regains a db connection.");
+                        ap_log_error(APLOG_MARK,ERRLEVEL,r->server,"mod_log_sql: log entries are being preserved in %s", cls->preserve_file);
+                        preserve_entry(r, query);
+                return retval;
+        } else {
+                ap_log_error(APLOG_MARK,ERRLEVEL,r->server,"mod_log_sql: reconnect successful");
+                }
 
-                                preserve_entry(r, query);
-                                ap_log_error(APLOG_MARK,ERRLEVEL,r->server,"entry preserved in %s", cls->preserve_file);
-                        } else {
-                    ap_log_error(APLOG_MARK,ERRLEVEL,r->server,"insert successful after a delayed retry.");
-                }
+                /* Attempt a single re-try... First sleep for a tiny amount of time. */
+            delay.tv_sec = 0;
+            delay.tv_nsec = 250000000;  /* max is 999999999 (nine nines) */
+            ret = nanosleep(&delay, &remainder);
+            if (ret && errno != EINTR)
+                        ap_log_error(APLOG_MARK,ERRLEVEL,r->server,"mod_log_sql: nanosleep unsuccessful");
+
+            /* Now make our second attempt */
+                retval = mysql_query(mysql_log,query);
+
+                /* If this one also failed, log that and append to our local offline file */
+                if ( retval != 0 )
+                {
+                ap_log_error(APLOG_MARK,ERRLEVEL,r->server,"mod_log_sql: second attempt failed, API said: %s", MYSQL_ERROR(mysql_log));
+                        preserve_entry(r, query);
+                        ap_log_error(APLOG_MARK,ERRLEVEL,r->server,"mod_log_sql: entry preserved in %s", cls->preserve_file);
+                } else {
+                ap_log_error(APLOG_MARK,ERRLEVEL,r->server,"mod_log_sql: second attempt successful");
+            }
         }
 
         /* Restore SIGPIPE to its original handler function */
@@ -646,43 +646,43 @@ int safe_mysql_query(request_rec *r, const char *query)
  * to the directives found at Apache runtime.      *
  * ------------------------------------------------*/
 
-const char *set_mysql_massvirtual(cmd_parms *parms, void *dummy, int flag)
+const char *set_log_sql_massvirtual(cmd_parms *parms, void *dummy, int flag)
 {
         massvirtual = ( flag ? 1 : 0);
         return NULL;
 }
 
-const char *set_log_mysql_create(cmd_parms *parms, void *dummy, int flag)
+const char *set_log_sql_create(cmd_parms *parms, void *dummy, int flag)
 {
         create_tables = ( flag ? 1 : 0);
         return NULL;
 }
 
-const char *set_log_mysql_db(cmd_parms *parms, void *dummy, char *arg)
+const char *set_log_sql_db(cmd_parms *parms, void *dummy, char *arg)
 {
         db_name = arg;
         return NULL;
 }
 
-const char *set_log_mysql_cookie(cmd_parms *parms, void *dummy, char *arg)
+const char *set_log_sql_cookie(cmd_parms *parms, void *dummy, char *arg)
 {
-        log_mysql_state *cls = get_module_config(parms->server->module_config, &mysql_log_module);
+        log_sql_state *cls = get_module_config(parms->server->module_config, &mysql_log_module);
 
         cls->cookie_name = arg;
         return NULL;
 }
 
-const char *set_log_mysql_preserve_file(cmd_parms *parms, void *dummy, char *arg)
+const char *set_log_sql_preserve_file(cmd_parms *parms, void *dummy, char *arg)
 {
         /* char *pfile; */
-        log_mysql_state *cls = get_module_config(parms->server->module_config, &mysql_log_module);
+        log_sql_state *cls = get_module_config(parms->server->module_config, &mysql_log_module);
 
         /* pfile = ap_pstrcat(parms->pool, "/tmp/", arg, NULL); */
         cls->preserve_file = arg;
         return NULL;
 }
 
-const char *set_log_mysql_info(cmd_parms *parms, void *dummy, char *host, char *user, char *pwd)
+const char *set_log_sql_info(cmd_parms *parms, void *dummy, char *host, char *user, char *pwd)
 {
         if (*host != '.') {
                 db_host = host;
@@ -696,37 +696,37 @@ const char *set_log_mysql_info(cmd_parms *parms, void *dummy, char *host, char *
         return NULL;
 }
 
-    then you can skip this step.  Otherwise you need to do it by hand:
 
-    Create a database and table to hold the new log data.  I log the
-    same data as the regular "combined log" plus a little extra information
-    that can be useful.
+        You may be especially security-paranoid and not want "loguser" to have
+        "create" capability within the "apachelogs" databse.  You can disable that
+        but the cost is that you cannot use the module's automatic-table-creation
+        feature.  If that's an acceptable cost, hand-create the tables as described
+        in step 1 and use the following GRANT statement instead of the one above:
 
-    The order that the fields appear in the table is irrelevant
-    because you can SELECT them in any order you choose.  To create
-    this table I first created a new database called "apache":
+    mysql> grant insert on apachelogs.* to loguser@my.apachemachine.com identified by 'l0gger';
 
-    # mysql -uadmin -pmypassword
-    mysql> create database apache;
+3) Enable full logging of your MySQL daemon (at least temporarily
+    for debugging purposes) if you don't do this already:
 
-    Then create the table called "access_log".  I enclosed an SQL file
-    that will create every column type that mod_log_sql supports. 
-    Unless you're just testing or playing around, this is probably NOT
-    what you want, so edit the file first and delete the lines that
-    don't pertain to you.  Then:
+    Edit /etc/my.cnf and add the following line to your [mysqld] section:
 
-    mysql> source access_log.sql
+      log=/var/log/mysql-messages
 
+    Then restart MySQL.
 
-11) Create a specific MySQL userid that httpd will use to authenticate
-    and enter data.  This userid need not be an actual Unix user.  It
-    is a userid internal to MySQL with specific privileges.
-        
-        mysql> grant insert,create on apache.* to loguser@my.apachemachine.com identified by 'l0gger';
-        
-        Security is a very real concern.  mod_log_sql by default is 
-        set up to create the SQL tables it needs.  If you have deactivated
-        this capability, then create a user called "loguser" with the password
-        "l0gger" with only the capability of INSERT to "access_log":
+4) Tell the module what database to use and the appropriate authentication
+   information.
 
-    mysql> grant insert on apache.access_log to loguser@my.apachemachine.com identified by 'l0gger';
+     OUR EXAMPLE: use the MySQL database called "apachelogs" running
+     on "dbmachine.foo.com".  The module uses username "loguser" and
+     password "l0gger" to authenticate to the database. The log entries
+     will be INSERTed into the table called "access_log".
 
+   So, edit httpd.conf and insert the following lines somewhere AFTER any
+   LoadModule / AddModule statements.  Make sure these statements are
+   "global," i.e. not inside any VirtualHost stanza.
 
-12) Enable full logging of your MySQL daemon (at least temporarily
-    for debugging purposes) if you don't do this already:
+   LogSQLDatabase apachelogs
+   LogSQLLoginInfo dbmachine.foo.com loguser l0gger
+   LogSQLCreateTables on
 
-    Edit /etc/my.cnf and add the following line to your [mysqld] section:
+   If your database resides on localhost instead of another host, specify
+   the MySQL server's socket file as follows:
 
-      log=/var/log/mysql-messages
+   LogSQLSocketFile /your/path/to/mysql.sock
 
-    Then restart MySQL.
+5) The actual logging is set up on a virtual-host-by-host basis.  So,
+   skip down to the virtual host you want to set up.  The LogSQLTransferLogTable
+   directive is the minimum required to log -- other directives simply
+   tune the module's behavior.
 
-13) Restart apache.
+   <VirtualHost 1.2.3.4>
+        [snip]
+
+        LogSQLTransferLogTable access_log
+
+        [snip]
+   </VirtualHost>
+
+6)  Restart apache.
 
     # /etc/rc.d/init.d/httpd start
 
-13) Load your web site in a browser to trigger some hits, then confirm that 
+7)  Load your web site in a browser to trigger some hits, then confirm that
     the entries are being successfully logged:
 
-    # mysql -hmysql.host.com -umysqladmin -p -e "select * from access_log" apache;
+    # mysql -hmysql.host.com -umysqladmin -p -e "select * from access_log" apachelogs
     Enter password:
 
     +---------------------------------------------------+-------------+-------------+------------------+------------------+------------+--------+------------+------------------------------------+
@@ -251,10 +313,29 @@ Installation (as a static module compiled into httpd)
         .
     +---------------------------------------------------+-------------+-------------+------------------+------------------+------------+--------+------------+------------------------------------+
 
-14) You have basic functionality.  Don't disable your regular Apache logs until
+    You have basic functionality.  Don't disable your regular Apache logs until
     you feel comfortable that the database is behaving as you'd like and that
-    things are going well.  
-
-
-
-
+    things are going well.
+
+8) If you do not see any entries in the access_log, then something is preventing
+   the inserts from happening.  This problem could be caused by several things:
+   - Improper privileges set up in the MySQL database
+   - You aren't hitting a VirtualHost that has a LogSQLTransferLogTable entry
+   - You didn't specify the right host
+
+   If you have confirmed your LogSQL* directives and know them to be correct,
+   you should examine the httpd server logs for mod_log_sql messages; the module
+   will offer hints as to why it cannot connect, etc.  Also examine the MySQL
+   log that you established in step 3.  Ensure that the INSERTs are not being
+   rejected because of a malformed table entry or other clerical error.  If you
+   see no INSERT attempts in the log, the module isn't successfully connecting
+   to the database.
+
+   The next thing to do is recompile the module with debugging output activated.
+   change the "#undef DEBUG" on line 8 of mod_log_sql.c to "#define DEBUG" and
+   recompile/reinstall.  The module will now output copious notes about what
+   it is doing, and this will help you (and the maintainer) solve the problem.
+
+9) You can now activate the advanced features of mod_log_sql.  These are all
+   described in the online directive documentation:
+   http://www.grubbybaby.com/mod_log_sql/directives.html