Chan.Part.API: channel participation relation and status metrics #2025

wlahti · 2020-10-19T20:38:12Z

Type of change

New feature

Description

Add metrics to track the channel participation relation and status per channel for the orderer.

Related issues

FAB-18087

tock-ibm · 2020-10-21T05:36:05Z

Generally, I am a bit confused by the design of the metrics here.

A channel can have cluster-relation: {member, follower, config-tracker, none}
so why only the boolean is_member is_follower ?

In addition, the status can be {onboarding, active, inactive} (and in the future also error or failed).

Labeling with channel is fine, but labeling with status will create |status| x |clusterRelation| = 12 boolean gauges.
Is this what we want?

Can't we have a gauge that directly spans the enum values of status? for example:
status.%channel = 0 (=onboarding) / 1 (=active) / 2 (=inactive) / 3 (=error)

wlahti · 2020-10-29T21:46:20Z

Adjusted the metrics and updated the places I was already setting the metrics in this PR. Need to add reporting of the metrics to at least the config-tracker codepath (and consider whether we care to report cluster relation "none" for non-raft orderers).

tock-ibm · 2020-11-01T09:59:14Z

orderer/common/multichannel/metrics.go

+	case types.StatusActive:
+		s = 2
+	}


How about adding a default here that would catch if we changed/added to the enums?
This way if we change the enum it translates to 0=inactive...
say -1? that would get the admins attention.

tock-ibm · 2020-11-01T09:59:49Z

orderer/common/multichannel/metrics.go

+func (m *Metrics) reportClusterRelation(channel string, relation types.ClusterRelation) {
+	var r int
+	switch relation {
+	case types.ClusterRelationNone:
+		r = 0
+	case types.ClusterRelationConfigTracker:
+		r = 1
+	case types.ClusterRelationFollower:
+		r = 2
+	case types.ClusterRelationMember:
+		r = 3
+	}
+	m.ClusterRelation.With("channel", channel).Set(float64(r))
+}


How about adding a default here that would catch if we changed/added to the enums?
This way if we change the enum it translates to 0=None...
say -1? that would get the admins attention.

or maybe a more robust approach would be to add to the types package a couple of methods that convert the types.ClusterRelation* and type.Status* into integers?

Do you think it's likely we end up with additional cluster relations/statuses? I do think we should have a default, but maybe panic instead stating it's a programming error and a new metric conversion is needed in the switch statement?

I ordered these in ascending order to go along with the highest number being the most active in terms of participating in the cluster/channel, but if we anticipate more, maybe it'd be best to have 0 remain none/inactive, 1 = member/active, and then the rest. That way we keep it stable and there isn't a presumed progression of the values. Thoughts?

So maybe for cluster relation:
0 = none
1 = member
2 = follower
3 = config-tracker

And status:
0 = inactive
1 = active
2 = onboarding

The ordering looks good. I like the fact that <1,1> is the "normal". I agree that panic() is a good solution for default. This way we would hopefully catch inconsistencies in IT.

I think one additional status that is already in the cards is types.StatusFailure - see FAB-18106, and also my comment here.

Maybe adding it (types.StatusFailure) in the context of this task is not a bad idea?

docs/source/metrics_reference.rst

tock-ibm · 2020-11-06T13:14:28Z

orderer/common/multichannel/metrics.go

+		Namespace:    "cluster",
+		Subsystem:    "",
+		Name:         "relation",
+		Help:         "The cluster relation of the node: 0 if none, 1 if member, 2 if follower, 3 if config-tracker.",


member -> consenter

tock-ibm · 2020-11-06T13:23:38Z

orderer/common/multichannel/metrics.go

+	switch relation {
+	case types.ClusterRelationNone:
+		r = 0
+	case types.ClusterRelationMember:


wasn't this supposed to be types.ClusterRelationConsenter? probably needs a rebase as well

tock-ibm · 2020-11-06T13:31:00Z

Looks good, except that the transition of cluster-relation member->consenter needs to be applied here as well.

wlahti · 2020-11-06T13:46:32Z

Looks good, except that the transition of cluster-relation member->consenter needs to be applied here as well.

Thanks for catching that! Forgot that this PR was also affected by the rename somehow. :)

orderer/common/follower/follower_chain.go

jyellick · 2020-11-06T18:21:07Z

orderer/common/multichannel/metrics.go

+		Namespace:    "cluster",
+		Subsystem:    "",
+		Name:         "status",
+		Help:         "The status of the cluster node: 0 if inactive, 1 if active, 2 if onboarding.",


I'm not totally convinced here, but wouldn't it be more natural to go 0 inactive, 1 onboarding, 2 active? (ie, typically you transition from 0 to 1 to 2?)

That's how I originally implemented this but decided against it due to the fact that we may end up adding different options in the future. For the sake of keeping a stable base set of statuses and allowing new ones in the future, I thought it might be best to move away from the concept of a progression in the enum.

jyellick · 2020-11-06T18:28:08Z

orderer/common/multichannel/metrics.go

+
+var (
+	clusterStatusOpts = metrics.GaugeOpts{
+		Namespace:    "cluster",


So, we already have a "cluster" namespace, used by orderer/common/cluster, but it does specify a subsystem of comm. Perhaps we should add a subsystem of participation here? Or pick a different namespace?

I definitely like the idea of adding "participation" as the subsystem. And I wasn't in love with using "cluster" as the namespace. Maybe it should really be "channel"?

Went with "cluster" and "participation" for now (and updated the help text) since the channel name is included as a label.

The more I think on this, the more convinced I am that 'cluster' isn't right, and I'm a little confused why we call it cluster status at all. It has nothing to do with the cluster? Being a follower does not make you a member of the cluster... why not a vanilla channel_participation?

Are you suggesting we change the clusterRelation in general reported by the channel participation API in addition to the metric? We could rename that to Relation if it's not cluster-related and then the metric could be namepsace "channel" subsystem "participation" and name "relation" for channel_participation_relation.

Updated to what I mentioned in my previous comment.

Updated to "participation" for the namespace and no subsystem.

jyellick · 2020-11-06T18:29:22Z

orderer/common/multichannel/metrics.go

+	clusterStatusOpts = metrics.GaugeOpts{
+		Namespace:    "cluster",
+		Subsystem:    "",
+		Name:         "status",


The name status is pretty hopelessly vague, unless we add more context with a subsystem or different namespace. A cluster status of 'active' for instance does not imply that the other nodes are reachable, etc. It is the participation_status or similar.

Done. It's participation_status now with the namespace change.

jyellick · 2020-11-06T18:30:05Z

orderer/common/multichannel/metrics.go

+	clusterRelationOpts = metrics.GaugeOpts{
+		Namespace:    "cluster",
+		Subsystem:    "",
+		Name:         "relation",


I also wonder if relation is the best we can do here? I understand that's what it's referred to in the code, but it's not exactly obvious to someone looking at a metrics dashboard.

Created follow-up task https://jira.hyperledger.org/browse/FAB-18327 for renaming this (and the underlying types.ClusterRelation) to consensus relation.

jyellick · 2020-11-06T18:32:19Z

orderer/consensus/etcdraft/consenter.go

 			c.InactiveChainRegistry.TrackChain(support.ChannelID(), support.Block(0), func() {
 				c.ChainManager.CreateChain(support.ChannelID())
 			})
+			c.ChainManager.ReportClusterRelationAndStatusMetrics(support.ChannelID(), types.ClusterRelationConfigTracker, types.StatusInactive)


I really do not like this. And this is one of the strong reasons why I asked that we please ditch the notion of the raft consenter switching the chain out, and delegate this back up to the multichannel registrar. Consensus plugins have no business setting cluster relation status.

Yes, I did not like doing this but it's the only way I found with the current code flow.

Do we have an outstanding JIRA to fix this flow?

No, I do not see a JIRA for this. I can try to create one but I'm not sure if I have enough context in this area to get it completely right.

I believe there is a Jira for it, or a variation on this theme: https://jira.hyperledger.org/browse/FAB-18152

However, the big landmine we have to dismantle is the pattern with the InactiveChainRegistry etc., and we decided back then to finish the channel participation functionality first, and only then start refactoring. Moreover, the dependencies around the InactiveChainRegistry and BlockPuller are pretty complex, so this should be done very carefully... (if at all - "if it ain't broke don't fix it" comes to mind). Hopefully when we eventually deprecate the system channel this task will be a lot easier.

FAB-18087 Signed-off-by: Will Lahti <wtlahti@us.ibm.com>

wlahti requested a review from a team as a code owner October 19, 2020 20:38

wlahti force-pushed the fab-18087 branch 2 times, most recently from 2eea787 to 55d8c9d Compare October 19, 2020 21:33

wlahti requested a review from a team as a code owner October 19, 2020 21:33

C0rWin added the channel-participation-api label Oct 19, 2020

wlahti force-pushed the fab-18087 branch from 55d8c9d to dba7e3f Compare October 29, 2020 21:42

wlahti changed the title ~~Chan.Part.API: Add IsMember and IsFollower metrics~~ Chan.Part.API: Add cluster relation and status metrics Oct 29, 2020

wlahti marked this pull request as draft October 29, 2020 21:43

tock-ibm reviewed Nov 1, 2020

View reviewed changes

wlahti force-pushed the fab-18087 branch 5 times, most recently from e608eab to 910ca0b Compare November 5, 2020 12:53

wlahti marked this pull request as ready for review November 5, 2020 12:55

wlahti marked this pull request as draft November 5, 2020 13:57

wlahti force-pushed the fab-18087 branch from 910ca0b to a339003 Compare November 5, 2020 15:13

wlahti marked this pull request as ready for review November 5, 2020 18:08

tock-ibm reviewed Nov 6, 2020

View reviewed changes

wlahti force-pushed the fab-18087 branch from a339003 to a2b521a Compare November 6, 2020 13:46

wlahti force-pushed the fab-18087 branch from a2b521a to 94a49b5 Compare November 6, 2020 13:49

jyellick reviewed Nov 6, 2020

View reviewed changes

wlahti force-pushed the fab-18087 branch 2 times, most recently from 0e87fde to a51e57e Compare November 7, 2020 13:45

wlahti changed the title ~~Chan.Part.API: Add cluster relation and status metrics~~ Chan.Part.API: channel participation relation and status metrics Nov 7, 2020

wlahti force-pushed the fab-18087 branch from a51e57e to a958ec0 Compare November 7, 2020 14:09

wlahti force-pushed the fab-18087 branch 2 times, most recently from dcd5cb3 to c73e26f Compare November 9, 2020 13:06

Chan.Part.API: channel participation relation/status metrics

6432c50

FAB-18087 Signed-off-by: Will Lahti <wtlahti@us.ibm.com>

wlahti force-pushed the fab-18087 branch from c73e26f to 6432c50 Compare November 9, 2020 15:06

jyellick approved these changes Nov 9, 2020

View reviewed changes

jyellick merged commit d54ed92 into hyperledger:master Nov 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chan.Part.API: channel participation relation and status metrics #2025

Chan.Part.API: channel participation relation and status metrics #2025

wlahti commented Oct 19, 2020 •

edited

Loading

tock-ibm commented Oct 21, 2020 •

edited

Loading

wlahti commented Oct 29, 2020

tock-ibm Nov 1, 2020

tock-ibm Nov 1, 2020

tock-ibm Nov 1, 2020

wlahti Nov 1, 2020

tock-ibm Nov 3, 2020 •

edited

Loading

tock-ibm Nov 6, 2020

wlahti Nov 6, 2020

tock-ibm Nov 6, 2020

wlahti Nov 6, 2020

tock-ibm commented Nov 6, 2020

wlahti commented Nov 6, 2020

jyellick Nov 6, 2020

wlahti Nov 6, 2020

jyellick Nov 6, 2020

wlahti Nov 6, 2020

wlahti Nov 6, 2020 •

edited

Loading

jyellick Nov 6, 2020

wlahti Nov 6, 2020 •

edited

Loading

wlahti Nov 7, 2020

wlahti Nov 9, 2020

jyellick Nov 6, 2020

wlahti Nov 9, 2020

jyellick Nov 6, 2020

wlahti Nov 9, 2020

jyellick Nov 6, 2020

wlahti Nov 6, 2020

jyellick Nov 6, 2020

wlahti Nov 6, 2020

tock-ibm Nov 8, 2020 •

edited

Loading

Chan.Part.API: channel participation relation and status metrics #2025

Chan.Part.API: channel participation relation and status metrics #2025

Conversation

wlahti commented Oct 19, 2020 • edited Loading

Type of change

Description

Related issues

tock-ibm commented Oct 21, 2020 • edited Loading

wlahti commented Oct 29, 2020

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

tock-ibm Nov 3, 2020 • edited Loading

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

Choose a reason for hiding this comment

tock-ibm commented Nov 6, 2020

wlahti commented Nov 6, 2020

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

wlahti Nov 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wlahti Nov 6, 2020 • edited Loading

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tock-ibm Nov 8, 2020 • edited Loading

Choose a reason for hiding this comment

wlahti commented Oct 19, 2020 •

edited

Loading

tock-ibm commented Oct 21, 2020 •

edited

Loading

tock-ibm Nov 3, 2020 •

edited

Loading

wlahti Nov 6, 2020 •

edited

Loading

wlahti Nov 6, 2020 •

edited

Loading

tock-ibm Nov 8, 2020 •

edited

Loading