diff --git a/instrumentation/jmx-metrics/README.md b/instrumentation/jmx-metrics/README.md index 22997b56e1a4..82b520147e26 100644 --- a/instrumentation/jmx-metrics/README.md +++ b/instrumentation/jmx-metrics/README.md @@ -371,6 +371,40 @@ rules: desc: Recent CPU utilization for the process as reported by the JVM. ``` +### Aggregation over multiple MBean instances + +Sometimes, multiple MBean instances are registered with distinct names and we need to capture the aggregate value over all the instances. + +For example, the JVM exposes the number of GC executions in the `CollectionCount` attribute of the MBean instances returned by `java.lang:name=*,type=GarbageCollector` query, +there are multiple instances each with a distinct value for the `name` parameter. + +To capture the total number of GC executions across all those instances in a single metric, we can use the following configuration +where the `name` parameter in the MBean name is NOT mapped to a metric attribute. + +```yaml + - bean: java.lang:name=*,type=GarbageCollector + mapping: + CollectionCount: + metric: custom.jvm.gc.count + unit: '{collection}' + type: counter + desc: JVM GC execution count +``` + +When two or more MBean parameters are used, it is also possible to perform a partial aggregation: +- parameters not mapped as metric attributes are discarded +- parameters mapped as metric attributes with `param()` are preserved +- values are aggregated with mapped metric attributes + +The applied aggregation depends on the metric type: +- `counter` or `updowncounter`: sum aggregation +- `gauge`: last-value aggregation + +As a consequence, it is not recommended to use it for `gauge` metrics when querying more than one MBean instance as it would produce unpredictable results. + +When there is only a single MBean instance, using a `gauge` metric produces the expected value, hence allowing to avoid mapping all the MBean parameters +to metric attributes. + ### 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. diff --git a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/engine/MetricAggregationTest.java b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/engine/MetricAggregationTest.java new file mode 100644 index 000000000000..7875947b7673 --- /dev/null +++ b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/engine/MetricAggregationTest.java @@ -0,0 +1,246 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.jmx.engine; + +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; +import static org.junit.jupiter.params.ParameterizedInvocationConstants.ARGUMENTS_PLACEHOLDER; + +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension; +import io.opentelemetry.sdk.testing.assertj.LongPointAssert; +import io.opentelemetry.sdk.testing.assertj.MetricAssert; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Locale; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; +import javax.annotation.Nullable; +import javax.management.InstanceNotFoundException; +import javax.management.MBeanRegistrationException; +import javax.management.MBeanServer; +import javax.management.MBeanServerFactory; +import javax.management.MalformedObjectNameException; +import javax.management.ObjectName; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +public class MetricAggregationTest { + + @SuppressWarnings({"unused", "checkstyle:AbbreviationAsWordInName"}) + public interface HelloMBean { + + int getValue(); + } + + public static class Hello implements HelloMBean { + + private final int value; + + public Hello(int value) { + this.value = value; + } + + @Override + public int getValue() { + return value; + } + } + + @RegisterExtension + static final InstrumentationExtension testing = LibraryInstrumentationExtension.create(); + + // used to generate non-conflicting metric names at runtime + private static final AtomicInteger metricCounter = new AtomicInteger(0); + + private static final String DOMAIN = "otel.jmx.test"; + private static MBeanServer theServer; + + @BeforeAll + static void setUp() { + theServer = MBeanServerFactory.createMBeanServer(DOMAIN); + } + + @AfterAll + static void tearDown() { + MBeanServerFactory.releaseMBeanServer(theServer); + } + + @AfterEach + void after() throws Exception { + ObjectName objectName = new ObjectName(DOMAIN + ":type=" + Hello.class.getSimpleName() + ",*"); + theServer + .queryMBeans(objectName, null) + .forEach( + instance -> { + try { + theServer.unregisterMBean(instance.getObjectName()); + } catch (InstanceNotFoundException | MBeanRegistrationException e) { + throw new RuntimeException(e); + } + }); + } + + private static ObjectName getObjectName(@Nullable String a, @Nullable String b) + throws MalformedObjectNameException { + StringBuilder parts = new StringBuilder(); + parts.append("otel.jmx.test:type=").append(Hello.class.getSimpleName()); + if (a != null) { + parts.append(",a=").append(a); + } + if (b != null) { + parts.append(",b=").append(b); + } + return new ObjectName(parts.toString()); + } + + static List metricTypes() { + return Arrays.asList( + MetricInfo.Type.COUNTER, MetricInfo.Type.UPDOWNCOUNTER, MetricInfo.Type.GAUGE); + } + + @ParameterizedTest(name = ARGUMENTS_PLACEHOLDER) + @MethodSource("metricTypes") + void singleInstance(MetricInfo.Type metricType) throws Exception { + ObjectName bean = getObjectName(null, null); + theServer.registerMBean(new Hello(42), bean); + + String metricName = generateMetricName(metricType); + startTestMetric(metricName, bean.toString(), Collections.emptyList(), metricType); + waitAndAssertMetric( + metricName, metricType, point -> point.hasValue(42).hasAttributes(Attributes.empty())); + } + + @ParameterizedTest(name = ARGUMENTS_PLACEHOLDER) + @MethodSource("metricTypes") + void aggregateOneParam(MetricInfo.Type metricType) throws Exception { + theServer.registerMBean(new Hello(42), getObjectName("value1", null)); + theServer.registerMBean(new Hello(37), getObjectName("value2", null)); + + String bean = getObjectName("*", null).toString(); + String metricName = generateMetricName(metricType); + startTestMetric(metricName, bean, Collections.emptyList(), metricType); + + // last-value aggregation produces an unpredictable result unless a single mbean instance is + // used + // test here is only used as a way to document behavior and should not be considered a feature + long expected = metricType == MetricInfo.Type.GAUGE ? 37 : 79; + waitAndAssertMetric( + metricName, + metricType, + point -> point.hasValue(expected).hasAttributes(Attributes.empty())); + } + + @ParameterizedTest(name = ARGUMENTS_PLACEHOLDER) + @MethodSource("metricTypes") + void aggregateMultipleParams(MetricInfo.Type metricType) throws Exception { + theServer.registerMBean(new Hello(1), getObjectName("1", "x")); + theServer.registerMBean(new Hello(2), getObjectName("2", "y")); + theServer.registerMBean(new Hello(3), getObjectName("3", "x")); + theServer.registerMBean(new Hello(4), getObjectName("4", "y")); + + String bean = getObjectName("*", "*").toString(); + String metricName = generateMetricName(metricType); + startTestMetric(metricName, bean, Collections.emptyList(), metricType); + + // last-value aggregation produces an unpredictable result unless a single mbean instance is + // used + // test here is only used as a way to document behavior and should not be considered a feature + long expected = metricType == MetricInfo.Type.GAUGE ? 1 : 10; + waitAndAssertMetric( + metricName, + metricType, + point -> point.hasValue(expected).hasAttributes(Attributes.empty())); + } + + @ParameterizedTest(name = ARGUMENTS_PLACEHOLDER) + @MethodSource("metricTypes") + void partialAggregateMultipleParams(MetricInfo.Type metricType) throws Exception { + theServer.registerMBean(new Hello(1), getObjectName("1", "x")); + theServer.registerMBean(new Hello(2), getObjectName("2", "y")); + theServer.registerMBean(new Hello(3), getObjectName("3", "x")); + theServer.registerMBean(new Hello(4), getObjectName("4", "y")); + + String bean = getObjectName("*", "*").toString(); + + List attributes = + Collections.singletonList( + new MetricAttribute( + "test.metric.param", MetricAttributeExtractor.fromObjectNameParameter("b"))); + String metricName = generateMetricName(metricType); + startTestMetric(metricName, bean, attributes, metricType); + + AttributeKey metricAttribute = AttributeKey.stringKey("test.metric.param"); + if (metricType == MetricInfo.Type.GAUGE) { + waitAndAssertMetric( + metricName, + metricType, + point -> point.hasValue(1).hasAttribute(metricAttribute, "x"), + point -> point.hasValue(4).hasAttribute(metricAttribute, "y")); + } else { + waitAndAssertMetric( + metricName, + metricType, + point -> point.hasValue(4).hasAttribute(metricAttribute, "x"), + point -> point.hasValue(6).hasAttribute(metricAttribute, "y")); + } + } + + private static String generateMetricName(MetricInfo.Type metricType) { + // generate a sequential metric name that prevents naming conflicts and unexpected behaviors + return "test.metric" + + metricCounter.incrementAndGet() + + "." + + metricType.name().toLowerCase(Locale.ROOT); + } + + @SafeVarargs + @SuppressWarnings("varargs") + private static void waitAndAssertMetric( + String metricName, MetricInfo.Type metricType, Consumer... pointAsserts) { + + testing.waitAndAssertMetrics( + "io.opentelemetry.jmx", + metricName, + metrics -> + metrics.anySatisfy( + metricData -> { + MetricAssert metricAssert = + assertThat(metricData).hasDescription("description").hasUnit("1"); + if (metricType == MetricInfo.Type.GAUGE) { + metricAssert.hasLongGaugeSatisfying( + gauge -> gauge.hasPointsSatisfying(pointAsserts)); + } else { + metricAssert.hasLongSumSatisfying(sum -> sum.hasPointsSatisfying(pointAsserts)); + } + })); + } + + private static void startTestMetric( + String metricName, String mbean, List attributes, MetricInfo.Type metricType) + throws MalformedObjectNameException { + JmxMetricInsight metricInsight = JmxMetricInsight.createService(testing.getOpenTelemetry(), 0); + MetricConfiguration metricConfiguration = new MetricConfiguration(); + List extractors = new ArrayList<>(); + + MetricInfo metricInfo = new MetricInfo(metricName, "description", null, "1", metricType); + BeanAttributeExtractor beanExtractor = BeanAttributeExtractor.fromName("Value"); + MetricExtractor extractor = new MetricExtractor(beanExtractor, metricInfo, attributes); + extractors.add(extractor); + MetricDef metricDef = + new MetricDef(BeanGroup.forBeans(Collections.singletonList(mbean)), extractors); + + metricConfiguration.addMetricDef(metricDef); + metricInsight.startLocal(metricConfiguration); + } +}