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.

Sign up for GitHub

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

Jump to bottom

tx submission tracing details at default log level #527

Closed
dcoutts opened this issue Jan 31, 2020 · 4 comments · Fixed by #624
Closed

tx submission tracing details at default log level #527

dcoutts opened this issue Jan 31, 2020 · 4 comments · Fixed by #624
Assignees
@intricate

Comments

@dcoutts
Copy link
Contributor

dcoutts commented Jan 31, 2020 •
edited
Loading

We need to ensure that at the default tracing levels we get the crucial information needed to trace a tx through the mempool of multiple nodes, to see where it got to.

So at default logging level:

  • for TraceMempoolAddTxs we must see the txids
  • for TraceMempoolRejectedTxs we must see the txids and rejection reason
  • for TraceMempoolRemoveTxs we must see the txids
  • for TraceMempoolManuallyRemovedTxs we must also see the txids here.

(I wonder if TraceMempoolManuallyRemovedTxs should not be merged with TraceMempoolRemovedTxs and include a reason enumeration instead. We can have things removed because of timeout or because of ledger state changes.)

We also need to trace at the default log level for the outbound side of tx submission:

  • when a remote peer has requested a tx id, and we do send the tx to them, then we should log that the txid was requested and sent
@dcoutts dcoutts changed the title tx submission tracing tx submission tracing details at default log level Jan 31, 2020
@dcoutts dcoutts mentioned this issue Jan 31, 2020
Open
@dcoutts
Copy link
Contributor Author

dcoutts commented Jan 31, 2020

So it looks like these ought to be included at the "Informational" severity level, not debug. Our default min severity level for the node is probably going to be set to Informational.

@intricate intricate mentioned this issue Feb 4, 2020
Merged
iohk-bors bot added a commit that referenced this issue Feb 6, 2020
@iohk-bors @intricate
Merge #538
931185e 
538: Mempool event tracing details at default log level r=intricate a=intricate

Related: #527 

Co-authored-by: Luke Nadur <[email protected]>
@intricate
Copy link
Contributor

Added tracing for TraceEventMempools in #538.

The protocol-level tx submission tracing seem to requires a change to ouroboros-network so I'll be raising another PR for that.

@intricate intricate added this to the S6 2020-02-13 milestone Feb 7, 2020
@intricate intricate mentioned this issue Feb 7, 2020
Merged
iohk-bors bot added a commit to IntersectMBO/ouroboros-network that referenced this issue Feb 11, 2020
@iohk-bors @intricate
Merge #1600
578147a 
1600: Add ProtocolTracers parameter to Ouroboros.Consensus.Node.run r=intricate a=intricate

Requirement for IntersectMBO/cardano-node#527.

Specifically:

>We also need to trace at the default log level for the outbound side of tx submission:
>* when a remote peer has requested a tx id, and we do send the tx to them, then we should log that the txid was requested and sent

Co-authored-by: Luke Nadur <[email protected]>
iohk-bors bot added a commit to IntersectMBO/ouroboros-network that referenced this issue Feb 11, 2020
@iohk-bors @intricate
Merge #1600
e60513f 
1600: Add ProtocolTracers parameter to Ouroboros.Consensus.Node.run r=intricate a=intricate

Requirement for IntersectMBO/cardano-node#527.

Specifically:

>We also need to trace at the default log level for the outbound side of tx submission:
>* when a remote peer has requested a tx id, and we do send the tx to them, then we should log that the txid was requested and sent

Co-authored-by: Luke Nadur <[email protected]>
@vhulchenko-iohk vhulchenko-iohk modified the milestones: S6 2020-02-13, S7 2020-02-27 Feb 17, 2020
@intricate
Copy link
Contributor

Following the merge of #569, I'll be raising another PR which will fully close this issue.

@intricate intricate mentioned this issue Feb 18, 2020
Merged
@intricate
Copy link
Contributor

See #584 (review).

@intricate intricate mentioned this issue Feb 24, 2020
Merged
iohk-bors bot added a commit to IntersectMBO/ouroboros-network that referenced this issue Feb 26, 2020
@iohk-bors @intricate
Merge #1688
ba0117e 
1688: Trace transaction requests and replies in txSubmissionOutbound r=intricate a=intricate

Related to IntersectMBO/cardano-node#527.

More specifically, see IntersectMBO/cardano-node#584 (review).

Co-authored-by: Luke Nadur <[email protected]>
@Jimbo4350 Jimbo4350 modified the milestones: S7 2020-02-27, S8 2020-03-12 Mar 2, 2020
@intricate intricate mentioned this issue Mar 3, 2020
Merged
@iohk-bors iohk-bors bot closed this as completed in 330d4c7 Mar 3, 2020
@iohk-bors iohk-bors bot closed this as completed in #624 Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@intricate intricate

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Implement ToObject instance for TraceTxSubmissionOutbound IntersectMBO/cardano-node
4 participants
@dcoutts @Jimbo4350 @intricate @vhulchenko-iohk