Skip to content

Add JDBC URL parsing support for OceanBase, PolarDB and Lindorm #14790

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

Jump to bottom
Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Add JDBC URL parsing support for OceanBase, PolarDB and Lindorm #14790

wants to merge 6 commits into from

Conversation

pepeshore
Copy link

@pepeshore pepeshore commented Sep 25, 2025 •
edited by steverao
Loading

Resolved #14702

@pepeshore pepeshore requested a review from a team as a code owner September 25, 2025 08:01
@linux-foundation-easycla Linux Foundation: EasyCLA
Copy link

linux-foundation-easycla bot commented Sep 25, 2025 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@github-actions github-actions bot added the test native This label can be applied to PRs to trigger them to run native tests label Sep 25, 2025
@otelbot-java-instrumentation
Copy link
Contributor

🔧 The result from spotlessApply was committed to the PR branch.

@steverao steverao changed the title feat: OceanBase、PolarDB、Lindorm JDBC URL Parse Support （#14702） Add JDBC URL parsing support for OceanBase, PolarDB and Lindorm Sep 26, 2025
steverao
steverao reviewed Sep 26, 2025
View reviewed changes
...ry/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcConnectionUrlParser.java Outdated
return DbSystemValues.CLICKHOUSE;
case "oceanbase": // Oceanbase
return DbSystemValues.OCEANBASE;
case "polardb": // polarDB
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
case "polardb": // polarDB
case "polardb": // PolarDB

Copy link
Author

Choose a reason for hiding this comment

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

fixed

steverao
steverao reviewed Sep 26, 2025
View reviewed changes
...ry/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcConnectionUrlParser.java Outdated
return DbSystemValues.OCEANBASE;
case "polardb": // polarDB
return DbSystemValues.POLARDB;
case "lindorm": // lindorm
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
case "lindorm": // lindorm
case "lindorm": // Lindorm

Copy link
Author

Choose a reason for hiding this comment

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

fixed

steverao
steverao reviewed Sep 26, 2025
View reviewed changes
...ry/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcConnectionUrlParser.java
},
/**
* <a
* href="https://www.alibabacloud.com/help/en/lindorm/user-guide/view-endpoints?spm=a2c63.p38356.help-menu-172543.d_2_0_1.7a1e41feMntzyJ">Driver
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* href="https://www.alibabacloud.com/help/en/lindorm/user-guide/view-endpoints?spm=a2c63.p38356.help-menu-172543.d_2_0_1.7a1e41feMntzyJ">Driver
* href="https://www.alibabacloud.com/help/en/lindorm/user-guide/view-endpoints">Driver

Copy link
Author

Choose a reason for hiding this comment

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

deleted

Copy link
Contributor

Choose a reason for hiding this comment

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

deleted

did you forget to actually delete the url parameters?

steverao
steverao reviewed Sep 26, 2025
View reviewed changes
...rc/test/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcConnectionUrlParserTest.java Outdated

private static Stream<Arguments> oceanbaseArguments() {
return args(
// https://docs.aws.amazon.com/secretsmanager/latest/userguide/retrieving-secrets_jdbc.html
Copy link
Contributor

Choose a reason for hiding this comment

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

It it correct url?

Copy link
Author

Choose a reason for hiding this comment

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

fixed

...rc/test/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcConnectionUrlParserTest.java Outdated

private static Stream<Arguments> lindormArguments() {
return args(
// https://docs.aws.amazon.com/secretsmanager/latest/userguide/retrieving-secrets_jdbc.html
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question with above

Copy link
Author

Choose a reason for hiding this comment

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

fixed

@pepeshore
fix：fix some code comments
a4a325f 
Change-Id: I39d2ce579c60d5afa2e792f3296eef5d0b495443
@pepeshore
Merge remote-tracking branch 'origin/feature/additonal-jdbc-url-parse…
28c2324 
…r' into feature/additonal-jdbc-url-parser

Change-Id: I5b6ee06da60fc32dc1edece63948d228c95f710b
laurit
laurit reviewed Sep 29, 2025
View reviewed changes
...ry/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcConnectionUrlParser.java
String subtype = jdbcUrl.substring(typeEndLoc + 1, protoLoc);
builder.subtype(subtype);
if (subtype.equals(DbSystemValues.ORACLE)) {
builder.system(DbSystemValues.ORACLE);
Copy link
Contributor

Choose a reason for hiding this comment

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

is oracle the only supported subtype?

laurit
laurit reviewed Sep 29, 2025
View reviewed changes
...ry/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcConnectionUrlParser.java
Comment on lines +924 to +926
if (subtype.equals(DbSystemValues.ORACLE)) {
builder.system(DbSystemValues.ORACLE);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

initially I though that it is weird to have subtype and system set to the same value but DATADIRECT url handling does the same

laurit
laurit reviewed Sep 29, 2025
View reviewed changes
...ry/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcConnectionUrlParser.java
return GENERIC_URL_LIKE.doParse(clickhouseUrl, builder);
}
},
/** jdbc:oceanbase://host:port/dbname jdbc:oceanbase:oracle://host:port/dbname */
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps change the comment to

  /**
   * Sample urls:
   *
   * <ul>
   *   <li>jdbc:oceanbase://host:port/dbname
   *   <li>jdbc:oceanbase:oracle://host:port/dbname
   * </ul>
   */

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laurit laurit laurit left review comments

@steverao steverao steverao left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
test native This label can be applied to PRs to trigger them to run native tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Some JDBC Connection String Parse failed
3 participants
@pepeshore @laurit @steverao