From 24f84b3845100a2a31cc8467b23844782743fefc Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Mon, 28 Apr 2025 17:15:17 +0300 Subject: [PATCH] Implement stable semconv for db connection pool metrics --- .../semconv/db/DbConnectionPoolMetrics.java | 78 +++++++++++++------ .../c3p0-0.9/javaagent/build.gradle.kts | 10 +++ .../c3p0-0.9/library/build.gradle.kts | 10 +++ .../hikaricp-3.0/javaagent/build.gradle.kts | 10 +++ .../hikaricp-3.0/library/build.gradle.kts | 10 +++ .../v3_0/OpenTelemetryMetricsTracker.java | 18 ++++- .../javaagent/build.gradle.kts | 8 ++ .../oracle-ucp-11.2/library/build.gradle.kts | 8 ++ .../javaagent/build.gradle.kts | 10 +++ .../vibur-dbcp-11.0/library/build.gradle.kts | 10 +++ .../db/DbConnectionPoolMetricsAssertions.java | 58 +++++++++----- 11 files changed, 182 insertions(+), 48 deletions(-) diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbConnectionPoolMetrics.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbConnectionPoolMetrics.java index 96eb6246f702..baad9837369f 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbConnectionPoolMetrics.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbConnectionPoolMetrics.java @@ -27,8 +27,10 @@ */ public final class DbConnectionPoolMetrics { - static final AttributeKey POOL_NAME = stringKey("pool.name"); - static final AttributeKey CONNECTION_STATE = stringKey("state"); + static final AttributeKey POOL_NAME = + stringKey(emitStableDatabaseSemconv() ? "db.client.connection.pool.name" : "pool.name"); + static final AttributeKey CONNECTION_STATE = + stringKey(emitStableDatabaseSemconv() ? "db.client.connection.state" : "state"); static final String STATE_IDLE = "idle"; static final String STATE_USED = "used"; @@ -61,42 +63,58 @@ public ObservableLongMeasurement connections() { emitStableDatabaseSemconv() ? "db.client.connection.count" : "db.client.connections.usage"; return meter .upDownCounterBuilder(metricName) - .setUnit("{connections}") + .setUnit(emitStableDatabaseSemconv() ? "{connection}" : "{connections}") .setDescription( "The number of connections that are currently in state described by the state attribute.") .buildObserver(); } public ObservableLongMeasurement minIdleConnections() { + String metricName = + emitStableDatabaseSemconv() + ? "db.client.connection.idle.min" + : "db.client.connections.idle.min"; return meter - .upDownCounterBuilder("db.client.connections.idle.min") - .setUnit("{connections}") + .upDownCounterBuilder(metricName) + .setUnit(emitStableDatabaseSemconv() ? "{connection}" : "{connections}") .setDescription("The minimum number of idle open connections allowed.") .buildObserver(); } public ObservableLongMeasurement maxIdleConnections() { + String metricName = + emitStableDatabaseSemconv() + ? "db.client.connection.idle.max" + : "db.client.connections.idle.max"; return meter - .upDownCounterBuilder("db.client.connections.idle.max") - .setUnit("{connections}") + .upDownCounterBuilder(metricName) + .setUnit(emitStableDatabaseSemconv() ? "{connection}" : "{connections}") .setDescription("The maximum number of idle open connections allowed.") .buildObserver(); } public ObservableLongMeasurement maxConnections() { + String metricName = + emitStableDatabaseSemconv() ? "db.client.connection.max" : "db.client.connections.max"; return meter - .upDownCounterBuilder("db.client.connections.max") - .setUnit("{connections}") + .upDownCounterBuilder(metricName) + .setUnit(emitStableDatabaseSemconv() ? "{connection}" : "{connections}") .setDescription("The maximum number of open connections allowed.") .buildObserver(); } public ObservableLongMeasurement pendingRequestsForConnection() { + String metricName = + emitStableDatabaseSemconv() + ? "db.client.connection.pending_requests" + : "db.client.connections.pending_requests"; return meter - .upDownCounterBuilder("db.client.connections.pending_requests") - .setUnit("{requests}") + .upDownCounterBuilder(metricName) + .setUnit(emitStableDatabaseSemconv() ? "{request}" : "{requests}") .setDescription( - "The number of pending requests for an open connection, cumulative for the entire pool.") + emitStableDatabaseSemconv() + ? "The number of current pending requests for an open connection." + : "The number of pending requests for an open connection, cumulative for the entire pool.") .buildObserver(); } @@ -107,39 +125,51 @@ public BatchCallback batchCallback( return meter.batchCallback(callback, observableMeasurement, additionalMeasurements); } - // TODO: should be a BoundLongCounter public LongCounter connectionTimeouts() { + String metricName = + emitStableDatabaseSemconv() + ? "db.client.connection.timeouts" + : "db.client.connections.timeouts"; return meter - .counterBuilder("db.client.connections.timeouts") - .setUnit("{timeouts}") + .counterBuilder(metricName) + .setUnit(emitStableDatabaseSemconv() ? "{timeout}" : "{timeouts}") .setDescription( "The number of connection timeouts that have occurred trying to obtain a connection from the pool.") .build(); } - // TODO: should be a BoundDoubleHistogram public DoubleHistogram connectionCreateTime() { + String metricName = + emitStableDatabaseSemconv() + ? "db.client.connection.create_time" + : "db.client.connections.create_time"; return meter - .histogramBuilder("db.client.connections.create_time") - .setUnit("ms") + .histogramBuilder(metricName) + .setUnit(emitStableDatabaseSemconv() ? "s" : "ms") .setDescription("The time it took to create a new connection.") .build(); } - // TODO: should be a BoundDoubleHistogram public DoubleHistogram connectionWaitTime() { + String metricName = + emitStableDatabaseSemconv() + ? "db.client.connection.wait_time" + : "db.client.connections.wait_time"; return meter - .histogramBuilder("db.client.connections.wait_time") - .setUnit("ms") + .histogramBuilder(metricName) + .setUnit(emitStableDatabaseSemconv() ? "s" : "ms") .setDescription("The time it took to obtain an open connection from the pool.") .build(); } - // TODO: should be a BoundDoubleHistogram public DoubleHistogram connectionUseTime() { + String metricName = + emitStableDatabaseSemconv() + ? "db.client.connection.use_time" + : "db.client.connections.use_time"; return meter - .histogramBuilder("db.client.connections.use_time") - .setUnit("ms") + .histogramBuilder(metricName) + .setUnit(emitStableDatabaseSemconv() ? "s" : "ms") .setDescription("The time between borrowing a connection and returning it to the pool.") .build(); } diff --git a/instrumentation/c3p0-0.9/javaagent/build.gradle.kts b/instrumentation/c3p0-0.9/javaagent/build.gradle.kts index 8cc92cb886c4..eb97e869462f 100644 --- a/instrumentation/c3p0-0.9/javaagent/build.gradle.kts +++ b/instrumentation/c3p0-0.9/javaagent/build.gradle.kts @@ -20,3 +20,13 @@ dependencies { testImplementation(project(":instrumentation:c3p0-0.9:testing")) } + +tasks { + val testStableSemconv by registering(Test::class) { + jvmArgs("-Dotel.semconv-stability.opt-in=database") + } + + check { + dependsOn(testStableSemconv) + } +} diff --git a/instrumentation/c3p0-0.9/library/build.gradle.kts b/instrumentation/c3p0-0.9/library/build.gradle.kts index d6970bff8f9e..a7b7f6a397ee 100644 --- a/instrumentation/c3p0-0.9/library/build.gradle.kts +++ b/instrumentation/c3p0-0.9/library/build.gradle.kts @@ -8,3 +8,13 @@ dependencies { testImplementation(project(":instrumentation:c3p0-0.9:testing")) } + +tasks { + val testStableSemconv by registering(Test::class) { + jvmArgs("-Dotel.semconv-stability.opt-in=database") + } + + check { + dependsOn(testStableSemconv) + } +} diff --git a/instrumentation/hikaricp-3.0/javaagent/build.gradle.kts b/instrumentation/hikaricp-3.0/javaagent/build.gradle.kts index ea7881dc34ed..714af1aaef50 100644 --- a/instrumentation/hikaricp-3.0/javaagent/build.gradle.kts +++ b/instrumentation/hikaricp-3.0/javaagent/build.gradle.kts @@ -21,3 +21,13 @@ dependencies { testImplementation(project(":instrumentation:hikaricp-3.0:testing")) } + +tasks { + val testStableSemconv by registering(Test::class) { + jvmArgs("-Dotel.semconv-stability.opt-in=database") + } + + check { + dependsOn(testStableSemconv) + } +} diff --git a/instrumentation/hikaricp-3.0/library/build.gradle.kts b/instrumentation/hikaricp-3.0/library/build.gradle.kts index df4c38e46395..73fda349d202 100644 --- a/instrumentation/hikaricp-3.0/library/build.gradle.kts +++ b/instrumentation/hikaricp-3.0/library/build.gradle.kts @@ -8,3 +8,13 @@ dependencies { testImplementation(project(":instrumentation:hikaricp-3.0:testing")) } + +tasks { + val testStableSemconv by registering(Test::class) { + jvmArgs("-Dotel.semconv-stability.opt-in=database") + } + + check { + dependsOn(testStableSemconv) + } +} diff --git a/instrumentation/hikaricp-3.0/library/src/main/java/io/opentelemetry/instrumentation/hikaricp/v3_0/OpenTelemetryMetricsTracker.java b/instrumentation/hikaricp-3.0/library/src/main/java/io/opentelemetry/instrumentation/hikaricp/v3_0/OpenTelemetryMetricsTracker.java index d670a6548e31..7dbfb5a3fdb6 100644 --- a/instrumentation/hikaricp-3.0/library/src/main/java/io/opentelemetry/instrumentation/hikaricp/v3_0/OpenTelemetryMetricsTracker.java +++ b/instrumentation/hikaricp-3.0/library/src/main/java/io/opentelemetry/instrumentation/hikaricp/v3_0/OpenTelemetryMetricsTracker.java @@ -5,6 +5,8 @@ package io.opentelemetry.instrumentation.hikaricp.v3_0; +import static io.opentelemetry.instrumentation.api.internal.SemconvStability.emitStableDatabaseSemconv; + import com.zaxxer.hikari.metrics.IMetricsTracker; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.metrics.BatchCallback; @@ -15,6 +17,8 @@ final class OpenTelemetryMetricsTracker implements IMetricsTracker { private static final double NANOS_PER_MS = TimeUnit.MILLISECONDS.toNanos(1); + private static final double NANOS_PER_S = TimeUnit.SECONDS.toNanos(1); + private static final double MILLIS_PER_S = TimeUnit.SECONDS.toMillis(1); private final IMetricsTracker userMetricsTracker; @@ -44,20 +48,26 @@ final class OpenTelemetryMetricsTracker implements IMetricsTracker { @Override public void recordConnectionCreatedMillis(long connectionCreatedMillis) { - createTime.record((double) connectionCreatedMillis, attributes); + double time = + emitStableDatabaseSemconv() + ? connectionCreatedMillis / MILLIS_PER_S + : connectionCreatedMillis; + createTime.record(time, attributes); userMetricsTracker.recordConnectionCreatedMillis(connectionCreatedMillis); } @Override public void recordConnectionAcquiredNanos(long elapsedAcquiredNanos) { - double millis = elapsedAcquiredNanos / NANOS_PER_MS; - waitTime.record(millis, attributes); + double time = elapsedAcquiredNanos / (emitStableDatabaseSemconv() ? NANOS_PER_S : NANOS_PER_MS); + waitTime.record(time, attributes); userMetricsTracker.recordConnectionAcquiredNanos(elapsedAcquiredNanos); } @Override public void recordConnectionUsageMillis(long elapsedBorrowedMillis) { - useTime.record((double) elapsedBorrowedMillis, attributes); + double time = + emitStableDatabaseSemconv() ? elapsedBorrowedMillis / MILLIS_PER_S : elapsedBorrowedMillis; + useTime.record(time, attributes); userMetricsTracker.recordConnectionUsageMillis(elapsedBorrowedMillis); } diff --git a/instrumentation/oracle-ucp-11.2/javaagent/build.gradle.kts b/instrumentation/oracle-ucp-11.2/javaagent/build.gradle.kts index 136a8c53bf76..ce212bdd92dc 100644 --- a/instrumentation/oracle-ucp-11.2/javaagent/build.gradle.kts +++ b/instrumentation/oracle-ucp-11.2/javaagent/build.gradle.kts @@ -24,4 +24,12 @@ tasks { test { usesService(gradle.sharedServices.registrations["testcontainersBuildService"].service) } + + val testStableSemconv by registering(Test::class) { + jvmArgs("-Dotel.semconv-stability.opt-in=database") + } + + check { + dependsOn(testStableSemconv) + } } diff --git a/instrumentation/oracle-ucp-11.2/library/build.gradle.kts b/instrumentation/oracle-ucp-11.2/library/build.gradle.kts index 937adaaf315b..b9b6020e3e2d 100644 --- a/instrumentation/oracle-ucp-11.2/library/build.gradle.kts +++ b/instrumentation/oracle-ucp-11.2/library/build.gradle.kts @@ -14,4 +14,12 @@ tasks { test { usesService(gradle.sharedServices.registrations["testcontainersBuildService"].service) } + + val testStableSemconv by registering(Test::class) { + jvmArgs("-Dotel.semconv-stability.opt-in=database") + } + + check { + dependsOn(testStableSemconv) + } } diff --git a/instrumentation/vibur-dbcp-11.0/javaagent/build.gradle.kts b/instrumentation/vibur-dbcp-11.0/javaagent/build.gradle.kts index d5e27facd406..01476009a2e2 100644 --- a/instrumentation/vibur-dbcp-11.0/javaagent/build.gradle.kts +++ b/instrumentation/vibur-dbcp-11.0/javaagent/build.gradle.kts @@ -18,3 +18,13 @@ dependencies { testImplementation(project(":instrumentation:vibur-dbcp-11.0:testing")) } + +tasks { + val testStableSemconv by registering(Test::class) { + jvmArgs("-Dotel.semconv-stability.opt-in=database") + } + + check { + dependsOn(testStableSemconv) + } +} diff --git a/instrumentation/vibur-dbcp-11.0/library/build.gradle.kts b/instrumentation/vibur-dbcp-11.0/library/build.gradle.kts index 381f5c75b8d5..9f0a4b44cb9c 100644 --- a/instrumentation/vibur-dbcp-11.0/library/build.gradle.kts +++ b/instrumentation/vibur-dbcp-11.0/library/build.gradle.kts @@ -8,3 +8,13 @@ dependencies { testImplementation(project(":instrumentation:vibur-dbcp-11.0:testing")) } + +tasks { + val testStableSemconv by registering(Test::class) { + jvmArgs("-Dotel.semconv-stability.opt-in=database") + } + + check { + dependsOn(testStableSemconv) + } +} 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 4ed89c7061fc..6716f292a6bc 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 @@ -20,8 +20,10 @@ public final class DbConnectionPoolMetricsAssertions { - private static final AttributeKey POOL_NAME_KEY = stringKey("pool.name"); - private static final AttributeKey STATE_KEY = stringKey("state"); + private static final AttributeKey POOL_NAME_KEY = + stringKey(emitStableDatabaseSemconv() ? "db.client.connection.pool.name" : "pool.name"); + private static final AttributeKey STATE_KEY = + stringKey(emitStableDatabaseSemconv() ? "db.client.connection.state" : "state"); public static DbConnectionPoolMetricsAssertions create( InstrumentationExtension testing, String instrumentationName, String poolName) { @@ -133,7 +135,7 @@ private void verifyConnectionUsage() { private void verifyUsageMetric(MetricData metric) { assertThat(metric) - .hasUnit("{connections}") + .hasUnit(emitStableDatabaseSemconv() ? "{connection}" : "{connections}") .hasDescription( "The number of connections that are currently in state described by the state attribute.") .hasLongSumSatisfying( @@ -151,13 +153,13 @@ private void verifyUsageMetric(MetricData metric) { private void verifyMaxConnections() { testing.waitAndAssertMetrics( instrumentationName, - "db.client.connections.max", + emitStableDatabaseSemconv() ? "db.client.connection.max" : "db.client.connections.max", metrics -> metrics.anySatisfy(this::verifyMaxConnectionsMetric)); } private void verifyMaxConnectionsMetric(MetricData metric) { assertThat(metric) - .hasUnit("{connections}") + .hasUnit(emitStableDatabaseSemconv() ? "{connection}" : "{connections}") .hasDescription("The maximum number of open connections allowed.") .hasLongSumSatisfying(this::verifyPoolName); } @@ -165,13 +167,15 @@ private void verifyMaxConnectionsMetric(MetricData metric) { private void verifyMinIdleConnections() { testing.waitAndAssertMetrics( instrumentationName, - "db.client.connections.idle.min", + emitStableDatabaseSemconv() + ? "db.client.connection.idle.min" + : "db.client.connections.idle.min", metrics -> metrics.anySatisfy(this::verifyMinIdleConnectionsMetric)); } private void verifyMinIdleConnectionsMetric(MetricData metric) { assertThat(metric) - .hasUnit("{connections}") + .hasUnit(emitStableDatabaseSemconv() ? "{connection}" : "{connections}") .hasDescription("The minimum number of idle open connections allowed.") .hasLongSumSatisfying(this::verifyPoolName); } @@ -179,13 +183,15 @@ private void verifyMinIdleConnectionsMetric(MetricData metric) { private void verifyMaxIdleConnections() { testing.waitAndAssertMetrics( instrumentationName, - "db.client.connections.idle.max", + emitStableDatabaseSemconv() + ? "db.client.connection.idle.max" + : "db.client.connections.idle.max", metrics -> metrics.anySatisfy(this::verifyMaxIdleConnectionsMetric)); } private void verifyMaxIdleConnectionsMetric(MetricData metric) { assertThat(metric) - .hasUnit("{connections}") + .hasUnit(emitStableDatabaseSemconv() ? "{connection}" : "{connections}") .hasDescription("The maximum number of idle open connections allowed.") .hasLongSumSatisfying(this::verifyPoolName); } @@ -198,28 +204,34 @@ private void verifyPoolName(LongSumAssert sum) { private void verifyPendingRequests() { testing.waitAndAssertMetrics( instrumentationName, - "db.client.connections.pending_requests", + emitStableDatabaseSemconv() + ? "db.client.connection.pending_requests" + : "db.client.connections.pending_requests", metrics -> metrics.anySatisfy(this::verifyPendingRequestsMetric)); } private void verifyPendingRequestsMetric(MetricData metric) { assertThat(metric) - .hasUnit("{requests}") + .hasUnit(emitStableDatabaseSemconv() ? "{request}" : "{requests}") .hasDescription( - "The number of pending requests for an open connection, cumulative for the entire pool.") + emitStableDatabaseSemconv() + ? "The number of current pending requests for an open connection." + : "The number of pending requests for an open connection, cumulative for the entire pool.") .hasLongSumSatisfying(this::verifyPoolName); } private void verifyTimeouts() { testing.waitAndAssertMetrics( instrumentationName, - "db.client.connections.timeouts", + emitStableDatabaseSemconv() + ? "db.client.connection.timeouts" + : "db.client.connections.timeouts", metrics -> metrics.anySatisfy(this::verifyTimeoutsMetric)); } private void verifyTimeoutsMetric(MetricData metric) { assertThat(metric) - .hasUnit("{timeouts}") + .hasUnit(emitStableDatabaseSemconv() ? "{timeout}" : "{timeouts}") .hasDescription( "The number of connection timeouts that have occurred trying to obtain a connection from the pool.") .hasLongSumSatisfying( @@ -232,13 +244,15 @@ private void verifyTimeoutsMetric(MetricData metric) { private void verifyCreateTime() { testing.waitAndAssertMetrics( instrumentationName, - "db.client.connections.create_time", + emitStableDatabaseSemconv() + ? "db.client.connection.create_time" + : "db.client.connections.create_time", metrics -> metrics.anySatisfy(this::verifyCreateTimeMetric)); } private void verifyCreateTimeMetric(MetricData metric) { assertThat(metric) - .hasUnit("ms") + .hasUnit(emitStableDatabaseSemconv() ? "s" : "ms") .hasDescription("The time it took to create a new connection.") .hasHistogramSatisfying( histogram -> @@ -249,13 +263,15 @@ private void verifyCreateTimeMetric(MetricData metric) { private void verifyWaitTime() { testing.waitAndAssertMetrics( instrumentationName, - "db.client.connections.wait_time", + emitStableDatabaseSemconv() + ? "db.client.connection.wait_time" + : "db.client.connections.wait_time", metrics -> metrics.anySatisfy(this::verifyWaitTimeMetric)); } private void verifyWaitTimeMetric(MetricData metric) { assertThat(metric) - .hasUnit("ms") + .hasUnit(emitStableDatabaseSemconv() ? "s" : "ms") .hasDescription("The time it took to obtain an open connection from the pool.") .hasHistogramSatisfying( histogram -> @@ -266,13 +282,15 @@ private void verifyWaitTimeMetric(MetricData metric) { private void verifyUseTime() { testing.waitAndAssertMetrics( instrumentationName, - "db.client.connections.use_time", + emitStableDatabaseSemconv() + ? "db.client.connection.use_time" + : "db.client.connections.use_time", metrics -> metrics.anySatisfy(this::verifyUseTimeMetric)); } private MetricAssert verifyUseTimeMetric(MetricData metric) { return assertThat(metric) - .hasUnit("ms") + .hasUnit(emitStableDatabaseSemconv() ? "s" : "ms") .hasDescription("The time between borrowing a connection and returning it to the pool.") .hasHistogramSatisfying( histogram ->