feat: logging can be enabled via env var (googleapis/google-cloud-cpp…

…-common#181)

Users can now enable logging to `std::clog` without any code changes or
recompiling their code simply by setting the
"GOOGLE_CLOUD_CPP_ENABLE_CLOG" environment variable before running their
program.

This env var is currently being inspected and set in some of our other
code, such as
https://github.com/googleapis/google-cloud-cpp-spanner/blob/20e2f9b011e3747fe5a8b8dc284245c11ff354b6/google/cloud/spanner/connection_options.cc#L68,
and we should remove that after this change is merged.

google/cloud/connection_options.cc

@@ -42,13 +42,6 @@ TracingOptions DefaultTracingOptions() {
   return TracingOptions{}.SetOptions(*tracing_options);
 }
 
-void DefaultLogging() {
-  if (google::cloud::internal::GetEnv("GOOGLE_CLOUD_CPP_ENABLE_CLOG")
-          .has_value()) {
-    google::cloud::LogSink::EnableStdClog();
-  }
-}
-
 std::unique_ptr<BackgroundThreads> DefaultBackgroundThreads() {
   return google::cloud::internal::make_unique<
       AutomaticallyCreatedBackgroundThreads>();

google/cloud/connection_options.h

@@ -31,7 +31,6 @@ inline namespace GOOGLE_CLOUD_CPP_NS {
 namespace internal {
 std::set<std::string> DefaultTracingComponents();
 TracingOptions DefaultTracingOptions();
-void DefaultLogging();
 std::unique_ptr<BackgroundThreads> DefaultBackgroundThreads();
 }  // namespace internal
 
@@ -53,9 +52,7 @@ class ConnectionOptions {
         tracing_components_(internal::DefaultTracingComponents()),
         tracing_options_(internal::DefaultTracingOptions()),
         user_agent_prefix_(ConnectionTraits::user_agent_prefix()),
-        background_threads_factory_(internal::DefaultBackgroundThreads) {
-    internal::DefaultLogging();
-  }
+        background_threads_factory_(internal::DefaultBackgroundThreads) {}
 
   /// Change the gRPC credentials value.
   ConnectionOptions& set_credentials(

google/cloud/connection_options_test.cc

@@ -233,25 +233,6 @@ TEST(ConnectionOptionsTest, DefaultTracingOptionsWithValue) {
   EXPECT_EQ(42, actual.truncate_string_field_longer_than());
 }
 
-TEST(ConnectionOptionsTest, DefaultLoggingNoEnvironment) {
-  EnvironmentVariableRestore restore("GOOGLE_CLOUD_CPP_ENABLE_CLOG");
-  LogSink::Instance().ClearBackends();
-  internal::UnsetEnv("GOOGLE_CLOUD_CPP_ENABLE_CLOG");
-  internal::DefaultLogging();
-  EXPECT_EQ(0, LogSink::Instance().BackendCount());
-}
-
-TEST(ConnectionOptionsTest, DefaultLoggingWithValue) {
-  EnvironmentVariableRestore restore("GOOGLE_CLOUD_CPP_ENABLE_CLOG");
-  LogSink::Instance().ClearBackends();
-  EXPECT_EQ(0, LogSink::Instance().BackendCount());
-  internal::SetEnv("GOOGLE_CLOUD_CPP_ENABLE_CLOG", "any-value");
-  internal::DefaultLogging();
-  EXPECT_EQ(1, LogSink::Instance().BackendCount());
-  LogSink::DisableStdClog();
-  EXPECT_EQ(0, LogSink::Instance().BackendCount());
-}
-
 TEST(ConnectionOptionsTest, DefaultBackgroundThreads) {
   auto actual = internal::DefaultBackgroundThreads();
   EXPECT_TRUE(actual);

google/cloud/log.cc

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 #include "google/cloud/log.h"
+#include "google/cloud/internal/getenv.h"
 
 namespace google {
 namespace cloud {
@@ -45,8 +46,14 @@ LogSink::LogSink()
       clog_backend_id_(0) {}
 
 LogSink& LogSink::Instance() {
-  static LogSink instance;
-  return instance;
+  static auto* const kInstance = [] {
+    auto* p = new LogSink;
+    if (internal::GetEnv("GOOGLE_CLOUD_CPP_ENABLE_CLOG").has_value()) {
+      p->EnableStdClogImpl();
+    }
+    return p;
+  }();
+  return *kInstance;
 }
 
 long LogSink::AddBackend(std::shared_ptr<LogBackend> backend) {

google/cloud/log.h

@@ -57,6 +57,11 @@
  * }
  * @endcode
  *
+ * Alternatively, the application can enable logging to `std::clog` without any
+ * code changes or recompiling by setting the "GOOGLE_CLOUD_CPP_ENABLE_CLOG"
+ * environment variable before the program starts. The existence of this
+ * variable is all that matters; the value is ignored.
+ *
  * Note that while `std::clog` is buffered, the framework will flush any log
  * message at severity `WARNING` or higher.
  *
@@ -297,7 +302,12 @@ class LogSink {
 
   void Log(LogRecord log_record);
 
-  /// Enable `std::clog` on `LogSink::Instance()`.
+  /**
+   * Enable `std::clog` on `LogSink::Instance()`.
+   *
+   * This is also enabled if the "GOOGLE_CLOUD_CPP_ENABLE_CLOG" environment
+   * variable is set.
+   */
   static void EnableStdClog() { Instance().EnableStdClogImpl(); }
 
   /// Disable `std::clog` on `LogSink::Instance()`.

google/cloud/log_test.cc

@@ -13,14 +13,19 @@
 // limitations under the License.
 
 #include "google/cloud/log.h"
+#include "google/cloud/internal/setenv.h"
+#include "google/cloud/testing_util/environment_variable_restore.h"
 #include <gmock/gmock.h>
 
 namespace google {
 namespace cloud {
 inline namespace GOOGLE_CLOUD_CPP_NS {
 namespace {
 
-using namespace ::testing;
+using ::testing::_;
+using ::testing::ExitedWithCode;
+using ::testing::HasSubstr;
+using ::testing::Invoke;
 
 TEST(LogSeverityTest, Streaming) {
   std::ostringstream os;
@@ -140,6 +145,29 @@ TEST(LogSinkTest, ClogMultiple) {
   EXPECT_EQ(0, LogSink::Instance().BackendCount());
 }
 
+TEST(LogSinkTest, ClogEnvironment) {
+  // We set the death test style to "threadsafe", which causes the
+  // ASSERT_EXIT() call to re-exec the test binary before executing the given
+  // statement. This makes the death test thread-safe and it also ensures that
+  // the LogSink singleton instance will be reconstructed in the child and will
+  // see the environment variable. See also:
+  // https://github.com/google/googletest/blob/master/googletest/docs/advanced.md#death-test-styles
+  auto old_style = testing::FLAGS_gtest_death_test_style;
+  testing::FLAGS_gtest_death_test_style = "threadsafe";
+
+  testing_util::EnvironmentVariableRestore restore(
+      "GOOGLE_CLOUD_CPP_ENABLE_CLOG");
+  internal::SetEnv("GOOGLE_CLOUD_CPP_ENABLE_CLOG", "anyvalue");
+
+  auto f = [] {
+    GCP_LOG(INFO) << "testing clog";
+    std::exit(42);
+  };
+  ASSERT_EXIT(f(), ExitedWithCode(42), HasSubstr("testing clog"));
+
+  testing::FLAGS_gtest_death_test_style = old_style;
+}
+
 namespace {
 /// A class to count calls to IOStream operator.
 struct IOStreamCounter {