butil/logging could be overwritten by glog

Change-Id: I14cf8724175d1fb5237fe20228f822afdd8521cc
ycllz · Sep 12, 2017 · 6fdeeac · 6fdeeac
1 parent 52afeeb
commit 6fdeeac
Show file tree

Hide file tree

Showing 40 changed files with 405 additions and 391 deletions.
diff --git a/Makefile b/Makefile
@@ -6,9 +6,9 @@ include config.mk
 # 2. Added -D__const__= : Avoid over-optimizations of TLS variables by GCC>=4.8
 # 3. Removed -Werror: Not block compilation for non-vital warnings, especially when the
 #    code is tested on newer systems. If the code is used in production, add -Werror back
-CPPFLAGS=-DBTHREAD_USE_FAST_PTHREAD_MUTEX -D__const__= -D_GNU_SOURCE -DUSE_SYMBOLIZE -DNO_TCMALLOC -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -D__STDC_CONSTANT_MACROS -DBRPC_REVISION=\"$(shell git rev-parse --short HEAD)\"
-CXXFLAGS=$(CPPFLAGS) -g -O2 -pipe -Wall -W -fPIC -fstrict-aliasing -Wno-invalid-offsetof -Wno-unused-parameter -fno-omit-frame-pointer -std=c++0x
-CFLAGS=$(CPPFLAGS) -g -O2 -pipe -Wall -W -fPIC -fstrict-aliasing -Wno-unused-parameter -fno-omit-frame-pointer
+CPPFLAGS+=-DBTHREAD_USE_FAST_PTHREAD_MUTEX -D__const__= -D_GNU_SOURCE -DUSE_SYMBOLIZE -DNO_TCMALLOC -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -D__STDC_CONSTANT_MACROS -DBRPC_REVISION=\"$(shell git rev-parse --short HEAD)\"
+CXXFLAGS+=$(CPPFLAGS) -g -O2 -pipe -Wall -W -fPIC -fstrict-aliasing -Wno-invalid-offsetof -Wno-unused-parameter -fno-omit-frame-pointer -std=c++0x
+CFLAGS+=$(CPPFLAGS) -g -O2 -pipe -Wall -W -fPIC -fstrict-aliasing -Wno-unused-parameter -fno-omit-frame-pointer
 HDRPATHS=-I./src $(addprefix -I, $(HDRS))
 LIBPATHS = $(addprefix -L, $(LIBS))
 COMMA = ,
@@ -270,7 +270,7 @@ output/bin:protoc-gen-mcpack
 
 %.dbg.o:%.cpp
 	@echo "Compiling $@"
-	@$(CXX) -c $(HDRPATHS) $(CXXFLAGS) $< -o $@
+	@$(CXX) -c $(HDRPATHS) $(CXXFLAGS) -DBVAR_NOT_LINK_DEFAULT_VARIABLES $< -o $@
 
 %.o:%.cc
 	@echo "Compiling $@"

diff --git a/config_brpc.sh b/config_brpc.sh
@@ -5,7 +5,8 @@ else
 fi
 # NOTE: This requires GNU getopt.  On Mac OS X and FreeBSD, you have to install this
 # separately; see below.
-TEMP=`getopt -o v: --long headers:,libs:,cc:,cxx: -n 'config_brpc' -- "$@"`
+TEMP=`getopt -o v: --long headers:,libs:,cc:,cxx:,with-glog -n 'config_brpc' -- "$@"`
+WITH_GLOG=0
 
 if [ $? != 0 ] ; then >&2 $ECHO "Terminating..."; exit 1 ; fi
 
@@ -19,10 +20,12 @@ while true; do
         --libs ) LIBS_IN="$(readlink -f $2)"; shift 2 ;;
         --cc ) CC=$2; shift 2 ;;
         --cxx ) CXX=$2; shift 2 ;;
+        --with-glog ) WITH_GLOG=1; shift 1 ;;
         -- ) shift; break ;;
         * ) break ;;
     esac
 done
+
 if [ -z "$CC" ]; then
     if [ ! -z "$CXX" ]; then
         >&2 $ECHO "--cc and --cxx must be both set or unset"
@@ -82,8 +85,17 @@ find_bin_or_die() {
 find_dir_of_header() {
     find ${HDRS_IN} -path "*/$1" | head -n1 | sed "s|$1||g"
 }
+
+find_dir_of_header_excluding() {
+    find ${HDRS_IN} -path "*/$1" | grep -v "$2\$" | head -n1 | sed "s|$1||g"
+}
+
 find_dir_of_header_or_die() {
-    local dir=$(find_dir_of_header $1)
+    if [ -z "$2" ]; then
+        local dir=$(find_dir_of_header $1)
+    else
+        local dir=$(find_dir_of_header_excluding $1 $2)
+    fi
     if [ -z "$dir" ]; then
         >&2 $ECHO "Fail to find $1 from --headers"
         exit 1
@@ -243,6 +255,23 @@ else
 fi
 append_to_output "endif"
 
+if [ $WITH_GLOG != 0 ]; then
+    GLOG_LIB=$(find_dir_of_lib glog)
+    GLOG_HDR=$(find_dir_of_header_or_die glog/logging.h windows/glog/logging.h)
+    append_to_output_headers "$GLOG_HDR" "    "
+    if [ -z "$GLOG_LIB" ]; then
+        append_to_output "    \$(error \"Fail to find glog\")"
+    else
+        append_to_output_libs "$GLOG_LIB" "    "
+        if [ -f $GLOG_LIB/libglog.a ]; then
+            append_to_output "    STATIC_LINKINGS+=-lglog"
+        else
+            append_to_output "    DYNAMIC_LINKINGS+=-lglog"
+        fi
+    fi
+fi
+append_to_output "CPPFLAGS+=-DBRPC_WITH_GLOG=$WITH_GLOG"
+
 # required by UT
 #gtest
 GTEST_LIB=$(find_dir_of_lib gtest)
@@ -276,5 +305,19 @@ else
 fi
 append_to_output "endif"
 
+# generate src/butil/config.h
+cat << EOF > src/butil/config.h
+// This file is auto-generated by $(basename "$0"). DON'T edit it!
+#ifndef  BUTIL_CONFIG_H
+#define  BUTIL_CONFIG_H
+
+#ifdef BRPC_WITH_GLOG
+#undef BRPC_WITH_GLOG
+#endif
+#define BRPC_WITH_GLOG $WITH_GLOG
+
+#endif  // BUTIL_CONFIG_H
+EOF
+
 # write to config.mk
 $ECHO "$OUTPUT_CONTENT" > config.mk
diff --git a/src/brpc/acceptor.cpp b/src/brpc/acceptor.cpp
@@ -15,6 +15,7 @@
 // Authors: Rujie Jiang([email protected])
 //          Ge,Jun([email protected])
 
+#include <inttypes.h>
 #include <gflags/gflags.h>
 #include "butil/fd_guard.h"                 // fd_guard 
 #include "butil/fd_utility.h"               // make_close_on_exec

diff --git a/src/brpc/amf.h b/src/brpc/amf.h
@@ -22,6 +22,7 @@
 #include <google/protobuf/io/zero_copy_stream.h>
 #include <google/protobuf/message.h>
 #include "butil/sys_byteorder.h"
+#include "butil/strings/string_piece.h"
 
 
 namespace brpc {

diff --git a/src/brpc/builtin/hotspots_service.cpp b/src/brpc/builtin/hotspots_service.cpp
@@ -547,17 +547,18 @@ static void DoProfiling(ProfilingType type,
     }
 
     // Log requester
-    LOG(INFO) << cntl->remote_side() << noflush;
+    std::ostringstream client_info;
+    client_info << cntl->remote_side();
     if (cntl->auth_context()) {
-        LOG(INFO) << "(auth=" << cntl->auth_context()->user() << ')' << noflush;
+        client_info << "(auth=" << cntl->auth_context()->user() << ')';
     } else {
-        LOG(INFO) << "(no auth)" << noflush;
+        client_info << "(no auth)";
     }
-    LOG(INFO) << " requests for profiling " << ProfilingType2String(type) << noflush;
+    client_info << " requests for profiling " << ProfilingType2String(type);
     if (type == PROFILING_CPU || type == PROFILING_CONTENTION) {
-        LOG(INFO) << " for " << seconds << " seconds";
+        LOG(INFO) << client_info.str() << " for " << seconds << " seconds";
     } else {
-        LOG(INFO);
+        LOG(INFO) << client_info.str();
     }
     int64_t prof_id = 0;
     const std::string* prof_id_str =

diff --git a/src/brpc/builtin/pprof_service.cpp b/src/brpc/builtin/pprof_service.cpp
@@ -108,13 +108,15 @@ void PProfService::profile(
         return;
     }
     // Log requester
-    LOG(INFO) << cntl->remote_side() << noflush;
+    std::ostringstream client_info;
+    client_info << cntl->remote_side();
     if (cntl->auth_context()) {
-        LOG(INFO) << "(auth=" << cntl->auth_context()->user() << ')' << noflush;
+        client_info << "(auth=" << cntl->auth_context()->user() << ')';
     } else {
-        LOG(INFO) << "(no auth)" << noflush;
+        client_info << "(no auth)";
     }
-    LOG(INFO) << " requests for cpu profile for " << sleep_sec << " seconds";
+    LOG(INFO) << client_info.str() << " requests for cpu profile for "
+              << sleep_sec << " seconds";
 
     char prof_name[256];
     if (MakeProfName(PROFILING_CPU, prof_name, sizeof(prof_name)) != 0) {
@@ -163,13 +165,15 @@ void PProfService::contention(
         return;
     }
     // Log requester
-    LOG(INFO) << cntl->remote_side() << noflush;
+    std::ostringstream client_info;
+    client_info << cntl->remote_side();
     if (cntl->auth_context()) {
-        LOG(INFO) << "(auth=" << cntl->auth_context()->user() << ')' << noflush;
+        client_info << "(auth=" << cntl->auth_context()->user() << ')';
     } else {
-        LOG(INFO) << "(no auth)" << noflush;
+        client_info << "(no auth)";
     }
-    LOG(INFO) << " requests for contention profile for " << sleep_sec << " seconds";
+    LOG(INFO) << client_info.str() << " requests for contention profile for "
+              << sleep_sec << " seconds";
 
     char prof_name[256];
     if (MakeProfName(PROFILING_CONTENTION, prof_name, sizeof(prof_name)) != 0) {
@@ -210,13 +214,14 @@ void PProfService::heap(
         return;
     }
     // Log requester
-    LOG(INFO) << cntl->remote_side() << noflush;
+    std::ostringstream client_info;
+    client_info << cntl->remote_side();
     if (cntl->auth_context()) {
-        LOG(INFO) << "(auth=" << cntl->auth_context()->user() << ')' << noflush;
+        client_info << "(auth=" << cntl->auth_context()->user() << ')';
     } else {
-        LOG(INFO) << "(no auth)" << noflush;
+        client_info << "(no auth)";
     }
-    LOG(INFO) << " requests for heap profile";
+    LOG(INFO) << client_info.str() << " requests for heap profile";
 
     std::string obj;
     malloc_ext->GetHeapSample(&obj);
@@ -239,13 +244,14 @@ void PProfService::growth(
         return;
     }
     // Log requester
-    LOG(INFO) << cntl->remote_side() << noflush;
+    std::ostringstream client_info;
+    client_info << cntl->remote_side();
     if (cntl->auth_context()) {
-        LOG(INFO) << "(auth=" << cntl->auth_context()->user() << ')' << noflush;
+        client_info << "(auth=" << cntl->auth_context()->user() << ')';
     } else {
-        LOG(INFO) << "(no auth)" << noflush;
+        client_info << "(no auth)";
     }
-    LOG(INFO) << " requests for growth profile";
+    LOG(INFO) << client_info.str() << " requests for growth profile";
 
     std::string obj;
     malloc_ext->GetHeapGrowthStacks(&obj);

diff --git a/src/brpc/builtin/vlog_service.cpp b/src/brpc/builtin/vlog_service.cpp
@@ -14,13 +14,14 @@
 
 // Authors: Ge,Jun ([email protected])
 
+#if !BRPC_WITH_GLOG 
+
 #include "brpc/log.h"
 #include "brpc/controller.h"           // Controller
 #include "brpc/closure_guard.h"        // ClosureGuard
 #include "brpc/builtin/vlog_service.h"
 #include "brpc/builtin/common.h"
 
-
 namespace brpc {
 
 class VLogPrinter : public VLogSitePrinter {
@@ -91,3 +92,6 @@ void VLogService::default_method(::google::protobuf::RpcController* cntl_base,
 }
 
 } // namespace brpc
+
+#endif
+
diff --git a/src/brpc/builtin/vlog_service.h b/src/brpc/builtin/vlog_service.h
@@ -17,6 +17,7 @@
 #ifndef  BRPC_VLOG_SERVICE_H
 #define  BRPC_VLOG_SERVICE_H
 
+#if !BRPC_WITH_GLOG 
 #include <ostream>
 #include "brpc/builtin_service.pb.h"
 
@@ -34,5 +35,6 @@ class VLogService : public vlog {
 
 } // namespace brpc
 
+#endif  // BRPC_WITH_GLOG
 
 #endif  //BRPC_VLOG_SERVICE_H
diff --git a/src/brpc/channel.cpp b/src/brpc/channel.cpp
@@ -16,6 +16,7 @@
 //          Rujie Jiang([email protected])
 //          Zhangyi Chen([email protected])
 
+#include <inttypes.h>
 #include <google/protobuf/descriptor.h>
 #include <gflags/gflags.h>
 #include "butil/time.h"                               // milliseconds_from_now

diff --git a/src/brpc/details/naming_service_thread.cpp b/src/brpc/details/naming_service_thread.cpp
@@ -168,15 +168,16 @@ void NamingServiceThread::Actions::ResetServers(
     }
 
     if (!_removed.empty() || !_added.empty()) {
-        LOG(INFO) << butil::class_name_str(*_owner->_ns) << "(\"" 
-                  << _owner->_service_name << "\"):" << noflush;
+        std::ostringstream info;
+        info << butil::class_name_str(*_owner->_ns) << "(\"" 
+             << _owner->_service_name << "\"):";
         if (!_added.empty()) {
-            LOG(INFO) << " added "<< _added.size() << noflush;
+            info << " added "<< _added.size();
         }
         if (!_removed.empty()) {
-            LOG(INFO) << " removed " << _removed.size() << noflush;
+            info << " removed " << _removed.size();
         }
-        LOG(INFO);
+        LOG(INFO) << info.str();
     }
 
     EndWait(servers.empty() ? ENODATA : 0);

diff --git a/src/brpc/global.cpp b/src/brpc/global.cpp
@@ -228,19 +228,19 @@ static void BaiduStreamingLogHandler(google::protobuf::LogLevel level,
                                      const std::string& message) {
     switch (level) {
     case google::protobuf::LOGLEVEL_INFO:
-        LOG_AT(INFO, filename, line) << message;
+        LOG(INFO) << filename << ':' << line << ' ' << message;
         return;
     case google::protobuf::LOGLEVEL_WARNING:
-        LOG_AT(WARNING, filename, line) << message;
+        LOG(WARNING) << filename << ':' << line << ' ' << message;
         return;
     case google::protobuf::LOGLEVEL_ERROR:
-        LOG_AT(ERROR, filename, line) << message;
+        LOG(ERROR) << filename << ':' << line << ' ' << message;
         return;
     case google::protobuf::LOGLEVEL_FATAL:
-        LOG_AT(FATAL, filename, line) << message;
+        LOG(FATAL) << filename << ':' << line << ' ' << message;
         return;
     }
-    LOG_AT(FATAL, filename, line) << message;
+    CHECK(false) << filename << ':' << line << ' ' << message;
 }
 
 static void GlobalInitializeOrDieImpl() {

diff --git a/src/brpc/protocol.cpp b/src/brpc/protocol.cpp
@@ -171,14 +171,14 @@ ProtocolType StringToProtocolType(const butil::StringPiece& name,
     //   "channel doesn't support protocol=unknown"
     // Some callsite may not need this log, so we keep a flag.
     if (print_log_on_unknown) {
-        LOG(ERROR) << "Unknown protocol `" << name << "', supported protocols:"
-                   << noflush;
+        std::ostringstream err;
+        err << "Unknown protocol `" << name << "', supported protocols:";
         for (size_t i = 0; i < MAX_PROTOCOL_SIZE; ++i) {
             if (protocol_map[i].valid.load(butil::memory_order_acquire)) {
-                LOG(ERROR) << ' ' << protocol_map[i].protocol.name << noflush;
+                err << ' ' << protocol_map[i].protocol.name;
             }
         }
-        LOG(ERROR);
+        LOG(ERROR) << err.str();
     }
     return PROTOCOL_UNKNOWN;
 }

diff --git a/src/brpc/server.cpp b/src/brpc/server.cpp
@@ -491,10 +491,14 @@ int Server::AddBuiltinServices() {
         LOG(ERROR) << "Fail to add ThreadsService";
         return -1;
     }
+
+#if !BRPC_WITH_GLOG
     if (AddBuiltinService(new (std::nothrow) VLogService)) {
         LOG(ERROR) << "Fail to add VLogService";
         return -1;
     }
+#endif
+
     if (AddBuiltinService(new (std::nothrow) PProfService)) {
         LOG(ERROR) << "Fail to add PProfService";
         return -1;
@@ -578,14 +582,15 @@ Acceptor* Server::BuildAcceptor() {
         }
     }
     if (!whitelist.empty()) {
-        LOG(ERROR) << "ServerOptions.enabled_protocols has unknown protocols=`"
-                   << noflush;
+        std::ostringstream err;
+        err << "ServerOptions.enabled_protocols has unknown protocols=`";
         for (std::set<std::string>::const_iterator it = whitelist.begin();
              it != whitelist.end(); ++it) {
-            LOG(ERROR) << *it << ' ' << noflush;
+            err << *it << ' ';
         }
-        LOG(ERROR) << '\'';
+        err << '\'';
         delete acceptor;
+        LOG(ERROR) << err.str();
         return NULL;
     }
     return acceptor;
@@ -950,13 +955,14 @@ int Server::StartInternal(const butil::ip_t& ip,
 
     // Print tips to server launcher.
     int http_port = _listen_addr.port;
-    LOG(INFO) << "Server[" << version() << "] is serving on port="
-              << _listen_addr.port << noflush;
+    std::ostringstream server_info;
+    server_info << "Server[" << version() << "] is serving on port="
+                << _listen_addr.port;
     if (_options.internal_port >= 0 && _options.has_builtin_services) {
         http_port = _options.internal_port;
-        LOG(INFO) << " and internal_port=" << _options.internal_port << noflush;
+        server_info << " and internal_port=" << _options.internal_port;
     }
-    LOG(INFO) << '.';
+    LOG(INFO) << server_info.str() << '.';
 
     if (_options.has_builtin_services) {
         LOG(INFO) << "Check out http://" << butil::my_hostname() << ':'