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

General feedback on kestrel connection metrics #52437

Open
1 task done
lmolkova opened this issue Nov 29, 2023 · 3 comments
Open
1 task done

General feedback on kestrel connection metrics #52437

lmolkova opened this issue Nov 29, 2023 · 3 comments
Assignees
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions

Comments

@lmolkova
Copy link

lmolkova commented Nov 29, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

Not related to any functional problem.

Describe the solution you'd like

Several kestrel connection metrics could be grouped together to better align with otel recommendations.

See the original discussion in open-telemetry/semantic-conventions#283 (comment)

TL;DR:

kestrel.active_connections, kestrel.queued_connections could be measured with something like kestrel.current.connections or kestrel.connections and additional attribute like kestrel.connection.state = active | queued.

kestrel.upgraded_connections might also be measured with the same counter using an attribute that captures original protocol version (e.g. http.protocol.initial_version).

Additional context

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label Nov 29, 2023
@amcasey
Copy link
Member

amcasey commented Dec 1, 2023

@JamesNK Please let me know if you're not the right person to look at this.

@JamesNK
Copy link
Member

JamesNK commented Dec 2, 2023

kestrel.active_connections, kestrel.queued_connections could be measured with something like kestrel.current.connections or kestrel.connections and additional attribute like kestrel.connection.state = active | queued.

How would a connection go from queued to active? Would you need to subtract it and then add it again?

e.g.
Connection is queued: kestrel.connections state=queued + 1
Connection is ready: kestrel.connections state=queued - 1 AND kestrel.connections state=active + 1
Connection is finished: kestrel.connections state=active - 1

@lmolkova
Copy link
Author

lmolkova commented Dec 4, 2023

kestrel.active_connections, kestrel.queued_connections could be measured with something like kestrel.current.connections or kestrel.connections and additional attribute like kestrel.connection.state = active | queued.

How would a connection go from queued to active? Would you need to subtract it and then add it again?

e.g. Connection is queued: kestrel.connections state=queued + 1 Connection is ready: kestrel.connections state=queued - 1 AND kestrel.connections state=active + 1 Connection is finished: kestrel.connections state=active - 1

yes, that'd be the way.

Still, the cost (breaking change) may be much higher than the benefit, so I've created this issue only for bookkeeping reasons.

@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

No branches or pull requests

5 participants
@JamesNK @lmolkova @amcasey @wtgodbe and others