Skip to content

[Util] tinyformat / LogPrint backports #1449

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 7 commits into from
Mar 27, 2020
Merged

[Util] tinyformat / LogPrint backports #1449

merged 7 commits into from
Mar 27, 2020

Conversation

random-zebra
Copy link

this backports the following pull requests from upstream bitcoin:

Now that we started using c++11, force use of variadic templates.
The autodetection may be wonky on some compilers, see discussion here and is unnecessary for us anyhow.

Updates tinyformat.h to commit c42f/tinyformat@3a33bbf upstream.

There are a few cases where hashes are computed inside LogPrint arguments - where they usually go unused. As LogPrint statements should never have side effects besides printing something, we can avoid the evaluation in this case.
Advantage: perhaps a small performance improvement; I haven't benchmarked.
Disadvantage: if we would have statements with side-effects, this could make this a bit harder to debug.

Instead of having an exception propagate into the program (which at worst causes a crash) when an error happens while formatting a log message, just print a message to the log. This message clearly indicates what log message was formatted wrongly, and what error happened during formatting it.

@random-zebra random-zebra added this to the 4.1.0 milestone Mar 23, 2020
@random-zebra random-zebra self-assigned this Mar 23, 2020
This was referenced Mar 23, 2020
Merged
Merged
Merged
Merged
Merged
Merged
Fuzzbawls
Fuzzbawls approved these changes Mar 26, 2020
View reviewed changes
Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 9fb0a43

@random-zebra random-zebra requested a review from furszy March 26, 2020 10:50
furszy
furszy approved these changes Mar 26, 2020
View reviewed changes
Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool, utACK 9fb0a43 .

@furszy furszy merged commit 9ef9f5d into PIVX-Project:master Mar 27, 2020
akshaynexus pushed a commit to ZENZO-Ecosystem/ZENZO-Core that referenced this pull request Mar 30, 2020
@furszy @akshaynexus
Merge PIVX-Project#1449: [Util] tinyformat / LogPrint backports
5c753ce 
9fb0a43 util: Throw tinyformat::format_error on formatting error (random-zebra)
67eb699 util: Properly handle errors during log message formatting (random-zebra)
66ec97b Do not evaluate hidden LogPrint arguments (random-zebra)
2713458 util: Remove zero-argument versions of LogPrint and error (random-zebra)
500dfee util: Update tinyformat (random-zebra)
6837887 util: switch LogPrint and error to variadic templates (random-zebra)
0fa578f tinyformat: force USE_VARIADIC_TEMPLATES (random-zebra)

Pull request description:

  this backports the following pull requests from upstream bitcoin:
  - bitcoin#8000 (0fa578f, 6837887)
  > Now that we started using c++11, force use of variadic templates.
  The autodetection may be wonky on some compilers, see discussion here and is unnecessary for us anyhow.

  - bitcoin#8274 (500dfee, 2713458)
  > Updates tinyformat.h to commit c42f/tinyformat@3a33bbf upstream.

  - bitcoin#9417 (66ec97b)
  > There are a few cases where hashes are computed inside LogPrint arguments - where they usually go unused. As LogPrint statements should never have side effects besides printing something, we can avoid the evaluation in this case.
  Advantage: perhaps a small performance improvement; I haven't benchmarked.
  Disadvantage: if we would have statements with side-effects, this could make this a bit harder to debug.

  - bitcoin#9963 (67eb699, 9fb0a43)
  > Instead of having an exception propagate into the program (which at worst causes a crash) when an error happens while formatting a log message, just print a message to the log. This message clearly indicates what log message was formatted wrongly, and what error happened during formatting it.

ACKs for top commit:
  Fuzzbawls:
    ACK 9fb0a43
  furszy:
    cool, utACK 9fb0a43 .

Tree-SHA512: 29a902bb712612ca093a8fe863e9eff1c17d40955bbc37a5ceb6f057068ac6150dfcffd6836a6b0bd6295aa9bffd0925eb69e50f82ccaa7d84215efc274a59a4
furszy added a commit that referenced this pull request Apr 1, 2020
@furszy
Merge #1437: [Util] LogAcceptCategory: use uint32_t rather than sets …
8a6f514 
…of strings

c3dc816 [Core] LogAcceptCategory: use uint32_t rather than sets of strings (random-zebra)

Pull request description:

  implemented on top of:
  - [x] #1449

  from bitcoin#9424

  > This changes the logging categories to boolean flags instead of strings.
  This simplifies the acceptance testing by avoiding accessing a scoped static thread local pointer to a thread local set of strings. It eliminates the only use of boost::thread_specific_ptr outside of lockorder debugging.
  This change allows log entries to be directed to multiple categories and makes it easy to change the logging flags at runtime (e.g. via an RPC, though that isn't done by this commit.)
  It also eliminates the fDebug global.
  Configuration of unknown logging categories now produces a warning.

  We add 4 more debug categories:
  - BCLog::STAKING
  - BCLog::MASTERNODE (which includes previous "masternode" and "mnpayments" categories)
  - BCLog::MNBUDGET
  - BCLog::LEGACYZC

ACKs for top commit:
  Fuzzbawls:
    ACK c3dc816

Tree-SHA512: 0c2ffc30df5b4239396c6f2ef6c551bbca5696aa1087b5e12a206fc9b1def9ef6cd9c6f6c0cbbb8da0d667e782848a884514de21609714668d0b5ad87eca8e56
random-zebra added a commit that referenced this pull request Apr 4, 2020
@random-zebra
Merge #1439: [Core] Add -debugexclude option
eb1ed66 
0a85445 [Core] Add -debugexclude option (random-zebra)

Pull request description:

  Implemented on top of:
  - [x] #1449
  - [x] #1437

  Backports bitcoin#10123

  > setting `-debug` can lead to very noisy debug.logs with subcomponents filling up the log file. See for example https://travis-ci.org/bitcoin/bitcoin/jobs/216767286 where there are hundreds of libevent debug logs.

  This commit adds an option to exclude certain components from debug logging. The usage is:
  ```
  pivxd -debug -debugexclude=<component1> -debugexclude=<component2> ...
  ```

  This finally reduces spammy logs in the functional tests debug, due to libevent and leveldb (bitcoin#10124 was already included backporting the test suite).

ACKs for top commit:
  furszy:
    Code looking good, utACK 0a85445 .
  Fuzzbawls:
    ACK 0a85445

Tree-SHA512: 6ccbe275ab11f3e1739ed117e49db0a2e56db345287cddc7cc18fc1131d7800d0f08a7e93005f70cf45ed8e340580f91c3017532bd2ea28fedb5735409ae068b
Fuzzbawls added a commit that referenced this pull request Apr 6, 2020
@Fuzzbawls
Merge #1450: [Util] Buffer log messages and explicitly open logs
cb6d15a 
d16b7b2 Buffer log messages and explicitly open logs (random-zebra)

Pull request description:

  implemented on top of
  - [x] #1449
  - [x] #1437
  - [x] #1439

  Backports bitcoin#6149

  > Prevents stomping on debug logs in datadirs that are locked by other
  instances and lost parameter interaction messages that can get wiped by
  ShrinkDebugFile().
  The log is now opened explicitly and all emitted messages are buffered
  until this open occurs.  The version message and log cut have also been
  moved to the earliest possible sensible location.

ACKs for top commit:
  furszy:
    utACK d16b7b2
  Fuzzbawls:
    ACK d16b7b2

Tree-SHA512: f3f859181499661641df1ccf118fdb583517c3f4104313df4c200471436e2c456bd9d15164215cfde274b235afdd2afe19a186c214d3c06461f0e3a03bd944b8
random-zebra added a commit that referenced this pull request Apr 6, 2020
@random-zebra
Merge #1451: [RPC] Add logging RPC
1fa5156 
f66e50e [Doc] Add logging RPC to release notes (random-zebra)
bf55911 allow libevent logging to be updated during runtime (random-zebra)
ec43b51 Set BCLog::LIBEVENT correctly for old libevent versions. (random-zebra)
cbaf724 [rpc] Add logging rpc (random-zebra)

Pull request description:

  Implemented on top of:
  - [x] #1449
  - [x] #1437
  - [x] #1439
  - [x] #1450

  Backports bitcoin#10150

  > Adds an RPC to get/set active logging categories.
  First commit allows all categories except libevent to be reconfigured during runtime.
  Second commit modifies InitHTTPServer() to allow leveldb logging to be reconfigured during runtime.

ACKs for top commit:
  furszy:
    Really nice 👍 , ACK f66e50e .
  Fuzzbawls:
    ACK f66e50e

Tree-SHA512: 7b735628d341fb8661eb76f57dadc78e69ea63c62edb778450b3d9e3b7137ce06e08ceab835967b923401a9687e5b2605722a9f256e6ed85fc372a0e46086b8f
random-zebra added a commit that referenced this pull request Apr 9, 2020
@random-zebra
Merge #1455: [Init] Add -debuglogfile option
b26dbc4 
882ef90 [Doc] Update release notes for `-debuglogfile` (random-zebra)
3d5ad7f test: Add tests for `-debuglogfile` with subdirs (random-zebra)
2c2e36d test: Add test for `-debuglogfile` (random-zebra)
b44a324 Add `-debuglogfile` option (random-zebra)

Pull request description:

  Implemented on top of
  - [x] #1449
  - [x] #1437
  - [x] #1439
  - [x] #1450
  - [x] #1451

  Only last 4 commits are relevant to this PR.
  Backports bitcoin#11781

  > This patch adds an option to configure the name and/or directory of the debug log file.</br></br>
  The user can specify either a relative path, in which case the path is relative to the (network specific) data directory. They can also specify an absolute path to put the log anywhere else in the file system.

  Functional test included.

ACKs for top commit:
  Fuzzbawls:
    ACK 882ef90
  furszy:
    utACK 882ef90

Tree-SHA512: 02a0e6487683e5111765af7296ef7ce035372febf037268d99d29b4c4a2f74bcc40f46a0f5b1bacddc2249c2a7e40255555e83ca9d51bf71d9e054c6e85765cc
Fuzzbawls added a commit that referenced this pull request Apr 14, 2020
@Fuzzbawls
Merge #1459: [Util] Refactor logging code into a global object
c94f241 
5c8e968 [Trivial] Document logtimemicros flag in the help (random-zebra)
4daa10a util: Store debug log file path in BCLog::Logger member. (random-zebra)
2f03e85 scripted-diff: Rename BCLog::Logger member variables. (random-zebra)
303700e util: Refactor GetLogCategory. (random-zebra)
0ae18c0 util: Encapsulate logCategories within BCLog::Logger. (random-zebra)
a2fb3fd util: Move debug file management functions into Logger. (random-zebra)
5a42d82 util: Establish global logger object. (random-zebra)
15c0da4 [Refactor] Complete boost::filesystem namespace in util (random-zebra)
81ddbf4 MOVEONLY: Move logging code from util.{h,cpp} to new files. (random-zebra)

Pull request description:

  Implemented on top of:
  - [x] #1449
  - [x] #1437
  - [x] #1439
  - [x] #1450
  - [x] #1451
  - [x] #1455

  This creates a new class BCLog::Logger to encapsulate all global logging configuration and state.

  Adapted from
  - bitcoin#13021
  - bitcoin#12954

ACKs for top commit:
  Fuzzbawls:
    ACK 5c8e968
  furszy:
    utACK 5c8e968

Tree-SHA512: 0b10a031dd7e32b48485236fbdd8249d011049e6f99e1df145b7dea4cab9e6e67e19d1bb13ff48e99eb2487a8399bbb8298fe851ad8873416fc1053aee0379bc
@random-zebra random-zebra deleted the 2020_tinyformat_backports branch September 24, 2020 00:27
@lyricidal lyricidal mentioned this pull request Sep 28, 2021
Merged
This was referenced Sep 28, 2021
Merged
Merged
Merged
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@furszy furszy furszy approved these changes

@Fuzzbawls Fuzzbawls Fuzzbawls approved these changes

Assignees

@random-zebra random-zebra

Labels
Upstream Util / log / libs
Projects
None yet
Milestone
4.1.0
Development

Successfully merging this pull request may close these issues.
3 participants
@random-zebra @furszy @Fuzzbawls