Skip to content

Wrap Logging#823

Merged
quantumagi merged 15 commits intostratisproject:release/1.2.0.0from
quantumagi:movetonlog
Dec 20, 2021
Merged

Wrap Logging#823
quantumagi merged 15 commits intostratisproject:release/1.2.0.0from
quantumagi:movetonlog

Conversation

@quantumagi
Copy link
Contributor

@quantumagi quantumagi commented Dec 16, 2021

https://app.clickup.com/t/1nq21qq

This PR creates a standardized wrapper over everything related to logging - i.e.:

using NLog is removed and replaced with using Stratis.Bitcoin.Configuration.Logging where needed.

this.logger = LogManager.GetCurrentClassLogger(); remains supported but so also the logger factory pattern - which I managed to get working correctly. The latter can be removed in a follow-up PR but in this PR it serves to keep the solution-wide changes minimal.

The interesting changes are found in the Stratis.Bitcoin.Configuration.Logging namespace and those mentioned in comments below. The overwhelming bulk of changes just relate to the using statements.

@quantumagi quantumagi changed the base branch from master to release/1.2.0.0 December 16, 2021 01:38
@quantumagi quantumagi changed the title Movetonlog Remove Microsoft.Extensions.Logging from source Dec 16, 2021
@quantumagi quantumagi changed the title Remove Microsoft.Extensions.Logging from source Wrap Logging Dec 16, 2021
@quantumagi quantumagi requested a review from fassadlr December 16, 2021 01:52
@quantumagi
Copy link
Contributor Author

quantumagi commented Dec 16, 2021

@zeptin, I also found and fixed two cases of malformed logging statements:

image

@quantumagi
Copy link
Contributor Author

It seems Debug filtering is going to require another look before we can commit this.

@quantumagi
Copy link
Contributor Author

@zeptin, I've fixed a legacy issue with how LogLevel is determined when the -debug flag is present versus when -debug=xx.xx.xx is used to debug specific modules. In the past either case would change the global log level to debug while we only want that behaviour in the former case.

@fassadlr
Copy link
Contributor

@quantumagi did you test this branch with debug logging enabled? Does the other projects like Poa etc still do debug logging as they use the LogManager.GetCurrentClassLogger.

@fassadlr
Copy link
Contributor

Also there are some tests still failing.

@quantumagi
Copy link
Contributor Author

quantumagi commented Dec 17, 2021

@quantumagi did you test this branch with debug logging enabled? Does the other projects like Poa etc still do debug logging as they use the LogManager.GetCurrentClassLogger.

@fassadlr , i've completed a round of testing successfully but will be re-testing due to recent changes/simplifications. The LogManager is a new custom one that wraps the underlying one and re-implements GetCurrentClassLogger.

@quantumagi
Copy link
Contributor Author

@fassadlr , I'm still able to debug specific projects:

[2021-12-17 11:01:38.8124 5] INFO: Stratis.Bitcoin.Connection.ConnectionManagerBehavior Peer '[::ffff:66.23.237.146]:16179' connected (outbound), agent 'StratisFullNode:1.1.1.0 (80000)', height 3405336
[2021-12-17 11:01:38.8319 5] DEBUG: Stratis.Bitcoin.Features.PoA.Behaviors.PoAConsensusManagerBehavior [3a06442] Sending getheaders payload with first hash: '6e30d5d9cd247ed8b9a67e70fca06915ecd15f36bc30757a8a714ffc05c4d623'.
[2021-12-17 11:01:38.8319 5] DEBUG: Stratis.Bitcoin.Features.PoA.Behaviors.PoAConsensusManagerBehavior [3a06442] Sending getheaders payload with first hash: '6e30d5d9cd247ed8b9a67e70fca06915ecd15f36bc30757a8a714ffc05c4d623'.
[2021-12-17 11:01:38.8319 24] DEBUG: Stratis.Bitcoin.Features.PoA.Behaviors.PoAConsensusManagerBehavior [3a06442] 0 headers were selected for sending, last one is '(null)'.
[2021-12-17 11:01:38.8319 24] DEBUG: Stratis.Bitcoin.Features.PoA.Behaviors.PoAConsensusManagerBehavior [3a06442] 0 headers were selected for sending, last one is '(null)'.
[2021-12-17 11:01:39.3147 13] DEBUG: Stratis.Bitcoin.Features.PoA.Behaviors.PoAConsensusManagerBehavior [3a06442] Received 0 headers. First: '(null)'  Last: '(null)'.
[2021-12-17 11:01:39.3151 13] DEBUG: Stratis.Bitcoin.Features.PoA.Behaviors.PoAConsensusManagerBehavior [3a06442] Received 0 headers. First: '(null)'  Last: '(null)'.
[2021-12-17 11:01:39.3151 13] DEBUG: Stratis.Bitcoin.Features.PoA.Behaviors.PoAConsensusManagerBehavior [3a06442] Headers payload with no headers was received. Assuming we're synced with the peer.
[2021-12-17 11:01:39.3151 13] DEBUG: Stratis.Bitcoin.Features.PoA.Behaviors.PoAConsensusManagerBehavior [3a06442] Headers payload with no headers was received. Assuming we're synced with the peer.

@quantumagi quantumagi requested a review from zeptin December 20, 2021 07:00
@quantumagi quantumagi merged commit 0e30d2e into stratisproject:release/1.2.0.0 Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants