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

quic: add support for quic-go metrics #2823

Merged
merged 2 commits into from
Jul 31, 2024
Merged

quic: add support for quic-go metrics #2823

merged 2 commits into from
Jul 31, 2024

Conversation

sukunrt
Copy link
Member

@sukunrt sukunrt commented Jun 6, 2024

No description provided.

@sukunrt sukunrt marked this pull request as ready for review June 7, 2024 06:38
@sukunrt sukunrt marked this pull request as draft June 10, 2024 10:10
@sukunrt
Copy link
Member Author

sukunrt commented Jun 10, 2024

Pending: quic-go patch release.

@sukunrt sukunrt marked this pull request as ready for review June 13, 2024 09:06
@sukunrt sukunrt force-pushed the quic-metrics branch 3 times, most recently from f379090 to e055461 Compare June 24, 2024 08:52
@sukunrt sukunrt requested a review from MarcoPolo June 26, 2024 10:14
@@ -10,9 +12,12 @@ func DisableReuseport() Option {
}

// EnableMetrics enables Prometheus metrics collection.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's add to the comment saying if nil is passed, it will use the default registerer

Copy link
Collaborator

@MarcoPolo MarcoPolo left a comment

Choose a reason for hiding this comment

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

One suggestion to improve the comment for the exported Option func.

@sukunrt
Copy link
Member Author

sukunrt commented Jul 16, 2024

@galargh The go-check is failing with a diff in the generated protobuf files. How do i find out what the diff in the generated protobuf file is? Locally I only see the version being changed which should be ignored as per #2631

@galargh
Copy link
Contributor

galargh commented Jul 17, 2024

Here's a run with full diff - https://github.com/libp2p/go-libp2p/actions/runs/9974916892/job/27563846282

I'll make sure we have the diff available on failures in the next release of uci.

@sukunrt sukunrt merged commit 475cb47 into master Jul 31, 2024
8 of 11 checks passed
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