Skip to content

Commit

Permalink
Remove redundant postgres exporter custom queries
Browse files Browse the repository at this point in the history
Postgres-exporter 13.0 moved queries from the default
queries.yaml to collectors.

The newly implemented collectors conflict with the
custom queries.yaml from Omnibus, causing errors
like:

```
An error has occurred while serving metrics:

collected metric pg_replication_is_replica gauge:{value:0} has help
"Indicates if the server is a replica" but should have "Indicates if
this host is a slave"
```

Upstream MR:
  prometheus-community/postgres_exporter#801

Closes https://gitlab.com/gitlab-org/omnibus-gitlab/-/issues/8164

Closes https://gitlab.com/gitlab-org/omnibus-gitlab/-/issues/8112

Changelog: fixed
  • Loading branch information
Clemens Beck authored and Robert Marshall committed Sep 14, 2023
1 parent c74c850 commit bec8913
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 106 deletions.
6 changes: 4 additions & 2 deletions files/gitlab-config-template/gitlab.rb.template
Original file line number Diff line number Diff line change
Expand Up @@ -2287,14 +2287,16 @@ external_url 'GENERATED_EXTERNAL_URL'
# postgres_exporter['home'] = '/var/opt/gitlab/postgres-exporter'
# postgres_exporter['log_directory'] = '/var/log/gitlab/postgres-exporter'
# postgres_exporter['log_group'] = nil
# postgres_exporter['flags'] = {}
# postgres_exporter['flags'] = {
# 'collector.stat_user_tables' => false,
# 'collector.postmaster' => true
# }
# postgres_exporter['listen_address'] = 'localhost:9187'
# postgres_exporter['env_directory'] = '/opt/gitlab/etc/postgres-exporter/env'
# postgres_exporter['env'] = {
# 'SSL_CERT_DIR' => "/opt/gitlab/embedded/ssl/certs/"
# }
# postgres_exporter['sslmode'] = nil
# postgres_exporter['per_table_stats'] = false

##! Service name used to register Postgres Exporter as a Consul service
# postgres_exporter['consul_service_name'] = 'postgres-exporter'
Expand Down
6 changes: 5 additions & 1 deletion files/gitlab-cookbooks/monitoring/attributes/default.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,12 @@
default['monitoring']['postgres_exporter']['env'] = {
'SSL_CERT_DIR' => "#{node['package']['install-dir']}/embedded/ssl/certs/"
}
default['monitoring']['postgres_exporter']['flags'] = {
'collector.stat_user_tables' => false,
'collector.postmaster' => true
}
default['monitoring']['postgres_exporter']['sslmode'] = nil
default['monitoring']['postgres_exporter']['per_table_stats'] = false
default['monitoring']['postgres_exporter']['per_table_stats'] = nil
default['monitoring']['postgres_exporter']['consul_service_name'] = 'postgres-exporter'
default['monitoring']['postgres_exporter']['consul_service_meta'] = nil

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@

node.default['monitoring']['postgres_exporter']['env']['DATA_SOURCE_NAME'] = "#{postgres_exporter_connection_string} " \
"database=#{postgres_exporter_database}"

deprecated_per_table_stats = node['monitoring']['postgres_exporter']['per_table_stats']
node.override['monitoring']['postgres_exporter']['flags']['collector.stat_user_tables'] = deprecated_per_table_stats unless deprecated_per_table_stats.nil?
include_recipe 'postgresql::user'

# Create log_directory
Expand Down
Original file line number Diff line number Diff line change
@@ -1,97 +1,3 @@
pg_replication:
query: "SELECT EXTRACT(EPOCH FROM (now() - pg_last_xact_replay_timestamp()))::INT as lag, CASE WHEN pg_is_in_recovery() THEN 1 ELSE 0 END as is_replica"
metrics:
- lag:
usage: "GAUGE"
description: "Replication lag behind master in seconds"
- is_replica:
usage: "GAUGE"
description: "Indicates if this host is a slave"

pg_postmaster:
query: "SELECT pg_postmaster_start_time as start_time_seconds from pg_postmaster_start_time()"
metrics:
- start_time_seconds:
usage: "GAUGE"
description: "Time at which postmaster started"

<% if node['monitoring']['postgres_exporter']['per_table_stats'] -%>
pg_stat_user_tables:
query: |
SELECT
current_database() datname,
schemaname,
relname,
seq_scan,
seq_tup_read,
idx_scan,
idx_tup_fetch,
n_tup_ins,
n_tup_upd,
n_tup_del,
n_tup_hot_upd,
n_live_tup,
n_dead_tup,
GREATEST(last_autovacuum, last_vacuum, '1970-01-01Z') as last_vacuum,
GREATEST(last_autoanalyze, last_analyze, '1970-01-01Z') as last_analyze,
(vacuum_count + autovacuum_count) as vacuum_count,
(analyze_count + autoanalyze_count) as analyze_count
FROM
pg_stat_user_tables
metrics:
- datname:
usage: "LABEL"
description: "Name of current database"
- schemaname:
usage: "LABEL"
description: "Name of the schema that this table is in"
- relname:
usage: "LABEL"
description: "Name of this table"
- seq_scan:
usage: "COUNTER"
description: "Number of sequential scans initiated on this table"
- seq_tup_read:
usage: "COUNTER"
description: "Number of live rows fetched by sequential scans"
- idx_scan:
usage: "COUNTER"
description: "Number of index scans initiated on this table"
- idx_tup_fetch:
usage: "COUNTER"
description: "Number of live rows fetched by index scans"
- n_tup_ins:
usage: "COUNTER"
description: "Number of rows inserted"
- n_tup_upd:
usage: "COUNTER"
description: "Number of rows updated"
- n_tup_del:
usage: "COUNTER"
description: "Number of rows deleted"
- n_tup_hot_upd:
usage: "COUNTER"
description: "Number of rows HOT updated (i.e., with no separate index update required)"
- n_live_tup:
usage: "GAUGE"
description: "Estimated number of live rows"
- n_dead_tup:
usage: "GAUGE"
description: "Estimated number of dead rows"
- last_vacuum:
usage: "GAUGE"
description: "Last time at which this table was vacuumed (not counting VACUUM FULL)"
- last_analyze:
usage: "GAUGE"
description: "Last time at which this table was analyzed"
- vacuum_count:
usage: "COUNTER"
description: "Number of times this table has been vacuumed"
- analyze_count:
usage: "COUNTER"
description: "Number of times this table has been analyzed"

<% end -%>
pg_total_relation_size:
query: |
SELECT relnamespace::regnamespace as schemaname,
Expand Down
8 changes: 7 additions & 1 deletion files/gitlab-cookbooks/package/libraries/deprecations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,13 @@ def list(existing_config = nil)
deprecation: '15.9',
removal: '17.0',
note: "Starting with GitLab 17.0, running Sidekiq with negate (`sidekiq['negate'] = true`) will be removed. We recommend to follow the steps at https://docs.gitlab.com/ee/administration/sidekiq/extra_sidekiq_processes.html#start-multiple-processes, to run Sidekiq with multiple processes while listening to all queues."
}
},
{
config_keys: %w(postgres_exporter per_table_stats),
deprecation: '16.4', # Remove message issue: https://gitlab.com/gitlab-org/omnibus-gitlab/-/issues/8170
removal: '17.0', # Removal issue: https://gitlab.com/gitlab-org/omnibus-gitlab/-/issues/8169
note: "Starting with GitLab 17.0, this directive will be controlled by `postgres_exporter['flags'] = { 'collector.stat_user_tables' => bool }`."
},
]

deprecations += praefect_legacy_configuration_deprecations
Expand Down
23 changes: 16 additions & 7 deletions spec/chef/cookbooks/monitoring/recipes/postgres_exporter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@
it 'creates the queries.yaml file' do
expect(chef_run).to render_file('/var/opt/gitlab/postgres-exporter/queries.yaml')
.with_content { |content|
expect(content).to match(/pg_replication:/)
expect(content).not_to match(/pg_stat_user_table:/)
expect(content).to match(/pg_total_relation_size:/)
expect(content).to match(/pg_blocked:/)
}
end

Expand All @@ -110,6 +110,13 @@
expect(content).to match(/extend.query-path=\/var\/opt\/gitlab\/postgres-exporter\/queries.yaml/)
}
end

it 'does disable user stats' do
expect(chef_run).to render_file('/opt/gitlab/sv/postgres-exporter/run')
.with_content { |content|
expect(content).to match(/no-collector.stat_user_tables/)
}
end
end

context 'when enabled and run as an isolated recipe' do
Expand Down Expand Up @@ -164,11 +171,6 @@
.with_content(/some.flag=foo/)
end

it 'creates the queries.yaml file' do
expect(chef_run).to render_file('/var/opt/gitlab/postgres-exporter/queries.yaml')
.with_content(/pg_stat_user_tables:/)
end

it 'creates necessary env variable files' do
expect(chef_run).to create_env_dir('/opt/gitlab/etc/postgres-exporter/env').with_variables(
default_vars.merge(
Expand All @@ -180,6 +182,13 @@
)
)
end

it 'does not disable user stats' do
expect(chef_run).to render_file('/opt/gitlab/sv/postgres-exporter/run')
.with_content { |content|
expect(content).not_to match(/no-collector.stat_user_tables/)
}
end
end

context 'log directory and runit group' do
Expand Down

0 comments on commit bec8913

Please sign in to comment.