[ethereum/go-ethereum] geth metrics export to InfluxDB (#16979)

karalabe requested changes on this pull request.

Maybe I missed it, but did you add the flags to the usage template too? `geth help`? If not, they’ll just get dumped at the end.

> @@ -532,6 +534,41 @@ var (
Usage: “Minimum POW accepted”,
Value: whisper.DefaultMinimumPoW,
}
+
+ // Metrics flags
+ MetricsEnableInfluxDBExportFlag = cli.BoolFlag{
+ Name: “metrics.influxdb.export”,

I think `metrics.influxdb` is enough here. It’s mostly obvious what that would do :)

> @@ -532,6 +534,41 @@ var (
Usage: “Minimum POW accepted”,
Value: whisper.DefaultMinimumPoW,
}
+
+ // Metrics flags
+ MetricsEnableInfluxDBExportFlag = cli.BoolFlag{
+ Name: “metrics.influxdb.export”,
+ Usage: “Enable metrics export/push to an external InfluxDB database”,
+ }
+ MetricsInfluxDBEndpointFlag = cli.StringFlag{
+ Name: “metrics.influxdb.endpoint”,
+ Usage: “Metrics InfluxDB endpoint”,

Lets expand the usage a bit (otherwise there’s not much point to have it). E.g.

`InfluxDB API endpoint to report metrics to`

> @@ -532,6 +534,41 @@ var (
Usage: “Minimum POW accepted”,
Value: whisper.DefaultMinimumPoW,
}
+
+ // Metrics flags
+ MetricsEnableInfluxDBExportFlag = cli.BoolFlag{
+ Name: “metrics.influxdb.export”,
+ Usage: “Enable metrics export/push to an external InfluxDB database”,
+ }
+ MetricsInfluxDBEndpointFlag = cli.StringFlag{
+ Name: “metrics.influxdb.endpoint”,
+ Usage: “Metrics InfluxDB endpoint”,
+ Value: “http://localhost:8086”,
+ }
+ MetricsInfluxDBDatabaseFlag = cli.StringFlag{
+ Name: “metrics.influxdb.database”,
+ Usage: “Metrics InfluxDB database”,

Lets expand the usage a bit (otherwise there’s not much point to have it). Perhaps

`InfluxDB database name to group reported metrics by`. ?

> @@ -532,6 +534,41 @@ var (
Usage: “Minimum POW accepted”,
Value: whisper.DefaultMinimumPoW,
}
+
+ // Metrics flags
+ MetricsEnableInfluxDBExportFlag = cli.BoolFlag{
+ Name: “metrics.influxdb.export”,
+ Usage: “Enable metrics export/push to an external InfluxDB database”,
+ }
+ MetricsInfluxDBEndpointFlag = cli.StringFlag{
+ Name: “metrics.influxdb.endpoint”,
+ Usage: “Metrics InfluxDB endpoint”,
+ Value: “http://localhost:8086”,
+ }
+ MetricsInfluxDBDatabaseFlag = cli.StringFlag{
+ Name: “metrics.influxdb.database”,
+ Usage: “Metrics InfluxDB database”,
+ Value: “metrics”,

Would `geth` be perhaps a more meaningful name than `metrics`? Not familiar with how InfluxDB organizes its data, just thinking that if we inject the data into en existing db, `metrics` might be a bit too generic?

> + Name: “metrics.influxdb.export”,
+ Usage: “Enable metrics export/push to an external InfluxDB database”,
+ }
+ MetricsInfluxDBEndpointFlag = cli.StringFlag{
+ Name: “metrics.influxdb.endpoint”,
+ Usage: “Metrics InfluxDB endpoint”,
+ Value: “http://localhost:8086”,
+ }
+ MetricsInfluxDBDatabaseFlag = cli.StringFlag{
+ Name: “metrics.influxdb.database”,
+ Usage: “Metrics InfluxDB database”,
+ Value: “metrics”,
+ }
+ MetricsInfluxDBUsernameFlag = cli.StringFlag{
+ Name: “metrics.influxdb.username”,
+ Usage: “Metrics InfluxDB username”,

“Username to authorize access to the database”

> + Usage: “Metrics InfluxDB endpoint”,
+ Value: “http://localhost:8086”,
+ }
+ MetricsInfluxDBDatabaseFlag = cli.StringFlag{
+ Name: “metrics.influxdb.database”,
+ Usage: “Metrics InfluxDB database”,
+ Value: “metrics”,
+ }
+ MetricsInfluxDBUsernameFlag = cli.StringFlag{
+ Name: “metrics.influxdb.username”,
+ Usage: “Metrics InfluxDB username”,
+ Value: “test”,
+ }
+ MetricsInfluxDBPasswordFlag = cli.StringFlag{
+ Name: “metrics.influxdb.password”,
+ Usage: “Metrics InfluxDB password”,

“Password to authorize access to the database”

> + MetricsInfluxDBUsernameFlag = cli.StringFlag{
+ Name: “metrics.influxdb.username”,
+ Usage: “Metrics InfluxDB username”,
+ Value: “test”,
+ }
+ MetricsInfluxDBPasswordFlag = cli.StringFlag{
+ Name: “metrics.influxdb.password”,
+ Usage: “Metrics InfluxDB password”,
+ Value: “test”,
+ }
+ // The `host` tag is part of every measurement sent to InfluxDB. Queries on tags are faster in InfluxDB.
+ // It is used so that we can group all nodes and average a measurement across all of them, but also so
+ // that we can select a specific node and inspect its measurements.
+ // https://docs.influxdata.com/influxdb/v1.4/concepts/key_concepts/#tag-key
+ MetricsInfluxDBHostTagFlag = cli.StringFlag{
+ Name: “metrics.influxdb.host.tag”,

I think `metrics.influxdb.host` is enough, don’t see the value of explicitly adding `tag`

This post was last modified on June 14, 2018, 12:45 pm