Fix the property metrics for prometheus

[jacomago]

Problem was that on the prometheus list wasn't getting a breakdown of properties more than the first defined one.
Discovered this is due to for each metric (Gauge in micrometer language) every tag has to have a value. So I changed it from

cf.channel.count{propertyName=propertyValue}

to

cf.channel.count{prop0=propValueA, prop1=propValueB}

and all the many combinations.

Note:
From the propertyRepository api, I don't think there is a way to get all of the property values for a property. I think this would be useful, but maybe a big query with the current database design. Can think about this maybe?

[simon-ess]

I have to admit that I'm not 100% certain that I follow what is being fixed when changing from

cf.channels.count{propertyName=propertyValue}

to

cf.channels.count.propertyName{propertyName=propertyValue}

Is the point that if you have two propertynames (propA and propB) then you only got one of the two, i.e. you would only see

cf.channels.count{propA=propertyValue}

instead of

cf.channels.count{propA=propertyValue}
cf.channels.count{propB=propertyOtherValue}

But does this still allow you to see

cf.channels.count.propertyName{propertyName=propertyValue1}
cf.channels.count.propertyName{propertyName=propertyValue2}

in the output?

[simon-ess]

Might be good to give an explicit example of using this syntax; the application.properties file only has a single metricProperty.

[jacomago]

Added some docs.

[anderslindho]

Don't love this from the description. The suffix _count should only be there for counters, so if this really is a gauge then it should not be present. The current metric we have been using appears to be a counter (but maybe this is just since CF never forgets channels), as we can do (somewhat pseudo)

• cf_channel_count(active)
• cf_channel_count(inactive)
  but also do
• cf_channel_count
  which then is the sum of the above two metrics

If we start to do things like cf_channel_count_whatever, then we're breaking the convention (suffix with _count for counters), and cf_channel_count_tag(tag) also strikes me as odd.

IMO I think we instead should go for a dumber implementation for counting properties and tags, and especially for counting channels with a specific property or tag. The biggest reason for this, in my opinion, is because cf_channel_count without (prometheus) tags always should be the total count of channels, which voids the use of props and prometheus tags outside of pvStatus.

So I would prefer:

• cf_channel_count(pvStatus)
  and then for any other property we want to track, we do something like
• cf_alias_count
• cf_aa_policy_count

[jacomago]

I have to admit that I'm not 100% certain that I follow what is being fixed when changing from

cf.channels.count{propertyName=propertyValue}

to

cf.channels.count.propertyName{propertyName=propertyValue}

Is the point that if you have two propertynames (propA and propB) then you only got one of the two, i.e. you would only see

cf.channels.count{propA=propertyValue}

instead of

cf.channels.count{propA=propertyValue}
cf.channels.count{propB=propertyOtherValue}

But does this still allow you to see

cf.channels.count.propertyName{propertyName=propertyValue1}
cf.channels.count.propertyName{propertyName=propertyValue2}

in the output?

Yes exactly. The alternative solution is to put every property in every cf.channels.count metric, i.e.

cf.channels.count{propA=a, propB=b}

and then to get counts for only propA you could have an ignore value or something like

cf.channels.count{propA=a, propB=IGNORED}

But this seems a bit overkill, as you probably don't care about every combination.

[anderslindho]

I have to admit that I'm not 100% certain that I follow what is being fixed when changing from

cf.channels.count{propertyName=propertyValue}

to

cf.channels.count.propertyName{propertyName=propertyValue}

Is the point that if you have two propertynames (propA and propB) then you only got one of the two, i.e. you would only see

cf.channels.count{propA=propertyValue}

instead of

cf.channels.count{propA=propertyValue}
cf.channels.count{propB=propertyOtherValue}

But does this still allow you to see

cf.channels.count.propertyName{propertyName=propertyValue1}
cf.channels.count.propertyName{propertyName=propertyValue2}

in the output?

Yes exactly. The alternative solution is to put every property in every cf.channels.count metric, i.e.

cf.channels.count{propA=a, propB=b}

and then to get counts for only propA you could have an ignore value or something like

cf.channels.count{propA=a, propB=IGNORED}

But this seems a bit overkill, as you probably don't care about every combination.

This actually sounds almost correct to me, if we are to continue with this level of flexibility and granularity. At least that way the aggregate count (without specifying props) is correct. 🤷

But rather than "IGNORED" this should be just something like cf_channel_count{propA=a, propB~=*} (which really is equivalent to cf_channel_count{propA=a}).

[anderslindho]

Since channels can have any number of tags, doesn't this break the sum channel count?

[jacomago]

Replaced the name to try and make it more clear what it means. And correspondingly updated the documentation.

[anderslindho]

Don't we need special handling for pvStatus?

The current design is very useful, where we get

cf_channel_count{pvStatus="Active"} 123.0
cf_channel_count{pvStatus="Inactive"} 3.0

Because then we know we can get the total channel count from

cf_channel_count # 126.0

but we can also use a gauge on it to see internally the split between active and inactive.

[jacomago]

Don't we need special handling for pvStatus?

The current design is very useful, where we get

cf_channel_count{pvStatus="Active"} 123.0
cf_channel_count{pvStatus="Inactive"} 3.0

Because then we know we can get the total channel count from

cf_channel_count # 126.0

but we can also use a gauge on it to see internally the split between active and inactive.

You will get that with cf_pvStatus_channel_count. In fact I'll add a test case for this.

[anderslindho]

Don't we need special handling for pvStatus?
The current design is very useful, where we get

cf_channel_count{pvStatus="Active"} 123.0
cf_channel_count{pvStatus="Inactive"} 3.0

Because then we know we can get the total channel count from

cf_channel_count # 126.0

but we can also use a gauge on it to see internally the split between active and inactive.

You will get that with cf_pvStatus_channel_count. In fact I'll add a test case for this.

I see your line of thinking, but just as a FYI this will break all displays we have right now (since there is one query up until deployment of this version, and a different one after deployment), and we will might never make use of the non-property metric for the channel count.

[sonarqubecloud]

Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[anderslindho]

I don't love how you name all of your test entitites 🙃