Add group name in task metrics

[sandreim]

The subsystem name metric label improves observability of task CPU usage in Grafana - paritytech/polkadot#4198

Added two new spawn methods to SpawnNamed trait. These allow the caller to also specify a subsystem name when starting a task. While the change doesn't break any existing API, it changes the way metrics are logged by adding an extra label. All code using the original spawn methods will default to having subsystem set to substrate-unspecified.

Added optional group parameter to SpawnNamed::spawn() and SpawnNamed::spawn_blocking() methods.

Polkadot companion: paritytech/polkadot#4239

skip check-dependent-cumulus

[bkchr]

What do you think of naming it group instead of subsystem? Because in the end it is something to group different tasks?

[sandreim]

What do you think of naming it group instead of subsystem? Because in the end it is something to group different tasks?

Sounds better, indeed we just want to group by.

[sandreim]

Synced offline with @bkchr and decided to drop the new trait methods and just add a the group to the existing spawn() methods.

[ordian]

Same as paritytech/polkadot#4239 (comment)

You might also need cargo +nightly fmt.

[sandreim]

Same as paritytech/polkadot#4239 (comment)

You might also need cargo +nightly fmt.

Yep, this seems odd, there is a version mismatch of some sort. I'll rerun.

[bkchr]

Something like:

enum GroupName {
    Default,
    Specific(&'static str),
}

impl From<Option<&'static str>> for GroupName {
    fn from(name: Option<&'static str>) -> Self {
        match name { Some(name) => Self::Specific(name), None => Self::Default }
    }
}

impl From<&'static str> for GroupName {
    fn from(name: &'static str) -> Self {
        Self::Specific(name)
    }
}

would have been super cool :P

Because then you could write:

pub fn spawn(.., group: impl Into<GroupName>, ..)

And stuff like spawn(.., "name", ..) or spawn(.., None, ..) should work :P

This would maybe be the icing on the top, but no requirement.

[sandreim]

Yup, you are right, that's more flexible. I'll implement this as a followup PR, at the moment I'm tackling other things.

[ordian]

Sounds like an easy issue for external contributors :)

[dharjeezy]

Picking the issue up @ordian

[ordian]

bot merge

[paritytech-processbot]

Error: Waiting on code owner review from tomusdrw.

[ordian]

bot merge