- Notifications
You must be signed in to change notification settings - Fork928
feat(coderd): add prometheus metrics to servertailnet#11988
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This stack of pull requests is managed by Graphite.Learn more about stacking. |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| } | ||
| func (c *instrumentedConn) Close() error { | ||
| c.closeOnce.Do(func() { |
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.
are network connectionsalways explicitly closed?
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.
This one I'm not 100% sure on. Admittedly, I found this idea from a stackoverflow post which seemed to work for a couple other people. Was planning to get this into dev and monitor to make sure it works as intended with a lot more usage than I can reproduce myself.
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.
I guess maybe theyshould be and this gauge will tell us if there's a leak...
coderd/tailnet.go Outdated
| connsPerAgent: prometheus.NewGauge(prometheus.GaugeOpts{ | ||
| Namespace: "coder", | ||
| Subsystem: "servertailnet", | ||
| Name: "open_tcp_connections", |
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.
I realize we only use servertailnet for HTTP proxying at present, but what do you think about changing this toopen_connections and making the network a label (e.g.network=tcp? Like, I dunno, what if some customer wants us to proxy QUIC or some shit?
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.
Yeah that's a good point. If we were to keep it like this we would definitely preclude ourselves from expanding this without a breaking change. Adding anetwork=tcp with it really only ever being tcp for the forseeable future basically has the same implications and usability as not having it, probably a good change.
5f9a1d5 to02a7089Compare
No description provided.