@fgiunchedi perhaps you might know a way to do this.

We now have stats like this in Prometheus, with values that tell us if BGP is up or down:

gnmi_bgp_neighbor_session_state{address="10.64.0.105", instance="cr1-eqiad", job="gnmi", network_instance_name="DEFAULT", peer_as="64601", peer_descr="wikikube-worker1242", peer_group="Kubernetes4", peer_type="EXTERNAL", prometheus="ops", protocol_identifier="BGP", protocol_name="DEFAULT", site="eqiad"}  6

If we replaced the existing LibreNMS alert rule with an alertmanager one, would there be any way to supress alerts if the "peer_descr" matched a hostname that was currently downtimed?

Yes we would be able to inhibit (i.e. send no notifications) say BGPPeerDown alert based on the fact that another alert is firing (e.g. HostDown alert, to be created).

Note this is slightly different from what you mentioned, namely not the fact that an host is downtimed but rather say to alertmanager "if the host is down, then don't notify for the same host being down as a bgp peer"

Hope that helps!

In T384731#10511163, @fgiunchedi wrote:

Yes we would be able to inhibit (i.e. send no notifications) say BGPPeerDown alert based on the fact that another alert is firing (e.g. HostDown alert, to be created).

Note this is slightly different from what you mentioned, namely not the fact that an host is downtimed but rather say to alertmanager "if the host is down, then don't notify for the same host being down as a bgp peer"

Hope that helps!

Thanks yes it does! But I'm actually thinking I'm looking at it the wrong way. The previous check ran against the router, checking for all the BGP peers, and alerting if any were down.

I think what we should do instead is create an alertmanager check at the host level for all K8s servers, with a query like:

gnmi_bgp_neighbor_session_state{peer_descr="$hostname"} == 6

Then this check, because it's at the host level, would automatically not alert if the host is downtimed? Or would that only work if instance=$hostname?

An alternative (or short term solution until the above/cleaner one is live) is to not alert for sessions on the router side towards k8s nodes, and only monitor BGP from the k8s side.

The only thing that we would lose is if there is a bug where the calico/bird instance thinks BGP is up, while it's actually down. This can also be a complementary monitoring.

From a quick look, https://docs.tigera.io/calico-cloud/operations/monitor/metrics/bgp-metrics is only for the paid version of Calico, but it might be possible to install prometheus-bird-exporter like we do for the Anycast service : https://grafana.wikimedia.org/d/dxbfeGDZk/anycast

In T384731#10511648, @cmooney wrote:

In T384731#10511163, @fgiunchedi wrote:

Yes we would be able to inhibit (i.e. send no notifications) say BGPPeerDown alert based on the fact that another alert is firing (e.g. HostDown alert, to be created).

Note this is slightly different from what you mentioned, namely not the fact that an host is downtimed but rather say to alertmanager "if the host is down, then don't notify for the same host being down as a bgp peer"

Hope that helps!

Thanks yes it does! But I'm actually thinking I'm looking at it the wrong way. The previous check ran against the router, checking for all the BGP peers, and alerting if any were down.

I think what we should do instead is create an alertmanager check at the host level for all K8s servers, with a query like:

gnmi_bgp_neighbor_session_state{peer_descr="$hostname"} == 6

Then this check, because it's at the host level, would automatically not alert if the host is downtimed? Or would that only work if instance=$hostname?

Technically we could, though I'd rather not create individual alerts for each host but rather rely on queries without explicit hostnames in them. At any rate let's brainstorm before/in/after Atlanta !

In T384731#10516013, @ayounsi wrote:

An alternative (or short term solution until the above/cleaner one is live) is to not alert for sessions on the router side towards k8s nodes, and only monitor BGP from the k8s side.

Yeah this is one of the options in the task description, and the cleanest as far as I am concerned. I don't believe it is trivial to do, however, the data is not exposed in Prometheus.

The only thing that we would lose is if there is a bug where the calico/bird instance thinks BGP is up, while it's actually down. This can also be a complementary monitoring.

Yeah that's unlikely.

From a quick look, https://docs.tigera.io/calico-cloud/operations/monitor/metrics/bgp-metrics is only for the paid version of Calico, but it might be possible to install prometheus-bird-exporter like we do for the Anycast service : https://grafana.wikimedia.org/d/dxbfeGDZk/anycast

I didn't get further than finding the issue and that it's in the paid version. But a quick check of prometheus-bird-exporter shows it uses Unix sockets to talk to Bird, so in theory it would be possible. There is also this I found but it depends on 'birdcl' / 'birdc' which is not on our k8s hosts:

https://github.com/kumina/birdwatcher

In T384731#10517106, @fgiunchedi wrote:

Technically we could, though I'd rather not create individual alerts for each host but rather rely on queries without explicit hostnames in them. At any rate let's brainstorm before/in/after Atlanta !

Yeah we don't want to do that. We can check for the whole group, the problem is suppressing the alerts for downtimed hosts. Let's talk at the summit!

From lunch discussion in Atlanta:
It would be ideal if we could create a recording rule that copies the gnmi_bgp_neighbor_session_state metric, rewriting the peer_descr label value to instance. That would auto downtime/silence alerts based on that metric when a kubernetes node (peer_descr) is silenced.
This has the benefit of still relying on router data for alerting, not having to introduce another data source and not having to patch the calico chart with the bird exporter.

If, for whatever reason, the above does not work, we could still add the prometheus-bird-exporter as sidecar to calico-node (bird v4 and v6 sockets are accessible there) and alerts on metrics reported by it rather than gnmi.

Since we have to overwrite instance with the host instead of the router, that information is effectively lost, unless we move instance elsewhere. At any rate I think for a limited (i.e. internal bgp peers) use case it is fine.

The base query looks like this: label_replace(sum without (instance) (gnmi_bgp_neighbor_session_state), "instance", "$1:0", "peer_descr", "(.*)"), we would record that query into a new metric such as instance:gnmi_bgp_neighbor_session_state in modules/profile/files/prometheus/rules_ops.yml and we can alert if said metric == 6.

I also noticed that for some of the results there's more information/duplication? That could be filtered too, e.g. of duplication

{address="10.128.0.11", instance="lvs4010:0", job="gnmi", network_instance_name="DEFAULT", peer_as="64600", peer_descr="lvs4010", peer_group="PyBal", peer_type="EXTERNAL", prometheus="ops", protocol_identifier="BGP", protocol_name="DEFAULT", site="ulsfo"}	6
{address="10.128.0.11", instance="lvs4010:0", job="gnmi", network_instance_name="master", peer_as="64600", peer_descr="lvs4010", peer_group="PyBal", peer_type="EXTERNAL", prometheus="ops", site="ulsfo"}	6

Thanks !

In T384731#10556225, @fgiunchedi wrote:

Since we have to overwrite instance with the host instead of the router, that information is effectively lost, unless we move instance elsewhere. At any rate I think for a limited (i.e. internal bgp peers) use case it is fine.

The base query looks like this: label_replace(sum without (instance) (gnmi_bgp_neighbor_session_state), "instance", "$1:0", "peer_descr", "(.*)"), we would record that query into a new metric such as instance:gnmi_bgp_neighbor_session_state in modules/profile/files/prometheus/rules_ops.yml and we can alert if said metric == 6.

Is it possible to duplicate the metric, before the transform, so it still shows up as a router metric ?

And what happens if peer_descr is missing or empty ?

I also noticed that for some of the results there's more information/duplication? That could be filtered too, e.g. of duplication

{address="10.128.0.11", instance="lvs4010:0", job="gnmi", network_instance_name="DEFAULT", peer_as="64600", peer_descr="lvs4010", peer_group="PyBal", peer_type="EXTERNAL", prometheus="ops", protocol_identifier="BGP", protocol_name="DEFAULT", site="ulsfo"}	6
{address="10.128.0.11", instance="lvs4010:0", job="gnmi", network_instance_name="master", peer_as="64600", peer_descr="lvs4010", peer_group="PyBal", peer_type="EXTERNAL", prometheus="ops", site="ulsfo"}	6

That's because in "legacy" subnets, the servers peer with the two core routers :

netflow4002:~$ curl localhost:9804/metrics | grep gnmi_bgp_neighbor_session_state | grep lvs4010 see the different source later renamed to instance

gnmi_bgp_neighbor_session_state{address="10.128.0.11",network_instance_name="master",peer_as="64600",peer_descr="lvs4010",peer_group="PyBal",peer_type="EXTERNAL",source="cr4-ulsfo.wikimedia.org:32767"} 6 1739965191643
gnmi_bgp_neighbor_session_state{address="10.128.0.11",network_instance_name="DEFAULT",peer_as="64600",peer_descr="lvs4010",peer_group="PyBal",peer_type="EXTERNAL",protocol_identifier="BGP",protocol_name="DEFAULT",source="cr3-ulsfo.wikimedia.org:32767"} 6 1739965192053

(the other fields difference are because each router runs a different junos version).

So if we re-write that state, it will lead to duplicates, and possibly wrong info if only one of the session is up and the other down. If we go that way we might want to keep the worse state. Or do more transformation to add a router_name (or peer_name or something else) field and add the current source/instance in there.

Overall for that usecase I think the cleanest would be to go with the prometheus-bird-exporter (especially if one day we go with dynamic BGP neighbors) but I lack knowledge on how complex it is to setup.

On the other hand I can imagine other usecases where such transform would be useful. For example if a switch port goes down, or saturates, to have it alert as the matching host/team.

In T384731#10563685, @ayounsi wrote:

! In T384731#10556225, @fgiunchedi wrote:
I also noticed that for some of the results there's more information/duplication? That could be filtered too, e.g. of duplication

{address="10.128.0.11", instance="lvs4010:0", job="gnmi", network_instance_name="DEFAULT", peer_as="64600", peer_descr="lvs4010", peer_group="PyBal", peer_type="EXTERNAL", prometheus="ops", protocol_identifier="BGP", protocol_name="DEFAULT", site="ulsfo"}	6
{address="10.128.0.11", instance="lvs4010:0", job="gnmi", network_instance_name="master", peer_as="64600", peer_descr="lvs4010", peer_group="PyBal", peer_type="EXTERNAL", prometheus="ops", site="ulsfo"}	6

That's because in "legacy" subnets, the servers peer with the two core routers

Yeah I hadn't considered that. I guess we need two rewrites:

instance -> router_name
peer_descr -> instance

So we have an additional field "router_name" to differentiate these two.

In T384731#10563685, @ayounsi wrote:

Is it possible to duplicate the metric, before the transform, so it still shows up as a router metric ?

My understanding is this is replacing labels and writing the resulting new metrics as separate series to the db. So we still have the source metric as-is with tags unchanged? @fgiunchedi can you clarify that?

In T384731#10565308, @cmooney wrote:

In T384731#10563685, @ayounsi wrote:

Is it possible to duplicate the metric, before the transform, so it still shows up as a router metric ?

My understanding is this is replacing labels and writing the resulting new metrics as separate series to the db. So we still have the source metric as-is with tags unchanged? @fgiunchedi can you clarify that?

Yes your understanding is correct: recording rules create new metrics based on the input query, metrics that are part of the input query are left untouched

In T384731#10563685, @ayounsi wrote:

Thanks !

In T384731#10556225, @fgiunchedi wrote:

Since we have to overwrite instance with the host instead of the router, that information is effectively lost, unless we move instance elsewhere. At any rate I think for a limited (i.e. internal bgp peers) use case it is fine.

The base query looks like this: label_replace(sum without (instance) (gnmi_bgp_neighbor_session_state), "instance", "$1:0", "peer_descr", "(.*)"), we would record that query into a new metric such as instance:gnmi_bgp_neighbor_session_state in modules/profile/files/prometheus/rules_ops.yml and we can alert if said metric == 6.

Is it possible to duplicate the metric, before the transform, so it still shows up as a router metric ?

yes, see above

And what happens if peer_descr is missing or empty ?

good question, in that case the instance label will be :0 since $1 will be empty. Is having peer_descr something that can happen?

I also noticed that for some of the results there's more information/duplication? That could be filtered too, e.g. of duplication

{address="10.128.0.11", instance="lvs4010:0", job="gnmi", network_instance_name="DEFAULT", peer_as="64600", peer_descr="lvs4010", peer_group="PyBal", peer_type="EXTERNAL", prometheus="ops", protocol_identifier="BGP", protocol_name="DEFAULT", site="ulsfo"}	6
{address="10.128.0.11", instance="lvs4010:0", job="gnmi", network_instance_name="master", peer_as="64600", peer_descr="lvs4010", peer_group="PyBal", peer_type="EXTERNAL", prometheus="ops", site="ulsfo"}	6

That's because in "legacy" subnets, the servers peer with the two core routers :

thank you that explains!

So if we re-write that state, it will lead to duplicates, and possibly wrong info if only one of the session is up and the other down. If we go that way we might want to keep the worse state. Or do more transformation to add a router_name (or peer_name or something else) field and add the current source/instance in there.

Indeed we'll have to rename both instance and peer_descr as you described

Overall for that usecase I think the cleanest would be to go with the prometheus-bird-exporter (especially if one day we go with dynamic BGP neighbors) but I lack knowledge on how complex it is to setup.

When we talked with @JMeybohm it looked possible though IIRC not very future proof since calico might ditch bird, totally doable though

On the other hand I can imagine other usecases where such transform would be useful. For example if a switch port goes down, or saturates, to have it alert as the matching host/team.

indeed that's a useful feature to have!

Thanks for the update @fgiunchedi

! In T384731#10566953, @fgiunchedi wrote:

And what happens if peer_descr is missing or empty ?

good question, in that case the instance label will be :0 since $1 will be empty. Is having peer_descr something that can happen?

The config for each peer is created by automation triggered by the 'bgp' flag for a host being set in Netbox. So in normal operations there is basically zero chance of "peer_descr" being missing/incorrect.

In T384731#10566953, @fgiunchedi wrote:

In T384731#10563685, @ayounsi wrote:

Overall for that usecase I think the cleanest would be to go with the prometheus-bird-exporter (especially if one day we go with dynamic BGP neighbors) but I lack knowledge on how complex it is to setup.

When we talked with @JMeybohm it looked possible though IIRC not very future proof since calico might ditch bird, totally doable though

I think it's easily doable by adding a sidecar to the calico-node pods that has access to the bird sockets (hostPath). Also there is on evidence that calico will be ditching bird so we could also go that route for now and revisit later if needed.

In T384731#10578273, @JMeybohm wrote:

In T384731#10566953, @fgiunchedi wrote:

In T384731#10563685, @ayounsi wrote:

Overall for that usecase I think the cleanest would be to go with the prometheus-bird-exporter (especially if one day we go with dynamic BGP neighbors) but I lack knowledge on how complex it is to setup.

When we talked with @JMeybohm it looked possible though IIRC not very future proof since calico might ditch bird, totally doable though

I think it's easily doable by adding a sidecar to the calico-node pods that has access to the bird sockets (hostPath). Also there is on evidence that calico will be ditching bird so we could also go that route for now and revisit later if needed.

My apologies for the noise -- I clearly wasn't remembering correctly! If the sidecar case is easy to do then that's something to seriously consider IMHO. At the same time, addressing/thinking about the general problem (namely "what to do with alerts about hosts which don't come from the host itself") is worthwhile doing anyways.

And what happens if peer_descr is missing or empty ?

good question, in that case the instance label will be :0 since $1 will be empty. Is having peer_descr something that can happen?

Looks like it can't be empty or missing but can become (null). See T387220: BGP peers with missing descriptions.
I also asked in case there was any bugs in the pipeline and it somehow became missing.

In T384731#10579181, @ayounsi wrote:

And what happens if peer_descr is missing or empty ?

good question, in that case the instance label will be :0 since $1 will be empty. Is having peer_descr something that can happen?

Looks like it can't be empty or missing but can become (null). See T387220: BGP peers with missing descriptions.
I also asked in case there was any bugs in the pipeline and it somehow became missing.

Good to check. One thing this made me think is it probably would be a good idea to limit the number of new metrics we make, to reduce the total amount of duplicate data?

I'm not sure what the best way to do this would be. One way might be to filter in the rewrite rule to just the groups we are interested in? I guess we should do it and make the rule for all the groups with servers doing BGP? But we'd need to maintain a list of the groups somewhere. Or perhaps there are other ways - or maybe we don't care about the duplication?

I forked the discussion to T387287: Prometheus: attach host's BGP/interface remote side metrics and T387288: Add prometheus-bird-exporter sidecar to calico-node pods as that task was becoming more difficult to read.