feat(probe): Fortiswitch port stats #219
Conversation
|
@Jurka007 can you rebase and fix the check errors? |
Jurka007
force-pushed
the
fortiswitch_port_stats
branch
from
4d6c403
to
9d0a753
Compare
| * `fortiswitch_port_receive_drops_total` | ||
| * `fortiswitch_port_receive_oversized_packets_total` | ||
| * _SwitchController/ManageSwitch/Health_ | ||
| * `fortiswitch_health_summary_cpu` |
There was a problem hiding this comment.
- Avoid using words like mem when memory works
- These health metrics names have no unit in them (e.g. add _celsius to
fortiswitch_health_temperature
| VDOM string | ||
| } | ||
| var r swResponse | ||
| //var r map[string]swResponse |
| mSumCPU = prometheus.NewDesc( | ||
| "fortiswitch_health_summary_cpu", | ||
| "Summary CPU health", | ||
| []string{"rating", "fortiswitch", "VDOM"}, nil, |
| //for _, sw := range r { | ||
| for fswitch, hr := range r.Results { | ||
|
|
||
| m = append(m, prometheus.MustNewConstMetric(mSumCPU, prometheus.GaugeValue, hr.Summary.CPU.Value, hr.Summary.CPU.Rating, fswitch, r.VDOM)) |
There was a problem hiding this comment.
The rating likely shouldn't be a level ä label but rather a metric like "xxx_is_good". That, or an enum label on a _rating metric. Otherwise it's really hard to use it in alerts.
| ) | ||
| mSumUpTime = prometheus.NewDesc( | ||
| "fortiswitch_health_summary_uptime", | ||
| "Summary Uptime", |
There was a problem hiding this comment.
If you have the documentation it would be really great if these descriptions could be made more verbose so I for example have any idea what these means. But if you don't, it's fine - sometimes we simply don't know
| } | ||
| m = append(m, prometheus.MustNewConstMetric(mPortStatus, prometheus.GaugeValue, pStatus, swr.VDOM, swr.Name, pi.Interface, pi.Vlan, pi.Duplex)) | ||
| m = append(m, prometheus.MustNewConstMetric(mPortSpeed, prometheus.GaugeValue, pi.Speed*1000*1000, swr.VDOM, swr.Name, pi.Interface, pi.Vlan, pi.Duplex)) | ||
|
|
There was a problem hiding this comment.
Having duplex as a label seems a bit off to me. Why not have it as a separate metric?
| if swr.Status == "Connected" { | ||
| swStatus = 1.0 | ||
| } | ||
| m = append(m, prometheus.MustNewConstMetric(mSwitchStatus, prometheus.GaugeValue, swStatus, swr.VDOM, swr.Name, swr.FgPeerIntfName, swr.Connection, swr.State)) |
There was a problem hiding this comment.
The state label here should likely be an enum label if the possible values of state is limited and known. It's hard to use it for anything useful otherwise.
| mPortStatus = prometheus.NewDesc( | ||
| "fortiswitch_port_status", | ||
| "Whether the switch port is up or not", | ||
| []string{"vdom", "name", "interface", "vlan", "duplex"}, nil, |
There was a problem hiding this comment.
Can a port only have one VLAN on a
Fortiswitch? That seems odd. What happens when the port is a VLAN trunk?
| mSwitchStatus = prometheus.NewDesc( | ||
| "fortiswitch_status", | ||
| "Whether the switch is connected or not", | ||
| []string{"vdom", "name", "fgt_peer_intf_name", "connection_from", "state"}, nil, |
There was a problem hiding this comment.
This meric is not like the rest. The fgt_peer_intf_name and connection_from is not clear to me what this metric is. Care to add it to the description or as a comment in code?
@bluecmd or @secustor