[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`

Добавить комментарий