Handle missing attributes without failing (#39)

open-telemetry · Jun 1, 2021 · 5f80eec · 5f80eec
1 parent 6edad7a
commit 5f80eec
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 2 deletions.
diff --git a/...b/jmx-metrics/src/main/groovy/io/opentelemetry/contrib/jmxmetrics/InstrumentHelper.groovy b/...b/jmx-metrics/src/main/groovy/io/opentelemetry/contrib/jmxmetrics/InstrumentHelper.groovy
@@ -106,7 +106,7 @@ class InstrumentHelper {
                     }
                     tupleToUpdates[tuple].add(prepareUpdateClosure(instrument, val, labels))
                 }
-            } else {
+            } else if (value != null) {
                 def labels = getLabels(mbean, labelFuncs)
                 def tuple = new Tuple(instrument, instrumentName, description, unit)
                 logger.fine("Recording ${instrumentName} - ${instrument.method} w/ ${value} - ${labels}")

diff --git a/contrib/jmx-metrics/src/main/groovy/io/opentelemetry/contrib/jmxmetrics/MBeanHelper.groovy b/contrib/jmx-metrics/src/main/groovy/io/opentelemetry/contrib/jmxmetrics/MBeanHelper.groovy
@@ -19,6 +19,7 @@ package io.opentelemetry.contrib.jmxmetrics
 import groovy.transform.PackageScope
 import javax.management.MBeanServerConnection
 import javax.management.ObjectName
+import javax.management.AttributeNotFoundException
 import java.util.logging.Logger
 
 /**
@@ -87,7 +88,12 @@ class MBeanHelper {
 
         def ofInterest = isSingle ? [mbeans[0]]: mbeans
         return ofInterest.collect {
-            it.getProperty(attribute)
+            try {
+                it.getProperty(attribute)
+            } catch (AttributeNotFoundException e) {
+                logger.warning("Expected attribute ${attribute} not found in mbean ${it.name()}")
+                null
+            }
         }
     }
 }
diff --git a/...x-metrics/src/test/groovy/io/opentelemetry/contrib/jmxmetrics/InstrumentHelperTest.groovy b/...x-metrics/src/test/groovy/io/opentelemetry/contrib/jmxmetrics/InstrumentHelperTest.groovy
@@ -189,6 +189,39 @@ class InstrumentHelperTest extends Specification {
         false | "multiple" | "Long" | "longValueObserver" | LONG_GAUGE | 234
     }
 
+    @Unroll
+    def "handles nulls returned from MBeanHelper"() {
+        setup: "Create and register four Things and create ${quantity} MBeanHelper"
+        def thingName = "${quantity}:type=${instrumentMethod}.${attribute}.Thing"
+        def things = (0..3).collect { new Thing() }
+        things.eachWithIndex { thing, i ->
+            def name = "${thingName},thing=${i}"
+            mBeanServer.registerMBean(thing, new ObjectName(name))
+        }
+        def mbeanHelper = new MBeanHelper(jmxClient, "${thingName},*", isSingle)
+        mbeanHelper.fetch()
+
+        expect:
+        when:
+        def instrumentName = "${quantity}.${instrumentMethod}.counter"
+        def description = "${quantity} double counter description"
+        def instrument = otel.&"${instrumentMethod}"
+        def instrumentHelper = new InstrumentHelper(
+                mbeanHelper, instrumentName, description, "1",
+                ["labelOne" : { "labelOneValue"}, "labelTwo": { mbean -> mbean.name().getKeyProperty("thing") }],
+                attribute, instrument)
+        instrumentHelper.update()
+
+        then:
+        def metrics = exportMetrics()
+        metrics.size() == 0
+
+        where:
+        isSingle | quantity | attribute | instrumentMethod | metricType | value
+        true | "single" | "Missing" | "longValueObserver" | LONG_GAUGE | null
+        false | "multiple" | "Missing" | "longValueObserver" | LONG_GAUGE | null
+    }
+
     @Unroll
     def "#instrumentMethod correctly classified"() {
         expect:

diff --git a/...ib/jmx-metrics/src/test/groovy/io/opentelemetry/contrib/jmxmetrics/MBeanHelperTest.groovy b/...ib/jmx-metrics/src/test/groovy/io/opentelemetry/contrib/jmxmetrics/MBeanHelperTest.groovy
@@ -85,6 +85,31 @@ class MBeanHelperTest extends Specification {
         false    | "multiple"
     }
 
+    @Unroll
+    def "handles missing attributes"() {
+        setup:
+        def thingName = "io.opentelemetry.contrib.jmxmetrics:type=${quantity}MissingAttributeThing"
+        def things = (0..100).collect { new Thing(it as String) }
+        things.eachWithIndex { thing, i ->
+            def name = "${thingName},thing=${i}"
+            mBeanServer.registerMBean(thing, new ObjectName(name))
+        }
+
+        expect:
+        when: "We request a nonexistent attribute via MBeanHelper"
+        def mbeanHelper = new MBeanHelper(jmxClient, "${thingName},*", isSingle)
+        mbeanHelper.fetch()
+
+        then: "nulls are returned"
+        def returned = mbeanHelper.getAttribute("MissingAttribute")
+        returned == isSingle ? [null]: (0..100).collect {null}
+
+        where:
+        isSingle | quantity
+        true     | "single"
+        false    | "multiple"
+    }
+
     interface ThingMBean {
 
         String getSomeAttribute()