jmx metrics attribute lowercase modifier (open-telemetry#13385)

Author: SylvainJuge

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
       <ATTRIBUTE1>: param(<PARAM>)    # <PARAM> is used as the key to extract value from actual ObjectName
       <ATTRIBUTE2>: beanattr(<ATTR>)  # <ATTR> is used as the MBean attribute name to extract the value
+      <ATTRIBUTE3>: const(<CONST>)    # <CONST> is used as a constant
     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

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);
   }
 }

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;
+    };
+  }
 }

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

@@ -136,36 +136,84 @@ private List<MetricAttribute> addMetricAttributes(Map<String, Object> 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) {

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<JmxRule> 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<JmxRule> 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);

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);
+  }
+}