Skip to content

[ETCM-273] Change logging behaviour and increase handshake timeout to… #758

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Oct 29, 2020
Merged

[ETCM-273] Change logging behaviour and increase handshake timeout to… #758

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8be0fbb
[ETCM-273] Change logging behaviour and increase handshake timeout to…
Oct 26, 2020
f334022
Merge branch 'develop' into etcm-273-testnet-connection-issue
mirkoAlic Oct 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/main/resources/logback.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@
<appender-ref ref="METRICS" />
</root>

<logger name="io.iohk.ethereum.network.rlpx.RLPxConnectionHandler" level="INFO" />
<logger name="io.iohk.ethereum.network.rlpx.RLPxConnectionHandler" level="DEBUG" />
<logger name="io.iohk.ethereum.blockchain.sync.SyncController" level="INFO" />
<logger name="io.iohk.ethereum.network.PeerActor" level="INFO" />
<logger name="io.iohk.ethereum.network.PeerActor" level="DEBUG" />
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we enable this permanently and remove this lines? (as we are setting the DEBUG option above)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be a temporary change only. When we will have more peers we will be flooded by logs from there

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how temporal this will be 🤔 we still have this sort of logging on the other project

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also have some bothering logs of all the messages sent, should we remove them? Wdyt @mirkoAlic @mmrozek ?

Copy link
Contributor

@mirkoAlic mirkoAlic Oct 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should re-think our log polices, IMHO:

  • info: Display logs that are meaningful for someone that runs the client and more a less wants to see what is going on behind the scene.
  • debug: Log everything you could

By following those definitions, i think our issue is that PeerActor or network in general is no logging relevant information at info level.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but I think the only thing that we can do about this for now is pay closer attention to the logs changed or added on new PRs (till maybe someone starts using Mantis and complains about something like that). We should probably better define what a client would like to see 🤔

Back to the question at hand, I don't see the logs of all the messages sent/received to be so useful (we haven't required them till now), or at least we should keep them in a single line for those that don't use the JSON formatting (I don't do so locally)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @ntallar. I don't remember when we needed logs from PeerActor and when we will be sure after merge that node doesn't receive any message we won't need it anymore also ;)

<logger name="io.iohk.ethereum.vm.VM" level="OFF" />

</configuration>
3 changes: 3 additions & 0 deletions src/universal/conf/testnet-internal.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ mantis {
# All testnet members are assumed to be honest so blacklisting is turned off
short-blacklist-duration = 0
long-blacklist-duration = 0

wait-for-handshake-timeout = 15 seconds

}

rpc {
Expand Down