Add support for pg_stat_statements to MetricBeat Postgresql module #7060

zinefer · 2018-05-09T16:22:42Z

This pull request adds a new metricset, statements to the MetricBeat Postgres module.

This metricset will poll pg_stat_statements and provide per-query statistic information which is not obtainable with just the current activity and database metricsets.

Related Issue #7048

elasticmachine · 2018-05-09T16:22:44Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

houndci-bot · 2018-05-09T16:22:54Z

metricbeat/module/postgresql/statements/statments_integration_test.go

+	assert.Contains(t, event["user"].(common.MapStr), "id")
+
+	assert.Contains(t, event, "database")
+	db_oid := event["database"].(common.MapStr)["oid"].(int64)


don't use underscores in Go names; var db_oid should be dbOid

houndci-bot · 2018-05-09T16:22:55Z

metricbeat/module/postgresql/statements/statments_integration_test.go

+	assert.Contains(t, event["user"].(common.MapStr), "id")
+
+	assert.Contains(t, event, "database")
+	db_oid := event["database"].(common.MapStr)["oid"].(int64)


don't use underscores in Go names; var db_oid should be dbOid

ruflin

Can you add a changelog? Should we also add this to our system tests?

ruflin · 2018-05-11T09:05:08Z

metricbeat/docs/modules/system.asciidoc

@@ -41,6 +39,8 @@ metricbeat.modules:
    - uptime          # System Uptime
    #- core           # Per CPU core usage
    #- diskio         # Disk IO
+    #- filesystem     # File system usage for each mountpoint


You probably need to rebase on master to get rid of this diff.

ruflin · 2018-05-11T09:05:27Z

metricbeat/module/postgresql/statements/_meta/data.json

+        "rtt": 115
+    },
+    "postgresql": {
+        "statements": {


Should the metricset be called statement? Because each even only contains one statement.

I'd agree in general with that, but if we keep it as statements then all metricsets follow the exact naming of pg stats tables (pg_stat_activity, pg_stat_database, pg_stat_bgwriter and pg_stat_statements).

In general I would prefer the the naming convention superseed service specific namings: https://www.elastic.co/guide/en/beats/devguide/current/event-conventions.html This provides a unified querying experience on the consumer side.

Ok, go for statement then.

jsoriano

It mostly LGTM, but I'd add it as beta, and disabled by default.

jsoriano · 2018-05-11T09:32:05Z

metricbeat/module/postgresql/statements/statements.go

+func init() {
+	mb.Registry.MustAddMetricSet("postgresql", "statements", New,
+		mb.WithHostParser(postgresql.ParseURL),
+		mb.DefaultMetricSet(),


Does it generate events on each executed statement? Maybe this is too much data to be enabled by default.

jsoriano · 2018-05-11T09:34:33Z

metricbeat/module/postgresql/statements/statements.go

+// New create a new instance of the MetricSet
+// Part of new is also setting up the configuration by processing additional
+// configuration entries if needed.
+func New(base mb.BaseMetricSet) (mb.MetricSet, error) {


Add this metricset as beta, for that print a warning here with something like:

cfgwarn.Beta("The statement metricset is beta")

And change release to beta in fields.yml.

zinefer · 2018-05-14T17:58:13Z

Thanks for the review. I'll make the suggested changes shortly.

This reverts commit 5bcb9ce This reverts commit c516a4f This reverts commit b812cba

houndci-bot · 2018-05-15T15:37:25Z

metricbeat/module/postgresql/statement/statement_integration_test.go

+  assert.Contains(t, event["user"].(common.MapStr), "id")
+
+  assert.Contains(t, event, "database")
+  db_oid := event["database"].(common.MapStr)["oid"].(int64)


don't use underscores in Go names; var db_oid should be dbOid

zinefer · 2018-05-24T17:29:46Z

Where should I add a changelog? To the pull request decription?

jsoriano

Thanks for addressing the comments, but it seems that some code is missing after the rename of the metricset.

jsoriano · 2018-05-25T08:43:42Z

metricbeat/module/postgresql/statement/_meta/data.json

+    "metricset": {
+        "host": "postgresql:5432",
+        "module": "postgresql",
+        "name": "statements",


statement

jsoriano · 2018-05-25T08:43:52Z

metricbeat/module/postgresql/statement/_meta/data.json

+        "rtt": 115
+    },
+    "postgresql": {
+        "statements": {


statement

jsoriano · 2018-05-25T08:47:46Z

metricbeat/module/postgresql/statement/statement.go

+// Fetch methods implements the data gathering and data conversion to the right
+// format. It publishes the event which is then forwarded to the output. In case
+// of an error set the Error field of mb.Event or simply call report.Error().
+func (m *MetricSet) Fetch(report mb.ReporterV2) {


The actual code for fetching is missing, but it was here in the previous versions 🤔

jsoriano · 2018-05-25T08:50:30Z

@zinefer changelog is in the CHANGELOG.md file in the repository, add a line there and include it in the pull request. You will also need to update your branch with master, there are merge conflicts.

find lost fetch code add changelog message catch rename stragglers

…beat-statements-metricset # Conflicts: # metricbeat/include/fields.go

zinefer · 2018-05-29T19:38:37Z

Thanks for the help everyone!

Hopefully I've added to the changelog correctly.

jsoriano · 2018-05-29T21:02:10Z

CHANGELOG.asciidoc

+==== Added
+
+*MetricBeat*
+- Add postgresql statement metricset. {issue}7048[7048] {pull}7060[7060]


Add it to the existing "Added" section for version HEAD please 🙂

jsoriano · 2018-05-29T21:05:25Z

jenkins, test this please

…beat-statements-metricset # Conflicts: # metricbeat/include/fields.go

jsoriano · 2018-05-31T17:49:25Z

@zinefer there seems to be a related test failing now 🤔 https://travis-ci.org/elastic/beats/jobs/386280791#L3561

zinefer · 2018-05-31T18:11:18Z

Yeah, I'm on it. :)

I need to enable pg_stat_statements in the docker container.

Edit: Having an issue with my environment... docker-compose fails on my machine.

remove unused file

jsoriano · 2018-06-01T08:48:43Z

Oh, tests pass now but there are conflicts in the fields file, could you please update your branch and make update again?

…beat-statements-metricset # Conflicts: # metricbeat/include/fields.go

zinefer · 2018-06-01T13:17:30Z

No problem. :)

jsoriano · 2018-06-01T15:11:10Z

jenkins, test this please

jsoriano · 2018-06-01T15:11:25Z

It looks good, let's wait to jenkins :)

…beat-statements-metricset # Conflicts: # metricbeat/include/fields.go

jsoriano · 2018-06-04T20:22:12Z

jenkins, test this again, please

jsoriano · 2018-06-04T22:18:34Z

@zinefer merged, thanks for your contribution!

Matt-Yorkley · 2019-06-17T13:06:14Z

@zinefer are there additional steps needed to get this working? I'm running it now, and my logs show: [postgresql.log][FATAL] database "metricbeat" does not exist. There's no way to specify another database?

Edit: I restarted everything and it picked up my configs. The error message is gone, but the Postgres metrics are not displayed anywhere...

houndci-bot reviewed May 9, 2018

View reviewed changes

jsoriano added module review Metricbeat Metricbeat labels May 11, 2018

ruflin reviewed May 11, 2018

View reviewed changes

ruflin requested a review from jsoriano May 11, 2018 09:06

jsoriano reviewed May 11, 2018

View reviewed changes

zinefer added 6 commits May 15, 2018 15:30

create statements metricset for the metricbeat postgresql module

b812cba

fix styling

c516a4f

fix styling for travis build

5bcb9ce

run make docs

0d517c0

This reverts commit 0d517c0.

4f76515

This reverts commit 5bcb9ce This reverts commit c516a4f This reverts commit b812cba

init metricbeat/postgresql/statement metricset

45aceea

zinefer force-pushed the feature/metricbeat-statements-metricset branch from 876250f to 45aceea Compare May 15, 2018 15:37

houndci-bot reviewed May 15, 2018

View reviewed changes

zinefer added 3 commits May 24, 2018 17:00

run make fmt

f186a8b

run make collect

80624f6

run make update

689eba5

jsoriano reviewed May 25, 2018

View reviewed changes

zinefer added 3 commits May 29, 2018 19:25

postgresql metricset fixes

8aa871d

find lost fetch code add changelog message catch rename stragglers

Merge branch 'master' of github.com:elastic/beats into feature/metric…

bd3c873

…beat-statements-metricset # Conflicts: # metricbeat/include/fields.go

run make collect, docs, fmt

99a2bfc

run make update

5b8caa9

jsoriano reviewed May 29, 2018

View reviewed changes

zinefer added 3 commits May 31, 2018 17:10

fix changelog

407e939

Merge branch 'master' of github.com:elastic/beats into feature/metric…

6dfac36

…beat-statements-metricset # Conflicts: # metricbeat/include/fields.go

run make update

01aface

zinefer added 3 commits May 31, 2018 19:26

enable pg_stat_statements for integration tests

b5b4581

enable pg_stat_statements for integration tests

51a9af8

bugfix column name

f9d7bd7

remove unused file

zinefer added 2 commits June 1, 2018 13:16

Merge branch 'master' of github.com:elastic/beats into feature/metric…

ddf0b43

…beat-statements-metricset # Conflicts: # metricbeat/include/fields.go

run make update

0c26c63

zinefer added 2 commits June 4, 2018 17:00

Merge branch 'master' of github.com:elastic/beats into feature/metric…

7c2c2ff

…beat-statements-metricset # Conflicts: # metricbeat/include/fields.go

run make update

1fe0057

jsoriano merged commit 7a80be8 into elastic:master Jun 4, 2018

ruflin mentioned this pull request Aug 31, 2018

Add support for pg_stat_statements to MetricBeat Postgresql module #7048

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for pg_stat_statements to MetricBeat Postgresql module #7060

Add support for pg_stat_statements to MetricBeat Postgresql module #7060

zinefer commented May 9, 2018

elasticmachine commented May 9, 2018

houndci-bot May 9, 2018

houndci-bot May 9, 2018

ruflin left a comment

ruflin May 11, 2018

ruflin May 11, 2018

jsoriano May 11, 2018

ruflin May 14, 2018

jsoriano May 14, 2018

jsoriano left a comment

jsoriano May 11, 2018

jsoriano May 11, 2018

zinefer commented May 14, 2018

houndci-bot May 15, 2018

zinefer commented May 24, 2018

jsoriano left a comment

jsoriano May 25, 2018

jsoriano May 25, 2018

jsoriano May 25, 2018

jsoriano commented May 25, 2018

zinefer commented May 29, 2018

jsoriano May 29, 2018

jsoriano commented May 29, 2018

jsoriano commented May 31, 2018

zinefer commented May 31, 2018 •

edited

Loading

jsoriano commented Jun 1, 2018

zinefer commented Jun 1, 2018

jsoriano commented Jun 1, 2018

jsoriano commented Jun 1, 2018

jsoriano commented Jun 4, 2018

jsoriano commented Jun 4, 2018

Matt-Yorkley commented Jun 17, 2019 •

edited

Loading

Add support for pg_stat_statements to MetricBeat Postgresql module #7060

Add support for pg_stat_statements to MetricBeat Postgresql module #7060

Conversation

zinefer commented May 9, 2018

elasticmachine commented May 9, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsoriano left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zinefer commented May 14, 2018

Choose a reason for hiding this comment

zinefer commented May 24, 2018

jsoriano left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsoriano commented May 25, 2018

zinefer commented May 29, 2018

Choose a reason for hiding this comment

jsoriano commented May 29, 2018

jsoriano commented May 31, 2018

zinefer commented May 31, 2018 • edited Loading

jsoriano commented Jun 1, 2018

zinefer commented Jun 1, 2018

jsoriano commented Jun 1, 2018

jsoriano commented Jun 1, 2018

jsoriano commented Jun 4, 2018

jsoriano commented Jun 4, 2018

Matt-Yorkley commented Jun 17, 2019 • edited Loading

zinefer commented May 31, 2018 •

edited

Loading

Matt-Yorkley commented Jun 17, 2019 •

edited

Loading