Add configurable logging to data directory #640
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
torkelrogstad
added
chain
torkelrogstad
force-pushed
the
2019-07-17-datadir-logging
branch
from
57a8ce9
to
ece5fa3
Compare
torkelrogstad
changed the title
nkohen
reviewed
Jul 18, 2019
Christewart
reviewed
Jul 22, 2019
chain/src/main/scala/org/bitcoins/chain/blockchain/sync/ChainSync.scala
Outdated
Show resolved
Hide resolved
chain/src/main/scala/org/bitcoins/chain/blockchain/Blockchain.scala
Outdated
Show resolved
Hide resolved
torkelrogstad
force-pushed
the
2019-07-17-datadir-logging
branch
2 times, most recently
from
c5bb910
to
85aee98
Compare
|
This fails to compile on 2.11 https://travis-ci.org/bitcoin-s/bitcoin-s/jobs/565913446#L621 |
|
Tests do not pass for me locally either |
In this commit we add the ability for the node, chain and wallet projects (+ the server) to log to the users data directory instead of whatever directory the binaries was launched from. This is inherently a bit more complicated than our previous setup, because we need to read the user directory before we can create loggers. As a result of this, some files/methods were moved around, so the relevant app config could be found in scope. We also introduce several logging categories that can be tuned individually through user configuration. These logggers are exposed both as traits that give a field `logger`, or as methods that return the required logger.
In this commit we add support for AppConfig to pick up the data directory configuration file. We also add a section to the contributing guide on how to tune logging levels.
torkelrogstad
force-pushed
the
2019-07-17-datadir-logging
branch
from
85aee98
to
5104998
Compare
|
The error you're having is not reproducing locally for me, nor on CI. Are you still seeing them, @Christewart ? |
|
I pushed a new commit that changes our app configs to explictly take in the data directory as a parameter, instead of magically resolving it. This also closes #588 |
Closed
Christewart
pushed a commit
that referenced
this pull request
May 1, 2021
* Add logging to data directory In this commit we add the ability for the node, chain and wallet projects (+ the server) to log to the users data directory instead of whatever directory the binaries was launched from. This is inherently a bit more complicated than our previous setup, because we need to read the user directory before we can create loggers. As a result of this, some files/methods were moved around, so the relevant app config could be found in scope. We also introduce several logging categories that can be tuned individually through user configuration. These logggers are exposed both as traits that give a field `logger`, or as methods that return the required logger. * Add datadir configuration to AppConfig In this commit we add support for AppConfig to pick up the data directory configuration file. We also add a section to the contributing guide on how to tune logging levels. * Pass data directories explicitly for configuration
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #588
In this PR we add the ability for the node, chain
and wallet projects (+ the server) to log to the users
data directory instead of whatever directory the
binaries was launched from. This is inherently a bit
more complicated than our previous setup, because
we need to read the user directory before we can create
loggers. As a result of this, some files/methods were
moved around, so the relevant app config could be
found in scope.
Discussion item:
Right now we log in a similar way to how we already do it, by passing in the name of the file (instead of the class, but leads to almost identical behavior), and then constructing a logger with that name. This is why
sourcecode(from Li Haoyi) is added as a dependency. Should we instead move to a category based system, where we log with a predefined set of categories? This is how both Bitcoin Core and Lnd handles it. Categories could be things such as chainsync, P2P messages, transaction creation, address derivation, peer handling, etc. IMO this is a better solution, primarily because when we have a predefined number of loggers its easy to expose options inreference.conffor fine-tuning behavior of each logger. I.e. the user could set P2P messages to be logged atDEBUG, but turn off chain sync logs.TODO:
UPDATE:
Added several logging categories, as well as options for tuning logging levels and file and console logs on/off.