Skip to content

Conversation

regeda
Copy link
Contributor

@regeda regeda commented Sep 4, 2018

Hi Everyone!
I've added an ability to define different data sources for one exporter setup.

Changes:

  1. "server" label is added to each exported metric. "server" label is parsed from DSN and represents "host:port" pair.
  2. To configure different data source you should list them in DATA_SOURCE_NAME separated by the comma.
  3. I've added disable-settings-metrics flag to the command line in order to avoid panics in Prometheus client when different Postgres versions return different descriptions for settings.

@regeda regeda mentioned this pull request Sep 4, 2018
@coveralls
Copy link

coveralls commented Sep 4, 2018

Coverage Status

Coverage increased (+0.9%) to 59.463% when pulling 2c47952 on gojuno:multi-server-exporter into 72446a5 on wrouesnel:master.

@regeda regeda force-pushed the multi-server-exporter branch from 103c12c to dcbf7b2 Compare September 4, 2018 10:31
@wrouesnel
Copy link
Contributor

wrouesnel commented Sep 22, 2018

This is good work. I'll need to give this a careful review - the need to disable the settings metrics makes me a bit wary of this type of change. There's some tension here against the idea that multi-server scraping should instead be driven by the Prometheus server and this exporter should act as a proxy for it.

EDIT: I'm thinking I may merge this onto a branch (since the CI builds and publishes docker images for those) and add a README.md comment about where people can try it out (as my responsiveness may indicate, I haven't had the most time to review PRs lately).

@uspen uspen mentioned this pull request Oct 4, 2018
@gle4er gle4er mentioned this pull request Nov 8, 2018
@regeda regeda force-pushed the multi-server-exporter branch from dcbf7b2 to d9acb54 Compare December 17, 2018 22:12
@regeda
Copy link
Contributor Author

regeda commented Dec 17, 2018

Hey @wrouesnel
I rebased the PR. The theme is hot. Somebody wants to scrape different instances from the one setup and somebody wants to disable the settings metrics. Pls, look the references above.
The change is backward compatible with the basic idea. The update is safe.
I think we should merge.

@regeda regeda force-pushed the multi-server-exporter branch from d9acb54 to 0e33dfe Compare December 17, 2018 22:42
@regeda regeda force-pushed the multi-server-exporter branch from 0e33dfe to 2c47952 Compare December 18, 2018 11:30
@wrouesnel
Copy link
Contributor

@regeda Thanks very much for the PR! You are right, it's an in-demand feature and passing test-suites make me happy. Merging this PR to master now.

I'll hold off on creating a new explicit release till I get more of a chance to play with it, but it'll go out under the wrouesnel/postgres_exporter:latest image (which is the point of that tag for this repo anyway)

@wrouesnel wrouesnel merged commit 1d6a733 into prometheus-community:master Dec 18, 2018
@uspen
Copy link

uspen commented Dec 19, 2018

Thank you for good work!

Can you add custom label or cluster_name from pg_settings for every metric by DSN?

@regeda
Copy link
Contributor Author

regeda commented Dec 20, 2018

@uspen Pls, open the issue. I will support it.

@regeda regeda deleted the multi-server-exporter branch December 20, 2018 09:42
@karnatinaresh
Copy link

@regeda
Can i define multiple endpoints with

DATA_SOURCE_URI
DATA_SOURCE_USER
DATA_SOURCE_PASS

instead of DATA_SOURCE_NAME

@regeda
Copy link
Contributor Author

regeda commented Feb 21, 2019

@karnatinaresh Unfortunately, multiple endpoints might be configured using DATA_SOURCE_NAME only

@karnatinaresh
Copy link

@regeda Thanks

@mlushpenko
Copy link

mlushpenko commented Feb 26, 2019

@regeda I tried multiple servers via URI and looks like it has problem with splitting a string:

time="2019-02-26T13:54:00Z" level=info msg="Established new database connection." source="postgres_exporter.go:1035"
time="2019-02-26T13:54:00Z" level=info msg="Error while closing non-pinging DB connection: <nil>" source="postgres_exporter.go:1041"
time="2019-02-26T13:54:00Z" level=info msg="Error opening connection to database (postgres://export:PASSWORD_REMOVED@localhost:5432/auth?sslmode=disable,postgres://wallets:test@localhost:5432/wallets?sslmode=disable): pq: unsupported sslmode \"disable,postgres://wallets:test@localhost:5432/wallets?sslmode=disable\"; only \"require\" (default), \"verify-full\", \"verify-ca\", and \"disable\" supported" source="postgres_exporter.go:1070"
time="2019-02-26T13:54:00Z" level=info msg="Starting Server: :9187" source="postgres_exporter.go:1178"

Also, doesn't hide password in second database string

Here are my settings:

- name: DATA_SOURCE_NAME
  value: "postgres://export:password@localhost:5432/auth?sslmode=disable,postgres://wallets:test@localhost:5432/wallets?sslmode=disable"

image: wrouesnel/postgres_exporter:v0.4.7

@regeda
Copy link
Contributor Author

regeda commented Feb 27, 2019

@mlushpenko Unfortunately, v0.4.7 doesn't contain that feature. Use master branch instead.

@mlushpenko
Copy link

mlushpenko commented Feb 27, 2019

Thanks you @regeda, no more problems with connection string. Now, with multiple databases I can't use simple queries, getting:

pq: relation \"users\" does not exist" source="postgres_exporter.go:1166"
pq: relation \"balances\" does not exist" source="postgres_exporter.go:1166"

It's probably a newbie question, but do you have idea how shall I reference tables with multiple connections? I tried:
users , public.users and bp-auth.public.users where bp-auth is database name

And when tried "bp-auth".public.users got:

cross-database references are not implemented

I am getting this error in pgadmin if I am trying to run query on database A from database B. So, wondering how do you map SQL queries from queries.yml with specific connection string?

@regeda
Copy link
Contributor Author

regeda commented Feb 27, 2019

The exporter establishes a separate connection for each data source. If a driver configured to connect with a database "XXX" then you can read relations existed in "XXX" database only.

@mlushpenko
Copy link

HI @regeda separate connection sounds good. Could please clarify about driver? or give example of querying multiple databases with different tables from queries.yml? Maybe I can provide PGadmin screenshot as some help?

@regeda
Copy link
Contributor Author

regeda commented Mar 4, 2019

@mlushpenko I'm not sure what do you want. A driver follows the database rules. You need to understand how Postgres works before diving into Go driver.

@codestation
Copy link

I think he means how to specify the connection to use in queries.yml. For example if i define two queries but they are meant to different databases, how i restrict one query for one of the configured connections?

@mlushpenko
Copy link

Thanks @codestation - exactly what I mean. Also, same thing here #224 (comment). @regeda are we all missing something or can I give another example?

If functionality for multiple databases was added, I don't understand why some example of queries weren't added. Maybe it's as simple as missing documentation or maybe I (and other people here) don't understand what was the purpose of separate connections if queries can work only for single connection.

@regeda
Copy link
Contributor Author

regeda commented Mar 11, 2019

@mlushpenko As I understand you want to restrict queries by a specific database. Do not scrape all available schemas. Is it?

@mlushpenko
Copy link

Yes, correct, thank you

@karnatinaresh
Copy link

karnatinaresh commented Aug 13, 2019

@regeda
Hi,
I am using Vault to get db secrets for which i need a shell to expose these secrets as env variables but i unable to run a shell in this container.
Should I build a docker image with a different version of debian.

@regeda
Copy link
Contributor Author

regeda commented Aug 14, 2019

@karnatinaresh I think you could build a docker image with any version of linux dist.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants