Skip to content

fix(druid): optimize dataSourceName to resolve metrics high cardinality #19108

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.

Sign up for GitHub

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

Open
YaoYingLong wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(druid): optimize dataSourceName to resolve metrics high cardinality #19108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ac0b6e2
fix(druid): optimize dataSourceName to resolve metrics high cardinali…
YaoYingLong Jun 30, 2026
df56163
refactor: spotlessApply
YaoYingLong Jun 30, 2026
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
14 changes: 11 additions & 3 deletions ...telemetry/javaagent/instrumentation/alibabadruid/v1_0/DruidDataSourceInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@
import static net.bytebuddy.matcher.ElementMatchers.isPublic;
import static net.bytebuddy.matcher.ElementMatchers.isStatic;
import static net.bytebuddy.matcher.ElementMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;

import com.alibaba.druid.pool.DruidDataSourceMBean;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import javax.annotation.Nullable;
import javax.management.ObjectName;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.type.TypeDescription;
Expand All @@ -27,7 +29,11 @@ public ElementMatcher<TypeDescription> typeMatcher() {
@Override
public void transform(TypeTransformer typeTransformer) {
typeTransformer.applyAdviceToMethod(
isPublic().and(isStatic()).and(named("addDataSource")),
isPublic()
.and(isStatic())
.and(named("addDataSource"))
.and(takesArgument(0, named("java.lang.Object")))
.and(takesArgument(1, named("java.lang.String"))),
getClass().getName() + "$AddDataSourceAdvice");

typeTransformer.applyAdviceToMethod(
Expand All @@ -40,10 +46,12 @@ public static class AddDataSourceAdvice {

@Advice.OnMethodExit(suppress = Throwable.class, inline = false)
public static void onExit(
@Advice.Argument(0) Object dataSource, @Advice.Return ObjectName objectName) {
@Advice.Argument(0) Object dataSource,
@Advice.Argument(1) @Nullable String name,
@Advice.Return ObjectName objectName) {
DruidDataSourceMBean druidDataSource = (DruidDataSourceMBean) dataSource;
String dataSourceName =
objectName.getKeyProperty("type") + "-" + objectName.getKeyProperty("id");
objectName.getKeyProperty("type") + "-" + (name == null ? "unknown" : name);

@laurit laurit Jul 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think that we usually just omit name components that are not present instead of using placeholders like unknown.
Noticed that spec https://opentelemetry.io/docs/specs/semconv/registry/attributes/db/#db-client-connection-pool-name provides some guidance on how to create a name when it is not available. Idk whether this is a viable strategy since these properties might not be available either.

@YaoYingLong YaoYingLong Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In typical business scenarios, when configuring a data source using DruidDataSource, the setName or setObjectName methods of DruidDataSource are not called by default. Similarly, in Spring projects, the name property is generally not configured via application.yml. The current implementation will definitely result in a high cardinality issue, and we can simply omit these fields as you mentioned, since neither the type nor the passed name attribute in objectName may be obtainable.

I’m wondering if we should skip the unavailable attribute as long as either one of the two can be retrieved. If neither attribute can be obtained, we need a fallback strategy with a default value such as unknown, default, or something component-related like druid.

@laurit @trask Could you two please review this and share any better suggestions?

telemetry().registerMetrics(druidDataSource, dataSourceName);
}
}
Expand Down
17 changes: 17 additions & 0 deletions ...o/opentelemetry/javaagent/instrumentation/alibabadruid/v1_0/DruidInstrumentationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import io.opentelemetry.instrumentation.alibabadruid.AbstractDruidInstrumentationTest;
import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

class DruidInstrumentationTest extends AbstractDruidInstrumentationTest {
Expand All @@ -31,4 +32,20 @@ protected void configure(DruidDataSource dataSource, String name) throws Excepti
protected void shutdown(DruidDataSource dataSource) throws Exception {
DruidDataSourceStatManager.removeDataSource(dataSource);
}

@Test
void shouldUseUnknownDataSourceNameWhenNameIsNull() throws Exception {
DruidDataSource dataSource = createDataSource();

try {
configure(dataSource, null);

assertConnectionUsagePoolNames("DruidDataSource-unknown");
} finally {
dataSource.close();
shutdown(dataSource);
}

assertNoMetrics();
}
}
100 changes: 68 additions & 32 deletions ...c/main/java/io/opentelemetry/instrumentation/alibabadruid/v1_0/ConnectionPoolMetrics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,54 +12,90 @@
import io.opentelemetry.api.metrics.ObservableLongMeasurement;
import io.opentelemetry.instrumentation.api.incubator.semconv.db.DbConnectionPoolMetrics;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;

final class ConnectionPoolMetrics {

private static final String INSTRUMENTATION_NAME = "io.opentelemetry.alibaba-druid-1.0";

private static final Map<DruidDataSourceMBean, BatchCallback> dataSourceMetrics =
private static final Map<DruidDataSourceMBean, DataSourceMetricsRegistration> dataSourceMetrics =
new ConcurrentHashMap<>();
private static final Set<String> activeDataSourceNames = ConcurrentHashMap.newKeySet();

static void registerMetrics(
OpenTelemetry openTelemetry, DruidDataSourceMBean dataSource, String dataSourceName) {
dataSourceMetrics.computeIfAbsent(
dataSource,
ds -> {
DbConnectionPoolMetrics metrics =
DbConnectionPoolMetrics.create(openTelemetry, INSTRUMENTATION_NAME, dataSourceName);

ObservableLongMeasurement connections = metrics.connections();
ObservableLongMeasurement minIdleConnections = metrics.minIdleConnections();
ObservableLongMeasurement maxIdleConnections = metrics.maxIdleConnections();
ObservableLongMeasurement maxConnections = metrics.maxConnections();
ObservableLongMeasurement pendingRequestsForConnection =
metrics.pendingRequestsForConnection();

Attributes attributes = metrics.getAttributes();
Attributes usedConnectionsAttributes = metrics.getUsedConnectionsAttributes();
Attributes idleConnectionsAttributes = metrics.getIdleConnectionsAttributes();

return metrics.batchCallback(
() -> {
connections.record(ds.getActiveCount(), usedConnectionsAttributes);
connections.record(ds.getPoolingCount(), idleConnectionsAttributes);
pendingRequestsForConnection.record(ds.getWaitThreadCount(), attributes);
minIdleConnections.record(ds.getMinIdle(), attributes);
maxIdleConnections.record(ds.getMaxIdle(), attributes);
maxConnections.record(ds.getMaxActive(), attributes);
},
connections,
pendingRequestsForConnection,
minIdleConnections,
maxIdleConnections,
maxConnections);
unused -> {
String rewrittenDataSourceName = reserveDataSourceName(dataSourceName);
return new DataSourceMetricsRegistration(
createCallback(openTelemetry, dataSource, rewrittenDataSourceName),
rewrittenDataSourceName);
});
}

private static String reserveDataSourceName(String dataSourceName) {

@laurit laurit Jul 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So the idea here is that when there are multiple data sources with the same name we'll append a suffix. Since we are not doing this for other connection pool instrumentations it is questionable whether we should be doing this here. If we decide to do this then we should apply the same strategy across the connection pool instrumentations.
cc @trask any suggestions on how to handle this?

if (activeDataSourceNames.add(dataSourceName)) {
return dataSourceName;
}
for (int count = 2; ; count++) {
String candidate = dataSourceName + "-" + count;

@trask trask Jul 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Pending the discussion in #19160 (comment), I think we should avoid process-local -N suffixes here. If no stable name exists, use a stable fallback and let duplicates merge so metric identities remain consistent across restarts.

if (activeDataSourceNames.add(candidate)) {
return candidate;
}
}
}

private static BatchCallback createCallback(
OpenTelemetry openTelemetry, DruidDataSourceMBean dataSource, String dataSourceName) {
DbConnectionPoolMetrics metrics =
DbConnectionPoolMetrics.create(openTelemetry, INSTRUMENTATION_NAME, dataSourceName);

ObservableLongMeasurement connections = metrics.connections();
ObservableLongMeasurement minIdleConnections = metrics.minIdleConnections();
ObservableLongMeasurement maxIdleConnections = metrics.maxIdleConnections();
ObservableLongMeasurement maxConnections = metrics.maxConnections();
ObservableLongMeasurement pendingRequestsForConnection = metrics.pendingRequestsForConnection();

Attributes attributes = metrics.getAttributes();
Attributes usedConnectionsAttributes = metrics.getUsedConnectionsAttributes();
Attributes idleConnectionsAttributes = metrics.getIdleConnectionsAttributes();

return metrics.batchCallback(
() -> {
connections.record(dataSource.getActiveCount(), usedConnectionsAttributes);
connections.record(dataSource.getPoolingCount(), idleConnectionsAttributes);
pendingRequestsForConnection.record(dataSource.getWaitThreadCount(), attributes);
minIdleConnections.record(dataSource.getMinIdle(), attributes);
maxIdleConnections.record(dataSource.getMaxIdle(), attributes);
maxConnections.record(dataSource.getMaxActive(), attributes);
},
connections,
pendingRequestsForConnection,
minIdleConnections,
maxIdleConnections,
maxConnections);
}

static void unregisterMetrics(DruidDataSourceMBean dataSource) {
BatchCallback callback = dataSourceMetrics.remove(dataSource);
if (callback != null) {
DataSourceMetricsRegistration registration = dataSourceMetrics.remove(dataSource);
if (registration != null) {
registration.close();
}
}

private static final class DataSourceMetricsRegistration {
private final BatchCallback callback;
private final String dataSourceName;

private DataSourceMetricsRegistration(BatchCallback callback, String dataSourceName) {
this.callback = callback;
this.dataSourceName = dataSourceName;
}

private void close() {
activeDataSourceNames.remove(dataSourceName);
callback.close();
}
}
Expand Down
4 changes: 3 additions & 1 deletion ...est/java/io/opentelemetry/instrumentation/alibabadruid/v1_0/DruidInstrumentationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ static void setup() {

@Override
protected void configure(DruidDataSource dataSource, String name) throws Exception {
ObjectName objectName = new ObjectName("com.alibaba.druid:type=DruidDataSource,id=" + name);
String dataSourceName = name == null ? "unknown" : name;
ObjectName objectName =
new ObjectName("com.alibaba.druid:type=DruidDataSource,id=" + dataSourceName);
telemetry.registerMetrics(
dataSource, objectName.getKeyProperty("type") + "-" + objectName.getKeyProperty("id"));
}
Expand Down
144 changes: 123 additions & 21 deletions .../java/io/opentelemetry/instrumentation/alibabadruid/AbstractDruidInstrumentationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,28 @@

package io.opentelemetry.instrumentation.alibabadruid;

import static io.opentelemetry.instrumentation.api.internal.SemconvStability.emitStableDatabaseSemconv;
import static java.util.stream.Collectors.toSet;
import static org.assertj.core.api.Assertions.assertThat;
import static org.awaitility.Awaitility.await;

import com.alibaba.druid.pool.DruidDataSource;
import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
import io.opentelemetry.instrumentation.testing.junit.db.DbConnectionPoolMetricsAssertions;
import io.opentelemetry.instrumentation.testing.junit.db.MockDriver;
import java.sql.SQLException;
import javax.management.ObjectName;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

public abstract class AbstractDruidInstrumentationTest {

private static final String INSTRUMENTATION_NAME = "io.opentelemetry.alibaba-druid-1.0";
private static final AttributeKey<String> POOL_NAME_KEY =
AttributeKey.stringKey(
emitStableDatabaseSemconv() ? "db.client.connection.pool.name" : "pool.name");
private static final String CONNECTION_USAGE_METRIC_NAME =
emitStableDatabaseSemconv() ? "db.client.connection.count" : "db.client.connections.usage";

protected abstract InstrumentationExtension testing();

Expand All @@ -36,32 +43,104 @@ static void setUpMocks() throws SQLException {
@Test
void shouldReportMetrics() throws Exception {
String name = "dataSourceName";
DruidDataSource dataSource = new DruidDataSource();
dataSource.setDriverClassName(MockDriver.class.getName());
dataSource.setUrl("db:///url");
dataSource.setTestWhileIdle(false);
configure(dataSource, name);
DruidDataSource dataSource = createDataSource();

// then
ObjectName objectName = new ObjectName("com.alibaba.druid:type=DruidDataSource,id=" + name);
try {
configure(dataSource, name);

DbConnectionPoolMetricsAssertions.create(
testing(),
INSTRUMENTATION_NAME,
objectName.getKeyProperty("type") + "-" + objectName.getKeyProperty("id"))
.disableConnectionTimeouts()
.disableCreateTime()
.disableWaitTime()
.disableUseTime()
.assertConnectionPoolEmitsMetrics();
DbConnectionPoolMetricsAssertions.create(
testing(), INSTRUMENTATION_NAME, "DruidDataSource-" + name)
.disableConnectionTimeouts()
.disableCreateTime()
.disableWaitTime()
.disableUseTime()
.assertConnectionPoolEmitsMetrics();
} finally {
dataSource.close();
shutdown(dataSource);
}

assertNoMetrics();
}

@Test
void shouldRewriteDuplicateDataSourceNames() throws Exception {
DruidDataSource firstDataSource = createDataSource();
DruidDataSource secondDataSource = createDataSource();

try {
configure(firstDataSource, "duplicatePool");
configure(secondDataSource, "duplicatePool");

assertConnectionUsagePoolNames(
"DruidDataSource-duplicatePool", "DruidDataSource-duplicatePool-2");
} finally {
firstDataSource.close();
secondDataSource.close();
shutdown(firstDataSource);
shutdown(secondDataSource);
}

assertNoMetrics();
}

@Test
void shouldReuseDataSourceNameAfterShutdown() throws Exception {
DruidDataSource firstDataSource = createDataSource();
try {
configure(firstDataSource, "reusablePool");

assertConnectionUsagePoolNames("DruidDataSource-reusablePool");
} finally {
firstDataSource.close();
shutdown(firstDataSource);
}

assertNoMetrics();

DruidDataSource secondDataSource = createDataSource();
try {
configure(secondDataSource, "reusablePool");

// when
dataSource.close();
shutdown(dataSource);
assertConnectionUsagePoolNames("DruidDataSource-reusablePool");
} finally {
secondDataSource.close();
shutdown(secondDataSource);
}

assertNoMetrics();
}

@Test
void shouldSkipReservedDataSourceNameSuffix() throws Exception {
DruidDataSource firstDataSource = createDataSource();
DruidDataSource reservedDataSource = createDataSource();
DruidDataSource secondDataSource = createDataSource();

try {
configure(firstDataSource, "reservedPool");
configure(reservedDataSource, "reservedPool-2");
configure(secondDataSource, "reservedPool");

assertConnectionUsagePoolNames(
"DruidDataSource-reservedPool",
"DruidDataSource-reservedPool-2",
"DruidDataSource-reservedPool-3");
} finally {
firstDataSource.close();
reservedDataSource.close();
secondDataSource.close();
shutdown(firstDataSource);
shutdown(reservedDataSource);
shutdown(secondDataSource);
}

assertNoMetrics();
}

protected void assertNoMetrics() {
testing().clearData();

// then
await()
.untilAsserted(
() ->
Expand All @@ -74,4 +153,27 @@ void shouldReportMetrics() throws Exception {
.equals(INSTRUMENTATION_NAME))
.isEmpty());
}

protected static DruidDataSource createDataSource() {
DruidDataSource dataSource = new DruidDataSource();
dataSource.setDriverClassName(MockDriver.class.getName());
dataSource.setUrl("db:///url");
dataSource.setTestWhileIdle(false);
return dataSource;
}

protected void assertConnectionUsagePoolNames(String... poolNames) {
testing()
.waitAndAssertMetrics(
INSTRUMENTATION_NAME,
CONNECTION_USAGE_METRIC_NAME,
metrics ->
metrics.anySatisfy(
metric ->
assertThat(
metric.getLongSumData().getPoints().stream()
.map(point -> point.getAttributes().get(POOL_NAME_KEY))
.collect(toSet()))
.containsExactlyInAnyOrder(poolNames)));
}
}
Loading