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.

Sign up for GitHub

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

Jump to bottom

Fix shared metrics between requests #722

Merged
merged 3 commits into from
Apr 11, 2023
Merged

Fix shared metrics between requests #722

merged 3 commits into from
Apr 11, 2023
+32 −70

Conversation

LeoQuote
Copy link
Contributor

@LeoQuote LeoQuote commented Apr 10, 2023 •
edited
Loading

Breaking changes

dropped metrics

  • mysql_exporter_scrapes_total
  • mysql_exporter_scrape_errors_total
  • mysql_last_scrape_failed

new metrics

  • mysql_exporter_collector_success (replace mysql_exporter_scrape_errors_total)

metrics isolated between requests

  • mysql_up
  • mysql_exporter_collector_success
  • mysql_exporter_collector_duration_seconds

Why is this neccessary

We are using mysqld_exporter in multi-target method, which means one exporter for multiple mysql instance, the crucial metric mysql_up must be isolated between requests, or there will be misunderstandings, some mysql instance would shown as down while in fact it is not.

all shared metrics are dropped especially mysql_exporter_scrapes_total and mysql_exporter_scrape_errors_total , the metrics are useless since prometheus can record almost everything, including scrape fail for each collector.

mysql_last_scrape_failed is covered by mysql_exporter_collector_success so it is also dropped.

fix #713

@LeoQuote
delete shared metrics between requests
5632e89 
Signed-off-by: qizhicheng <qizhicheng@douban.com>
@LeoQuote LeoQuote force-pushed the delete_shared_metrics branch from 727881d to 5632e89 Compare April 10, 2023 11:22
SuperQ
SuperQ requested changes Apr 10, 2023
View reviewed changes
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, most of this makes sense. I think we should move all the Prometheus metric descriptors to package level vars to reduce the Go GC churn.

collector/exporter.go Show resolved Hide resolved
SuperQ
SuperQ reviewed Apr 10, 2023
View reviewed changes
collector/exporter.go Outdated Show resolved Hide resolved
@LeoQuote
use shared description
2d33eeb 
Signed-off-by: qizhicheng <qizhicheng@douban.com>
@LeoQuote LeoQuote requested a review from SuperQ April 11, 2023 03:57
SuperQ
SuperQ reviewed Apr 11, 2023
View reviewed changes
collector/exporter.go Outdated Show resolved Hide resolved
@LeoQuote
drop last scrape failed metric
f131f1e 
Signed-off-by: qizhicheng <qizhicheng@douban.com>
@LeoQuote LeoQuote requested a review from SuperQ April 11, 2023 05:59
SuperQ
SuperQ approved these changes Apr 11, 2023
View reviewed changes
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!

@SuperQ SuperQ changed the title delete shared metrics between requests Fix shared metrics between requests Apr 11, 2023
@SuperQ SuperQ merged commit 6ad9f25 into prometheus:main Apr 11, 2023
@SuperQ SuperQ mentioned this pull request Apr 11, 2023
Merged
@LeoQuote LeoQuote deleted the delete_shared_metrics branch April 12, 2023 01:22
@SuperQ SuperQ mentioned this pull request Apr 14, 2023
Merged
SuperQ added a commit that referenced this pull request Apr 14, 2023
@SuperQ
Release v0.15.0-rc.0
b67b861 
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`

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
* [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
* [BUGFIX] Fix infoSchemaInnodbMetricsEnabledColumnQuery #687

Signed-off-by: SuperQ <superq@gmail.com>
SuperQ added a commit that referenced this pull request Apr 14, 2023
@SuperQ
Release v0.15.0-rc.0
6e73fce 
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`

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
* [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
* [BUGFIX] Fix infoSchemaInnodbMetricsEnabledColumnQuery #687

Signed-off-by: SuperQ <superq@gmail.com>
SuperQ added a commit that referenced this pull request Jun 19, 2023
@SuperQ
Release 0.15.0
a726b4d 
* [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
Merged
SuperQ added a commit that referenced this pull request Jun 22, 2023
@SuperQ
Release 0.15.0
0eeb39b 
* [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
@SuperQ
Release 0.15.0
95b3d20 
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
@SuperQ
Release 0.15.0 (#744)
6ca2a42 
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
Reviewers

@SuperQ SuperQ SuperQ approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

hard to trounbleshooting scrape error when using multi target scrape
2 participants
@LeoQuote @SuperQ