Commit f3618360 by Andy Getzendanner Committed by Copybara-Service

Add an API to clear the saved LogBacktraceAt location, and call it when setting…

Add an API to clear the saved LogBacktraceAt location, and call it when setting an empty or invalid flag value.

PiperOrigin-RevId: 525065479
Change-Id: I3c0822db8301e0999b0669394b415df82edd445f
parent 387e1bf5
...@@ -90,19 +90,27 @@ ABSL_FLAG(std::string, log_backtrace_at, "", ...@@ -90,19 +90,27 @@ ABSL_FLAG(std::string, log_backtrace_at, "",
.OnUpdate([] { .OnUpdate([] {
const std::string log_backtrace_at = const std::string log_backtrace_at =
absl::GetFlag(FLAGS_log_backtrace_at); absl::GetFlag(FLAGS_log_backtrace_at);
if (log_backtrace_at.empty()) return; if (log_backtrace_at.empty()) {
absl::ClearLogBacktraceLocation();
return;
}
const size_t last_colon = log_backtrace_at.rfind(':'); const size_t last_colon = log_backtrace_at.rfind(':');
if (last_colon == log_backtrace_at.npos) return; if (last_colon == log_backtrace_at.npos) {
absl::ClearLogBacktraceLocation();
return;
}
const absl::string_view file = const absl::string_view file =
absl::string_view(log_backtrace_at).substr(0, last_colon); absl::string_view(log_backtrace_at).substr(0, last_colon);
int line; int line;
if (absl::SimpleAtoi( if (!absl::SimpleAtoi(
absl::string_view(log_backtrace_at).substr(last_colon + 1), absl::string_view(log_backtrace_at).substr(last_colon + 1),
&line)) { &line)) {
absl::SetLogBacktraceLocation(file, line); absl::ClearLogBacktraceLocation();
return;
} }
absl::SetLogBacktraceLocation(file, line);
}); });
ABSL_FLAG(bool, log_prefix, true, ABSL_FLAG(bool, log_prefix, true,
......
...@@ -92,23 +92,23 @@ TEST_F(LogFlagsTest, PrependLogPrefix) { ...@@ -92,23 +92,23 @@ TEST_F(LogFlagsTest, PrependLogPrefix) {
TEST_F(LogFlagsTest, EmptyBacktraceAtFlag) { TEST_F(LogFlagsTest, EmptyBacktraceAtFlag) {
absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo); absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo);
absl::SetFlag(&FLAGS_log_backtrace_at, "");
absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected); absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected);
EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:"))))); EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:")))));
test_sink.StartCapturingLogs(); test_sink.StartCapturingLogs();
absl::SetFlag(&FLAGS_log_backtrace_at, "");
LOG(INFO) << "hello world"; LOG(INFO) << "hello world";
} }
TEST_F(LogFlagsTest, BacktraceAtNonsense) { TEST_F(LogFlagsTest, BacktraceAtNonsense) {
absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo); absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo);
absl::SetFlag(&FLAGS_log_backtrace_at, "gibberish");
absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected); absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected);
EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:"))))); EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:")))));
test_sink.StartCapturingLogs(); test_sink.StartCapturingLogs();
absl::SetFlag(&FLAGS_log_backtrace_at, "gibberish");
LOG(INFO) << "hello world"; LOG(INFO) << "hello world";
} }
...@@ -116,13 +116,13 @@ TEST_F(LogFlagsTest, BacktraceAtWrongFile) { ...@@ -116,13 +116,13 @@ TEST_F(LogFlagsTest, BacktraceAtWrongFile) {
absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo); absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo);
const int log_line = __LINE__ + 1; const int log_line = __LINE__ + 1;
auto do_log = [] { LOG(INFO) << "hello world"; }; auto do_log = [] { LOG(INFO) << "hello world"; };
absl::SetFlag(&FLAGS_log_backtrace_at,
absl::StrCat("some_other_file.cc:", log_line));
absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected); absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected);
EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:"))))); EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:")))));
test_sink.StartCapturingLogs(); test_sink.StartCapturingLogs();
absl::SetFlag(&FLAGS_log_backtrace_at,
absl::StrCat("some_other_file.cc:", log_line));
do_log(); do_log();
} }
...@@ -130,13 +130,13 @@ TEST_F(LogFlagsTest, BacktraceAtWrongLine) { ...@@ -130,13 +130,13 @@ TEST_F(LogFlagsTest, BacktraceAtWrongLine) {
absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo); absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo);
const int log_line = __LINE__ + 1; const int log_line = __LINE__ + 1;
auto do_log = [] { LOG(INFO) << "hello world"; }; auto do_log = [] { LOG(INFO) << "hello world"; };
absl::SetFlag(&FLAGS_log_backtrace_at,
absl::StrCat("flags_test.cc:", log_line + 1));
absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected); absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected);
EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:"))))); EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:")))));
test_sink.StartCapturingLogs(); test_sink.StartCapturingLogs();
absl::SetFlag(&FLAGS_log_backtrace_at,
absl::StrCat("flags_test.cc:", log_line + 1));
do_log(); do_log();
} }
...@@ -144,12 +144,12 @@ TEST_F(LogFlagsTest, BacktraceAtWholeFilename) { ...@@ -144,12 +144,12 @@ TEST_F(LogFlagsTest, BacktraceAtWholeFilename) {
absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo); absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo);
const int log_line = __LINE__ + 1; const int log_line = __LINE__ + 1;
auto do_log = [] { LOG(INFO) << "hello world"; }; auto do_log = [] { LOG(INFO) << "hello world"; };
absl::SetFlag(&FLAGS_log_backtrace_at, absl::StrCat(__FILE__, ":", log_line));
absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected); absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected);
EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:"))))); EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:")))));
test_sink.StartCapturingLogs(); test_sink.StartCapturingLogs();
absl::SetFlag(&FLAGS_log_backtrace_at, absl::StrCat(__FILE__, ":", log_line));
do_log(); do_log();
} }
...@@ -157,13 +157,13 @@ TEST_F(LogFlagsTest, BacktraceAtNonmatchingSuffix) { ...@@ -157,13 +157,13 @@ TEST_F(LogFlagsTest, BacktraceAtNonmatchingSuffix) {
absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo); absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo);
const int log_line = __LINE__ + 1; const int log_line = __LINE__ + 1;
auto do_log = [] { LOG(INFO) << "hello world"; }; auto do_log = [] { LOG(INFO) << "hello world"; };
absl::SetFlag(&FLAGS_log_backtrace_at,
absl::StrCat("flags_test.cc:", log_line, "gibberish"));
absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected); absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected);
EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:"))))); EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:")))));
test_sink.StartCapturingLogs(); test_sink.StartCapturingLogs();
absl::SetFlag(&FLAGS_log_backtrace_at,
absl::StrCat("flags_test.cc:", log_line, "gibberish"));
do_log(); do_log();
} }
...@@ -171,13 +171,17 @@ TEST_F(LogFlagsTest, LogsBacktrace) { ...@@ -171,13 +171,17 @@ TEST_F(LogFlagsTest, LogsBacktrace) {
absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo); absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo);
const int log_line = __LINE__ + 1; const int log_line = __LINE__ + 1;
auto do_log = [] { LOG(INFO) << "hello world"; }; auto do_log = [] { LOG(INFO) << "hello world"; };
absl::SetFlag(&FLAGS_log_backtrace_at,
absl::StrCat("flags_test.cc:", log_line));
absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected); absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected);
testing::InSequence seq;
EXPECT_CALL(test_sink, Send(TextMessage(HasSubstr("(stacktrace:")))); EXPECT_CALL(test_sink, Send(TextMessage(HasSubstr("(stacktrace:"))));
EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:")))));
test_sink.StartCapturingLogs(); test_sink.StartCapturingLogs();
absl::SetFlag(&FLAGS_log_backtrace_at,
absl::StrCat("flags_test.cc:", log_line));
do_log();
absl::SetFlag(&FLAGS_log_backtrace_at, "");
do_log(); do_log();
} }
......
...@@ -123,7 +123,7 @@ namespace log_internal { ...@@ -123,7 +123,7 @@ namespace log_internal {
bool ShouldLogBacktraceAt(absl::string_view file, int line) { bool ShouldLogBacktraceAt(absl::string_view file, int line) {
const size_t flag_hash = const size_t flag_hash =
log_backtrace_at_hash.load(std::memory_order_acquire); log_backtrace_at_hash.load(std::memory_order_relaxed);
return flag_hash != 0 && flag_hash == HashSiteForLogBacktraceAt(file, line); return flag_hash != 0 && flag_hash == HashSiteForLogBacktraceAt(file, line);
} }
...@@ -132,7 +132,11 @@ bool ShouldLogBacktraceAt(absl::string_view file, int line) { ...@@ -132,7 +132,11 @@ bool ShouldLogBacktraceAt(absl::string_view file, int line) {
void SetLogBacktraceLocation(absl::string_view file, int line) { void SetLogBacktraceLocation(absl::string_view file, int line) {
log_backtrace_at_hash.store(HashSiteForLogBacktraceAt(file, line), log_backtrace_at_hash.store(HashSiteForLogBacktraceAt(file, line),
std::memory_order_release); std::memory_order_relaxed);
}
void ClearLogBacktraceLocation() {
log_backtrace_at_hash.store(0, std::memory_order_relaxed);
} }
bool ShouldPrependLogPrefix() { bool ShouldPrependLogPrefix() {
......
...@@ -110,8 +110,8 @@ class ScopedStderrThreshold final { ...@@ -110,8 +110,8 @@ class ScopedStderrThreshold final {
// Log Backtrace At // Log Backtrace At
//------------------------------------------------------------------------------ //------------------------------------------------------------------------------
// //
// Users can request backtrace to be logged at specific locations, specified // Users can request an existing `LOG` statement, specified by file and line
// by file and line number. // number, to also include a backtrace when logged.
// ShouldLogBacktraceAt() // ShouldLogBacktraceAt()
// //
...@@ -123,9 +123,16 @@ ABSL_MUST_USE_RESULT bool ShouldLogBacktraceAt(absl::string_view file, ...@@ -123,9 +123,16 @@ ABSL_MUST_USE_RESULT bool ShouldLogBacktraceAt(absl::string_view file,
// SetLogBacktraceLocation() // SetLogBacktraceLocation()
// //
// Sets the location the backtrace should be logged at. // Sets the location the backtrace should be logged at. If the specified
// location isn't a `LOG` statement, the effect will be the same as
// `ClearLogBacktraceLocation` (but less efficient).
void SetLogBacktraceLocation(absl::string_view file, int line); void SetLogBacktraceLocation(absl::string_view file, int line);
// ClearLogBacktraceLocation()
//
// Clears the set location so that backtraces will no longer be logged at it.
void ClearLogBacktraceLocation();
//------------------------------------------------------------------------------ //------------------------------------------------------------------------------
// Prepend Log Prefix // Prepend Log Prefix
//------------------------------------------------------------------------------ //------------------------------------------------------------------------------
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment