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

[DBM-2685] Fix explaining parameterized queries flood server logs #15612

Merged
merged 6 commits into from
Aug 30, 2023

Conversation

lu-zhengda
Copy link
Contributor

@lu-zhengda lu-zhengda commented Aug 17, 2023

What does this PR do?

This PR fixes explaining parameterized queries flood server by pre-check if a query is parametrized or not. The pre-check helps us not blindly run explain on a parametrized query that expected to fail.

https://datadoghq.atlassian.net/browse/DBM-2685

Motivation

  • Not blindly run explain on a parametrized query that expected to fail. This way we are not running explain twice for the parametrized query while expecting first explain to fail then run _explain_parameterized_queries.
  • Stops flooding server logs with errors like ERROR: there is no parameter $1 at character 86

Additional Notes

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Changelog entries must be created for modifications to shipped code
  • Add the qa/skip-qa label if the PR doesn't need to be tested during QA.

@ghost ghost added the integration/postgres label Aug 17, 2023
@lu-zhengda lu-zhengda changed the title [DBM-2685] Fix explaining parameterized queries flood server logs with pre-check [DBM-2685] Fix explaining parameterized queries flood server logs Aug 17, 2023
@ghost ghost added the documentation label Aug 17, 2023
@github-actions
Copy link

github-actions bot commented Aug 17, 2023

Test Results

     12 files       12 suites   14m 36s ⏱️
   236 tests    234 ✔️   2 💤 0
1 422 runs  1 374 ✔️ 48 💤 0

Results for commit ada80b0.

♻️ This comment has been updated with latest results.

@codecov
Copy link

codecov bot commented Aug 17, 2023

Codecov Report

Merging #15612 (ada80b0) into master (d205671) will decrease coverage by 0.06%.
Report is 25 commits behind head on master.
The diff coverage is 97.29%.

Flag Coverage Δ
postgres 91.91% <97.29%> (+0.09%) ⬆️

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

@lu-zhengda lu-zhengda marked this pull request as ready for review August 18, 2023 15:29
@lu-zhengda lu-zhengda requested review from a team as code owners August 18, 2023 15:29
Copy link
Contributor

@alexandre-normand alexandre-normand left a comment

Choose a reason for hiding this comment

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

🙇

@lu-zhengda lu-zhengda self-assigned this Aug 21, 2023
@lu-zhengda lu-zhengda merged commit e470c55 into master Aug 30, 2023
35 checks passed
@lu-zhengda lu-zhengda deleted the zhengda.lu/explain-parameterized-queries branch August 30, 2023 14:25
boluwaji-deriv pushed a commit to boluwaji-deriv/integrations-core that referenced this pull request Aug 30, 2023
…taDog#15612)

* check if query is parameterized before blindly explain then fail

* update CHANGELOG

* skip explain parameterized query test for pg version < 12
FlorentClarret added a commit that referenced this pull request Aug 31, 2023
#15629)

* Revise postgresql.replication_delay to Function with Archive WAL-driven PostgreSQL Replica

* modify query and also add changelog message

* update master with branch

* update master with branch

* Add short hand for force env rebuild in test (#15716)

* add short hand for force-env-rebuild

* add changelog entry

* styling

* [Release] Update metadata (#15717)

* [Release] Bumped postgres version to 14.2.1 (#15713)

* Bump oracledb version (#15595)

* Bump oracledb version

* update reqs

* update changelog

* Mark one metric as optional (#15719)

* Update the test environments (#15644)

* Update the test environments

* Apply suggestions from code review

Co-authored-by: Ofek Lev <ofekmeister@gmail.com>

* update metadata.csv

---------

Co-authored-by: Ofek Lev <ofekmeister@gmail.com>

* [DBM-2685] Fix explaining parameterized queries flood server logs (#15612)

* check if query is parameterized before blindly explain then fail

* update CHANGELOG

* skip explain parameterized query test for pg version < 12

* fix rate limited activity collection test case (#15715)

* [DBM-2734] fix test_snapshot_xmin for pg > 13 (#15718)

* fix test_snapshot_xmin for pg > 13

* reorder the steps to first collect metrics

* remove check.cancel()

---------

Co-authored-by: Steven Yuen <steven.yuen@datadoghq.com>
Co-authored-by: Andrew Zhang <andrew.zhang@datadoghq.com>
Co-authored-by: vivek-datadog <131680079+vivek-datadog@users.noreply.github.com>
Co-authored-by: Florent Clarret <florent.clarret@datadoghq.com>
Co-authored-by: Ofek Lev <ofekmeister@gmail.com>
Co-authored-by: Zhengda Lu <zhengda.lu@datadoghq.com>
jmeunier28 pushed a commit that referenced this pull request Sep 19, 2023
…5612)

* check if query is parameterized before blindly explain then fail

* update CHANGELOG

* skip explain parameterized query test for pg version < 12
lu-zhengda added a commit that referenced this pull request Sep 27, 2023
#15629)

* Revise postgresql.replication_delay to Function with Archive WAL-driven PostgreSQL Replica

* modify query and also add changelog message

* update master with branch

* update master with branch

* Add short hand for force env rebuild in test (#15716)

* add short hand for force-env-rebuild

* add changelog entry

* styling

* [Release] Update metadata (#15717)

* [Release] Bumped postgres version to 14.2.1 (#15713)

* Bump oracledb version (#15595)

* Bump oracledb version

* update reqs

* update changelog

* Mark one metric as optional (#15719)

* Update the test environments (#15644)

* Update the test environments

* Apply suggestions from code review

Co-authored-by: Ofek Lev <ofekmeister@gmail.com>

* update metadata.csv

---------

Co-authored-by: Ofek Lev <ofekmeister@gmail.com>

* [DBM-2685] Fix explaining parameterized queries flood server logs (#15612)

* check if query is parameterized before blindly explain then fail

* update CHANGELOG

* skip explain parameterized query test for pg version < 12

* fix rate limited activity collection test case (#15715)

* [DBM-2734] fix test_snapshot_xmin for pg > 13 (#15718)

* fix test_snapshot_xmin for pg > 13

* reorder the steps to first collect metrics

* remove check.cancel()

---------

Co-authored-by: Steven Yuen <steven.yuen@datadoghq.com>
Co-authored-by: Andrew Zhang <andrew.zhang@datadoghq.com>
Co-authored-by: vivek-datadog <131680079+vivek-datadog@users.noreply.github.com>
Co-authored-by: Florent Clarret <florent.clarret@datadoghq.com>
Co-authored-by: Ofek Lev <ofekmeister@gmail.com>
Co-authored-by: Zhengda Lu <zhengda.lu@datadoghq.com>
lu-zhengda added a commit that referenced this pull request Sep 28, 2023
#15925)

* Revise postgresql.replication_delay to Function with Archive WAL-driv… (#15629)

* Revise postgresql.replication_delay to Function with Archive WAL-driven PostgreSQL Replica

* modify query and also add changelog message

* update master with branch

* update master with branch

* Add short hand for force env rebuild in test (#15716)

* add short hand for force-env-rebuild

* add changelog entry

* styling

* [Release] Update metadata (#15717)

* [Release] Bumped postgres version to 14.2.1 (#15713)

* Bump oracledb version (#15595)

* Bump oracledb version

* update reqs

* update changelog

* Mark one metric as optional (#15719)

* Update the test environments (#15644)

* Update the test environments

* Apply suggestions from code review

Co-authored-by: Ofek Lev <ofekmeister@gmail.com>

* update metadata.csv

---------

Co-authored-by: Ofek Lev <ofekmeister@gmail.com>

* [DBM-2685] Fix explaining parameterized queries flood server logs (#15612)

* check if query is parameterized before blindly explain then fail

* update CHANGELOG

* skip explain parameterized query test for pg version < 12

* fix rate limited activity collection test case (#15715)

* [DBM-2734] fix test_snapshot_xmin for pg > 13 (#15718)

* fix test_snapshot_xmin for pg > 13

* reorder the steps to first collect metrics

* remove check.cancel()

---------

Co-authored-by: Steven Yuen <steven.yuen@datadoghq.com>
Co-authored-by: Andrew Zhang <andrew.zhang@datadoghq.com>
Co-authored-by: vivek-datadog <131680079+vivek-datadog@users.noreply.github.com>
Co-authored-by: Florent Clarret <florent.clarret@datadoghq.com>
Co-authored-by: Ofek Lev <ofekmeister@gmail.com>
Co-authored-by: Zhengda Lu <zhengda.lu@datadoghq.com>

* update changelog

* update changelog

* update changelog

* remove duplicate changelog entry

* update changelog

---------

Co-authored-by: boluwaji-deriv <97861462+boluwaji-deriv@users.noreply.github.com>
Co-authored-by: Steven Yuen <steven.yuen@datadoghq.com>
Co-authored-by: Andrew Zhang <andrew.zhang@datadoghq.com>
Co-authored-by: vivek-datadog <131680079+vivek-datadog@users.noreply.github.com>
Co-authored-by: Florent Clarret <florent.clarret@datadoghq.com>
Co-authored-by: Ofek Lev <ofekmeister@gmail.com>
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.

3 participants