-
Notifications
You must be signed in to change notification settings - Fork 267
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add prometheus metrics support #180
Conversation
main.go
Outdated
@@ -286,6 +290,10 @@ func run(args []string) error { | |||
if *metricsURL != "" { | |||
logger.Printf("metrics enabled; reporting metrics via POST to %s", *metricsURL) | |||
} | |||
// Always enable prometheus registry. The overhead should be quite minimal as an in-mem map is updated | |||
// with the values. | |||
pClient := prometheusmetrics.NewPrometheusProvider(metrics.DefaultRegistry, "ghostunnel", "", prometheus.DefaultRegisterer, 1*time.Second) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the “ghostunnel” string used for here? Depending on how this is used the metrics-prefix (defaults to “ghostunnel”) flag might make more sense here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Thanks @gouthamve! Code looks good to me apart from the comment I left. Also I’m traveling w/o my laptop and so can’t do a release right now, but I’ll make sure to put tag this in the next version when I’m back home. |
Signed-off-by: Goutham Veeramachaneni <[email protected]>
5d840f8
to
eae156c
Compare
@csstaub Thanks for the review! I did notice the flag before and it fits perfectly. I am in no hurry for a release as I'll be building it myself for now. Have fun on your travels :) |
Fixes: #178
➜ ghostunnel git:(add-prometheus-support) curl -k --cacert test-keys/cacert.pem