On 12/02/2014 02:08 PM, Jeff Trawick wrote:
On Wed, Sep 17, 2014 at 9:22 AM, Jeff Trawick <traw...@gmail.com
<mailto:traw...@gmail.com>> wrote:

    On Mon, Sep 15, 2014 at 2:00 AM, Jan Kaluža <jkal...@redhat.com
    <mailto:jkal...@redhat.com>> wrote:

        On 09/14/2014 01:21 PM, Martynas Bendorius wrote:

            Hello,

            Is there any special reason why mod_systemd and mod_journald
            (available
            in trunk) are not backported to 2.4 yet?


        Hi,

        I think mod_systemd could be proposed for 2.4 branch (maybe even
        with the changes adding socket activation), but for
        mod_journald, we would have to backport "modular logging", which
        breaks the API/ABI and therefore I'm afraid that won't happen in
        2.4 branch :(.


    I have an old patch somewhere that doesn't break the API/ABI, and
    accommodates such differences as syslog being built-in in 2.4.x.  I
    didn't realize that anybody besides me actually cared.

    I'll try to find time to see how it fits in 2.4.x-HEAD.


I've simply attached it from its state one year ago, not having time to
play with it.  I don't think it is necessary to break the ABI.  syslog
support remains part of core logging instead of in a module.

I've created my version of the patch based on yours. It includes also more recent commits from trunk related to errorlog providers. Can you try it? I presume you have done that backport, because you are running that somewhere :).

Regards,
Jan Kaluza

        Jan Kaluza


            As we have a lot of distributions already using systemd by
            default
            (CentOS/RHEL 7, Fedora, Arch Linux, CoreOS, openSUSE), and
            more of them
            are going to use systemd by default (Debian 8 (Jessie),
            Ubuntu), it
            requires manual patching of apache for the support of
            systemd/journald.

            Thank you!





--
Born in Roswell... married an alien...
http://emptyhammock.com/


Index: docs/manual/mod/core.xml
===================================================================
--- docs/manual/mod/core.xml	(revision 1643223)
+++ docs/manual/mod/core.xml	(working copy)
@@ -1315,6 +1315,9 @@
 
     <highlight language="config">ErrorLog syslog:user</highlight>
 
+    <p>Additional modules can provide their own ErrorLog providers. The syntax
+    is similar to <code>syslog</code> example above.</p>
+
     <p>SECURITY: See the <a
     href="../misc/security_tips.html#serverroot">security tips</a>
     document for details on why your security could be compromised
Index: include/http_core.h
===================================================================
--- include/http_core.h	(revision 1643223)
+++ include/http_core.h	(working copy)
@@ -835,8 +835,44 @@
 
     /** message format */
     const char *format;
+    /** 1 if logging using provider, 0 otherwise */
+    int using_provider;
 } ap_errorlog_info;
 
+#define AP_ERRORLOG_PROVIDER_GROUP "error_log_writer"
+#define AP_ERRORLOG_PROVIDER_VERSION "0"
+#define AP_ERRORLOG_DEFAULT_PROVIDER "file"
+
+/** add APR_EOL_STR to the end of log message */
+#define AP_ERRORLOG_PROVIDER_ADD_EOL_STR       1
+
+typedef struct ap_errorlog_provider ap_errorlog_provider;
+
+struct ap_errorlog_provider {
+    /** Initializes the error log writer.
+     * @param p The pool to create any storage from
+     * @param s Server for which the logger is initialized
+     * @return Pointer to handle passed later to writer() function
+     * @note On success, the provider must return non-NULL, even if
+     * the handle is not necessary when the writer() function is
+     * called.  On failure, the provider should log a startup error
+     * message and return NULL to abort httpd startup.
+     */
+    void * (*init)(apr_pool_t *p, server_rec *s);
+
+    /** Logs the error message to external error log.
+     * @param info Context of the error message
+     * @param handle Handle created by init() function
+     * @param errstr Error message
+     * @param len Length of the error message
+     */
+    apr_status_t (*writer)(const ap_errorlog_info *info, void *handle,
+                           const char *errstr, apr_size_t len);
+
+    /** a combination of the AP_ERRORLOG_PROVIDER_* flags */
+    unsigned int flags;
+};
+
 /**
  * callback function prototype for a external errorlog handler
  * @note To avoid unbounded memory usage, these functions must not allocate
Index: include/httpd.h
===================================================================
--- include/httpd.h	(revision 1643223)
+++ include/httpd.h	(working copy)
@@ -1248,6 +1248,10 @@
     apr_file_t *error_log;
     /** The log level configuration */
     struct ap_logconf log;
+    /** External error log writer provider */
+    struct ap_errorlog_provider *errorlog_provider;
+    /** Handle to be passed to external log provider's logging method */
+    void *errorlog_provider_handle;
 
     /* Module-specific configuration for server, and defaults... */
 
Index: server/core.c
===================================================================
--- server/core.c	(revision 1643223)
+++ server/core.c	(working copy)
@@ -48,6 +48,7 @@
 #include "mod_core.h"
 #include "mod_proxy.h"
 #include "ap_listen.h"
+#include "ap_provider.h"
 
 #include "mod_so.h" /* for ap_find_loaded_module_symbol */
 
@@ -3833,6 +3834,49 @@
     return a;
 }
 
+static const char *set_errorlog(cmd_parms *cmd, void *dummy, const char *arg1,
+                                const char *arg2)
+{
+    ap_errorlog_provider *provider;
+    cmd->server->errorlog_provider = NULL;
+
+    if (!arg2) {
+        /* Stay backward compatible and check for "syslog" */
+        if (strncmp("syslog", arg1, 6) == 0) {
+            arg2 = arg1 + 7; /* skip the ':' if any */
+            arg1 = "syslog";
+        }
+        else {
+            /* Admin can define only "ErrorLog provider" and we should 
+             * still handle that using the defined provider, but with empty
+             * error_fname. */
+            provider = ap_lookup_provider(AP_ERRORLOG_PROVIDER_GROUP, arg1,
+                                          AP_ERRORLOG_PROVIDER_VERSION);
+            if (provider) {
+                arg2 = "";
+            }
+            else {
+                return set_server_string_slot(cmd, dummy, arg1);
+            }
+        }
+    }
+
+    if (strcmp("file", arg1) == 0) {
+        return set_server_string_slot(cmd, dummy, arg2);
+    }
+
+    provider = ap_lookup_provider(AP_ERRORLOG_PROVIDER_GROUP, arg1,
+                                    AP_ERRORLOG_PROVIDER_VERSION);
+    if (!provider) {
+        return apr_psprintf(cmd->pool,
+                            "Unknown ErrorLog provider: %s",
+                            arg1);
+    }
+
+    cmd->server->errorlog_provider = provider;
+    return set_server_string_slot(cmd, dummy, arg2);
+}
+
 static const char *set_errorlog_format(cmd_parms *cmd, void *dummy,
                                        const char *arg1, const char *arg2)
 {
@@ -4004,7 +4048,7 @@
   "Common directory of server-related files (logs, confs, etc.)"),
 AP_INIT_TAKE1("DefaultRuntimeDir", set_runtime_dir, NULL, RSRC_CONF | EXEC_ON_READ,
   "Common directory for run-time files (shared memory, locks, etc.)"),
-AP_INIT_TAKE1("ErrorLog", set_server_string_slot,
+AP_INIT_TAKE12("ErrorLog", set_errorlog,
   (void *)APR_OFFSETOF(server_rec, error_fname), RSRC_CONF,
   "The filename of the error log"),
 AP_INIT_TAKE12("ErrorLogFormat", set_errorlog_format, NULL, RSRC_CONF,
@@ -4440,7 +4484,8 @@
 static int check_errorlog_dir(apr_pool_t *p, server_rec *s)
 {
     if (!s->error_fname || s->error_fname[0] == '|'
-        || strcmp(s->error_fname, "syslog") == 0) {
+        || strcmp(s->error_fname, "syslog") == 0
+        || s->errorlog_provider != NULL) {
         return APR_SUCCESS;
     }
     else {
@@ -4847,7 +4892,8 @@
     apr_file_printf(out, "ServerRoot: \"%s\"\n", ap_server_root);
     tmp = ap_server_root_relative(p, sconf->ap_document_root);
     apr_file_printf(out, "Main DocumentRoot: \"%s\"\n", tmp);
-    if (s->error_fname[0] != '|' && strcmp(s->error_fname, "syslog") != 0)
+    if (s->error_fname[0] != '|' && strcmp(s->error_fname, "syslog") != 0
+        && s->errorlog_provider == NULL)
         tmp = ap_server_root_relative(p, s->error_fname);
     else
         tmp = s->error_fname;
Index: server/log.c
===================================================================
--- server/log.c	(revision 1643223)
+++ server/log.c	(working copy)
@@ -53,6 +53,7 @@
 #include "http_main.h"
 #include "util_time.h"
 #include "ap_mpm.h"
+#include "ap_provider.h"
 
 #if HAVE_GETTID
 #include <sys/syscall.h>
@@ -406,7 +407,7 @@
                             fac->t_val);
                     s->error_log = NULL;
                     return OK;
-                }
+               }
             }
         }
         else {
@@ -416,6 +417,14 @@
         s->error_log = NULL;
     }
 #endif
+    else if (s->errorlog_provider) {
+        s->errorlog_provider_handle = s->errorlog_provider->init(p, s);
+        s->error_log = NULL;
+        if (!s->errorlog_provider_handle) {
+            /* provider must log something to the console */
+            return DONE;
+        }
+    }
     else {
         fname = ap_server_root_relative(p, s->error_fname);
         if (!fname) {
@@ -514,9 +523,12 @@
 #define NULL_DEVICE "/dev/null"
 #endif
 
-    if (replace_stderr && freopen(NULL_DEVICE, "w", stderr) == NULL) {
-        ap_log_error(APLOG_MARK, APLOG_CRIT, errno, s_main, APLOGNO(00093)
-                     "unable to replace stderr with %s", NULL_DEVICE);
+    if (replace_stderr) {
+        if (freopen(NULL_DEVICE, "w", stderr) == NULL) {
+            ap_log_error(APLOG_MARK, APLOG_CRIT, errno, s_main, APLOGNO(00093)
+                        "unable to replace stderr with %s", NULL_DEVICE);
+        }
+        stderr_log = NULL;
     }
 
     for (virt = s_main->next; virt; virt = virt->next) {
@@ -537,6 +549,18 @@
                 virt->error_log = q->error_log;
             }
         }
+        else if (virt->errorlog_provider) {
+            /* separately-configured vhost-specific provider */
+            if (open_error_log(virt, 0, p) != OK) {
+                return DONE;
+            }
+        }
+        else if (s_main->errorlog_provider) {
+            /* inherit provider from s_main */
+            virt->errorlog_provider = s_main->errorlog_provider;
+            virt->errorlog_provider_handle = s_main->errorlog_provider_handle;
+            virt->error_log = NULL;
+        }
         else {
             virt->error_log = s_main->error_log;
         }
@@ -909,7 +933,8 @@
 
 /*
  * This is used if no error log format is defined and during startup.
- * It automatically omits the timestamp if logging to syslog.
+ * It automatically omits the timestamp if logging to syslog or
+ * using provider.
  */
 static int do_errorlog_default(const ap_errorlog_info *info, char *buf,
                                int buflen, int *errstr_start, int *errstr_end,
@@ -922,7 +947,7 @@
     char scratch[MAX_STRING_LEN];
 #endif
 
-    if (!info->using_syslog && !info->startup) {
+    if (!info->using_syslog && !info->using_provider && !info->startup) {
         buf[len++] = '[';
         len += log_ctime(info, "u", buf + len, buflen - len);
         buf[len++] = ']';
@@ -1081,13 +1106,8 @@
 static void write_logline(char *errstr, apr_size_t len, apr_file_t *logf,
                           int level)
 {
-    /* NULL if we are logging to syslog */
+    /* NULL if we are writting to syslog */
     if (logf) {
-        /* Truncate for the terminator (as apr_snprintf does) */
-        if (len > MAX_STRING_LEN - sizeof(APR_EOL_STR)) {
-            len = MAX_STRING_LEN - sizeof(APR_EOL_STR);
-        }
-        strcpy(errstr + len, APR_EOL_STR);
         apr_file_puts(errstr, logf);
         apr_file_flush(logf);
     }
@@ -1112,6 +1132,8 @@
     const request_rec *rmain = NULL;
     core_server_config *sconf = NULL;
     ap_errorlog_info info;
+    ap_errorlog_provider *errorlog_provider = NULL;
+    void *errorlog_provider_handle = NULL;
 
     /* do we need to log once-per-req or once-per-conn info? */
     int log_conn_info = 0, log_req_info = 0;
@@ -1138,33 +1160,31 @@
 #endif
 
         logf = stderr_log;
+        if (!logf && ap_server_conf && ap_server_conf->errorlog_provider) {
+            errorlog_provider = ap_server_conf->errorlog_provider;
+            errorlog_provider_handle = ap_server_conf->errorlog_provider_handle;
+        }
     }
     else {
         int configured_level = r ? ap_get_request_module_loglevel(r, module_index)        :
                                c ? ap_get_conn_server_module_loglevel(c, s, module_index) :
                                    ap_get_server_module_loglevel(s, module_index);
+        /*
+         * If we are doing normal logging, don't log messages that are
+         * above the module's log level unless it is a startup/shutdown notice
+         */
+        if ((level_and_mask != APLOG_NOTICE)
+            && (level_and_mask > configured_level)) {
+            return;
+        }
+
         if (s->error_log) {
-            /*
-             * If we are doing normal logging, don't log messages that are
-             * above the module's log level unless it is a startup/shutdown notice
-             */
-            if ((level_and_mask != APLOG_NOTICE)
-                && (level_and_mask > configured_level)) {
-                return;
-            }
-
             logf = s->error_log;
         }
-        else {
-            /*
-             * If we are doing syslog logging, don't log messages that are
-             * above the module's log level (including a startup/shutdown notice)
-             */
-            if (level_and_mask > configured_level) {
-                return;
-            }
-        }
 
+        errorlog_provider = s->errorlog_provider;
+        errorlog_provider_handle = s->errorlog_provider_handle;
+
         /* the faked server_rec from mod_cgid does not have s->module_config */
         if (s->module_config) {
             sconf = ap_get_core_module_config(s->module_config);
@@ -1193,6 +1213,15 @@
         }
     }
 
+    if (!logf && !(errorlog_provider && errorlog_provider_handle)) {
+        /* There is no file to send the log message to (or it is
+         * redirected to /dev/null and therefore any formating done below
+         * would be lost anyway) and there is no initialized log provider
+         * available, so we just return here.
+         */
+        return;
+    }
+
     info.s             = s;
     info.c             = c;
     info.pool          = pool;
@@ -1199,7 +1228,8 @@
     info.file          = NULL;
     info.line          = 0;
     info.status        = 0;
-    info.using_syslog  = (logf == NULL);
+    info.using_provider= s && errorlog_provider != NULL;
+    info.using_syslog  = logf == NULL && !info.using_provider;
     info.startup       = ((level & APLOG_STARTUP) == APLOG_STARTUP);
     info.format        = fmt;
 
@@ -1277,8 +1307,27 @@
              */
             continue;
         }
-        write_logline(errstr, len, logf, level_and_mask);
 
+        if (logf
+            || (info.using_provider
+                && (errorlog_provider->flags
+                    & AP_ERRORLOG_PROVIDER_ADD_EOL_STR))) {
+            /* Truncate for the terminator (as apr_snprintf does) */
+            if (len > MAX_STRING_LEN - sizeof(APR_EOL_STR)) {
+                len = MAX_STRING_LEN - sizeof(APR_EOL_STR);
+            }
+            strcpy(errstr + len, APR_EOL_STR);
+            len += strlen(APR_EOL_STR);
+        }
+
+        if (!info.using_provider) {
+            write_logline(errstr, len, logf, level_and_mask);
+        }
+        else {
+            errorlog_provider->writer(&info, errorlog_provider_handle,
+                                      errstr, len);
+        }
+
         if (done) {
             /*
              * We don't call the error_log hook for per-request/per-conn

Reply via email to