vaerh / mikrotik-prom-exporter

0 stars 0 forks source link

Prometheus listener #1

Closed jlpedrosa closed 3 months ago

jlpedrosa commented 3 months ago

I will add explanations inline with the code as comments.

After this PR when running curl http://localhost:8080/metrics

# HELP ethernet_rx_bytes_counter Number of bytes received in the interface
# TYPE ethernet_rx_bytes_counter counter
ethernet_rx_bytes_counter{interface_name="core-link-1",interface_type="ethernet"} 3.171947205e+10
ethernet_rx_bytes_counter{interface_name="core-link-2",interface_type="ethernet"} 1.8312768006e+10
ethernet_rx_bytes_counter{interface_name="ether1",interface_type="ethernet"} 0
ethernet_rx_bytes_counter{interface_name="fast-poe-link-1",interface_type="ethernet"} 3.747857694e+09
ethernet_rx_bytes_counter{interface_name="fast-poe-link-2",interface_type="ethernet"} 4.140803823e+09
ethernet_rx_bytes_counter{interface_name="ont-link",interface_type="ethernet"} 2.30514633367e+11
ethernet_rx_bytes_counter{interface_name="sfp-sfpplus2",interface_type="ethernet"} 0
ethernet_rx_bytes_counter{interface_name="sfp-sfpplus3",interface_type="ethernet"} 0
ethernet_rx_bytes_counter{interface_name="sfp-sfpplus4",interface_type="ethernet"} 0
ethernet_rx_bytes_counter{interface_name="sfp-sfpplus5",interface_type="ethernet"} 0
ethernet_rx_bytes_counter{interface_name="sfp-sfpplus6",interface_type="ethernet"} 0
ethernet_rx_bytes_counter{interface_name="sfp28-1",interface_type="ethernet"} 0
ethernet_rx_bytes_counter{interface_name="sfp28-2",interface_type="ethernet"} 0
ethernet_rx_bytes_counter{interface_name="slow-poe-link-1",interface_type="ethernet"} 1.38348560765e+11
ethernet_rx_bytes_counter{interface_name="slow-poe-link-2",interface_type="ethernet"} 1.09458891241e+11
vaerh commented 3 months ago

I've sat and thought about what a schema should look like for a resource and looked at the changes and here's what I want to say: it seems to me that in order to reduce the time it takes to collect metrics, we need to extract the maximum number of values from a single MT query. For this reason, I would describe as many metrics as possible in one schema. The quality of the file reading will drop a bit, but it can be tolerated. Perhaps it is necessary to divide simple metrics by types registered by promauto and describe them in some way like namespace_subsystem_name, mikrotik_field, labels, data type?! (for type conversion). For composite types like mktxp bgp, pool, route, etc. we will have to do inline processing.

jlpedrosa commented 3 months ago

Yeah, I agree, that is why I was proposing something like this:

type ResourceMetric struct {
   name, labels, filter...
}

type Resource struct {
   MikrotikPath string (/interface/ethernet)
   Metrics []ResourceMetrics
}

So we query one and expose all the metrics related to that resource. so for instance:

 path: /interface/ethernet
 metrics:
  - name: interface_total_received_bytes_counter
    constLabels:
       interface_type: "ethernet"
     labelNamesFields:
       speed: "speed"
     metricValue: "rx-frames" 
  - name: interface_total_sent_bytes_counter
    constLabels:
       interface_type: "ethernet"
     labelNamesFields:
       speed: "speed"
     metricValue: "tx-frames"  
vaerh commented 3 months ago

I think we need to get rid of duplicate descriptions or make them compact. I will try to represent it on the current branch.

jlpedrosa commented 3 months ago

I think we need to get rid of duplicate descriptions or make them compact. I will try to represent it on the current branch.

We can add constLabel and labels at root level too, that makes sense IMO.

vaerh commented 3 months ago

Yep, that's exactly what I'm doing!

vaerh commented 3 months ago

https://github.com/vaerh/mikrotik-prom-exporter/tree/poc2

vaerh commented 3 months ago

By the way, I also wanted to ask a question about using ConstLabel, the documentation recommends to do everything with regular labels. Is it reasonable to use constant labels in our case?

jlpedrosa commented 3 months ago

By the way, I also wanted to ask a question about using ConstLabel, the documentation recommends to do everything with regular labels. Is it reasonable to use constant labels in our case?

Humm, didn't saw that in the documentation. But I think we kind of need it, we may or may not use the API from prom, but I think we will need it, as in the example "interface_type" feels like a very natural label to have.

Also, heads up, I added the firts one as a counter, but it's not actually a counter, because Mikrotik already computes the totals, so either we use a different API in prom to take control of the total value or we use a gaguge.

That API (counter) is supposed to be used as, I am processing a request, I add 1 to the counter and the prom exposes the full counter. But in this case we are constantly adding the total so the metric does not makes sense.

Also, we will need to use different type of metric types at some point we need to add that configuration option per metric. I am not saying we have to do it in this PR, it's more a heads up.

vaerh commented 3 months ago

Splitting the tags is not a problem.

I didn't bother with the Prometheus data type yesterday and used the one in the example. I need to update the API information, it has changed a lot since the last time I created an exporter for Prometheus.

I have provided the difference in metrics type in poc2, you can see it in the yaml file. The only thing I don't remember and didn't have time to look at is whether each metric always corresponds to the same type of input data like float64? It seems not...

vaerh commented 3 months ago

I found this section of the documentation. But I can't interpret it properly yet, unfortunately.

// ConstLabels are used to attach fixed labels to this metric. Metrics // with the same fully-qualified name must have the same label names in // their ConstLabels. // // ConstLabels are only used rarely. In particular, do not use them to // attach the same labels to all your metrics. Those use cases are // better covered by target labels set by the scraping Prometheus // server, or by one specific metric (e.g. a build_info or a // machine_role metric). See also // https://prometheus.io/docs/instrumenting/writing_exporters/#target-labels-not-static-scraped-labels

jlpedrosa commented 3 months ago

I found this section of the documentation. But I can't interpret it properly yet, unfortunately.

// ConstLabels are used to attach fixed labels to this metric. Metrics // with the same fully-qualified name must have the same label names in // their ConstLabels. // // ConstLabels are only used rarely. In particular, do not use them to // attach the same labels to all your metrics. Those use cases are // better covered by target labels set by the scraping Prometheus // server, or by one specific metric (e.g. a build_info or a // machine_role metric). See also // https://prometheus.io/docs/instrumenting/writing_exporters/#target-labels-not-static-scraped-labels

This means that you need to add a label to all the metrics, there are betters way to do it. Imagine "router-brand" mikrotik. For the other examples this is good.

I think we should merge this PR and then open another PR from your branch. SGTY?

vaerh commented 3 months ago

Sure. You're home, there are no restrictions on mergers and commits. Do it!

I still don't understand the advantage of static labels, except that Prometheus uses them to create unique time series and aggregate them into collectors. In this case, it is probably specifically about collecting labels into a separate collector. Somewhere I saw a recommendation not to overuse the number of labels.