Skip to content

Conversation

@jnewbery
Copy link
Contributor

More detailed logs are better

@maflcko
Copy link
Member

maflcko commented Apr 30, 2019

Should add a comment to initialize_datadir to explain why logging related configs should not go into the conf file?

@DrahtBot DrahtBot added the Tests label Apr 30, 2019
@jnewbery
Copy link
Contributor Author

@MarcoFalke - is there a good reason? Is it just so these options get picked up before the conf file is read?

@maflcko
Copy link
Member

maflcko commented Apr 30, 2019

I think it was something like "I don't want logging to be verbose and spam the debug.log when I start a regtest node in the temp dir for debugging after a test failure"

@practicalswift
Copy link
Contributor

Concept ACK

@laanwj
Copy link
Member

laanwj commented May 1, 2019

I think it was something like "I don't want logging to be verbose and spam the debug.log when I start a regtest node in the temp dir for debugging after a test failure"

Would definitely make sense to add a comment then. This is far from obvious, especially for new contributors.

Concept ACK ofc.

@jnewbery
Copy link
Contributor Author

jnewbery commented May 3, 2019

Should add a comment to initialize_datadir to explain why logging related configs should not go into the conf file?

Added comment as requested.

@jamesob
Copy link
Contributor

jamesob commented May 3, 2019

utACK 7b29ec2

@jonatack
Copy link
Member

jonatack commented May 6, 2019

Hi John, I tried to test this (recompiled, ran various functional tests, added changes to make them fail), and was unable to find a change in logging. Could you give an example I can run of where this makes a difference?

@maflcko
Copy link
Member

maflcko commented May 6, 2019

@jonatack Good catch. I suggest to extend some assert_debug_log with a string that also matches the thread name. Maybe in script checking or net?

@jnewbery
Copy link
Contributor Author

jnewbery commented May 6, 2019

Note that logthreadnames does not work on mingw32 and darwin.

I've verified that this works for me. Before:

head /tmp/user/1000/bitcoin_func_test_4apf_c6t/node0/regtest/debug.log
2019-05-06T16:44:58Z 




2019-05-06T16:44:58.838174Z Bitcoin Core version v0.18.99.0-unk (release build)
2019-05-06T16:44:58.838180Z InitParameterInteraction: parameter interaction: -bind set -> setting -listen=1
2019-05-06T16:44:58.838216Z Validating signatures for all blocks.
2019-05-06T16:44:58.838220Z Setting nMinimumChainWork=0000000000000000000000000000000000000000000000000000000000000000
2019-05-06T16:44:58.838283Z Using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation

After:

head -10 /tmp/user/1000/bitcoin_func_test_aihi949j/node0/regtest/debug.log 
2019-05-06T16:28:51Z 




2019-05-06T16:28:51.180256Z [init] Bitcoin Core version v0.18.99.0-unk (release build)
2019-05-06T16:28:51.180272Z [init] InitParameterInteraction: parameter interaction: -bind set -> setting -listen=1
2019-05-06T16:28:51.180392Z [init] Validating signatures for all blocks.
2019-05-06T16:28:51.180404Z [init] Setting nMinimumChainWork=0000000000000000000000000000000000000000000000000000000000000000
2019-05-06T16:28:51.180553Z [init] Using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation

(note the [init] prefix on the log lines).

I suggest to extend some assert_debug_log with a string that also matches the thread name.

This would break the functional tests on platforms that don't support -logthreadnames

@maflcko
Copy link
Member

maflcko commented May 6, 2019

Oh, yeah. Would have to add to test/.config.ini ...

Not asking you to do it here, though.

@maflcko maflcko merged commit 7b29ec2 into bitcoin:master May 6, 2019
maflcko pushed a commit that referenced this pull request May 6, 2019
7b29ec2 [tests] Comment for why logging config is set as command-line args. (John Newbery)
ba534cc [tests] log thread names by default in functional tests (John Newbery)

Pull request description:

  More detailed logs are better

ACKs for commit 7b29ec:
  jamesob:
    utACK 7b29ec2

Tree-SHA512: 327cfedb7b7bf32f7ce1e2de5f70c7092041a8e868e14285a79176277c6cf47ebea27027f68787332f8ad21c7f64d2640dd21813eda5b2bd0e5208a65364a879
@jnewbery jnewbery deleted the 2019-04-log-threads-tests branch May 6, 2019 17:17
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 10, 2019
…l tests

7b29ec2 [tests] Comment for why logging config is set as command-line args. (John Newbery)
ba534cc [tests] log thread names by default in functional tests (John Newbery)

Pull request description:

  More detailed logs are better

ACKs for commit 7b29ec:
  jamesob:
    utACK bitcoin@7b29ec2

Tree-SHA512: 327cfedb7b7bf32f7ce1e2de5f70c7092041a8e868e14285a79176277c6cf47ebea27027f68787332f8ad21c7f64d2640dd21813eda5b2bd0e5208a65364a879
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Jul 30, 2020
Summary:
[tests] Comment for why logging config is set as command-line args. (John Newbery)
[tests] log thread names by default in functional tests (John Newbery)

Pull request description:

  More detailed logs are better

https://github.com/bitcoin/bitcoin/pull/15927/files

---

Backport of Core [[bitcoin/bitcoin#15927 | PR15927]]

Test Plan:
  ninja
  test_runner.py

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D7082
kwvg added a commit to kwvg/dash that referenced this pull request Nov 4, 2021
kwvg added a commit to kwvg/dash that referenced this pull request Nov 11, 2021
pravblockc pushed a commit to pravblockc/dash that referenced this pull request Nov 18, 2021
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants