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

adding a description/comment for each target #35

Open
lodpp opened this issue Mar 13, 2020 · 16 comments
Open

adding a description/comment for each target #35

lodpp opened this issue Mar 13, 2020 · 16 comments

Comments

@lodpp
Copy link

lodpp commented Mar 13, 2020

Hello,

Would it be possible to be able to add a description/free form field for each target ?

On top of getting the graph of latency/jitter for a given target, you can add a target description to give more information about that target.
Then it's easier to search for a given keyword in the description. Eventually even better if you can add other fields.

Here are 2 use cases on how I use the original smokeping:

  • For QOS to certain destination, I monitor remote ips or hostname (not in my ASN). I also add as a description the source AS for that IP, eventually the geographical zone / origin country.
    ip/hostname does not speak to me when I want to check quality to a given ASN, but the ASN itself or the location does.

I've added a screenshot for that one
smokeping

  • For internal monitoring, on top of the target ip/hostname, I add a description of the service behind it. Loadbalancer VIP1 DC1 is easier to remember than an IP.

The goal is to get that information as labels in prometheus, which can be showed on graph legend or set as variables.

I don't think of an easy way to do this right now, as you need to specify the target list directly when executing the exporter.
A config file might be more appropriate for that purpose.

Does this feature request would make sense for the project ?

Thanks,

@SuperQ
Copy link
Owner

SuperQ commented Mar 13, 2020

Yes, this might be an ok thing to do, given that this prober is a little unusual, and the typical service discovery recommendations don't quite fit.

I've been wanting to enhance the control of probe targets by implementing a yaml config file. I've been a bit stuck behind one upstream ping feature that would allow v4 vs v6. The upstream library I've been using has mostly died, and I'm thinking about making a breaking fork.

What do you think of something like this:

targets:
- name: Loadbalancer VIP1 DC1 v4
  target: foo.example.com
  proto: v4
- name: Loadbalancer VIP1 DC1 v6
  target: foo.example.com
  proto: v6

@lodpp
Copy link
Author

lodpp commented Mar 13, 2020

That would be okay to me :)

my 2cents on the config file ( but it's a separate feature request of course):

  • being able to assign a probe to a group: right now to organize groups of targets, I use one prober ( separate listening port) per group and I filter them in grafana based on the job label name

@SuperQ
Copy link
Owner

SuperQ commented Mar 13, 2020

Hrm, I'm not sure probe groups make sense. The individual probes are completely independent threads. They only group by per prober process.

If you want groups, it's pretty easy to spin up multiple probers and assign them group labels in Prometheus.

@lodpp
Copy link
Author

lodpp commented Mar 13, 2020

Yeah I did it based on the job name, but what you propose is also valid.

@cleonello
Copy link

Would be great if you could implement the proposed config file. I'm running smokeping_prober now but only in a very limited context. I would like to replace blackbox exporter with smokeping prober but can only do that if:

  1. Hosts could be defined in a config file.
  2. Custom labels could be added to hosts in the config file (we add a custom unique name to targets for identification).

@SuperQ
Copy link
Owner

SuperQ commented Dec 14, 2020

Here's a slight variation on the proposal:

targets:
- host: foo.example.com
  network: ip4
  protocol: icmp
  labels:
    name: Loadbalancer VIP1 DC1 v4
- host: foo.example.com
  network: ip6
  protocol: udp
  labels:
    name: Loadbalancer VIP1 DC1 v6
- host: 10.0.0.1
  network: ip # Automatic ip4/ip6
  # protocol: icmp # Defaults to ICMP

@SuperQ
Copy link
Owner

SuperQ commented May 8, 2021

I started working on this and realized we need to be a bit careful about how we add this.

The main problem is that the list of label names needs to be consistent for all active pingers in order to not cause problems with Prometheus/OpenMetrics parsing.

For example, this would be invalid output:

smokeping_requests_total{target="foo",description="Some Servcie"} 0
smokeping_requests_total{target="bar"} 0

In order to fix this, we would need to make the label names consistent:

smokeping_requests_total{target="foo",description="Some Servcie"} 0
smokeping_requests_total{target="bar",description=""} 0

The good news is Prometheus treats empty string label as if no label exists. So there's no problem on the query side.

@lodpp
Copy link
Author

lodpp commented May 10, 2021

Thanks for working on it :)
If a description is unset by the user, either set it to an empty string as you did, or eventuallly set the target name as description ?
I'm just thinking out loud that it might be easier to show the description in graphana if {{ description }} is always set to something.

@joelsdc
Copy link

joelsdc commented Jan 2, 2022

Hello, any updates? I've seen the config file is now available since 0.5.0 but labels/comments/descriptions still WIP?

@SuperQ
Copy link
Owner

SuperQ commented Jan 2, 2022

@joelsdc PRs welcome.

@joelsdc
Copy link

joelsdc commented Jan 2, 2022

I'll give it a try. Do you have a branch or anywhere were you started your work for this?

@SuperQ
Copy link
Owner

SuperQ commented Jan 2, 2022

No, I just added the config, additional config features I haven't started yet.

@SuperQ
Copy link
Owner

SuperQ commented Jan 2, 2022

One thing to note about this. Since metrics must all have the same set of labels, it will require parsing the config before doing anything. It's probably going to require completely refactoring collector.go so that the metrics are declared after the config is parsed.

@kzxuan
Copy link

kzxuan commented Mar 10, 2022

Hello, it looks like a great function. In your plan, when will it be available for use?

@manigarg31
Copy link

Hello, it looks like a great function. In your plan, when will it be available for use?

Hey, is there any update on this?

@Nachtfalkeaw
Copy link

Hello,

for my environment I use a workaround and add an additional label to ech metric:

smokeping_prober.yml

targets:

#======================================================================================================================================================

- hosts:

  ### target_group: LAN-GW
  - 192.168.10.1
  - 192.168.178.1
  ### taget_group: Public-DNS
  - 8.8.8.8      # google DNS
  - 8.8.4.4      # google DNS
  - 1.1.1.1      # cloudflare DNS
  - 9.9.9.9      # quad9 DNS
  - 209.244.0.3  # level 3 DNS
  - 209.244.0.4  # level 3 DNS

  interval: 0.2s # Duration, Default 1s.
  network: ip4 # One of ip, ip4, ip6. Default: ip (automatic IPv4/IPv6)
  protocol: icmp # One of icmp, udp. Default: icmp (Requires privileged operation)
  size: 24 # Packet data size in bytes. Default 56 (Range: 24 - 65535)
  source: 192.168.10.51 # Souce IP address to use. Default: None (automatic selection)

#======================================================================================================================================================

And my prometheus.yml looks like this. I add an additional label based on [host].

scrape_configs:

#======================================================================================================================================================

  - job_name: 'smokeping_prober'
    scheme: https

    tls_config:
      ca_file: "/opt/prometheus/certs/bb.crt"
      cert_file: "/opt/prometheus/certs/bb.crt"
      key_file: "/opt/prometheus/certs/bb.key"
      insecure_skip_verify: false

    scrape_interval: 5s
    scrape_timeout: 5s
    static_configs:
      - targets:
        - 192.168.10.51:9374

    ### this grouping needs to happen manually. [host] is based on the value from smokeping_prober.yml
    metric_relabel_configs:
      ### only used to add all metrics the new label and a value
      - source_labels: [host]
        regex: '.*'
        replacement: "no_group"
        target_label: target_group
      ### group #1 - LAN-GW
      - source_labels: [host]
        regex: '.*(192\.168\.10\.1|192\.168\.178\.1)'
        replacement: "LAN-GW"
        target_label: target_group
      ### group #2 - Public-DNS
      - source_labels: [host]
        regex: '.*(8\.8\.8\.8|8\.8\.4\.4|1\.1\.1\.1|9\.9\.9\.9|209\.244\.0\.3|209\.244\.0\.4)'
        replacement: "Public-DNS"
        target_label: target_group

#======================================================================================================================================================

My intention was to group some IP addresses. This is my home environment and I just want to ping some gateways and some public DNS servers to compare latency. So I added a new label "target_group" and the value is the group which is something to help me indicate which IP belongs to which.

Unfortunately this is manual work, however it is a simple workaround and at work I have around 20 groups with each 3-4 IPs and not a big problem to add a new group if needed.

However this workaround is probabaly no help if you have hundreds of targets and if these are changing dynamically.

At least - my opinion - we need an additional, independent identifier for the metrik which is NOT the IP address or DNS name because at the moment it is not possible to address the same target with different parameters. e.g. ping 8.8.8.8 every 0.2s with 24byte size and ping every 5s with 1024byte size. The only possibility is to start smokeping two times with the additional overhead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants