diff --git a/instrumentation/jmx-metrics/javaagent/README.md b/instrumentation/jmx-metrics/javaagent/README.md index b91ce05a7b79..6bd867d6d91d 100644 --- a/instrumentation/jmx-metrics/javaagent/README.md +++ b/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: # optional, useful for avoiding specifying metric names below unit: # optional, redefines the default unit for the whole rule type: # optional, redefines the default type for the whole rule + dropNegativeValues: # optional, redefines if negative values are dropped for the whole rule mapping: : # an MBean attribute name defining the metric value metric: # metric name will be type: # optional, the default type is gauge desc: # optional unit: # optional + dropNegativeValues: # optional, defines if negative values are dropped for the metric metricAttribute: # optional, will be used in addition to the shared metric attributes above : const() # direct value for the metric attribute : # 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 diff --git a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/BeanAttributeExtractor.java b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/BeanAttributeExtractor.java index 95b750be615e..cb75e81f7711 100644 --- a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/BeanAttributeExtractor.java +++ b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/BeanAttributeExtractor.java @@ -8,6 +8,7 @@ import static java.util.logging.Level.FINE; import static java.util.logging.Level.INFO; +import io.opentelemetry.instrumentation.jmx.yaml.StateMapping; import java.util.ArrayList; import java.util.List; import java.util.logging.Level; @@ -103,7 +104,7 @@ private static void verifyAndAddNameSegment(List segments, StringBuilder segments.add(newSegment); } - public BeanAttributeExtractor(String baseName, String... nameChain) { + private BeanAttributeExtractor(String baseName, String... nameChain) { if (baseName == null || nameChain == null) { throw new IllegalArgumentException("null argument for BeanAttributeExtractor"); } @@ -287,4 +288,49 @@ private String extractStringAttribute(MBeanServerConnection connection, ObjectNa } return null; } + + /** + * Provides a bean attribute extractor to filter-out negative values by replacing them with + * {@literal null}. + * + * @param extractor original extractor + * @return equivalent 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; + } + }; + } + + public static BeanAttributeExtractor forStateMetric( + BeanAttributeExtractor extractor, String key, StateMapping stateMapping) { + return new BeanAttributeExtractor(extractor.baseName, extractor.nameChain) { + @Override + protected Object getSampleValue(MBeanServerConnection connection, ObjectName objectName) { + // metric actual type is sampled in the discovery process, so we have to + // make this extractor as extracting integers. + return 0; + } + + @Nullable + @Override + protected Number extractNumericalAttribute( + MBeanServerConnection connection, ObjectName objectName) { + String rawStateValue = extractor.extractValue(connection, objectName); + String mappedStateValue = stateMapping.getStateValue(rawStateValue); + return key.equals(mappedStateValue) ? 1 : 0; + } + }; + } } diff --git a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/JmxRule.java b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/JmxRule.java index 2bb0c4b631e3..993bd4d181a2 100644 --- a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/JmxRule.java +++ b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/JmxRule.java @@ -19,7 +19,6 @@ import java.util.Map; import java.util.Set; import javax.annotation.Nullable; -import javax.management.MBeanServerConnection; import javax.management.MalformedObjectNameException; import javax.management.ObjectName; @@ -172,6 +171,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 { @@ -206,25 +213,8 @@ private static List createStateMappingExtractors( } } - BeanAttributeExtractor stateMetricExtractor = - new BeanAttributeExtractor(attrExtractor.getAttributeName()) { - - @Override - protected Object getSampleValue( - MBeanServerConnection connection, ObjectName objectName) { - // metric actual type is sampled in the discovery process, so we have to - // make this extractor as extracting integers. - return 0; - } - - @Override - protected Number extractNumericalAttribute( - MBeanServerConnection connection, ObjectName objectName) { - String rawStateValue = attrExtractor.extractValue(connection, objectName); - String mappedStateValue = stateMapping.getStateValue(rawStateValue); - return key.equals(mappedStateValue) ? 1 : 0; - } - }; + BeanAttributeExtractor extractor = + BeanAttributeExtractor.forStateMetric(attrExtractor, key, stateMapping); // state metric are always up/down counters MetricInfo stateMetricInfo = @@ -235,8 +225,7 @@ protected Number extractNumericalAttribute( metricInfo.getUnit(), MetricInfo.Type.UPDOWNCOUNTER); - extractors.add( - new MetricExtractor(stateMetricExtractor, stateMetricInfo, stateMetricAttributes)); + extractors.add(new MetricExtractor(extractor, stateMetricInfo, stateMetricAttributes)); } return extractors; } @@ -267,4 +256,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()); + } + } } diff --git a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/MetricStructure.java b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/MetricStructure.java index 980a78545f87..88d3916c8804 100644 --- a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/MetricStructure.java +++ b/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 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)) { diff --git a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/RuleParser.java b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/RuleParser.java index d7042b80d0a6..eb361d05f401 100644 --- a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/RuleParser.java +++ b/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 yaml) { diff --git a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/engine/AttributeExtractorTest.java b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/engine/AttributeExtractorTest.java index e162ed123d12..eedbecabd370 100644 --- a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/engine/AttributeExtractorTest.java +++ b/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 set = theServer.queryNames(objectName, null); @@ -220,7 +231,7 @@ void testStringAttribute() { } @Test - void testBooleanAttribute() throws Exception { + void testBooleanAttribute() { BeanAttributeExtractor extractor = BeanAttributeExtractor.fromName("BooleanAttribute"); AttributeInfo info = extractor.getAttributeInfo(theServer, objectName); assertThat(info).isNull(); @@ -228,10 +239,31 @@ void testBooleanAttribute() throws Exception { } @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(); + } } diff --git a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/engine/RuleParserTest.java b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/engine/RuleParserTest.java index 23883672f912..e0396dc290ae 100644 --- a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/engine/RuleParserTest.java +++ b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/engine/RuleParserTest.java @@ -29,6 +29,8 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; class RuleParserTest { private static RuleParser parser; @@ -87,6 +89,10 @@ void testConf2() { .containsEntry("LABEL_KEY1", "param(PARAMETER)") .containsEntry("LABEL_KEY2", "beanattr(ATTRIBUTE)"); + assertThat(def1.getDropNegativeValues()) + .describedAs("dropping negative values should not be set") + .isNull(); + Map attr = def1.getMapping(); assertThat(attr) .hasSize(4) @@ -99,6 +105,9 @@ void testConf2() { assertThat(m1.getSourceUnit()).isNull(); assertThat(m1.getUnit()).isEqualTo("UNIT1"); assertThat(m1.getMetricAttribute()).containsExactly(entry("LABEL_KEY3", "const(CONSTANT)")); + assertThat(m1.getDropNegativeValues()) + .describedAs("dropping negative values should not be set") + .isNull(); Metric m2 = attr.get("ATTRIBUTE2"); assertThat(m2).isNotNull(); @@ -501,6 +510,75 @@ void testEmptyConf() { assertThat(config.getRules()).isEmpty(); } + private static final String CONF11 = + "--- # keep stupid spotlessJava at bay\n" + + "rules:\n" + + " - bean: my-test:type=11_Hello\n" + + " type: counter\n" + + " mapping:\n" + + " negativeDropped:\n" + + " dropNegativeValues: true\n" + + " metric: negative_drop\n" + + " negativeKept:\n" + + " dropNegativeValues: false\n" + + " metric: negative_keep\n"; + + @ParameterizedTest + @ValueSource(ints = {-1, 0, 1}) + void negativeValueFiltering(int value) throws Exception { + JmxConfig config = parseConf(CONF11); + + List rules = config.getRules(); + assertThat(rules).hasSize(1); + JmxRule rule = rules.get(0); + + assertThat(rule.getBean()).isEqualTo("my-test:type=11_Hello"); + + // test that negative filtering is being applied + + MetricDef metricDef = rule.buildMetricDef(); + assertThat(metricDef).isNotNull(); + + MBeanServerConnection mockConnection = mock(MBeanServerConnection.class); + ObjectName objectName = new ObjectName(rule.getBean()); + + // mock attribute values + when(mockConnection.getAttribute(objectName, "negativeDropped")).thenReturn(value); + when(mockConnection.getAttribute(objectName, "negativeKept")).thenReturn(value); + + // mock attribute discovery + MBeanInfo mockBeanInfo = mock(MBeanInfo.class); + when(mockBeanInfo.getAttributes()) + .thenReturn( + new MBeanAttributeInfo[] { + new MBeanAttributeInfo( + "negativeDropped", "java.lang.Integer", "", true, false, false), + new MBeanAttributeInfo("negativeKept", "java.lang.Integer", "", true, false, false) + }); + when(mockConnection.getMBeanInfo(objectName)).thenReturn(mockBeanInfo); + + assertThat(metricDef.getMetricExtractors()).hasSize(2); + Number filteredValue = + metricDef + .getMetricExtractors() + .get(0) + .getMetricValueExtractor() + .extractNumericalAttribute(mockConnection, objectName); + Number unFilteredValue = + metricDef + .getMetricExtractors() + .get(1) + .getMetricValueExtractor() + .extractNumericalAttribute(mockConnection, objectName); + + if (value < 0) { + assertThat(filteredValue).isNull(); + assertThat(unFilteredValue).isEqualTo(value); + } else { + assertThat(filteredValue).isEqualTo(unFilteredValue).isEqualTo(value); + } + } + private static void checkConstantMetricAttribute( MetricAttribute attribute, String expectedName, String expectedValue) { assertThat(attribute.getAttributeName()).isEqualTo(expectedName);