Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for multiple ports in annotations #3756

Open
vaijira opened this Issue Jan 29, 2018 · 20 comments

Comments

Projects
None yet
9 participants
@vaijira
Copy link

vaijira commented Jan 29, 2018

The use case is if you have for example several containers in a pod, currently with the port annotation it seems you can only specify one port. The only way i see to scrape several ports is removing the port annotation, the problem is that ports that i dont wanna get scraped are scraped.

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Jan 29, 2018

This is really a feature request for Kubernetes to have per-port labels/annotations, we can only work off the metadata that k8 provides to us.

@vaijira

This comment has been minimized.

Copy link
Author

vaijira commented Jan 29, 2018

Thanks for the fast answer, perhaps i'm saying nonsense but it wouldn't be possible to have an annotation like prometheus.io/ports: "9101,9102" in prural?

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Jan 29, 2018

Of course you can have such an annotation, Kubernetes permits it.
That's not going to be much use in Prometheus though if you're using it for what I think you're trying to use it for, which is attempting a workaround for there not being per-port annotations in Kubernetes.

@matthiasr

This comment has been minimized.

Copy link
Contributor

matthiasr commented Jan 29, 2018

@brian-brazil you're not being helpful here. Per-port annotations are one way to make this possible, a more expressive relabelling language would be another. You don't want the latter, so you're just pushing for the former, which is not going to happen either because fundamentally ports are not annotatable objects and never will be.

@vaijira There is a way to do this already, but it comes with some caveats.

Because of the deliberately limited relabelling language, you cannot support an arbitrary number of ports, but you can support ports up to some limit of your choice (up to 3 ports, up to 10 ports, any number you choose). The way you can do this is to have one job definition for each of the possible positions of ports in the list, and then for example for the 2nd port match something like [0-9]+,([0-9]+)(:?,[0-9]+)*.

The other consequence is higher load on the Kubernetes API, because Prometheus will open a separate connection and watch for each job description (there is an issue for avoiding that somewhere). For this reason, we default to supporting 1 port only, and raise this limit only for the few apps that actually need more than one.

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Jan 29, 2018

there is an issue for avoiding that somewhere

#2191

@tomwilkie

This comment has been minimized.

Copy link
Member

tomwilkie commented Feb 1, 2018

One other alternative is to key SD off of port names - for instance, we only scrape pod ports who's name ends with -metrics. This allows you to scrape multiple port per pod quite easily. Heres our Prometheus config:

https://github.com/kausalco/public/blob/master/prometheus-ksonnet/lib/prometheus-config.libsonnet#L72

@josdotso

This comment has been minimized.

Copy link

josdotso commented Mar 31, 2018

@tomwilkie This approach is brilliant. Thanks!

Defined as a second pod job to what is at kubernetes/charts, with some tweaks, I was able to achieve multi-port scrapes without annotations. The port name suffix I'm using is xp because go text/template is sensitive to hyphens, which might become an issue in some of the stock charts.

@bwplotka

This comment has been minimized.

Copy link
Contributor

bwplotka commented May 3, 2018

Yea, we use port naming as an indicator what port should be scraped for pod, but it is limited:

  • port name has limit of 15 chars
  • you need to have same scheme and __metric_path__ for all containers within pod

It's fine for us for now, but can cause some troubles if you don't have any way to change metric_path on which metrics are exposed for both containers within pod.

Started some discussion here: https://groups.google.com/forum/#!topic/prometheus-users/ihMUWtX477Q Sorry I missed this issue here, my topic is kind of duplicate.

@bwplotka

This comment has been minimized.

Copy link
Contributor

bwplotka commented May 3, 2018

There is another, more verbose explanation of solution mentioned in begginning (from google group's topic):

My post actually made me think about more explicit, but flexible solution - that we can just do annotation like in the example but with key like this:
prometheus.io/0/{port, scrape, scheme, path} and add additional scrape config for each potential sidecar. So if you have 3 containers that you want to scrape you can put in the pod appropriate annotations:
prometheus.io/0/{port, scrape, scheme, path}, prometheus.io/1/{port, scrape, scheme, path} and prometheus.io/2/{port, scrape, scheme, path} and have 3 separate scrape jobs (pods k8s SD) that scrape first, second and third container. This will not automatically work for 4-container pods, but for most users, you never have more than 3 you want to monitor.

@bwplotka

This comment has been minimized.

Copy link
Contributor

bwplotka commented Mar 5, 2019

How crazy would be to extend relabelling to add some kind of conditional logic? For example:

    - source_labels: [a, b, c, d]
      regex:         "^(.*);(x.*);(something.*);.*"
      target_label:  new_label
      replacement:  "$1-$3"
      # New relabelling field for comparison. Rule is executed only if those N args string matches.
      if:  ["eq", "$1", "$2"]

That would solve this issue immidiately and will allow for more flexibility for different providers.

Any thoughts?

So then you can have annotations like:

example.com/9090metric_path="/_metrics"
example.com/9093metric_path="/metrics"
example.com/9099metric_path="/status?format=prometheus"
@discordianfish

This comment has been minimized.

Copy link
Member

discordianfish commented Apr 4, 2019

Not tested but I think another alternative is using a service for each port and use the annotations there. But I'd also happy if we had a one-to-n mapping in relabeling.

@bwplotka

This comment has been minimized.

Copy link
Contributor

bwplotka commented Apr 5, 2019

So.. again, there is no good solution here.

Other option and it seems recommended & ultimate way is to do CRD based scrape configuration, which quite neat and insane in the same time. This is exactly what Prometheus Operator does here, however I am not necessarily like sticking to Endpoints.

I wonder if the better way would be to allow pointing to pods directly. I might be missing some important discussion that lead to decision for Endpoints. If I am not wrong, Endpoints are created only by Service that has some selectors, otherwise you need to create those manually. Selecting pods would need probably need some even more magic scrape configuration.

Anyway, all of this unfortunately involves using some kind of operator, like Prom Operator itself. Since there is no custom resource definition API in scrape Kubernetes discovery (why?), the only solution is genertaing Prometheus configuration on the fly, based on CRDs from KubeAPI and reload it in runtime to given Prometheus. It's quite insane ! Also kudos to Prometheus Operator maintainers for implementing this and delivering so well.

All of this would be not needed if the scrape configuration would be improved in Prometheus itself ):

@discordianfish

This comment has been minimized.

Copy link
Member

discordianfish commented Apr 6, 2019

@bwplotka Prometheus discovers pods, but what I and @vaijira want is finding multiple targets per single pod. That's what isn't possible because there is only one target in SD (the pod) and no way to create two targets from one target with relabeling.

@bwplotka

This comment has been minimized.

Copy link
Contributor

bwplotka commented Apr 7, 2019

@discordianfish You are missing important thing here. Prometheus discovers Pod's ports not just pods. However because of:

  • pattern for reusing Pod annotations and limitation of relabel I described in 4 comments above
  • lack of port annotations for Kubernetes

.. Prometheus discovery gives false impression of just one target/port per pod, as annotation happend to be (accidently) a popular pattern.

There is ongoing discussion for years how to resolve this in easiest way and I think we should start maintain some table of potential solutions and their pros/cons ;p

@bwplotka

This comment has been minimized.

Copy link
Contributor

bwplotka commented Apr 7, 2019

@pbenefice

This comment has been minimized.

Copy link

pbenefice commented Apr 16, 2019

Hi,

I'm also facing the need to scrape two container within a single pod and gave a shot at the "Multiple Pod annotations" solution within the google doc

I though I'd share my issues :

  • This is minor but I think the prometheus.io/0/{port, scrape, scheme, path} won't work as is (see here).

    Valid annotation keys have two segments: an optional prefix and name, separated by a slash
    Instead, maybe we could go for :

    0.prometheus.io/{port, scrape, scheme, path}
    1.prometheus.io/{port, scrape, scheme, path}
    
  • Unless I'm wrong this solution imply using the prometheus deduplication of exact same targets. When I use for example :

    #Annotations in kubernetes (within a StatefulSet exposing two container, and one port per container) : 
    0.prometheus.io/scrape: "true"
    0.prometheus.io/port: "9090"
    0.prometheus.io/path: "/metrics"
    1.prometheus.io/scrape: "true"
    1.prometheus.io/port: "8080"
    1.prometheus.io/path: "/metrics"
    
    #jobs definitions :
    - job_name: "kubernetes-containers-0"
        kubernetes_sd_configs:
        - role: pod
        relabel_configs:
        - source_labels: [__meta_kubernetes_pod_annotation_0_prometheus_io_path]
        action: replace
        target_label: __metrics_path__
        regex: (.+)
        - source_labels: [__address__, __meta_kubernetes_pod_annotation_0_prometheus_io_port]
        action: replace
        regex: ([^:]+)(?::\d+)?;(\d+)
        replacement: $1:$2
        target_label: __address__
    
    - job_name: "kubernetes-containers-1"
        kubernetes_sd_configs:
        - role: pod
        relabel_configs:
        - source_labels: [__meta_kubernetes_pod_annotation_1_prometheus_io_path]
        action: replace
        target_label: __metrics_path__
        regex: (.+)
        - source_labels: [__address__, __meta_kubernetes_pod_annotation_1_prometheus_io_port]
        action: replace
        regex: ([^:]+)(?::\d+)?;(\d+)
        replacement: $1:$2
        target_label: __address__
    

    Unless I'm mistaking, prometheus discover two targets for each jobs, but the two targets are exactly the same.
    Therefore Prometheus keeps only one target for each job and I end up "well"
    But, except for the job name, the labels will be identical for the two series, I would have wanted to add the __meta_kubernetes_pod_container_name with relabelling,
    but if I do so I lose the unicity of labels and end up with duplicated targets : prometheus won't deduplicate those anymore.

    One more downside to this solution is that you can't have labels that differ (other than the job name) from a container to another, in my case the name.

@pbenefice

This comment has been minimized.

Copy link

pbenefice commented Apr 16, 2019

Hi @bwplotka, (related to my comment just above)

I don't know if this idea is similar to the if you already mentionned but how crazy would it be to be able to use labels within the regex ?
In my case I guess it would solve the thing :

#Annotations in kubernetes (within a StatefulSet exposing two container, and one port per container) : 
0.prometheus.io/port: "9090"
0.prometheus.io/path: "/metrics"
1.prometheus.io/port: "8080"
1.prometheus.io/path: "/metrics"

#job definition :
- job_name: "kubernetes-containers-0"
    kubernetes_sd_configs:
    - role: pod
    relabel_configs:
    - source_labels: [__meta_kubernetes_pod_container_port_number]
    action: keep
    regex: __meta_kubernetes_pod_annotation_0_prometheus_io_scrape

- job_name: "kubernetes-containers-1"
    kubernetes_sd_configs:
    - role: pod
    relabel_configs:
    - source_labels: [__meta_kubernetes_pod_container_port_number]
    action: keep
    regex: __meta_kubernetes_pod_annotation_1_prometheus_io_scrape

The idea here would be : "If the port defined within the annotations match the port of the pod discovered : keep the target, otherwise drop it"

@bwplotka

This comment has been minimized.

Copy link
Contributor

bwplotka commented Apr 16, 2019

Unless I'm mistaking, prometheus discover two targets for each jobs, but the two targets are exactly the same.

Why? If you target address you will get two targets per JOB (if you have single Pod with only 2 containers). Since the address is exactly the same K8s service will use just one.

would have wanted to add the __meta_kubernetes_pod_container_name with relabelling,
but if I do so I lose the unicity of labels and end up with duplicated targets : prometheus won't deduplicate those anymore.

Yes! You have couple of options, I will add them to the doc. Also please comment there.

  • Add instance label with ip:port
  • (Better): map job to be app+container_name
@bwplotka

This comment has been minimized.

Copy link
Contributor

bwplotka commented Apr 16, 2019

Your second idea is kind of if -> relabelling does not allow to use regex build from values from another labels. Regexp is build on config reload time.

@pbenefice

This comment has been minimized.

Copy link

pbenefice commented Apr 16, 2019

Your second idea is kind of if -> relabelling does not allow to use regex build from values from another labels. Regexp is build on config reload time.

I did'nt expect that quick of answer, but yeah, I know it's not possible, I was just saying this feature would solve my case, as the Port Annotations from kubernetes would solve it, but I understand it's not available. (for now ? 😜 )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.