feat: add label 'state' to metric 'gitea_users'

[tobiasbp]

This PR adds the label state to the metric gitea_users. With the change, gitea_users would be reported like this:

...
# HELP gitea_users Number of Users
# TYPE gitea_users gauge
gitea_users{state="active"} 20
gitea_users{state="inactive"} 10
...

The metrics above would be from a Gitea instance with 30 user accounts. 20 of the accounts are active and 10 of the accounts are not active.

Resolve #34325

[TheFox0x7]

I'd personally prefer to have label state with active,inactive states. It's more flexible that way in case of extension needed.

# HELP gitea_users Number of Users
# TYPE gitea_users gauge
gitea_users{state="active"} 20
gitea_users{state="inactive"} 10

Also the metrics should probably end with _total, but I have to doublecheck this...

[tobiasbp]

I'd personally prefer to have label state with active,inactive states. It's more flexible that way in case of extension needed.

Good suggestion. I'll change it.

[tobiasbp]

Also the metrics should probably end with _total, but I have to doublecheck this...

I'm not sure about making that change a part of this PR, since it would be a breaking change.

[TheFox0x7]

I'm not sure about making that change a part of this PR, since it would be a breaking change.

Sorry, I didn't look that closely at the code and figured it's a new metric. In this case you're right, it's not worth breaking here, I've been meaning to clean this up anyway (whenever I finally set my infrastructure up to actually gather and see those...) so I'll look at doing this then.

[lunny]

How about keep the old metric as total users and added a new active users, removing the inactive users.

[TheFox0x7]

I get that would be fully backwards compatible but it feels wrong to do IMO.
gitea_users still has the total user amount here - it's just split between user states (active/inactive). Those are directly linked to the metrics itself so label. You still can get the old amount by summing.
Contrast it with gitea_users, gitea_users_inactive and the derived active users: gitea_users - gitea_users_inactive.

Also labels better show links between metrics, than having two separate ones.

[techknowlogick]

Added the "breaking" label to make sure we mention this in release notes. FWIW I think this change is beneficial and should be included. If it's not done in a backwards compatible way, that's probably ok since we have a process for alerting users.

[tobiasbp]

I get that would be fully backwards compatible but it feels wrong to do IMO.

I agree. It's much cleaner to keep the current metric and just add the label.

[tobiasbp]

Added the "breaking" label to make sure we mention this in release notes.

Adding a label is not a breaking change. If a user does not specify a label, they get the current value (the total number of user accounts).

[TheFox0x7]

I might be misremembering but don't they get both metrics separately and you need to sum them to get the total number?

[tobiasbp]

I might be misremembering but don't they get both metrics separately and you need to sum them to get the total number?

Actually, I think you and @techknowlogick are right. Adding the label is a breaking change since users would get two metrics, and would need to sum to get to the current value (as you describe).

[tobiasbp]

Do you think we need any changes made to this PR?