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

Add UNIX domain socket support for multi-target scraping #707

Merged
merged 1 commit into from
Apr 14, 2023

Conversation

yosida95
Copy link
Contributor

@yosida95 yosida95 commented Feb 10, 2023

This pull request expands the multi-target scraping ability introduced by #504 and #651 to UNIX domain sockets.

If the target query starts with unix://, connect to the server using UNIX domain socket. Otherwise, use TCP.

This PR adds a new query parameter network to specify the network type either "tcp" or "unix". If no value is set, it defaults to "tcp".

Support for multiple UNIX domain sockets is useful if, for example, mysqld_exporter runs on Cloud Run configured with multiple Cloud SQL connections. Cloud Run mounts multiple UNIX domain sockets on the /cloudsql directory. https://cloud.google.com/sql/docs/mysql/connect-run?hl=en#connect_to

(Fixed in #714) By the way, I found that the /probe endpoint emits error messages to the client along with metrics in some error conditions. I don't know if Prometheus can handle it correctly, but at least prom2json couldn't handle it.

mysqld_exporter/probe.go

Lines 45 to 54 in 593b009

cfg := c.GetConfig()
cfgsection, ok := cfg.Sections[authModule]
if !ok {
level.Error(logger).Log("msg", fmt.Sprintf("Failed to parse section [%s] from config file", authModule), "err", err)
http.Error(w, fmt.Sprintf("Error parsing config section [%s]", authModule), http.StatusBadRequest)
}
if dsn, err = cfgsection.FormDSN(target); err != nil {
level.Error(logger).Log("msg", fmt.Sprintf("Failed to form dsn from section [%s]", authModule), "err", err)
http.Error(w, fmt.Sprintf("Error forming dsn from config section [%s]", authModule), http.StatusBadRequest)
}

In the same condition, the /metrics endpoint doesn't emit error messages.

cfg := c.GetConfig()
cfgsection, ok := cfg.Sections["client"]
if !ok {
level.Error(logger).Log("msg", "Failed to parse section [client] from config file", "err", err)
}
if dsn, err = cfgsection.FormDSN(""); err != nil {
level.Error(logger).Log("msg", "Failed to form dsn from section [client]", "err", err)
}

Is that OK or should be patched to emit only either error messages or metrics? I'd like to send a PR as needed.

@SuperQ
Copy link
Member

SuperQ commented Mar 5, 2023

Looks like there's a merge conflict.

@yosida95 yosida95 force-pushed the unix-domain-socket branch from 5d4f5f6 to bf70e5b Compare March 7, 2023 16:47
@yosida95 yosida95 closed this Mar 7, 2023
@yosida95 yosida95 force-pushed the unix-domain-socket branch from bf70e5b to 6f45888 Compare March 7, 2023 16:51
@yosida95
Copy link
Contributor Author

yosida95 commented Mar 7, 2023

@SuperQ Resolved

@soara
Copy link
Contributor

soara commented Mar 8, 2023

According to Go MySQL Driver documentation, UNIX domain socket is the absolute path.
So, I thought that target value is tcp or unix can be determined by whether it is the absolute path or not.

Copy link
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

Thanks

probe.go Outdated Show resolved Hide resolved
@yosida95
Copy link
Contributor Author

@soara Thanks for the suggestion.

Despite your suggestion, it seems the UNIX domain socket path is not necessary to be absolute. The driver uses the standard net.Dialer, which resolves relative paths based on the cwd. https://github.com/go-sql-driver/mysql/blob/v1.7.0/connector.go#L48-L49

Therefore checking whether the target value contains a slash (or starts with a slash) does not always distinguish the network.

Copy link
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

Please make this handle the network with unix:// prefixing.

@yosida95 yosida95 force-pushed the unix-domain-socket branch from 6f45888 to c1b23d1 Compare April 14, 2023 05:53
Signed-off-by: Kohei YOSHIDA <kohei@yosida95.com>
@yosida95 yosida95 force-pushed the unix-domain-socket branch from c1b23d1 to 0c4f492 Compare April 14, 2023 05:56
@yosida95
Copy link
Contributor Author

@SuperQ Done.

Copy link
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

Nice, super easy!

@SuperQ SuperQ merged commit 34817b7 into prometheus:main Apr 14, 2023
SuperQ added a commit that referenced this pull request Jun 19, 2023
* [CHANGE] Allow `tlsCfg.InsecureSkipVerify` outside of mTLS #631
* [CHANGE] Update to exporter-toolkit v0.8.1 #677
* [CHANGE] Fix shared metrics between requests #722
* [FEATURE] Add support for collecting metrics from `sys.user_summary` #628
* [FEATURE] Support for multi-target mysqld probes #651
* [FEATURE] Add MySQL TLS configurations #718
* [ENHANCEMENT] Add UNIX domain socket support for multi-target scraping #707
* [BUGFIX] Fix `infoSchemaInnodbMetricsEnabledColumnQuery` #687
* [BUGFIX] Allow empty passwords #742

Signed-off-by: SuperQ <superq@gmail.com>
@SuperQ SuperQ mentioned this pull request Jun 19, 2023
SuperQ added a commit that referenced this pull request Jun 22, 2023
* [CHANGE] Allow `tlsCfg.InsecureSkipVerify` outside of mTLS #631
* [CHANGE] Update to exporter-toolkit v0.8.1 #677
* [CHANGE] Fix shared metrics between requests #722
* [FEATURE] Add support for collecting metrics from `sys.user_summary` #628
* [FEATURE] Support for multi-target mysqld probes #651
* [FEATURE] Add MySQL TLS configurations #718
* [ENHANCEMENT] Add UNIX domain socket support for multi-target scraping #707
* [BUGFIX] Fix `infoSchemaInnodbMetricsEnabledColumnQuery` #687
* [BUGFIX] Allow empty passwords #742

Signed-off-by: SuperQ <superq@gmail.com>
SuperQ added a commit that referenced this pull request Jun 22, 2023
BREAKING CHANGES:

The exporter no longer supports the monolithic `DATA_SOURCE_NAME` environment variable.
To configure connections to MySQL you can either use a `my.cnf` style config file or command line arguments.

For example:

    export MYSQLD_EXPORTER_PASSWORD=secret
    mysqld_exporter --mysqld.address=localhost:3306 --mysqld.username=exporter

We have also dropped some internal scrape metrics:
- `mysql_exporter_scrapes_total`
- `mysql_exporter_scrape_errors_total`
- `mysql_last_scrape_failed`

The default client configuration file is now `.my.cnf` in the process working directory. Use `--config.my-cnf="$HOME/.my.cnf"` to retain the previous default.

Changes:

* [CHANGE] Allow `tlsCfg.InsecureSkipVerify` outside of mTLS #631
* [CHANGE] Update to exporter-toolkit v0.8.1 #677
* [CHANGE] Fix shared metrics between requests #722
* [CHANGE] Allow empty passwords #742
* [CHANGE] Don't use HOME env in the my-cnf config path. #745
* [FEATURE] Add support for collecting metrics from `sys.user_summary` #628
* [FEATURE] Support for multi-target mysqld probes #651
* [FEATURE] Add MySQL TLS configurations #718
* [FEATURE] Add config reload via /-/reload #734
* [ENHANCEMENT] Add UNIX domain socket support for multi-target scraping #707
* [ENHANCEMENT] Use `STRAIGHT_JOIN` in infoSchemaAutoIncrementQuery #726
* [BUGFIX] Fix `infoSchemaInnodbMetricsEnabledColumnQuery` #687
* [BUGFIX] Allow empty passwords #742

Signed-off-by: SuperQ <superq@gmail.com>
SuperQ added a commit that referenced this pull request Jun 24, 2023
BREAKING CHANGES:

The exporter no longer supports the monolithic `DATA_SOURCE_NAME` environment variable.
To configure connections to MySQL you can either use a `my.cnf` style config file or command line arguments.

For example:

    export MYSQLD_EXPORTER_PASSWORD=secret
    mysqld_exporter --mysqld.address=localhost:3306 --mysqld.username=exporter

We have also dropped some internal scrape metrics:
- `mysql_exporter_scrapes_total`
- `mysql_exporter_scrape_errors_total`
- `mysql_last_scrape_failed`

The default client configuration file is now `.my.cnf` in the process working directory. Use `--config.my-cnf="$HOME/.my.cnf"` to retain the previous default.

Changes:

* [CHANGE] Allow `tlsCfg.InsecureSkipVerify` outside of mTLS #631
* [CHANGE] Update to exporter-toolkit v0.8.1 #677
* [CHANGE] Fix shared metrics between requests #722
* [CHANGE] Allow empty passwords #742
* [CHANGE] Don't use HOME env in the my-cnf config path. #745
* [FEATURE] Add support for collecting metrics from `sys.user_summary` #628
* [FEATURE] Support for multi-target mysqld probes #651
* [FEATURE] Add MySQL TLS configurations #718
* [FEATURE] Add config reload via /-/reload #734
* [ENHANCEMENT] Add UNIX domain socket support for multi-target scraping #707
* [ENHANCEMENT] Use `STRAIGHT_JOIN` in infoSchemaAutoIncrementQuery #726
* [BUGFIX] Fix `infoSchemaInnodbMetricsEnabledColumnQuery` #687
* [BUGFIX] Allow empty passwords #742

Signed-off-by: SuperQ <superq@gmail.com>
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