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

Instrument connection limit and current count for http blackholes #1190

Merged

Conversation

blt
Copy link
Collaborator

@blt blt commented Dec 30, 2024

What does this PR do?

This commit introduces telemetry keeping track of the connection
limit for http blackholes and the current connection count. This is
desirable information for targets that wish to trade total connections
for other resources.

Copy link
Collaborator Author

blt commented Dec 30, 2024

@blt blt added the no-changelog label Dec 30, 2024 — with Graphite App
@blt blt marked this pull request as ready for review December 30, 2024 18:44
@blt blt requested a review from a team as a code owner December 30, 2024 18:44
@blt
Copy link
Collaborator Author

blt commented Dec 30, 2024

CC @remeh

let shutdown_fut = shutdown.recv();
pin!(shutdown_fut);
loop {
gauge!("connection.current", &labels).set(sem.available_permits() as f64);
Copy link
Contributor

Choose a reason for hiding this comment

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

we claim a permit upon accepting a connection, so wouldn't available_permits be the inverse of what we want? ie, we should use sem.claimed_permits to track the number of current connections

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

D'oh, yep.

Copy link
Collaborator Author

@blt blt Dec 30, 2024

Choose a reason for hiding this comment

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

Addressed in 75ecd70

@blt blt changed the base branch from blt/unifty_httpd_in_blackholes to graphite-base/1190 December 30, 2024 19:21
@blt blt force-pushed the blt/instrument_connection_limit_and_current_count_for_http_blackholes branch from 2e1a3a2 to a8a5566 Compare December 30, 2024 19:22
@blt blt force-pushed the graphite-base/1190 branch from 4d7e361 to b2f3c8b Compare December 30, 2024 19:22
@blt blt changed the base branch from graphite-base/1190 to main December 30, 2024 19:22
@blt blt force-pushed the blt/instrument_connection_limit_and_current_count_for_http_blackholes branch from a8a5566 to 3fe8e76 Compare December 30, 2024 19:22
blt added 2 commits December 30, 2024 11:33
This commit introduces telemetry keeping track of the connection
limit for http blackholes and the current connection count. This is
desirable information for targets that wish to trade total connections
for other resources.

Signed-off-by: Brian L. Troutwine <[email protected]>
Signed-off-by: Brian L. Troutwine <[email protected]>
@blt blt force-pushed the blt/instrument_connection_limit_and_current_count_for_http_blackholes branch from 3fe8e76 to 75ecd70 Compare December 30, 2024 19:33
@blt blt merged commit 2d11692 into main Dec 31, 2024
17 checks passed
Copy link
Collaborator Author

blt commented Dec 31, 2024

Merge activity

  • Dec 31, 12:35 PM EST: A user merged this pull request with Graphite.

@blt blt deleted the blt/instrument_connection_limit_and_current_count_for_http_blackholes branch December 31, 2024 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants