Skip to content

Fetch metrics for multiple databases #88

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ This project adheres to [Semantic Versioning](http://semver.org/).
This CHANGELOG follows the format listed [here](https://github.com/sensu-plugins/community/blob/master/HOW_WE_CHANGELOG.md).

## [Unreleased]
### Changed
- Option `database (-d, --db)` for metrics now supports semicolon separated list of databases to fetch metrics of multiple databases at once.
### Breaking Changes
- If no `database (-d, --db)` argument is supplied for the metric bins through the command line now metrics for all databases are returned.

## [2.3.2] - 2019-03-12
### Fixed
Expand Down
55 changes: 31 additions & 24 deletions bin/metric-postgres-connections.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#
# USAGE:
# ./metric-postgres-connections.rb -u db_user -p db_pass -h db_host -d db
# ./metric-postgres-connections.rb -u db_user -p db_pass -h db_host -d 'db1;db2'
#
# NOTES:
#
Expand All @@ -29,6 +30,7 @@
#

require 'sensu-plugins-postgres/pgpass'
require 'sensu-plugins-postgres/pgdatabases'
require 'sensu-plugin/metric/cli'
require 'pg'
require 'socket'
Expand Down Expand Up @@ -60,10 +62,11 @@ class PostgresStatsDBMetrics < Sensu::Plugin::Metric::CLI::Graphite
short: '-P PORT',
long: '--port PORT'

option :database,
description: 'Database name',
option :databases,
description: 'Database names, separated by ";"',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am thinking it might be safer to pass it with a comma separated list? Is it because we are passing this as raw SQL?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, using "," as separator is better than ";".
Also it was not escaped when used in a query.

I've updated the PR with patches for both.

short: '-d DB',
long: '--db DB'
long: '--db DB',
default: nil

option :scheme,
description: 'Metric naming scheme, text to prepend to $queue_name.$metric',
Expand All @@ -77,12 +80,14 @@ class PostgresStatsDBMetrics < Sensu::Plugin::Metric::CLI::Graphite
default: nil

include Pgpass
include Pgdatabases

def run
timestamp = Time.now.to_i
pgpass
databases = pgdatabases
con = PG.connect(host: config[:hostname],
dbname: config[:database],
dbname: databases.first,
user: config[:user],
password: config[:password],
port: config[:port],
Expand All @@ -96,30 +101,32 @@ def run
]
wait_col = con.exec(request.join(' ')).first['wait_col']

request = [
"select count(*), #{wait_col} as waiting from pg_stat_activity",
"where datname = '#{config[:database]}' group by #{wait_col}"
]

metrics = {
active: 0,
waiting: 0,
total: 0
}
con.exec(request.join(' ')) do |result|
result.each do |row|
if row['waiting'] == 't'
metrics[:waiting] = row['count']
elsif row['waiting'] == 'f'
metrics[:active] = row['count']
databases.each do |database|
request = [
"select count(*), #{wait_col} as waiting from pg_stat_activity",
"where datname = '#{database}' group by #{wait_col}"
]

metrics = {
active: 0,
waiting: 0,
total: 0
}
con.exec(request.join(' ')) do |result|
result.each do |row|
if row['waiting'] == 't'
metrics[:waiting] = row['count']
elsif row['waiting'] == 'f'
metrics[:active] = row['count']
end
end
end
end

metrics[:total] = (metrics[:waiting].to_i + metrics[:active].to_i)
metrics[:total] = (metrics[:waiting].to_i + metrics[:active].to_i)

metrics.each do |metric, value|
output "#{config[:scheme]}.connections.#{config[:database]}.#{metric}", value, timestamp
metrics.each do |metric, value|
output "#{config[:scheme]}.connections.#{database}.#{metric}", value, timestamp
end
end

ok
Expand Down
38 changes: 23 additions & 15 deletions bin/metric-postgres-dbsize.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#
# USAGE:
# ./metric-postgres-dbsize.rb -u db_user -p db_pass -h db_host -d db
# ./metric-postgres-dbsize.rb -u db_user -p db_pass -h db_host -d 'db1;db2'
#
# NOTES:
#
Expand All @@ -29,6 +30,7 @@
#

require 'sensu-plugins-postgres/pgpass'
require 'sensu-plugins-postgres/pgdatabases'
require 'sensu-plugin/metric/cli'
require 'pg'
require 'socket'
Expand Down Expand Up @@ -60,10 +62,11 @@ class PostgresStatsDBMetrics < Sensu::Plugin::Metric::CLI::Graphite
short: '-P PORT',
long: '--port PORT'

option :database,
description: 'Database name',
option :databases,
description: 'Database names, separated by ";"',
short: '-d DB',
long: '--db DB'
long: '--db DB',
default: nil

option :scheme,
description: 'Metric naming scheme, text to prepend to $queue_name.$metric',
Expand All @@ -77,23 +80,28 @@ class PostgresStatsDBMetrics < Sensu::Plugin::Metric::CLI::Graphite
default: nil

include Pgpass
include Pgdatabases

def run
timestamp = Time.now.to_i
pgpass
con = PG.connect(host: config[:hostname],
dbname: config[:database],
user: config[:user],
password: config[:password],
port: config[:port],
connect_timeout: config[:timeout])
request = [
"select pg_database_size('#{config[:database]}')"
]
databases = pgdatabases
con = PG.connect(host: config[:hostname],
dbname: databases.first,
user: config[:user],
password: config[:password],
port: config[:port],
connect_timeout: config[:timeout])

databases.each do |database|
request = [
"select pg_database_size('#{database}')"
]

con.exec(request.join(' ')) do |result|
result.each do |row|
output "#{config[:scheme]}.size.#{config[:database]}", row['pg_database_size'], timestamp
con.exec(request.join(' ')) do |result|
result.each do |row|
output "#{config[:scheme]}.size.#{database}", row['pg_database_size'], timestamp
end
end
end

Expand Down
52 changes: 30 additions & 22 deletions bin/metric-postgres-locks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#
# USAGE:
# ./metric-postgres-locks.rb -u db_user -p db_pass -h db_host -d db
# ./metric-postgres-locks.rb -u db_user -p db_pass -h db_host -d 'db1;db2'
#
# NOTES:
#
Expand All @@ -29,6 +30,7 @@
#

require 'sensu-plugins-postgres/pgpass'
require 'sensu-plugins-postgres/pgdatabases'
require 'sensu-plugin/metric/cli'
require 'pg'
require 'socket'
Expand Down Expand Up @@ -60,10 +62,11 @@ class PostgresStatsDBMetrics < Sensu::Plugin::Metric::CLI::Graphite
short: '-P PORT',
long: '--port PORT'

option :database,
description: 'Database name',
option :databases,
description: 'Database names, separated by ";"',
short: '-d DB',
long: '--db DB'
long: '--db DB',
default: nil

option :scheme,
description: 'Metric naming scheme, text to prepend to $queue_name.$metric',
Expand All @@ -77,33 +80,38 @@ class PostgresStatsDBMetrics < Sensu::Plugin::Metric::CLI::Graphite
default: nil

include Pgpass
include Pgdatabases

def run
timestamp = Time.now.to_i

locks_per_type = Hash.new(0)
pgpass
con = PG.connect(host: config[:hostname],
dbname: config[:database],
user: config[:user],
password: config[:password],
port: config[:port],
connect_timeout: config[:timeout])
request = [
'SELECT mode, count(mode) AS count FROM pg_locks',
"WHERE database = (SELECT oid FROM pg_database WHERE datname = '#{config[:database]}')",
'GROUP BY mode'
]

con.exec(request.join(' ')) do |result|
result.each do |row|
lock_name = row['mode'].downcase.to_sym
locks_per_type[lock_name] = row['count']
databases = pgdatabases
con = PG.connect(host: config[:hostname],
dbname: databases.first,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why only the first database?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first database from the list is simply used to make a valid connection and the locks statistics are then queried through SQL statements for all databases. It is simply not necessary to make individual connections for each database. Please see line 99-103 for the queries run for each database.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah gotcha, that makes sense, would not hurt to add a comment about that. I will review this in greater detail when I have more time. Is there any way we can provide backwards compatibility?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I currently see two possible options:

Option 1:
Require the user to set the --db option to * if all databases should be checked. If it is not set to * the current behaviour is retained.
But in lib/sensu-plugins-postgres/pgpass.rb there is a check for * to switch to fallbacks and to be consistent the switch to fallbacks should no longer be done which will also introduce a breaking change (with less users affected).

Option 2:
Add a new option e.g. --all-databases which will override the --db option. This means the user has to specify the --all-databases option if he really would like to get metrics for all databases or use the existing --db option to list database names explicitly.

Also Option 1 has the advantage over Option 2 that it is easier to parameterize the check through tokens if there is no option to invalidate/override another.

Let me know what you think or if you have another idea.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its a trade off between backwards compatibility and code readability I tend to prefer to not break users regardless of how small the numbers are unless there is a compelling reason.

user: config[:user],
password: config[:password],
port: config[:port],
connect_timeout: config[:timeout])

databases.each do |database|
request = [
'SELECT mode, count(mode) AS count FROM pg_locks',
"WHERE database = (SELECT oid FROM pg_database WHERE datname = '#{database}')",
'GROUP BY mode'
]

con.exec(request.join(' ')) do |result|
result.each do |row|
lock_name = row['mode'].downcase.to_sym
locks_per_type[lock_name] = row['count']
end
end
end

locks_per_type.each do |lock_type, count|
output "#{config[:scheme]}.locks.#{config[:database]}.#{lock_type}", count, timestamp
locks_per_type.each do |lock_type, count|
output "#{config[:scheme]}.locks.#{database}.#{lock_type}", count, timestamp
end
end

ok
Expand Down
1 change: 1 addition & 0 deletions bin/metric-postgres-statsdb.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#
# USAGE:
# ./metric-postgres-statsdb.rb -u db_user -p db_pass -h db_host -d db
# ./metric-postgres-statsdb.rb -u db_user -p db_pass -h db_host -d 'db1;db2;db3'
#
# NOTES:
# Requires PSQL `track_counts` `track_io_timing` for some metrics enabled
Expand Down
51 changes: 33 additions & 18 deletions bin/metric-postgres-statsio.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#
# USAGE:
# ./metric-postgres-statsio.rb -u db_user -p db_pass -h db_host -d db -s scope
# ./metric-postgres-statsio.rb -u db_user -p db_pass -h db_host -d 'db1;db2' -s scope
#
# NOTES:
# Requires PSQL `track_io_timing` enabled
Expand All @@ -30,6 +31,7 @@
#

require 'sensu-plugins-postgres/pgpass'
require 'sensu-plugins-postgres/pgdatabases'
require 'sensu-plugin/metric/cli'
require 'pg'
require 'socket'
Expand Down Expand Up @@ -61,10 +63,11 @@ class PostgresStatsIOMetrics < Sensu::Plugin::Metric::CLI::Graphite
short: '-P PORT',
long: '--port PORT'

option :database,
description: 'Database name',
option :databases,
description: 'Database names, separated by ";"',
short: '-d DB',
long: '--db DB'
long: '--db DB',
default: nil

option :scope,
description: 'Scope, see http://www.postgresql.org/docs/9.2/static/monitoring-stats.html',
Expand All @@ -84,16 +87,28 @@ class PostgresStatsIOMetrics < Sensu::Plugin::Metric::CLI::Graphite
default: nil

include Pgpass
include Pgdatabases

def run
timestamp = Time.now.to_i
pgpass
con = PG.connect(host: config[:hostname],
dbname: config[:database],
user: config[:user],
password: config[:password],
port: config[:port],
connect_timeout: config[:timeout])

pgdatabases.each do |database|
output_database(database, timestamp)
end

ok
end

private

def output_database(database)
con = PG.connect(host: config[:hostname],
dbname: database,
user: config[:user],
password: config[:password],
port: config[:port],
connect_timeout: config[:timeout])
request = [
'select sum(heap_blks_read) as heap_blks_read, sum(heap_blks_hit) as heap_blks_hit,',
'sum(idx_blks_read) as idx_blks_read, sum(idx_blks_hit) as idx_blks_hit,',
Expand All @@ -103,17 +118,17 @@ def run
]
con.exec(request.join(' ')) do |result|
result.each do |row|
output "#{config[:scheme]}.statsio.#{config[:database]}.heap_blks_read", row['heap_blks_read'], timestamp
output "#{config[:scheme]}.statsio.#{config[:database]}.heap_blks_hit", row['heap_blks_hit'], timestamp
output "#{config[:scheme]}.statsio.#{config[:database]}.idx_blks_read", row['idx_blks_read'], timestamp
output "#{config[:scheme]}.statsio.#{config[:database]}.idx_blks_hit", row['idx_blks_hit'], timestamp
output "#{config[:scheme]}.statsio.#{config[:database]}.toast_blks_read", row['toast_blks_read'], timestamp
output "#{config[:scheme]}.statsio.#{config[:database]}.toast_blks_hit", row['toast_blks_hit'], timestamp
output "#{config[:scheme]}.statsio.#{config[:database]}.tidx_blks_read", row['tidx_blks_read'], timestamp
output "#{config[:scheme]}.statsio.#{config[:database]}.tidx_blks_hit", row['tidx_blks_hit'], timestamp
output "#{config[:scheme]}.statsio.#{database}.heap_blks_read", row['heap_blks_read'], timestamp
output "#{config[:scheme]}.statsio.#{database}.heap_blks_hit", row['heap_blks_hit'], timestamp
output "#{config[:scheme]}.statsio.#{database}.idx_blks_read", row['idx_blks_read'], timestamp
output "#{config[:scheme]}.statsio.#{database}.idx_blks_hit", row['idx_blks_hit'], timestamp
output "#{config[:scheme]}.statsio.#{database}.toast_blks_read", row['toast_blks_read'], timestamp
output "#{config[:scheme]}.statsio.#{database}.toast_blks_hit", row['toast_blks_hit'], timestamp
output "#{config[:scheme]}.statsio.#{database}.tidx_blks_read", row['tidx_blks_read'], timestamp
output "#{config[:scheme]}.statsio.#{database}.tidx_blks_hit", row['tidx_blks_hit'], timestamp
end
end

ok
con.close
end
end
Loading