Tracing documentation improvements #3500

deepfire · 2022-01-14T19:32:58Z

Expose the trace-documentation tracer documentation generator as a cardano-node subcommand and wire into Makefile
Regenerate the tracer documentation
Fix a small mistake in the New Tracing Quickstart document (thanks @coot !).

newhoggy · 2022-01-16T00:17:56Z

Can you add some background information?

For example, an example run, example output, screen capture, documentation or ticket?

newhoggy · 2022-01-16T00:24:46Z

doc/new-tracing/tracers_doc_generated.md

+1. [Cardano.Node.AcceptPolicy.ConnectionHardLimit](#cardanonodeacceptpolicyconnectionhardlimit)
+1. [Cardano.Node.BlockFetch.NodeToNode.Send.BatchDone](#cardanonodeblockfetchnodetonodesendbatchdone)
+1. [Cardano.Node.BlockFetch.NodeToNode.Send.Block](#cardanonodeblockfetchnodetonodesendblock)
+1. [Cardano.Node.BlockFetch.NodeToNode.Send.ClientDone](#cardanonodeblockfetchnodetonodesendclientdone)


Is this renumbering it all ones intentional?

@newhoggy, we're relying on Github's markdown auto-numbering.

oh, but we read markdowns not only on github.

Jimbo4350

I'm not so sure about having the trace documentation as an additional command in the cardano-node executable. This would make more sense living in cardano-cli under the Miscellanous Commands heading.

Is the plan to re-generate the documentation at doc/new-tracing/tracers_doc_generated.md each time the relevant Documented values change?

deepfire · 2022-01-17T15:59:59Z

Is the plan to re-generate the documentation at doc/new-tracing/tracers_doc_generated.md each time the relevant Documented values change?

Yes, precisely, that's the goal.

deepfire · 2022-01-17T18:04:20Z

I'm not so sure about having the trace documentation as an additional command in the cardano-node executable. This would make more sense living in cardano-cli under the Miscellanous Commands heading.

Then cardano-cli would have to link to cardano-node to get all the tracing instances.

Jimbo4350 · 2022-01-17T19:44:25Z

cardano-node/src/Cardano/Node/Tracing/Documentation.hs

-  ( Show t
-  , forall result. Show (Query blk result)
+docTracers :: forall blk peer remotePeer.
+  (forall result. Show (Query blk result)


forall result. Show (Query blk result) is unnecessary

Jimbo4350 · 2022-01-17T19:45:06Z

cardano-node/src/Cardano/Node/Tracing/Documentation.hs

-  ( Show t
-  , forall result. Show (Query blk result)
+docTracers :: forall blk peer remotePeer.
+  (forall result. Show (Query blk result)
  , TraceConstraints blk
  , LogFormatting (ChainDB.InvalidBlockReason blk)
  , LedgerSupportsProtocol blk


LedgerSupportsProtocol blk is unnecessary.

RunNode blk is pulling in 18 unnecessary constraints. I would consider creating a type synonym for the constraints needed.

@Jimbo4350, I've removed what I could, and consolidated the constraints a bit.

..but removing the RunNode constraint is very difficult as of now, because of how tangled things are at the moment.
I've spent some hours on it, and it's just not easy at all.

It'll be far easier to deal with it at the point when we'll be removing the legacy system.

Let me help you 😄

diff --git a/cardano-node/src/Cardano/Node/Tracing/Documentation.hs b/cardano-node/src/Cardano/Node/Tracing/Documentation.hs index 887705e29..44d19cb3b 100644 --- a/cardano-node/src/Cardano/Node/Tracing/Documentation.hs +++ b/cardano-node/src/Cardano/Node/Tracing/Documentation.hs @@ -47,11 +47,15 @@ import Cardano.Node.Handlers.Shutdown (ShutdownTrace) import Cardano.Node.Startup import Cardano.Node.TraceConstraints +import Ouroboros.Consensus.Block.Forging import Ouroboros.Consensus.BlockchainTime.WallClock.Types (RelativeTime) import Ouroboros.Consensus.BlockchainTime.WallClock.Util (TraceBlockchainTimeEvent (..)) import Ouroboros.Consensus.Cardano.Block -import Ouroboros.Consensus.Ledger.Query (Query) -import Ouroboros.Consensus.Ledger.SupportsMempool (ApplyTxErr, GenTxId) +import Ouroboros.Consensus.Ledger.Inspect +import Ouroboros.Consensus.Ledger.Query (Query, ShowQuery) +import Ouroboros.Consensus.Ledger.SupportsMempool (ApplyTxErr, GenTxId, + LedgerSupportsMempool) +import Ouroboros.Consensus.Ledger.SupportsProtocol import Ouroboros.Consensus.Mempool.API (TraceEventMempool (..)) import Ouroboros.Consensus.MiniProtocol.BlockFetch.Server (TraceBlockFetchServerEvent (..)) @@ -59,6 +63,7 @@ import Ouroboros.Consensus.MiniProtocol.ChainSync.Client (TraceChainSy import Ouroboros.Consensus.MiniProtocol.ChainSync.Server (TraceChainSyncServerEvent) import Ouroboros.Consensus.MiniProtocol.LocalTxSubmission.Server (TraceLocalTxSubmissionServerEvent (..)) +import Ouroboros.Consensus.Node.NetworkProtocolVersion import qualified Ouroboros.Consensus.Node.Run as Consensus import qualified Ouroboros.Consensus.Node.Tracers as Consensus import qualified Ouroboros.Consensus.Protocol.Ledger.HotKey as HotKey @@ -85,8 +90,8 @@ import Ouroboros.Network.PeerSelection.RootPeersDNS (TraceLocalRootPee TracePublicRootPeers (..)) import Ouroboros.Network.Protocol.BlockFetch.Type (BlockFetch) import Ouroboros.Network.Protocol.ChainSync.Type (ChainSync) -import Ouroboros.Network.Protocol.Handshake.Unversioned (UnversionedProtocolData (..), - UnversionedProtocol (..)) +import Ouroboros.Network.Protocol.Handshake.Unversioned (UnversionedProtocol (..), + UnversionedProtocolData (..)) import Ouroboros.Network.Protocol.LocalStateQuery.Type (LocalStateQuery) import qualified Ouroboros.Network.Protocol.LocalTxSubmission.Type as LTS import Ouroboros.Network.Protocol.TxSubmission.Type (TxSubmission) @@ -152,11 +157,19 @@ runTraceDocumentationCmd TraceDocumentationCmd{..} = do -- Can be changed, when old tracers have gone docTracers :: forall blk peer remotePeer. ( TraceConstraints blk + , InspectLedger blk + , LedgerSupportsMempool blk + , LedgerSupportsProtocol blk + , Consensus.SerialiseNodeToNodeConstraints blk , LogFormatting peer , LogFormatting remotePeer - , Consensus.RunNode blk + , Show (BlockNodeToClientVersion blk) + , Show (BlockNodeToNodeVersion blk) , Show remotePeer , Show peer + , Show (ForgeStateUpdateError blk) + , Show (CannotForge blk) + , ShowQuery (BlockQuery blk) ) => FilePath -> FilePath

What part in particular was difficult for you?

@Jimbo4350, applied your patch -- thanks a lot!

…in Makefile

deepfire · 2022-01-19T18:03:20Z

This is for #3510

Jimbo4350

LGTM! Just squash the last two commits into one.

deepfire · 2022-01-25T13:08:45Z

Thank you @Jimbo4350 !

deepfire · 2022-01-25T13:08:54Z

bors r+

3500: Tracing documentation improvements r=deepfire a=deepfire 1. Expose the `trace-documentation` tracer documentation generator as a `cardano-node` subcommand and wire into Makefile 2. Regenerate the tracer documentation 3. Fix a small mistake in the `New Tracing Quickstart` document (thanks `@coot` !). Co-authored-by: Kosyrev Serge <serge.kosyrev@iohk.io> Co-authored-by: Yupanqui <jnf@arcor.de>

Jimbo4350 · 2022-01-25T13:15:52Z

bors cancel

iohk-bors · 2022-01-25T13:15:54Z

Canceled.

Jimbo4350 · 2022-01-25T13:16:32Z

cardano-node/src/Cardano/Node/TraceConstraints.hs

-import           Cardano.Logging (LogFormatting)
-import           Cardano.Node.Queries (ConvertTxId, GetKESInfo (..),
-                     HasKESInfo (..), HasKESMetricsData (..), LedgerQueries)
+import Cardano.BM.Tracing (ToObject)


Style regression here

Undone the change to ImportQualifiedPost after discussion with @Jimbo4350

Co-Authored-by: Jordan Millar <jordan.millar@iohk.io>

deepfire · 2022-01-26T17:37:03Z

bors r+

3500: Tracing documentation improvements r=deepfire a=deepfire 1. Expose the `trace-documentation` tracer documentation generator as a `cardano-node` subcommand and wire into Makefile 2. Regenerate the tracer documentation 3. Fix a small mistake in the `New Tracing Quickstart` document (thanks `@coot` !). Co-authored-by: Kosyrev Serge <serge.kosyrev@iohk.io> Co-authored-by: Yupanqui <jnf@arcor.de>

iohk-bors · 2022-01-26T18:02:36Z

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 denisshevchenko and/or jutaro.","documentation_url":"https://docs.github.com/articles/about-protected-branches"}

deepfire · 2022-01-26T20:02:48Z

Ok, all checks and reviews passed, but some necessary people are absent, so force-merging this.

deepfire requested review from DavidEichmann, dcoutts, denisshevchenko, erikd, JaredCorduan, Jimbo4350, jutaro, nc6 and newhoggy as code owners January 14, 2022 19:33

deepfire force-pushed the trace-doc branch 3 times, most recently from b253f50 to 7f3a215 Compare January 14, 2022 20:00

newhoggy reviewed Jan 16, 2022

View reviewed changes

Jimbo4350 requested changes Jan 17, 2022

View reviewed changes

deepfire and others added 5 commits January 19, 2022 20:01

trace-dispatcher / cardano-node: trace-documentation command

67b0a69

node: Fix type variables

6beb1ad

tracing | Quickstart: fix the reference to HumanFormat*

6e582d2

doc | tracing: complete the tracer doc generator command and wire up …

3b4071e

…in Makefile

doc | tracers: regenerate documentation

db65013

deepfire force-pushed the trace-doc branch from 7f3a215 to db65013 Compare January 19, 2022 17:03

deepfire requested a review from MarcFontaine as a code owner January 19, 2022 17:03

deepfire self-assigned this Jan 19, 2022

deepfire force-pushed the trace-doc branch 3 times, most recently from 21af766 to 2680968 Compare January 24, 2022 22:54

Jimbo4350 self-requested a review January 25, 2022 11:57

Jimbo4350 approved these changes Jan 25, 2022

View reviewed changes

deepfire force-pushed the trace-doc branch from 2680968 to 3b5fce2 Compare January 25, 2022 13:06

Jimbo4350 self-requested a review January 25, 2022 13:16

Jimbo4350 requested changes Jan 25, 2022

View reviewed changes

node documentation: address review

2af1826

Co-Authored-by: Jordan Millar <jordan.millar@iohk.io>

deepfire force-pushed the trace-doc branch from 3b5fce2 to 2af1826 Compare January 26, 2022 16:58

deepfire requested a review from Jimbo4350 January 26, 2022 17:00

Jimbo4350 approved these changes Jan 26, 2022

View reviewed changes

deepfire merged commit 70b9874 into master Jan 26, 2022

iohk-bors bot deleted the trace-doc branch January 26, 2022 20:02

deepfire added the tracing label Jun 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tracing documentation improvements #3500

Tracing documentation improvements #3500

deepfire commented Jan 14, 2022

newhoggy commented Jan 16, 2022 •

edited

newhoggy Jan 16, 2022

deepfire Jan 17, 2022

coot Jan 25, 2022

Jimbo4350 left a comment

deepfire commented Jan 17, 2022

deepfire commented Jan 17, 2022

Jimbo4350 Jan 17, 2022

deepfire Jan 19, 2022

Jimbo4350 Jan 17, 2022

deepfire Jan 20, 2022

Jimbo4350 Jan 24, 2022 •

edited

deepfire Jan 24, 2022

deepfire commented Jan 19, 2022

Jimbo4350 left a comment

deepfire commented Jan 25, 2022

deepfire commented Jan 25, 2022

Jimbo4350 commented Jan 25, 2022

iohk-bors bot commented Jan 25, 2022

Jimbo4350 Jan 25, 2022

deepfire Jan 26, 2022

deepfire commented Jan 26, 2022

iohk-bors bot commented Jan 26, 2022

deepfire commented Jan 26, 2022

Tracing documentation improvements #3500

Tracing documentation improvements #3500

Conversation

deepfire commented Jan 14, 2022

newhoggy commented Jan 16, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jimbo4350 left a comment

Choose a reason for hiding this comment

deepfire commented Jan 17, 2022

deepfire commented Jan 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jimbo4350 Jan 24, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deepfire commented Jan 19, 2022

Jimbo4350 left a comment

Choose a reason for hiding this comment

deepfire commented Jan 25, 2022

deepfire commented Jan 25, 2022

Jimbo4350 commented Jan 25, 2022

iohk-bors bot commented Jan 25, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deepfire commented Jan 26, 2022

iohk-bors bot commented Jan 26, 2022

deepfire commented Jan 26, 2022

newhoggy commented Jan 16, 2022 •

edited

Jimbo4350 Jan 24, 2022 •

edited