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

Stop query activity collection due to misconfiguration #12343

Merged
merged 7 commits into from
Jun 8, 2022

Conversation

alexbarksdale
Copy link
Member

@alexbarksdale alexbarksdale commented Jun 7, 2022

What does this PR do?

Stops the collection of query activity if the database instance is not configured properly.

Motivation

If a specific config option isn’t enabled at the database level (performance-schema-consumer-events-waits-current), our collection query for activity does not error out, but rather collects data that is wrong or misleading.

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 Jun 7, 2022
@codecov
Copy link

codecov bot commented Jun 7, 2022

Codecov Report

Merging #12343 (4e231fa) into master (d8a8f09) will increase coverage by 0.05%.
The diff coverage is 87.50%.

Flag Coverage Δ
mysql 89.08% <87.50%> (+1.56%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@alexbarksdale alexbarksdale added changelog/Fixed category/bugfix For use during Agent Release period labels Jun 8, 2022
@alexbarksdale alexbarksdale changed the title Surface query activity misconfiguration Stop query activity collection and surface misconfiguration Jun 8, 2022
@alexbarksdale alexbarksdale changed the title Stop query activity collection and surface misconfiguration Stop query activity collection due to misconfiguration Jun 8, 2022
@alexbarksdale alexbarksdale marked this pull request as ready for review June 8, 2022 19:49
@alexbarksdale alexbarksdale requested review from a team as code owners June 8, 2022 19:49
mysql/datadog_checks/mysql/activity.py Outdated Show resolved Hide resolved
Comment on lines +161 to +168
cursor.execute(
"""\
SELECT
NAME,
ENABLED
FROM performance_schema.setup_consumers WHERE NAME = 'events_waits_current'
"""
)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a nit that is out of scope for this PR because the pattern already exists, but for the future I think it would be better to make this a boolean function and change the query to:

 cursor.execute(
                    """\
                    SELECT count(*)
                    FROM performance_schema.setup_consumers WHERE NAME = 'events_waits_current'
                    AND ENABLED = 'YES'
                    """
                )
return cursor.fetchone()[0] > 0

This would avoid leaking state and making the monolithic class more complex with additional public variables.

@alexbarksdale alexbarksdale requested a review from justiniso June 8, 2022 21:28
@alexbarksdale alexbarksdale merged commit 07020f1 into master Jun 8, 2022
@alexbarksdale alexbarksdale deleted the alex.barksdale/surface-mysql-misconfiguration branch June 8, 2022 22:11
github-actions bot pushed a commit that referenced this pull request Jun 8, 2022
* Update mysql activity query

* Check if events-waits-current is enabled

* Revert activity query changes

* Update query activity query

* Update warning message

* Revert 07020f1
sarah-witt pushed a commit that referenced this pull request Jun 8, 2022
* Update mysql activity query

* Check if events-waits-current is enabled

* Revert activity query changes

* Update query activity query

* Update warning message

* Revert
@sarah-witt sarah-witt mentioned this pull request Jun 8, 2022
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category/bugfix For use during Agent Release period integration/mysql
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants