Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Update ouroboros-network & tracing changes #3497

Merged
merged 8 commits into from
Feb 3, 2022
Merged

Conversation

coot
Copy link
Contributor

@coot coot commented Jan 13, 2022

Make the following traces on by default:

  • TraceConnectionManager
  • TraceInboundGovernor
  • TracePeerSelection
  • TracePeerSelectionActions

All of them are not chatty if network conditions are stable.

  • Fix peer selection tracer.
  • Add transition tracers

IntersectMBO/ouroboros-network#3571

@coot coot added networking Issues and PRs related to networking peer2peer Issues / PRs related to peer-2-peer labels Jan 13, 2022
@coot coot marked this pull request as draft January 13, 2022 14:19
@coot coot marked this pull request as ready for review January 13, 2022 15:43
@coot coot requested a review from karknu as a code owner January 13, 2022 15:43
@coot coot changed the title P2P Trace Configuration Changes P2P Tracing Changes Jan 13, 2022
@coot coot marked this pull request as draft January 13, 2022 15:47
@coot coot force-pushed the coot/p2p-trace-config branch from f1feeb5 to b45322e Compare January 14, 2022 11:57
@coot coot force-pushed the coot/p2p-trace-config branch from b45322e to bdace4a Compare January 14, 2022 11:58
@coot coot changed the title P2P Tracing Changes Update ouroboros-network & tracing changes Jan 14, 2022
@coot coot self-assigned this Jan 14, 2022
@coot coot force-pushed the coot/p2p-trace-config branch from bdace4a to 911f7c3 Compare January 14, 2022 14:14
@coot coot force-pushed the coot/p2p-trace-config branch 2 times, most recently from 69dfdd1 to 4cd87ec Compare January 20, 2022 16:09
Copy link
Contributor

@karknu karknu left a comment

Choose a reason for hiding this comment

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

LGTM.
I'd like a newer ouroboros-network version once IntersectMBO/ouroboros-network#3535 is merged.

@coot coot force-pushed the coot/p2p-trace-config branch from 4cd87ec to 28dce18 Compare January 21, 2022 09:37
@coot coot marked this pull request as ready for review January 21, 2022 09:40
@coot coot requested a review from jutaro as a code owner January 21, 2022 09:40
@coot coot requested a review from newhoggy January 21, 2022 09:40
@coot coot force-pushed the coot/p2p-trace-config branch from 28dce18 to 8a3f65a Compare January 21, 2022 10:29
Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

A couple minor formatting changes and 1 question concerning the setting of default tracers. Looks good otherwise 👍

cardano-node/src/Cardano/Node/Tracing/Tracers.hs Outdated Show resolved Hide resolved
cardano-node/src/Cardano/Tracing/Config.hs Show resolved Hide resolved
@coot coot force-pushed the coot/p2p-trace-config branch from 90f9b7a to 75d8f90 Compare January 24, 2022 09:21
@coot coot requested a review from Jimbo4350 January 25, 2022 22:12
coot and others added 6 commits January 26, 2022 18:52
This change integrates the new `LocalTxMonitor` mini-protocol for
`node-to-client` connections.
* TraceConnectionManager
* TraceInboundGovernor
* TracePeerSelection
* TracePeerSelectionActions

All of them are not chatty if network conditions are stable.
Peer selection counters should be logged if `tracePeerSelectionCounters`
is set, not `tracePeerSelection`.

Their name space should be `PeerSelectionCounters` not `PeerSelection`.
The network related tracers, has been codeowned by the networking team;
let's preserve this for the new tracing system.
@coot coot force-pushed the coot/p2p-trace-config branch from 75d8f90 to cfe3da0 Compare January 26, 2022 17:57
@coot coot mentioned this pull request Feb 1, 2022
@coot
Copy link
Contributor Author

coot commented Feb 1, 2022

bors merge

iohk-bors bot added a commit that referenced this pull request Feb 1, 2022
3497: Update ouroboros-network & tracing changes r=coot a=coot

Make the following traces on by default:
* TraceConnectionManager
* TraceInboundGovernor
* TracePeerSelection
* TracePeerSelectionActions

All of them are not chatty if network conditions are stable.

* Fix peer selection tracer.
* Add transition tracers 

IntersectMBO/ouroboros-network#3571




Co-authored-by: Marcin Szamotulski <coot@coot.me>
Co-authored-by: Marcin Szamotulski <profunctor@pm.me>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 1, 2022

Build failed:

@coot
Copy link
Contributor Author

coot commented Feb 1, 2022

bors merge

iohk-bors bot added a commit that referenced this pull request Feb 1, 2022
3497: Update ouroboros-network & tracing changes r=coot a=coot

Make the following traces on by default:
* TraceConnectionManager
* TraceInboundGovernor
* TracePeerSelection
* TracePeerSelectionActions

All of them are not chatty if network conditions are stable.

* Fix peer selection tracer.
* Add transition tracers 

IntersectMBO/ouroboros-network#3571




Co-authored-by: Marcin Szamotulski <coot@coot.me>
Co-authored-by: Marcin Szamotulski <profunctor@pm.me>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 1, 2022

This PR was included in a batch that successfully built, but then failed to merge into master. It will not be retried.

Additional information:

{"message":"Waiting on code owner review from deepfire, denisshevchenko, and/or jutaro.","documentation_url":"https://docs.github.com/articles/about-protected-branches"}

@coot
Copy link
Contributor Author

coot commented Feb 3, 2022

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 3, 2022

Build succeeded:

@iohk-bors iohk-bors bot merged commit 40cf9b4 into master Feb 3, 2022
@iohk-bors iohk-bors bot deleted the coot/p2p-trace-config branch February 3, 2022 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
networking Issues and PRs related to networking peer2peer Issues / PRs related to peer-2-peer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants