Merge pull request #11770 from ncteisen/too-much-trace

Ensure that GRPC_VERBOSITY is checked before logging

7 files changed, 55 insertions, 21 deletions

diff --git a/src/core/lib/support/env.h b/src/core/lib/support/env.h
index 18bc08ac62..e2c012a728 100644
--- a/src/core/lib/support/env.h
+++ b/src/core/lib/support/env.h
@@ -36,6 +36,12 @@ char *gpr_getenv(const char *name);
 /* Sets the the environment with the specified name to the specified value. */
 void gpr_setenv(const char *name, const char *value);
 
+/* This is a version of gpr_getenv that does not produce any output if it has to
+   use an insecure version of the function. It is ONLY to be used to solve the
+   problem in which we need to check an env variable to configure the verbosity
+   level of logging. So DO NOT USE THIS. */
+const char *gpr_getenv_silent(const char *name, char **dst);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/core/lib/support/env_linux.c b/src/core/lib/support/env_linux.c
index 0c79a2c401..4c45a977ca 100644
--- a/src/core/lib/support/env_linux.c
+++ b/src/core/lib/support/env_linux.c
@@ -38,7 +38,9 @@
 
 #include "src/core/lib/support/string.h"
 
-char *gpr_getenv(const char *name) {
+const char *gpr_getenv_silent(const char *name, char **dst) {
+  const char *insecure_func_used = NULL;
+  char *result = NULL;
 #if defined(GPR_BACKWARDS_COMPATIBILITY_MODE)
   typedef char *(*getenv_type)(const char *);
   static getenv_type getenv_func = NULL;
@@ -48,22 +50,28 @@ char *gpr_getenv(const char *name) {
   for (size_t i = 0; getenv_func == NULL && i < GPR_ARRAY_SIZE(names); i++) {
     getenv_func = (getenv_type)dlsym(RTLD_DEFAULT, names[i]);
     if (getenv_func != NULL && strstr(names[i], "secure") == NULL) {
-      gpr_log(GPR_DEBUG,
-              "Warning: insecure environment read function '%s' used",
-              names[i]);
+      insecure_func_used = names[i];
     }
   }
-  char *result = getenv_func(name);
-  return result == NULL ? result : gpr_strdup(result);
+  result = getenv_func(name);
 #elif __GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 17)
-  char *result = secure_getenv(name);
-  return result == NULL ? result : gpr_strdup(result);
+  result = secure_getenv(name);
 #else
-  gpr_log(GPR_DEBUG, "Warning: insecure environment read function '%s' used",
-          "getenv");
-  char *result = getenv(name);
-  return result == NULL ? result : gpr_strdup(result);
+  result = getenv(name);
+  insecure_func_used = "getenv";
 #endif
+  *dst = result == NULL ? result : gpr_strdup(result);
+  return insecure_func_used;
+}
+
+char *gpr_getenv(const char *name) {
+  char *result = NULL;
+  const char *insecure_func_used = gpr_getenv_silent(name, &result);
+  if (insecure_func_used != NULL) {
+    gpr_log(GPR_DEBUG, "Warning: insecure environment read function '%s' used",
+            insecure_func_used);
+  }
+  return result;
 }
 
 void gpr_setenv(const char *name, const char *value) {
diff --git a/src/core/lib/support/env_posix.c b/src/core/lib/support/env_posix.c
index bdbc4da95a..b88822ca02 100644
--- a/src/core/lib/support/env_posix.c
+++ b/src/core/lib/support/env_posix.c
@@ -29,6 +29,11 @@
 #include <grpc/support/string_util.h>
 #include "src/core/lib/support/string.h"
 
+const char *gpr_getenv_silent(const char *name, char **dst) {
+  *dst = gpr_getenv(name);
+  return NULL;
+}
+
 char *gpr_getenv(const char *name) {
   char *result = getenv(name);
   return result == NULL ? result : gpr_strdup(result);
diff --git a/src/core/lib/support/env_windows.c b/src/core/lib/support/env_windows.c
index c1d557e219..652eeb61c6 100644
--- a/src/core/lib/support/env_windows.c
+++ b/src/core/lib/support/env_windows.c
@@ -30,6 +30,11 @@
 #include <grpc/support/log.h>
 #include <grpc/support/string_util.h>
 
+const char *gpr_getenv_silent(const char *name, char **dst) {
+  *dst = gpr_getenv(name);
+  return NULL;
+}
+
 char *gpr_getenv(const char *name) {
   char *result = NULL;
   DWORD size;
diff --git a/src/core/lib/support/log.c b/src/core/lib/support/log.c
index bcc336b8ae..fadb4d9a2c 100644
--- a/src/core/lib/support/log.c
+++ b/src/core/lib/support/log.c
@@ -64,7 +64,8 @@ void gpr_set_log_verbosity(gpr_log_severity min_severity_to_print) {
 }
 
 void gpr_log_verbosity_init() {
-  char *verbosity = gpr_getenv("GRPC_VERBOSITY");
+  char *verbosity = NULL;
+  const char *insecure_getenv = gpr_getenv_silent("GRPC_VERBOSITY", &verbosity);
 
   gpr_atm min_severity_to_print = GPR_LOG_SEVERITY_ERROR;
   if (verbosity != NULL) {
@@ -81,6 +82,11 @@ void gpr_log_verbosity_init() {
       GPR_LOG_VERBOSITY_UNSET) {
     gpr_atm_no_barrier_store(&g_min_severity_to_print, min_severity_to_print);
   }
+
+  if (insecure_getenv != NULL) {
+    gpr_log(GPR_DEBUG, "Warning: insecure environment read function '%s' used",
+            insecure_getenv);
+  }
 }
 
 void gpr_set_log_function(gpr_log_func f) {
diff --git a/src/cpp/server/server_builder.cc b/src/cpp/server/server_builder.cc
index c90f96c0b7..200e477822 100644
--- a/src/cpp/server/server_builder.cc
+++ b/src/cpp/server/server_builder.cc
@@ -250,14 +250,6 @@ std::unique_ptr<Server> ServerBuilder::BuildAndStart() {
       has_sync_methods && num_frequently_polled_cqs > 0;
 
   if (has_sync_methods) {
-    // This is a Sync server
-    gpr_log(GPR_INFO,
-            "Synchronous server. Num CQs: %d, Min pollers: %d, Max Pollers: "
-            "%d, CQ timeout (msec): %d",
-            sync_server_settings_.num_cqs, sync_server_settings_.min_pollers,
-            sync_server_settings_.max_pollers,
-            sync_server_settings_.cq_timeout_msec);
-
     grpc_cq_polling_type polling_type =
         is_hybrid_server ? GRPC_CQ_NON_POLLING : GRPC_CQ_DEFAULT_POLLING;
 
@@ -272,6 +264,16 @@ std::unique_ptr<Server> ServerBuilder::BuildAndStart() {
       sync_server_settings_.min_pollers, sync_server_settings_.max_pollers,
       sync_server_settings_.cq_timeout_msec));
 
+  if (has_sync_methods) {
+    // This is a Sync server
+    gpr_log(GPR_INFO,
+            "Synchronous server. Num CQs: %d, Min pollers: %d, Max Pollers: "
+            "%d, CQ timeout (msec): %d",
+            sync_server_settings_.num_cqs, sync_server_settings_.min_pollers,
+            sync_server_settings_.max_pollers,
+            sync_server_settings_.cq_timeout_msec);
+  }
+
   ServerInitializer* initializer = server->initializer();
 
   // Register all the completion queues with the server. i.e
diff --git a/tools/run_tests/sanity/core_banned_functions.py b/tools/run_tests/sanity/core_banned_functions.py
index b394bbbeaf..1f13905484 100755
--- a/tools/run_tests/sanity/core_banned_functions.py
+++ b/tools/run_tests/sanity/core_banned_functions.py
@@ -41,6 +41,8 @@ BANNED_EXCEPT = {
     'grpc_closure_sched(' : ['src/core/lib/iomgr/closure.c'],
     'grpc_closure_run(' : ['src/core/lib/iomgr/closure.c'],
     'grpc_closure_list_sched(' : ['src/core/lib/iomgr/closure.c'],
+    'gpr_getenv_silent(' : ['src/core/lib/support/log.c', 'src/core/lib/support/env_linux.c', 
+                            'src/core/lib/support/env_posix.c', 'src/core/lib/support/env_windows.c'],
 }
 
 errors = 0