Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support dots in jmx attribute names #2921

Merged
merged 4 commits into from
Feb 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
Expand All @@ -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)");
Expand All @@ -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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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() {}
}
1 change: 1 addition & 0 deletions settings.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
13 changes: 13 additions & 0 deletions smoke-tests/apps/DotInJmxMetric/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -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")
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
}
Original file line number Diff line number Diff line change
@@ -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!";
}
}
Original file line number Diff line number Diff line change
@@ -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 {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"role": {
"name": "testrolename",
"instance": "testroleinstance"
},
"sampling": {
"percentage": 100
},
"jmxMetrics": [
{
"name": "NameWithDot",
"objectName": "test:name=X",
"attribute": "NestedObject\\.Value"
}
],
"metricIntervalSeconds": 5
}
Original file line number Diff line number Diff line change
@@ -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>