From 81f3150ad29844ab56124882b43896abb95c9063 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Wed, 2 Apr 2025 10:39:33 +0200 Subject: [PATCH 1/6] extract method --- .../semconv/db/DbResponseStatusUtil.java | 20 +++++++++++++++++++ .../semconv/http/HttpStatusCodeConverter.java | 4 ++-- .../ElasticsearchDbAttributesGetter.java | 7 ++----- .../rest/OpenSearchRestAttributesGetter.java | 8 +++----- 4 files changed, 27 insertions(+), 12 deletions(-) create mode 100644 instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbResponseStatusUtil.java diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbResponseStatusUtil.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbResponseStatusUtil.java new file mode 100644 index 000000000000..f6313913ceef --- /dev/null +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbResponseStatusUtil.java @@ -0,0 +1,20 @@ +package io.opentelemetry.instrumentation.api.incubator.semconv.db; + +import javax.annotation.Nullable; + +public class DbResponseStatusUtil { + private DbResponseStatusUtil() { + } + + @Nullable + public static String dbResponseStatusCode(int responseStatusCode) { + return isError(responseStatusCode) ? Integer.toString(responseStatusCode) : null; + } + + private static boolean isError(int responseStatusCode) { + return responseStatusCode >= 400 + || + // invalid status code, does not exist + responseStatusCode < 100; + } +} diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpStatusCodeConverter.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpStatusCodeConverter.java index eb1724d69d35..740c4280422a 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpStatusCodeConverter.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpStatusCodeConverter.java @@ -12,7 +12,7 @@ enum HttpStatusCodeConverter { boolean isError(int responseStatusCode) { return responseStatusCode >= 500 || - // invalid status code, does not exists + // invalid status code, does not exist responseStatusCode < 100; } }, @@ -21,7 +21,7 @@ boolean isError(int responseStatusCode) { boolean isError(int responseStatusCode) { return responseStatusCode >= 400 || - // invalid status code, does not exists + // invalid status code, does not exist responseStatusCode < 100; } }; diff --git a/instrumentation/elasticsearch/elasticsearch-rest-common-5.0/library/src/main/java/io/opentelemetry/instrumentation/elasticsearch/rest/common/v5_0/internal/ElasticsearchDbAttributesGetter.java b/instrumentation/elasticsearch/elasticsearch-rest-common-5.0/library/src/main/java/io/opentelemetry/instrumentation/elasticsearch/rest/common/v5_0/internal/ElasticsearchDbAttributesGetter.java index 8e8cf35835ca..1bf981028ec9 100644 --- a/instrumentation/elasticsearch/elasticsearch-rest-common-5.0/library/src/main/java/io/opentelemetry/instrumentation/elasticsearch/rest/common/v5_0/internal/ElasticsearchDbAttributesGetter.java +++ b/instrumentation/elasticsearch/elasticsearch-rest-common-5.0/library/src/main/java/io/opentelemetry/instrumentation/elasticsearch/rest/common/v5_0/internal/ElasticsearchDbAttributesGetter.java @@ -5,6 +5,7 @@ package io.opentelemetry.instrumentation.elasticsearch.rest.common.v5_0.internal; +import static io.opentelemetry.instrumentation.api.incubator.semconv.db.DbResponseStatusUtil.dbResponseStatusCode; import static java.util.logging.Level.FINE; import io.opentelemetry.instrumentation.api.incubator.semconv.db.DbClientAttributesGetter; @@ -96,10 +97,6 @@ public String getDbOperationName(ElasticsearchRestRequest request) { @Nullable @Override public String getResponseStatus(@Nullable Response response, @Nullable Throwable error) { - if (response != null) { - int httpStatus = response.getStatusLine().getStatusCode(); - return httpStatus >= 400 && httpStatus < 600 ? Integer.toString(httpStatus) : null; - } - return null; + return response != null ? dbResponseStatusCode(response.getStatusLine().getStatusCode()) : null; } } diff --git a/instrumentation/opensearch/opensearch-rest-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opensearch/rest/OpenSearchRestAttributesGetter.java b/instrumentation/opensearch/opensearch-rest-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opensearch/rest/OpenSearchRestAttributesGetter.java index 6a572e8ca234..ff7ecd470f5f 100644 --- a/instrumentation/opensearch/opensearch-rest-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opensearch/rest/OpenSearchRestAttributesGetter.java +++ b/instrumentation/opensearch/opensearch-rest-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opensearch/rest/OpenSearchRestAttributesGetter.java @@ -5,6 +5,8 @@ package io.opentelemetry.javaagent.instrumentation.opensearch.rest; +import static io.opentelemetry.instrumentation.api.incubator.semconv.db.DbResponseStatusUtil.dbResponseStatusCode; + import io.opentelemetry.instrumentation.api.incubator.semconv.db.DbClientAttributesGetter; import io.opentelemetry.semconv.incubating.DbIncubatingAttributes; import javax.annotation.Nullable; @@ -54,10 +56,6 @@ public String getDbOperationName(OpenSearchRestRequest request) { @Nullable @Override public String getResponseStatus(@Nullable Response response, @Nullable Throwable error) { - if (response != null) { - int httpStatus = response.getStatusLine().getStatusCode(); - return httpStatus >= 400 && httpStatus < 600 ? Integer.toString(httpStatus) : null; - } - return null; + return response != null ? dbResponseStatusCode(response.getStatusLine().getStatusCode()) : null; } } From 5dc4f515c8823e84bbec1f775888dff4ee7dbbc7 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Wed, 2 Apr 2025 10:43:20 +0200 Subject: [PATCH 2/6] cleanup test code --- .../spymemcached/SpymemcachedTest.java | 58 +++++++++---------- 1 file changed, 27 insertions(+), 31 deletions(-) diff --git a/instrumentation/spymemcached-2.12/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spymemcached/SpymemcachedTest.java b/instrumentation/spymemcached-2.12/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spymemcached/SpymemcachedTest.java index 4b5a3231474e..0408bbdd19af 100644 --- a/instrumentation/spymemcached-2.12/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spymemcached/SpymemcachedTest.java +++ b/instrumentation/spymemcached-2.12/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spymemcached/SpymemcachedTest.java @@ -29,17 +29,14 @@ import io.opentelemetry.instrumentation.testing.internal.AutoCleanupExtension; import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; -import io.opentelemetry.sdk.testing.assertj.AttributeAssertion; import io.opentelemetry.sdk.trace.data.StatusData; import io.opentelemetry.semconv.incubating.DbIncubatingAttributes; import java.net.InetSocketAddress; import java.time.Duration; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.BlockingQueue; @@ -263,22 +260,15 @@ void getTimeout() throws InterruptedException { EXCEPTION_STACKTRACE, val -> val.isInstanceOf(String.class)))) .hasAttributesSatisfyingExactly( - withErrorType( - "net.spy.memcached.internal.CheckedOperationTimeoutException", - equalTo( - maybeStable(DB_SYSTEM), - DbIncubatingAttributes.DbSystemIncubatingValues.MEMCACHED), - equalTo(maybeStable(DB_OPERATION), "get"))))); - } - - private static List withErrorType( - String errorType, AttributeAssertion... assertions) { - List list = new ArrayList<>(Arrays.asList(assertions)); - - if (SemconvStability.emitStableDatabaseSemconv()) { - list.add(equalTo(ERROR_TYPE, errorType)); - } - return list; + equalTo( + ERROR_TYPE, + SemconvStability.emitStableDatabaseSemconv() + ? "net.spy.memcached.internal.CheckedOperationTimeoutException" + : null), + equalTo( + maybeStable(DB_SYSTEM), + DbIncubatingAttributes.DbSystemIncubatingValues.MEMCACHED), + equalTo(maybeStable(DB_OPERATION), "get")))); } @Test @@ -896,12 +886,15 @@ void decrException() { .hasException( new IllegalArgumentException("Key is too long (maxlen = 250)")) .hasAttributesSatisfyingExactly( - withErrorType( - "java.lang.IllegalArgumentException", - equalTo( - maybeStable(DB_SYSTEM), - DbIncubatingAttributes.DbSystemIncubatingValues.MEMCACHED), - equalTo(maybeStable(DB_OPERATION), "decr"))))); + equalTo( + ERROR_TYPE, + SemconvStability.emitStableDatabaseSemconv() + ? "java.lang.IllegalArgumentException" + : null), + equalTo( + maybeStable(DB_SYSTEM), + DbIncubatingAttributes.DbSystemIncubatingValues.MEMCACHED), + equalTo(maybeStable(DB_OPERATION), "decr")))); } @Test @@ -984,12 +977,15 @@ void incrException() { .hasException( new IllegalArgumentException("Key is too long (maxlen = 250)")) .hasAttributesSatisfyingExactly( - withErrorType( - "java.lang.IllegalArgumentException", - equalTo( - maybeStable(DB_SYSTEM), - DbIncubatingAttributes.DbSystemIncubatingValues.MEMCACHED), - equalTo(maybeStable(DB_OPERATION), "incr"))))); + equalTo( + ERROR_TYPE, + SemconvStability.emitStableDatabaseSemconv() + ? "java.lang.IllegalArgumentException" + : null), + equalTo( + maybeStable(DB_SYSTEM), + DbIncubatingAttributes.DbSystemIncubatingValues.MEMCACHED), + equalTo(maybeStable(DB_OPERATION), "incr")))); } private static String key(String k) { From aeb5b6baacfdcf3e12ace65e39e92b10d8277382 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Wed, 2 Apr 2025 14:17:50 +0200 Subject: [PATCH 3/6] support newer Database ex --- .../api/internal/SemconvStability.java | 9 +- .../javaagent/build.gradle.kts | 26 ++- .../sql/VertxSqlClientAttributesGetter.java | 50 ++++-- .../vertx/v4_0/sql/VertxSqlClientTest.java | 159 ++++++++++++++++++ 4 files changed, 228 insertions(+), 16 deletions(-) create mode 100644 instrumentation/vertx/vertx-sql-client-4.0/javaagent/src/testVertx5/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/sql/VertxSqlClientTest.java diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SemconvStability.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SemconvStability.java index 6976bb268cba..7a4ce1783047 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SemconvStability.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SemconvStability.java @@ -18,8 +18,8 @@ */ public final class SemconvStability { - private static final boolean emitOldDatabaseSemconv; - private static final boolean emitStableDatabaseSemconv; + private static boolean emitOldDatabaseSemconv; + private static boolean emitStableDatabaseSemconv; static { boolean oldDatabase = true; @@ -78,4 +78,9 @@ public static String stableDbSystemName(String oldDbSystem) { } private SemconvStability() {} + + public static void setForTesting(boolean emitOldDatabaseSemconv, boolean emitStableDatabaseSemconv) { + SemconvStability.emitOldDatabaseSemconv = emitOldDatabaseSemconv; + SemconvStability.emitStableDatabaseSemconv = emitStableDatabaseSemconv; + } } diff --git a/instrumentation/vertx/vertx-sql-client-4.0/javaagent/build.gradle.kts b/instrumentation/vertx/vertx-sql-client-4.0/javaagent/build.gradle.kts index eff4457a3eec..380b1c7a186c 100644 --- a/instrumentation/vertx/vertx-sql-client-4.0/javaagent/build.gradle.kts +++ b/instrumentation/vertx/vertx-sql-client-4.0/javaagent/build.gradle.kts @@ -12,13 +12,31 @@ muzzle { } dependencies { - library("io.vertx:vertx-sql-client:4.0.0") - compileOnly("io.vertx:vertx-codegen:4.0.0") + val version = "4.0.0" + library("io.vertx:vertx-sql-client:$version") + compileOnly("io.vertx:vertx-codegen:$version") testInstrumentation(project(":instrumentation:netty:netty-4.1:javaagent")) - testLibrary("io.vertx:vertx-pg-client:4.0.0") - testLibrary("io.vertx:vertx-codegen:4.0.0") + testLibrary("io.vertx:vertx-pg-client:$version") + testLibrary("io.vertx:vertx-codegen:$version") +} + +testing { + suites { + val testVertx5 by registering(JvmTestSuite::class) { + val version = "4.5.13" + dependencies { + implementation(project()) + implementation("io.vertx:vertx-sql-client:$version") + implementation("io.vertx:vertx-codegen:$version") + implementation("io.vertx:vertx-pg-client:$version") + implementation("org.testcontainers:testcontainers") + + implementation(project(":instrumentation:netty:netty-4.1:javaagent")) + } + } + } } tasks { diff --git a/instrumentation/vertx/vertx-sql-client-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/sql/VertxSqlClientAttributesGetter.java b/instrumentation/vertx/vertx-sql-client-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/sql/VertxSqlClientAttributesGetter.java index 63a4e04cfd21..8064e874c783 100644 --- a/instrumentation/vertx/vertx-sql-client-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/sql/VertxSqlClientAttributesGetter.java +++ b/instrumentation/vertx/vertx-sql-client-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/sql/VertxSqlClientAttributesGetter.java @@ -9,13 +9,20 @@ import io.opentelemetry.instrumentation.api.incubator.semconv.db.SqlClientAttributesGetter; import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.Arrays; import java.util.Collection; +import java.util.List; +import java.util.function.Function; import javax.annotation.Nullable; public enum VertxSqlClientAttributesGetter implements SqlClientAttributesGetter { INSTANCE; + private static final List> responseStatusExtractors = + createResponseStatusExtractors(); + @Override public String getDbSystem(VertxSqlClientRequest request) { return null; @@ -49,18 +56,41 @@ public Collection getRawQueryTexts(VertxSqlClientRequest request) { @Nullable @Override public String getResponseStatus(@Nullable Void response, @Nullable Throwable error) { - try { - // loaded via reflection, because this class is not available in all versions that we support - Class ex = Class.forName("io.vertx.pgclient.PgException"); - if (ex.isInstance(error)) { - return (String) ex.getMethod("getCode").invoke(error); + for (Function extractor : responseStatusExtractors) { + String status = extractor.apply((Exception) error); + if (status != null) { + return status; } - } catch (ClassNotFoundException - | NoSuchMethodException - | IllegalAccessException - | InvocationTargetException e) { - return null; } return null; } + + private static List> createResponseStatusExtractors() { + return Arrays.asList( + responseStatusExtractor("io.vertx.sqlclient.DatabaseException", "getSqlState"), + // older version only have this method + responseStatusExtractor("io.vertx.pgclient.PgException", "getCode")); + } + + private static Function responseStatusExtractor( + String className, String methodName) { + try { + // loaded via reflection, because this class is not available in all versions that we support + Class exClass = Class.forName(className); + Method method = exClass.getDeclaredMethod(methodName); + + return (error) -> { + if (exClass.isInstance(error)) { + try { + return String.valueOf(method.invoke(error)); // can be String or int + } catch (IllegalAccessException | InvocationTargetException e) { + return null; + } + } + return null; + }; + } catch (ClassNotFoundException | NoSuchMethodException e) { + return (error) -> null; + } + } } diff --git a/instrumentation/vertx/vertx-sql-client-4.0/javaagent/src/testVertx5/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/sql/VertxSqlClientTest.java b/instrumentation/vertx/vertx-sql-client-4.0/javaagent/src/testVertx5/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/sql/VertxSqlClientTest.java new file mode 100644 index 000000000000..36e4ccde7bf2 --- /dev/null +++ b/instrumentation/vertx/vertx-sql-client-4.0/javaagent/src/testVertx5/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/sql/VertxSqlClientTest.java @@ -0,0 +1,159 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.vertx.v4_0.sql; + +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.satisfies; +import static io.opentelemetry.semconv.ErrorAttributes.ERROR_TYPE; +import static io.opentelemetry.semconv.ExceptionAttributes.EXCEPTION_MESSAGE; +import static io.opentelemetry.semconv.ExceptionAttributes.EXCEPTION_STACKTRACE; +import static io.opentelemetry.semconv.ExceptionAttributes.EXCEPTION_TYPE; +import static io.opentelemetry.semconv.ServerAttributes.SERVER_ADDRESS; +import static io.opentelemetry.semconv.ServerAttributes.SERVER_PORT; +import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_NAMESPACE; +import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_QUERY_TEXT; +import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_RESPONSE_STATUS_CODE; + +import io.opentelemetry.api.trace.SpanKind; +import io.opentelemetry.instrumentation.api.internal.SemconvStability; +import io.opentelemetry.instrumentation.testing.internal.AutoCleanupExtension; +import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import io.opentelemetry.sdk.trace.data.StatusData; +import io.vertx.core.Vertx; +import io.vertx.pgclient.PgConnectOptions; +import io.vertx.pgclient.PgException; +import io.vertx.sqlclient.Pool; +import io.vertx.sqlclient.PoolOptions; +import java.time.Duration; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.containers.output.Slf4jLogConsumer; + +class VertxSqlClientTest { + private static final Logger logger = LoggerFactory.getLogger(VertxSqlClientTest.class); + + private static final String USER_DB = "SA"; + private static final String PW_DB = "password123"; + private static final String DB = "tempdb"; + + @RegisterExtension + private static final InstrumentationExtension testing = AgentInstrumentationExtension.create(); + + @RegisterExtension + private static final AutoCleanupExtension cleanup = AutoCleanupExtension.create(); + + private static GenericContainer container; + private static Vertx vertx; + private static Pool pool; + private static String host; + private static int port; + + @BeforeAll + static void setUp() throws Exception { + SemconvStability.setForTesting(false, true); + container = + new GenericContainer<>("postgres:9.6.8") + .withEnv("POSTGRES_USER", USER_DB) + .withEnv("POSTGRES_PASSWORD", PW_DB) + .withEnv("POSTGRES_DB", DB) + .withExposedPorts(5432) + .withLogConsumer(new Slf4jLogConsumer(logger)) + .withStartupTimeout(Duration.ofMinutes(2)); + container.start(); + vertx = Vertx.vertx(); + host = container.getHost(); + port = container.getMappedPort(5432); + PgConnectOptions options = + new PgConnectOptions() + .setPort(port) + .setHost(host) + .setDatabase(DB) + .setUser(USER_DB) + .setPassword(PW_DB); + pool = Pool.pool(vertx, options, new PoolOptions().setMaxSize(4)); + pool.query("create table test(id int primary key, name varchar(255))") + .execute() + .compose( + r -> + // insert some test data + pool.query("insert into test values (1, 'Hello'), (2, 'World')").execute()) + .toCompletionStage() + .toCompletableFuture() + .get(30, TimeUnit.SECONDS); + } + + @AfterAll + static void cleanUp() { + pool.close(); + vertx.close(); + container.stop(); + } + + @Test + void testInvalidQuery() throws Exception { + CountDownLatch latch = new CountDownLatch(1); + CompletableFuture result = new CompletableFuture<>(); + result.whenComplete((rows, throwable) -> testing.runWithSpan("callback", latch::countDown)); + testing.runWithSpan( + "parent", + () -> + pool.query("invalid") + .execute( + rowSetAsyncResult -> { + if (rowSetAsyncResult.succeeded()) { + result.complete(rowSetAsyncResult.result()); + } else { + result.completeExceptionally(rowSetAsyncResult.cause()); + } + })); + + latch.await(30, TimeUnit.SECONDS); + + testing.waitAndAssertTraces( + trace -> + trace.hasSpansSatisfyingExactly( + span -> span.hasName("parent").hasKind(SpanKind.INTERNAL), + span -> + span.hasName("tempdb") + .hasKind(SpanKind.CLIENT) + .hasParent(trace.getSpan(0)) + .hasStatus(StatusData.error()) + .hasEventsSatisfyingExactly( + event -> + event + .hasName("exception") + .hasAttributesSatisfyingExactly( + equalTo(EXCEPTION_TYPE, PgException.class.getName()), + satisfies( + EXCEPTION_MESSAGE, + val -> val.contains("syntax error at or near")), + satisfies( + EXCEPTION_STACKTRACE, + val -> val.isInstanceOf(String.class)))) + .hasAttributesSatisfyingExactly( + equalTo(DB_NAMESPACE, DB), + equalTo(DB_QUERY_TEXT, "invalid"), + equalTo(SERVER_ADDRESS, host), + equalTo(SERVER_PORT, port), + equalTo(DB_RESPONSE_STATUS_CODE, "42601"), + // is the same as in the older versions of vertx, but extracted from + // io.vertx.sqlclient.DatabaseException + equalTo(ERROR_TYPE, "io.vertx.pgclient.PgException")), + span -> + span.hasName("callback") + .hasKind(SpanKind.INTERNAL) + .hasParent(trace.getSpan(0)))); + } +} From f89d54b55d5f9c629bed0cef2b97dc39d8c063d9 Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Wed, 2 Apr 2025 14:33:40 +0200 Subject: [PATCH 4/6] support newer Database ex --- .../api/incubator/semconv/db/DbResponseStatusUtil.java | 8 ++++++-- .../instrumentation/api/internal/SemconvStability.java | 3 ++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbResponseStatusUtil.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbResponseStatusUtil.java index f6313913ceef..7e11f911605d 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbResponseStatusUtil.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbResponseStatusUtil.java @@ -1,10 +1,14 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + package io.opentelemetry.instrumentation.api.incubator.semconv.db; import javax.annotation.Nullable; public class DbResponseStatusUtil { - private DbResponseStatusUtil() { - } + private DbResponseStatusUtil() {} @Nullable public static String dbResponseStatusCode(int responseStatusCode) { diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SemconvStability.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SemconvStability.java index 7a4ce1783047..d53285ec01c0 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SemconvStability.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SemconvStability.java @@ -79,7 +79,8 @@ public static String stableDbSystemName(String oldDbSystem) { private SemconvStability() {} - public static void setForTesting(boolean emitOldDatabaseSemconv, boolean emitStableDatabaseSemconv) { + public static void setForTesting( + boolean emitOldDatabaseSemconv, boolean emitStableDatabaseSemconv) { SemconvStability.emitOldDatabaseSemconv = emitOldDatabaseSemconv; SemconvStability.emitStableDatabaseSemconv = emitStableDatabaseSemconv; } From 507442b417d4260c03d952ecca62971dd106253b Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Wed, 2 Apr 2025 17:23:55 +0200 Subject: [PATCH 5/6] pr review --- .../db/{ => internal}/DbResponseStatusUtil.java | 9 +++++++-- .../instrumentation/api/internal/SemconvStability.java | 10 ++-------- .../v5_0/internal/ElasticsearchDbAttributesGetter.java | 2 +- .../rest/OpenSearchRestAttributesGetter.java | 2 +- .../vertx-sql-client-4.0/javaagent/build.gradle.kts | 10 +++++++++- .../vertx/v4_0/sql/VertxSqlClientTest.java | 2 -- 6 files changed, 20 insertions(+), 15 deletions(-) rename instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/{ => internal}/DbResponseStatusUtil.java (67%) rename instrumentation/vertx/vertx-sql-client-4.0/javaagent/src/{testVertx5 => testVertx4_5}/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/sql/VertxSqlClientTest.java (98%) diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbResponseStatusUtil.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/DbResponseStatusUtil.java similarity index 67% rename from instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbResponseStatusUtil.java rename to instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/DbResponseStatusUtil.java index 7e11f911605d..9678218817c9 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbResponseStatusUtil.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/DbResponseStatusUtil.java @@ -3,11 +3,16 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.instrumentation.api.incubator.semconv.db; +package io.opentelemetry.instrumentation.api.incubator.semconv.db.internal; import javax.annotation.Nullable; -public class DbResponseStatusUtil { +/** + * This class is internal and experimental. Its APIs are unstable and can change at any time. Its + * APIs (or a version of them) may be promoted to the public stable API in the future, but no + * guarantees are made. + */ +public final class DbResponseStatusUtil { private DbResponseStatusUtil() {} @Nullable diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SemconvStability.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SemconvStability.java index d53285ec01c0..6976bb268cba 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SemconvStability.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SemconvStability.java @@ -18,8 +18,8 @@ */ public final class SemconvStability { - private static boolean emitOldDatabaseSemconv; - private static boolean emitStableDatabaseSemconv; + private static final boolean emitOldDatabaseSemconv; + private static final boolean emitStableDatabaseSemconv; static { boolean oldDatabase = true; @@ -78,10 +78,4 @@ public static String stableDbSystemName(String oldDbSystem) { } private SemconvStability() {} - - public static void setForTesting( - boolean emitOldDatabaseSemconv, boolean emitStableDatabaseSemconv) { - SemconvStability.emitOldDatabaseSemconv = emitOldDatabaseSemconv; - SemconvStability.emitStableDatabaseSemconv = emitStableDatabaseSemconv; - } } diff --git a/instrumentation/elasticsearch/elasticsearch-rest-common-5.0/library/src/main/java/io/opentelemetry/instrumentation/elasticsearch/rest/common/v5_0/internal/ElasticsearchDbAttributesGetter.java b/instrumentation/elasticsearch/elasticsearch-rest-common-5.0/library/src/main/java/io/opentelemetry/instrumentation/elasticsearch/rest/common/v5_0/internal/ElasticsearchDbAttributesGetter.java index 1bf981028ec9..26c788f00f31 100644 --- a/instrumentation/elasticsearch/elasticsearch-rest-common-5.0/library/src/main/java/io/opentelemetry/instrumentation/elasticsearch/rest/common/v5_0/internal/ElasticsearchDbAttributesGetter.java +++ b/instrumentation/elasticsearch/elasticsearch-rest-common-5.0/library/src/main/java/io/opentelemetry/instrumentation/elasticsearch/rest/common/v5_0/internal/ElasticsearchDbAttributesGetter.java @@ -5,7 +5,7 @@ package io.opentelemetry.instrumentation.elasticsearch.rest.common.v5_0.internal; -import static io.opentelemetry.instrumentation.api.incubator.semconv.db.DbResponseStatusUtil.dbResponseStatusCode; +import static io.opentelemetry.instrumentation.api.incubator.semconv.db.internal.DbResponseStatusUtil.dbResponseStatusCode; import static java.util.logging.Level.FINE; import io.opentelemetry.instrumentation.api.incubator.semconv.db.DbClientAttributesGetter; diff --git a/instrumentation/opensearch/opensearch-rest-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opensearch/rest/OpenSearchRestAttributesGetter.java b/instrumentation/opensearch/opensearch-rest-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opensearch/rest/OpenSearchRestAttributesGetter.java index ff7ecd470f5f..172a7883fc72 100644 --- a/instrumentation/opensearch/opensearch-rest-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opensearch/rest/OpenSearchRestAttributesGetter.java +++ b/instrumentation/opensearch/opensearch-rest-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opensearch/rest/OpenSearchRestAttributesGetter.java @@ -5,7 +5,7 @@ package io.opentelemetry.javaagent.instrumentation.opensearch.rest; -import static io.opentelemetry.instrumentation.api.incubator.semconv.db.DbResponseStatusUtil.dbResponseStatusCode; +import static io.opentelemetry.instrumentation.api.incubator.semconv.db.internal.DbResponseStatusUtil.dbResponseStatusCode; import io.opentelemetry.instrumentation.api.incubator.semconv.db.DbClientAttributesGetter; import io.opentelemetry.semconv.incubating.DbIncubatingAttributes; diff --git a/instrumentation/vertx/vertx-sql-client-4.0/javaagent/build.gradle.kts b/instrumentation/vertx/vertx-sql-client-4.0/javaagent/build.gradle.kts index 380b1c7a186c..6565b7e587ff 100644 --- a/instrumentation/vertx/vertx-sql-client-4.0/javaagent/build.gradle.kts +++ b/instrumentation/vertx/vertx-sql-client-4.0/javaagent/build.gradle.kts @@ -24,7 +24,7 @@ dependencies { testing { suites { - val testVertx5 by registering(JvmTestSuite::class) { + val testVertx4_5 by registering(JvmTestSuite::class) { val version = "4.5.13" dependencies { implementation(project()) @@ -35,6 +35,14 @@ testing { implementation(project(":instrumentation:netty:netty-4.1:javaagent")) } + + targets { + all { + testTask.configure { + jvmArgs("-Dotel.semconv-stability.opt-in=database") + } + } + } } } } diff --git a/instrumentation/vertx/vertx-sql-client-4.0/javaagent/src/testVertx5/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/sql/VertxSqlClientTest.java b/instrumentation/vertx/vertx-sql-client-4.0/javaagent/src/testVertx4_5/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/sql/VertxSqlClientTest.java similarity index 98% rename from instrumentation/vertx/vertx-sql-client-4.0/javaagent/src/testVertx5/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/sql/VertxSqlClientTest.java rename to instrumentation/vertx/vertx-sql-client-4.0/javaagent/src/testVertx4_5/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/sql/VertxSqlClientTest.java index 36e4ccde7bf2..1f14ef420fe4 100644 --- a/instrumentation/vertx/vertx-sql-client-4.0/javaagent/src/testVertx5/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/sql/VertxSqlClientTest.java +++ b/instrumentation/vertx/vertx-sql-client-4.0/javaagent/src/testVertx4_5/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/sql/VertxSqlClientTest.java @@ -18,7 +18,6 @@ import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_RESPONSE_STATUS_CODE; import io.opentelemetry.api.trace.SpanKind; -import io.opentelemetry.instrumentation.api.internal.SemconvStability; import io.opentelemetry.instrumentation.testing.internal.AutoCleanupExtension; import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; @@ -62,7 +61,6 @@ class VertxSqlClientTest { @BeforeAll static void setUp() throws Exception { - SemconvStability.setForTesting(false, true); container = new GenericContainer<>("postgres:9.6.8") .withEnv("POSTGRES_USER", USER_DB) From 8ab9374bedb7807128dfc0fe3445f78d21afa96e Mon Sep 17 00:00:00 2001 From: Gregor Zeitlinger Date: Thu, 3 Apr 2025 10:41:40 +0200 Subject: [PATCH 6/6] test functionality is covered by latest dep tests --- .../javaagent/build.gradle.kts | 25 --- .../vertx/v4_0/sql/VertxSqlClientTest.java | 157 ------------------ 2 files changed, 182 deletions(-) delete mode 100644 instrumentation/vertx/vertx-sql-client-4.0/javaagent/src/testVertx4_5/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/sql/VertxSqlClientTest.java diff --git a/instrumentation/vertx/vertx-sql-client-4.0/javaagent/build.gradle.kts b/instrumentation/vertx/vertx-sql-client-4.0/javaagent/build.gradle.kts index 6565b7e587ff..4e2363c90876 100644 --- a/instrumentation/vertx/vertx-sql-client-4.0/javaagent/build.gradle.kts +++ b/instrumentation/vertx/vertx-sql-client-4.0/javaagent/build.gradle.kts @@ -22,31 +22,6 @@ dependencies { testLibrary("io.vertx:vertx-codegen:$version") } -testing { - suites { - val testVertx4_5 by registering(JvmTestSuite::class) { - val version = "4.5.13" - dependencies { - implementation(project()) - implementation("io.vertx:vertx-sql-client:$version") - implementation("io.vertx:vertx-codegen:$version") - implementation("io.vertx:vertx-pg-client:$version") - implementation("org.testcontainers:testcontainers") - - implementation(project(":instrumentation:netty:netty-4.1:javaagent")) - } - - targets { - all { - testTask.configure { - jvmArgs("-Dotel.semconv-stability.opt-in=database") - } - } - } - } - } -} - tasks { withType().configureEach { usesService(gradle.sharedServices.registrations["testcontainersBuildService"].service) diff --git a/instrumentation/vertx/vertx-sql-client-4.0/javaagent/src/testVertx4_5/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/sql/VertxSqlClientTest.java b/instrumentation/vertx/vertx-sql-client-4.0/javaagent/src/testVertx4_5/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/sql/VertxSqlClientTest.java deleted file mode 100644 index 1f14ef420fe4..000000000000 --- a/instrumentation/vertx/vertx-sql-client-4.0/javaagent/src/testVertx4_5/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/sql/VertxSqlClientTest.java +++ /dev/null @@ -1,157 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.vertx.v4_0.sql; - -import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; -import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.satisfies; -import static io.opentelemetry.semconv.ErrorAttributes.ERROR_TYPE; -import static io.opentelemetry.semconv.ExceptionAttributes.EXCEPTION_MESSAGE; -import static io.opentelemetry.semconv.ExceptionAttributes.EXCEPTION_STACKTRACE; -import static io.opentelemetry.semconv.ExceptionAttributes.EXCEPTION_TYPE; -import static io.opentelemetry.semconv.ServerAttributes.SERVER_ADDRESS; -import static io.opentelemetry.semconv.ServerAttributes.SERVER_PORT; -import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_NAMESPACE; -import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_QUERY_TEXT; -import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_RESPONSE_STATUS_CODE; - -import io.opentelemetry.api.trace.SpanKind; -import io.opentelemetry.instrumentation.testing.internal.AutoCleanupExtension; -import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension; -import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; -import io.opentelemetry.sdk.trace.data.StatusData; -import io.vertx.core.Vertx; -import io.vertx.pgclient.PgConnectOptions; -import io.vertx.pgclient.PgException; -import io.vertx.sqlclient.Pool; -import io.vertx.sqlclient.PoolOptions; -import java.time.Duration; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.RegisterExtension; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.testcontainers.containers.GenericContainer; -import org.testcontainers.containers.output.Slf4jLogConsumer; - -class VertxSqlClientTest { - private static final Logger logger = LoggerFactory.getLogger(VertxSqlClientTest.class); - - private static final String USER_DB = "SA"; - private static final String PW_DB = "password123"; - private static final String DB = "tempdb"; - - @RegisterExtension - private static final InstrumentationExtension testing = AgentInstrumentationExtension.create(); - - @RegisterExtension - private static final AutoCleanupExtension cleanup = AutoCleanupExtension.create(); - - private static GenericContainer container; - private static Vertx vertx; - private static Pool pool; - private static String host; - private static int port; - - @BeforeAll - static void setUp() throws Exception { - container = - new GenericContainer<>("postgres:9.6.8") - .withEnv("POSTGRES_USER", USER_DB) - .withEnv("POSTGRES_PASSWORD", PW_DB) - .withEnv("POSTGRES_DB", DB) - .withExposedPorts(5432) - .withLogConsumer(new Slf4jLogConsumer(logger)) - .withStartupTimeout(Duration.ofMinutes(2)); - container.start(); - vertx = Vertx.vertx(); - host = container.getHost(); - port = container.getMappedPort(5432); - PgConnectOptions options = - new PgConnectOptions() - .setPort(port) - .setHost(host) - .setDatabase(DB) - .setUser(USER_DB) - .setPassword(PW_DB); - pool = Pool.pool(vertx, options, new PoolOptions().setMaxSize(4)); - pool.query("create table test(id int primary key, name varchar(255))") - .execute() - .compose( - r -> - // insert some test data - pool.query("insert into test values (1, 'Hello'), (2, 'World')").execute()) - .toCompletionStage() - .toCompletableFuture() - .get(30, TimeUnit.SECONDS); - } - - @AfterAll - static void cleanUp() { - pool.close(); - vertx.close(); - container.stop(); - } - - @Test - void testInvalidQuery() throws Exception { - CountDownLatch latch = new CountDownLatch(1); - CompletableFuture result = new CompletableFuture<>(); - result.whenComplete((rows, throwable) -> testing.runWithSpan("callback", latch::countDown)); - testing.runWithSpan( - "parent", - () -> - pool.query("invalid") - .execute( - rowSetAsyncResult -> { - if (rowSetAsyncResult.succeeded()) { - result.complete(rowSetAsyncResult.result()); - } else { - result.completeExceptionally(rowSetAsyncResult.cause()); - } - })); - - latch.await(30, TimeUnit.SECONDS); - - testing.waitAndAssertTraces( - trace -> - trace.hasSpansSatisfyingExactly( - span -> span.hasName("parent").hasKind(SpanKind.INTERNAL), - span -> - span.hasName("tempdb") - .hasKind(SpanKind.CLIENT) - .hasParent(trace.getSpan(0)) - .hasStatus(StatusData.error()) - .hasEventsSatisfyingExactly( - event -> - event - .hasName("exception") - .hasAttributesSatisfyingExactly( - equalTo(EXCEPTION_TYPE, PgException.class.getName()), - satisfies( - EXCEPTION_MESSAGE, - val -> val.contains("syntax error at or near")), - satisfies( - EXCEPTION_STACKTRACE, - val -> val.isInstanceOf(String.class)))) - .hasAttributesSatisfyingExactly( - equalTo(DB_NAMESPACE, DB), - equalTo(DB_QUERY_TEXT, "invalid"), - equalTo(SERVER_ADDRESS, host), - equalTo(SERVER_PORT, port), - equalTo(DB_RESPONSE_STATUS_CODE, "42601"), - // is the same as in the older versions of vertx, but extracted from - // io.vertx.sqlclient.DatabaseException - equalTo(ERROR_TYPE, "io.vertx.pgclient.PgException")), - span -> - span.hasName("callback") - .hasKind(SpanKind.INTERNAL) - .hasParent(trace.getSpan(0)))); - } -}