NMS-19599: Fix metadata tags not being added to InterfaceLevelResource (#8352)

cgorantla · web-flow · commit 7699b9b39775 · 2026-03-17T09:00:01.000-04:00
* NMS-19599: Fix metadata tags not being added to InterfaceLevelResource

* NMS-19599: Add indexing for snmp ifname
diff --git a/core/ipc/rpc/mock-impl/src/main/java/org/opennms/core/rpc/mock/MockEntityScopeProvider.java b/core/ipc/rpc/mock-impl/src/main/java/org/opennms/core/rpc/mock/MockEntityScopeProvider.java
@@ -49,6 +49,11 @@ public Scope getScopeForInterfaceByIfIndex(Integer nodeId, int ifIndex) {
         return EmptyScope.EMPTY;
     }
 
+    @Override
+    public Scope getScopeForInterfaceByIfName(Integer nodeId, String ifName) {
+        return EmptyScope.EMPTY;
+    }
+
     @Override
     public Scope getScopeForService(final Integer nodeId, final InetAddress ipAddress, final String serviceName) {
         return EmptyScope.EMPTY;
diff --git a/core/mate/api/src/main/java/org/opennms/core/mate/api/EntityScopeProvider.java b/core/mate/api/src/main/java/org/opennms/core/mate/api/EntityScopeProvider.java
@@ -40,6 +40,8 @@ interface Contexts {
 
     Scope getScopeForInterfaceByIfIndex(final Integer nodeId, final int ifIndex);
 
+    Scope getScopeForInterfaceByIfName(final Integer nodeId, final String ifName);
+
     Scope getScopeForService(final Integer nodeId, final InetAddress ipAddress, final String serviceName);
 
     default ScopeProvider getScopeProviderForScv() {
@@ -58,6 +60,10 @@ default ScopeProvider getScopeProviderForInterfaceByIfIndex(final Integer nodeId
         return () -> getScopeForInterfaceByIfIndex(nodeId, ifIndex);
     }
 
+    default ScopeProvider getScopeProviderForInterfaceByIfName(final Integer nodeId, final String ifName) {
+        return () -> getScopeForInterfaceByIfName(nodeId, ifName);
+    }
+
     default ScopeProvider getScopeProviderForService(final Integer nodeId, final InetAddress ipAddress, final String serviceName) {
         return () -> getScopeForService(nodeId, ipAddress, serviceName);
     }
diff --git a/core/mate/model/src/main/java/org/opennms/core/mate/model/EntityScopeProviderImpl.java b/core/mate/model/src/main/java/org/opennms/core/mate/model/EntityScopeProviderImpl.java
@@ -34,6 +34,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
 import org.opennms.core.mate.api.ContextKey;
@@ -273,9 +274,20 @@ public Scope getScopeForInterfaceByIfIndex(final Integer nodeId, final int ifInd
         if (nodeId == null) {
             return EmptyScope.EMPTY;
         }
+        return buildScopeForSnmpInterface(() -> this.snmpInterfaceDao.findByNodeIdAndIfIndex(nodeId, ifIndex));
+    }
+
+    @Override
+    public Scope getScopeForInterfaceByIfName(final Integer nodeId, final String ifName) {
+        if (nodeId == null || Strings.isNullOrEmpty(ifName)) {
+            return EmptyScope.EMPTY;
+        }
+        return buildScopeForSnmpInterface(() -> this.snmpInterfaceDao.findByNodeIdAndIfName(nodeId, ifName));
+    }
 
+    private Scope buildScopeForSnmpInterface(final Supplier<OnmsSnmpInterface> snmpInterfaceLookup) {
         return this.sessionUtils.withReadOnlyTransaction(() -> {
-            final OnmsSnmpInterface snmpInterface = this.snmpInterfaceDao.findByNodeIdAndIfIndex(nodeId, ifIndex);
+            final OnmsSnmpInterface snmpInterface = snmpInterfaceLookup.get();
             if (snmpInterface == null) {
                 return EmptyScope.EMPTY;
             }
@@ -286,7 +298,7 @@ public Scope getScopeForInterfaceByIfIndex(final Integer nodeId, final int ifInd
             scopes.add(new ObjectScope<>(Scope.ScopeName.INTERFACE, snmpInterface)
                     .map(INTERFACE, "if-alias", (i) -> Optional.ofNullable(i.getIfAlias()))
                     .map(INTERFACE, "if-description", (i) -> Optional.ofNullable(i.getIfDescr()))
-                    .map(INTERFACE, "if-name", (i) -> Optional.ofNullable(i.getIfName())) 
+                    .map(INTERFACE, "if-name", (i) -> Optional.ofNullable(i.getIfName()))
                     .map(INTERFACE, "phy-addr", (i) -> Optional.ofNullable(i.getPhysAddr())));
 
             // IP interface facts w/ meta-data extracted from IP interface
diff --git a/core/mate/model/src/test/java/org/opennms/core/mate/model/EntityScopeProviderIT.java b/core/mate/model/src/test/java/org/opennms/core/mate/model/EntityScopeProviderIT.java
@@ -144,6 +144,23 @@ public final void testInterface() throws Exception {
         assertThat(scope.get(new ContextKey("interface", "phy-addr")), Matchers.is(Optional.of(new Scope.ScopeValue(Scope.ScopeName.INTERFACE, "34E45604BB69"))));
     }
 
+    @Test
+    public final void testInterfaceByIfName() throws Exception {
+        final Scope scope = this.provider.getScopeForInterfaceByIfName(this.populator.getNode1().getId(), "atm0");
+
+        assertThat(scope.get(new ContextKey("interface", "if-alias")), Matchers.is(Optional.of(new Scope.ScopeValue(Scope.ScopeName.INTERFACE, "Initial ifAlias value"))));
+        assertThat(scope.get(new ContextKey("interface", "if-description")), Matchers.is(Optional.of(new Scope.ScopeValue(Scope.ScopeName.INTERFACE, "ATM0"))));
+        assertThat(scope.get(new ContextKey("interface", "if-name")), Matchers.is(Optional.of(new Scope.ScopeValue(Scope.ScopeName.INTERFACE, "atm0"))));
+        assertThat(scope.get(new ContextKey("interface", "phy-addr")), Matchers.is(Optional.of(new Scope.ScopeValue(Scope.ScopeName.INTERFACE, "34E45604BB69"))));
+    }
+
+    @Test
+    public final void testInterfaceByIfNameNotFound() throws Exception {
+        final Scope scope = this.provider.getScopeForInterfaceByIfName(this.populator.getNode1().getId(), "nonexistent0");
+
+        assertThat(scope.get(new ContextKey("interface", "if-name")), Matchers.is(Optional.empty()));
+    }
+
     @Test
     public final void testService() throws Exception {
         final Scope scope = this.provider.getScopeForService(this.populator.getNode1().getId(), InetAddressUtils.getInetAddress("192.168.1.1"), "ICMP");
diff --git a/core/schema/src/main/liquibase/33.0.3/changelog.xml b/core/schema/src/main/liquibase/33.0.3/changelog.xml
@@ -26,4 +26,11 @@
     <changeSet author="cpape" id="33.0.3-assets-userlastmodified">
         <modifyDataType tableName="assets" columnName="userlastmodified" newDataType="varchar(256)"/>
     </changeSet>
+
+    <changeSet author="cgorantla" id="33.0.3-snmpinterface_nodeid_ifname-index">
+        <createIndex tableName="snmpinterface" indexName="snmpinterface_nodeid_ifname_idx" unique="false">
+            <column name="nodeid" />
+            <column name="snmpifname" />
+        </createIndex>
+    </changeSet>
 </databaseChangeLog>
diff --git a/features/timeseries/src/main/java/org/opennms/netmgt/timeseries/samplewrite/MetaTagDataLoader.java b/features/timeseries/src/main/java/org/opennms/netmgt/timeseries/samplewrite/MetaTagDataLoader.java
@@ -94,12 +94,14 @@ public Set<Tag> load(final CollectionResource resource) {
                 OnmsNode node = nodeOptional.get();
                 scopes.add(this.entityScopeProvider.getScopeForNode(node.getId()));
                 if (resource.getResourceTypeName().equals(CollectionResource.RESOURCE_TYPE_IF)) {
-                    // We expect #getInstance to return the ifIndex for interface-level resources
-                    try {
-                        int ifIndex = Integer.parseInt(resource.getInstance());
-                        scopes.add(this.entityScopeProvider.getScopeForInterfaceByIfIndex(node.getId(), ifIndex));
-                    } catch(NumberFormatException nfe) {
-                        // pass
+                    String instance = resource.getInstance();
+                    if (instance != null && !instance.isEmpty()) {
+                        if (checkNumeric(instance)) {
+                            scopes.add(this.entityScopeProvider.getScopeForInterfaceByIfIndex(node.getId(), Integer.parseInt(instance)));
+                        } else {
+                            // Streaming telemetry resources use interface name instead of ifIndex
+                            scopes.add(this.entityScopeProvider.getScopeForInterfaceByIfName(node.getId(), instance));
+                        }
                     }
                 }
 
diff --git a/features/timeseries/src/test/java/org/opennms/netmgt/timeseries/samplewrite/TimeseriesRoundtripIT.java b/features/timeseries/src/test/java/org/opennms/netmgt/timeseries/samplewrite/TimeseriesRoundtripIT.java
@@ -247,6 +247,42 @@ public void canPersist() throws InterruptedException, StorageException, UnknownH
     }
 
 
+    @Test
+    public void verifyStreamingTelemetryInterfaceResource() {
+        // Streaming telemetry uses interface name (e.g. "en1/0") instead of numeric ifIndex
+        var collectionResource = mock(CollectionResource.class);
+        when(collectionResource.getResourceTypeName()).thenReturn(CollectionResource.RESOURCE_TYPE_IF);
+        when(collectionResource.getInstance()).thenReturn("en1/0"); // interface name, not ifIndex
+        when(collectionResource.getParent()).thenReturn(ResourcePath.get("1"));
+        when(collectionResource.getTags()).thenReturn(Collections.emptyMap());
+        when(collectionResource.getServiceParams()).thenReturn(Collections.emptyMap());
+        when(collectionResource.getInterfaceLabel()).thenReturn(null);
+
+        var tags = metaTagDataLoader.load(collectionResource);
+
+        // Should resolve interface scope via ifName fallback
+        assertTrue("if_name tag should be present", tags.stream().anyMatch(tag -> tag.getKey().equals("if_name") && tag.getValue().equals("en1/0")));
+        assertTrue("if_description tag should be present", tags.stream().anyMatch(tag -> tag.getKey().equals("if_description") && tag.getValue().equals("myDescription")));
+    }
+
+    @Test
+    public void verifySnmpInterfaceResource() {
+        // SNMP uses numeric ifIndex
+        var collectionResource = mock(CollectionResource.class);
+        when(collectionResource.getResourceTypeName()).thenReturn(CollectionResource.RESOURCE_TYPE_IF);
+        when(collectionResource.getInstance()).thenReturn("1"); // numeric ifIndex
+        when(collectionResource.getParent()).thenReturn(ResourcePath.get("1"));
+        when(collectionResource.getTags()).thenReturn(Collections.emptyMap());
+        when(collectionResource.getServiceParams()).thenReturn(Collections.emptyMap());
+        when(collectionResource.getInterfaceLabel()).thenReturn(null);
+
+        var tags = metaTagDataLoader.load(collectionResource);
+
+        // Should resolve interface scope via ifIndex
+        assertTrue("if_name tag should be present", tags.stream().anyMatch(tag -> tag.getKey().equals("if_name") && tag.getValue().equals("en1/0")));
+        assertTrue("if_description tag should be present", tags.stream().anyMatch(tag -> tag.getKey().equals("if_description") && tag.getValue().equals("myDescription")));
+    }
+
     @Test
     public void verifyLatencyResourceTags() throws UnknownHostException {
 
diff --git a/opennms-dao-api/src/main/java/org/opennms/netmgt/dao/api/SnmpInterfaceDao.java b/opennms-dao-api/src/main/java/org/opennms/netmgt/dao/api/SnmpInterfaceDao.java
@@ -66,6 +66,8 @@ public interface SnmpInterfaceDao extends LegacyOnmsDao<OnmsSnmpInterface, Integ
 
     OnmsSnmpInterface findByNodeIdAndDescription(Integer nodeId, String description);
 
+    OnmsSnmpInterface findByNodeIdAndIfName(Integer nodeId, String ifName);
+
     void markHavingIngressFlows(final Integer nodeId, final Collection<Integer> ingressSnmpIfIndexes);
     void markHavingEgressFlows(final Integer nodeId, final Collection<Integer> egressSnmpIfIndexes);
 
diff --git a/opennms-dao-mock/src/main/java/org/opennms/netmgt/dao/mock/MockSnmpInterfaceDao.java b/opennms-dao-mock/src/main/java/org/opennms/netmgt/dao/mock/MockSnmpInterfaceDao.java
@@ -125,6 +125,16 @@ public OnmsSnmpInterface findByNodeIdAndDescription(Integer nodeId, String descr
         return null;
     }
 
+    @Override
+    public OnmsSnmpInterface findByNodeIdAndIfName(Integer nodeId, String ifName) {
+        for (final OnmsSnmpInterface iface : findAll()) {
+            if (iface.getNode() != null && nodeId.equals(iface.getNode().getId()) && ifName.equals(iface.getIfName())) {
+                return iface;
+            }
+        }
+        return null;
+    }
+
     @Override
     public void markHavingIngressFlows(final Integer nodeId, final Collection<Integer> ingressSnmpIfIndexes) {
     }
diff --git a/opennms-dao/src/main/java/org/opennms/netmgt/dao/hibernate/SnmpInterfaceDaoHibernate.java b/opennms-dao/src/main/java/org/opennms/netmgt/dao/hibernate/SnmpInterfaceDaoHibernate.java
@@ -93,6 +93,14 @@ public OnmsSnmpInterface findByNodeIdAndDescription(Integer nodeId, String descr
         );
     }
 
+    @Override
+    public OnmsSnmpInterface findByNodeIdAndIfName(Integer nodeId, String ifName) {
+        Assert.notNull(nodeId, "nodeId may not be null");
+        Assert.notNull(ifName, "ifName may not be null");
+        return findUnique("select snmpIf from OnmsSnmpInterface as snmpIf where snmpIf.node.id = ? and snmpIf.ifName = ?",
+            nodeId, ifName);
+    }
+
     @Override
     public void markHavingIngressFlows(final Integer nodeId, final Collection<Integer> ingressSnmpIfIndexes) {
         getHibernateTemplate().executeWithNativeSession(session -> session.createSQLQuery("update snmpinterface set last_ingress_flow = NOW() where nodeid = :nodeid and snmpifindex in (:snmpIfIndexes)")
diff --git a/opennms-dao/src/test/java/org/opennms/netmgt/dao/SnmpInterfaceDaoIT.java b/opennms-dao/src/test/java/org/opennms/netmgt/dao/SnmpInterfaceDaoIT.java
@@ -23,6 +23,7 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 
 import java.util.Collection;
 
@@ -79,9 +80,34 @@ public void testGet() throws Exception {
 			assertNotNull(snmpIf.getNode());
 			assertEquals(1, snmpIf.getNode().getId().intValue());
 			assertEquals("node1", snmpIf.getNode().getLabel());
-			
+
 		}
-		
+	}
+
+	@Test
+	@Transactional
+	public void testFindByNodeIdAndIfName() {
+		int nodeId = m_databasePopulator.getNode1().getId();
+
+		OnmsSnmpInterface snmpIf = m_snmpInterfaceDao.findByNodeIdAndIfName(nodeId, "atm0");
+		assertNotNull(snmpIf);
+		assertEquals("atm0", snmpIf.getIfName());
+		assertEquals(1, snmpIf.getIfIndex().intValue());
+		assertEquals("ATM0", snmpIf.getIfDescr());
+
+		OnmsSnmpInterface snmpIf2 = m_snmpInterfaceDao.findByNodeIdAndIfName(nodeId, "eth0");
+		assertNotNull(snmpIf2);
+		assertEquals("eth0", snmpIf2.getIfName());
+		assertEquals(2, snmpIf2.getIfIndex().intValue());
+	}
+
+	@Test
+	@Transactional
+	public void testFindByNodeIdAndIfNameNotFound() {
+		int nodeId = m_databasePopulator.getNode1().getId();
+
+		OnmsSnmpInterface snmpIf = m_snmpInterfaceDao.findByNodeIdAndIfName(nodeId, "nonexistent0");
+		assertNull(snmpIf);
 	}
 
 }
