refactor: require cgo for oracle module #42044
Conversation
godror requires cgo so only add oracle module if cgo is enabled update method call to use dsn package to avoid cgo dependency move oracle integration test to separate file for consistency with other oracle tests
|
This pull request doesn't have a |
|
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
|
|
|
@elastic/obs-infraobs-integrations friendly ping for review 🙂 |
|
But, while working on Oracle last year, I came across this package: https://github.com/sijms/go-ora which is pure-go client. Didn't get a chance to replace godror with this; but now that they are sponsored by JB and looking at the users of the package, I think it is that stage that we can use it. So, even with CGO_ENABLED=0; we will be able to use oracle module. |
|
|
||
| return &base, nil | ||
| } | ||
|
|
||
| func NewConnection(connString string) (*sql.DB, error) { | ||
| db, err := sql.Open("godror", connString) |
There was a problem hiding this comment.
This still relies on godor ?
There was a problem hiding this comment.
Yes. If you check x-pack/metricbeat/module/oracle/oracle.go, you will notice build tags at the top. The Oracle module is now only included when CGO is enabled; otherwise, it is excluded.
However, I am also thinking of SQL module. When CGO_ENABLED=0, the SQL module in x-pack/metricbeat is still available, and from there, queries to an Oracle database can be sent.
There was a problem hiding this comment.
Having the tag in oracle.go ensures that module registeration doesn't happen. But if we don't isolate this function, this file will still compile for CGO disabled ?
However, I am also thinking of SQL module
That part is handled in this PR, right ?
Proposed commit message
godror requires cgo so only add oracle module if cgo is enabled
update method call to use dsn package to avoid cgo dependency
move oracle integration test to separate file for consistency with other oracle tests
this also allows building x-pack/metricbeat with CGO_ENABLED=0 (without oracledb support)
Checklist
CHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.Disruptive User Impact
Author's Checklist
How to test this PR locally
Related issues
Use cases
Screenshots
Logs