-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
receiver/postgresql: improve connection management #30831
Comments
Pinging code owners:
See Adding Labels via Comments if you do not have permissions to add labels yourself. |
This seems like a reasonable improvement to me. @kevinnoel-be are you interested in submitting a PR for this? |
@djaglowski
|
Thanks @kevinnoel-be, I'll assign the issue to you and look forward to your PR.
Since this is a substantial change to the internals of the receiver, I think we must manage this transition with a feature gate. Basically, we'll have to give the user a choice between the libraries. When the gate is alpha,
We can work out details here but I think the following guidelines should apply:
|
Thanks @djaglowski I'll take those in account for creating the PR. Another question that'll drive which version of |
@kevinnoel-be, I think it's probably reasonable to drop support for older versions but this should be a separate issue so others have a chance to comment. |
@djaglowski I've created #30923 to open the discussion around the version support. |
**Description:** <Describe what has changed.> Reuse of connections created per database (configured or discovered) vs current behavior to create & close connection per database on each scrape. **Link to tracking Issue:** #30831 **Testing:** Updated unit & integration tests. Also, ran locally multiple scenario: - no feature gate specified (default): current behavior maintained, connections created/closed on each database per scrape - feature gate connection pool enabled, no connection pool config specified (default): reduction of the number of connections created/closed - feature gate connection pool enabled, connection pool config tweaked: connections created on first scrape & closed when configured lifetime reached or collector shutdown **Documentation:** - change log - readme for the feature gate & related optional configurations linked to this feature **Note** Checking internally for getting the CLA signed
**Description:** <Describe what has changed.> Reuse of connections created per database (configured or discovered) vs current behavior to create & close connection per database on each scrape. **Link to tracking Issue:** open-telemetry#30831 **Testing:** Updated unit & integration tests. Also, ran locally multiple scenario: - no feature gate specified (default): current behavior maintained, connections created/closed on each database per scrape - feature gate connection pool enabled, no connection pool config specified (default): reduction of the number of connections created/closed - feature gate connection pool enabled, connection pool config tweaked: connections created on first scrape & closed when configured lifetime reached or collector shutdown **Documentation:** - change log - readme for the feature gate & related optional configurations linked to this feature **Note** Checking internally for getting the CLA signed
**Description:** <Describe what has changed.> Reuse of connections created per database (configured or discovered) vs current behavior to create & close connection per database on each scrape. **Link to tracking Issue:** open-telemetry#30831 **Testing:** Updated unit & integration tests. Also, ran locally multiple scenario: - no feature gate specified (default): current behavior maintained, connections created/closed on each database per scrape - feature gate connection pool enabled, no connection pool config specified (default): reduction of the number of connections created/closed - feature gate connection pool enabled, connection pool config tweaked: connections created on first scrape & closed when configured lifetime reached or collector shutdown **Documentation:** - change log - readme for the feature gate & related optional configurations linked to this feature **Note** Checking internally for getting the CLA signed
Component(s)
receiver/postgresql
Is your feature request related to a problem? Please describe.
We've been trying to enable the postgresql receiver on our fleet but did encounter a "spam log" issue due to how this receiver manages the connection i.e. connect/disconnect on each scrape per database.
All of our PostgreSQL are configured, for security/compliance reasons, with
log_connections=on
&log_disconnections=on
which makes this receiver very verbose, especially at this scale (2000+ PGs).For example, here is an excerpt of PG logs generated during one basic test scrape:
PG logs
This prevents us using/rolling out this receiver more widely as we cannot drop logs nor disable those log configurations on the PostgreSQL side either.
Describe the solution you'd like
Connection reuse/pooling to avoid having these spike of logs on each scrape interval across our fleet e.g. by using a connection pool (maybe?). Could be an opt-in configuration of course.
Describe alternatives you've considered
Forking the receiver in our internal custom build
Additional context
To get an idea, I've ran the integration test locally with both PG configurations
log_connections
&log_disconnections
enabled:lib/pq
, no reuse of connectionconnection authorized: user=otelu
jackc/pgx/v5/pgxpool
connection authorized: user=otelu
The text was updated successfully, but these errors were encountered: