jmx filter negative values in yaml

instrumentation/jmx-metrics/javaagent/README.md

@@ -320,10 +320,10 @@ If such a conversion is not available, then an error is reported during JMX metr
 Currently available unit conversions:
 
 | `sourceUnit` | `unit` |
-|--------------|-------|
-| ms           | s     |
-| us           | s     |
-| ns           | s     |
+|--------------|--------|
+| ms           | s      |
+| us           | s      |
+| ns           | s      |
 
 Example of defining unit conversion in yaml file:
 ```yaml
@@ -341,6 +341,29 @@ rules:
 ```
 `sourceUnit` can also be defined on rule level (see [Making shortcuts](#making-shortcuts))
 
+### Filtering negative values
+
+Sometimes a negative value is returned by the MBean implementation when a metric is not available or not supported.
+For example, [`OperatingSystemMXBean.getProcessCpuLoad`](https://docs.oracle.com/javase/7/docs/jre/api/management/extension/com/sun/management/OperatingSystemMXBean.html#getProcessCpuLoad()) can return a negative value.
+
+In this case, it is recommended to filter out the negative values by setting the `dropNegativeValues` metric (or rule) property to `true`, it is set to `false` by default.
+
+```yaml
+rules:
+  - bean: java.lang:type=OperatingSystem
+    # can also be set at rule-level (with lower priority)
+    dropNegativeValues: false
+    mapping:
+      # jvm.cpu.recent_utilization
+      ProcessCpuLoad:
+        metric: jvm.cpu.recent_utilization
+        type: gauge
+        unit: '1'
+        # setting dropNegativeValues at metric level has priority over rule level.
+        dropNegativeValues: true
+        desc: Recent CPU utilization for the process as reported by the JVM.
+```
+
 ### General Syntax
 
 Here is the general description of the accepted configuration file syntax. The whole contents of the file is case-sensitive, with exception for `type` as described in the table below.
@@ -356,12 +379,14 @@ rules:                                # start of list of configuration rules
     prefix: <METRIC_NAME_PREFIX>      # optional, useful for avoiding specifying metric names below
     unit: <UNIT>                      # optional, redefines the default unit for the whole rule
     type: <TYPE>                      # optional, redefines the default type for the whole rule
+    dropNegativeValues: <BOOL>        # optional, redefines if negative values are dropped for the whole rule
     mapping:
       <BEANATTR1>:                    # an MBean attribute name defining the metric value
         metric: <METRIC_NAME1>        # metric name will be <METRIC_NAME_PREFIX><METRIC_NAME1>
         type: <TYPE>                  # optional, the default type is gauge
         desc: <DESCRIPTION1>          # optional
         unit: <UNIT1>                 # optional
+        dropNegativeValues: <BOOL>    # optional, defines if negative values are dropped for the metric
         metricAttribute:              # optional, will be used in addition to the shared metric attributes above
           <ATTRIBUTE3>: const(<STR>)  # direct value for the metric attribute
       <BEANATTR2>:                    # use a.b to get access into CompositeData
@@ -381,19 +406,20 @@ rules:                                # start of list of configuration rules
 
 The following table explains the used terms with more details.
 
-| Syntactic Element | Description |
-| ---------------- | --------------- |
-| OBJECTNAME | A syntactically valid string representing an ObjectName (see [ObjectName constructor](https://docs.oracle.com/javase/8/docs/api/javax/management/ObjectName.html#ObjectName-java.lang.String-)). |
-| ATTRIBUTE | Any well-formed string that can be used as a metric [attribute](https://opentelemetry.io/docs/reference/specification/common/#attribute) key. |
-| ATTR | A non-empty string used as a name of the MBean attribute. The MBean attribute value must be a String, otherwise the specified metric attribute will not be used. |
-| PARAM | A non-empty string used as a property key in the ObjectName identifying the MBean which provides the metric value. If the ObjectName does not have a property with the given key, the specified metric attribute will not be used. |
-| METRIC_NAME_PREFIX | Any non-empty string which will be prepended to the specified metric (instrument) names. |
-| METRIC_NAME | Any non-empty string. The string, prefixed by the optional prefix (see above) must satisfy [instrument naming rule](https://opentelemetry.io/docs/reference/specification/metrics/api/#instrument-naming-rule). |
-| TYPE | One of `counter`, `updowncounter`, or `gauge`. The default is `gauge`. This value is case insensitive. |
-| DESCRIPTION | Any string to be used as human-readable [description](https://opentelemetry.io/docs/reference/specification/metrics/api/#instrument-description) of the metric. If the description is not provided by the rule, an attempt will be made to extract one automatically from the corresponding MBean. |
-| UNIT | A string identifying the [unit](https://opentelemetry.io/docs/reference/specification/metrics/api/#instrument-unit) of measurements reported by the metric. Enclose the string in single or double quotes if using unit annotations. |
-| STR | Any string to be used directly as the metric attribute value. |
-| BEANATTR | A non-empty string representing the MBean attribute defining the metric value. The attribute value must be a number. Special dot-notation _attributeName.itemName_ can be used to access numerical items within attributes of [CompositeType](https://docs.oracle.com/javase/8/docs/api/javax/management/openmbean/CompositeType.html). If a dot happens to be an integral part of the MBean attribute name, it must be escaped by backslash (`\`). |
+| Syntactic Element  | Description                                                                                                                                                                                                                                                                                                                                                                                                                                         |
+|--------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| OBJECTNAME         | A syntactically valid string representing an ObjectName (see [ObjectName constructor](https://docs.oracle.com/javase/8/docs/api/javax/management/ObjectName.html#ObjectName-java.lang.String-)).                                                                                                                                                                                                                                                    |
+| ATTRIBUTE          | Any well-formed string that can be used as a metric [attribute](https://opentelemetry.io/docs/reference/specification/common/#attribute) key.                                                                                                                                                                                                                                                                                                       |
+| ATTR               | A non-empty string used as a name of the MBean attribute. The MBean attribute value must be a String, otherwise the specified metric attribute will not be used.                                                                                                                                                                                                                                                                                    |
+| PARAM              | A non-empty string used as a property key in the ObjectName identifying the MBean which provides the metric value. If the ObjectName does not have a property with the given key, the specified metric attribute will not be used.                                                                                                                                                                                                                  |
+| METRIC_NAME_PREFIX | Any non-empty string which will be prepended to the specified metric (instrument) names.                                                                                                                                                                                                                                                                                                                                                            |
+| METRIC_NAME        | Any non-empty string. The string, prefixed by the optional prefix (see above) must satisfy [instrument naming rule](https://opentelemetry.io/docs/reference/specification/metrics/api/#instrument-naming-rule).                                                                                                                                                                                                                                     |
+| TYPE               | One of `counter`, `updowncounter`, or `gauge`. The default is `gauge`. This value is case insensitive.                                                                                                                                                                                                                                                                                                                                              |
+| DESCRIPTION        | Any string to be used as human-readable [description](https://opentelemetry.io/docs/reference/specification/metrics/api/#instrument-description) of the metric. If the description is not provided by the rule, an attempt will be made to extract one automatically from the corresponding MBean.                                                                                                                                                  |
+| UNIT               | A string identifying the [unit](https://opentelemetry.io/docs/reference/specification/metrics/api/#instrument-unit) of measurements reported by the metric. Enclose the string in single or double quotes if using unit annotations.                                                                                                                                                                                                                |
+| STR                | Any string to be used directly as the metric attribute value.                                                                                                                                                                                                                                                                                                                                                                                       |
+| BEANATTR           | A non-empty string representing the MBean attribute defining the metric value. The attribute value must be a number. Special dot-notation _attributeName.itemName_ can be used to access numerical items within attributes of [CompositeType](https://docs.oracle.com/javase/8/docs/api/javax/management/openmbean/CompositeType.html). If a dot happens to be an integral part of the MBean attribute name, it must be escaped by backslash (`\`). |
+| BOOL               | A boolean value, either `true` or `false`                                                                                                                                                                                                                                                                                                                                                                                                           |
 
 ## Assumptions and Limitations
 

instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/BeanAttributeExtractor.java

@@ -287,4 +287,27 @@ private String extractStringAttribute(MBeanServerConnection connection, ObjectNa
     }
     return null;
   }
+
+  /**
+   * Wraps provided extractor to filter-out negative values by replacing them with {@literal null}.
+   *
+   * @param extractor extractor to wrap
+   * @return extractor filtering-out negative values
+   */
+  public static BeanAttributeExtractor filterNegativeValues(BeanAttributeExtractor extractor) {
+    return new BeanAttributeExtractor(extractor.baseName, extractor.nameChain) {
+      @Nullable
+      @Override
+      protected Number extractNumericalAttribute(
+          MBeanServerConnection connection, ObjectName objectName) {
+        Number v = super.extractNumericalAttribute(connection, objectName);
+        if (v instanceof Long || v instanceof Integer || v instanceof Short || v instanceof Byte) {
+          return v.longValue() < 0 ? null : v;
+        } else if (v instanceof Double || v instanceof Float) {
+          return v.doubleValue() < 0 ? null : v;
+        }
+        return v;
+      }
+    };
+  }
 }

instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/JmxRule.java

@@ -172,6 +172,14 @@ public MetricDef buildMetricDef() throws Exception {
       StateMapping stateMapping = getEffectiveStateMapping(m, this);
 
       if (stateMapping.isEmpty()) {
+
+        // higher priority to metric level, then jmx rule as fallback
+        boolean dropNegative = getEffectiveDropNegativeValues(m, this);
+
+        if (dropNegative) {
+          attrExtractor = BeanAttributeExtractor.filterNegativeValues(attrExtractor);
+        }
+
         metricExtractors.add(new MetricExtractor(attrExtractor, metricInfo, attributeList));
       } else {
 
@@ -267,4 +275,12 @@ private static StateMapping getEffectiveStateMapping(Metric m, JmxRule rule) {
       return m.getStateMapping();
     }
   }
+
+  private static boolean getEffectiveDropNegativeValues(Metric m, JmxRule rule) {
+    if (m == null || m.getDropNegativeValues() == null) {
+      return Boolean.TRUE.equals(rule.getDropNegativeValues());
+    } else {
+      return Boolean.TRUE.equals(m.getDropNegativeValues());
+    }
+  }
 }

instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/MetricStructure.java

@@ -13,6 +13,7 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import javax.annotation.Nullable;
 
 /**
  * An abstract class containing skeletal info about Metrics:
@@ -48,6 +49,7 @@ abstract class MetricStructure {
   private static final String STATE_MAPPING_DEFAULT = "*";
   private String sourceUnit;
   private String unit;
+  @Nullable private Boolean dropNegativeValues;
 
   private MetricInfo.Type metricType;
   private List<MetricAttribute> metricAttributes;
@@ -76,6 +78,15 @@ public void setUnit(String unit) {
     this.unit = validateUnit(unit.trim());
   }
 
+  public void setDropNegativeValues(Boolean dropNegativeValues) {
+    this.dropNegativeValues = dropNegativeValues;
+  }
+
+  @Nullable
+  public Boolean getDropNegativeValues() {
+    return dropNegativeValues;
+  }
+
   private static void addMappedValue(
       StateMapping.Builder builder, String stateValue, String stateKey) {
     if (stateValue.equals(STATE_MAPPING_DEFAULT)) {

instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/RuleParser.java

@@ -146,6 +146,9 @@ private static void parseMetricStructure(
     if (sourceUnit != null) {
       out.setSourceUnit(sourceUnit);
     }
+
+    Boolean dropNegativeValues = (Boolean) metricStructureYaml.remove("dropNegativeValues");
+    out.setDropNegativeValues(dropNegativeValues);
   }
 
   private static void failOnExtraKeys(Map<String, Object> yaml) {

instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/engine/AttributeExtractorTest.java

@@ -13,7 +13,10 @@
 import javax.management.ObjectName;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 class AttributeExtractorTest {
 
@@ -41,6 +44,9 @@ public interface Test1MBean {
   }
 
   private static class Test1 implements Test1MBean {
+
+    boolean negativeValues;
+
     @Override
     public byte getByteAttribute() {
       return 10;
@@ -53,22 +59,22 @@ public short getShortAttribute() {
 
     @Override
     public int getIntAttribute() {
-      return 12;
+      return negativeValues ? -12 : 12;
     }
 
     @Override
     public long getLongAttribute() {
-      return 13;
+      return negativeValues ? -13 : 13;
     }
 
     @Override
     public float getFloatAttribute() {
-      return 14.0f;
+      return negativeValues ? -14.0f : 14.0f;
     }
 
     @Override
     public double getDoubleAttribute() {
-      return 15.0;
+      return negativeValues ? -15.0 : 15.0;
     }
 
     @Override
@@ -93,13 +99,13 @@ private enum DummyEnum {
 
   private static final String DOMAIN = "otel.jmx.test";
   private static final String OBJECT_NAME = "otel.jmx.test:type=Test1";
+  private static final Test1 test1 = new Test1();
   private static ObjectName objectName;
   private static MBeanServer theServer;
 
   @BeforeAll
   static void setUp() throws Exception {
     theServer = MBeanServerFactory.createMBeanServer(DOMAIN);
-    Test1 test1 = new Test1();
     objectName = new ObjectName(OBJECT_NAME);
     theServer.registerMBean(test1, objectName);
   }
@@ -110,6 +116,11 @@ static void tearDown() {
     theServer = null;
   }
 
+  @BeforeEach
+  void reset() {
+    test1.negativeValues = false;
+  }
+
   @Test
   void testSetup() {
     Set<ObjectName> set = theServer.queryNames(objectName, null);
@@ -220,18 +231,39 @@ void testStringAttribute() {
   }
 
   @Test
-  void testBooleanAttribute() throws Exception {
+  void testBooleanAttribute() {
     BeanAttributeExtractor extractor = BeanAttributeExtractor.fromName("BooleanAttribute");
     AttributeInfo info = extractor.getAttributeInfo(theServer, objectName);
     assertThat(info).isNull();
     assertThat(extractor.extractValue(theServer, objectName)).isEqualTo("true");
   }
 
   @Test
-  void testEnumAttribute() throws Exception {
+  void testEnumAttribute() {
     BeanAttributeExtractor extractor = BeanAttributeExtractor.fromName("EnumAttribute");
     AttributeInfo info = extractor.getAttributeInfo(theServer, objectName);
     assertThat(info).isNull();
     assertThat(extractor.extractValue(theServer, objectName)).isEqualTo("ENUM_VALUE");
   }
+
+  @ParameterizedTest
+  @ValueSource(strings = {"LongAttribute", "IntAttribute", "DoubleAttribute", "FloatAttribute"})
+  void testNegativeFilter(String attributeName) {
+    test1.negativeValues = false;
+    BeanAttributeExtractor rawExtractor = BeanAttributeExtractor.fromName(attributeName);
+    BeanAttributeExtractor filteringExtractor =
+        BeanAttributeExtractor.filterNegativeValues(rawExtractor);
+    assertThat(rawExtractor.extractNumericalAttribute(theServer, objectName))
+        .isNotNull()
+        .describedAs("when value is not negative original numerical value is returned")
+        .isEqualTo(filteringExtractor.extractNumericalAttribute(theServer, objectName));
+
+    test1.negativeValues = true;
+    Number rawValue = rawExtractor.extractNumericalAttribute(theServer, objectName);
+    assertThat(rawValue).isNotNull();
+    assertThat(rawValue.doubleValue()).isNegative();
+    assertThat(filteringExtractor.extractNumericalAttribute(theServer, objectName))
+        .describedAs("negative value should be filtered")
+        .isNull();
+  }
 }