Skip to content
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.

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

Fix handling of DSN extra arguments #485

Closed
wants to merge 1 commit into from

Conversation

nabadger
Copy link

I believe this addresses the following issues

#458 #303 #376

DATA_SOURCE_NAME="my-user:my-password-with&?-chars@(127.0.0.1:3306)/ ./mysqld_exporter

This fix will just look for ? by checking only the chars that appear after the final / .

If there is no / in the DSN, then it's already being treated as an invalid DSN

invalid DSN: missing the slash separating the database name

Signed-off-by: Nick <nbadger@mintel.com>
@nabadger nabadger force-pushed the fix-dsn-args-handling branch from 89b7693 to 8087d90 Compare June 27, 2020 15:19
@nabadger nabadger changed the title Fix handling of DSN extra arguments WIP: Fix handling of DSN extra arguments Jun 29, 2020
@nabadger nabadger changed the title WIP: Fix handling of DSN extra arguments Fix handling of DSN extra arguments Jun 29, 2020
@SuperQ
Copy link
Member

SuperQ commented Jun 29, 2020

I've been thinking about completely dropping the DSN env var parsing. It's difficult to get right.

I would suggest looking at #306, I think we would be better off eliminating this feature entirely.

@nabadger
Copy link
Author

Makes sense - we are actually writing the .my.cnf file, it's just that the exporter takes the contents and parses them out into a DSN.

@SuperQ
Copy link
Member

SuperQ commented Jun 29, 2020

The whole DSN thing in the Go mysql driver is not fun.

@SuperQ
Copy link
Member

SuperQ commented Apr 14, 2023

We've reworked the exporter to use mysql.Config() to configure the DSN. The old DATA_SOURCE_NAME env var has been removed.

@SuperQ SuperQ closed this Apr 14, 2023
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