Include version string of version message we send in the debug log #4829

pull rebroad wants to merge 1 commits into bitcoin:master from rebroad:ShowVersionString changing 1 files +3 −4

rebroad commented at 7:11 AM on September 3, 2014: contributor

Include the version string we are sending to nodes in the debug output.
sipa commented at 7:13 AM on September 3, 2014: member

Would it suffice to log this once at startup instead? It's an immutable string anyway.
rebroad commented at 7:22 AM on September 3, 2014: contributor

@sipa well, yes, but they same could be said for other items in the version string, e.g. protocol version - do you want these moving out of the line per node also?
sipa commented at 7:26 AM on September 3, 2014: member

@rebroad That sounds reasonable; no need to log it on every established connection.
Include version string of version message we send in the debug log 99c9399a43
rebroad force-pushed on Sep 3, 2014
rebroad commented at 8:07 AM on September 3, 2014: contributor

@sipa changes suggested done
BitcoinPullTester commented at 8:34 AM on September 3, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4829_99c9399a436b2af7fb94545f7c48f15760e6c4fe/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

in src/net.cpp:None in 99c9399a43

1752 | @@ -1756,6 +1753,8 @@ void StartNode(boost::thread_group& threadGroup)
1753 |  
1754 |      Discover(threadGroup);
1755 |  
1756 | +    LogPrint("net", "Our version message: %s version %d\n", FormatSubVersion(CLIENT_NAME, CLIENT_VERSION, std::vector<string>()), PROTOCOL_VERSION);

laanwj commented at 8:54 AM on September 3, 2014:

Not 100% sure what you are trying to accomplish with this, but wouldn't it be better to offer this information on RPC (getnetworkinfo)? That's easier to query than searching in the log.

jgarzik commented at 12:51 PM on September 3, 2014: contributor

Indeed, one-time information should usually be returned via an RPC call. No need to log same information repeatedly at all, even at startup.
rebroad commented at 6:05 PM on September 3, 2014: contributor

Not sure I understand. This is logged only once in the debug.log now, as @sipa requested. Do we want it logged once, or every time it's used? It was initially every time, (as was PROTOCOL_VERSION), but then I changed it to just once.

There's much info in debug.log that isn't available via RPC, so I'm assuming you're not suggesting we move everything to RPC instead.
jgarzik commented at 6:12 PM on September 3, 2014: contributor

The protocol version is already in RPC. It makes sense to export that alongside.

Logs are awful for parsing.
rebroad commented at 3:16 AM on September 4, 2014: contributor

@jgarzik I'm not interested in using RPC currently. I just want it logged somewhere, hence this pull request.
laanwj commented at 7:51 AM on September 4, 2014: member

Agree with @jgarzik here. I think what you're trying to do (collect statistics by parsing the log) is misguided. The log is not an interface, RPC is. A better solution would be to have an external tool periodically collect the information through RPC. This is what I'm trying to say in https://github.com/rebroad/bitcoin/commit/a973b79e78cf4139efd8d778736cb7831ce775b6#commitcomment-7654200 too.
rebroad commented at 8:01 AM on September 4, 2014: contributor

@laanwj This pull request is unrelated to my stats collection. This pull request is just to store somewhere what the version string we are sending to clients looks like.
jgarzik commented at 1:57 AM on September 15, 2014: contributor

Added #4914 which exports this information via much more easily parseable RPC/json.
laanwj commented at 9:15 AM on September 16, 2014: member

Closing in favor of #4914
laanwj closed this on Sep 16, 2014
MarcoFalke locked this on Sep 8, 2021

Contributors

Linked (view graph)

#4914 RPC getinfo,getnetworkinfo: export local node's client sub-version string