From fb0530eb37740fb7d042a018916ae460c3f67c60 Mon Sep 17 00:00:00 2001 From: Greg Miller <9447643+devjgm@users.noreply.github.com> Date: Thu, 20 Feb 2020 13:26:53 -0500 Subject: [PATCH] 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 | 7 ------ google/cloud/connection_options.h | 5 +---- google/cloud/connection_options_test.cc | 19 ---------------- google/cloud/log.cc | 11 +++++++-- google/cloud/log.h | 12 +++++++++- google/cloud/log_test.cc | 30 ++++++++++++++++++++++++- 6 files changed, 50 insertions(+), 34 deletions(-) diff --git a/google/cloud/connection_options.cc b/google/cloud/connection_options.cc index 76cac49c1a405..d6f41006672ab 100644 --- a/google/cloud/connection_options.cc +++ b/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 DefaultBackgroundThreads() { return google::cloud::internal::make_unique< AutomaticallyCreatedBackgroundThreads>(); diff --git a/google/cloud/connection_options.h b/google/cloud/connection_options.h index 90c2a4c5e1468..fe5a13d19eee3 100644 --- a/google/cloud/connection_options.h +++ b/google/cloud/connection_options.h @@ -31,7 +31,6 @@ inline namespace GOOGLE_CLOUD_CPP_NS { namespace internal { std::set DefaultTracingComponents(); TracingOptions DefaultTracingOptions(); -void DefaultLogging(); std::unique_ptr 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( diff --git a/google/cloud/connection_options_test.cc b/google/cloud/connection_options_test.cc index 539b3f8687b56..0540ce9a244a0 100644 --- a/google/cloud/connection_options_test.cc +++ b/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); diff --git a/google/cloud/log.cc b/google/cloud/log.cc index 620b794a22e0c..f4a034a65d573 100644 --- a/google/cloud/log.cc +++ b/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 backend) { diff --git a/google/cloud/log.h b/google/cloud/log.h index c6d36ae9ab703..c072a1521bd43 100644 --- a/google/cloud/log.h +++ b/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()`. diff --git a/google/cloud/log_test.cc b/google/cloud/log_test.cc index 987887aff563f..ea02c68ead621 100644 --- a/google/cloud/log_test.cc +++ b/google/cloud/log_test.cc @@ -13,6 +13,8 @@ // 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 namespace google { @@ -20,7 +22,10 @@ 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 {