parachain informant by sistemd · Pull Request #8332 · paritytech/polkadot-sdk

sistemd · 2025-04-24T22:57:53Z

Closes #8216.

alindima · 2025-04-25T07:08:32Z

I think this has an overlap with #8230, which adds several similar metrics and logs much of the same events

sandreim · 2025-04-25T09:33:44Z

I think this has an overlap with #8230, which adds several similar metrics and logs much of the same events

Yes, there is indeed, but it seems to focus on other metrics.

@sistemd thanks for the PR, please also add an unincluded segment size metric. You could also check if we already have other metric for block authorship duration, if we don't we should have that metric. We already have the log for it.

skunert · 2025-04-25T13:12:09Z

Ah, I was not aware of the overlap. However, the scope seems slightly different. On the parachain informant we can also give a bit more detailed info since we decode the header.

Otherwise already looking pretty good already, some points:

The log messages should be a bit more scarce, we can put both the backed and included block in one message, together with the relay parent. Maybe something like Parachain status changed at #relay_num (0xrelay_block): backed #x (0xbacked) included #y (0xincluded)
The parachain informant itself should live in cumulus-client-service crate . This is the crate where we have the components that are instantiated during node construction. Then we can also instantiate it in cumulus-test-service and parachain-template nodes. (should be added to this PR)
The informant should then not depend on cumulus-client-pov-recovery. IMO transforming the streams of relay blocks into a stream of backed blocks and a stream of finalized blocks can be done in some utility crate, which is then used by the previous users and the informant. Looked a bit around but nothing stuck out. Maybe we should create cumulus-client-relay-chain-streams or something.

iulianbarbu

I don't have much context over this, but I am catching up on the go. Nothing outstanding from me, just a few comments. Looking forward to the rest of the changes.

cumulus/polkadot-omni-node/lib/src/common/spec.rs

…nformant

sistemd · 2025-04-27T20:44:16Z

@skunert (in the order of your points above):

sistemd · 2025-04-27T20:45:13Z

I think this has an overlap with #8230, which adds several similar metrics and logs much of the same events

Yes, there is indeed, but it seems to focus on other metrics.

@sistemd thanks for the PR, please also add an unincluded segment size metric. You could also check if we already have other metric for block authorship duration, if we don't we should have that metric. We already have the log for it.

@sandreim Does this make sense? I'm not sure if those are the right metrics, but hopefully it's on the right track.

cumulus/client/relay-chain-streams/src/lib.rs

cumulus/client/consensus/common/src/parachain_consensus.rs

cumulus/client/service/src/lib.rs

sistemd · 2025-04-29T09:47:29Z

Sorry for re-requesting your review @skunert, I haven't addressed your comments above yet.

sistemd · 2025-04-29T12:58:42Z

@skunert Should be ready now 👍

cumulus/client/relay-chain-streams/src/lib.rs

cumulus/client/service/src/lib.rs

skunert · 2025-04-29T16:53:20Z

cumulus/client/service/src/lib.rs

+	loop {
+		select! {
+			(backed_block, relay_block) = backed_blocks.select_next_some() => {
+				match &last_backed_block {


What if there is a new included block but nothing backed? This can happen as well. In that case we should still print the included update.

I did not know this was possible. In this case I think it's better to separate the included and backed logs again. Because what do you log if you get to included block N at relay block 15 and after that you get backed block 0 at relay block 1 for example? How are you supposed to log both at the same time in this case? It starts to get a bit confusing, I think separating them is the simplest option. 24747e7

cumulus/client/service/src/lib.rs

…nformant

iulianbarbu

Looks pretty nice! I am wondering if you tried out running a parachain with zombienet and check how the logs and prometheus metrics show up, while relaychain & parachain blocks get produced and finalized. Would just test it at the end to wrap everything up, once open comments are resolved. LMK what you think.

cumulus/client/relay-chain-streams/src/lib.rs

cumulus/client/service/src/lib.rs

iulianbarbu · 2025-04-30T14:10:16Z

cumulus/client/service/src/lib.rs

+				log::info!(
+					"New backed block at relay block #{} ({}): #{} ({})",
+					relay_block.number(),
+					relay_block.hash(),
+					backed_block.number(),
+					backed_block.hash(),
+				);


why not use tracing? I think that even if it is not structured, should still work

This crate uses log everywhere, that's why I used log as well. Maybe the whole crate should change? BTW, why is this not standard across the entire codebase? Why would you ever use log instead of tracing anyway?

I think this codebase started long ago when maybe tracing wasn't stabilized, or not so popular (just a guess). Currently the binaries we have use a tracing-subscriber that is configured to capture logs coming from both tracing & log. I noticed the initiative is to use more of tracing (some places had recent changes from log to tracing, e.g. transaction-pool). I think all in all tracing ecosystem is a better fit for what we do here. I haven't used log that much in the past, but I find it maybe useful for small projects like simple tools, not very oriented on running in a production environment (although that's certainly not the case here, and it can still be used in production).

This crate uses log everywhere, that's why I used log as well. Maybe the whole crate should change?

Looks like it pulls log via sc_telemetry. However, its purpose is not to help with logging utilities for consumer crates, it is meant to consume tracing based events via the workers it defines as far as I understand from the public docs. I think we should use sc_tracing, even if our public docs mentions the crate is unstable. So yeah, maybe the whole crate needs to change, but of course, should be separate work. I guess we can live with the log crate for a while here.

I can resolve this once a new issue is created that tracks this (we can also add a getting started label to the issue since it should be trivial - it can be done by you or anyone else who has time).

cumulus/client/service/src/lib.rs

sandreim · 2025-04-30T16:53:47Z

I think this has an overlap with #8230, which adds several similar metrics and logs much of the same events

Yes, there is indeed, but it seems to focus on other metrics.
@sistemd thanks for the PR, please also add an unincluded segment size metric. You could also check if we already have other metric for block authorship duration, if we don't we should have that metric. We already have the log for it.

@sandreim Does this make sense? I'm not sure if those are the right metrics, but hopefully it's on the right track.

Yeah, looks good, thx

cumulus/client/service/src/lib.rs

…nformant

sistemd · 2025-05-09T15:06:19Z

/cmd prdoc --audience runtime_dev --audience node_operator --bump patch

…time_dev --audience node_operator --bump patch'

skunert

Last nits, then is ready to merge IMO.

Please add it to the parachain template as @iulianbarbu suggested here:

polkadot-sdk/templates/parachain/node/src/service.rs

Line 235 in f49189a

pub async fn start_parachain_node(

Nice work!

cumulus/client/service/src/lib.rs

skunert · 2025-05-12T14:55:18Z

We should probably start thinking about how long will omni-node and parachain-template-node be "in sync". I would open an issue and tackle it sometime soon where we deprecate the node in the README, and a date due for this, and recommend clearly omni-node usage (which should be 100% the same in terms of usage and features). There are other templates nodes like minimal or solochain which showcase aspects of implementing a node from scratch, but parachain-template-node is mostly replaced by omni-node from what I can tell.

Hmm I am not fully convinced that we can remove it. For sure usage should be discouraged if one can get away with omni-node. But in the end we still need a way to start a fresh parachain, and the simple node-template will not be a good starting point. People would need to look at omni-node-lib and copy their service from there.

sistemd · 2025-05-12T17:19:24Z

Last nits, then is ready to merge IMO.

Please add it to the parachain template as @iulianbarbu suggested here:

polkadot-sdk/templates/parachain/node/src/service.rs

Line 235 in f49189a

pub async fn start_parachain_node(

Nice work!

I think start_parachain_node calls start_relay_chain_tasks here, which in turn spawns the parachain informant. Is this all right? Am I missing something?

…nformant

iulianbarbu · 2025-05-12T17:42:43Z

Last nits, then is ready to merge IMO.
Please add it to the parachain template as @iulianbarbu suggested here:

polkadot-sdk/templates/parachain/node/src/service.rs

Line 235 in f49189a

pub async fn start_parachain_node(

Nice work!

I think start_parachain_node calls start_relay_chain_tasks here, which in turn spawns the parachain informant. Is this all right? Am I missing something?

Upsy, I think you are right. I think I looked at it wrong, thought that we add the task to omni-node only, but now I see it is in cumulus/client/service. We should be fine.

iulianbarbu

Nice work 💪 ! Just a few more nits.

LE: would be good to include @skunert suggestions before merging, CI needs some love too.

prdoc/pr_8332.prdoc

cumulus/parachains/runtimes/testing/yet-another-parachain/Cargo.toml

cumulus/client/service/src/lib.rs

paritytech-workflow-stopper · 2025-05-13T13:52:56Z

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/14997912118
Failed job name: test-linux-stable

Closes #8216. --------- Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Upgrade all polkadot-sdk git dependencies from branch `stable2503` to `stable2506`. Upgrade all Frontier EVM dependencies from branch `stable2503` to tag `frontier-stable2506`. Fix all compilation errors and test failures caused by upstream breaking API changes in polkadot-sdk stable2506 (134 PRs) and 9 patch releases. Upstream breaking changes addressed: - Remove local `AccountIdFor` alias, now provided by `frame_system::pallet_prelude` (paritytech/polkadot-sdk#7229) - Add `RelayParentOffset = ConstU32<0>` to parachain system config (paritytech/polkadot-sdk#8299) - Fix `Outcome` enum pattern matching: `Error` is now tuple variant wrapping `InstructionError` (paritytech/polkadot-sdk#8535) - Remove `RuntimeEvent` from `pallet_evm::Config` and `pallet_ethereum::Config` (frontier stable2506) - Add `RuntimeEvent` to `pallet_session::historical::Config` - Remove `PassByInner` import, use `.0` for `H256` inner access (paritytech/polkadot-sdk#7375) - Destructure 4 return values from `build_relay_chain_interface` (paritytech/polkadot-sdk#8072) - Add `metrics` field to `BuildNetworkParams` (paritytech/polkadot-sdk#8332) - Add `prometheus_registry` to `StartRelayChainTasksParams` (paritytech/polkadot-sdk#8332) Release: https://github.com/paritytech/polkadot-sdk/releases/tag/polkadot-stable2506 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

parachain informant

4ab10a7

sistemd requested review from iulianbarbu and skunert April 24, 2025 22:57

iulianbarbu reviewed Apr 25, 2025

View reviewed changes

cumulus/polkadot-omni-node/lib/src/common/spec.rs Outdated Show resolved Hide resolved

cumulus/polkadot-omni-node/lib/src/common/spec.rs Outdated Show resolved Hide resolved

sistemd added 5 commits April 27, 2025 14:15

terser log messages

f4feee1

cumulus-relay-chain-streams crate

8555c03

move the informant to start_relay_chain_tasks

5877fdd

metrics

3647ee1

Merge remote-tracking branch 'origin/master' into sistemd/parachain-i…

d4fdb8b

…nformant

sistemd marked this pull request as ready for review April 27, 2025 20:43

skunert reviewed Apr 28, 2025

View reviewed changes

sistemd requested review from iulianbarbu, sandreim and skunert April 29, 2025 09:46

sistemd added 2 commits April 29, 2025 14:54

move more streams into the cumulus-relay-chain-streams crate

7fe5c7e

calculate unincluded segment size correctly

6d8a764

skunert reviewed Apr 29, 2025

View reviewed changes

sistemd added 2 commits April 30, 2025 13:33

handle included and backed blocks separately

24747e7

Merge remote-tracking branch 'origin/master' into sistemd/parachain-i…

297064a

…nformant

iulianbarbu reviewed Apr 30, 2025

View reviewed changes

alindima reviewed May 1, 2025

View reviewed changes

cumulus/client/service/src/lib.rs Outdated Show resolved Hide resolved

cumulus/client/service/src/lib.rs Outdated Show resolved Hide resolved

cumulus/client/service/src/lib.rs Outdated Show resolved Hide resolved

Merge remote-tracking branch 'origin/master' into sistemd/parachain-i…

094e5c1

…nformant

sistemd force-pushed the sistemd/parachain-informant branch from 1b595dd to 2fa4062 Compare May 9, 2025 15:01

github-actions bot and others added 2 commits May 9, 2025 15:08

Update from github-actions[bot] running command 'prdoc --audience run…

f6d636e

…time_dev --audience node_operator --bump patch'

add prdoc

f49189a

skunert reviewed May 12, 2025

View reviewed changes

cumulus/client/service/src/lib.rs Outdated Show resolved Hide resolved

cumulus/client/service/src/lib.rs Outdated Show resolved Hide resolved

sistemd added 2 commits May 12, 2025 19:27

some nits

89ec28c

Merge remote-tracking branch 'origin/master' into sistemd/parachain-i…

a0a702c

…nformant

skunert approved these changes May 12, 2025

View reviewed changes

iulianbarbu approved these changes May 12, 2025

View reviewed changes

prdoc/pr_8332.prdoc Outdated Show resolved Hide resolved

cumulus/parachains/runtimes/testing/yet-another-parachain/Cargo.toml Outdated Show resolved Hide resolved

cumulus/client/service/src/lib.rs Show resolved Hide resolved

alindima approved these changes May 13, 2025

View reviewed changes

sistemd added 5 commits May 13, 2025 12:05

undo formatting change

e709bfe

update prdoc

9281d3f

parachain informant doc comment and small changes

c39d476

fix prdoc again

8f35b0d

ditto

7713c0a

sistemd force-pushed the sistemd/parachain-informant branch from 130d6ef to 7713c0a Compare May 13, 2025 13:29

sistemd enabled auto-merge May 14, 2025 08:59

Merge branch 'master' into sistemd/parachain-informant

4b2b6de

sistemd added this pull request to the merge queue May 14, 2025

Merged via the queue into master with commit fc3a207 May 14, 2025
247 of 249 checks passed

sistemd deleted the sistemd/parachain-informant branch May 14, 2025 13:21

alvicsam pushed a commit that referenced this pull request Oct 17, 2025

parachain informant (#8332)

453be8a

Closes #8216. --------- Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>

github-actions bot mentioned this pull request Oct 24, 2025

Update polkadot-sdk from stable2503 to stable2506 moondance-labs/tanssi#1340

Closed

snowmead mentioned this pull request Mar 16, 2026

build: ⬆️ Upgrade polkadot-sdk deps to stable2506 Moonsong-Labs/storage-hub#709

Draft

2 tasks

Conversation

sistemd commented Apr 24, 2025

Uh oh!

alindima commented Apr 25, 2025

Uh oh!

sandreim commented Apr 25, 2025

Uh oh!

skunert commented Apr 25, 2025

Uh oh!

iulianbarbu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sistemd commented Apr 27, 2025

Uh oh!

sistemd commented Apr 27, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sistemd commented Apr 29, 2025

Uh oh!

sistemd commented Apr 29, 2025

Uh oh!

Uh oh!

Uh oh!

skunert Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

sistemd Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

iulianbarbu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

iulianbarbu Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

sistemd May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iulianbarbu May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iulianbarbu May 7, 2025

Choose a reason for hiding this comment

Uh oh!

iulianbarbu May 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sandreim commented Apr 30, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sistemd commented May 9, 2025

Uh oh!

skunert left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

skunert commented May 12, 2025

Uh oh!

sistemd commented May 12, 2025

Uh oh!

iulianbarbu commented May 12, 2025

Uh oh!

iulianbarbu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

sistemd May 7, 2025 •

edited

Loading

iulianbarbu May 7, 2025 •

edited

Loading

iulianbarbu left a comment •

edited

Loading