From 149d433bac0a57d07fdc2480a87ef16c8530bc30 Mon Sep 17 00:00:00 2001 From: Jo-Ann Meunier Date: Mon, 19 Jun 2023 14:43:09 -0400 Subject: [PATCH] more fixes --- .../postgres/explain_parameterized_queries.py | 6 +- postgres/datadog_checks/postgres/metadata.py | 1 + .../datadog_checks/postgres/statements.py | 65 ++++++++++--------- 3 files changed, 40 insertions(+), 32 deletions(-) diff --git a/postgres/datadog_checks/postgres/explain_parameterized_queries.py b/postgres/datadog_checks/postgres/explain_parameterized_queries.py index 18a58ffb94d49..448a3f104ff61 100644 --- a/postgres/datadog_checks/postgres/explain_parameterized_queries.py +++ b/postgres/datadog_checks/postgres/explain_parameterized_queries.py @@ -5,6 +5,7 @@ import logging import psycopg +from psycopg.rows import dict_row from datadog_checks.base.utils.db.sql import compute_sql_signature from datadog_checks.base.utils.tracking import tracked_method @@ -112,7 +113,10 @@ def _get_number_of_parameters_for_prepared_statement(self, dbname, query_signatu rows = self._execute_query_and_fetch_rows( dbname, PARAM_TYPES_COUNT_QUERY.format(query_signature=query_signature) ) - return rows[0][0] if rows else 0 + count = 0 + if rows and 'count' in rows[0]: + count = rows[0]['count'] + return count @tracked_method(agent_check_getter=agent_check_getter) def _explain_prepared_statement(self, dbname, statement, obfuscated_statement, query_signature): diff --git a/postgres/datadog_checks/postgres/metadata.py b/postgres/datadog_checks/postgres/metadata.py index ab3bc486f4f76..ff89b881f59f3 100644 --- a/postgres/datadog_checks/postgres/metadata.py +++ b/postgres/datadog_checks/postgres/metadata.py @@ -6,6 +6,7 @@ from typing import Dict, Optional, Tuple # noqa: F401 import psycopg +from psycopg.rows import dict_row try: import datadog_agent diff --git a/postgres/datadog_checks/postgres/statements.py b/postgres/datadog_checks/postgres/statements.py index c070d4979929f..33d8b5e3a64a1 100644 --- a/postgres/datadog_checks/postgres/statements.py +++ b/postgres/datadog_checks/postgres/statements.py @@ -170,9 +170,10 @@ def _get_pg_stat_statements_columns(self): query = STATEMENTS_QUERY.format( cols='*', pg_stat_statements_view=self._config.pg_stat_statements_view, extra_clauses="LIMIT 0", filters="" ) - # TODO: using a with block here actually closes the connection https://www.psycopg.org/psycopg3/docs/basic/from_pg2.html#diff-with + # TODO: using a with blo`ck here actually closes the connection https://www.psycopg.org/psycopg3/docs/basic/from_pg2.html#diff-with with self._check._get_db(self._config.dbname).cursor() as cursor: - self._execute_query(cursor, query, params=(self._config.dbname,)) + # TODO: we do not need the dbname as a param here, psycopg2 just ignored it, but psycopg3 will fail + self._execute_query(cursor, query, params=()) col_names = [desc[0] for desc in cursor.description] if cursor.description else [] self._stat_column_cache = col_names return col_names @@ -247,31 +248,33 @@ def _load_pg_stat_statements(self): if self._check.pg_settings.get("track_io_timing") != "on": desired_columns -= PG_STAT_STATEMENTS_TIMING_COLUMNS - pg_stat_statements_max = int(self._check.pg_settings.get("pg_stat_statements.max")) - if pg_stat_statements_max > self._pg_stat_statements_max_warning_threshold: - self._check.record_warning( - DatabaseConfigurationError.high_pg_stat_statements_max, - warning_with_tags( - "pg_stat_statements.max is set to %d which is higher than the supported " - "value of %d. This can have a negative impact on database and collection of " - "query metrics performance. Consider lowering the pg_stat_statements.max value to %d. " - "Alternatively, you may acknowledge the potential performance impact by increasing the " - "query_metrics.pg_stat_statements_max_warning_threshold to equal or greater than %d to " - "silence this warning. " - "See https://docs.datadoghq.com/database_monitoring/setup_postgres/" - "troubleshooting#%s for more details", - pg_stat_statements_max, - self._pg_stat_statements_max_warning_threshold, - self._pg_stat_statements_max_warning_threshold, - self._pg_stat_statements_max_warning_threshold, - DatabaseConfigurationError.high_pg_stat_statements_max.value, - host=self._check.resolved_hostname, - dbname=self._config.dbname, - code=DatabaseConfigurationError.high_pg_stat_statements_max.value, - value=pg_stat_statements_max, - threshold=self._pg_stat_statements_max_warning_threshold, - ), - ) + # TODO: make this work again with upgraded psycopg + # this is just for monitoring & isn't important for the actual functionality in the check + # pg_stat_statements_max = int(self._check.pg_settings.get("pg_stat_statements.max")) + # if pg_stat_statements_max > self._pg_stat_statements_max_warning_threshold: + # self._check.record_warning( + # DatabaseConfigurationError.high_pg_stat_statements_max, + # warning_with_tags( + # "pg_stat_statements.max is set to %d which is higher than the supported " + # "value of %d. This can have a negative impact on database and collection of " + # "query metrics performance. Consider lowering the pg_stat_statements.max value to %d. " + # "Alternatively, you may acknowledge the potential performance impact by increasing the " + # "query_metrics.pg_stat_statements_max_warning_threshold to equal or greater than %d to " + # "silence this warning. " + # "See https://docs.datadoghq.com/database_monitoring/setup_postgres/" + # "troubleshooting#%s for more details", + # pg_stat_statements_max, + # self._pg_stat_statements_max_warning_threshold, + # self._pg_stat_statements_max_warning_threshold, + # self._pg_stat_statements_max_warning_threshold, + # DatabaseConfigurationError.high_pg_stat_statements_max.value, + # host=self._check.resolved_hostname, + # dbname=self._config.dbname, + # code=DatabaseConfigurationError.high_pg_stat_statements_max.value, + # value=pg_stat_statements_max, + # threshold=self._pg_stat_statements_max_warning_threshold, + # ), + # ) query_columns = sorted(available_columns & desired_columns) params = () @@ -301,7 +304,7 @@ def _load_pg_stat_statements(self): if ( isinstance(e, psycopg.errors.ObjectNotInPrerequisiteState) - ) and 'pg_stat_statements must be loaded' in str(e.pgerror): + ) and 'pg_stat_statements must be loaded' in str(e.sqlstate): error_tag = "error:database-{}-pg_stat_statements_not_loaded".format(type(e).__name__) self._check.record_warning( DatabaseConfigurationError.pg_stat_statements_not_loaded, @@ -317,7 +320,7 @@ def _load_pg_stat_statements(self): code=DatabaseConfigurationError.pg_stat_statements_not_loaded.value, ), ) - elif isinstance(e, psycopg.errors.UndefinedTable) and 'pg_stat_statements' in str(e.pgerror): + elif isinstance(e, psycopg.errors.UndefinedTable) and 'pg_stat_statements' in str(e.sqlstate): error_tag = "error:database-{}-pg_stat_statements_not_created".format(type(e).__name__) self._check.record_warning( DatabaseConfigurationError.pg_stat_statements_not_created, @@ -362,7 +365,7 @@ def _emit_pg_stat_statements_dealloc(self): self._check._get_db(self._config.dbname).cursor(row_factory=dict_row), PG_STAT_STATEMENTS_DEALLOC, ) - if rows: + if rows and 'count' in rows[0]: dealloc = rows[0]['count'] self._check.monotonic_count( "postgresql.pg_stat_statements.dealloc", @@ -382,7 +385,7 @@ def _emit_pg_stat_statements_metrics(self): query, ) count = 0 - if rows: + if rows and 'count' in rows[0]: count = rows[0]['count'] self._check.gauge( "postgresql.pg_stat_statements.max",