Skip to content

ClickHouse JDBC URL support #13884

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

Merged
merged 4 commits into from
May 21, 2025
Merged

ClickHouse JDBC URL support #13884

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
112fb6a
ClickHouse JDBC URL support
john9x May 20, 2025
31a976a
./gradlew spotlessApply
otelbot[bot] May 20, 2025
54c1580
comment about `ch` prefix
john9x May 21, 2025
d1c56f2
./gradlew spotlessApply
otelbot[bot] May 21, 2025
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
21 changes: 21 additions & 0 deletions ...src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcConnectionUrlParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,24 @@ DbInfo.Builder doParse(String jdbcUrl, DbInfo.Builder builder) {

return builder;
}
},
/**
* <a
* href="https://clickhouse.com/docs/integrations/language-clients/java/jdbc#configuration">Driver
* configuration doc</a> mentions that besides <code>clickhouse</code> <code>ch</code> could also
* be used but ClickHouse Connection implementation always returns full prefix <code>
* jdbc:clickhouse:</code>
*/
CLICKHOUSE("clickhouse") {
@Override
DbInfo.Builder doParse(String jdbcUrl, DbInfo.Builder builder) {
String clickhouseUrl = jdbcUrl.substring("clickhouse:".length());
int typeEndIndex = clickhouseUrl.indexOf("://");
if (typeEndIndex > 0) {
builder.subtype(clickhouseUrl.substring(0, typeEndIndex));
}
return GENERIC_URL_LIKE.doParse(clickhouseUrl, builder);
}
};

private static final Logger logger = Logger.getLogger(JdbcConnectionUrlParser.class.getName());
Expand Down Expand Up @@ -1079,6 +1097,8 @@ private static String toDbSystem(String type) {
return DbSystemValues.MSSQL;
case "sap": // SAP Hana
return DbSystemValues.HANADB;
case "clickhouse": // ClickHouse
return DbSystemValues.CLICKHOUSE;
default:
return DbSystemValues.OTHER_SQL; // Unknown DBMS
}
Expand All @@ -1098,6 +1118,7 @@ private static final class DbSystemValues {
static final String DERBY = "derby";
static final String MARIADB = "mariadb";
static final String H2 = "h2";
static final String CLICKHOUSE = "clickhouse";

private DbSystemValues() {}
}
Expand Down
29 changes: 29 additions & 0 deletions ...test/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcConnectionUrlParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,35 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
}
}

@ParameterizedTest(name = "{index}: {0}")
@ArgumentsSource(ClickHouseProvider.class)
void testClickHouseParsing(ParseTestArgument argument) {
testVerifySystemSubtypeParsingOfUrl(argument);
}

static final class ClickHouseProvider implements ArgumentsProvider {
@Override
public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
return args(
// https://clickhouse.com/docs/integrations/language-clients/java/jdbc#configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

https://clickhouse.com/docs/integrations/language-clients/java/jdbc#configuration mentions that besides clickhouse ch could also be used. Should jdbc:ch://localhost:8123 also be handled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the driver code ch used as alias for clickhouse but Connection metadata always return full prefix jdbc:clickhouse:
https://github.com/ClickHouse/clickhouse-java/blob/main/clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseConnectionImpl.java#L1141

Copy link
Contributor

Choose a reason for hiding this comment

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

could you add a comment about this to the parsing logic to make clear that ch doesn't need to be handled

arg("jdbc:clickhouse:http://localhost:8123/mydb")
.setShortUrl("clickhouse:http://localhost:8123")
.setSystem("clickhouse")
.setSubtype("http")
.setHost("localhost")
.setPort(8123)
.setDb("mydb")
.build(),
arg("jdbc:clickhouse:https://localhost:8443?ssl=true")
.setShortUrl("clickhouse:https://localhost:8443")
.setSystem("clickhouse")
.setSubtype("https")
.setHost("localhost")
.setPort(8443)
.build());
}
}

@ParameterizedTest(name = "{index}: {0}")
@ArgumentsSource(PostgresProvider.class)
void testPostgresParsing(ParseTestArgument argument) {
Expand Down
Loading