Add Monitor group name to metric labels

mhkarimi1383 commented 1 year ago

⚠️ Please verify that this feature request has NOT been suggested before.

[X] I checked and didn't find similar feature request

🏷️ Feature Request Type

API, Other

🔖 Feature description

Hi

Making monitors group name available in metric labels will be great when creating dashboads in Grafana

✔️ Solution

Add a label named group in metrics

❓ Alternatives

No response

📝 Additional Context

No response

Craftoncu commented 8 months ago

+1

Craftoncu commented 8 months ago

+1, not a spam. This is essentially for managing the data inside of grafana to build feature-rich dashboards

CommanderStorm commented 8 months ago

+1, not a spam

Disagree. +1 on issues just makes issue-management harder. Issues are for discussing what needs to be done how by whom. You can use 👍🏻 on issues to show your support instead, as always: Pull Requests welcome.

CommanderStorm commented 8 months ago

@mhkarimi1383 It is not clear to me how this feature should work. A monitor can be in multiple groups ⇒ how should this be labelled?

mhkarimi1383 commented 8 months ago

@CommanderStorm As long as we are not having a lot of group names we could have multiple metrics for the same monitor since, I know It's not recommended since it could make a lot of metrics and it's not good for Prometheus

But we could make this feature configurable and disable it by default

CommanderStorm commented 8 months ago

we could have multiple metrics for the same monitor since

You are not making a lot of sense to me. What do you mean with this? How would you need this be labelled concretely?

mhkarimi1383 commented 8 months ago

we could have multiple metrics for the same monitor since

You are not making a lot of sense to me. What do you mean with this? How would you need this be labelled concretely?

We are making SLA dashboards with the help of Prometheus metrics, but we are not able to categorize things, because we are not having access to group or tags from there

CommanderStorm commented 8 months ago

That is not what I asked. I asked how the metrics would need to be labeled concretely.

Our group monitors are hierarchical (a group can contain a group), while Prometheus (as far as I can tell only supports Key/value pairs.

Craftoncu commented 8 months ago

Its not about wrapping objects. Its about adding new information to the single line. I think this should be the /metrics output.

- monitor_response_time{monitor_name="UPTIME REFERENCE GOOGLE.COM",monitor_type="http",monitor_url="https://google.com",monitor_hostname="null",monitor_port="null"} 200
+ monitor_response_time{monitor_name="UPTIME REFERENCE GOOGLE.COM",monitor_type="http",monitor_url="https://google.com",monitor_hostname="null",monitor_port="null",monitor_parent="**THIS_IS_THE_PARENT_NAME**"} 200

Craftoncu commented 8 months ago

Its just a new kv pair inside the json object.

CommanderStorm commented 8 months ago

What would you put under **THIS_IS_THE_PARENT_NAME** exactly?

I think we are unclear about prerequesits. Are you aware that groups can contain groups?

Group A
|--> Monitor B
|--> Group B
        |--> Monitor C

mhkarimi1383 commented 8 months ago

Are you aware that groups can contain groups?
Group A
|--> Monitor B
|--> Group B
        |--> Monitor C

Yap I think that's Ok, we are having groups as monitors and they can be parents and/or children for others

CommanderStorm commented 8 months ago

Only selectively answering questions is quite painfull.

What would you put under **THIS_IS_THE_PARENT_NAME** exactly?

Craftoncu commented 8 months ago

Jup, Im aware of that. What about chaining these? Like GroupA/Group/B

There is the opportunity to build complex selection strategy in Grafana. But for this the groups should be available

Craftoncu commented 7 months ago

Are there any news about it? I tried to implement the changes yesterday in my lunch, but failed.

CommanderStorm commented 7 months ago

What about chaining these? Like GroupA/Group/B

See https://github.com/louislam/uptime-kuma/pull/4472

I don't know what is the best-practice in this situation

Are there any news about it

Please have a look at https://github.com/louislam/uptime-kuma/pull/898 for a related PR which had a prerequesite merged recently.

CommanderStorm commented 1 month ago

Thinking about it: Maybe replacing the individual group names with .replace("/", "|") and then joining with / could work. If someone wants to invest the time to add this feature, the code for this is avaliable here

louislam / uptime-kuma