Skip to content

Add gRPC performance Prometheus exporter. - #3134

Merged
Roasbeef merged 2 commits into
lightningnetwork:masterfrom
valentinewallace:grpc-perf-interceptor
Jun 12, 2019
Merged

Roasbeef merged 2 commits into
lightningnetwork:masterfrom
valentinewallace:grpc-perf-interceptor

Conversation

@valentinewallace

Copy link
Copy Markdown
Contributor

Optionally export lnd gRPC performance metrics for use by a Prometheus server through gRPC interceptors.

If the user compiles lnd with the monitoring tag, the exporter will automatically start listening for Prometheus requests when lnd starts up. The user has the option of specifying an address for the exporter to listen on, with the default being localhost:8989.

Comment thread monitoring/config.go Outdated
@@ -0,0 +1,9 @@
package monitoring

// PrometheusConfig is the set of configuration data that specifies

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, this can live in lncfg and have two versions: one with w/ the build tag, and one w/o it. This way, if the user doesn't have this tag enabled, then it won't show up in lnd -h.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, fixed.

Comment thread monitoring/monitoring_on.go Outdated
Comment thread rpcserver.go Outdated
Comment thread rpcserver.go Outdated
case macService != nil:
unaryInterceptor = macService.UnaryServerInterceptor(permissions)
streamInterceptor = macService.StreamServerInterceptor(permissions)
case monitoring.Enabled:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than check this manually here independent of the build tag, I think this can be melded in with the current monitoring.Start method.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, server options need to be set before any services can register with the grpc server. Lmk if my solution works.

@valentinewallace
valentinewallace force-pushed the grpc-perf-interceptor branch 2 times, most recently from 0c2ca1b to 9fc1a34 Compare May 31, 2019 19:52
Comment thread monitoring/monitoring_on.go Outdated
Comment thread lncfg/monitoring_off.go Outdated
Comment thread monitoring/monitoring_on.go Outdated
Comment thread lncfg/monitoring_off.go Outdated
Comment thread lncfg/monitoring_on.go Outdated
Comment thread lncfg/monitoring_off.go Outdated
Comment thread monitoring/monitoring_on.go Outdated
Comment thread monitoring/monitoring_on.go Outdated
@valentinewallace
valentinewallace force-pushed the grpc-perf-interceptor branch 3 times, most recently from b409d45 to 5e8a404 Compare June 6, 2019 08:47

@Roasbeef Roasbeef left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diff looking much better now IMO, few more final comments, and then I think this is ready to land

Comment thread monitoring/monitoring_on.go Outdated
Comment thread monitoring/monitoring_on.go Outdated
Comment thread rpcserver.go Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@valentinewallace
valentinewallace force-pushed the grpc-perf-interceptor branch 3 times, most recently from 858a13c to 9b44375 Compare June 6, 2019 23:11
Comment thread lncfg/monitoring_on.go Outdated
@valentinewallace
valentinewallace force-pushed the grpc-perf-interceptor branch 2 times, most recently from 9c367ca to 920919d Compare June 7, 2019 17:50
Comment thread monitoring/monitoring_off.go Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this method possibly return an error if called on !monitoring build?

Start the Prometheus exporter in rpcserver.go if monitoring is enabled through the
build tag. Also allow users to specify what address they want the Prometheus
exporter to be listening on.

@Roasbeef Roasbeef left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👾

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants