roadrunner icon indicating copy to clipboard operation
roadrunner copied to clipboard

[💡 FEATURE REQUEST]: Add default labels for rr metrics

Open asanikovich opened this issue 2 years ago • 12 comments

Plugin

Metrics

I have an idea!

I have an idea, listen to me!! Currently difficultly to distinguish metrics with many rr pods. (e.x. in K8s). Add to config parameter 'metrics' list of labels:

metrics:
  labels:
    - env: "APP_ENV"
    - app: "APP_NAME"

Format - label_name: ENV_NAME (value will be read from ENV variable)

Which should be added to all default metrics by all plugins described at https://roadrunner.dev/docs/lab-metrics/current/en For custom application metrics - labels should be defined by user (as is).

asanikovich avatar Jun 28 '23 11:06 asanikovich

Hey @asanikovich 👋🏻 You can add a pod name (or IP address) to the specific metric in the dashboard. Our dashboards are examples of all possible RR metrics.

Custom metrics you are able to declare via configuration or directly from the PHP worker.

rustatian avatar Jun 28 '23 21:06 rustatian

@rustatian The best practice is not to define some extra parameters in Prometheus. Each metric should contain all the needed information.

In the case of running RR in k8s in many pods, we have next metrics:

{__name__="rr_http_uptime_seconds", instance="main-1.svc.cluster.local:8088", job="dev-main"}
{__name__="rr_http_uptime_seconds", instance="main-2.svc.cluster.local:8088", job="dev-main"}

As you see it is difficult to distinguish metrics. Yeah, we can do it by instance label - but it is not so elegant way in case we want to have a filter (main-1 or main-2), because we need to parse extra labels from the instance label.

asanikovich avatar Jun 29 '23 14:06 asanikovich

The best practice is not to define some extra parameters in Prometheus.

I don't know actually about what practices you are talking about. Would be great to see link to them.

If you need to use these metrics in Grafana for example, this is as easy as adding {instance=~"$instance"} to the metric. So, every metric will contain needed field.

rustatian avatar Jun 29 '23 14:06 rustatian

@rustatian We have mane many pods with RR and thats why some labels will help to distinguish them.

asanikovich avatar Jun 29 '23 14:06 asanikovich

Let's see the community respond on that. Because personally I'm not sure, that this case should be solved on the RR side. However, few notes about the proposal:

metrics:
  labels:
    - env: "APP_ENV"
    - app: "APP_NAME"

You may specify env variables in the configuration w/o specifying the env key. Like that for example:

metrics:
  labels:
    - env: ${SOME_ENV:-default_value}
    - app: ${SECOND_ENV}

:- this is funny, but official syntax for the default parameter expansion (supported by RR as well): https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter-Expansion.html

rustatian avatar Jun 29 '23 14:06 rustatian

@rustatian This format will be good

asanikovich avatar Jun 29 '23 14:06 asanikovich

@rustatian Hello. Any news?

asanikovich avatar Sep 13 '23 17:09 asanikovich

Hey @asanikovich 👋🏻 Unfortunately no one is interested in this feature (0 upvotes). I have moved it to low priority. But, as always, I would be happy to review (or help with) a PR 😃

  1. Labels are defined here: https://github.com/roadrunner-server/metrics/blob/master/config.go#L54C5-L54C5. They are defined per-collector: https://github.com/roadrunner-server/metrics/blob/master/config.go#L27 (you need to choose the right one, I guess you need a counter)
  2. Then, you need to validate those labels here: https://github.com/roadrunner-server/metrics/blob/master/config.go#L69. By check I mean register. But before, since your proposal about env variables, you have to expand these envs: https://github.com/roadrunner-server/config/blob/master/expand.go
  3. Final step - register your metric.

P.S: Since the expand algorithm should be used in two places, to DRY, I suggest you to create a folder named expand in our SDK: https://github.com/roadrunner-server/sdk and put the algorithm in it (also move it from the config).

rustatian avatar Sep 13 '23 19:09 rustatian

Hi @rustatian. I would be interested to help with this feature. Is the feature still relevant?

mtamazlicaru avatar Apr 21 '24 15:04 mtamazlicaru

Hey @mtamazlicaru 👋 Definitely, if you need any help from my side, feel free to ask (here or on our Discord server).

rustatian avatar Apr 21 '24 15:04 rustatian

@rustatian I have a question regarding first point from https://github.com/roadrunner-server/roadrunner/issues/1632#issuecomment-1718178118, how I understand it labels that are mentioned there are per application metrics.

Since the suggestion was to have generic labels for all metrics, I think labels should be added in this struct.

What is your opinion on this? Thank you.

mtamazlicaru avatar May 04 '24 08:05 mtamazlicaru

Hey @mtamazlicaru 👋 Yes, they should be added as a top-level key in the Config structure and then spread across all registered Collectors. Keep in mind, that the user can register metrics in runtime via RPC.

rustatian avatar May 04 '24 08:05 rustatian