Skip to content
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 metrics collector #87

Merged
merged 6 commits into from
Mar 18, 2019
Merged

Add metrics collector #87

merged 6 commits into from
Mar 18, 2019

Conversation

bigs
Copy link
Contributor

@bigs bigs commented Mar 14, 2019

This will be used by testbed, but is disabled by default. Exposes an http endpoint prometheus can scrape to gather metrics. For now, I like using prometheus directly as it provides us with a much richer API for instrumenting code. If there is a strong desire to use our stripped down go-metrics-interface, I'm happy to, though I'd like to add annotation support.

@bigs bigs requested a review from vyzo March 14, 2019 23:18
@ghost ghost assigned bigs Mar 14, 2019
@ghost ghost added the in progress label Mar 14, 2019
@anacrolix
Copy link
Contributor

anacrolix commented Mar 15, 2019

See libp2p/go-libp2p-kad-dht#297 and anacrolix/go-libp2p-dht-tool#2 where we're working on similar things.

p2pd/main.go Outdated
@@ -208,5 +211,10 @@ func main() {
}
}

if *metricsAddr != "" {
http.Handle("/metrics", prometheus.Handler())
go log.Println(http.ListenAndServe(*metricsAddr, nil))
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's this Println doing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if the http server terminates with an error, it will be logged

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's fine, but I am a little confused by the semantics of this. It seems that only the Println will be executed in the goroutine, the listenAndServe will be synchronous -- which isn't what you want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ahhhh i think the entire block is run asynchronously but i need to confirm. let me test. thanks for pointing it out!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep you nailed it. fixed.

p2pd/main.go Outdated
multiaddr "github.com/multiformats/go-multiaddr"
"github.com/libp2p/go-libp2p/p2p/protocol/identify"
"github.com/multiformats/go-multiaddr"
"github.com/prometheus/client_golang/prometheus/promhttp"
Copy link
Collaborator

Choose a reason for hiding this comment

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

what happened to the named imports?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lord have mercy i'm trying out goland since gocode is so slow right now and now i'm suffering the same fate

@vyzo vyzo merged commit 0d7e568 into master Mar 18, 2019
@ghost ghost removed the in progress label Mar 18, 2019
@vyzo vyzo deleted the feat/metrics branch March 18, 2019 17:39
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