From d081c2946eec7636d272bb70a75fc0eb12553b33 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Tue, 25 Feb 2025 16:09:41 +0100 Subject: [PATCH 01/15] add test code to test yaml jmx metrics --- .../jmx-metrics/javaagent/README.md | 2 +- .../jmx-metrics/library/build.gradle.kts | 26 ++ .../jmx/rules/MetricsVerifier.java | 124 ++++++++ .../jmx/rules/TargetSystemTest.java | 299 ++++++++++++++++++ .../rules/assertions/AttributeMatcher.java | 61 ++++ .../assertions/AttributeMatcherGroup.java | 60 ++++ .../rules/assertions/DataPointAttributes.java | 53 ++++ .../jmx/rules/assertions/JmxAssertj.java | 18 ++ .../jmx/rules/assertions/MetricAssert.java | 266 ++++++++++++++++ 9 files changed, 908 insertions(+), 1 deletion(-) create mode 100644 instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/MetricsVerifier.java create mode 100644 instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TargetSystemTest.java create mode 100644 instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/assertions/AttributeMatcher.java create mode 100644 instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/assertions/AttributeMatcherGroup.java create mode 100644 instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/assertions/DataPointAttributes.java create mode 100644 instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/assertions/JmxAssertj.java create mode 100644 instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/assertions/MetricAssert.java diff --git a/instrumentation/jmx-metrics/javaagent/README.md b/instrumentation/jmx-metrics/javaagent/README.md index b91ce05a7b79..753c62da8239 100644 --- a/instrumentation/jmx-metrics/javaagent/README.md +++ b/instrumentation/jmx-metrics/javaagent/README.md @@ -302,7 +302,7 @@ rules: unit: By metricAttribute: jvm.memory.pool.name : param(name) - jvm.memory.type: lowercase(beanattr(type)) + jvm.memory.type: lowercase(beanattr(Type)) ``` For now, only the `lowercase` transformation is supported, other additions might be added in the future if needed. diff --git a/instrumentation/jmx-metrics/library/build.gradle.kts b/instrumentation/jmx-metrics/library/build.gradle.kts index 375e5e77d932..86f0cb0515a8 100644 --- a/instrumentation/jmx-metrics/library/build.gradle.kts +++ b/instrumentation/jmx-metrics/library/build.gradle.kts @@ -1,3 +1,5 @@ +import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar + plugins { id("otel.library-instrumentation") } @@ -6,4 +8,28 @@ dependencies { implementation("org.snakeyaml:snakeyaml-engine") testImplementation(project(":testing-common")) + testImplementation("org.testcontainers:testcontainers") + + testImplementation("org.testcontainers:junit-jupiter") + testImplementation("com.linecorp.armeria:armeria-junit5:1.31.3") + testImplementation("com.linecorp.armeria:armeria-junit5:1.31.3") + testImplementation("com.linecorp.armeria:armeria-grpc:1.31.3") + testImplementation("io.opentelemetry.proto:opentelemetry-proto:1.5.0-alpha") +} + +tasks { + test { + // get packaged agent jar for testing + val shadowTask = project(":javaagent").tasks.named("shadowJar").get() + + dependsOn(shadowTask) + + inputs.files(layout.files(shadowTask)) + .withPropertyName("javaagent") + .withNormalizer(ClasspathNormalizer::class) + + doFirst { + jvmArgs("-Dio.opentelemetry.javaagent.path=${shadowTask.archiveFile.get()}") + } + } } diff --git a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/MetricsVerifier.java b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/MetricsVerifier.java new file mode 100644 index 000000000000..b89659806d65 --- /dev/null +++ b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/MetricsVerifier.java @@ -0,0 +1,124 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.jmx.rules; + +import static io.opentelemetry.instrumentation.jmx.rules.assertions.JmxAssertj.assertThat; +import static org.assertj.core.api.Assertions.fail; + +import com.google.errorprone.annotations.CanIgnoreReturnValue; +import io.opentelemetry.instrumentation.jmx.rules.assertions.MetricAssert; +import io.opentelemetry.proto.metrics.v1.Metric; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Consumer; +import java.util.stream.Collectors; + +public class MetricsVerifier { + + private final Map> assertions = new HashMap<>(); + private boolean strictMode = true; + + private MetricsVerifier() {} + + /** + * Create instance of MetricsVerifier configured to fail verification if any metric was not + * verified because there is no assertion defined for it. This behavior can be changed by calling + * {@link #disableStrictMode()} method. + * + * @return new instance of MetricsVerifier + * @see #disableStrictMode() + */ + public static MetricsVerifier create() { + return new MetricsVerifier(); + } + + /** + * Disable strict checks of metric assertions. It means that all metrics checks added after + * calling this method will not enforce asserting all metric properties and will not detect + * duplicate property assertions. Also, there will be no error reported if any of metrics was + * skipped because no assertion was added for it. + * + * @return this + * @see #verify(List) + * @see #add(String, Consumer) + */ + @CanIgnoreReturnValue + public MetricsVerifier disableStrictMode() { + strictMode = false; + return this; + } + + /** + * Add assertion for given metric + * + * @param metricName name of metric to be verified by provided assertion + * @param assertion an assertion to verify properties of the metric + * @return this + */ + @CanIgnoreReturnValue + public MetricsVerifier add(String metricName, Consumer assertion) { + if (assertions.containsKey(metricName)) { + throw new IllegalArgumentException("Duplicate assertion for metric " + metricName); + } + assertions.put( + metricName, + metric -> { + MetricAssert metricAssert = assertThat(metric); + metricAssert.setStrict(strictMode); + assertion.accept(metricAssert); + metricAssert.strictCheck(); + }); + return this; + } + + /** + * Execute all defined assertions against provided list of metrics. Error is reported if any of + * defined assertions failed. Error is also reported if any of expected metrics was not present in + * the metrics list, unless strict mode is disabled with {@link #disableStrictMode()}. + * + * @param metrics list of metrics to be verified + * @see #add(String, Consumer) + * @see #disableStrictMode() + */ + public void verify(List metrics) { + verifyAllExpectedMetricsWereReceived(metrics); + + Set unverifiedMetrics = new HashSet<>(); + + for (Metric metric : metrics) { + String metricName = metric.getName(); + Consumer assertion = assertions.get(metricName); + + if (assertion != null) { + assertion.accept(metric); + } else { + unverifiedMetrics.add(metricName); + } + } + + if (strictMode && !unverifiedMetrics.isEmpty()) { + fail("Metrics received but not verified because no assertion exists: " + unverifiedMetrics); + } + } + + private void verifyAllExpectedMetricsWereReceived(List metrics) { + Set receivedMetricNames = + metrics.stream().map(Metric::getName).collect(Collectors.toSet()); + Set assertionNames = new HashSet<>(assertions.keySet()); + + assertionNames.removeAll(receivedMetricNames); + if (!assertionNames.isEmpty()) { + fail( + "Metrics expected but not received: " + + assertionNames + + "\nReceived only: " + + receivedMetricNames); + } + } +} diff --git a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TargetSystemTest.java b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TargetSystemTest.java new file mode 100644 index 000000000000..8dcee7738699 --- /dev/null +++ b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TargetSystemTest.java @@ -0,0 +1,299 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.jmx.rules; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; +import static org.awaitility.Awaitility.await; + +import com.linecorp.armeria.server.ServerBuilder; +import com.linecorp.armeria.server.grpc.GrpcService; +import com.linecorp.armeria.testing.junit5.server.ServerExtension; +import io.grpc.stub.StreamObserver; +import io.opentelemetry.instrumentation.jmx.yaml.JmxConfig; +import io.opentelemetry.instrumentation.jmx.yaml.JmxRule; +import io.opentelemetry.instrumentation.jmx.yaml.RuleParser; +import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceRequest; +import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceResponse; +import io.opentelemetry.proto.collector.metrics.v1.MetricsServiceGrpc; +import io.opentelemetry.proto.metrics.v1.Metric; +import io.opentelemetry.proto.metrics.v1.ResourceMetrics; +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.time.Duration; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.LinkedBlockingDeque; +import java.util.stream.Collectors; +import javax.annotation.Nullable; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testcontainers.Testcontainers; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.containers.Network; +import org.testcontainers.containers.output.Slf4jLogConsumer; +import org.testcontainers.utility.MountableFile; + +/** Base class for testing YAML metric definitions with a real target system */ +public class TargetSystemTest { + + private static final Logger logger = LoggerFactory.getLogger(TargetSystemTest.class); + private static final Logger targetSystemLogger = LoggerFactory.getLogger("targetSystem"); + + private static final String AGENT_PATH = "/opentelemetry-instrumentation-javaagent.jar"; + + private static final Network network = Network.newNetwork(); + + private static OtlpGrpcServer otlpServer; + private static Path agentPath; + private static String otlpEndpoint; + + private GenericContainer targetSystem; + private Collection> targetDependencies; + + @BeforeAll + static void beforeAll() { + otlpServer = new OtlpGrpcServer(); + otlpServer.start(); + Testcontainers.exposeHostPorts(otlpServer.httpPort()); + otlpEndpoint = "http://host.testcontainers.internal:" + otlpServer.httpPort(); + + String path = System.getProperty("io.opentelemetry.javaagent.path"); + assertThat(path).isNotNull(); + agentPath = Paths.get(path); + assertThat(agentPath).isReadable().isNotEmptyFile(); + } + + @BeforeEach + void beforeEach() { + otlpServer.reset(); + } + + @AfterEach + void afterEach() { + stop(targetSystem); + targetSystem = null; + + if (targetDependencies != null) { + for (GenericContainer targetDependency : targetDependencies) { + stop(targetDependency); + } + } + targetDependencies = Collections.emptyList(); + } + + private static void stop(@Nullable GenericContainer container) { + if (container != null && container.isRunning()) { + container.stop(); + } + } + + @AfterAll + static void afterAll() { + try { + otlpServer.stop().get(); + } catch (InterruptedException | ExecutionException e) { + throw new RuntimeException(e); + } + } + + protected static String javaAgentJvmArgument() { + return "-javaagent:" + AGENT_PATH; + } + + protected static List javaPropertiesToJvmArgs(Map config) { + return config.entrySet().stream() + .map(e -> String.format("-D%s=%s", e.getKey(), e.getValue())) + .collect(Collectors.toList()); + } + + /** + * Generates otel configuration for JMX testing with instrumentation agent + * + * @param yamlFiles JMX metrics definitions in YAML + * @return map of otel configuration properties for JMX testing + */ + protected static Map otelConfigProperties(List yamlFiles) { + Map config = new HashMap<>(); + // only export metrics + config.put("otel.logs.exporter", "none"); + config.put("otel.traces.exporter", "none"); + config.put("otel.metrics.exporter", "otlp"); + // use test grpc endpoint + config.put("otel.exporter.otlp.endpoint", otlpEndpoint); + config.put("otel.exporter.otlp.protocol", "grpc"); + // short export interval for testing + config.put("otel.metric.export.interval", "5s"); + // disable runtime telemetry metrics + config.put("otel.instrumentation.runtime-telemetry.enabled", "false"); + // set yaml config files to test + config.put("otel.jmx.target", "tomcat"); + config.put( + "otel.jmx.config", + yamlFiles.stream() + .map(TargetSystemTest::containerYamlPath) + .collect(Collectors.joining(","))); + return config; + } + + /** + * Starts the target system + * + * @param target target system to start + */ + protected void startTarget(GenericContainer target) { + startTarget(target, Collections.emptyList()); + } + + /** + * Starts the target system with its dependencies first + * + * @param target target system + * @param targetDependencies dependencies of target system + */ + protected void startTarget( + GenericContainer target, Collection> targetDependencies) { + + // If there are any containers that must be started before target then initialize them. + // Then make target depending on them, so it is started after dependencies + this.targetDependencies = targetDependencies; + for (GenericContainer container : targetDependencies) { + container.withNetwork(network); + target.dependsOn(container); + } + + targetSystem = + target.withLogConsumer(new Slf4jLogConsumer(targetSystemLogger)).withNetwork(network); + targetSystem.start(); + } + + protected static void copyFilesToTarget(GenericContainer target, List yamlFiles) { + // copy agent to target system + target.withCopyFileToContainer(MountableFile.forHostPath(agentPath), AGENT_PATH); + + // copy yaml files to target system + for (String file : yamlFiles) { + String resourcePath = yamlResourcePath(file); + String destPath = containerYamlPath(file); + logger.info("copying yaml from resources {} to container {}", resourcePath, destPath); + target.withCopyFileToContainer(MountableFile.forClasspathResource(resourcePath), destPath); + } + } + + private static String yamlResourcePath(String yaml) { + return "jmx/rules/" + yaml; + } + + private static String containerYamlPath(String yaml) { + return "/" + yaml; + } + + /** + * Validates YAML definition by parsing it to check for syntax errors + * + * @param yaml path to YAML resource (in classpath) + */ + protected void validateYamlSyntax(String yaml) { + String path = yamlResourcePath(yaml); + try (InputStream input = TargetSystemTest.class.getClassLoader().getResourceAsStream(path)) { + JmxConfig config; + // try-catch to provide a slightly better error + try { + config = RuleParser.get().loadConfig(input); + } catch (RuntimeException e) { + fail("Failed to parse yaml file " + path, e); + throw e; + } + + // make sure all the rules in that file are valid + for (JmxRule rule : config.getRules()) { + try { + rule.buildMetricDef(); + } catch (Exception e) { + fail("Failed to build metric definition " + rule.getBean(), e); + } + } + } catch (IOException e) { + fail("Failed to read yaml file " + path, e); + } + } + + protected void verifyMetrics(MetricsVerifier metricsVerifier) { + await() + .atMost(Duration.ofSeconds(60)) + .pollInterval(Duration.ofSeconds(1)) + .untilAsserted( + () -> { + List receivedMetrics = otlpServer.getMetrics(); + assertThat(receivedMetrics).describedAs("No metric received").isNotEmpty(); + + List metrics = + receivedMetrics.stream() + .map(ExportMetricsServiceRequest::getResourceMetricsList) + .flatMap(rm -> rm.stream().map(ResourceMetrics::getScopeMetricsList)) + .flatMap(Collection::stream) + .filter( + // TODO: disabling batch span exporter might help remove unwanted metrics + sm -> sm.getScope().getName().equals("io.opentelemetry.jmx")) + .flatMap(sm -> sm.getMetricsList().stream()) + .collect(Collectors.toList()); + + assertThat(metrics).describedAs("Metrics received but not from JMX").isNotEmpty(); + + metricsVerifier.verify(metrics); + }); + } + + /** Minimal OTLP gRPC backend to capture metrics */ + private static class OtlpGrpcServer extends ServerExtension { + + private final BlockingQueue metricRequests = + new LinkedBlockingDeque<>(); + + List getMetrics() { + return new ArrayList<>(metricRequests); + } + + void reset() { + metricRequests.clear(); + } + + @Override + protected void configure(ServerBuilder sb) { + sb.service( + GrpcService.builder() + .addService( + new MetricsServiceGrpc.MetricsServiceImplBase() { + @Override + public void export( + ExportMetricsServiceRequest request, + StreamObserver responseObserver) { + + // verbose but helpful to diagnose what is received + logger.debug("receiving metrics {}", request); + + metricRequests.add(request); + responseObserver.onNext(ExportMetricsServiceResponse.getDefaultInstance()); + responseObserver.onCompleted(); + } + }) + .build()); + sb.http(0); + } + } +} diff --git a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/assertions/AttributeMatcher.java b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/assertions/AttributeMatcher.java new file mode 100644 index 000000000000..33721e66cc0c --- /dev/null +++ b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/assertions/AttributeMatcher.java @@ -0,0 +1,61 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.jmx.rules.assertions; + +import javax.annotation.Nullable; + +/** Implements functionality of matching data point attributes. */ +public class AttributeMatcher { + private final String attributeName; + @Nullable private final String attributeValue; + + /** + * Create instance used to match data point attribute with any value. + * + * @param attributeName matched attribute name + */ + AttributeMatcher(String attributeName) { + this(attributeName, null); + } + + /** + * Create instance used to match data point attribute with te same name and with the same value. + * + * @param attributeName attribute name + * @param attributeValue attribute value + */ + AttributeMatcher(String attributeName, @Nullable String attributeValue) { + this.attributeName = attributeName; + this.attributeValue = attributeValue; + } + + /** + * Return name of data point attribute that this AttributeMatcher is supposed to match value with. + * + * @return name of validated attribute + */ + public String getAttributeName() { + return attributeName; + } + + @Override + public String toString() { + return attributeValue == null + ? '{' + attributeName + '}' + : '{' + attributeName + '=' + attributeValue + '}'; + } + + /** + * Verify if this matcher is matching provided attribute value. If this matcher holds null value + * then it is matching any attribute value. + * + * @param value a value to be matched + * @return true if this matcher is matching provided value, false otherwise. + */ + boolean matchesValue(String value) { + return attributeValue == null || attributeValue.equals(value); + } +} diff --git a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/assertions/AttributeMatcherGroup.java b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/assertions/AttributeMatcherGroup.java new file mode 100644 index 000000000000..90e6a0da77c5 --- /dev/null +++ b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/assertions/AttributeMatcherGroup.java @@ -0,0 +1,60 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.jmx.rules.assertions; + +import java.util.Collection; +import java.util.Map; +import java.util.stream.Collectors; + +/** Group of attribute matchers */ +public class AttributeMatcherGroup { + + // stored as a Map for easy lookup by name + private final Map matchers; + + /** + * Constructor for a set of attribute matchers + * + * @param matchers collection of matchers to build a group from + * @throws IllegalStateException if there is any duplicate key + */ + AttributeMatcherGroup(Collection matchers) { + this.matchers = + matchers.stream().collect(Collectors.toMap(AttributeMatcher::getAttributeName, m -> m)); + } + + /** + * Checks if attributes match this attribute matcher group + * + * @param attributes attributes to check as map + * @return {@literal true} when the attributes match all attributes from this group + */ + public boolean matches(Map attributes) { + if (attributes.size() != matchers.size()) { + return false; + } + + for (Map.Entry entry : attributes.entrySet()) { + AttributeMatcher matcher = matchers.get(entry.getKey()); + if (matcher == null) { + // no matcher for this key: unexpected key + return false; + } + + if (!matcher.matchesValue(entry.getValue())) { + // value does not match: unexpected value + return false; + } + } + + return true; + } + + @Override + public String toString() { + return matchers.values().toString(); + } +} diff --git a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/assertions/DataPointAttributes.java b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/assertions/DataPointAttributes.java new file mode 100644 index 000000000000..c72099e85eac --- /dev/null +++ b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/assertions/DataPointAttributes.java @@ -0,0 +1,53 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.jmx.rules.assertions; + +import java.util.Arrays; + +/** + * Utility class implementing convenience static methods to construct data point attribute matchers + * and sets of matchers. + */ +public class DataPointAttributes { + private DataPointAttributes() {} + + /** + * Create instance of matcher that should be used to check if data point attribute with given name + * has value identical to the one provided as a parameter (exact match). + * + * @param name name of the data point attribute to check + * @param value expected value of checked data point attribute + * @return instance of matcher + */ + public static AttributeMatcher attribute(String name, String value) { + return new AttributeMatcher(name, value); + } + + /** + * Create instance of matcher that should be used to check if data point attribute with given name + * exists. Any value of the attribute is considered as matching (any value match). + * + * @param name name of the data point attribute to check + * @return instance of matcher + */ + public static AttributeMatcher attributeWithAnyValue(String name) { + return new AttributeMatcher(name); + } + + /** + * Creates a group of attribute matchers that should be used to verify data point attributes. + * + * @param attributes list of matchers to create group. It must contain matchers with unique names. + * @return group of attribute matchers + * @throws IllegalArgumentException if provided list contains two or more matchers with the same + * attribute name + * @see MetricAssert#hasDataPointsWithAttributes(AttributeMatcherGroup...) for detailed + * description off the algorithm used for matching + */ + public static AttributeMatcherGroup attributeGroup(AttributeMatcher... attributes) { + return new AttributeMatcherGroup(Arrays.asList(attributes)); + } +} diff --git a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/assertions/JmxAssertj.java b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/assertions/JmxAssertj.java new file mode 100644 index 000000000000..45a2d0cd6588 --- /dev/null +++ b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/assertions/JmxAssertj.java @@ -0,0 +1,18 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.jmx.rules.assertions; + +import io.opentelemetry.proto.metrics.v1.Metric; + +/** Dedicated Assertj extension to provide convenient fluent API for metrics testing */ +// TODO: we should contribute this back to sdk-testing +// This has been intentionally not named `*Assertions` to prevent checkstyle rule to be triggered +public class JmxAssertj extends org.assertj.core.api.Assertions { + + public static MetricAssert assertThat(Metric metric) { + return new MetricAssert(metric); + } +} diff --git a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/assertions/MetricAssert.java b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/assertions/MetricAssert.java new file mode 100644 index 000000000000..2ead92ae7668 --- /dev/null +++ b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/assertions/MetricAssert.java @@ -0,0 +1,266 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.jmx.rules.assertions; + +import static io.opentelemetry.instrumentation.jmx.rules.assertions.DataPointAttributes.attributeGroup; + +import com.google.errorprone.annotations.CanIgnoreReturnValue; +import io.opentelemetry.proto.common.v1.KeyValue; +import io.opentelemetry.proto.metrics.v1.Metric; +import io.opentelemetry.proto.metrics.v1.NumberDataPoint; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.function.Consumer; +import java.util.stream.Collectors; +import org.assertj.core.api.AbstractAssert; +import org.assertj.core.internal.Integers; +import org.assertj.core.internal.Iterables; +import org.assertj.core.internal.Objects; + +public class MetricAssert extends AbstractAssert { + + private static final Objects objects = Objects.instance(); + private static final Iterables iterables = Iterables.instance(); + private static final Integers integers = Integers.instance(); + + private boolean strict; + + private boolean descriptionChecked; + private boolean unitChecked; + private boolean typeChecked; + private boolean dataPointAttributesChecked; + + MetricAssert(Metric actual) { + super(actual, MetricAssert.class); + } + + public void setStrict(boolean strict) { + this.strict = strict; + } + + public void strictCheck() { + strictCheck("description", /* expectedCheckStatus= */ true, descriptionChecked); + strictCheck("unit", /* expectedCheckStatus= */ true, unitChecked); + strictCheck("type", /* expectedCheckStatus= */ true, typeChecked); + strictCheck( + "data point attributes", /* expectedCheckStatus= */ true, dataPointAttributesChecked); + } + + private void strictCheck( + String metricProperty, boolean expectedCheckStatus, boolean actualCheckStatus) { + if (!strict) { + return; + } + String failMsgPrefix = expectedCheckStatus ? "Missing" : "Duplicate"; + info.description( + "%s assertion on %s for metric '%s'", failMsgPrefix, metricProperty, actual.getName()); + objects.assertEqual(info, actualCheckStatus, expectedCheckStatus); + } + + /** + * Verifies metric description + * + * @param description expected description + * @return this + */ + @CanIgnoreReturnValue + public MetricAssert hasDescription(String description) { + isNotNull(); + + info.description("unexpected description for metric '%s'", actual.getName()); + objects.assertEqual(info, actual.getDescription(), description); + strictCheck("description", /* expectedCheckStatus= */ false, descriptionChecked); + descriptionChecked = true; + return this; + } + + /** + * Verifies metric unit + * + * @param unit expected unit + * @return this + */ + @CanIgnoreReturnValue + public MetricAssert hasUnit(String unit) { + isNotNull(); + + info.description("unexpected unit for metric '%s'", actual.getName()); + objects.assertEqual(info, actual.getUnit(), unit); + strictCheck("unit", /* expectedCheckStatus= */ false, unitChecked); + unitChecked = true; + return this; + } + + /** + * Verifies the metric is a gauge + * + * @return this + */ + @CanIgnoreReturnValue + public MetricAssert isGauge() { + isNotNull(); + + info.description("gauge expected for metric '%s'", actual.getName()); + objects.assertEqual(info, actual.hasGauge(), true); + strictCheck("type", /* expectedCheckStatus= */ false, typeChecked); + typeChecked = true; + return this; + } + + @CanIgnoreReturnValue + private MetricAssert hasSum(boolean monotonic) { + isNotNull(); + + info.description("sum expected for metric '%s'", actual.getName()); + objects.assertEqual(info, actual.hasSum(), true); + + String sumType = monotonic ? "monotonic" : "non-monotonic"; + info.description("sum for metric '%s' is expected to be %s", actual.getName(), sumType); + objects.assertEqual(info, actual.getSum().getIsMonotonic(), monotonic); + return this; + } + + /** + * Verifies the metric is a counter + * + * @return this + */ + @CanIgnoreReturnValue + public MetricAssert isCounter() { + // counters have a monotonic sum as their value can't decrease + hasSum(true); + strictCheck("type", /* expectedCheckStatus= */ false, typeChecked); + typeChecked = true; + return this; + } + + /** + * Verifies the metric is an up-down counter + * + * @return this + */ + @CanIgnoreReturnValue + public MetricAssert isUpDownCounter() { + // up down counters are non-monotonic as their value can increase & decrease + hasSum(false); + strictCheck("type", /* expectedCheckStatus= */ false, typeChecked); + typeChecked = true; + return this; + } + + /** + * Verifies that there is no attribute in any of data points. + * + * @return this + */ + @CanIgnoreReturnValue + public MetricAssert hasDataPointsWithoutAttributes() { + isNotNull(); + + return checkDataPoints( + dataPoints -> { + dataPointsCommonCheck(dataPoints); + + // all data points must not have any attribute + for (NumberDataPoint dataPoint : dataPoints) { + info.description( + "no attribute expected on data point for metric '%s'", actual.getName()); + iterables.assertEmpty(info, dataPoint.getAttributesList()); + } + }); + } + + @CanIgnoreReturnValue + private MetricAssert checkDataPoints(Consumer> listConsumer) { + // in practice usually one set of data points is provided but the + // protobuf does not enforce that, so we have to ensure checking at least one + int count = 0; + if (actual.hasGauge()) { + count++; + listConsumer.accept(actual.getGauge().getDataPointsList()); + } + if (actual.hasSum()) { + count++; + listConsumer.accept(actual.getSum().getDataPointsList()); + } + info.description("at least one set of data points expected for metric '%s'", actual.getName()); + integers.assertGreaterThan(info, count, 0); + + strictCheck( + "data point attributes", /* expectedCheckStatus= */ false, dataPointAttributesChecked); + dataPointAttributesChecked = true; + return this; + } + + private void dataPointsCommonCheck(List dataPoints) { + info.description("unable to retrieve data points from metric '%s'", actual.getName()); + objects.assertNotNull(info, dataPoints); + + // at least one data point must be reported + info.description("at least one data point expected for metric '%s'", actual.getName()); + iterables.assertNotEmpty(info, dataPoints); + } + + /** + * Verifies that all metric data points have the same expected one attribute + * + * @param expectedAttribute attribute matcher to validate data points attributes + * @return this + */ + @CanIgnoreReturnValue + public final MetricAssert hasDataPointsWithOneAttribute(AttributeMatcher expectedAttribute) { + return hasDataPointsWithAttributes(attributeGroup(expectedAttribute)); + } + + /** + * Verifies that every data point attributes is matched exactly by one of the matcher groups + * provided. Also, each matcher group must match at least one data point attributes set. Data + * point attributes are matched by matcher group if each attribute is matched by one matcher and + * each matcher matches one attribute. In other words: number of attributes is the same as number + * of matchers and there is 1:1 matching between them. + * + * @param matcherGroups array of attribute matcher groups + * @return this + */ + @CanIgnoreReturnValue + public final MetricAssert hasDataPointsWithAttributes(AttributeMatcherGroup... matcherGroups) { + return checkDataPoints( + dataPoints -> { + dataPointsCommonCheck(dataPoints); + + boolean[] matchedSets = new boolean[matcherGroups.length]; + + // validate each datapoint attributes match exactly one of the provided attributes sets + for (NumberDataPoint dataPoint : dataPoints) { + Map dataPointAttributes = + dataPoint.getAttributesList().stream() + .collect( + Collectors.toMap(KeyValue::getKey, kv -> kv.getValue().getStringValue())); + int matchCount = 0; + for (int i = 0; i < matcherGroups.length; i++) { + if (matcherGroups[i].matches(dataPointAttributes)) { + matchedSets[i] = true; + matchCount++; + } + } + + info.description( + "data point attributes '%s' for metric '%s' must match exactly one of the attribute sets '%s'.\nActual data points: %s", + dataPointAttributes, actual.getName(), Arrays.asList(matcherGroups), dataPoints); + integers.assertEqual(info, matchCount, 1); + } + + // check that all attribute sets matched at least once + for (int i = 0; i < matchedSets.length; i++) { + info.description( + "no data point matched attribute set '%s' for metric '%s'", + matcherGroups[i], actual.getName()); + objects.assertEqual(info, matchedSets[i], true); + } + }); + } +} From dfae1ac014b9c009fef81ce8b9bc1a074aad3c62 Mon Sep 17 00:00:00 2001 From: robsunday Date: Tue, 1 Apr 2025 09:51:48 +0200 Subject: [PATCH 02/15] Tomcat metrics updated --- .../src/main/resources/jmx/rules/tomcat.yaml | 61 ++++++++++--------- 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/instrumentation/jmx-metrics/javaagent/src/main/resources/jmx/rules/tomcat.yaml b/instrumentation/jmx-metrics/javaagent/src/main/resources/jmx/rules/tomcat.yaml index 3f03d8f8ae56..997a0d3d439b 100644 --- a/instrumentation/jmx-metrics/javaagent/src/main/resources/jmx/rules/tomcat.yaml +++ b/instrumentation/jmx-metrics/javaagent/src/main/resources/jmx/rules/tomcat.yaml @@ -6,74 +6,77 @@ rules: - beans: - Catalina:type=GlobalRequestProcessor,name=* - Tomcat:type=GlobalRequestProcessor,name=* - unit: "1" prefix: http.server.tomcat. metricAttribute: name: param(name) mapping: errorCount: - metric: errorCount - type: gauge - desc: The number of errors per second on all request processors + metric: error_count + type: counter + unit: "{error}" + desc: The number of errors. requestCount: - metric: requestCount - type: gauge - desc: The number of requests per second across all request processors + metric: request.count + type: counter + unit: "{request}" + desc: The number of requests processed. maxTime: - metric: maxTime + metric: request.max_time type: gauge - unit: ms - desc: The longest request processing time + sourceUnit: ms + unit: s + desc: The longest request processing time. processingTime: - metric: processingTime + metric: request.processing_time type: counter - unit: ms - desc: Total time for processing all requests + sourceUnit: ms + unit: s + desc: Total time for processing all requests. bytesReceived: - metric: traffic - type: counter - unit: By - desc: The number of bytes transmitted + metric: &metric traffic + type: &type counter + unit: &unit By + desc: &desc The number of bytes transmitted. metricAttribute: direction: const(received) bytesSent: - metric: traffic - type: counter - unit: By - desc: The number of bytes transmitted + metric: *metric + type: *type + unit: *unit + desc: *desc metricAttribute: direction: const(sent) - beans: - Catalina:type=Manager,host=localhost,context=* - Tomcat:type=Manager,host=localhost,context=* - unit: "1" prefix: http.server.tomcat. - type: updowncounter metricAttribute: context: param(context) mapping: activeSessions: metric: sessions.activeSessions - desc: The number of active sessions + type: updowncounter + unit: "{session}" + desc: The number of active sessions. - beans: - Catalina:type=ThreadPool,name=* - Tomcat:type=ThreadPool,name=* - unit: "{threads}" + unit: "{thread}" prefix: http.server.tomcat. type: updowncounter metricAttribute: name: param(name) mapping: currentThreadCount: - metric: threads - desc: Thread Count of the Thread Pool + metric: &metric thread_count + desc: &desc Thread Count of the Thread Pool. metricAttribute: state: const(idle) currentThreadsBusy: - metric: threads - desc: Thread Count of the Thread Pool + metric: *metric + desc: *desc metricAttribute: state: const(busy) From e272d841d3d1cf8f454beb9cee72cfa39e26d74c Mon Sep 17 00:00:00 2001 From: robsunday Date: Fri, 4 Apr 2025 12:46:48 +0200 Subject: [PATCH 03/15] Tomcat test added --- .../src/main/resources/jmx/rules/tomcat.yaml | 28 ++-- .../jmx/rules/TomcatIntegrationTest.java | 127 ++++++++++++++++++ 2 files changed, 141 insertions(+), 14 deletions(-) rename instrumentation/jmx-metrics/{javaagent => library}/src/main/resources/jmx/rules/tomcat.yaml (77%) create mode 100644 instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java diff --git a/instrumentation/jmx-metrics/javaagent/src/main/resources/jmx/rules/tomcat.yaml b/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml similarity index 77% rename from instrumentation/jmx-metrics/javaagent/src/main/resources/jmx/rules/tomcat.yaml rename to instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml index 997a0d3d439b..851a311aae94 100644 --- a/instrumentation/jmx-metrics/javaagent/src/main/resources/jmx/rules/tomcat.yaml +++ b/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml @@ -6,12 +6,12 @@ rules: - beans: - Catalina:type=GlobalRequestProcessor,name=* - Tomcat:type=GlobalRequestProcessor,name=* - prefix: http.server.tomcat. + prefix: tomcat. metricAttribute: - name: param(name) + tomcat.request_processor.name: param(name) mapping: errorCount: - metric: error_count + metric: error.count type: counter unit: "{error}" desc: The number of errors. @@ -21,7 +21,7 @@ rules: unit: "{request}" desc: The number of requests processed. maxTime: - metric: request.max_time + metric: request.duration.max type: gauge sourceUnit: ms unit: s @@ -33,29 +33,29 @@ rules: unit: s desc: Total time for processing all requests. bytesReceived: - metric: &metric traffic + metric: &metric network.io type: &type counter unit: &unit By desc: &desc The number of bytes transmitted. metricAttribute: - direction: const(received) + tomcat.network.io.direction: const(received) bytesSent: metric: *metric type: *type unit: *unit desc: *desc metricAttribute: - direction: const(sent) + tomcat.network.io.direction: const(sent) - beans: - Catalina:type=Manager,host=localhost,context=* - Tomcat:type=Manager,host=localhost,context=* - prefix: http.server.tomcat. + prefix: tomcat. metricAttribute: - context: param(context) + tomcat.web_app_context: param(context) mapping: activeSessions: - metric: sessions.activeSessions + metric: active_session.count type: updowncounter unit: "{session}" desc: The number of active sessions. @@ -64,14 +64,14 @@ rules: - Catalina:type=ThreadPool,name=* - Tomcat:type=ThreadPool,name=* unit: "{thread}" - prefix: http.server.tomcat. + prefix: tomcat. type: updowncounter metricAttribute: - name: param(name) + tomcat.thread_pool.name: param(name) mapping: currentThreadCount: - metric: &metric thread_count - desc: &desc Thread Count of the Thread Pool. + metric: &metric thread.count + desc: &desc Thread count of the thread pool. metricAttribute: state: const(idle) currentThreadsBusy: diff --git a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java new file mode 100644 index 000000000000..9d4e7fa7171f --- /dev/null +++ b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java @@ -0,0 +1,127 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.jmx.rules; + +import static io.opentelemetry.instrumentation.jmx.rules.assertions.DataPointAttributes.attribute; +import static io.opentelemetry.instrumentation.jmx.rules.assertions.DataPointAttributes.attributeGroup; +import static io.opentelemetry.instrumentation.jmx.rules.assertions.DataPointAttributes.attributeWithAnyValue; + +import java.time.Duration; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.containers.wait.strategy.Wait; + +public class TomcatIntegrationTest extends TargetSystemTest { + + @ParameterizedTest + @ValueSource( + strings = { + "tomcat:10.0", + "tomcat:9.0" + }) + void testCollectedMetrics(String dockerImageName) throws Exception { + List yamlFiles = Collections.singletonList("tomcat.yaml"); + + yamlFiles.forEach(this::validateYamlSyntax); + + List jvmArgs = new ArrayList<>(); + jvmArgs.add(javaAgentJvmArgument()); + jvmArgs.addAll(javaPropertiesToJvmArgs(otelConfigProperties(yamlFiles))); + + // testing with a basic tomcat image as test application to capture JVM metrics + GenericContainer target = + new GenericContainer<>(dockerImageName) + .withEnv("CATALINA_OPTS", String.join(" ", jvmArgs)) + .withStartupTimeout(Duration.ofMinutes(2)) + .withExposedPorts(8080) + .waitingFor(Wait.forListeningPorts(8080)); + + copyFilesToTarget(target, yamlFiles); + + startTarget(target); + + verifyMetrics(createMetricsVerifier()); + } + + private static MetricsVerifier createMetricsVerifier() { + return MetricsVerifier.create() + .add( + "tomcat.error.count", + metric -> + metric + .hasDescription("The number of errors.") + .hasUnit("{error}") + .isCounter() + .hasDataPointsWithOneAttribute(attribute("tomcat.request_processor.name", "\"http-nio-8080\""))) + .add( + "tomcat.request.count", + metric -> + metric + .hasDescription("The number of requests processed.") + .hasUnit("{request}") + .isCounter() + .hasDataPointsWithOneAttribute(attribute("tomcat.request_processor.name", "\"http-nio-8080\""))) + .add( + "tomcat.request.duration.max", + metric -> + metric + .hasDescription("The longest request processing time.") + .hasUnit("s") + .isGauge() + .hasDataPointsWithOneAttribute(attribute("tomcat.request_processor.name", "\"http-nio-8080\""))) + .add( + "tomcat.request.processing_time", + metric -> + metric + .hasDescription("Total time for processing all requests.") + .hasUnit("s") + .isCounter() + .hasDataPointsWithOneAttribute(attribute("tomcat.request_processor.name", "\"http-nio-8080\""))) + .add( + "tomcat.network.io", + metric -> + metric + .hasDescription("The number of bytes transmitted and received") + .hasUnit("By") + .isCounter() + .hasDataPointsWithAttributes( + attributeGroup( + attribute("tomcat.network.io.direction", "sent"), + attribute("tomcat.request_processor.name", "\"http-nio-8080\"")), + attributeGroup( + attribute("tomcat.network.io.direction", "received"), + attribute("tomcat.request_processor.name", "\"http-nio-8080\"")))) + + .add( + "tomcat.active_session.count", + metric -> + metric + .hasDescription("The number of active sessions.") + .hasUnit("{session}") + .isUpDownCounter() + .hasDataPointsWithOneAttribute(attributeWithAnyValue("tomcat.web_app_context"))) + + .add( + "tomcat.thread.count", + metric -> + metric + .hasDescription("Thread count of the thread pool.") + .hasUnit("{thread}") + .isUpDownCounter() + .hasDataPointsWithAttributes( + attributeGroup( + attribute("state", "idle"), + attribute("tomcat.thread_pool.name", "\"http-nio-8080\"")), + attributeGroup( + attribute("state", "busy"), + attribute("tomcat.thread_pool.name", "\"http-nio-8080\"")))); + + } +} From 2d2c26495c9c08d43fe42da5d4aa0fb8c2273e56 Mon Sep 17 00:00:00 2001 From: robsunday Date: Fri, 4 Apr 2025 15:16:37 +0200 Subject: [PATCH 04/15] tomcat.active_session.count support added in test --- .../jmx/rules/TomcatIntegrationTest.java | 46 ++++++++++--------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java index 9d4e7fa7171f..373aeb7a3f47 100644 --- a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java +++ b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java @@ -9,24 +9,25 @@ import static io.opentelemetry.instrumentation.jmx.rules.assertions.DataPointAttributes.attributeGroup; import static io.opentelemetry.instrumentation.jmx.rules.assertions.DataPointAttributes.attributeWithAnyValue; +import io.opentelemetry.instrumentation.jmx.rules.assertions.AttributeMatcher; import java.time.Duration; import java.util.ArrayList; import java.util.Collections; import java.util.List; import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; +import org.junit.jupiter.params.provider.CsvSource; import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.wait.strategy.Wait; public class TomcatIntegrationTest extends TargetSystemTest { @ParameterizedTest - @ValueSource( - strings = { - "tomcat:10.0", - "tomcat:9.0" - }) - void testCollectedMetrics(String dockerImageName) throws Exception { + @CsvSource({ + "tomcat:10.0, https://tomcat.apache.org/tomcat-10.0-doc/appdev/sample/sample.war", + "tomcat:9.0, https://tomcat.apache.org/tomcat-9.0-doc/appdev/sample/sample.war" + }) + void testCollectedMetrics(String dockerImageName, String sampleWebApplicationURL) + throws Exception { List yamlFiles = Collections.singletonList("tomcat.yaml"); yamlFiles.forEach(this::validateYamlSyntax); @@ -46,11 +47,19 @@ void testCollectedMetrics(String dockerImageName) throws Exception { copyFilesToTarget(target, yamlFiles); startTarget(target); + target.execInContainer("rm", "-fr", "/usr/local/tomcat/webapps/ROOT"); + target.execInContainer( + "curl", sampleWebApplicationURL, "-o", "/usr/local/tomcat/webapps/ROOT.war"); verifyMetrics(createMetricsVerifier()); } private static MetricsVerifier createMetricsVerifier() { + AttributeMatcher requestProcessorNameAttribute = + attribute("tomcat.request_processor.name", "\"http-nio-8080\""); + AttributeMatcher threadPoolNameAttribute = + attribute("tomcat.thread_pool.name", "\"http-nio-8080\""); + return MetricsVerifier.create() .add( "tomcat.error.count", @@ -59,7 +68,7 @@ private static MetricsVerifier createMetricsVerifier() { .hasDescription("The number of errors.") .hasUnit("{error}") .isCounter() - .hasDataPointsWithOneAttribute(attribute("tomcat.request_processor.name", "\"http-nio-8080\""))) + .hasDataPointsWithOneAttribute(requestProcessorNameAttribute)) .add( "tomcat.request.count", metric -> @@ -67,7 +76,7 @@ private static MetricsVerifier createMetricsVerifier() { .hasDescription("The number of requests processed.") .hasUnit("{request}") .isCounter() - .hasDataPointsWithOneAttribute(attribute("tomcat.request_processor.name", "\"http-nio-8080\""))) + .hasDataPointsWithOneAttribute(requestProcessorNameAttribute)) .add( "tomcat.request.duration.max", metric -> @@ -75,7 +84,7 @@ private static MetricsVerifier createMetricsVerifier() { .hasDescription("The longest request processing time.") .hasUnit("s") .isGauge() - .hasDataPointsWithOneAttribute(attribute("tomcat.request_processor.name", "\"http-nio-8080\""))) + .hasDataPointsWithOneAttribute(requestProcessorNameAttribute)) .add( "tomcat.request.processing_time", metric -> @@ -83,22 +92,21 @@ private static MetricsVerifier createMetricsVerifier() { .hasDescription("Total time for processing all requests.") .hasUnit("s") .isCounter() - .hasDataPointsWithOneAttribute(attribute("tomcat.request_processor.name", "\"http-nio-8080\""))) + .hasDataPointsWithOneAttribute(requestProcessorNameAttribute)) .add( "tomcat.network.io", metric -> metric - .hasDescription("The number of bytes transmitted and received") + .hasDescription("The number of bytes transmitted.") .hasUnit("By") .isCounter() .hasDataPointsWithAttributes( attributeGroup( attribute("tomcat.network.io.direction", "sent"), - attribute("tomcat.request_processor.name", "\"http-nio-8080\"")), + requestProcessorNameAttribute), attributeGroup( attribute("tomcat.network.io.direction", "received"), - attribute("tomcat.request_processor.name", "\"http-nio-8080\"")))) - + requestProcessorNameAttribute))) .add( "tomcat.active_session.count", metric -> @@ -107,7 +115,6 @@ private static MetricsVerifier createMetricsVerifier() { .hasUnit("{session}") .isUpDownCounter() .hasDataPointsWithOneAttribute(attributeWithAnyValue("tomcat.web_app_context"))) - .add( "tomcat.thread.count", metric -> @@ -117,11 +124,8 @@ private static MetricsVerifier createMetricsVerifier() { .isUpDownCounter() .hasDataPointsWithAttributes( attributeGroup( - attribute("state", "idle"), - attribute("tomcat.thread_pool.name", "\"http-nio-8080\"")), + attribute("tomcat.thread.state", "idle"), threadPoolNameAttribute), attributeGroup( - attribute("state", "busy"), - attribute("tomcat.thread_pool.name", "\"http-nio-8080\"")))); - + attribute("tomcat.thread.state", "busy"), threadPoolNameAttribute))); } } From 15ad8398db6ca1774042b49e733b460f41b945ec Mon Sep 17 00:00:00 2001 From: robsunday Date: Fri, 4 Apr 2025 15:35:08 +0200 Subject: [PATCH 05/15] Comment added to test --- .../instrumentation/jmx/rules/TomcatIntegrationTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java index 373aeb7a3f47..a585fe104b18 100644 --- a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java +++ b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java @@ -47,6 +47,9 @@ void testCollectedMetrics(String dockerImageName, String sampleWebApplicationURL copyFilesToTarget(target, yamlFiles); startTarget(target); + + // Deploy example web application to the tomcat to enable reporting tomcat.active_session.count + // metric target.execInContainer("rm", "-fr", "/usr/local/tomcat/webapps/ROOT"); target.execInContainer( "curl", sampleWebApplicationURL, "-o", "/usr/local/tomcat/webapps/ROOT.war"); From 297fb6b8ae204ed2d216c785156dba507df4e568 Mon Sep 17 00:00:00 2001 From: robsunday Date: Fri, 4 Apr 2025 16:49:25 +0200 Subject: [PATCH 06/15] Checkstyle error fix --- .../instrumentation/jmx/rules/TomcatIntegrationTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java index a585fe104b18..e1bbc2f29e7d 100644 --- a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java +++ b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java @@ -26,7 +26,7 @@ public class TomcatIntegrationTest extends TargetSystemTest { "tomcat:10.0, https://tomcat.apache.org/tomcat-10.0-doc/appdev/sample/sample.war", "tomcat:9.0, https://tomcat.apache.org/tomcat-9.0-doc/appdev/sample/sample.war" }) - void testCollectedMetrics(String dockerImageName, String sampleWebApplicationURL) + void testCollectedMetrics(String dockerImageName, String sampleWebApplicationUrl) throws Exception { List yamlFiles = Collections.singletonList("tomcat.yaml"); @@ -52,7 +52,7 @@ void testCollectedMetrics(String dockerImageName, String sampleWebApplicationURL // metric target.execInContainer("rm", "-fr", "/usr/local/tomcat/webapps/ROOT"); target.execInContainer( - "curl", sampleWebApplicationURL, "-o", "/usr/local/tomcat/webapps/ROOT.war"); + "curl", sampleWebApplicationUrl, "-o", "/usr/local/tomcat/webapps/ROOT.war"); verifyMetrics(createMetricsVerifier()); } From e8fdc29afd108c76bce4b283e7aebd0495c6b6af Mon Sep 17 00:00:00 2001 From: robsunday Date: Fri, 4 Apr 2025 16:55:48 +0200 Subject: [PATCH 07/15] Attr name fixed --- .../library/src/main/resources/jmx/rules/tomcat.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml b/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml index 851a311aae94..c841c65aa0b2 100644 --- a/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml +++ b/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml @@ -73,10 +73,10 @@ rules: metric: &metric thread.count desc: &desc Thread count of the thread pool. metricAttribute: - state: const(idle) + tomcat.thread.state: const(idle) currentThreadsBusy: metric: *metric desc: *desc metricAttribute: - state: const(busy) + tomcat.thread.state: const(busy) From 8a9bb0f406938d2297570a5b133a9c7e2c6c195d Mon Sep 17 00:00:00 2001 From: robsunday Date: Mon, 7 Apr 2025 17:00:19 +0200 Subject: [PATCH 08/15] Test fixed. Tomcat metrics description md file moved to library Code review followup --- instrumentation/jmx-metrics/javaagent/README.md | 2 +- .../jmx/JmxMetricInsightInstallerTest.java | 7 ++++++- instrumentation/jmx-metrics/javaagent/tomcat.md | 13 ------------- .../src/main/resources/jmx/rules/tomcat.yaml | 6 +++--- .../jmx/rules/TomcatIntegrationTest.java | 4 ++-- instrumentation/jmx-metrics/library/tomcat.md | 13 +++++++++++++ 6 files changed, 25 insertions(+), 20 deletions(-) delete mode 100644 instrumentation/jmx-metrics/javaagent/tomcat.md create mode 100644 instrumentation/jmx-metrics/library/tomcat.md diff --git a/instrumentation/jmx-metrics/javaagent/README.md b/instrumentation/jmx-metrics/javaagent/README.md index 753c62da8239..7e42e7294911 100644 --- a/instrumentation/jmx-metrics/javaagent/README.md +++ b/instrumentation/jmx-metrics/javaagent/README.md @@ -29,7 +29,7 @@ No targets are enabled by default. The supported target environments are listed - [camel](camel.md) - [jetty](jetty.md) - [kafka-broker](kafka-broker.md) -- [tomcat](tomcat.md) +- [tomcat](../library/tomcat.md) - [wildfly](wildfly.md) - [hadoop](hadoop.md) diff --git a/instrumentation/jmx-metrics/javaagent/src/test/java/io/opentelemetry/instrumentation/javaagent/jmx/JmxMetricInsightInstallerTest.java b/instrumentation/jmx-metrics/javaagent/src/test/java/io/opentelemetry/instrumentation/javaagent/jmx/JmxMetricInsightInstallerTest.java index 55d0c51f8c1b..88d8d6787357 100644 --- a/instrumentation/jmx-metrics/javaagent/src/test/java/io/opentelemetry/instrumentation/javaagent/jmx/JmxMetricInsightInstallerTest.java +++ b/instrumentation/jmx-metrics/javaagent/src/test/java/io/opentelemetry/instrumentation/javaagent/jmx/JmxMetricInsightInstallerTest.java @@ -22,6 +22,12 @@ import java.util.Set; import org.junit.jupiter.api.Test; +/** + * TODO: This test will eventually go away when all yaml files are moved from javaagent to library + * directory. When yaml file is moved from javaagent to library then appropriate item must be + * removed from JmxMetricInsightInstallerTest#FILES_TO_BE_TESTED and corresponding test must be + * added in the library. + */ class JmxMetricInsightInstallerTest { private static final String PATH_TO_ALL_EXISTING_RULES = "src/main/resources/jmx/rules"; private static final Set FILES_TO_BE_TESTED = @@ -32,7 +38,6 @@ class JmxMetricInsightInstallerTest { "hadoop.yaml", "jetty.yaml", "kafka-broker.yaml", - "tomcat.yaml", "wildfly.yaml")); @Test diff --git a/instrumentation/jmx-metrics/javaagent/tomcat.md b/instrumentation/jmx-metrics/javaagent/tomcat.md deleted file mode 100644 index 6ac2e3fea16e..000000000000 --- a/instrumentation/jmx-metrics/javaagent/tomcat.md +++ /dev/null @@ -1,13 +0,0 @@ -# Tomcat Metrics - -Here is the list of metrics based on MBeans exposed by Tomcat. - -| Metric Name | Type | Attributes | Description | -| ------------------------------------------ | ------------- | --------------- | --------------------------------------------------------------- | -| http.server.tomcat.sessions.activeSessions | UpDownCounter | context | The number of active sessions | -| http.server.tomcat.errorCount | Gauge | name | The number of errors per second on all request processors | -| http.server.tomcat.requestCount | Gauge | name | The number of requests per second across all request processors | -| http.server.tomcat.maxTime | Gauge | name | The longest request processing time | -| http.server.tomcat.processingTime | Counter | name | Represents the total time for processing all requests | -| http.server.tomcat.traffic | Counter | name, direction | The number of bytes transmitted | -| http.server.tomcat.threads | UpDownCounter | name, state | Thread Count of the Thread Pool | diff --git a/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml b/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml index c841c65aa0b2..034eef3a54aa 100644 --- a/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml +++ b/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml @@ -38,21 +38,21 @@ rules: unit: &unit By desc: &desc The number of bytes transmitted. metricAttribute: - tomcat.network.io.direction: const(received) + network.io.direction: const(receive) bytesSent: metric: *metric type: *type unit: *unit desc: *desc metricAttribute: - tomcat.network.io.direction: const(sent) + network.io.direction: const(transmit) - beans: - Catalina:type=Manager,host=localhost,context=* - Tomcat:type=Manager,host=localhost,context=* prefix: tomcat. metricAttribute: - tomcat.web_app_context: param(context) + tomcat.context: param(context) mapping: activeSessions: metric: active_session.count diff --git a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java index e1bbc2f29e7d..f07c4c14f655 100644 --- a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java +++ b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java @@ -105,10 +105,10 @@ private static MetricsVerifier createMetricsVerifier() { .isCounter() .hasDataPointsWithAttributes( attributeGroup( - attribute("tomcat.network.io.direction", "sent"), + attribute("network.io.direction", "receive"), requestProcessorNameAttribute), attributeGroup( - attribute("tomcat.network.io.direction", "received"), + attribute("network.io.direction", "transmit"), requestProcessorNameAttribute))) .add( "tomcat.active_session.count", diff --git a/instrumentation/jmx-metrics/library/tomcat.md b/instrumentation/jmx-metrics/library/tomcat.md new file mode 100644 index 000000000000..0861fac1cf4f --- /dev/null +++ b/instrumentation/jmx-metrics/library/tomcat.md @@ -0,0 +1,13 @@ +# Tomcat Metrics + +Here is the list of metrics based on MBeans exposed by Tomcat. + +| Metric Name | Type | Attributes | Description | +|--------------------------------|---------------|-----------------------------------------------------|-----------------------------------------| +| tomcat.active_session.count | UpDownCounter | tomcat.context | The number of active sessions. | +| tomcat.error.count | Counter | tomcat.request_processor.name | The number of errors. | +| tomcat.request.count | Counter | tomcat.request_processor.name | The number of requests processed. | +| tomcat.request.duration.max | Gauge | tomcat.request_processor.name | The longest request processing time. | +| tomcat.request.processing_time | Counter | tomcat.request_processor.name | Total time for processing all requests. | +| tomcat.network.io | Counter | tomcat.request_processor.name, network.io.direction | The number of bytes transmitted. | +| tomcat.thread.count | UpDownCounter | tomcat.thread_pool.name, tomcat.thread.state | Thread Count of the Thread Pool | From 1cc40809a194c5496c5b1a3a4636be0195307b5a Mon Sep 17 00:00:00 2001 From: robsunday Date: Tue, 8 Apr 2025 09:18:05 +0200 Subject: [PATCH 09/15] Test fixed. --- .../instrumentation/jmx/rules/TomcatIntegrationTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java index f07c4c14f655..7f26010dea87 100644 --- a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java +++ b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java @@ -117,7 +117,7 @@ private static MetricsVerifier createMetricsVerifier() { .hasDescription("The number of active sessions.") .hasUnit("{session}") .isUpDownCounter() - .hasDataPointsWithOneAttribute(attributeWithAnyValue("tomcat.web_app_context"))) + .hasDataPointsWithOneAttribute(attributeWithAnyValue("tomcat.context"))) .add( "tomcat.thread.count", metric -> From 910e837a869f017556d6fb673a43a20667106592 Mon Sep 17 00:00:00 2001 From: robsunday Date: Thu, 24 Apr 2025 09:56:57 +0200 Subject: [PATCH 10/15] Thread count related metrics fixed after code review --- .../src/main/resources/jmx/rules/tomcat.yaml | 14 +++++--------- .../jmx/rules/TomcatIntegrationTest.java | 16 ++++++++++------ 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml b/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml index 034eef3a54aa..db1773df68c9 100644 --- a/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml +++ b/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml @@ -64,19 +64,15 @@ rules: - Catalina:type=ThreadPool,name=* - Tomcat:type=ThreadPool,name=* unit: "{thread}" - prefix: tomcat. + prefix: tomcat.thread. type: updowncounter metricAttribute: tomcat.thread_pool.name: param(name) mapping: currentThreadCount: - metric: &metric thread.count - desc: &desc Thread count of the thread pool. - metricAttribute: - tomcat.thread.state: const(idle) + metric: count + desc: Total thread count of the thread pool. currentThreadsBusy: - metric: *metric - desc: *desc - metricAttribute: - tomcat.thread.state: const(busy) + metric: busy.count + desc: Number of busy threads in the thread pool. diff --git a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java index 7f26010dea87..456834dcbeee 100644 --- a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java +++ b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java @@ -122,13 +122,17 @@ private static MetricsVerifier createMetricsVerifier() { "tomcat.thread.count", metric -> metric - .hasDescription("Thread count of the thread pool.") + .hasDescription("Total thread count of the thread pool.") .hasUnit("{thread}") .isUpDownCounter() - .hasDataPointsWithAttributes( - attributeGroup( - attribute("tomcat.thread.state", "idle"), threadPoolNameAttribute), - attributeGroup( - attribute("tomcat.thread.state", "busy"), threadPoolNameAttribute))); + .hasDataPointsWithOneAttribute(threadPoolNameAttribute)) + .add( + "tomcat.thread.busy.count", + metric -> + metric + .hasDescription("Number of busy threads in the thread pool.") + .hasUnit("{thread}") + .isUpDownCounter() + .hasDataPointsWithOneAttribute(threadPoolNameAttribute)); } } From 1dcd568c1a93b68218484f2621bd2bd484085b1d Mon Sep 17 00:00:00 2001 From: robsunday Date: Thu, 24 Apr 2025 10:30:25 +0200 Subject: [PATCH 11/15] Readme updates --- instrumentation/jmx-metrics/README.md | 2 +- instrumentation/jmx-metrics/library/tomcat.md | 19 ++++++++++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/instrumentation/jmx-metrics/README.md b/instrumentation/jmx-metrics/README.md index eea8b7a610f5..d4164f251a4b 100644 --- a/instrumentation/jmx-metrics/README.md +++ b/instrumentation/jmx-metrics/README.md @@ -29,7 +29,7 @@ No targets are enabled by default. The supported target environments are listed - [camel](javaagent/camel.md) - [jetty](javaagent/jetty.md) - [kafka-broker](javaagent/kafka-broker.md) -- [tomcat](javaagent/tomcat.md) +- [tomcat](library/tomcat.md) - [wildfly](javaagent/wildfly.md) - [hadoop](javaagent/hadoop.md) diff --git a/instrumentation/jmx-metrics/library/tomcat.md b/instrumentation/jmx-metrics/library/tomcat.md index 0861fac1cf4f..c1a3778a2ca3 100644 --- a/instrumentation/jmx-metrics/library/tomcat.md +++ b/instrumentation/jmx-metrics/library/tomcat.md @@ -2,12 +2,13 @@ Here is the list of metrics based on MBeans exposed by Tomcat. -| Metric Name | Type | Attributes | Description | -|--------------------------------|---------------|-----------------------------------------------------|-----------------------------------------| -| tomcat.active_session.count | UpDownCounter | tomcat.context | The number of active sessions. | -| tomcat.error.count | Counter | tomcat.request_processor.name | The number of errors. | -| tomcat.request.count | Counter | tomcat.request_processor.name | The number of requests processed. | -| tomcat.request.duration.max | Gauge | tomcat.request_processor.name | The longest request processing time. | -| tomcat.request.processing_time | Counter | tomcat.request_processor.name | Total time for processing all requests. | -| tomcat.network.io | Counter | tomcat.request_processor.name, network.io.direction | The number of bytes transmitted. | -| tomcat.thread.count | UpDownCounter | tomcat.thread_pool.name, tomcat.thread.state | Thread Count of the Thread Pool | +| Metric Name | Type | Attributes | Description | +|--------------------------------|---------------|-----------------------------------------------------|---------------------------------------------| +| tomcat.active_session.count | UpDownCounter | tomcat.context | The number of active sessions. | +| tomcat.error.count | Counter | tomcat.request_processor.name | The number of errors. | +| tomcat.request.count | Counter | tomcat.request_processor.name | The number of requests processed. | +| tomcat.request.duration.max | Gauge | tomcat.request_processor.name | The longest request processing time. | +| tomcat.request.processing_time | Counter | tomcat.request_processor.name | Total time for processing all requests. | +| tomcat.network.io | Counter | tomcat.request_processor.name, network.io.direction | The number of bytes transmitted. | +| tomcat.thread.count | UpDownCounter | tomcat.thread_pool.name | Total thread count of the thread pool. | +| tomcat.thread.busy.count | UpDownCounter | tomcat.thread_pool.name | Number of busy threads in the thread pool. | From 99104eed19431c92a517324e81947716c43b8e25 Mon Sep 17 00:00:00 2001 From: Robert Niedziela <175605712+robsunday@users.noreply.github.com> Date: Wed, 14 May 2025 14:13:12 +0200 Subject: [PATCH 12/15] Update instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml Co-authored-by: SylvainJuge <763082+SylvainJuge@users.noreply.github.com> --- .../library/src/main/resources/jmx/rules/tomcat.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml b/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml index db1773df68c9..e0256ae5276e 100644 --- a/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml +++ b/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml @@ -55,7 +55,7 @@ rules: tomcat.context: param(context) mapping: activeSessions: - metric: active_session.count + metric: session.active.count type: updowncounter unit: "{session}" desc: The number of active sessions. From f8cc64782b7f6126598e45e19a5c6f91d5e28bd7 Mon Sep 17 00:00:00 2001 From: Robert Niedziela <175605712+robsunday@users.noreply.github.com> Date: Wed, 14 May 2025 14:15:09 +0200 Subject: [PATCH 13/15] Update instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml Co-authored-by: SylvainJuge <763082+SylvainJuge@users.noreply.github.com> --- .../library/src/main/resources/jmx/rules/tomcat.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml b/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml index e0256ae5276e..f4314e94f856 100644 --- a/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml +++ b/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml @@ -67,7 +67,7 @@ rules: prefix: tomcat.thread. type: updowncounter metricAttribute: - tomcat.thread_pool.name: param(name) + tomcat.thread.pool.name: param(name) mapping: currentThreadCount: metric: count From 299b35c941fc3d5de198894356812c7c7a78921a Mon Sep 17 00:00:00 2001 From: robsunday Date: Wed, 14 May 2025 15:33:59 +0200 Subject: [PATCH 14/15] Code review changes --- .../src/main/resources/jmx/rules/tomcat.yaml | 10 +++++++- .../jmx/rules/TomcatIntegrationTest.java | 24 +++++++++++++++---- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml b/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml index f4314e94f856..53de232274c4 100644 --- a/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml +++ b/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml @@ -58,7 +58,12 @@ rules: metric: session.active.count type: updowncounter unit: "{session}" - desc: The number of active sessions. + desc: The number of currently active sessions. + maxActiveSessions: + metric: session.active.limit + type: updowncounter + unit: "{session}" + desc: Maximum number of active sessions. - beans: - Catalina:type=ThreadPool,name=* @@ -72,6 +77,9 @@ rules: currentThreadCount: metric: count desc: Total thread count of the thread pool. + maxThreads: + metric: limit + desc: Maximum thread count of the thread pool. currentThreadsBusy: metric: busy.count desc: Number of busy threads in the thread pool. diff --git a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java index 456834dcbeee..0ea8ca8d8155 100644 --- a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java +++ b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java @@ -48,7 +48,7 @@ void testCollectedMetrics(String dockerImageName, String sampleWebApplicationUrl startTarget(target); - // Deploy example web application to the tomcat to enable reporting tomcat.active_session.count + // Deploy example web application to the tomcat to enable reporting tomcat.session.active.count // metric target.execInContainer("rm", "-fr", "/usr/local/tomcat/webapps/ROOT"); target.execInContainer( @@ -61,7 +61,7 @@ private static MetricsVerifier createMetricsVerifier() { AttributeMatcher requestProcessorNameAttribute = attribute("tomcat.request_processor.name", "\"http-nio-8080\""); AttributeMatcher threadPoolNameAttribute = - attribute("tomcat.thread_pool.name", "\"http-nio-8080\""); + attribute("tomcat.thread.pool.name", "\"http-nio-8080\""); return MetricsVerifier.create() .add( @@ -111,10 +111,18 @@ private static MetricsVerifier createMetricsVerifier() { attribute("network.io.direction", "transmit"), requestProcessorNameAttribute))) .add( - "tomcat.active_session.count", + "tomcat.session.active.count", metric -> metric - .hasDescription("The number of active sessions.") + .hasDescription("The number of currently active sessions.") + .hasUnit("{session}") + .isUpDownCounter() + .hasDataPointsWithOneAttribute(attributeWithAnyValue("tomcat.context"))) + .add( + "tomcat.session.active.limit", + metric -> + metric + .hasDescription("Maximum number of active sessions.") .hasUnit("{session}") .isUpDownCounter() .hasDataPointsWithOneAttribute(attributeWithAnyValue("tomcat.context"))) @@ -126,6 +134,14 @@ private static MetricsVerifier createMetricsVerifier() { .hasUnit("{thread}") .isUpDownCounter() .hasDataPointsWithOneAttribute(threadPoolNameAttribute)) + .add( + "tomcat.thread.limit", + metric -> + metric + .hasDescription("Maximum thread count of the thread pool.") + .hasUnit("{thread}") + .isUpDownCounter() + .hasDataPointsWithOneAttribute(threadPoolNameAttribute)) .add( "tomcat.thread.busy.count", metric -> From ee7faece518bffa25ab6d8e050c5e4538028dfc3 Mon Sep 17 00:00:00 2001 From: robsunday Date: Thu, 15 May 2025 15:59:31 +0200 Subject: [PATCH 15/15] Fixed tomcat.md tomcat.request.processing_time changed to tomcat.request.duration.total --- .../src/main/resources/jmx/rules/tomcat.yaml | 4 ++-- .../jmx/rules/TomcatIntegrationTest.java | 4 ++-- instrumentation/jmx-metrics/library/tomcat.md | 22 ++++++++++--------- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml b/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml index 53de232274c4..7123dabbd831 100644 --- a/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml +++ b/instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/tomcat.yaml @@ -27,11 +27,11 @@ rules: unit: s desc: The longest request processing time. processingTime: - metric: request.processing_time + metric: request.duration.total type: counter sourceUnit: ms unit: s - desc: Total time for processing all requests. + desc: Total time of processing all requests. bytesReceived: metric: &metric network.io type: &type counter diff --git a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java index 0ea8ca8d8155..b111028c3790 100644 --- a/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java +++ b/instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/rules/TomcatIntegrationTest.java @@ -89,10 +89,10 @@ private static MetricsVerifier createMetricsVerifier() { .isGauge() .hasDataPointsWithOneAttribute(requestProcessorNameAttribute)) .add( - "tomcat.request.processing_time", + "tomcat.request.duration.total", metric -> metric - .hasDescription("Total time for processing all requests.") + .hasDescription("Total time of processing all requests.") .hasUnit("s") .isCounter() .hasDataPointsWithOneAttribute(requestProcessorNameAttribute)) diff --git a/instrumentation/jmx-metrics/library/tomcat.md b/instrumentation/jmx-metrics/library/tomcat.md index c1a3778a2ca3..8fa61a8477e4 100644 --- a/instrumentation/jmx-metrics/library/tomcat.md +++ b/instrumentation/jmx-metrics/library/tomcat.md @@ -2,13 +2,15 @@ Here is the list of metrics based on MBeans exposed by Tomcat. -| Metric Name | Type | Attributes | Description | -|--------------------------------|---------------|-----------------------------------------------------|---------------------------------------------| -| tomcat.active_session.count | UpDownCounter | tomcat.context | The number of active sessions. | -| tomcat.error.count | Counter | tomcat.request_processor.name | The number of errors. | -| tomcat.request.count | Counter | tomcat.request_processor.name | The number of requests processed. | -| tomcat.request.duration.max | Gauge | tomcat.request_processor.name | The longest request processing time. | -| tomcat.request.processing_time | Counter | tomcat.request_processor.name | Total time for processing all requests. | -| tomcat.network.io | Counter | tomcat.request_processor.name, network.io.direction | The number of bytes transmitted. | -| tomcat.thread.count | UpDownCounter | tomcat.thread_pool.name | Total thread count of the thread pool. | -| tomcat.thread.busy.count | UpDownCounter | tomcat.thread_pool.name | Number of busy threads in the thread pool. | +| Metric Name | Type | Attributes | Description | +|-------------------------------|---------------|-----------------------------------------------------|--------------------------------------------| +| tomcat.session.active.count | UpDownCounter | tomcat.context | The number of currently active sessions. | +| tomcat.session.active.limit | UpDownCounter | tomcat.context | Maximum number of active sessions. | +| tomcat.error.count | Counter | tomcat.request_processor.name | The number of errors. | +| tomcat.request.count | Counter | tomcat.request_processor.name | The number of requests processed. | +| tomcat.request.duration.max | Gauge | tomcat.request_processor.name | The longest request processing time. | +| tomcat.request.duration.total | Counter | tomcat.request_processor.name | Total time of processing all requests. | +| tomcat.network.io | Counter | tomcat.request_processor.name, network.io.direction | The number of bytes transmitted. | +| tomcat.thread.count | UpDownCounter | tomcat.thread_pool.name | Total thread count of the thread pool. | +| tomcat.thread.max | UpDownCounter | tomcat.thread_pool.name | Maximum thread count of the thread pool. | +| tomcat.thread.busy.count | UpDownCounter | tomcat.thread_pool.name | Number of busy threads in the thread pool. |