Add some metrics to the postgres check #1373
Conversation
|
Thanks, looks like the pg tests are failing. Can you fix accordingly please ? |
DorianZaccaria
force-pushed
the
dorian/postgres
branch
from
63d555f to
1d5bc28
Compare
|
@DorianZaccaria can you rebase your branch as i can't merge it for now. |
DorianZaccaria
force-pushed
the
dorian/postgres
branch
from
e10c362 to
d10bd65
Compare
|
@remh Any updates for this one? |
| if self._is_9_1_or_above(key,db): | ||
| # Only available for >= 9.2 due to | ||
| # pg_xlog_location_diff | ||
| if self._is_9_2_or_above(key,db): |
There was a problem hiding this comment.
Looks like you will break replication metrics for PG 9.1 users.
You should append the replication_delay_bytes metrics where PG >= 9.2 and keep replication_delay in case of 9.1
There was a problem hiding this comment.
I'll split this one.
DorianZaccaria
force-pushed
the
dorian/postgres
branch
2 times, most recently
from
e4912eb to
ea85e41
Compare
| self.assertTrue(len([m for m in metrics if m[0] == u'postgresql.percent_usage_connections']) >= 1, pprint(metrics)) | ||
| self.assertTrue(len([m for m in metrics if m[0] == u'postgresql.total_tables']) >= 1, pprint(metrics)) | ||
| self.assertTrue(len([m for m in metrics if m[0] == u'postgresql.db.count']) == 1, pprint(metrics)) | ||
| self.assertTrue(len([m for m in metrics if m[0] == u'postgresql.total_tables']) == 1, pprint(metrics)) |
There was a problem hiding this comment.
Looks like you are asserting twice this total_tables metric.
|
@DorianZaccaria looks good except the few nitpicks |
DorianZaccaria
force-pushed
the
dorian/postgres
branch
from
ea85e41 to
1b1bd47
Compare
| if self._is_9_1_or_above(key, db) and metrics is None: | ||
| self.replication_metrics[key] = dict(self.REPLICATION_METRICS_9_1) | ||
| if self._is_9_2_or_above(key, db): | ||
| self.replication_metrics.update(self.REPLICATION_METRICS_9_2) |
There was a problem hiding this comment.
shouldn't it be:
self.replication_metrics[key].update(self.REPLICATION_METRICS_9_2)?
Also this seems a bit weird, as you're then overriding a constant (self.REPLICATION_METRICS), which in result will be applied to all instances. So your code won't work in the (weird) case where you have multiples instances of postgres running different versions.
I think _get_replication_metrics should just return the replication metrics (as the name of the method seems to indicate)
|
@DorianZaccaria i made one last comment but it should be good after this one! Thanks a lot ! |
* Add postgresql.replication_delay_bytes metric * Add postgresql.total_tables metric * Update postgresql.replication_delay because it wasn't accurate * Update the test suite
DorianZaccaria
force-pushed
the
dorian/postgres
branch
from
1b1bd47 to
bee8eec
Compare
|
Looks great! Waiting for the tests to finish and will merge. Thanks a lot @DorianZaccaria ! |
|
🇫🇷 🍰 👍 💥 |
Add some metrics to the postgres check
|
Yay! |
|
|
Improve postgresql.replication_delay, add postgresql.replication_delay_bytes and postgresql.total_tables.