logging: properly interpret google-style LOG() levels

config

@@ -129,7 +129,11 @@ if [ $ngx_found = yes ]; then
     $ps_src/ngx_server_context.h \
     $ps_src/ngx_rewrite_options.h \
     $ps_src/ngx_rewrite_driver_factory.h \
-    $ps_src/ngx_thread_system.h"
+    $ps_src/ngx_thread_system.h \
+    $ps_src/ngx_message_handler.h \
+    $ps_src/pthread_shared_mem.h \
+    $ps_src/ngx_request_context.h \
+    $ps_src/log_message_handler.h"
   NGX_ADDON_SRCS="$NGX_ADDON_SRCS \
     $ps_src/ngx_pagespeed.cc \
     $ps_src/ngx_rewrite_driver_factory.cc \
@@ -142,6 +146,7 @@ if [ $ngx_found = yes ]; then
     $ps_src/ngx_message_handler.cc \
     $ps_src/pthread_shared_mem.cc \
     $ps_src/ngx_request_context.cc \
+    $ps_src/log_message_handler.cc \
     $mod_pagespeed_dir/net/instaweb/apache/add_headers_fetcher.cc \
     $mod_pagespeed_dir/net/instaweb/apache/loopback_route_fetcher.cc \
     $mod_pagespeed_dir/net/instaweb/apache/serf_url_async_fetcher.cc"

src/log_message_handler.cc

@@ -0,0 +1,116 @@
+// Copyright 2010 Google Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+// Author: jmarantz@google.com (Joshua Marantz)
+// Author: sligocki@google.com (Shawn Ligocki)
+// Author: jefftk@google.com (Jeff Kaufman)
+
+// TODO(jefftk): share more of this code with apache's log_message_handler
+
+#include "log_message_handler.h"
+
+#include <unistd.h>
+
+#include <limits>
+#include <string>
+
+#include "base/debug/debugger.h"
+#include "base/debug/stack_trace.h"
+#include "base/logging.h"
+#include "net/instaweb/public/version.h"
+#include "net/instaweb/util/public/string_util.h"
+
+// Make sure we don't attempt to use LOG macros here, since doing so
+// would cause us to go into an infinite log loop.
+#undef LOG
+#define LOG USING_LOG_HERE_WOULD_CAUSE_INFINITE_RECURSION
+
+namespace {
+
+ngx_log_t* log = NULL;
+
+ngx_uint_t GetNgxLogLevel(int severity) {
+  switch (severity) {
+    case logging::LOG_INFO:
+      return NGX_LOG_INFO;
+    case logging::LOG_WARNING:
+      return NGX_LOG_WARN;
+    case logging::LOG_ERROR:
+      return NGX_LOG_ERR;
+    case logging::LOG_ERROR_REPORT:
+    case logging::LOG_FATAL:
+      return NGX_LOG_ALERT;
+    default:  // For VLOG(s)
+      return NGX_LOG_DEBUG;
+  }
+}
+
+bool LogMessageHandler(int severity, const char* file, int line,
+                       size_t message_start, const GoogleString& str) {
+  ngx_uint_t this_log_level = GetNgxLogLevel(severity);
+
+  GoogleString message = str;
+  if (severity == logging::LOG_FATAL) {
+    if (base::debug::BeingDebugged()) {
+      base::debug::BreakDebugger();
+    } else {
+      base::debug::StackTrace trace;
+      std::ostringstream stream;
+      trace.OutputToStream(&stream);
+      message.append(stream.str());
+    }
+  }
+
+  // Trim the newline off the end of the message string.
+  size_t last_msg_character_index = message.length() - 1;
+  if (message[last_msg_character_index] == '\n') {
+    message.resize(last_msg_character_index);
+  }
+
+  ngx_log_error(this_log_level, log, 0, "[ngx_pagespeed %s] %s",
+                net_instaweb::kModPagespeedVersion,
+                message.c_str());
+
+  if (severity == logging::LOG_FATAL) {
+    // Crash the process to generate a dump.
+    base::debug::BreakDebugger();
+  }
+
+  return true;
+}
+
+}  // namespace
+
+
+namespace net_instaweb {
+
+namespace log_message_handler {
+
+
+const int kDebugLogLevel = -2;
+
+void Install(ngx_log_t* log_in) {
+  log = log_in;
+  logging::SetLogMessageHandler(&LogMessageHandler);
+
+  // All VLOG(2) and higher will be displayed as DEBUG logs if the nginx log
+  // level is DEBUG.
+  if (log->log_level >= NGX_LOG_DEBUG) {
+    logging::SetMinLogLevel(-2);
+  }
+}
+
+}  // namespace log_message_handler
+
+}  // namespace net_instaweb

src/log_message_handler.h

@@ -0,0 +1,44 @@
+// Copyright 2010 Google Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+// Author: jmarantz@google.com (Joshua Marantz)
+// Author: sligocki@google.com (Shawn Ligocki)
+// Author: jefftk@google.com (Jeff Kaufman)
+
+#ifndef LOG_MESSAGE_HANDLER_H_
+#define LOG_MESSAGE_HANDLER_H_
+
+extern "C" {
+  #include <ngx_config.h>
+  #include <ngx_core.h>
+  #include <ngx_http.h>
+  #include <ngx_log.h>
+}
+
+namespace net_instaweb {
+
+namespace log_message_handler {
+
+// Install a log message handler that routes LOG() messages to the
+// server error log.  Should be called once at startup.  If server blocks define
+// their own logging files you would expect that LOG() messages would be routed
+// appropriately, but because logging is all done with global variables this
+// isn't possible.
+void Install(ngx_log_t* log_in);
+
+}  // namespace log_message_handler
+
+}  // namespace net_instaweb
+
+#endif  // LOG_MESSAGE_HANDLER_H_

src/ngx_message_handler.cc

@@ -104,7 +104,7 @@ void NgxMessageHandler::set_buffer(SharedCircularBuffer* buff) {
 }
 
 void NgxMessageHandler::MessageVImpl(MessageType type, const char* msg,
-                                        va_list args) {
+                                     va_list args) {
   ngx_uint_t log_level = GetNgxLogLevel(type);
   GoogleString formatted_message = Format(msg, args);
   if (log_ != NULL) {
@@ -136,8 +136,8 @@ void NgxMessageHandler::MessageVImpl(MessageType type, const char* msg,
 }
 
 void NgxMessageHandler::FileMessageVImpl(MessageType type, const char* file,
-                                            int line, const char* msg,
-                                            va_list args) {
+                                         int line, const char* msg,
+                                         va_list args) {
   ngx_uint_t log_level = GetNgxLogLevel(type);
   GoogleString formatted_message = Format(msg, args);
   if (log_ != NULL) {

src/ngx_pagespeed.cc

@@ -29,6 +29,7 @@ extern "C" {
   #include <ngx_config.h>
   #include <ngx_core.h>
   #include <ngx_http.h>
+  #include <ngx_log.h>
 }
 
 #include <unistd.h>

src/ngx_rewrite_driver_factory.cc

@@ -20,6 +20,7 @@
 
 #include <cstdio>
 
+#include "log_message_handler.h"
 #include "ngx_message_handler.h"
 #include "ngx_rewrite_options.h"
 #include "ngx_thread_system.h"
@@ -302,6 +303,8 @@ void NgxRewriteDriverFactory::SharedCircularBufferInit(bool is_root) {
 }
 
 void NgxRewriteDriverFactory::RootInit(ngx_log_t* log) {
+  net_instaweb::log_message_handler::Install(log);
+
   ParentOrChildInit(log);
 
   // Let SystemCaches know about the various paths we have in configuration

test/nginx_system_test.sh

@@ -478,6 +478,15 @@ fetch_until $URL 'fgrep -c web.example.ssp.css' 1
 # directly from the filesystem.
 fetch_until $URL 'fgrep -c file.exception.ssp.css' 1
 
+# Test that ngx_pagespeed keeps working after nginx gets a signal to reload the
+# configuration.  This is in the middle of tests so that significant work
+# happens both before and after.
+start_test "Reload config"
+
+check wget $EXAMPLE_ROOT/styles/W.rewrite_css_images.css.pagespeed.cf.Hash.css
+check_simple "$NGINX_EXECUTABLE" -s reload -c "$PAGESPEED_CONF"
+check wget $EXAMPLE_ROOT/styles/W.rewrite_css_images.css.pagespeed.cf.Hash.css
+
 start_test ModPagespeedLoadFromFileMatch
 URL=$TEST_ROOT/load_from_file_match/index.html?ModPagespeedFilters=inline_css
 fetch_until $URL 'grep -c blue' 1