Skip to content

Conversation

@mustansir14
Copy link
Contributor

@mustansir14 mustansir14 commented Dec 1, 2025

Description:

We need to unify JDBC URL parsing across the detector and analyzer. This PR supports that by publicizing the methods required by the analyzer, and also some changes to make the methods compatible for both detector and analyzer.

The changes here will make more sense when viewed in conjunction with the corresponding integrations PR.

Checklist:

  • Tests passing (make test-community)?
  • Lint passing (make lint this requires golangci-lint)?

@mustansir14 mustansir14 marked this pull request as ready for review December 3, 2025 15:45
@mustansir14 mustansir14 requested a review from a team December 3, 2025 15:45
@mustansir14 mustansir14 requested a review from a team as a code owner December 3, 2025 15:45
Copy link
Contributor

@amanfcp amanfcp left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks @mustansir14
Left a few suggestions

Comment on lines +86 to +90
postgresJDBC := &PostgresJDBC{
Host: u.Host,
Database: dbName,
Params: params,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We can use a single ConnectionInfo struct across all handlers, and later clean it up from jdbc analyzer models.go.

Comment on lines +137 to 140
if h, p, found := strings.Cut(host, ":"); found {
data["host"] = h
data["port"] = p
}
Copy link
Contributor

Choose a reason for hiding this comment

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

As per the coding guidelines

Suggested change
if h, p, found := strings.Cut(host, ":"); found {
data["host"] = h
data["port"] = p
}
if h, p, ok := strings.Cut(host, ":"); ok {
data["host"] = h
data["port"] = p
}

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants