Skip to content
This repository has been archived by the owner on May 26, 2022. It is now read-only.

expose some Prometheus metrics #75

Merged
merged 2 commits into from Apr 8, 2021
Merged

expose some Prometheus metrics #75

merged 2 commits into from Apr 8, 2021

Conversation

marten-seemann
Copy link
Contributor

This PR exposes some basic TCP transport metrics to Prometheus.

We're using the TCP_INFO socket option to query the kernel for TCP metrics (things like the RTT, congestion window, etc).
In the future, I'm planning to add more metrics, to mirror what we're measuring for QUIC wherever possible.

tcp_conn.go Outdated Show resolved Hide resolved
Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

What was the motivation here? Usually, we just pull metrics out of the kernel itself.

tcp_conn.go Outdated Show resolved Hide resolved
@Stebalien
Copy link
Member

We're using the TCP_INFO socket option to query the kernel for TCP metrics (things like the RTT, congestion window, etc).

Metrics are usually aggregate. I'm not sure aggregate RTT, congestion, etc. is really all that useful.

@Stebalien
Copy link
Member

(maybe aggregate bandwidth would be useful?)

@Stebalien
Copy link
Member

Apparently, RTT etc distributions (which will help us pick better timeouts, etc) conn lifetime (may help with connection management choices) etc. So yeah, useful.

@Stebalien
Copy link
Member

In terms of making this work with things that aren't tcp.Conn connections. I'm fine forking and maintaining a small fork.

@marten-seemann
Copy link
Contributor Author

I created a fork of mikioh/tcp that includes mikioh/tcp#2, so we don't have to use any unsafe type castings here. Not my preferred resolution, but until @mikioh merges the PR our options are limited.

@Stebalien
Copy link
Member

We have a lot of forks that fall into that category.

Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

Sorry for the delayed review.

metrics.go Outdated Show resolved Hide resolved
metrics.go Outdated Show resolved Hide resolved
metrics.go Outdated Show resolved Hide resolved
Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

Looks good to me from the Prometheus perspective!

@marten-seemann marten-seemann merged commit d3fdb0f into master Apr 8, 2021
@marten-seemann marten-seemann deleted the prometheus branch April 12, 2021 14:42
@aschmahmann aschmahmann mentioned this pull request May 14, 2021
71 tasks
"sync"
"time"

"github.com/marten-seemann/tcp"
Copy link
Member

Choose a reason for hiding this comment

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

Can we move this to libp2p? Our forks like this have a tendency to live longer than intended.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants