throttled regular log of peers list at debug and trace

[macfarla]

PR description

Instead of trace logging the full list of peers at each connect/disconnect, log it at a defined interval 15s (trace) 120s (debug)

Fixed Issue(s)

Thanks for sending a pull request! Have you done the following?

• Checked out our contribution guidelines?
• Considered documentation and added the doc-change-required label to this PR if updates are required.
• Considered the changelog and included an update if required.
• For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

• unit tests: ./gradlew build
• acceptance tests: ./gradlew acceptanceTest
• integration tests: ./gradlew integrationTest
• reference tests: ./gradlew ethereum:referenceTests:referenceTests

[macfarla]

maybe these log lines should be not within connect/disconnect methods, since if there are no connections or disconnections you'd see no logging.

[fab-10]

Could you check if you can get the same result using a filter like this https://logging.apache.org/log4j/2.x/manual/filters.html#BurstFilter? since tuning this kind of things by configuration is much more flexible.

[macfarla]

Could you check if you can get the same result using a filter like this https://logging.apache.org/log4j/2.x/manual/filters.html#BurstFilter? since tuning this kind of things by configuration is much more flexible.

I can have a look tomorrow - thanks @fab-10
Also this same logic is used in SyncTargetManager - should it be refactored to use BurstFilter?

[fab-10]

if the filter can achieve the same goal then yes, it is better to tune the logs by config than by code when possible

[siladu]

Instead of trace logging the full list of peers at each connect/disconnect, log it at a defined interval 15s (trace) 120s (debug)

I think logging at connect/disconnect is actually quite useful, at least at TRACE level, otherwise we might miss a peer that connects and then immediately disconnects. We could probably do something clever here to log when it's a quick disconnection (under some threshold), but I would be happy for TRACE to be completely verbose and log them all each time so we don't mislead ourselves when we analyze it.

Logging an additional throttled version at DEBUG looks good 👍

[macfarla]

coming back to this - I think burst filter isn't the right mechanism to limit this at DEBUG because it involves user action (logging config) to configure.
Think we should leave this as is for now and maybe have a scheduled timer task to DEBUG log the EthPeers full list periodically if we still want that.