Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add tomcat-jdbc connection pool metrics instrumentation #6102

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/supported-libraries.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ These are the supported libraries and frameworks:
| [Spring Web Services](https://spring.io/projects/spring-ws) | 2.0+ |
| [Spring WebFlux](https://docs.spring.io/spring/docs/current/javadoc-api/org/springframework/web/reactive/package-summary.html) | 5.0+ |
| [Spymemcached](https://github.com/couchbase/spymemcached) | 2.12+ |
| [Tomcat JDBC Pool](https://tomcat.apache.org/tomcat-7.0-doc/jdbc-pool.html) | 8.5.0+ |
| [Twilio](https://github.com/twilio/twilio-java) | 6.6+ (not including 8.x yet) |
| [Undertow](https://undertow.io/) | 1.4+ |
| [Vaadin](https://vaadin.com/) | 14.2+ |
Expand Down
17 changes: 17 additions & 0 deletions instrumentation/tomcat/tomcat-jdbc/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
plugins {
id("otel.javaagent-instrumentation")
}

muzzle {
pass {
group.set("org.apache.tomcat")
module.set("tomcat-jdbc")
versions.set("[8.5.0,)")
// no assertInverse because tomcat-jdbc < 8.5 doesn't have methods that we hook into
}
}

dependencies {
compileOnly("org.apache.tomcat:tomcat-jdbc:8.5.0")
testImplementation("org.apache.tomcat:tomcat-jdbc:8.5.0")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.tomcat.jdbc;

import static net.bytebuddy.matcher.ElementMatchers.isPublic;
import static net.bytebuddy.matcher.ElementMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
import static net.bytebuddy.matcher.ElementMatchers.takesNoArguments;

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.apache.tomcat.jdbc.pool.DataSourceProxy;

class DataSourceProxyInstrumentation implements TypeInstrumentation {

@Override
public ElementMatcher<TypeDescription> typeMatcher() {
return named("org.apache.tomcat.jdbc.pool.DataSourceProxy");
}

@Override
public void transform(TypeTransformer transformer) {
transformer.applyAdviceToMethod(
isPublic().and(named("createPool")).and(takesNoArguments()),
this.getClass().getName() + "$CreatePoolAdvice");

transformer.applyAdviceToMethod(
isPublic().and(named("close")).and(takesArguments(1)),
this.getClass().getName() + "$CloseAdvice");
}

@SuppressWarnings("unused")
public static class CreatePoolAdvice {
@Advice.OnMethodExit(suppress = Throwable.class)
public static void onExit(@Advice.This DataSourceProxy dataSource) {
TomcatConnectionPoolMetrics.registerMetrics(dataSource);
}
}

@SuppressWarnings("unused")
public static class CloseAdvice {
@Advice.OnMethodExit(suppress = Throwable.class)
public static void onExit(@Advice.This DataSourceProxy dataSource) {
TomcatConnectionPoolMetrics.unregisterMetrics(dataSource);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.tomcat.jdbc;

import io.opentelemetry.api.GlobalOpenTelemetry;
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.apache.tomcat.jdbc.pool.DataSourceProxy;

public final class TomcatConnectionPoolMetrics {

private static final OpenTelemetry openTelemetry = GlobalOpenTelemetry.get();
private static final String INSTRUMENTATION_NAME = "io.opentelemetry.tomcat-jdbc";

// a weak map does not make sense here because each Meter holds a reference to the dataSource
// DataSourceProxy does not implement equals()/hashCode(), so it's safe to keep them in a plain
// ConcurrentHashMap
private static final Map<DataSourceProxy, List<ObservableLongUpDownCounter>> dataSourceMetrics =
breedx-splk marked this conversation as resolved.
Show resolved Hide resolved
new ConcurrentHashMap<>();

public static void registerMetrics(DataSourceProxy dataSource) {
dataSourceMetrics.computeIfAbsent(dataSource, TomcatConnectionPoolMetrics::createCounters);
}

private static List<ObservableLongUpDownCounter> createCounters(DataSourceProxy dataSource) {

DbConnectionPoolMetrics metrics =
DbConnectionPoolMetrics.create(
openTelemetry, INSTRUMENTATION_NAME, dataSource.getPoolName());

return Arrays.asList(
metrics.usedConnections(dataSource::getActive),
metrics.idleConnections(dataSource::getIdle),
metrics.minIdleConnections(dataSource::getMinIdle),
metrics.maxIdleConnections(dataSource::getMaxIdle),
metrics.maxConnections(dataSource::getMaxActive),
metrics.pendingRequestsForConnection(dataSource::getWaitCount));
}

public static void unregisterMetrics(DataSourceProxy dataSource) {
List<ObservableLongUpDownCounter> counters = dataSourceMetrics.remove(dataSource);
if (counters != null) {
for (ObservableLongUpDownCounter meter : counters) {
meter.close();
}
}
}

private TomcatConnectionPoolMetrics() {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.tomcat.jdbc;

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 TomcatJdbcInstrumentationModule extends InstrumentationModule {
public TomcatJdbcInstrumentationModule() {
super("tomcat-jdbc");
}

@Override
public List<TypeInstrumentation> typeInstrumentations() {
return singletonList(new DataSourceProxyInstrumentation());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.tomcat.jdbc;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.BDDMockito.given;

import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
import io.opentelemetry.instrumentation.testing.junit.db.DbConnectionPoolMetricsAssertions;
import java.sql.Connection;
import org.apache.tomcat.jdbc.pool.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;

@ExtendWith(MockitoExtension.class)
public class TomcatJdbcInstrumentationTest {

@RegisterExtension
static final AgentInstrumentationExtension testing = AgentInstrumentationExtension.create();

@Mock javax.sql.DataSource dataSourceMock;
@Mock Connection connectionMock;

@Test
void shouldReportMetrics() throws Exception {
// given
given(dataSourceMock.getConnection()).willReturn(connectionMock);

DataSource tomcatDataSource = new DataSource();
tomcatDataSource.setDataSource(dataSourceMock);

// there shouldn't be any problems if this methods gets called more than once
tomcatDataSource.createPool();
tomcatDataSource.createPool();

// when
Connection connection = tomcatDataSource.getConnection();
Thread.sleep(100);
connection.close();

// then
assertConnectionPoolMetrics(tomcatDataSource.getPoolName());

// when
// this one too shouldn't cause any problems when called more than once
tomcatDataSource.close();
tomcatDataSource.close();
Thread.sleep(100);
testing.clearData();
Thread.sleep(100);

// then
assertNoConnectionPoolMetrics();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to wait at least [metric interval] before this assertion?

also having similar conversation #6099 (comment)

}

private static void assertConnectionPoolMetrics(String poolName) {
assertThat(poolName)
.as("tomcat-jdbc generates a unique pool name if it's not explicitly provided")
.isNotEmpty();

DbConnectionPoolMetricsAssertions.create(testing, "io.opentelemetry.tomcat-jdbc", poolName)
// no timeouts happen during this test
.disableConnectionTimeouts()
.disableCreateTime()
.disableWaitTime()
.disableUseTime()
.assertConnectionPoolEmitsMetrics();
}

private static void assertNoConnectionPoolMetrics() {
testing.waitAndAssertMetrics(
"io.opentelemetry.tomcat-jdbc",
"db.client.connections.usage",
AbstractIterableAssert::isEmpty);
testing.waitAndAssertMetrics(
"io.opentelemetry.tomcat-jdbc",
"db.client.connections.idle.min",
AbstractIterableAssert::isEmpty);
testing.waitAndAssertMetrics(
"io.opentelemetry.tomcat-jdbc",
"db.client.connections.idle.max",
AbstractIterableAssert::isEmpty);
testing.waitAndAssertMetrics(
"io.opentelemetry.tomcat-jdbc",
"db.client.connections.max",
AbstractIterableAssert::isEmpty);
testing.waitAndAssertMetrics(
"io.opentelemetry.tomcat-jdbc",
"db.client.connections.pending_requests",
AbstractIterableAssert::isEmpty);
}
}
1 change: 1 addition & 0 deletions settings.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ include(":instrumentation:tapestry-5.4:javaagent")
include(":instrumentation:tomcat:tomcat-7.0:javaagent")
include(":instrumentation:tomcat:tomcat-10.0:javaagent")
include(":instrumentation:tomcat:tomcat-common:javaagent")
include(":instrumentation:tomcat:tomcat-jdbc")
include(":instrumentation:twilio-6.6:javaagent")
include(":instrumentation:undertow-1.4:bootstrap")
include(":instrumentation:undertow-1.4:javaagent")
Expand Down