All work

Select view

Select search mode

 

Kafka producer uses resource name instead of ifIndex as the instance for InterfaceLevelResource

Fixed

Description

When exporting metrics to Kafka, each resource type has an entity. For physical interfaces, it is defined like this:

https://github.com/OpenNMS/opennms/blob/develop/features/kafka/producer/src/main/proto/collectionset.proto#L29

Considering that interface resources are especial, and it is the ifIndex that identifies a resource (a.k.a., instance), that Protobuf field should be populated with:

https://github.com/OpenNMS/opennms/blob/develop/features/collection/snmp-collector/src/main/java/org/opennms/netmgt/collectd/IfInfo.java#L267

Unfortunately, that is not the case:

https://github.com/OpenNMS/opennms/blob/develop/features/kafka/producer/src/main/java/org/opennms/features/kafka/producer/collection/CollectionSetMapper.java#L95

I don't think there is any usefulness in using the interface label (which is what the "getInterfaceLabel" method provides) as the instance here.

The correct behavior is that the instance should point to the ifIndex (i.e., use the "getInstance" method), to be consistent with the other resources (if you scroll down, you'll see that for Generic Index Resources, that is what it is used).

If someone needs the interface name, assuming there is a need for that, it is better to add it as a new field in the Protobuf content.

Acceptance / Success Criteria

None

Lucidchart Diagrams

Details

Assignee

Reporter

HB Backlog Status

Components

Sprint

Priority

PagerDuty

Created March 1, 2021 at 8:35 PM
Updated March 15, 2021 at 2:51 PM
Resolved March 8, 2021 at 8:31 PM

Activity

Alejandro GalueMarch 10, 2021 at 3:36 PM

I believe the Fix Version should contain the next M2019 and M2020, as the PR was merged to foundation-2019, right?

Chandra GorantlaMarch 8, 2021 at 8:36 PM

This was fixed by adding __ifIndex  to string attributes in foundation-2019.

For develop  new field if_index  is added for  InterfaceLevelResource

Chandra GorantlaMarch 8, 2021 at 8:33 PM

Alejandro GalueMarch 3, 2021 at 3:27 PM
Edited

The Aliased Resources was an enhancement for a particular use case that one and only one customer had years ago (and they are not a customer anymore). I'm 100% certain saying no one uses it nowadays. In my humble opinion, any code related to those resources should be permanently removed from our codebase (an effort that deserves another JIRA issue).

Thanks to the PR, those duplicate and unused resources won't appear as part of the collection set sent to Kafka. However, this would be the case only for M2021 and H28, not the existing versions, based on what we discussed.

What is going to happen is that the Aliased Resource introduces a copy of the original Interface Resource with a different label. That label is created based on a configured set of prefixes called Domains plus the ifAlias of the interface (if exists), meaning that all the interfaces with ifAlias populated would appear in the Protobuf payload as duplicates.

That's why removing them will not only reduce the size of the payload in half but also remove the need to add extra code on the consumer to ignore them.

Here is how the actual interface resource and its evil twin appear in the Protobuf payload (I used JSON to have a human-readable representation).

As you can see, two identical resources with different content for the "instance" field. Also, note that there is no way to know the ifIndex (unless you add a mibObj defined as a string for that purpose).

Interestingly, getInstance() returns null for the Aliased Resources, which is why we need the validation to avoid an exception at runtime:

https://github.com/OpenNMS/opennms/blob/develop/features/collection/snmp-collector/src/main/java/org/opennms/netmgt/collectd/AliasedResource.java#L202

To avoid changing content or the Protobuf format for Meridian 2019, Meridian 2020, and Horizon 27, the solution for these versions will be only to add a new string property with the ifIndex for the Interface Resources. We decided to call that property "__ifIndex". The rest of the content remains the same, meaning the aliased resources will still be present (but those won't have the new property).

In the future, for the next new releases (Horizon 28 and Meridian 2021), the Protobuf field will change. In this case, the "instance" will contain the ifIndex, and a new one called "label" will be added to have what you currently see under "instance". The string attribute "__ifindex" won't exist in this case (because it is not necessary anymore). Also, the aliased resources won't be part of the collection set, as they are duplicates.

Chandra GorantlaMarch 2, 2021 at 8:08 PM