Skip to content

Commit

Permalink
Added persistent setting for logtrace killswitch. (#3005)
Browse files Browse the repository at this point in the history 
Added persistent setting for logtrace killswitch. When kPersistentSettingLogtraceEnable = false, then all logtrace methods are noop.

b/327680765

Change-Id: I78fecc80d64c8fe1b3d1a9caf8fd89608471ec14
  • Loading branch information
@alunev
alunev authored Apr 22, 2024
1 parent 3ef72cd commit 77b6385
Show file tree
Hide file tree
Showing 3 changed files with 148 additions and 15 deletions.
47 changes: 33 additions & 14 deletions cobalt/watchdog/watchdog.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,19 +53,6 @@ const int kWatchdogMaxPingInfoLength = 1024;
// The maximum number of milliseconds old of an unfetched Watchdog violation.
const int64_t kWatchdogMaxViolationsAge = 86400000;

// Persistent setting name and default setting for the boolean that controls
// whether or not Watchdog is enabled. When disabled, Watchdog behaves like a
// stub except that persistent settings can still be get/set. Requires a
// restart to take effect.
const char kPersistentSettingWatchdogEnable[] =
"kPersistentSettingWatchdogEnable";
const bool kDefaultSettingWatchdogEnable = true;
// Persistent setting name and default setting for the boolean that controls
// whether or not a Watchdog violation will trigger a crash.
const char kPersistentSettingWatchdogCrash[] =
"kPersistentSettingWatchdogCrash";
const bool kDefaultSettingWatchdogCrash = false;

} // namespace

bool Watchdog::Initialize(
Expand All @@ -80,6 +67,7 @@ bool Watchdog::InitializeCustom(
std::string watchdog_file_name, int64_t watchdog_monitor_frequency) {
persistent_settings_ = persistent_settings;
is_disabled_ = !GetPersistentSettingWatchdogEnable();
is_logtrace_disabled_ = !GetPersistentSettingLogtraceEnable();

if (is_disabled_) return true;

Expand Down Expand Up @@ -805,14 +793,45 @@ void Watchdog::SetPersistentSettingWatchdogCrash(bool can_trigger_crash) {
}

bool Watchdog::LogEvent(const std::string& event) {
if (is_logtrace_disabled_) {
return true;
}

return instrumentation_log_.LogEvent(event);
}

std::vector<std::string> Watchdog::GetLogTrace() {
if (is_logtrace_disabled_) {
return {};
}

return instrumentation_log_.GetLogTrace();
}

void Watchdog::ClearLog() { instrumentation_log_.ClearLog(); }
void Watchdog::ClearLog() {
if (is_logtrace_disabled_) {
return;
}

instrumentation_log_.ClearLog();
}

bool Watchdog::GetPersistentSettingLogtraceEnable() {
if (!persistent_settings_) return kDefaultSettingLogtraceEnable;

// Gets the boolean that controls whether or not LogTrace is enabled.
return persistent_settings_->GetPersistentSettingAsBool(
kPersistentSettingLogtraceEnable, kDefaultSettingLogtraceEnable);
}

void Watchdog::SetPersistentSettingLogtraceEnable(bool enable_logtrace) {
if (!persistent_settings_) return;

// Sets the boolean that controls whether or not LogTrace is enabled.
persistent_settings_->SetPersistentSetting(
kPersistentSettingLogtraceEnable,
std::make_unique<base::Value>(enable_logtrace));
}

#if defined(_DEBUG)
// Sleeps threads for Watchdog debugging.
Expand Down
27 changes: 27 additions & 0 deletions cobalt/watchdog/watchdog.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,28 @@
namespace cobalt {
namespace watchdog {

// Persistent setting name and default setting for the boolean that controls
// whether or not Watchdog is enabled. When disabled, Watchdog behaves like a
// stub except that persistent settings can still be get/set. Requires a
// restart to take effect.
constexpr char kPersistentSettingWatchdogEnable[] =
"kPersistentSettingWatchdogEnable";
constexpr bool kDefaultSettingWatchdogEnable = true;

// Persistent setting name and default setting for the boolean that controls
// whether or not a Watchdog violation will trigger a crash.
constexpr char kPersistentSettingWatchdogCrash[] =
"kPersistentSettingWatchdogCrash";
constexpr bool kDefaultSettingWatchdogCrash = false;

// Persistent setting name and default setting for the boolean that controls
// whether or not LogTrace API is enabled. When disabled, all LogTrace methods
// behave like a stub except for persistent settings itself. Requires a
// restart to take effect.
constexpr char kPersistentSettingLogtraceEnable[] =
"kPersistentSettingLogtraceEnable";
constexpr bool kDefaultSettingLogtraceEnable = true;

// Client to monitor
typedef struct Client {
std::string name;
Expand Down Expand Up @@ -112,6 +134,8 @@ class Watchdog : public Singleton<Watchdog> {
bool LogEvent(const std::string& event);
std::vector<std::string> GetLogTrace();
void ClearLog();
bool GetPersistentSettingLogtraceEnable();
void SetPersistentSettingLogtraceEnable(bool enable_logtrace);

#if defined(_DEBUG)
// Sleeps threads based off of environment variables for Watchdog debugging.
Expand Down Expand Up @@ -181,6 +205,9 @@ class Watchdog : public Singleton<Watchdog> {
int64_t watchdog_monitor_frequency_;
// Captures string events emitted from Kabuki via logEvent() h5vcc API.
InstrumentationLog instrumentation_log_;
// Flag to disable LogTrace API. When disabled, all LogTrace methods behave
// like a stub except that the flag itself can still be get/set.
bool is_logtrace_disabled_;

#if defined(_DEBUG)
starboard::Mutex delay_mutex_;
Expand Down
89 changes: 88 additions & 1 deletion cobalt/watchdog/watchdog_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,15 @@

#include "base/json/json_reader.h"
#include "base/json/json_writer.h"
#include "base/test/task_environment.h"
#include "starboard/common/file.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace cobalt {
namespace watchdog {

using persistent_storage::PersistentSettings;

namespace {

const char kWatchdogViolationsJson[] = "watchdog_test.json";
Expand All @@ -38,7 +41,9 @@ const int64_t kWatchdogSleepDuration = kWatchdogMonitorFrequency * 4;

class WatchdogTest : public testing::Test {
protected:
WatchdogTest() {}
WatchdogTest()
: task_environment_(
base::test::TaskEnvironment::MainThreadType::DEFAULT) {}

void SetUp() final {
watchdog_ = new watchdog::Watchdog();
Expand All @@ -51,6 +56,8 @@ class WatchdogTest : public testing::Test {
watchdog_->Uninitialize();
delete watchdog_;
watchdog_ = nullptr;

DeletePersistentSettingsFile();
}

base::Value CreateDummyViolationDict(std::string desc, int begin, int end) {
Expand Down Expand Up @@ -82,7 +89,23 @@ class WatchdogTest : public testing::Test {
return violation.Clone();
}

void DeletePersistentSettingsFile() {
std::vector<char> storage_dir(kSbFileMaxPath + 1, 0);
SbSystemGetPath(kSbSystemPathCacheDirectory, storage_dir.data(),
kSbFileMaxPath);
std::string path =
std::string(storage_dir.data()) + kSbFileSepString + kSettingsFileName;

starboard::SbFileDeleteRecursive(path.c_str(), true);
}

const std::string kSettingsFileName = "test-settings.json";

watchdog::Watchdog* watchdog_;
base::test::TaskEnvironment task_environment_;
base::WaitableEvent task_done_ = {
base::WaitableEvent::ResetPolicy::MANUAL,
base::WaitableEvent::InitialState::NOT_SIGNALED};
};

TEST_F(WatchdogTest, RedundantRegistersShouldFail) {
Expand Down Expand Up @@ -719,5 +742,69 @@ TEST_F(WatchdogTest, ViolationContainsEmptyLogTrace) {
ASSERT_EQ(logTrace->size(), 0);
}

TEST_F(WatchdogTest, WatchdogMethodsAreNoopWhenWatchdogIsDisabled) {
// init and destroy existing watchdog to re-initialize it later
watchdog_->Register("test-name", "test-desc", base::kApplicationStateStarted,
kWatchdogMonitorFrequency);
TearDown();

// PersistentSettings doesn't have interface so it's not mockable
auto persistent_settings =
std::make_unique<PersistentSettings>(kSettingsFileName);
persistent_settings->ValidatePersistentSettings();

base::OnceClosure closure = base::BindOnce(
[](base::WaitableEvent* task_done) { task_done->Signal(); }, &task_done_);
persistent_settings->SetPersistentSetting(
kPersistentSettingWatchdogEnable, std::make_unique<base::Value>(false),
std::move(closure), true);
task_done_.Wait();

watchdog_ = new watchdog::Watchdog();
watchdog_->InitializeCustom(persistent_settings.get(),
std::string(kWatchdogViolationsJson),
kWatchdogMonitorFrequency);

ASSERT_TRUE(watchdog_->Register("test-name", "test-desc",
base::kApplicationStateStarted,
kWatchdogMonitorFrequency));
ASSERT_TRUE(watchdog_->Ping("test-name"));
ASSERT_TRUE(watchdog_->PingByClient(nullptr));

usleep(kWatchdogSleepDuration);

ASSERT_EQ(watchdog_->GetWatchdogViolations(), "");
ASSERT_TRUE(watchdog_->Unregister("test-name"));
ASSERT_TRUE(watchdog_->Unregister(""));
}

TEST_F(WatchdogTest, LogtraceMethodsAreNoopWhenLogtraceIsDisabled) {
// init and destroy existing watchdog to re-initialize it later
watchdog_->Register("test-name", "test-desc", base::kApplicationStateStarted,
kWatchdogMonitorFrequency);
TearDown();

// PersistentSettings doesn't have interface so it's not mockable
auto persistent_settings =
std::make_unique<PersistentSettings>(kSettingsFileName);
persistent_settings->ValidatePersistentSettings();

base::OnceClosure closure = base::BindOnce(
[](base::WaitableEvent* task_done) { task_done->Signal(); }, &task_done_);
persistent_settings->SetPersistentSetting(
kPersistentSettingLogtraceEnable, std::make_unique<base::Value>(false),
std::move(closure), true);
task_done_.Wait();

watchdog_ = new watchdog::Watchdog();
watchdog_->InitializeCustom(persistent_settings.get(),
std::string(kWatchdogViolationsJson),
kWatchdogMonitorFrequency);

ASSERT_TRUE(watchdog_->LogEvent("foo"));
ASSERT_EQ(watchdog_->GetLogTrace().size(), 0);
ASSERT_NO_FATAL_FAILURE(watchdog_->ClearLog());
}

} // namespace watchdog
} // namespace cobalt

0 comments on commit 77b6385

Please sign in to comment.