diff --git a/instrumentation/jmx-metrics/javaagent/README.md b/instrumentation/jmx-metrics/javaagent/README.md index 9481cc61a9d2..8272e893e327 100644 --- a/instrumentation/jmx-metrics/javaagent/README.md +++ b/instrumentation/jmx-metrics/javaagent/README.md @@ -282,6 +282,31 @@ In the particular case where only two values are defined, we can simplify furthe off: '*' ``` +### Metric attributes modifiers + +JMX attributes values may require modification or normalization in order to fit semantic conventions. + +For example, with JVM memory, the `java.lang:name=*,type=MemoryPool` MBeans have `type` attribute with either `HEAP` or `NON_HEAP` value. +However, in the semantic conventions the metric attribute `jvm.memory.type` should be lower-cased to fit the `jvm.memory.used` definition, in this case we can +apply the `lowercase` metric attribute transformation as follows: + + +```yaml +--- +rules: + - bean: java.lang:name=*,type=MemoryPool + mapping: + Usage.used: + type: updowncounter + metric: jvm.memory.used + unit: By + metricAttribute: + jvm.memory.pool.name : param(name) + jvm.memory.type: lowercase(beanattr(type)) +``` + +For now, only the `lowercase` transformation is supported, other additions might be added in the future if needed. + ### 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. @@ -293,6 +318,7 @@ rules: # start of list of configuration rules metricAttribute: # optional metric attributes, they apply to all metrics below : param() # is used as the key to extract value from actual ObjectName : beanattr() # is used as the MBean attribute name to extract the value + : const() # is used as a constant 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 diff --git a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricAttribute.java b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricAttribute.java index 22cc9ced3ba0..a143524c11da 100644 --- a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricAttribute.java +++ b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricAttribute.java @@ -5,6 +5,7 @@ package io.opentelemetry.instrumentation.jmx.engine; +import javax.annotation.Nullable; import javax.management.MBeanServerConnection; import javax.management.ObjectName; @@ -30,7 +31,8 @@ public String getAttributeName() { return name; } - String acquireAttributeValue(MBeanServerConnection connection, ObjectName objectName) { + @Nullable + public String acquireAttributeValue(MBeanServerConnection connection, ObjectName objectName) { return extractor.extractValue(connection, objectName); } } diff --git a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricAttributeExtractor.java b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricAttributeExtractor.java index c359aacfe764..4ed0fa6eec97 100644 --- a/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricAttributeExtractor.java +++ b/instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricAttributeExtractor.java @@ -5,6 +5,7 @@ package io.opentelemetry.instrumentation.jmx.engine; +import java.util.Locale; import javax.annotation.Nullable; import javax.management.MBeanServerConnection; import javax.management.ObjectName; @@ -35,10 +36,31 @@ static MetricAttributeExtractor fromObjectNameParameter(String parameterKey) { if (parameterKey.isEmpty()) { throw new IllegalArgumentException("Empty parameter name"); } - return (dummy, objectName) -> objectName.getKeyProperty(parameterKey); + return (dummy, objectName) -> { + if (objectName == null) { + throw new IllegalArgumentException("Missing object name"); + } + return objectName.getKeyProperty(parameterKey); + }; } static MetricAttributeExtractor fromBeanAttribute(String attributeName) { return BeanAttributeExtractor.fromName(attributeName); } + + /** + * Provides an extractor that will transform provided string value in lower-case + * + * @param extractor extractor to wrap + * @return lower-case extractor + */ + static MetricAttributeExtractor toLowerCase(MetricAttributeExtractor extractor) { + return (connection, objectName) -> { + String value = extractor.extractValue(connection, objectName); + if (value != null) { + value = value.toLowerCase(Locale.ROOT); + } + return value; + }; + } } 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 9967ac080c73..4f444cd05cf5 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 @@ -136,36 +136,84 @@ private List addMetricAttributes(Map metricAttr return list; } - private static MetricAttribute buildMetricAttribute(String key, String target) { + // package protected for testing + static MetricAttribute buildMetricAttribute(String key, String target) { + String errorMsg = + String.format("Invalid metric attribute expression for '%s' : '%s'", key, target); + + String targetExpr = target; + + // an optional modifier may wrap the target + // - lowercase(param(STRING)) + boolean lowercase = false; + String lowerCaseExpr = tryParseFunction("lowercase", targetExpr, target); + if (lowerCaseExpr != null) { + lowercase = true; + targetExpr = lowerCaseExpr; + } + + // // The recognized forms of target are: // - param(STRING) // - beanattr(STRING) // - const(STRING) // where STRING is the name of the corresponding parameter key, attribute name, - // or the direct value to use - int k = target.indexOf(')'); - - // Check for one of the cases as above - if (target.startsWith("param(")) { - if (k > 0) { - String jmxAttribute = target.substring(6, k).trim(); - return new MetricAttribute( - key, MetricAttributeExtractor.fromObjectNameParameter(jmxAttribute)); - } - } else if (target.startsWith("beanattr(")) { - if (k > 0) { - String jmxAttribute = target.substring(9, k).trim(); - return new MetricAttribute(key, MetricAttributeExtractor.fromBeanAttribute(jmxAttribute)); + // or the constant value to use + + MetricAttributeExtractor extractor = null; + + String paramName = tryParseFunction("param", targetExpr, target); + if (paramName != null) { + extractor = MetricAttributeExtractor.fromObjectNameParameter(paramName); + } + + if (extractor == null) { + String attributeName = tryParseFunction("beanattr", targetExpr, target); + if (attributeName != null) { + extractor = MetricAttributeExtractor.fromBeanAttribute(attributeName); } - } else if (target.startsWith("const(")) { - if (k > 0) { - String constantValue = target.substring(6, k).trim(); - return new MetricAttribute(key, MetricAttributeExtractor.fromConstant(constantValue)); + } + + if (extractor == null) { + String constantValue = tryParseFunction("const", targetExpr, target); + if (constantValue != null) { + extractor = MetricAttributeExtractor.fromConstant(constantValue); } } - String msg = "Invalid metric attribute specification for '" + key + "': " + target; - throw new IllegalArgumentException(msg); + if (extractor == null) { + // expression did not match any supported syntax + throw new IllegalArgumentException(errorMsg); + } + + if (lowercase) { + extractor = MetricAttributeExtractor.toLowerCase(extractor); + } + return new MetricAttribute(key, extractor); + } + + /** + * Parses a function expression for metric attributes + * + * @param function function name to attempt parsing from expression + * @param expression expression to parse + * @param errorMsg error message to use when syntax error is present + * @return {@literal null} if expression does not start with function + * @throws IllegalArgumentException if expression syntax is invalid + */ + private static String tryParseFunction(String function, String expression, String errorMsg) { + if (!expression.startsWith(function)) { + return null; + } + String expr = expression.substring(function.length()).trim(); + if (expr.charAt(0) != '(' || expr.charAt(expr.length() - 1) != ')') { + throw new IllegalArgumentException(errorMsg); + } + expr = expr.substring(1, expr.length() - 1).trim(); + if (expr.isEmpty()) { + throw new IllegalArgumentException(errorMsg); + } + return expr.trim(); } private MetricAttribute buildStateMetricAttribute(String key, Map stateMap) { 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 15b10df7cba9..9ea5df7a02b2 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 @@ -379,7 +379,6 @@ void testConf8() throws Exception { @Test void testStateMetricConf() throws Exception { JmxConfig config = parseConf(CONF9); - assertThat(config).isNotNull(); List rules = config.getRules(); assertThat(rules).hasSize(1); @@ -452,6 +451,39 @@ void testStateMetricConf() throws Exception { }); } + private static final String CONF10 = + "--- # keep stupid spotlessJava at bay\n" + + "rules:\n" + + " - bean: my-test:type=10_Hello\n" + + " mapping:\n" + + " jmxAttribute:\n" + + " type: counter\n" + + " metric: my_metric\n" + + " metricAttribute:\n" + + " to_lower_const: lowercase(const(Hello))\n" + + " to_lower_attribute: lowercase(beanattr(beanAttribute))\n" + + " to_lower_param: lowercase(param(type))\n"; + + @Test + void attributeValueLowercase() { + + JmxConfig config = parseConf(CONF10); + + List rules = config.getRules(); + assertThat(rules).hasSize(1); + JmxRule jmxRule = rules.get(0); + + assertThat(jmxRule.getBean()).isEqualTo("my-test:type=10_Hello"); + Metric metric = jmxRule.getMapping().get("jmxAttribute"); + assertThat(metric.getMetricType()).isEqualTo(MetricInfo.Type.COUNTER); + assertThat(metric.getMetric()).isEqualTo("my_metric"); + assertThat(metric.getMetricAttribute()) + .hasSize(3) + .containsEntry("to_lower_const", "lowercase(const(Hello))") + .containsEntry("to_lower_attribute", "lowercase(beanattr(beanAttribute))") + .containsEntry("to_lower_param", "lowercase(param(type))"); + } + @Test void testEmptyConf() { JmxConfig config = parseConf(EMPTY_CONF); diff --git a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/yaml/MetricStructureTest.java b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/yaml/MetricStructureTest.java new file mode 100644 index 000000000000..fa37327b8189 --- /dev/null +++ b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/yaml/MetricStructureTest.java @@ -0,0 +1,97 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.jmx.yaml; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import io.opentelemetry.instrumentation.jmx.engine.MetricAttribute; +import javax.management.MBeanAttributeInfo; +import javax.management.MBeanInfo; +import javax.management.MBeanServerConnection; +import javax.management.ObjectName; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; + +public class MetricStructureTest { + + @ParameterizedTest + @CsvSource({"const(Hello),Hello", "lowercase(const(Hello)),hello"}) + void metricAttribute_constant(String target, String expectedValue) { + MetricAttribute ma = MetricStructure.buildMetricAttribute("name", target); + assertThat(ma.getAttributeName()).isEqualTo("name"); + assertThat(ma.isStateAttribute()).isFalse(); + assertThat(ma.acquireAttributeValue(null, null)).isEqualTo(expectedValue); + } + + @ParameterizedTest + @CsvSource({ + "beanattr(beanAttribute),Hello,Hello", + "lowercase(beanattr(beanAttribute)),Hello,hello", + }) + void metricAttribute_beanAttribute(String target, String value, String expectedValue) + throws Exception { + MetricAttribute ma = MetricStructure.buildMetricAttribute("name", target); + assertThat(ma.getAttributeName()).isEqualTo("name"); + assertThat(ma.isStateAttribute()).isFalse(); + + ObjectName objectName = new ObjectName("test:name=_beanAttribute"); + MBeanServerConnection mockConnection = mock(MBeanServerConnection.class); + + MBeanInfo mockBeanInfo = mock(MBeanInfo.class); + when(mockBeanInfo.getAttributes()) + .thenReturn( + new MBeanAttributeInfo[] { + new MBeanAttributeInfo("beanAttribute", "java.lang.String", "", true, false, false) + }); + when(mockConnection.getMBeanInfo(objectName)).thenReturn(mockBeanInfo); + when(mockConnection.getAttribute(objectName, "beanAttribute")).thenReturn(value); + + assertThat(ma.acquireAttributeValue(mockConnection, objectName)).isEqualTo(expectedValue); + } + + @ParameterizedTest + @CsvSource({ + "param(name),Hello,Hello", + "lowercase(param(name)),Hello,hello", + }) + void metricAttribute_beanParam(String target, String value, String expectedValue) + throws Exception { + MetricAttribute ma = MetricStructure.buildMetricAttribute("name", target); + assertThat(ma.getAttributeName()).isEqualTo("name"); + assertThat(ma.isStateAttribute()).isFalse(); + + ObjectName objectName = new ObjectName("test:name=" + value); + MBeanServerConnection mockConnection = mock(MBeanServerConnection.class); + + assertThat(ma.acquireAttributeValue(mockConnection, objectName)).isEqualTo(expectedValue); + } + + @ParameterizedTest + @ValueSource( + strings = { + "missing(name)", // non-existing target + "param()", // missing parameter + "param( )", // missing parameter with empty string + "param(name)a", // something after parenthesis + "lowercase()", // misng target in modifier + "lowercase(param(name)", // missing parenthesis for modifier + "lowercase(missing(name))", // non-existing target within modifier + "lowercase(param())", // missing parameter in modifier + "lowercase(param( ))", // missing parameter in modifier with empty string + "lowercase(param))", // missing parenthesis within modifier + }) + void invalidTargetSyntax(String target) { + assertThatThrownBy(() -> MetricStructure.buildMetricAttribute("metric_attribute", target)) + .isInstanceOf(IllegalArgumentException.class) + .describedAs( + "exception should be thrown with original expression to help end-user understand the syntax error") + .hasMessageContaining(target); + } +}