Skip to content

Commit

Permalink
more fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@jmeunier28
jmeunier28 committed Jun 19, 2023
1 parent f298868 commit 149d433
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 32 deletions.
6 changes: 5 additions & 1 deletion postgres/datadog_checks/postgres/explain_parameterized_queries.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down
1 change: 1 addition & 0 deletions postgres/datadog_checks/postgres/metadata.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from typing import Dict, Optional, Tuple # noqa: F401

import psycopg
from psycopg.rows import dict_row

try:
import datadog_agent
Expand Down
65 changes: 34 additions & 31 deletions postgres/datadog_checks/postgres/statements.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 = ()
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down

0 comments on commit 149d433

Please sign in to comment.