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 prepared_stmt_count metrics to mysql integration #11155

Merged
merged 6 commits into from
Jan 20, 2022

Conversation

keisku
Copy link
Contributor

@keisku keisku commented Jan 18, 2022

What does this PR do?

Add following metrics:

  • mysql.performance.prepared_stmt_count
  • mysql.performance.max_prepared_stmt_count

Motivation

mysql.performance.prepared_stmt_count is to get the current number of prepared statements.
https://dev.mysql.com/doc/refman/8.0/en/server-status-variables.html#statvar_Prepared_stmt_count

mysql.performance.prepared_stmt_count is to get the total number of prepared statements in the server.
https://dev.mysql.com/doc/refman/8.0/en/server-system-variables.html#sysvar_max_prepared_stmt_count

Additional Notes

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached

@ghost ghost added the integration/mysql label Jan 18, 2022
@keisku keisku changed the title Add prepared stmt count metrics to mysql integration Add prepared_stmt_count metrics to mysql integration Jan 18, 2022
@keisku keisku marked this pull request as ready for review January 18, 2022 04:13
@keisku keisku requested review from a team as code owners January 18, 2022 04:13
Copy link
Contributor

@justiniso justiniso left a comment

Choose a reason for hiding this comment

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

Hey @kei6u thanks for the PR!

Overall, this looks good to me but I left a few suggested changes. I believe the metrics should be GAUGEs because they reflect the current state at the time of capture according to the documentation, but let me know if there was a reason you went with COUNTs. Thanks!

mysql/metadata.csv Outdated Show resolved Hide resolved
mysql/metadata.csv Outdated Show resolved Hide resolved
mysql/datadog_checks/mysql/const.py Outdated Show resolved Hide resolved
mysql/datadog_checks/mysql/const.py Outdated Show resolved Hide resolved
@keisku
Copy link
Contributor Author

keisku commented Jan 19, 2022

@justiniso
I appreciate your review!

let me know if there was a reason you went with COUNTs.

I set it as COUNT by getting from the name of metric(prepared stmt "count") since I actually was not sure how to decide a metric type. Now, I understand why I should've set it as GAUGEs.

@keisku keisku requested a review from justiniso January 19, 2022 23:48
@justiniso justiniso force-pushed the add_mysql_prepared_stmt_count branch from 41f7677 to 42bf02b Compare January 20, 2022 17:11
@justiniso justiniso merged commit ce3b6dd into master Jan 20, 2022
@justiniso justiniso deleted the add_mysql_prepared_stmt_count branch January 20, 2022 18:28
@justiniso
Copy link
Contributor

Thanks @kei6u, I went ahead and merged this. You can expect it to go out in agent version 7.35.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants