roadrunner-server / roadrunner

🤯 High-performance PHP application server, process manager written in Go and powered with plugins
https://docs.roadrunner.dev
MIT License
7.9k stars 412 forks source link

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

Open asanikovich opened 1 year ago

asanikovich commented 1 year ago

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).

rustatian commented 1 year ago

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.

asanikovich commented 1 year ago

@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.

rustatian commented 1 year ago

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.

asanikovich commented 1 year ago

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

rustatian commented 1 year ago

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

asanikovich commented 1 year ago

@rustatian This format will be good

asanikovich commented 1 year ago

@rustatian Hello. Any news?

rustatian commented 1 year ago

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).

mtamazlicaru commented 6 months ago

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

rustatian commented 6 months ago

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

mtamazlicaru commented 5 months ago

@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.

rustatian commented 5 months ago

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.