From 45b9141f77629f19bbc3988eb22bcb4af806aa95 Mon Sep 17 00:00:00 2001 From: zenparsing Date: Wed, 21 Apr 2021 15:23:12 -0400 Subject: [PATCH] Log Rewards errors by default and add a feature flag for verbose logging --- chromium_src/chrome/browser/about_flags.cc | 4 +++ .../chrome/browser/flag_descriptions.cc | 4 +++ .../chrome/browser/flag_descriptions.h | 2 ++ .../browser/rewards_service_impl.cc | 22 ++++---------- .../browser/rewards_service_impl.h | 2 +- components/brave_rewards/common/features.cc | 3 ++ components/brave_rewards/common/features.h | 1 + .../ledger/internal/logging/logging_util.cc | 29 +++++++++---------- 8 files changed, 35 insertions(+), 32 deletions(-) diff --git a/chromium_src/chrome/browser/about_flags.cc b/chromium_src/chrome/browser/about_flags.cc index 92654a5d5ab4..563451552350 100644 --- a/chromium_src/chrome/browser/about_flags.cc +++ b/chromium_src/chrome/browser/about_flags.cc @@ -174,6 +174,10 @@ using ntp_background_images::features::kBraveNTPSuperReferralWallpaper; flag_descriptions::kBravePermissionLifetimeName, \ flag_descriptions::kBravePermissionLifetimeDescription, kOsAll, \ FEATURE_VALUE_TYPE(permissions::features::kPermissionLifetime)}, \ + {"brave-rewards-verbose-logging", \ + flag_descriptions::kBraveRewardsVerboseLoggingName, \ + flag_descriptions::kBraveRewardsVerboseLoggingDescription, kOsDesktop, \ + FEATURE_VALUE_TYPE(brave_rewards::features::kVerboseLoggingFeature)}, \ {"brave-rewards-bitflyer", \ flag_descriptions::kBraveRewardsBitflyerName, \ flag_descriptions::kBraveRewardsBitflyerDescription, kOsDesktop, \ diff --git a/chromium_src/chrome/browser/flag_descriptions.cc b/chromium_src/chrome/browser/flag_descriptions.cc index a45a9b0a98cb..5f3a30058a32 100644 --- a/chromium_src/chrome/browser/flag_descriptions.cc +++ b/chromium_src/chrome/browser/flag_descriptions.cc @@ -57,6 +57,10 @@ const char kBraveSyncDescription[] = const char kBraveIpfsName[] = "Enable IPFS"; const char kBraveIpfsDescription[] = "Enable native support of IPFS."; +const char kBraveRewardsVerboseLoggingName[] = + "Enable Brave Rewards verbose logging"; +const char kBraveRewardsVerboseLoggingDescription[] = + "Enables detailed logging of Brave Rewards system events to a log file"; const char kBraveRewardsBitflyerName[] = "Enable bitFlyer for Brave Rewards"; const char kBraveRewardsBitflyerDescription[] = "Enables support for bitFlyer as an external wallet provider for Brave " diff --git a/chromium_src/chrome/browser/flag_descriptions.h b/chromium_src/chrome/browser/flag_descriptions.h index 0d5b67109c03..c9a8a3059e49 100644 --- a/chromium_src/chrome/browser/flag_descriptions.h +++ b/chromium_src/chrome/browser/flag_descriptions.h @@ -35,6 +35,8 @@ extern const char kBraveSyncName[]; extern const char kBraveSyncDescription[]; extern const char kBraveIpfsName[]; extern const char kBraveIpfsDescription[]; +extern const char kBraveRewardsVerboseLoggingName[]; +extern const char kBraveRewardsVerboseLoggingDescription[]; extern const char kBraveRewardsBitflyerName[]; extern const char kBraveRewardsBitflyerDescription[]; extern const char kNativeBraveWalletName[]; diff --git a/components/brave_rewards/browser/rewards_service_impl.cc b/components/brave_rewards/browser/rewards_service_impl.cc index 71e4efa45151..12480c6f60d9 100644 --- a/components/brave_rewards/browser/rewards_service_impl.cc +++ b/components/brave_rewards/browser/rewards_service_impl.cc @@ -334,6 +334,9 @@ RewardsServiceImpl::RewardsServiceImpl(Profile* profile) content::URLDataSource::Add(profile_, std::make_unique(profile_)); ready_ = std::make_unique(); + + if (base::FeatureList::IsEnabled(features::kVerboseLoggingFeature)) + persist_log_level_ = kDiagnosticLogMaxVerboseLevel; } RewardsServiceImpl::~RewardsServiceImpl() { @@ -2263,15 +2266,12 @@ void RewardsServiceImpl::WriteDiagnosticLog(const std::string& file, const int line, const int verbose_level, const std::string& message) { - if (ledger_for_testing_ || !should_persist_logs_) { + if (ledger_for_testing_ || resetting_rewards_) { return; } - if (resetting_rewards_) { - return; - } - - if (verbose_level > kDiagnosticLogMaxVerboseLevel) { + if (verbose_level > kDiagnosticLogMaxVerboseLevel || + verbose_level > persist_log_level_) { return; } @@ -2414,16 +2414,6 @@ void RewardsServiceImpl::HandleFlags(const std::string& options) { continue; } - if (name == "persist-logs") { - const std::string lower = base::ToLowerASCII(value); - - if (lower == "true" || lower == "1") { - should_persist_logs_ = true; - } else { - should_persist_logs_ = false; - } - } - if (name == "countryid") { int country_id; if (base::StringToInt(value, &country_id)) { diff --git a/components/brave_rewards/browser/rewards_service_impl.h b/components/brave_rewards/browser/rewards_service_impl.h index 871d6823c3d6..049ae938876f 100644 --- a/components/brave_rewards/browser/rewards_service_impl.h +++ b/components/brave_rewards/browser/rewards_service_impl.h @@ -790,7 +790,7 @@ class RewardsServiceImpl : public RewardsService, bool reset_states_; bool ledger_for_testing_ = false; bool resetting_rewards_ = false; - bool should_persist_logs_ = false; + int persist_log_level_ = 0; GetTestResponseCallback test_response_callback_; diff --git a/components/brave_rewards/common/features.cc b/components/brave_rewards/common/features.cc index 07f5e1884e5e..3232fdda2643 100644 --- a/components/brave_rewards/common/features.cc +++ b/components/brave_rewards/common/features.cc @@ -13,5 +13,8 @@ namespace features { const base::Feature kBitflyerFeature{"BraveRewardsBitflyer", base::FEATURE_DISABLED_BY_DEFAULT}; +const base::Feature kVerboseLoggingFeature{"BraveRewardsVerboseLogging", + base::FEATURE_DISABLED_BY_DEFAULT}; + } // namespace features } // namespace brave_rewards diff --git a/components/brave_rewards/common/features.h b/components/brave_rewards/common/features.h index 674c6f8e5025..8b131f3f4b39 100644 --- a/components/brave_rewards/common/features.h +++ b/components/brave_rewards/common/features.h @@ -14,6 +14,7 @@ namespace brave_rewards { namespace features { extern const base::Feature kBitflyerFeature; +extern const base::Feature kVerboseLoggingFeature; } // namespace features } // namespace brave_rewards diff --git a/vendor/bat-native-ledger/src/bat/ledger/internal/logging/logging_util.cc b/vendor/bat-native-ledger/src/bat/ledger/internal/logging/logging_util.cc index 7394282f9418..88bcd38b41e3 100644 --- a/vendor/bat-native-ledger/src/bat/ledger/internal/logging/logging_util.cc +++ b/vendor/bat-native-ledger/src/bat/ledger/internal/logging/logging_util.cc @@ -10,6 +10,7 @@ #include "base/strings/string_util.h" #include "base/strings/stringprintf.h" #include "bat/ledger/internal/logging/logging.h" +#include "url/gurl.h" namespace ledger { @@ -99,12 +100,15 @@ void LogUrlResponse( const char* func, const type::UrlResponse& response, const bool long_response) { + bool is_error = false; std::string result; if (!response.error.empty()) { + is_error = true; result = "Error (" + response.error + ")"; } else if (response.status_code >= 200 && response.status_code < 300) { result = "Success"; } else { + is_error = true; result = "Failure"; } @@ -115,25 +119,20 @@ void LogUrlResponse( } const std::string response_basic = base::StringPrintf( - "\n[ RESPONSE - %s ]\n" - "> Url: %s\n" + "\n[ RESPONSE ]\n" + "> URL: %s\n" "> Result: %s\n" - "> HTTP Code: %d\n" - "> Body: %s", - func, - response.url.c_str(), - result.c_str(), - response.status_code, - response.body.c_str()); + "> HTTP Code: %d", + response.url.c_str(), result.c_str(), response.status_code); + + const std::string response_body = + base::StringPrintf("\n[ RESPONSE BODY ]\n%s", response.body.c_str()); const std::string response_headers = base::StringPrintf( - "\n[ RESPONSE HEADERS ]\n" - "> Url: %s\n" - "%s", - response.url.c_str(), - formatted_headers.c_str()); + "\n[ RESPONSE HEADERS ]\n%s", formatted_headers.c_str()); - BLOG(long_response ? 7 : 6, response_basic); + BLOG(is_error ? 0 : 6, response_basic); + BLOG(long_response ? 7 : 6, response_body); BLOG(9, response_headers); }