From 2b7527e44a522bd506f755d249cd71cfee220a00 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 25 May 2022 15:34:33 +0300 Subject: [PATCH 1/5] Implement Vibur DBCP connection pool metrics --- docs/standalone-library-instrumentation.md | 1 + docs/supported-libraries.md | 1 + .../javaagent/build.gradle.kts | 20 ++ .../ViburDbcpDataSourceInstrumentation.java | 51 +++++ .../ViburDbcpInstrumentationModule.java | 25 +++ .../viburdbcp/ViburSingletons.java | 21 ++ .../viburdbcp/ViburInstrumentationTest.java | 26 +++ .../vibur-dbcp-11.0/library/README.md | 47 ++++ .../vibur-dbcp-11.0/library/build.gradle.kts | 10 + .../viburdbcp/ConnectionPoolMetrics.java | 52 +++++ .../viburdbcp/ViburTelemetry.java | 34 +++ .../viburdbcp/ViburInstrumentationTest.java | 32 +++ .../vibur-dbcp-11.0/testing/build.gradle.kts | 11 + .../AbstractViburInstrumentationTest.java | 97 +++++++++ settings.gradle.kts | 3 + .../db/DbConnectionPoolMetricsAssertions.java | 204 +++++++++++------- 16 files changed, 553 insertions(+), 82 deletions(-) create mode 100644 instrumentation/vibur-dbcp-11.0/javaagent/build.gradle.kts create mode 100644 instrumentation/vibur-dbcp-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/ViburDbcpDataSourceInstrumentation.java create mode 100644 instrumentation/vibur-dbcp-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/ViburDbcpInstrumentationModule.java create mode 100644 instrumentation/vibur-dbcp-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/ViburSingletons.java create mode 100644 instrumentation/vibur-dbcp-11.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/ViburInstrumentationTest.java create mode 100644 instrumentation/vibur-dbcp-11.0/library/README.md create mode 100644 instrumentation/vibur-dbcp-11.0/library/build.gradle.kts create mode 100644 instrumentation/vibur-dbcp-11.0/library/src/main/java/io/opentelemetry/instrumentation/viburdbcp/ConnectionPoolMetrics.java create mode 100644 instrumentation/vibur-dbcp-11.0/library/src/main/java/io/opentelemetry/instrumentation/viburdbcp/ViburTelemetry.java create mode 100644 instrumentation/vibur-dbcp-11.0/library/src/test/java/io/opentelemetry/instrumentation/viburdbcp/ViburInstrumentationTest.java create mode 100644 instrumentation/vibur-dbcp-11.0/testing/build.gradle.kts create mode 100644 instrumentation/vibur-dbcp-11.0/testing/src/main/java/io/opentelemetry/instrumentation/viburdbcp/AbstractViburInstrumentationTest.java diff --git a/docs/standalone-library-instrumentation.md b/docs/standalone-library-instrumentation.md index 79ac9527da33..758556f02054 100644 --- a/docs/standalone-library-instrumentation.md +++ b/docs/standalone-library-instrumentation.md @@ -31,6 +31,7 @@ that can be used if you prefer that over using the Java agent: * [Spring RestTemplate](../instrumentation/spring/spring-web-3.1/library) * [Spring Web MVC](../instrumentation/spring/spring-webmvc-3.1/library) * [Spring WebFlux Client](../instrumentation/spring/spring-webflux-5.0/library) +* [Vibur DBCP](../instrumentation/vibur-dbcp-11.0/library) And some libraries are publishing their own OpenTelemetry instrumentation (yay!), e.g. diff --git a/docs/supported-libraries.md b/docs/supported-libraries.md index 45d9548e192f..8b92cfd52e67 100644 --- a/docs/supported-libraries.md +++ b/docs/supported-libraries.md @@ -117,6 +117,7 @@ These are the supported libraries and frameworks: | [Vert.x HttpClient](https://vertx.io/docs/apidocs/io/vertx/core/http/HttpClient.html) | 3.0+ | | [Vert.x Kafka Client](https://vertx.io/docs/vertx-kafka-client/java/) | 3.6+ | | [Vert.x RxJava2](https://vertx.io/docs/vertx-rx/java2/) | 3.5+ | +| [Vibur DBCP](https://www.vibur.org/) | 11.0+ | ## Application Servers diff --git a/instrumentation/vibur-dbcp-11.0/javaagent/build.gradle.kts b/instrumentation/vibur-dbcp-11.0/javaagent/build.gradle.kts new file mode 100644 index 000000000000..d5e27facd406 --- /dev/null +++ b/instrumentation/vibur-dbcp-11.0/javaagent/build.gradle.kts @@ -0,0 +1,20 @@ +plugins { + id("otel.javaagent-instrumentation") +} + +muzzle { + pass { + group.set("org.vibur") + module.set("vibur-dbcp") + versions.set("[11.0,)") + assertInverse.set(true) + } +} + +dependencies { + library("org.vibur:vibur-dbcp:11.0") + + implementation(project(":instrumentation:vibur-dbcp-11.0:library")) + + testImplementation(project(":instrumentation:vibur-dbcp-11.0:testing")) +} diff --git a/instrumentation/vibur-dbcp-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/ViburDbcpDataSourceInstrumentation.java b/instrumentation/vibur-dbcp-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/ViburDbcpDataSourceInstrumentation.java new file mode 100644 index 000000000000..bbae64f2673c --- /dev/null +++ b/instrumentation/vibur-dbcp-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/ViburDbcpDataSourceInstrumentation.java @@ -0,0 +1,51 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.viburdbcp; + +import static io.opentelemetry.javaagent.instrumentation.viburdbcp.ViburSingletons.telemetry; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; +import org.vibur.dbcp.ViburDBCPDataSource; + +final class ViburDbcpDataSourceInstrumentation implements TypeInstrumentation { + + @Override + public ElementMatcher typeMatcher() { + return named("org.vibur.dbcp.ViburDBCPDataSource"); + } + + @Override + public void transform(TypeTransformer transformer) { + transformer.applyAdviceToMethod( + named("start").and(takesArguments(0)), this.getClass().getName() + "$StartAdvice"); + transformer.applyAdviceToMethod( + named("close").and(takesArguments(0)), this.getClass().getName() + "$CloseAdvice"); + } + + @SuppressWarnings("unused") + public static class StartAdvice { + + @Advice.OnMethodExit(suppress = Throwable.class) + public static void onExit(@Advice.This ViburDBCPDataSource dataSource) { + telemetry().registerMetrics(dataSource); + } + } + + @SuppressWarnings("unused") + public static class CloseAdvice { + + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) + public static void onExit(@Advice.This ViburDBCPDataSource dataSource) { + telemetry().unregisterMetrics(dataSource); + } + } +} diff --git a/instrumentation/vibur-dbcp-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/ViburDbcpInstrumentationModule.java b/instrumentation/vibur-dbcp-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/ViburDbcpInstrumentationModule.java new file mode 100644 index 000000000000..ea83bd6b8ce1 --- /dev/null +++ b/instrumentation/vibur-dbcp-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/ViburDbcpInstrumentationModule.java @@ -0,0 +1,25 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.viburdbcp; + +import static java.util.Collections.singletonList; + +import com.google.auto.service.AutoService; +import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import java.util.List; + +@AutoService(InstrumentationModule.class) +public class ViburDbcpInstrumentationModule extends InstrumentationModule { + public ViburDbcpInstrumentationModule() { + super("vibur-dbcp", "vibur-dbcp-11.0"); + } + + @Override + public List typeInstrumentations() { + return singletonList(new ViburDbcpDataSourceInstrumentation()); + } +} diff --git a/instrumentation/vibur-dbcp-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/ViburSingletons.java b/instrumentation/vibur-dbcp-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/ViburSingletons.java new file mode 100644 index 000000000000..db7c8e148aa7 --- /dev/null +++ b/instrumentation/vibur-dbcp-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/ViburSingletons.java @@ -0,0 +1,21 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.viburdbcp; + +import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.instrumentation.viburdbcp.ViburTelemetry; + +public final class ViburSingletons { + + private static final ViburTelemetry viburTelemetry = + ViburTelemetry.create(GlobalOpenTelemetry.get()); + + public static ViburTelemetry telemetry() { + return viburTelemetry; + } + + private ViburSingletons() {} +} diff --git a/instrumentation/vibur-dbcp-11.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/ViburInstrumentationTest.java b/instrumentation/vibur-dbcp-11.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/ViburInstrumentationTest.java new file mode 100644 index 000000000000..ca08eeaf98ba --- /dev/null +++ b/instrumentation/vibur-dbcp-11.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/ViburInstrumentationTest.java @@ -0,0 +1,26 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.viburdbcp; + +import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import io.opentelemetry.instrumentation.viburdbcp.AbstractViburInstrumentationTest; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.vibur.dbcp.ViburDBCPDataSource; + +class ViburInstrumentationTest extends AbstractViburInstrumentationTest { + + @RegisterExtension + static final InstrumentationExtension testing = AgentInstrumentationExtension.create(); + + @Override + protected InstrumentationExtension testing() { + return testing; + } + + @Override + protected void configure(ViburDBCPDataSource viburDataSource) {} +} diff --git a/instrumentation/vibur-dbcp-11.0/library/README.md b/instrumentation/vibur-dbcp-11.0/library/README.md new file mode 100644 index 000000000000..2244a9dca673 --- /dev/null +++ b/instrumentation/vibur-dbcp-11.0/library/README.md @@ -0,0 +1,47 @@ +# Manual Instrumentation for HikariCP + +Provides OpenTelemetry instrumentation for [Vibur DBCP](https://www.vibur.org/). + +## Quickstart + +### Add these dependencies to your project: + +Replace `OPENTELEMETRY_VERSION` with the latest stable +[release](https://mvnrepository.com/artifact/io.opentelemetry). `Minimum version: 1.15.0` + +For Maven, add to your `pom.xml` dependencies: + +```xml + + + + io.opentelemetry.instrumentation + opentelemetry-viburdbcp-11.0 + OPENTELEMETRY_VERSION + + +``` + +For Gradle, add to your dependencies: + +```groovy +implementation("io.opentelemetry.instrumentation:opentelemetry-viburdbcp-11.0:OPENTELEMETRY_VERSION") +``` + +### Usage + +The instrumentation library allows registering `ViburDBCPDataSource` instances for collecting +OpenTelemetry-based metrics. + +```java +ViburTelemetry viburTelemetry; + +void configure(OpenTelemetry openTelemetry, ViburDBCPDataSource viburDataSource) { + viburTelemetry = ViburTelemetry.create(openTelemetry); + viburTelemetry.registerMetrics(viburDataSource); +} + +void destroy(ViburDBCPDataSource viburDataSource) { + viburTelemetry.unregisterMetrics(viburDataSource); +} +``` diff --git a/instrumentation/vibur-dbcp-11.0/library/build.gradle.kts b/instrumentation/vibur-dbcp-11.0/library/build.gradle.kts new file mode 100644 index 000000000000..381f5c75b8d5 --- /dev/null +++ b/instrumentation/vibur-dbcp-11.0/library/build.gradle.kts @@ -0,0 +1,10 @@ +plugins { + id("otel.library-instrumentation") + id("otel.nullaway-conventions") +} + +dependencies { + library("org.vibur:vibur-dbcp:11.0") + + testImplementation(project(":instrumentation:vibur-dbcp-11.0:testing")) +} diff --git a/instrumentation/vibur-dbcp-11.0/library/src/main/java/io/opentelemetry/instrumentation/viburdbcp/ConnectionPoolMetrics.java b/instrumentation/vibur-dbcp-11.0/library/src/main/java/io/opentelemetry/instrumentation/viburdbcp/ConnectionPoolMetrics.java new file mode 100644 index 000000000000..f699ca5aa449 --- /dev/null +++ b/instrumentation/vibur-dbcp-11.0/library/src/main/java/io/opentelemetry/instrumentation/viburdbcp/ConnectionPoolMetrics.java @@ -0,0 +1,52 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.viburdbcp; + +import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.api.metrics.ObservableLongUpDownCounter; +import io.opentelemetry.instrumentation.api.metrics.db.DbConnectionPoolMetrics; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import org.vibur.dbcp.ViburDBCPDataSource; + +final class ConnectionPoolMetrics { + private static final String INSTRUMENTATION_NAME = "io.opentelemetry.viburdbcp-11.0"; + + // a weak map does not make sense here because each Meter holds a reference to the dataSource + // ViburDBCPDataSource does not implement equals()/hashCode(), so it's safe to keep them in a + // plain ConcurrentHashMap + private static final Map> + dataSourceMetrics = new ConcurrentHashMap<>(); + + public static void registerMetrics(OpenTelemetry openTelemetry, ViburDBCPDataSource dataSource) { + dataSourceMetrics.computeIfAbsent( + dataSource, (unused) -> createMeters(openTelemetry, dataSource)); + } + + private static List createMeters( + OpenTelemetry openTelemetry, ViburDBCPDataSource dataSource) { + DbConnectionPoolMetrics metrics = + DbConnectionPoolMetrics.create(openTelemetry, INSTRUMENTATION_NAME, dataSource.getName()); + + return Arrays.asList( + metrics.usedConnections(() -> dataSource.getPool().taken()), + metrics.idleConnections(() -> dataSource.getPool().remainingCreated()), + metrics.maxConnections(dataSource::getPoolMaxSize)); + } + + public static void unregisterMetrics(ViburDBCPDataSource dataSource) { + List observableInstruments = dataSourceMetrics.remove(dataSource); + if (observableInstruments != null) { + for (ObservableLongUpDownCounter observable : observableInstruments) { + observable.close(); + } + } + } + + private ConnectionPoolMetrics() {} +} diff --git a/instrumentation/vibur-dbcp-11.0/library/src/main/java/io/opentelemetry/instrumentation/viburdbcp/ViburTelemetry.java b/instrumentation/vibur-dbcp-11.0/library/src/main/java/io/opentelemetry/instrumentation/viburdbcp/ViburTelemetry.java new file mode 100644 index 000000000000..328cc3922b7c --- /dev/null +++ b/instrumentation/vibur-dbcp-11.0/library/src/main/java/io/opentelemetry/instrumentation/viburdbcp/ViburTelemetry.java @@ -0,0 +1,34 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.viburdbcp; + +import io.opentelemetry.api.OpenTelemetry; +import org.vibur.dbcp.ViburDBCPDataSource; + +/** Entrypoint for instrumenting Hikari database connection pools. */ +public final class ViburTelemetry { + + /** Returns a new {@link ViburTelemetry} configured with the given {@link OpenTelemetry}. */ + public static ViburTelemetry create(OpenTelemetry openTelemetry) { + return new ViburTelemetry(openTelemetry); + } + + private final OpenTelemetry openTelemetry; + + private ViburTelemetry(OpenTelemetry openTelemetry) { + this.openTelemetry = openTelemetry; + } + + /** Start collecting metrics for given data source. */ + public void registerMetrics(ViburDBCPDataSource dataSource) { + ConnectionPoolMetrics.registerMetrics(openTelemetry, dataSource); + } + + /** Stop collecting metrics for given data source. */ + public void unregisterMetrics(ViburDBCPDataSource dataSource) { + ConnectionPoolMetrics.unregisterMetrics(dataSource); + } +} diff --git a/instrumentation/vibur-dbcp-11.0/library/src/test/java/io/opentelemetry/instrumentation/viburdbcp/ViburInstrumentationTest.java b/instrumentation/vibur-dbcp-11.0/library/src/test/java/io/opentelemetry/instrumentation/viburdbcp/ViburInstrumentationTest.java new file mode 100644 index 000000000000..3126c9662965 --- /dev/null +++ b/instrumentation/vibur-dbcp-11.0/library/src/test/java/io/opentelemetry/instrumentation/viburdbcp/ViburInstrumentationTest.java @@ -0,0 +1,32 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.viburdbcp; + +import io.opentelemetry.instrumentation.testing.internal.AutoCleanupExtension; +import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.vibur.dbcp.ViburDBCPDataSource; + +class ViburInstrumentationTest extends AbstractViburInstrumentationTest { + + @RegisterExtension + static final InstrumentationExtension testing = LibraryInstrumentationExtension.create(); + + @RegisterExtension static final AutoCleanupExtension cleanup = AutoCleanupExtension.create(); + + @Override + protected InstrumentationExtension testing() { + return testing; + } + + @Override + protected void configure(ViburDBCPDataSource viburDataSource) { + ViburTelemetry telemetry = ViburTelemetry.create(testing().getOpenTelemetry()); + telemetry.registerMetrics(viburDataSource); + cleanup.deferCleanup(() -> telemetry.unregisterMetrics(viburDataSource)); + } +} diff --git a/instrumentation/vibur-dbcp-11.0/testing/build.gradle.kts b/instrumentation/vibur-dbcp-11.0/testing/build.gradle.kts new file mode 100644 index 000000000000..0d7c62cdf46e --- /dev/null +++ b/instrumentation/vibur-dbcp-11.0/testing/build.gradle.kts @@ -0,0 +1,11 @@ +plugins { + id("otel.java-conventions") +} + +dependencies { + api(project(":testing-common")) + api("org.mockito:mockito-core") + api("org.mockito:mockito-junit-jupiter") + + compileOnly("org.vibur:vibur-dbcp:11.0") +} diff --git a/instrumentation/vibur-dbcp-11.0/testing/src/main/java/io/opentelemetry/instrumentation/viburdbcp/AbstractViburInstrumentationTest.java b/instrumentation/vibur-dbcp-11.0/testing/src/main/java/io/opentelemetry/instrumentation/viburdbcp/AbstractViburInstrumentationTest.java new file mode 100644 index 000000000000..f356cc8be83d --- /dev/null +++ b/instrumentation/vibur-dbcp-11.0/testing/src/main/java/io/opentelemetry/instrumentation/viburdbcp/AbstractViburInstrumentationTest.java @@ -0,0 +1,97 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.viburdbcp; + +import static org.mockito.Mockito.when; + +import io.opentelemetry.instrumentation.testing.internal.AutoCleanupExtension; +import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.db.DbConnectionPoolMetricsAssertions; +import java.sql.Connection; +import java.sql.SQLException; +import java.util.concurrent.TimeUnit; +import javax.sql.DataSource; +import org.assertj.core.api.AbstractIterableAssert; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.vibur.dbcp.ViburDBCPDataSource; + +@ExtendWith(MockitoExtension.class) +public abstract class AbstractViburInstrumentationTest { + + @RegisterExtension static final AutoCleanupExtension cleanup = AutoCleanupExtension.create(); + + @Mock DataSource dataSourceMock; + @Mock Connection connectionMock; + + protected abstract InstrumentationExtension testing(); + + protected abstract void configure(ViburDBCPDataSource viburDataSource); + + @Test + void shouldReportMetrics() throws SQLException, InterruptedException { + // given + when(dataSourceMock.getConnection()).thenReturn(connectionMock); + + ViburDBCPDataSource viburDataSource = new ViburDBCPDataSource(); + viburDataSource.setExternalDataSource(dataSourceMock); + viburDataSource.setName("testPool"); + configure(viburDataSource); + viburDataSource.start(); + + // when + Connection hikariConnection = viburDataSource.getConnection(); + TimeUnit.MILLISECONDS.sleep(100); + hikariConnection.close(); + + // then + DbConnectionPoolMetricsAssertions.create( + testing(), "io.opentelemetry.viburdbcp-11.0", "testPool") + .disableMinIdleConnections() + .disableMaxIdleConnections() + .disablePendingRequests() + .disableConnectionTimeouts() + .disableCreateTime() + .disableWaitTime() + .disableUseTime() + .assertConnectionPoolEmitsMetrics(); + + // when + // this one too shouldn't cause any problems when called more than once + viburDataSource.close(); + viburDataSource.close(); + + // sleep exporter interval + Thread.sleep(100); + testing().clearData(); + Thread.sleep(100); + + // then + testing() + .waitAndAssertMetrics( + "io.opentelemetry.hikaricp-3.0", + "db.client.connections.usage", + AbstractIterableAssert::isEmpty); + testing() + .waitAndAssertMetrics( + "io.opentelemetry.hikaricp-3.0", + "db.client.connections.idle.min", + AbstractIterableAssert::isEmpty); + testing() + .waitAndAssertMetrics( + "io.opentelemetry.hikaricp-3.0", + "db.client.connections.max", + AbstractIterableAssert::isEmpty); + testing() + .waitAndAssertMetrics( + "io.opentelemetry.hikaricp-3.0", + "db.client.connections.pending_requests", + AbstractIterableAssert::isEmpty); + } +} diff --git a/settings.gradle.kts b/settings.gradle.kts index e05efbc38471..2d226c637b1b 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -442,6 +442,9 @@ include(":instrumentation:vertx:vertx-kafka-client-3.6:testing") include(":instrumentation:vertx:vertx-rx-java-3.5:javaagent") include(":instrumentation:vertx:vertx-web-3.0:javaagent") include(":instrumentation:vertx:vertx-web-3.0:testing") +include(":instrumentation:vibur-dbcp-11.0:javaagent") +include(":instrumentation:vibur-dbcp-11.0:library") +include(":instrumentation:vibur-dbcp-11.0:testing") include(":instrumentation:wicket-8.0:javaagent") // benchmark diff --git a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/db/DbConnectionPoolMetricsAssertions.java b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/db/DbConnectionPoolMetricsAssertions.java index 30ae076b462e..2dfc9fae327d 100644 --- a/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/db/DbConnectionPoolMetricsAssertions.java +++ b/testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/db/DbConnectionPoolMetricsAssertions.java @@ -22,8 +22,13 @@ public static DbConnectionPoolMetricsAssertions create( private final String instrumentationName; private final String poolName; + private boolean testMinIdleConnections = true; private boolean testMaxIdleConnections = true; + private boolean testPendingRequests = true; private boolean testConnectionTimeouts = true; + private boolean testCreateTime = true; + private boolean testWaitTime = true; + private boolean testUseTime = true; DbConnectionPoolMetricsAssertions( InstrumentationExtension testing, String instrumentationName, String poolName) { @@ -32,16 +37,41 @@ public static DbConnectionPoolMetricsAssertions create( this.poolName = poolName; } + public DbConnectionPoolMetricsAssertions disableMinIdleConnections() { + testMinIdleConnections = false; + return this; + } + public DbConnectionPoolMetricsAssertions disableMaxIdleConnections() { testMaxIdleConnections = false; return this; } + public DbConnectionPoolMetricsAssertions disablePendingRequests() { + testPendingRequests = false; + return this; + } + public DbConnectionPoolMetricsAssertions disableConnectionTimeouts() { testConnectionTimeouts = false; return this; } + public DbConnectionPoolMetricsAssertions disableCreateTime() { + testCreateTime = false; + return this; + } + + public DbConnectionPoolMetricsAssertions disableWaitTime() { + testWaitTime = false; + return this; + } + + public DbConnectionPoolMetricsAssertions disableUseTime() { + testUseTime = false; + return this; + } + public void assertConnectionPoolEmitsMetrics() { testing.waitAndAssertMetrics( instrumentationName, @@ -71,23 +101,25 @@ public void assertConnectionPoolEmitsMetrics() { poolName, stringKey("state"), "used")))))); - testing.waitAndAssertMetrics( - instrumentationName, - "db.client.connections.idle.min", - metrics -> - metrics.anySatisfy( - metric -> - assertThat(metric) - .hasUnit("connections") - .hasDescription("The minimum number of idle open connections allowed.") - .hasLongSumSatisfying( - sum -> - sum.isNotMonotonic() - .hasPointsSatisfying( - point -> - point.hasAttributes( - Attributes.of( - stringKey("pool.name"), poolName)))))); + if (testMinIdleConnections) { + testing.waitAndAssertMetrics( + instrumentationName, + "db.client.connections.idle.min", + metrics -> + metrics.anySatisfy( + metric -> + assertThat(metric) + .hasUnit("connections") + .hasDescription("The minimum number of idle open connections allowed.") + .hasLongSumSatisfying( + sum -> + sum.isNotMonotonic() + .hasPointsSatisfying( + point -> + point.hasAttributes( + Attributes.of( + stringKey("pool.name"), poolName)))))); + } if (testMaxIdleConnections) { testing.waitAndAssertMetrics( instrumentationName, @@ -124,24 +156,26 @@ public void assertConnectionPoolEmitsMetrics() { point.hasAttributes( Attributes.of( stringKey("pool.name"), poolName)))))); - testing.waitAndAssertMetrics( - instrumentationName, - "db.client.connections.pending_requests", - metrics -> - metrics.anySatisfy( - metric -> - assertThat(metric) - .hasUnit("requests") - .hasDescription( - "The number of pending requests for an open connection, cumulative for the entire pool.") - .hasLongSumSatisfying( - sum -> - sum.isNotMonotonic() - .hasPointsSatisfying( - point -> - point.hasAttributes( - Attributes.of( - stringKey("pool.name"), poolName)))))); + if (testPendingRequests) { + testing.waitAndAssertMetrics( + instrumentationName, + "db.client.connections.pending_requests", + metrics -> + metrics.anySatisfy( + metric -> + assertThat(metric) + .hasUnit("requests") + .hasDescription( + "The number of pending requests for an open connection, cumulative for the entire pool.") + .hasLongSumSatisfying( + sum -> + sum.isNotMonotonic() + .hasPointsSatisfying( + point -> + point.hasAttributes( + Attributes.of( + stringKey("pool.name"), poolName)))))); + } if (testConnectionTimeouts) { testing.waitAndAssertMetrics( instrumentationName, @@ -162,52 +196,58 @@ public void assertConnectionPoolEmitsMetrics() { Attributes.of( stringKey("pool.name"), poolName)))))); } - testing.waitAndAssertMetrics( - instrumentationName, - "db.client.connections.create_time", - metrics -> - metrics.anySatisfy( - metric -> - assertThat(metric) - .hasUnit("ms") - .hasDescription("The time it took to create a new connection.") - .hasHistogramSatisfying( - histogram -> - histogram.hasPointsSatisfying( - point -> - point.hasAttributes( - Attributes.of(stringKey("pool.name"), poolName)))))); - testing.waitAndAssertMetrics( - instrumentationName, - "db.client.connections.wait_time", - metrics -> - metrics.anySatisfy( - metric -> - assertThat(metric) - .hasUnit("ms") - .hasDescription( - "The time it took to obtain an open connection from the pool.") - .hasHistogramSatisfying( - histogram -> - histogram.hasPointsSatisfying( - point -> - point.hasAttributes( - Attributes.of(stringKey("pool.name"), poolName)))))); - testing.waitAndAssertMetrics( - instrumentationName, - "db.client.connections.use_time", - metrics -> - metrics.anySatisfy( - metric -> - assertThat(metric) - .hasUnit("ms") - .hasDescription( - "The time between borrowing a connection and returning it to the pool.") - .hasHistogramSatisfying( - histogram -> - histogram.hasPointsSatisfying( - point -> - point.hasAttributes( - Attributes.of(stringKey("pool.name"), poolName)))))); + if (testCreateTime) { + testing.waitAndAssertMetrics( + instrumentationName, + "db.client.connections.create_time", + metrics -> + metrics.anySatisfy( + metric -> + assertThat(metric) + .hasUnit("ms") + .hasDescription("The time it took to create a new connection.") + .hasHistogramSatisfying( + histogram -> + histogram.hasPointsSatisfying( + point -> + point.hasAttributes( + Attributes.of(stringKey("pool.name"), poolName)))))); + } + if (testWaitTime) { + testing.waitAndAssertMetrics( + instrumentationName, + "db.client.connections.wait_time", + metrics -> + metrics.anySatisfy( + metric -> + assertThat(metric) + .hasUnit("ms") + .hasDescription( + "The time it took to obtain an open connection from the pool.") + .hasHistogramSatisfying( + histogram -> + histogram.hasPointsSatisfying( + point -> + point.hasAttributes( + Attributes.of(stringKey("pool.name"), poolName)))))); + } + if (testUseTime) { + testing.waitAndAssertMetrics( + instrumentationName, + "db.client.connections.use_time", + metrics -> + metrics.anySatisfy( + metric -> + assertThat(metric) + .hasUnit("ms") + .hasDescription( + "The time between borrowing a connection and returning it to the pool.") + .hasHistogramSatisfying( + histogram -> + histogram.hasPointsSatisfying( + point -> + point.hasAttributes( + Attributes.of(stringKey("pool.name"), poolName)))))); + } } } From 09be39ef46723518a68746c91e160f82f2527d1b Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 25 May 2022 22:19:50 +0300 Subject: [PATCH 2/5] Apply suggestions from code review Co-authored-by: Trask Stalnaker --- instrumentation/vibur-dbcp-11.0/library/README.md | 2 +- .../opentelemetry/instrumentation/viburdbcp/ViburTelemetry.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/vibur-dbcp-11.0/library/README.md b/instrumentation/vibur-dbcp-11.0/library/README.md index 2244a9dca673..3f33d76c2e32 100644 --- a/instrumentation/vibur-dbcp-11.0/library/README.md +++ b/instrumentation/vibur-dbcp-11.0/library/README.md @@ -1,4 +1,4 @@ -# Manual Instrumentation for HikariCP +# Manual Instrumentation for Vibur DBCP Provides OpenTelemetry instrumentation for [Vibur DBCP](https://www.vibur.org/). diff --git a/instrumentation/vibur-dbcp-11.0/library/src/main/java/io/opentelemetry/instrumentation/viburdbcp/ViburTelemetry.java b/instrumentation/vibur-dbcp-11.0/library/src/main/java/io/opentelemetry/instrumentation/viburdbcp/ViburTelemetry.java index 328cc3922b7c..a6c9a4b394ac 100644 --- a/instrumentation/vibur-dbcp-11.0/library/src/main/java/io/opentelemetry/instrumentation/viburdbcp/ViburTelemetry.java +++ b/instrumentation/vibur-dbcp-11.0/library/src/main/java/io/opentelemetry/instrumentation/viburdbcp/ViburTelemetry.java @@ -8,7 +8,7 @@ import io.opentelemetry.api.OpenTelemetry; import org.vibur.dbcp.ViburDBCPDataSource; -/** Entrypoint for instrumenting Hikari database connection pools. */ +/** Entrypoint for instrumenting Vibur database connection pools. */ public final class ViburTelemetry { /** Returns a new {@link ViburTelemetry} configured with the given {@link OpenTelemetry}. */ From ae5f4ab893429bb61ac932c0ee8003ce56847ce2 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 25 May 2022 22:29:20 +0300 Subject: [PATCH 3/5] address review comments --- .../viburdbcp/ViburInstrumentationTest.java | 3 +++ .../viburdbcp/ViburInstrumentationTest.java | 11 +++++++---- .../AbstractViburInstrumentationTest.java | 15 +++++++++------ 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/instrumentation/vibur-dbcp-11.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/ViburInstrumentationTest.java b/instrumentation/vibur-dbcp-11.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/ViburInstrumentationTest.java index ca08eeaf98ba..c97f653db2af 100644 --- a/instrumentation/vibur-dbcp-11.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/ViburInstrumentationTest.java +++ b/instrumentation/vibur-dbcp-11.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/viburdbcp/ViburInstrumentationTest.java @@ -23,4 +23,7 @@ protected InstrumentationExtension testing() { @Override protected void configure(ViburDBCPDataSource viburDataSource) {} + + @Override + protected void shutdown(ViburDBCPDataSource viburDataSource) {} } diff --git a/instrumentation/vibur-dbcp-11.0/library/src/test/java/io/opentelemetry/instrumentation/viburdbcp/ViburInstrumentationTest.java b/instrumentation/vibur-dbcp-11.0/library/src/test/java/io/opentelemetry/instrumentation/viburdbcp/ViburInstrumentationTest.java index 3126c9662965..8873818f99f4 100644 --- a/instrumentation/vibur-dbcp-11.0/library/src/test/java/io/opentelemetry/instrumentation/viburdbcp/ViburInstrumentationTest.java +++ b/instrumentation/vibur-dbcp-11.0/library/src/test/java/io/opentelemetry/instrumentation/viburdbcp/ViburInstrumentationTest.java @@ -5,7 +5,6 @@ package io.opentelemetry.instrumentation.viburdbcp; -import io.opentelemetry.instrumentation.testing.internal.AutoCleanupExtension; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension; import org.junit.jupiter.api.extension.RegisterExtension; @@ -16,7 +15,7 @@ class ViburInstrumentationTest extends AbstractViburInstrumentationTest { @RegisterExtension static final InstrumentationExtension testing = LibraryInstrumentationExtension.create(); - @RegisterExtension static final AutoCleanupExtension cleanup = AutoCleanupExtension.create(); + private ViburTelemetry telemetry; @Override protected InstrumentationExtension testing() { @@ -25,8 +24,12 @@ protected InstrumentationExtension testing() { @Override protected void configure(ViburDBCPDataSource viburDataSource) { - ViburTelemetry telemetry = ViburTelemetry.create(testing().getOpenTelemetry()); + telemetry = ViburTelemetry.create(testing().getOpenTelemetry()); telemetry.registerMetrics(viburDataSource); - cleanup.deferCleanup(() -> telemetry.unregisterMetrics(viburDataSource)); + } + + @Override + protected void shutdown(ViburDBCPDataSource viburDataSource) { + telemetry.unregisterMetrics(viburDataSource); } } diff --git a/instrumentation/vibur-dbcp-11.0/testing/src/main/java/io/opentelemetry/instrumentation/viburdbcp/AbstractViburInstrumentationTest.java b/instrumentation/vibur-dbcp-11.0/testing/src/main/java/io/opentelemetry/instrumentation/viburdbcp/AbstractViburInstrumentationTest.java index f356cc8be83d..1b13987103a0 100644 --- a/instrumentation/vibur-dbcp-11.0/testing/src/main/java/io/opentelemetry/instrumentation/viburdbcp/AbstractViburInstrumentationTest.java +++ b/instrumentation/vibur-dbcp-11.0/testing/src/main/java/io/opentelemetry/instrumentation/viburdbcp/AbstractViburInstrumentationTest.java @@ -34,6 +34,8 @@ public abstract class AbstractViburInstrumentationTest { protected abstract void configure(ViburDBCPDataSource viburDataSource); + protected abstract void shutdown(ViburDBCPDataSource viburDataSource); + @Test void shouldReportMetrics() throws SQLException, InterruptedException { // given @@ -46,9 +48,9 @@ void shouldReportMetrics() throws SQLException, InterruptedException { viburDataSource.start(); // when - Connection hikariConnection = viburDataSource.getConnection(); + Connection viburConnection = viburDataSource.getConnection(); TimeUnit.MILLISECONDS.sleep(100); - hikariConnection.close(); + viburConnection.close(); // then DbConnectionPoolMetricsAssertions.create( @@ -66,6 +68,7 @@ void shouldReportMetrics() throws SQLException, InterruptedException { // this one too shouldn't cause any problems when called more than once viburDataSource.close(); viburDataSource.close(); + shutdown(viburDataSource); // sleep exporter interval Thread.sleep(100); @@ -75,22 +78,22 @@ void shouldReportMetrics() throws SQLException, InterruptedException { // then testing() .waitAndAssertMetrics( - "io.opentelemetry.hikaricp-3.0", + "io.opentelemetry.viburdbcp-11.0", "db.client.connections.usage", AbstractIterableAssert::isEmpty); testing() .waitAndAssertMetrics( - "io.opentelemetry.hikaricp-3.0", + "io.opentelemetry.viburdbcp-11.0", "db.client.connections.idle.min", AbstractIterableAssert::isEmpty); testing() .waitAndAssertMetrics( - "io.opentelemetry.hikaricp-3.0", + "io.opentelemetry.viburdbcp-11.0", "db.client.connections.max", AbstractIterableAssert::isEmpty); testing() .waitAndAssertMetrics( - "io.opentelemetry.hikaricp-3.0", + "io.opentelemetry.viburdbcp-11.0", "db.client.connections.pending_requests", AbstractIterableAssert::isEmpty); } From 1f90673d192a6eb32b40ca8e0cdf510325956109 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 25 May 2022 22:31:27 +0300 Subject: [PATCH 4/5] don't check for metircs that aren't reported --- .../viburdbcp/AbstractViburInstrumentationTest.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/instrumentation/vibur-dbcp-11.0/testing/src/main/java/io/opentelemetry/instrumentation/viburdbcp/AbstractViburInstrumentationTest.java b/instrumentation/vibur-dbcp-11.0/testing/src/main/java/io/opentelemetry/instrumentation/viburdbcp/AbstractViburInstrumentationTest.java index 1b13987103a0..a7e250ef5fe7 100644 --- a/instrumentation/vibur-dbcp-11.0/testing/src/main/java/io/opentelemetry/instrumentation/viburdbcp/AbstractViburInstrumentationTest.java +++ b/instrumentation/vibur-dbcp-11.0/testing/src/main/java/io/opentelemetry/instrumentation/viburdbcp/AbstractViburInstrumentationTest.java @@ -81,20 +81,10 @@ void shouldReportMetrics() throws SQLException, InterruptedException { "io.opentelemetry.viburdbcp-11.0", "db.client.connections.usage", AbstractIterableAssert::isEmpty); - testing() - .waitAndAssertMetrics( - "io.opentelemetry.viburdbcp-11.0", - "db.client.connections.idle.min", - AbstractIterableAssert::isEmpty); testing() .waitAndAssertMetrics( "io.opentelemetry.viburdbcp-11.0", "db.client.connections.max", AbstractIterableAssert::isEmpty); - testing() - .waitAndAssertMetrics( - "io.opentelemetry.viburdbcp-11.0", - "db.client.connections.pending_requests", - AbstractIterableAssert::isEmpty); } } From 7ce4c7ce35c91897e8347ab6678111d9f33d4ccb Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 25 May 2022 23:12:05 +0300 Subject: [PATCH 5/5] rework library test setup --- .../viburdbcp/ViburInstrumentationTest.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/instrumentation/vibur-dbcp-11.0/library/src/test/java/io/opentelemetry/instrumentation/viburdbcp/ViburInstrumentationTest.java b/instrumentation/vibur-dbcp-11.0/library/src/test/java/io/opentelemetry/instrumentation/viburdbcp/ViburInstrumentationTest.java index 8873818f99f4..58a327cc735e 100644 --- a/instrumentation/vibur-dbcp-11.0/library/src/test/java/io/opentelemetry/instrumentation/viburdbcp/ViburInstrumentationTest.java +++ b/instrumentation/vibur-dbcp-11.0/library/src/test/java/io/opentelemetry/instrumentation/viburdbcp/ViburInstrumentationTest.java @@ -7,6 +7,7 @@ import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.extension.RegisterExtension; import org.vibur.dbcp.ViburDBCPDataSource; @@ -15,16 +16,20 @@ class ViburInstrumentationTest extends AbstractViburInstrumentationTest { @RegisterExtension static final InstrumentationExtension testing = LibraryInstrumentationExtension.create(); - private ViburTelemetry telemetry; + private static ViburTelemetry telemetry; @Override protected InstrumentationExtension testing() { return testing; } + @BeforeAll + static void setup() { + telemetry = ViburTelemetry.create(testing.getOpenTelemetry()); + } + @Override protected void configure(ViburDBCPDataSource viburDataSource) { - telemetry = ViburTelemetry.create(testing().getOpenTelemetry()); telemetry.registerMetrics(viburDataSource); }