Support dots in jmx attribute names (#2921)

agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/perfcounter/AvailableJmxMetricLogger.java

@@ -114,7 +114,7 @@ private static Set<String> getAttributes(MBeanServer server, ObjectName objectNa
     Set<String> attributes = new HashSet<>();
     for (MBeanAttributeInfo attribute : mbeanInfo.getAttributes()) {
       if (!attribute.isReadable()) {
-        attributes.add(attribute.getName() + " (not readable)");
+        attributes.add(escapedName(attribute) + " (not readable)");
         continue;
       }
       try {
@@ -125,7 +125,7 @@ private static Set<String> getAttributes(MBeanServer server, ObjectName objectNa
         // "java.lang.UnsupportedOperationException: CollectionUsage threshold is not supported"
         // and available jmx metrics are already only logged at debug
         logger.trace(e.getMessage(), e);
-        attributes.add(attribute.getName() + " (exception)");
+        attributes.add(escapedName(attribute) + " (exception)");
       }
     }
     return attributes;
@@ -148,14 +148,14 @@ private static List<String> getAttributes(MBeanAttributeInfo attribute, @Nullabl
       }
     }
 
-    return singletonList(attribute.getName() + " (" + valueType(value) + ")");
+    return singletonList(escapedName(attribute) + " (" + valueType(value) + ")");
   }
 
   private static List<String> getCompositeTypeAttributes(
       MBeanAttributeInfo attribute, @Nullable Object compositeData, CompositeType compositeType) {
     List<String> attributes = new ArrayList<>();
     for (String itemName : compositeType.keySet()) {
-      String attributeName = attribute.getName() + "." + itemName;
+      String attributeName = escapedName(attribute) + "." + itemName;
       OpenType<?> itemType = compositeType.getType(itemName);
       if (itemType == null) {
         attributes.add(attributeName + " (null)");
@@ -173,6 +173,10 @@ private static List<String> getCompositeTypeAttributes(
     return attributes;
   }
 
+  private static String escapedName(MBeanAttributeInfo attribute) {
+    return attribute.getName().replace(".", "\\.");
+  }
+
   private static String valueType(@Nullable Object value) {
     if (value instanceof Number) {
       return "number";

agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/perfcounter/JmxDataFetcher.java

@@ -72,18 +72,14 @@ private static List<Object> fetch(
           InstanceNotFoundException {
     ArrayList<Object> result = new ArrayList<>();
 
-    String[] inners = attributeName.split("\\.");
+    List<String> inners = splitByDot(attributeName);
 
     for (ObjectName object : objects) {
-
-      Object value;
-
-      if (inners.length == 1) {
-        value = server.getAttribute(object, attributeName);
-      } else {
-        value = server.getAttribute(object, inners[0]);
+      Object value = server.getAttribute(object, inners.get(0));
+      if (inners.size() > 1) {
         if (value != null) {
-          value = ((CompositeData) value).get(inners[1]);
+          // TODO (trask) will support more nesting after moving to upstream otel jmx component
+          value = ((CompositeData) value).get(inners.get(1));
         }
       }
       if (value != null) {
@@ -94,5 +90,54 @@ private static List<Object> fetch(
     return result;
   }
 
+  // This code is copied in from upstream otel java instrumentation repository
+  // until we move to upstream version
+  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);
+          }
+        }
+      }
+
+      // The returned list is never empty ...
+      verifyAndAddNameSegment(components, currentSegment);
+
+    } catch (IllegalArgumentException unused) {
+      // Drop the original exception. We have more meaningful context here.
+      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();
+    }
+    segments.add(newSegment);
+  }
+
   private JmxDataFetcher() {}
 }

settings.gradle.kts

@@ -92,6 +92,7 @@ hideFromDependabot(":smoke-tests:apps:CustomDimensions")
 hideFromDependabot(":smoke-tests:apps:CustomInstrumentation")
 hideFromDependabot(":smoke-tests:apps:DiagnosticExtension:MockExtension")
 hideFromDependabot(":smoke-tests:apps:DiagnosticExtension")
+hideFromDependabot(":smoke-tests:apps:DotInJmxMetric")
 hideFromDependabot(":smoke-tests:apps:gRPC")
 hideFromDependabot(":smoke-tests:apps:HeartBeat")
 hideFromDependabot(":smoke-tests:apps:HttpClients")

smoke-tests/apps/ActuatorMetrics/src/smokeTest/java/com/microsoft/applicationinsights/smoketest/ActuatorMetricsTest.java

@@ -31,7 +31,7 @@ abstract class ActuatorMetricsTest {
   @Test
   @TargetUri("/test")
   void doMostBasicTest() throws Exception {
-    testing.mockedIngestion.waitForItems("RequestData", 1);
+    testing.getTelemetry(0);
 
     List<Envelope> metricItems =
         testing.mockedIngestion.waitForItems(

smoke-tests/apps/DotInJmxMetric/build.gradle.kts

@@ -0,0 +1,13 @@
+plugins {
+  id("ai.smoke-test-war")
+}
+
+dependencies {
+  implementation("org.springframework.boot:spring-boot-starter-web:2.1.7.RELEASE") {
+    exclude("org.springframework.boot", "spring-boot-starter-tomcat")
+  }
+  // this dependency is needed to make wildfly happy
+  implementation("org.reactivestreams:reactive-streams:1.0.3")
+
+  implementation("org.weakref:jmxutils:1.22")
+}

smoke-tests/apps/DotInJmxMetric/src/main/java/com/microsoft/applicationinsights/smoketestapp/SpringBootApp.java

@@ -0,0 +1,41 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+package com.microsoft.applicationinsights.smoketestapp;
+
+import java.lang.management.ManagementFactory;
+import org.springframework.boot.autoconfigure.SpringBootApplication;
+import org.springframework.boot.builder.SpringApplicationBuilder;
+import org.springframework.boot.web.servlet.support.SpringBootServletInitializer;
+import org.weakref.jmx.MBeanExporter;
+import org.weakref.jmx.Managed;
+import org.weakref.jmx.Nested;
+
+@SpringBootApplication
+public class SpringBootApp extends SpringBootServletInitializer {
+
+  @Override
+  protected SpringApplicationBuilder configure(SpringApplicationBuilder applicationBuilder) {
+
+    MBeanExporter exporter = new MBeanExporter(ManagementFactory.getPlatformMBeanServer());
+    exporter.export("test:name=X", new NestedExample());
+
+    return applicationBuilder.sources(SpringBootApp.class);
+  }
+
+  public static class NestedExample {
+    private final NestedObject nestedObject = new NestedObject();
+
+    @Nested
+    public NestedObject getNestedObject() {
+      return nestedObject;
+    }
+
+    public static final class NestedObject {
+      @Managed
+      public int getValue() {
+        return 5;
+      }
+    }
+  }
+}

smoke-tests/apps/DotInJmxMetric/src/main/java/com/microsoft/applicationinsights/smoketestapp/TestController.java

@@ -0,0 +1,21 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+package com.microsoft.applicationinsights.smoketestapp;
+
+import org.springframework.web.bind.annotation.GetMapping;
+import org.springframework.web.bind.annotation.RestController;
+
+@RestController
+public class TestController {
+
+  @GetMapping("/")
+  public String root() {
+    return "OK";
+  }
+
+  @GetMapping("/test")
+  public String test() {
+    return "OK!";
+  }
+}

smoke-tests/apps/DotInJmxMetric/src/smokeTest/java/com/microsoft/applicationinsights/smoketest/DotInJmxMetricTest.java

@@ -0,0 +1,68 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+package com.microsoft.applicationinsights.smoketest;
+
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_11;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_17;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_19;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_20;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_8;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import com.microsoft.applicationinsights.smoketest.schemav2.Data;
+import com.microsoft.applicationinsights.smoketest.schemav2.DataPoint;
+import com.microsoft.applicationinsights.smoketest.schemav2.Envelope;
+import com.microsoft.applicationinsights.smoketest.schemav2.MetricData;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+@UseAgent
+abstract class DotInJmxMetricTest {
+
+  @RegisterExtension
+  static final SmokeTestExtension testing =
+      SmokeTestExtension.builder().setSelfDiagnosticsLevel("debug").build();
+
+  @Test
+  @TargetUri("/test")
+  void doMostBasicTest() throws Exception {
+    testing.getTelemetry(0);
+
+    List<Envelope> metricItems =
+        testing.mockedIngestion.waitForItems(
+            envelope -> isMetricWithName(envelope, "NameWithDot"), 1, 10, TimeUnit.SECONDS);
+
+    MetricData data = (MetricData) ((Data<?>) metricItems.get(0).getData()).getBaseData();
+    List<DataPoint> points = data.getMetrics();
+    assertThat(points).hasSize(1);
+
+    DataPoint point = points.get(0);
+    assertThat(point.getValue()).isEqualTo(5);
+  }
+
+  private static boolean isMetricWithName(Envelope envelope, String metricName) {
+    if (!envelope.getData().getBaseType().equals("MetricData")) {
+      return false;
+    }
+    MetricData md = SmokeTestExtension.getBaseData(envelope);
+    return metricName.equals(md.getMetrics().get(0).getName());
+  }
+
+  @Environment(TOMCAT_8_JAVA_8)
+  static class Tomcat8Java8Test extends DotInJmxMetricTest {}
+
+  @Environment(TOMCAT_8_JAVA_11)
+  static class Tomcat8Java11Test extends DotInJmxMetricTest {}
+
+  @Environment(TOMCAT_8_JAVA_17)
+  static class Tomcat8Java17Test extends DotInJmxMetricTest {}
+
+  @Environment(TOMCAT_8_JAVA_19)
+  static class Tomcat8Java19Test extends DotInJmxMetricTest {}
+
+  @Environment(TOMCAT_8_JAVA_20)
+  static class Tomcat8Java20Test extends DotInJmxMetricTest {}
+}

smoke-tests/apps/DotInJmxMetric/src/smokeTest/resources/applicationinsights.json

@@ -0,0 +1,17 @@
+{
+  "role": {
+    "name": "testrolename",
+    "instance": "testroleinstance"
+  },
+  "sampling": {
+    "percentage": 100
+  },
+  "jmxMetrics": [
+    {
+      "name": "NameWithDot",
+      "objectName": "test:name=X",
+      "attribute": "NestedObject\\.Value"
+    }
+  ],
+  "metricIntervalSeconds": 5
+}

smoke-tests/apps/DotInJmxMetric/src/smokeTest/resources/logback-test.xml

@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<configuration>
+  <appender name="CONSOLE" class="ch.qos.logback.core.ConsoleAppender">
+    <encoder>
+      <pattern>%d{yyyy-MM-dd HH:mm:ss.SSS} %-5level %logger{36} - %msg%n</pattern>
+    </encoder>
+  </appender>
+  <root level="warn">
+    <appender-ref ref="CONSOLE"/>
+  </root>
+</configuration>