-
Notifications
You must be signed in to change notification settings - Fork 754
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
Versions in queries.yaml #553
Conversation
Signed-off-by: vl4deee11 <vl4deee11@gmail.com>
b5c5125
to
bc3ef78
Compare
Signed-off-by: vl4deee11 <vl4deee11@gmail.com>
aab345e
to
a38d5f0
Compare
@sysadmind Check this PR please =) |
@vl4deee11 you're awesome. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are planning to remove user defined queries from the exporter. It's not a very straightforward way to use the exporter and the generic sql exporter supports user defined queries and is the preferred path for custom queries.
That said, I'm not very familiar with the pg_stat_statements table. It seems that it could be part of a collector in the exporter itself. I think that it would be disabled by default because it would lead to a very high cardinality but users could enable it with a flag.
@sysadmind I don't fully understand what you want to do, why not just let users version their requests at this stage? |
Then what is the purpose of postgres_exporter except a set of SQL queries for sql_exporter ? |
May I try to explain the case? When you want to "explain" the structure of the DB workload i.e. to know which queries consume the most CPU time and/or (from the other side) which queries do the most "calls" - you have no options except pg_stat_statements. Yes, there is the "cardinality" issue but if you need that information - you have no other way. I.e we're discussing here not the "user-defined queries" but the ability to "cover" the heterogenous infrastructure (in the meaning of different pg versions) by one exporter. As for me, that makes sense. |
The purpose of postgres_exporter is to provide monitoring of PostgreSQL servers internal metrics. Providing a simple easy deployment with minimal configuration necessary. For example, rather than have a bunch of heavily versioned queries.yaml, the exporter would be able to detect the version of PG and automatically send the correctly formatted query. We do similar things in the mysqld_exporter. We detect column formats for various internal tables and map them correctly to metrics that make more sense from a monitoring perspective. |
The basic queries are defined in the queries.yaml file, this task was performed for early use of versions, or one config for two versions or more, and it seems it would be convenient) |
Yes, that's most of the problem. For monitoring PostgreSQL, users should not need to worry about queries.yaml. Everything will be automatic without it. |
I know that there are a bunch of useful queries today in queries.yaml, and I don't plan to ignore the value that they bring. My plan is to bring those queries into the exporter itself. That's easier to maintain for us from a code perspective. I know that users depend on those metrics, so I want to find a way to expose those metrics while also making this project more maintainable for the long term. Also, I previously mentioned the generic sql exporter, but that project seems to be unmaintained right now. There is a fork that seems to be much more current here: https://github.com/burningalchemist/sql_exporter |
I was discussing with @burningalchemist about bringing that repo into the Prometheus Community org. |
Yup, I'm down to it. 👍 There's some chore to be done beforehand, but overall I'm happy to transfer the repo. 🙂 |
Fix issue #502
Reopen from #543 for integrating with new code structure
Added template for breaking changes
This will help users redefine the fields for their specific version without editing the project code