7706 Handle JMX MBean attributes with embedded dots (#7841)

Allowing to use backslash as an escape character within MBean attribute
names.

instrumentation/jmx-metrics/javaagent/README.md

@@ -271,7 +271,7 @@ The following table explains the used terms with more details.
 | 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). |
+| 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 (`\`). |
 
 ## Assumptions and Limitations
 

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

@@ -8,6 +8,8 @@
 import static java.util.logging.Level.FINE;
 import static java.util.logging.Level.INFO;
 
+import java.util.ArrayList;
+import java.util.List;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.annotation.Nullable;
@@ -42,31 +44,63 @@ public class BeanAttributeExtractor implements MetricAttributeExtractor {
    * @throws IllegalArgumentException if the attribute name is malformed
    */
   public static BeanAttributeExtractor fromName(String rawName) {
-    if (rawName.isEmpty()) {
-      throw new IllegalArgumentException("Empty attribute name");
+    List<String> segments = splitByDot(rawName);
+    String baseName = segments.remove(0);
+    if (segments.isEmpty()) {
+      return new BeanAttributeExtractor(baseName);
     }
+    return new BeanAttributeExtractor(baseName, segments.toArray(new String[segments.size()]));
+  }
 
-    // Check if a CompositeType value is expected
-    int k = rawName.indexOf('.');
-    if (k < 0) {
-      return new BeanAttributeExtractor(rawName);
-    }
+  /*
+   * Split a given name into segments, assuming that a dot is used to separate the segments.
+   * However, a dot preceded by a backslash is not a separator.
+   */
+  private static List<String> splitByDot(String rawName) {
+    List<String> components = new ArrayList<>();
+    try {
+      StringBuilder currentSegment = new StringBuilder();
+      boolean escaped = false;
+      for (int i = 0; i < rawName.length(); ++i) {
+        char ch = rawName.charAt(i);
+        if (escaped) {
+          // Allow only '\' and '.' to be escaped
+          if (ch != '\\' && ch != '.') {
+            throw new IllegalArgumentException(
+                "Invalid escape sequence in attribute name '" + rawName + "'");
+          }
+          currentSegment.append(ch);
+          escaped = false;
+        } else {
+          if (ch == '\\') {
+            escaped = true;
+          } else if (ch == '.') {
+            // this is a segment separator
+            verifyAndAddNameSegment(components, currentSegment);
+            currentSegment = new StringBuilder();
+          } else {
+            currentSegment.append(ch);
+          }
+        }
+      }
 
-    // Set up extraction from CompositeType values
-    String baseName = rawName.substring(0, k).trim();
-    String[] components = rawName.substring(k + 1).split("\\.");
+      // The returned list is never empty ...
+      verifyAndAddNameSegment(components, currentSegment);
 
-    // sanity check
-    if (baseName.isEmpty()) {
+    } catch (IllegalArgumentException unused) {
+      // Drop the original exception. We have more meaningful context here.
       throw new IllegalArgumentException("Invalid attribute name '" + rawName + "'");
     }
-    for (int j = 0; j < components.length; ++j) {
-      components[j] = components[j].trim();
-      if (components[j].isEmpty()) {
-        throw new IllegalArgumentException("Invalid attribute name '" + rawName + "'");
-      }
+
+    return components;
+  }
+
+  private static void verifyAndAddNameSegment(List<String> segments, StringBuilder candidate) {
+    String newSegment = candidate.toString().trim();
+    if (newSegment.isEmpty()) {
+      throw new IllegalArgumentException();
     }
-    return new BeanAttributeExtractor(baseName, components);
+    segments.add(newSegment);
   }
 
   public BeanAttributeExtractor(String baseName, String... nameChain) {
@@ -77,8 +111,11 @@ public BeanAttributeExtractor(String baseName, String... nameChain) {
     this.nameChain = nameChain;
   }
 
-  /** Get a human readable name of the attribute to extract. Useful for logging or debugging. */
-  String getAttributeName() {
+  /**
+   * Get a human readable name of the attribute to extract. Used to form the metric name if none is
+   * provided. Also useful for logging or debugging.
+   */
+  public String getAttributeName() {
     if (nameChain.length > 0) {
       StringBuilder builder = new StringBuilder(baseName);
       for (String component : nameChain) {

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

@@ -143,19 +143,21 @@ public MetricDef buildMetricDef() throws Exception {
     MetricExtractor[] metricExtractors = new MetricExtractor[attrNames.size()];
     int n = 0;
     for (String attributeName : attrNames) {
+      BeanAttributeExtractor attrExtractor = BeanAttributeExtractor.fromName(attributeName);
+      // This is essentially the same as 'attributeName' but with escape characters removed
+      String niceAttributeName = attrExtractor.getAttributeName();
       MetricInfo metricInfo;
       Metric m = mapping.get(attributeName);
       if (m == null) {
         metricInfo =
             new MetricInfo(
-                prefix == null ? attributeName : (prefix + attributeName),
+                prefix == null ? niceAttributeName : (prefix + niceAttributeName),
                 null,
                 getUnit(),
                 getMetricType());
       } else {
-        metricInfo = m.buildMetricInfo(prefix, attributeName, getUnit(), getMetricType());
+        metricInfo = m.buildMetricInfo(prefix, niceAttributeName, getUnit(), getMetricType());
       }
-      BeanAttributeExtractor attrExtractor = BeanAttributeExtractor.fromName(attributeName);
 
       List<MetricAttribute> attributeList;
       List<MetricAttribute> ownAttributes = getAttributeList();

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

@@ -102,103 +102,103 @@ void testSetup() throws Exception {
 
   @Test
   void testByteAttribute() throws Exception {
-    BeanAttributeExtractor extractor = new BeanAttributeExtractor("ByteAttribute");
+    BeanAttributeExtractor extractor = BeanAttributeExtractor.fromName("ByteAttribute");
     AttributeInfo info = extractor.getAttributeInfo(theServer, objectName);
     assertThat(info == null).isFalse();
     assertThat(info.usesDoubleValues()).isFalse();
   }
 
   @Test
   void testByteAttributeValue() throws Exception {
-    BeanAttributeExtractor extractor = new BeanAttributeExtractor("ByteAttribute");
+    BeanAttributeExtractor extractor = BeanAttributeExtractor.fromName("ByteAttribute");
     Number number = extractor.extractNumericalAttribute(theServer, objectName);
     assertThat(number == null).isFalse();
     assertThat(number.longValue() == 10).isTrue();
   }
 
   @Test
   void testShortAttribute() throws Exception {
-    BeanAttributeExtractor extractor = new BeanAttributeExtractor("ShortAttribute");
+    BeanAttributeExtractor extractor = BeanAttributeExtractor.fromName("ShortAttribute");
     AttributeInfo info = extractor.getAttributeInfo(theServer, objectName);
     assertThat(info == null).isFalse();
     assertThat(info.usesDoubleValues()).isFalse();
   }
 
   @Test
   void testShortAttributeValue() throws Exception {
-    BeanAttributeExtractor extractor = new BeanAttributeExtractor("ShortAttribute");
+    BeanAttributeExtractor extractor = BeanAttributeExtractor.fromName("ShortAttribute");
     Number number = extractor.extractNumericalAttribute(theServer, objectName);
     assertThat(number == null).isFalse();
     assertThat(number.longValue() == 11).isTrue();
   }
 
   @Test
   void testIntAttribute() throws Exception {
-    BeanAttributeExtractor extractor = new BeanAttributeExtractor("IntAttribute");
+    BeanAttributeExtractor extractor = BeanAttributeExtractor.fromName("IntAttribute");
     AttributeInfo info = extractor.getAttributeInfo(theServer, objectName);
     assertThat(info == null).isFalse();
     assertThat(info.usesDoubleValues()).isFalse();
   }
 
   @Test
   void testIntAttributeValue() throws Exception {
-    BeanAttributeExtractor extractor = new BeanAttributeExtractor("IntAttribute");
+    BeanAttributeExtractor extractor = BeanAttributeExtractor.fromName("IntAttribute");
     Number number = extractor.extractNumericalAttribute(theServer, objectName);
     assertThat(number == null).isFalse();
     assertThat(number.longValue() == 12).isTrue();
   }
 
   @Test
   void testLongAttribute() throws Exception {
-    BeanAttributeExtractor extractor = new BeanAttributeExtractor("LongAttribute");
+    BeanAttributeExtractor extractor = BeanAttributeExtractor.fromName("LongAttribute");
     AttributeInfo info = extractor.getAttributeInfo(theServer, objectName);
     assertThat(info == null).isFalse();
     assertThat(info.usesDoubleValues()).isFalse();
   }
 
   @Test
   void testLongAttributeValue() throws Exception {
-    BeanAttributeExtractor extractor = new BeanAttributeExtractor("LongAttribute");
+    BeanAttributeExtractor extractor = BeanAttributeExtractor.fromName("LongAttribute");
     Number number = extractor.extractNumericalAttribute(theServer, objectName);
     assertThat(number == null).isFalse();
     assertThat(number.longValue() == 13).isTrue();
   }
 
   @Test
   void testFloatAttribute() throws Exception {
-    BeanAttributeExtractor extractor = new BeanAttributeExtractor("FloatAttribute");
+    BeanAttributeExtractor extractor = BeanAttributeExtractor.fromName("FloatAttribute");
     AttributeInfo info = extractor.getAttributeInfo(theServer, objectName);
     assertThat(info == null).isFalse();
     assertThat(info.usesDoubleValues()).isTrue();
   }
 
   @Test
   void testFloatAttributeValue() throws Exception {
-    BeanAttributeExtractor extractor = new BeanAttributeExtractor("FloatAttribute");
+    BeanAttributeExtractor extractor = BeanAttributeExtractor.fromName("FloatAttribute");
     Number number = extractor.extractNumericalAttribute(theServer, objectName);
     assertThat(number == null).isFalse();
     assertThat(number.doubleValue() == 14.0).isTrue(); // accurate representation
   }
 
   @Test
   void testDoubleAttribute() throws Exception {
-    BeanAttributeExtractor extractor = new BeanAttributeExtractor("DoubleAttribute");
+    BeanAttributeExtractor extractor = BeanAttributeExtractor.fromName("DoubleAttribute");
     AttributeInfo info = extractor.getAttributeInfo(theServer, objectName);
     assertThat(info == null).isFalse();
     assertThat(info.usesDoubleValues()).isTrue();
   }
 
   @Test
   void testDoubleAttributeValue() throws Exception {
-    BeanAttributeExtractor extractor = new BeanAttributeExtractor("DoubleAttribute");
+    BeanAttributeExtractor extractor = BeanAttributeExtractor.fromName("DoubleAttribute");
     Number number = extractor.extractNumericalAttribute(theServer, objectName);
     assertThat(number == null).isFalse();
     assertThat(number.doubleValue() == 15.0).isTrue(); // accurate representation
   }
 
   @Test
   void testStringAttribute() throws Exception {
-    BeanAttributeExtractor extractor = new BeanAttributeExtractor("StringAttribute");
+    BeanAttributeExtractor extractor = BeanAttributeExtractor.fromName("StringAttribute");
     AttributeInfo info = extractor.getAttributeInfo(theServer, objectName);
     assertThat(info == null).isTrue();
   }

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

@@ -281,6 +281,37 @@ void testConf7() throws Exception {
 
   private static final String EMPTY_CONF = "---\n";
 
+  private static final String CONF8 = // a dot in attribute name
+      "---                                   # keep stupid spotlessJava at bay\n"
+          + "rules:\n"
+          + "  - bean: my-test:type=8\n"
+          + "    mapping:\n"
+          + "      Attr.with\\.dot:\n";
+
+  @Test
+  void testConf8() throws Exception {
+    InputStream is = new ByteArrayInputStream(CONF8.getBytes(StandardCharsets.UTF_8));
+    JmxConfig config = parser.loadConfig(is);
+    assertThat(config).isNotNull();
+
+    List<JmxRule> defs = config.getRules();
+    assertThat(defs).hasSize(1);
+
+    MetricDef metricDef = defs.get(0).buildMetricDef();
+    assertThat(metricDef).isNotNull();
+    assertThat(metricDef.getMetricExtractors()).hasSize(1);
+
+    MetricExtractor m1 = metricDef.getMetricExtractors()[0];
+    assertThat(m1.getMetricValueExtractor().getAttributeName()).isEqualTo("Attr.with.dot");
+    assertThat(m1.getAttributes()).isEmpty();
+
+    MetricInfo mb1 = m1.getInfo();
+    // Make sure the metric name has no backslash
+    assertThat(mb1.getMetricName()).isEqualTo("Attr.with.dot");
+    assertThat(mb1.getType()).isEqualTo(MetricInfo.Type.GAUGE);
+    assertThat(mb1.getUnit()).isNull();
+  }
+
   @Test
   void testEmptyConf() {
     InputStream is = new ByteArrayInputStream(EMPTY_CONF.getBytes(StandardCharsets.UTF_8));