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

Network tracers #2178

Merged
merged 2 commits into from Dec 14, 2020
Merged

Network tracers #2178

merged 2 commits into from Dec 14, 2020

Conversation

coot
Copy link
Contributor

@coot coot commented Dec 5, 2020

This PR improves network tracers; originally done in coot/p2p-integration branch,
but this part is indepentend of p2p changes in ouroboros-network. This PR consists of two patches:

  • Improved mini-protocol tracers
    Which includes agency together with the mini-protocol message tag.

  • networking trace using show instances rather than json values

@coot coot marked this pull request as draft December 5, 2020 12:51
@coot coot marked this pull request as ready for review December 5, 2020 13:11
Trace the agancy together with a mini-protocol message tag.
Ouroboros-Network was able to do that for some time, this patch provides
necessary tracing class instance changes.
When using 'ScText' format, Show instances provide much more readable
logging output.
@coot coot added the networking Issues and PRs related to networking label Dec 8, 2020
getSeverityAnnotation BlockFetch.CompletedBlockFetch {} = Info
getSeverityAnnotation BlockFetch.CompletedFetchBatch {} = Info
getSeverityAnnotation BlockFetch.RejectedFetchBatch {} = Info
getSeverityAnnotation BlockFetch.ClientTerminating {} = Notice

Copy link
Contributor

@deepfire deepfire Dec 11, 2020

Choose a reason for hiding this comment

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

@coot, do you think it can be:

Suggested change
getSeverityAnnotation BlockFetch.ClientTerminating {} = Notice
getSeverityAnnotation _ = Info

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We will have something like 20 hot peers, so these messages will rather be sparse. I thought that initially its better to start with Notice and we can move to Info later. Are you ok with it @deepfire ?

Copy link
Contributor

@deepfire deepfire left a comment

Choose a reason for hiding this comment

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

LGTM with one small nit.

Thank you, @coot!

@coot
Copy link
Contributor Author

coot commented Dec 11, 2020

bors merge

iohk-bors bot added a commit that referenced this pull request Dec 11, 2020
2178: Network tracers r=coot a=coot

This PR improves network tracers; originally done in `coot/p2p-integration` branch,
but this part is indepentend of p2p changes in `ouroboros-network`.  This PR consists of two patches:

- Improved mini-protocol tracers
  Which includes agency together with the mini-protocol message tag.

- networking trace using show instances rather than json values


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

coot commented Dec 14, 2020

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Dec 14, 2020

@iohk-bors iohk-bors bot merged commit 1db3aee into master Dec 14, 2020
@iohk-bors iohk-bors bot deleted the coot/network-tracers branch December 14, 2020 09:18
Copy link
Contributor

@dcoutts dcoutts left a comment

Choose a reason for hiding this comment

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

LGTM!

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants