Skip to content

Conversation

@giautm
Copy link
Contributor

@giautm giautm commented Oct 27, 2025

  • This refactor should has no changes in the behaviour, because both otelsql.OpenDB() and otelsql.WrapDriver() has the same effect. This also avoids a copy of traceOpts passing around the connector.
  • This using the mysql.NewConnector() to open the connection: It has two points: No need to call FormatDSN() and ask the MySQL driver parse it again to the config object. It also allow the PR mysql/awsmysql: pass TLS directly to the config #3627 pass the TLS config to the underlying driver.

This allow us to use `mysql.NewConnector()` directly
@giautm giautm changed the title g/awsmysql otelsql mysql/awsmysql: migrate to use otelsql.OpenDB() for wrap the connector Oct 27, 2025
@giautm
Copy link
Contributor Author

giautm commented Oct 27, 2025

cc @vangent to review both PRs. Thank you,

@vangent
Copy link
Contributor

vangent commented Oct 27, 2025

It would be helpful if you could update the PR description to explain what you are trying to accomplish. Is this a refactoring with no change in behavior? Are you fixing a bug? ...?

@giautm
Copy link
Contributor Author

giautm commented Oct 28, 2025

It would be helpful if you could update the PR description to explain what you are trying to accomplish. Is this a refactoring with no change in behavior? Are you fixing a bug? ...?

Updated the description.

@giautm
Copy link
Contributor Author

giautm commented Oct 28, 2025

Let me add a test with this to the PR: https://github.com/XSAM/otelsql/blob/main/example/stdout/main.go

Added simple Otel test.

@giautm giautm force-pushed the g/awsmysql-otelsql branch from 5d09ce9 to 2cb2685 Compare October 28, 2025 05:54
@giautm giautm force-pushed the g/awsmysql-otelsql branch from 2cb2685 to 2a92009 Compare October 28, 2025 05:56
@vangent vangent merged commit bf2b874 into google:master Oct 28, 2025
4 checks passed
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.

2 participants