-
Notifications
You must be signed in to change notification settings - Fork 1.5k
[dbm] Fix logic for only_custom_queries parameter #21708
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
[dbm] Fix logic for only_custom_queries parameter #21708
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files🚀 New features to boost your workflow:
|
eric-weaver
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! One side effect of this change is it looks like we're now running custom queries last at the check loop. This is probably for the better until we can move custom queries into it's own out of band loop. It's not uncommon for broken or slow custom queries to cause checks to crash and miss capturing builtin metrics and events. So one thing we should note and be aware of for this change is additional data that might have been missed might start making it through on upgrade
* fix logic for only_custom_queries parameter * fix custom queries test * add changelog * run ddev test format
What does this PR do?
This PR fixes the logic for the
only_custom_queriesconfiguration option. Previously this parameter wasn't doing what it said as we still ran the_collect_statsfunction and other dbm related functions even when the parameter was turned on.I tested this by running a local version of the agent against the postgres local stack, having the primary with
dbm:falseandonly_custom_queries:true. The results are as follows:Our custom query is emitting correctly:

No metrics can be found in either of the default postgres integration dashboards. The database instance can't be found in the databases list as well.
One positive effect from this is that the custom queries metric emission is done at the end of the check with this new PR. Previously, custom queries would cause check crashes if they were malformed or really slow, by having them at the end we now make sure everything we collect is captured before trying to emit the custom queries' metrics.
Motivation
This functionality was not working as expected for a customer, leading us to finding the issue: Ticket
Review checklist (to be filled by reviewers)
qa/skip-qalabel if the PR doesn't need to be tested during QA.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged