On 08.06.2010 10:21, Joe Orton wrote:
On Mon, Jun 07, 2010 at 12:23:26PM -0000, rj...@apache.org wrote:
Author: rjung
Date: Mon Jun 7 12:23:26 2010
New Revision: 952201
URL: http://svn.apache.org/viewvc?rev=952201&view=rev
Log:
Add process id and thread id (if APR has thread support)
to the error log.
...
@@ -620,6 +621,18 @@ static void log_error_core(const char *f
if ((level& APLOG_STARTUP) != APLOG_STARTUP) {
len += apr_snprintf(errstr + len, MAX_STRING_LEN - len,
"[%s] ", priorities[level_and_mask].t_name);
+
+ len += apr_snprintf(errstr + len, MAX_STRING_LEN - len,
+ "[%" APR_PID_T_FMT, getpid());
+#if APR_HAS_THREADS
+ {
+ apr_os_thread_t tid = apr_os_thread_current();
+ len += apr_snprintf(errstr + len, MAX_STRING_LEN - len,
+ ":%pT",&tid);
+ }
+#endif
+ errstr[len++] = ']';
+ errstr[len++] = ' ';
These numbers made me double-take when reading error_log since they
don't have a descriptive prefix. Also the tid is not much use in a
non-threaded server. Could we do something like this? (Hopefully this
function won't start showing up in CPU profiles!)
I'm neutral on adding the descriptive prefixes (+0). It's helpful to get
acquainted with the new format, on the long term it might become
unnecessary. I guess when adding new standard tokens to the error log we
might end up doing a callback based configurable format like in
mod_log_config.
Removing the tid for non-threaded servers is fine. I thought about that
too. Would it make sense to somehow cache the result of
ap_mpm_query(AP_MPMQ_IS_THREADED,&result) in one of the structs
available from the log function, because it won't change during the
runtime of the instance (at least not during the runtime of the child;
don't know whether it would be feasible to switch the new dynamically
loaded MPMs during restart, but that's another topic).
Index: server/log.c
===================================================================
--- server/log.c (revision 952555)
+++ server/log.c (working copy)
@@ -628,12 +628,17 @@
"[%s] ", priorities[level_and_mask].t_name);
len += apr_snprintf(errstr + len, MAX_STRING_LEN - len,
- "[%" APR_PID_T_FMT, getpid());
+ "[pid %" APR_PID_T_FMT, getpid());
#if APR_HAS_THREADS
{
- apr_os_thread_t tid = apr_os_thread_current();
- len += apr_snprintf(errstr + len, MAX_STRING_LEN - len,
- ":%pT",&tid);
+ int result;
+
+ if (ap_mpm_query(AP_MPMQ_IS_THREADED,&result) == 0
+&& result == 1) {
+ apr_os_thread_t tid = apr_os_thread_current();
+ len += apr_snprintf(errstr + len, MAX_STRING_LEN - len,
+ ":tid %pT",&tid);
+ }
}
#endif
errstr[len++] = ']';
Thanks for reviewing!
Rainer
