Skip to content

Commit 5b65299

Browse files
sethsamuelalopezz
andauthored
Upgrade PGBouncer to psycopg3 (#19325)
* WIP * WIP * v3 * Changelog * Use source * Comments * Binary * Build psycopg (#19326) * WIP * Merge fix * Dep * Fix license * WIP * WIP * Changelog * test * test * Version * Changelog * Always add bin path to $PATH on macos builders * Add a dummy change to test that a macos build with a cache hit works * Revert "Add a dummy change to test that a macos build with a cache hit works" This reverts commit 4df3963. --------- Co-authored-by: Alex Lopez <[email protected]> Co-authored-by: Alex Lopez <[email protected]>
1 parent b0ae51d commit 5b65299

File tree

14 files changed

+92
-91
lines changed

14 files changed

+92
-91
lines changed

.builders/images/macos-x86_64/builder_setup.sh

+2-2
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,8 @@ rm "${DD_PREFIX_PATH}/bin/curl"
9696

9797
# libpq and pg_config as needed by psycopg
9898
DOWNLOAD_URL="https://ftp.postgresql.org/pub/source/v{{version}}/postgresql-{{version}}.tar.bz2" \
99-
VERSION="16.0" \
100-
SHA256="df9e823eb22330444e1d48e52cc65135a652a6fdb3ce325e3f08549339f51b99" \
99+
VERSION="16.6" \
100+
SHA256="23369cdaccd45270ac5dcc30fa9da205d5be33fa505e1f17a0418d2caeca477b" \
101101
RELATIVE_PATH="postgresql-{{version}}" \
102102
install-from-source --without-readline --with-openssl --without-icu
103103

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#!/bin/bash
2+
3+
set -ex
4+
5+
sudo apt update
6+
sudo apt install -y --no-install-recommends build-essential python3-dev libpq-dev

.ddev/config.toml

-7
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,6 @@ mmh3 = ['CC0-1.0']
8585
paramiko = ['LGPL-2.1-only']
8686
# https://github.com/psycopg/psycopg/blob/master/LICENSE.txt
8787
psycopg = ['LGPL-3.0-only']
88-
# https://github.com/psycopg/psycopg2/blob/master/LICENSE
89-
# https://github.com/psycopg/psycopg2/blob/master/doc/COPYING.LESSER
90-
psycopg2-binary = ['LGPL-3.0-only', 'BSD-3-Clause']
9188
# https://github.com/Legrandin/pycryptodome/blob/master/LICENSE.rst
9289
pycryptodomex = ['Unlicense', 'BSD-2-Clause']
9390
# https://github.com/mongodb/mongo-python-driver/blob/master/LICENSE
@@ -118,7 +115,6 @@ lxml = 'https://github.com/lxml/lxml'
118115
packaging = 'https://github.com/pypa/packaging'
119116
paramiko = 'https://github.com/paramiko/paramiko'
120117
protobuf = 'https://github.com/protocolbuffers/protobuf'
121-
psycopg2-binary = 'https://github.com/psycopg/psycopg2'
122118
pycryptodomex = 'https://github.com/Legrandin/pycryptodome'
123119
redis = 'https://github.com/redis/redis-py'
124120
requests = 'https://github.com/psf/requests'
@@ -170,9 +166,6 @@ exclude = [
170166
'aerospike', # v8+ breaks agent build.
171167
# https://github.com/DataDog/integrations-core/pull/16080
172168
'lxml',
173-
# We're not ready to switch to v3 of the postgres library, see:
174-
# https://github.com/DataDog/integrations-core/pull/15859
175-
'psycopg2-binary',
176169
'psutil',
177170
'pymongo[srv]', # Upgrade from 4.8.0 to 4.10.1 causes "AttributeError: module 'pymongo' has no attribute 'mongo_client'"
178171
]

LICENSE-3rdparty.csv

-2
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,6 @@ prometheus-client,PyPI,Apache-2.0,Copyright 2015 The Prometheus Authors
4646
protobuf,PyPI,BSD-3-Clause,Copyright 2008 Google Inc. All rights reserved.
4747
psutil,PyPI,BSD-3-Clause,"Copyright (c) 2009, Jay Loden, Dave Daeschler, Giampaolo Rodola"
4848
psycopg,PyPI,LGPL-3.0-only,Copyright (C) 2020 The Psycopg Team
49-
psycopg2-binary,PyPI,BSD-3-Clause,Copyright 2013 Federico Di Gregorio
50-
psycopg2-binary,PyPI,LGPL-3.0-only,Copyright (C) 2013 Federico Di Gregorio
5149
pyOpenSSL,PyPI,Apache-2.0,Copyright The pyOpenSSL developers
5250
pyasn1,PyPI,BSD-3-Clause,"Copyright (c) 2005-2019, Ilya Etingof <[email protected]>"
5351
pycryptodomex,PyPI,BSD-2-Clause,Copyright 2014 Helder Eijs

agent_requirements.in

-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ ply==3.11
3131
prometheus-client==0.21.1
3232
protobuf==5.29.3
3333
psutil==6.0.0
34-
psycopg2-binary==2.9.9
3534
psycopg[c]==3.2.3
3635
pyasn1==0.4.8
3736
pycryptodomex==3.21.0
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Removed dependency on psycopg2

datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/licenses.py

-4
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,6 @@
4545
'mmh3': ['CC0-1.0'],
4646
# https://github.com/paramiko/paramiko/blob/master/LICENSE
4747
'paramiko': ['LGPL-2.1-only'],
48-
# https://github.com/psycopg/psycopg2/blob/master/LICENSE
49-
# https://github.com/psycopg/psycopg2/blob/master/doc/COPYING.LESSER
50-
'psycopg2-binary': ['LGPL-3.0-only', 'BSD-3-Clause'],
5148
# https://github.com/psycopg/psycopg/blob/master/LICENSE.txt
5249
'psycopg': ['LGPL-3.0-only'],
5350
# https://github.com/psycopg/psycopg/blob/master/psycopg_pool/LICENSE.txt
@@ -161,7 +158,6 @@
161158
'packaging': 'https://github.com/pypa/packaging',
162159
'paramiko': 'https://github.com/paramiko/paramiko',
163160
'protobuf': 'https://github.com/protocolbuffers/protobuf',
164-
'psycopg2-binary': 'https://github.com/psycopg/psycopg2',
165161
'psycopg': 'https://github.com/psycopg/psycopg',
166162
'pycryptodomex': 'https://github.com/Legrandin/pycryptodome',
167163
'redis': 'https://github.com/redis/redis-py',

pgbouncer/changelog.d/19325.added

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Upgrade pgbouncer to psycopg3

pgbouncer/changelog.d/19325.security

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Removed dependency on psycopg2

pgbouncer/datadog_checks/pgbouncer/pgbouncer.py

+62-60
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@
55
import time
66
from urllib.parse import urlparse
77

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

1112
from datadog_checks.base import AgentCheck, ConfigurationError, is_affirmative
1213
from datadog_checks.pgbouncer.metrics import (
@@ -73,47 +74,46 @@ def _collect_stats(self, db):
7374
metric_scope.append(SERVERS_METRICS)
7475

7576
try:
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)
77+
for scope in metric_scope:
78+
descriptors = scope['descriptors']
79+
metrics = scope['metrics']
80+
query = scope['query']
81+
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 Exception as e:
89+
self.log.exception("Not all metrics may be available: %s", str(e))
90+
91+
else:
92+
for row in rows:
93+
if 'key' in row: # We are processing "config metrics"
94+
# Make a copy of the row to allow mutation
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)
117117

118118
except pg.Error:
119119
self.log.exception("Connection error")
@@ -138,21 +138,25 @@ def iter_rows(self, cursor):
138138

139139
def _get_connect_kwargs(self):
140140
"""
141-
Get the params to pass to psycopg2.connect() based on passed-in vals
141+
Get the params to pass to psycopg.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
144146
if self.database_url:
145-
return {'dsn': self.database_url}
147+
return {'conninfo': self.database_url, 'client_encoding': 'utf-8'}
146148

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

151153
args = {
152154
'host': self.host,
153155
'user': self.user,
154156
'password': self.password,
155-
'database': self.DB_NAME,
157+
'dbname': self.DB_NAME,
158+
'cursor_factory': ClientCursor,
159+
'client_encoding': 'utf-8',
156160
}
157161
if self.port:
158162
args['port'] = self.port
@@ -166,8 +170,9 @@ def _get_connection(self, use_cached=None):
166170
return self.connection
167171
try:
168172
connect_kwargs = self._get_connect_kwargs()
169-
connection = pg.connect(**connect_kwargs)
170-
connection.set_isolation_level(pg.extensions.ISOLATION_LEVEL_AUTOCOMMIT)
173+
# Somewhat counterintuitively, we need to set autocommit to True to avoid a BEGIN/COMMIT block
174+
# https://www.psycopg.org/psycopg3/docs/basic/transactions.html#autocommit-transactions
175+
connection = pg.connect(**connect_kwargs, autocommit=True)
171176
except Exception:
172177
redacted_url = self._get_redacted_dsn()
173178
message = u'Cannot establish connection to {}'.format(redacted_url)
@@ -180,6 +185,10 @@ def _get_connection(self, use_cached=None):
180185
self.connection = connection
181186
return connection
182187

188+
def _close_connection(self):
189+
self.connection.close()
190+
self.connection = None
191+
183192
def _get_redacted_dsn(self):
184193
if not self.database_url:
185194
return u'pgbouncer://%s:******@%s:%s/%s' % (self.user, self.host, self.port, self.DB_NAME)
@@ -200,6 +209,8 @@ def check(self, instance):
200209

201210
self.service_check(self.SERVICE_CHECK_NAME, AgentCheck.OK, tags=self._get_service_checks_tags())
202211
self._set_metadata()
212+
# Avoid holding an open connection
213+
self._close_connection()
203214

204215
def _set_metadata(self):
205216
if self.is_metadata_collection_enabled():
@@ -209,14 +220,5 @@ def _set_metadata(self):
209220

210221
def get_version(self):
211222
db = self._get_connection()
212-
regex = r'\d+\.\d+\.\d+'
213-
with db.cursor(cursor_factory=pgextras.DictCursor) as cursor:
214-
cursor.execute('SHOW VERSION;')
215-
if db.notices:
216-
data = db.notices[0]
217-
else:
218-
data = cursor.fetchone()[0]
219-
res = re.findall(regex, data)
220-
if res:
221-
return res[0]
222-
self.log.debug("Couldn't detect version from %s", data)
223+
version = pg.pq.version_pretty(db.connection.info.server_version)
224+
return version

pgbouncer/pyproject.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ license = "BSD-3-Clause"
3737

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

4343
[project.urls]

pgbouncer/tests/common.py

+7
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,13 @@
2020
'tags': ['optional:tag1'],
2121
}
2222

23+
E2E_METADATA = {
24+
'start_commands': [
25+
'apt update',
26+
'apt install -y --no-install-recommends build-essential python3-dev libpq-dev',
27+
],
28+
}
29+
2330

2431
def get_version_from_env():
2532
return version.parse(os.environ.get('PGBOUNCER_VERSION'))

pgbouncer/tests/conftest.py

+4-4
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import os
66
from copy import deepcopy
77

8-
import psycopg2
8+
import psycopg
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-
psycopg2.connect(
24-
host=common.HOST, port=port, user=common.USER, password=common.PASS, database=common.DB, connect_timeout=2
23+
psycopg.connect(
24+
host=common.HOST, port=port, user=common.USER, password=common.PASS, dbname=common.DB, connect_timeout=2
2525
)
2626

2727

@@ -45,7 +45,7 @@ def dd_environment():
4545
],
4646
):
4747

48-
yield common.DEFAULT_INSTANCE
48+
yield common.DEFAULT_INSTANCE, common.E2E_METADATA
4949

5050

5151
@pytest.fixture

pgbouncer/tests/test_pgbouncer_integration_e2e.py

+7-10
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 psycopg2
5+
import psycopg
66
import pytest
77
from packaging import version
88

@@ -15,15 +15,14 @@
1515
@pytest.mark.usefixtures("dd_environment")
1616
def test_check(instance, aggregator, datadog_agent, dd_run_check):
1717
# add some stats
18-
connection = psycopg2.connect(
18+
connection = psycopg.connect(
1919
host=common.HOST,
2020
port=common.PORT,
2121
user=common.USER,
2222
password=common.PASS,
23-
database=common.DB,
23+
dbname=common.DB,
2424
connect_timeout=1,
2525
)
26-
connection.set_isolation_level(psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT)
2726
cur = connection.cursor()
2827
cur.execute('SELECT * FROM persons;')
2928

@@ -49,15 +48,14 @@ def test_check(instance, aggregator, datadog_agent, dd_run_check):
4948
@pytest.mark.usefixtures("dd_environment")
5049
def test_check_with_clients(instance, aggregator, datadog_agent, dd_run_check):
5150
# add some stats
52-
connection = psycopg2.connect(
51+
connection = psycopg.connect(
5352
host=common.HOST,
5453
port=common.PORT,
5554
user=common.USER,
5655
password=common.PASS,
57-
database=common.DB,
56+
dbname=common.DB,
5857
connect_timeout=1,
5958
)
60-
connection.set_isolation_level(psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT)
6159
cur = connection.cursor()
6260
cur.execute('SELECT * FROM persons;')
6361

@@ -80,15 +78,14 @@ def test_check_with_clients(instance, aggregator, datadog_agent, dd_run_check):
8078
@pytest.mark.usefixtures("dd_environment")
8179
def test_check_with_servers(instance, aggregator, datadog_agent, dd_run_check):
8280
# add some stats
83-
connection = psycopg2.connect(
81+
connection = psycopg.connect(
8482
host=common.HOST,
8583
port=common.PORT,
8684
user=common.USER,
8785
password=common.PASS,
88-
database=common.DB,
86+
dbname=common.DB,
8987
connect_timeout=1,
9088
)
91-
connection.set_isolation_level(psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT)
9289
cur = connection.cursor()
9390
cur.execute('SELECT * FROM persons;')
9491

0 commit comments

Comments
 (0)