Repository: minio/minio (language: go)

Issue:
MINIO_PROMETHEUS_JOB_ID is insufficient when scraping metrics via prometheus-operator when having more minio instances
**Is your feature request related to a problem? Please describe.**
Right now minio exposes metrics and is possible to scrape it with prometheus with servicemonitor etc.
The problem is that currently minio is using only MINIO_PROMETHEUS_JOB_ID to filter out metrics, which is extremely limiting when using prometheus operator and minio in kubernetes with more than 1 minio installation.
Prometheus operator + ServiceMonitor will cause jobs to be names as service is named on deployment.

Let say we have 2 namespaces (lets say bravo and charlie), with the same minio installs in each namespace.
As a result we have a services minio.bravo.svc and minio.charlie.svc
Now we need to configure minio to use metrics from prometheus, but because how prometheus operator and servicemonitors are created in both cases job name will be 'minio'.
Now minio querying for the metric in the UI will get metrics fro BOTH installations.

What is missing is extra option to pass additional labels to filter out metrics.


**Describe the solution you'd like**
The best option would be to add another env var to pass additional labels to be used when querying prometheus.
Something like MINIO_PROMETHEUS_LABEL_FILTER so I could add `namespace="bravo"` and `namespace="charlie"` for each individual config of the minio. This way they could use it to fetch metrics only for specific instance.
This would also allow to pass fine tuned labels such as `namespace="bravo",cluster="echo"` to fine grain metric even more.

**Describe alternatives you've considered**
- what works now is to actually exploit the way metrics are queried, so that injecting desired labels occurs, in example setting MINIO_PROMETHEUS_JOB_ID to `minio",namespace="beta` and `minio",namespace="charlie` will in effect allow to query with labels `job="minio",namespace="beta"` and `job="minio",namespace="charlie"`, but it's dirty ;)

- differently named minio installs, but that tend to be problematic in certain situations
- some weird metric label renaming which ends in ugly hacks
- prometheus-operator provides `additionalScrapeConfigs` to load extra snippets to prometheus to scrape, the problem is that user may not be able to control prometheus installation for that. There is also ongoing process of defining new CRD for that case https://github.com/prometheus-operator/prometheus-operator/issues/2787 but it will probably take a lot of time before it will be available.


**Additional context**




Discussion / hints:
looks like it should go to minio/console somhwere around admin info api https://github.com/minio/console/blob/5f281518fcc0cc3040e805ca9b42640d59645b7d/restapi/admin_info.go#L192
sorry this issue should rather be in minio/minio (not operator), because we do not use minio operator.
@nvtkaszpir could you please clarify. are you not using Prometheus operator or minio-operator? If you can provide more details about your setup, it will help us.
we are using:
- minio from helm chart
- prometheus from prometheus-operator (via helm chart kube-prometheus-stack)
@nvtkaszpir Using ServiceMonitor to create the prometheus config for MinIO could lead to duplicate metrics. When ServiceMonitor is used, prometheus-operator will create one endpoint per pod, and prometheus will scrape them individually. This could lead to duplicate metrics. MinIO exposes cluster level metrics, it should collected only from the service not directly from the individual pods. If you are using Grafana or similar tools, you might see some issues with the metrics values when ServiceMonitor is used. This is the main reason for us to remove the ServiceMonitor support from minio-operator and use AdditionalScrapeConfig (https://github.com/minio/operator/pull/920). 

@nvtkaszpir closing as explained by @kanagarajkm 
@harshavardhana @kanagarajkm This still does not solve the main issue described in the first post.
It's not just about servicemonitor per se, but the way how minio is requesting metrics from prometheus.

I'm very aware of the issue with servicemonitor and metric duplication.
@nvtkaszpir if we don't use the ServiceMonitor, we can use different `job_name` for the minio deployments and the MinIO console should be able to filter the metrics. do you see any issues with this?
> @harshavardhana @kanagarajkm This still does not solve the main issue described in the first post.
> It's not just about servicemonitor per se, but the way how minio is requesting metrics from prometheus.
> 
> I'm very aware of the issue with servicemonitor and metric duplication.

All we need is a separate JOB_ID, nothing more. I do not know of any other way to differentiate.
Just job_id is not enough in certain configurations, that's what I am trying to say. There needs to be option to pass additional labels for more fine-grained filtering.
> Just job_id is not enough in certain configurations, that's what I am trying to say. There needs to be option to pass additional labels for more fine-grained filtering.

- JOB_ID is what we know that made sense, can you say what else is needed? and also why not have a different job_id() for each scraping instance?

- Just like what @kanagarajkm mentioned you cannot scrape all endpoints from MinIO in a cluster. If this is what you are doing it's not correct - You scrape only one endpoint i.e a central ClusterIP or Ingress LB. 


 
1. we have a metrics which are not controlled by us (and trust me we really tried to talk to them to change job ids), each deployment per cluster generates the same job names but adds additional label 'cluster_name', thus when this is aggregated in the one place it is possible to differentiate metrcis based on that additional label.

That's what I want - to be able to pass additional labels in minio settings as metric selector so that it can fine-grain metrics which are specific to that cluster, just as described in the first issue.

The best option would be to leave MINIO_PROMETHEUS_JOB_ID as is and make MINIO_PROMETHEUS_EXTRA_LABELS as an additional labels selector, this way it could be empty by default:
```shell
MINIO_PROMETHEUS_JOB_ID="minio-job"
MINIO_PROMETHEUS_EXTRA_LABELS=""
```
which would make prometheus query equal
```promql
{job="minio-job"}
```

And with customization it would allow something like this:
```shell
MINIO_PROMETHEUS_JOB_ID="minio-job"
MINIO_PROMETHEUS_EXTRA_LABELS="cluster_name='cluster-eu'"
```
which would make prometheus query equal
```promql
{job="minio-job",cluster_name="cluster-eu"}
```

Of course there would be a need to add simple logic to add a comma to the query if MINIO_PROMETHEUS_EXTRA_LABELS is not empty.

Another option is to replace MINIO_PROMETHEUS_JOB_ID with MINIO_PROMETHEUS_LABEL_SELECTOR, where it would be possible to write something like
```shell
MINIO_PROMETHEUS_LABEL_SELECTOR="job='minio',cluster_name='cluster-eu'"
```

For backward compatibility it would be good enough to set it to
```shell
MINIO_PROMETHEUS_LABEL_SELECTOR="job='minio-job'"
```
but I guess sometime breaking backward compatibility is a problem.


2. I know, we scrape data from the cluster loadbalancer so the metrics are okay and not duplicated, it's not servicemonitor as it is.
We can provide a more automated way here by adding `deployment-id` which is unique per deployment @nvtkaszpir and that's a string. like

> MINIO_PROMETHEUS_JOB_ID="minio-job"

We can't change this anymore, it's breaking if we change the meaning here 100's of customer deployments would have issues. 

> ```shell
> MINIO_PROMETHEUS_EXTRA_LABELS=""
> ```

We could add this extend this to automatically add "deployment-id" @nvtkaszpir 


> We could add this extend this to automatically add "deployment-id"
that sounds as really usable!

Fix patch:
diff --git a/cmd/common-main.go b/cmd/common-main.go
index debd7bcdbb4d7..5bdbab2d9723c 100644
--- a/cmd/common-main.go
+++ b/cmd/common-main.go
@@ -190,6 +190,10 @@ func minioConfigToConsoleFeatures() {
 		os.Setenv("CONSOLE_PROMETHEUS_URL", value)
 		if value := env.Get("MINIO_PROMETHEUS_JOB_ID", "minio-job"); value != "" {
 			os.Setenv("CONSOLE_PROMETHEUS_JOB_ID", value)
+			// Support additional labels for more granular filtering.
+			if value := env.Get("MINIO_PROMETHEUS_EXTRA_LABELS", ""); value != "" {
+				os.Setenv("CONSOLE_PROMETHEUS_EXTRA_LABELS", value)
+			}
 		}
 	}
 	// Enable if LDAP is enabled.


Test patch: