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

Register oc metrics #7593

Merged
merged 2 commits into from
Dec 9, 2020
Merged

Register oc metrics #7593

merged 2 commits into from
Dec 9, 2020

Conversation

lanzafame
Copy link
Contributor

This PR exposes OpenCensus metrics from go-ipfs to enable the collection and export of QUIC metrics. This was done on a separate port to the prometheus metrics as there were issues getting OC metrics into the prometheus registry. I have gotten it to work the other way but I can't seem to get to work this way.

There is also a small fix to the plugin loading script.

Copy link
Member

@marten-seemann marten-seemann left a comment

Choose a reason for hiding this comment

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

LGTM, but let me cut an official quic-go and go-libp2p-quic-transport release that supports metrics before merging this.

@lanzafame
Copy link
Contributor Author

lanzafame commented Aug 14, 2020

@marten-seemann Ping me when you have merged the metrics package and tagged into quic-go and I will update this PR.

Yeah what you just said. 😛

@lanzafame
Copy link
Contributor Author

@marten-seemann any update on tagging quic-go?

@marten-seemann
Copy link
Member

@lanzafame The new quic-go release now has metrics support. All that's left to do is to enable metrics collection in go-libp2p-quic-transport: libp2p/go-libp2p-quic-transport#172.

@marten-seemann
Copy link
Member

marten-seemann commented Sep 23, 2020

Hi @lanzafame, I just released a v0.8.1 of go-libp2p-quic-transport that enables metrics collection (but doesn't pull in the recent stream interface changes yet). Could you update this PR?

@lanzafame
Copy link
Contributor Author

lanzafame commented Sep 29, 2020

@marten-seemann @aschmahmann This is now up-to-date and working.
Screen Shot 2020-09-29 at 11 49 18

@marten-seemann
Copy link
Member

Once #7769 is merged, this PR should be ready to go (minus the quic-go update to go.mod).

@aschmahmann aschmahmann merged commit 78c6dba into ipfs:master Dec 9, 2020
@aschmahmann
Copy link
Contributor

@lanzafame @marten-seemann I merged this since it looks like it was just blocked on a quic-go update which is now in. I noticed that the log level of the opencensus initialization was Error which is pretty loud so I decreased it to Info in #7815.

If there's anything I've missed here lmk and we can get it into the next RC.

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.

3 participants