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

adding logging around libp2p one-to-one stream management #785

Merged
merged 4 commits into from Jun 8, 2021

Conversation

vishalchangrani
Copy link
Contributor

To help debug the stream reset issue, I have made some minor logging changes.

@@ -277,13 +277,13 @@ func (m *Middleware) SendDirect(msg *message.Message, targetID flow.Identifier)
// flush the stream
err = bufw.Flush()
if err != nil {
return fmt.Errorf("failed to flush stream for %s: %w", targetID.String(), err)
return fmt.Errorf("failed to flush stream for %s: %w", targetIdentity.String(), err)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

identity gives IP address in addition to node id.

@@ -79,7 +79,7 @@ func (rc *readConnection) receiveLoop(wg *sync.WaitGroup) {
rc.closeStream()
return
}
rc.log.Error().Err(err)
rc.log.Error().Err(err).Msg("failed to read message")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

logger won't print log if there is no Msg field.

@@ -120,12 +118,3 @@ func (rc *readConnection) resetStream() {
rc.log.Error().Err(err).Msg("failed to reset stream")
}
}

func streamLogger(log zerolog.Logger, stream libp2pnetwork.Stream) zerolog.Logger {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved this to libp2pUtils.go

@codecov-commenter
Copy link

Codecov Report

Merging #785 (af9bacb) into master (802be06) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #785      +/-   ##
==========================================
- Coverage   56.42%   56.42%   -0.01%     
==========================================
  Files         423      423              
  Lines       24793    24789       -4     
==========================================
- Hits        13990    13987       -3     
+ Misses       8909     8907       -2     
- Partials     1894     1895       +1     
Flag Coverage Δ
unittests 56.42% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
network/p2p/libp2pUtils.go 46.57% <0.00%> (-4.18%) ⬇️
network/p2p/middleware.go 0.00% <0.00%> (ø)
network/p2p/readConnection.go 0.00% <0.00%> (ø)
engine/execution/ingestion/engine.go 52.19% <0.00%> (-1.53%) ⬇️
engine/verification/match/engine.go 66.18% <0.00%> (+1.07%) ⬆️
engine/verification/match/chunks.go 90.00% <0.00%> (+6.66%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 802be06...af9bacb. Read the comment docs.

@vishalchangrani vishalchangrani merged commit cb558ea into master Jun 8, 2021
@vishalchangrani vishalchangrani deleted the vishal/adding_reset_logging branch June 8, 2021 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants