Skip to content

Commit ff8bf3b

Browse files
authored
Revert "Upgrade PGBouncer to psycopg3 (DataDog#20550)" (DataDog#20884)
This reverts commit 274b861.
1 parent 10c3d0d commit ff8bf3b

8 files changed

Lines changed: 136 additions & 137 deletions

File tree

.ddev/config.toml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,6 @@ oauthlib = ['BSD-3-Clause']
7070
mmh3 = ['CC0-1.0']
7171
# https://github.com/paramiko/paramiko/blob/master/LICENSE
7272
paramiko = ['LGPL-2.1-only']
73-
# https://github.com/psycopg/psycopg/blob/master/LICENSE.txt
74-
psycopg = ['LGPL-3.0-only']
7573
# https://github.com/psycopg/psycopg2/blob/master/LICENSE
7674
# https://github.com/psycopg/psycopg2/blob/master/doc/COPYING.LESSER
7775
psycopg2-binary = ['LGPL-3.0-only', 'BSD-3-Clause']

LICENSE-3rdparty.csv

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ ply,PyPI,BSD-3-Clause,Copyright (C) 2001-2018
4646
prometheus-client,PyPI,Apache-2.0,Copyright 2015 The Prometheus Authors
4747
protobuf,PyPI,BSD-3-Clause,Copyright 2008 Google Inc. All rights reserved.
4848
psutil,PyPI,BSD-3-Clause,"Copyright (c) 2009, Jay Loden, Dave Daeschler, Giampaolo Rodola"
49-
psycopg,PyPI,LGPL-3.0-only,Copyright (C) 2020 The Psycopg Team
5049
psycopg2-binary,PyPI,BSD-3-Clause,Copyright 2013 Federico Di Gregorio
5150
psycopg2-binary,PyPI,LGPL-3.0-only,Copyright (C) 2013 Federico Di Gregorio
5251
pyOpenSSL,PyPI,Apache-2.0,Copyright The pyOpenSSL developers

agent_requirements.in

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ ply==3.11
3232
prometheus-client==0.22.1
3333
protobuf==6.31.1
3434
psutil==6.0.0
35-
psycopg[binary]==3.2.9
3635
psycopg2-binary==2.9.9
3736
psycopg[binary]==3.2.9
3837
pyasn1==0.4.8

pgbouncer/datadog_checks/pgbouncer/pgbouncer.py

Lines changed: 62 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,8 @@
55
import time
66
from urllib.parse import urlparse
77

8-
import psycopg as pg
9-
from psycopg import ClientCursor
10-
from psycopg.rows import dict_row
8+
import psycopg2 as pg
9+
from psycopg2 import extras as pgextras
1110

1211
from datadog_checks.base import AgentCheck, ConfigurationError, is_affirmative
1312
from datadog_checks.pgbouncer.metrics import (
@@ -74,50 +73,51 @@ def _collect_stats(self, db):
7473
metric_scope.append(SERVERS_METRICS)
7574

7675
try:
77-
for scope in metric_scope:
78-
descriptors = scope['descriptors']
79-
metrics = scope['metrics']
80-
query = scope['query']
76+
with db.cursor(cursor_factory=pgextras.DictCursor) as cursor:
77+
for scope in metric_scope:
78+
descriptors = scope['descriptors']
79+
metrics = scope['metrics']
80+
query = scope['query']
81+
82+
try:
83+
self.log.debug("Running query: %s", query)
84+
cursor.execute(query)
85+
rows = self.iter_rows(cursor)
86+
87+
except Exception as e:
88+
self.log.exception("Not all metrics may be available: %s", str(e))
89+
90+
else:
91+
for row in rows:
92+
if 'key' in row: # We are processing "config metrics"
93+
# Make a copy of the row to allow mutation
94+
# (a `psycopg2.lib.extras.DictRow` object doesn't accept a new key)
95+
row = row.copy()
96+
# We flip/rotate the row: row value becomes the column name
97+
row[row['key']] = row['value']
98+
# Skip the "pgbouncer" database
99+
elif row.get('database') == self.DB_NAME:
100+
continue
101+
102+
tags = list(self.tags)
103+
tags += ["%s:%s" % (tag, row[column]) for (column, tag) in descriptors if column in row]
104+
for column, (name, reporter) in metrics:
105+
if column in row:
106+
value = row[column]
107+
if column in ['connect_time', 'request_time']:
108+
self.log.debug("Parsing timestamp; original value: %s", value)
109+
# First get rid of any UTC suffix.
110+
value = re.findall(r'^[^ ]+ [^ ]+', value)[0]
111+
value = time.strptime(value, '%Y-%m-%d %H:%M:%S')
112+
value = time.mktime(value)
113+
reporter(self, name, value, tags)
114+
115+
if not rows:
116+
self.log.warning("No results were found for query: %s", query)
117+
118+
except pg.Error:
119+
self.log.exception("Connection error")
81120

82-
try:
83-
cursor = db.cursor(row_factory=dict_row)
84-
self.log.debug("Running query: %s", query)
85-
cursor.execute(query)
86-
rows = self.iter_rows(cursor)
87-
88-
except (pg.InterfaceError, pg.OperationalError) as e:
89-
self.log.error("Not all metrics may be available: %s", e)
90-
raise ShouldReconnectException
91-
92-
else:
93-
for row in rows:
94-
if 'key' in row: # We are processing "config metrics"
95-
# Make a copy of the row to allow mutation
96-
row = row.copy()
97-
# We flip/rotate the row: row value becomes the column name
98-
row[row['key']] = row['value']
99-
# Skip the "pgbouncer" database
100-
elif row.get('database') == self.DB_NAME:
101-
continue
102-
103-
tags = list(self.tags)
104-
tags += ["%s:%s" % (tag, row[column]) for (column, tag) in descriptors if column in row]
105-
for column, (name, reporter) in metrics:
106-
if column in row:
107-
value = row[column]
108-
if column in ['connect_time', 'request_time']:
109-
self.log.debug("Parsing timestamp; original value: %s", value)
110-
# First get rid of any UTC suffix.
111-
value = re.findall(r'^[^ ]+ [^ ]+', value)[0]
112-
value = time.strptime(value, '%Y-%m-%d %H:%M:%S')
113-
value = time.mktime(value)
114-
reporter(self, name, value, tags)
115-
116-
if not rows:
117-
self.log.warning("No results were found for query: %s", query)
118-
119-
except pg.Error as e:
120-
self.log.error("Not all metrics may be available: %s", e)
121121
raise ShouldReconnectException
122122

123123
def iter_rows(self, cursor):
@@ -138,25 +138,21 @@ def iter_rows(self, cursor):
138138

139139
def _get_connect_kwargs(self):
140140
"""
141-
Get the params to pass to psycopg.connect() based on passed-in vals
141+
Get the params to pass to psycopg2.connect() based on passed-in vals
142142
from yaml settings file
143143
"""
144-
# It's important to set the client_encoding to utf-8
145-
# PGBouncer defaults to an encoding of 'UNICODE`, which will cause psycopg to error out
146144
if self.database_url:
147-
return {'conninfo': self.database_url, 'client_encoding': 'utf-8'}
145+
return {'dsn': self.database_url}
148146

149147
if self.host in ('localhost', '127.0.0.1') and self.password == '':
150148
# Use ident method
151-
return {'conninfo': "user={} dbname={} client_encoding=utf-8".format(self.user, self.DB_NAME)}
149+
return {'dsn': "user={} dbname={}".format(self.user, self.DB_NAME)}
152150

153151
args = {
154152
'host': self.host,
155153
'user': self.user,
156154
'password': self.password,
157-
'dbname': self.DB_NAME,
158-
'cursor_factory': ClientCursor,
159-
'client_encoding': 'utf-8',
155+
'database': self.DB_NAME,
160156
}
161157
if self.port:
162158
args['port'] = self.port
@@ -168,9 +164,8 @@ def _new_connection(self):
168164
connection = None
169165
try:
170166
connect_kwargs = self._get_connect_kwargs()
171-
# Somewhat counterintuitively, we need to set autocommit to True to avoid a BEGIN/COMMIT block
172-
# https://www.psycopg.org/psycopg3/docs/basic/transactions.html#autocommit-transactions
173-
connection = pg.connect(**connect_kwargs, autocommit=True)
167+
connection = pg.connect(**connect_kwargs)
168+
connection.set_isolation_level(pg.extensions.ISOLATION_LEVEL_AUTOCOMMIT)
174169
return connection
175170
except Exception:
176171
if connection:
@@ -201,7 +196,6 @@ def _get_redacted_dsn(self):
201196
return self.database_url
202197

203198
def _close_connection(self):
204-
"""Close the connection to PgBouncer"""
205199
if self.connection:
206200
try:
207201
self.connection.close()
@@ -244,31 +238,18 @@ def _collect_metadata(self, db):
244238
self.set_metadata('version', pgbouncer_version)
245239

246240
def get_version(self, db):
247-
"""
248-
Get the version of PgBouncer.
249-
"""
250241
if not db:
251242
self.log.warning("Cannot get version: no active connection")
252243
return None
253244

254245
regex = r'\d+\.\d+\.\d+'
255-
try:
256-
with db.cursor() as cursor:
257-
# This command was added in pgbouncer 1.12
258-
cursor.execute('SHOW VERSION;')
259-
result = cursor.fetchone()
260-
if result:
261-
# Result looks like: ['PgBouncer 1.2.3 ...']
262-
version_string = result[0]
263-
return re.findall(regex, version_string)[0]
264-
else:
265-
self.log.debug("No version found in result: %s", result)
266-
return None
267-
except pg.ProgrammingError as e:
268-
# This is expected on versions of pgbouncer < 1.12
269-
self.log.debug("Cannot retrieve pgbouncer version using `SHOW VERSION;`: %s", e)
270-
return None
271-
except Exception as e:
272-
self.log.error("An unexpected error occurred when retrieving pgbouncer version: %s", e)
273-
return None
274-
return None
246+
with db.cursor(cursor_factory=pgextras.DictCursor) as cursor:
247+
cursor.execute('SHOW VERSION;')
248+
if db.notices:
249+
data = db.notices[0]
250+
else:
251+
data = cursor.fetchone()[0]
252+
res = re.findall(regex, data)
253+
if res:
254+
return res[0]
255+
self.log.debug("Couldn't detect version from %s", data)

pgbouncer/pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ license = "BSD-3-Clause"
3737

3838
[project.optional-dependencies]
3939
deps = [
40-
"psycopg[binary]==3.2.9",
40+
"psycopg2-binary==2.9.9",
4141
]
4242

4343
[project.urls]

pgbouncer/tests/conftest.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import os
66
from copy import deepcopy
77

8-
import psycopg
8+
import psycopg2
99
import pytest
1010
from packaging import version
1111

@@ -20,8 +20,8 @@ def container_up(service_name, port):
2020
"""
2121
Try to connect to postgres/pgbouncer
2222
"""
23-
psycopg.connect(
24-
host=common.HOST, port=port, user=common.USER, password=common.PASS, dbname=common.DB, connect_timeout=2
23+
psycopg2.connect(
24+
host=common.HOST, port=port, user=common.USER, password=common.PASS, database=common.DB, connect_timeout=2
2525
)
2626

2727

pgbouncer/tests/test_pgbouncer_integration_e2e.py

Lines changed: 39 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
# All rights reserved
33
# Licensed under Simplified BSD License (see LICENSE)
44

5-
import psycopg
5+
import psycopg2
66
import pytest
77
from packaging import version
88

@@ -13,40 +13,51 @@
1313

1414
@pytest.mark.integration
1515
@pytest.mark.usefixtures("dd_environment")
16-
def test_check(aggregator, instance, datadog_agent, dd_run_check):
16+
def test_check(instance, aggregator, datadog_agent, dd_run_check):
17+
# add some stats
18+
connection = psycopg2.connect(
19+
host=common.HOST,
20+
port=common.PORT,
21+
user=common.USER,
22+
password=common.PASS,
23+
database=common.DB,
24+
connect_timeout=1,
25+
)
26+
connection.set_isolation_level(psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT)
27+
cur = connection.cursor()
28+
cur.execute('SELECT * FROM persons;')
29+
30+
# run the check
1731
check = PgBouncer('pgbouncer', {}, [instance])
32+
check.check_id = 'test:123'
1833
dd_run_check(check)
1934

2035
env_version = common.get_version_from_env()
2136
assert_metric_coverage(env_version, aggregator)
2237

23-
# SHOW VERSION; is only available on pgbouncer 1.12+
24-
if env_version >= version.parse('1.12.0'):
25-
version_metadata = {
26-
'version.raw': str(env_version),
27-
'version.scheme': 'semver',
28-
'version.major': str(env_version.major),
29-
'version.minor': str(env_version.minor),
30-
'version.patch': str(env_version.micro),
31-
}
32-
datadog_agent.assert_metadata(check.check_id, version_metadata)
33-
else:
34-
# No version metadata expected for older versions
35-
datadog_agent.assert_metadata(check.check_id, {})
38+
version_metadata = {
39+
'version.raw': str(env_version),
40+
'version.scheme': 'semver',
41+
'version.major': str(env_version.major),
42+
'version.minor': str(env_version.minor),
43+
'version.patch': str(env_version.micro),
44+
}
45+
datadog_agent.assert_metadata('test:123', version_metadata)
3646

3747

3848
@pytest.mark.integration
3949
@pytest.mark.usefixtures("dd_environment")
4050
def test_check_with_clients(instance, aggregator, datadog_agent, dd_run_check):
4151
# add some stats
42-
connection = psycopg.connect(
52+
connection = psycopg2.connect(
4353
host=common.HOST,
4454
port=common.PORT,
4555
user=common.USER,
4656
password=common.PASS,
47-
dbname=common.DB,
57+
database=common.DB,
4858
connect_timeout=1,
4959
)
60+
connection.set_isolation_level(psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT)
5061
cur = connection.cursor()
5162
cur.execute('SELECT * FROM persons;')
5263

@@ -69,14 +80,15 @@ def test_check_with_clients(instance, aggregator, datadog_agent, dd_run_check):
6980
@pytest.mark.usefixtures("dd_environment")
7081
def test_check_with_servers(instance, aggregator, datadog_agent, dd_run_check):
7182
# add some stats
72-
connection = psycopg.connect(
83+
connection = psycopg2.connect(
7384
host=common.HOST,
7485
port=common.PORT,
7586
user=common.USER,
7687
password=common.PASS,
77-
dbname=common.DB,
88+
database=common.DB,
7889
connect_timeout=1,
7990
)
91+
connection.set_isolation_level(psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT)
8092
cur = connection.cursor()
8193
cur.execute('SELECT * FROM persons;')
8294

@@ -115,17 +127,14 @@ def test_check_with_url(instance_with_url, aggregator, datadog_agent, dd_run_che
115127
env_version = common.get_version_from_env()
116128
assert_metric_coverage(env_version, aggregator)
117129

118-
if env_version >= version.parse('1.12.0'):
119-
version_metadata = {
120-
'version.raw': str(env_version),
121-
'version.scheme': 'semver',
122-
'version.major': str(env_version.major),
123-
'version.minor': str(env_version.minor),
124-
'version.patch': str(env_version.micro),
125-
}
126-
datadog_agent.assert_metadata(check.check_id, version_metadata)
127-
else:
128-
datadog_agent.assert_metadata(check.check_id, {})
130+
version_metadata = {
131+
'version.raw': str(env_version),
132+
'version.scheme': 'semver',
133+
'version.major': str(env_version.major),
134+
'version.minor': str(env_version.minor),
135+
'version.patch': str(env_version.micro),
136+
}
137+
datadog_agent.assert_metadata('test:123', version_metadata)
129138

130139

131140
@pytest.mark.e2e

0 commit comments

Comments
 (0)