log: Don’t use scientific notation in log messages #29254

1. 
   kristapsk commented at 1:27 pm on January 16, 2024: contributor

   Don’t see any benefits here, only harder to read for most of the users.

   Before:

   02024-01-16T13:11:36Z Dumped mempool: 8.165e-06s to copy, 0.00224268s to dump
   

   After:

   02024-01-16T13:11:36Z Dumped mempool: 0.000s to copy, 0.002s to dump
   

2. 
   DrahtBot commented at 1:27 pm on January 16, 2024: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	glozow
   Concept ACK	epiccurious

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

3. kristapsk renamed this:
   Don't use scientific notation in "Dumped mempool" log message
   log: Don't use scientific notation in "Dumped mempool" message
   on Jan 16, 2024
4. DrahtBot added the label Utils/log/libs on Jan 16, 2024
5. 
   maflcko commented at 1:47 pm on January 16, 2024: member

   I don’t think nanosecond precision helps here. Might as well limit to %.2fs?

   Same here?

   0src/policy/fees.cpp:    LogPrint(BCLog::ESTIMATEFEE, "Recorded %u unconfirmed txs from mempool in %gs\n", num_entries, Ticks<SecondsDouble>(endclear - startclear));
   

6. 
   kristapsk commented at 11:52 am on January 17, 2024: contributor

   I don’t think nanosecond precision helps here. Might as well limit to %.2fs?

   Agree, such precision is not needed, but didn’t seem too important, message itself is relatively small anyway, so few extra characters doesn’t hurt.

   I would prefer millisecond precision, but plain %.3f will output 0.000 for 0.0009, don’t think that’s the right thing to do, should ceil or round before output then.

   Same here?

   0src/policy/fees.cpp:    LogPrint(BCLog::ESTIMATEFEE, "Recorded %u unconfirmed txs from mempool in %gs\n", num_entries, Ticks<SecondsDouble>(endclear - startclear));
   

   Will look at it.

7. 
   glozow commented at 11:36 am on January 18, 2024: member

   plain %.3f will output 0.000 for 0.0009, don’t think that’s the right thing to do, should ceil or round before output then.

   Why? “It took less than 1ms” should be pretty intuitive.

8. 
   epiccurious commented at 4:45 pm on January 31, 2024: none

   Tested ACK.

   Before: 2024-01-31T16:34:07Z Dumped mempool: 2.007e-06s to copy, 0.00121729s to dump

   With %f: 2024-01-31T16:36:03Z Dumped mempool: 0.000002s to copy, 0.001363s to dump

   With %0.3f: 2024-01-31T16:38:05Z Dumped mempool: 0.000s to copy, 0.001s to dump

   I would prefer millisecond precision, but plain %.3f will output 0.000 for 0.0009, don’t think that’s the right thing to do, should ceil or round before output then.

   A ceiling function wouldn’t be right either, since it would report 1ms for my test when it actually took 2 µs.

   Rounding to the nearest 1ms would be nice (1.99 ms would be logged as 2ms not 1ms), but might not be worth the added code complexity for little benefit.

9. 
   maflcko commented at 4:48 pm on January 31, 2024: member

   Are you still working on this, or should it be marked “up for grabs”?
10. 
    epiccurious commented at 5:09 pm on January 31, 2024: none

    @kristapsk if you prefer, I’d be happy to see it through.
11. Don't use scientific notation in log messages c819a83b4d
12. kristapsk force-pushed on Jan 31, 2024
13. kristapsk renamed this:
    log: Don't use scientific notation in "Dumped mempool" message
    log: Don't use scientific notation in log messages
    on Jan 31, 2024
14. 
    kristapsk commented at 7:21 pm on January 31, 2024: contributor

    Changed also other message and added rouding to ms precision.
15. 
    glozow commented at 5:50 pm on February 1, 2024: member

    lgtm ACK c819a83b4d83413a31323c1304664ee4c228ca29. can you update the PR description?
16. 
    kristapsk commented at 6:06 pm on February 1, 2024: contributor

    lgtm ACK c819a83. can you update the PR description?

    Updated.

17. 
    maflcko commented at 8:02 am on February 2, 2024: member

    lgtm
18. 
    glozow commented at 9:13 am on February 2, 2024: member

    lgtm ACK c819a83. can you update the PR description?

    Updated.

    Are you sure the ‘before" is accurate?

19. 
    kristapsk commented at 9:42 am on February 2, 2024: contributor

    lgtm ACK c819a83. can you update the PR description?

    Updated.

    Are you sure the ‘before" is accurate?

    Yes, %g results in using %e or %f, depending on what results in shorter string.

20. 
    kristapsk commented at 10:05 am on February 2, 2024: contributor

    lgtm ACK c819a83. can you update the PR description?

    Updated.

    Are you sure the ‘before" is accurate?

    Yes, %g results in using %e or %f, depending on what results in shorter string.

    Oh, wait, no, accidentally removed precision for “before” when editing, double checked debug.log, fixed.

21. glozow merged this on Feb 5, 2024
22. glozow closed this on Feb 5, 2024

    ---

23. kristapsk deleted the branch on Apr 24, 2024

Contributors
kristapsk DrahtBot maflcko glozow epiccurious

Labels
Utils/log/libs