[extension/dbstorage] Replace mattn/go-sqlite with non-CGO modernc.org/sqlite #37882
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fiery-Fenix
requested review from
dmitryax,
atoulme and
a team
as code owners
github-actions
bot
requested review from
djaglowski,
swiatekm and
VihasMakwana
atoulme
reviewed
Feb 12, 2025
|
|
||
| Implementors can add additional driver support by importing SQL drivers into the program. | ||
| See [Golang database/sql package documentation](https://pkg.go.dev/database/sql) for more information. | ||
|
|
||
| `datasource`: the url of the database, in the format accepted by the driver. | ||
|
|
||
| >**NOTE:** If you are using legacy driver `sqlite3` and have additional driver parameters in `datasource` - please follow [migration guide](#migration-guide-from-sqlite3-to-sqlite-driver-options) to update your configuration. | ||
| `db_storage` component has compatibility convertor in place, but it's better to review and update used driver parameters. |
There was a problem hiding this comment.
should the changelog indicate the change as breaking?
There was a problem hiding this comment.
I'm still not confident that this change is breaking, because compatibility layer allows graceful migration from one driver to another. But if breaking change label will bring more user attention to this change - it sounds reasonable to make it breaking
There was a problem hiding this comment.
Updated changelog type to breaking
atoulme
reviewed
Feb 12, 2025
atoulme
reviewed
Feb 12, 2025
|
|
atoulme
approved these changes
Mar 6, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Main purpose of this PR - to make 1 component less with CGO dependency according to Coding Guidelines
Despite of the fact that
dbstoragecomponent is in Alpha state - was added compatibility layer to improve user experience and simplify migration pathTaking in account Alpha stability of component and written compatibility layer - I would expect that this change is not a breaking change
Respective warnings about outdated driver and it's parameters will be added to log during component start
Link to tracking issue
Fixes #35280
Testing
Patched OpenTelemetry Collector with modernc.org/sqlite driver was battle-tested in our environment for several month's in both Persistent Queue Storage and Tracking Storage appliances without no issues
Documentation
README for component was heavily updated to reflect:
sqlitedriver parameterssqlitedriver recommended parameterspgxdriver parameterssqlite3tosqlitemigration guide