review

Author: trask

instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/parser/PostgresqlUrlParser.java

@@ -8,6 +8,7 @@
 import static io.opentelemetry.instrumentation.jdbc.internal.parser.UrlParsingUtils.splitQuery;
 
 import java.util.Map;
+import java.util.Properties;
 
 /**
  * Parses PostgreSQL JDBC connection strings.
@@ -51,8 +52,10 @@ public void parse(String jdbcUrl, ParseContext ctx) {
     // Delegate to generic parser for standard URL parsing
     GenericUrlParser.INSTANCE.parse(jdbcUrl, ctx);
 
-    // Extract schema from currentSchema URL parameter for namespace formatting
     String schema = extractCurrentSchema(jdbcUrl);
+    if (schema == null) {
+      schema = extractCurrentSchema(ctx.props());
+    }
     if (schema == null && ctx.user() != null) {
       // Fall back to user as schema if no currentSchema param
       schema = ctx.user();
@@ -73,4 +76,12 @@ private static String extractCurrentSchema(String jdbcUrl) {
     Map<String, String> params = splitQuery(jdbcUrl.substring(queryIndex + 1), "&");
     return params.get("currentschema");
   }
+
+  private static String extractCurrentSchema(Properties props) {
+    if (props == null) {
+      return null;
+    }
+    String currentSchema = props.getProperty("currentSchema");
+    return currentSchema == null || currentSchema.isEmpty() ? null : currentSchema;
+  }
 }

instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcConnectionUrlParserTest.java

@@ -44,6 +44,13 @@ private static Properties stdProps() {
     return prop;
   }
 
+  private static Properties postgresProps(String user, String currentSchema) {
+    Properties prop = new Properties();
+    prop.setProperty("user", user);
+    prop.setProperty("currentSchema", currentSchema);
+    return prop;
+  }
+
   @ParameterizedTest
   @ValueSource(strings = {"", "jdbc:", "jdbc::", "bogus:string"})
   void testInvalidUrlReturnsDefault(String url) {
@@ -273,6 +280,28 @@ private static Stream<Arguments> postgresArguments() {
             .setNamespace("pgdb|myschema")
             .setName("pgdb")
             .build(),
+        // currentSchema from connection properties is used when the URL does not specify it
+        arg("jdbc:postgresql://pg.host/pgdb")
+            .setProperties(postgresProps("pguser", "propertyschema"))
+            .setShortUrl("postgresql://pg.host:5432")
+            .setSystem("postgresql")
+            .setUser("pguser")
+            .setHost("pg.host")
+            .setPort(5432)
+            .setNamespace("pgdb|propertyschema")
+            .setName("pgdb")
+            .build(),
+        // currentSchema URL param takes precedence over currentSchema property
+        arg("jdbc:postgresql://pg.host/pgdb?currentSchema=urlschema")
+            .setProperties(postgresProps("pguser", "propertyschema"))
+            .setShortUrl("postgresql://pg.host:5432")
+            .setSystem("postgresql")
+            .setUser("pguser")
+            .setHost("pg.host")
+            .setPort(5432)
+            .setNamespace("pgdb|urlschema")
+            .setName("pgdb")
+            .build(),
         // database only, no schema or user — namespace falls back to database name
         arg("jdbc:postgresql://pg.host/pgdb")
             .setShortUrl("postgresql://pg.host:5432")

instrumentation/kafka/kafka-connect-2.6/testing/src/test/java/io/opentelemetry/instrumentation/kafkaconnect/v2_6/PostgresKafkaConnectSinkTaskTest.java

@@ -67,15 +67,6 @@ class PostgresKafkaConnectSinkTaskTest extends KafkaConnectSinkTaskBaseTest {
   private static final String DB_USERNAME = "postgres";
   private static final String DB_PASSWORD = "password";
   private static final String DB_TABLE_PERSON = "person";
-  private static final String DB_SELECT_SPAN_NAME = "SELECT " + DATABASE_NAME;
-  private static final String DB_INSERT_SPAN_NAME =
-      emitStableDatabaseSemconv()
-          ? "INSERT \"" + DB_TABLE_PERSON + "\""
-          : "INSERT " + DATABASE_NAME + "." + DB_TABLE_PERSON;
-  private static final String DB_BATCH_INSERT_SPAN_NAME =
-      emitStableDatabaseSemconv()
-          ? "BATCH INSERT \"" + DB_TABLE_PERSON + "\""
-          : "INSERT " + DATABASE_NAME + "." + DB_TABLE_PERSON;
   private static final String CONNECTOR_NAME = "test-postgres-connector";
   private static final String TOPIC_NAME = "test-postgres-topic";
 
@@ -175,7 +166,7 @@ void testSingleMessage() throws Exception {
                 if (emitStableDatabaseSemconv()) {
                   span.satisfies(spanData -> assertThat(spanData.getName()).startsWith("SELECT"));
                 } else {
-                  span.hasName(DB_SELECT_SPAN_NAME);
+                  span.hasName("SELECT " + DATABASE_NAME);
                 }
                 span.hasKind(SpanKind.CLIENT).hasParent(trace.getSpan(0));
               };
@@ -199,7 +190,10 @@ void testSingleMessage() throws Exception {
               selectAssertion,
               selectAssertion,
               span ->
-                  span.hasName(DB_INSERT_SPAN_NAME)
+                  span.hasName(
+                          emitStableDatabaseSemconv()
+                              ? "INSERT \"" + DB_TABLE_PERSON + "\""
+                              : "INSERT " + DATABASE_NAME + "." + DB_TABLE_PERSON)
                       .hasKind(SpanKind.CLIENT)
                       .hasParent(trace.getSpan(0)));
         },
@@ -306,7 +300,7 @@ void testMultiTopic() throws Exception {
                 if (emitStableDatabaseSemconv()) {
                   span.satisfies(spanData -> assertThat(spanData.getName()).startsWith("SELECT"));
                 } else {
-                  span.hasName(DB_SELECT_SPAN_NAME);
+                  span.hasName("SELECT " + DATABASE_NAME);
                 }
                 span.hasKind(SpanKind.CLIENT).hasParent(trace.getSpan(0));
               };
@@ -332,7 +326,10 @@ void testMultiTopic() throws Exception {
               selectAssertion,
               selectAssertion,
               span ->
-                  span.hasName(DB_BATCH_INSERT_SPAN_NAME)
+                  span.hasName(
+                          emitStableDatabaseSemconv()
+                              ? "BATCH INSERT \"" + DB_TABLE_PERSON + "\""
+                              : "INSERT " + DATABASE_NAME + "." + DB_TABLE_PERSON)
                       .hasKind(SpanKind.CLIENT)
                       .hasParent(trace.getSpan(0)));
         },