kvclient: add metrics for proxy requests #120239
Conversation
|
This change is |
andrewbaptist
force-pushed
the
2024-03-07-proxy-metrics
branch
8 times, most recently
from
7bb5517
to
88cb4c3
Compare
andrewbaptist
force-pushed
the
2024-03-07-proxy-metrics
branch
2 times, most recently
from
9db45e1
to
47162bc
Compare
andrewbaptist
changed the title
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @andrewbaptist)
pkg/kv/kvclient/kvcoord/dist_sender.go line 206 at r1 (raw file):
metaDistSenderProxySentCount = metric.Metadata{ Name: "distsender.rpc.proxy.sent", Help: `This counts the number of proxy attempts for Send requests which originated on this client`,
nit: consider making this more understandable to operators -- they don't know what Send requests or stacks are, and to them a client is a SQL client not a DistSender. Something along the lines of "Number of gateway attempts to proxy a request to an unreachable leaseholder via a follower replica." Same goes for the other descriptions.
pkg/ui/workspaces/db-console/src/views/cluster/containers/nodeGraphs/dashboards/networking.tsx line 207 at r2 (raw file):
<LineGraph title="Proxy requests"
Should we try to present some of these in the same chart? For example, we could group either of proxy/forward or requests/errors into 2 charts rather than having 4 charts.
pkg/ui/workspaces/db-console/src/views/cluster/containers/nodeGraphs/dashboards/networking.tsx line 264 at r2 (raw file):
<LineGraph title="Proxy forwards"
Should this be "Proxy forward errors"?
There was a problem hiding this comment.
Whoops, accidentally published before approving.
Reviewable status:
andrewbaptist
force-pushed
the
2024-03-07-proxy-metrics
branch
from
47162bc
to
c02032f
Compare
There was a problem hiding this comment.
No problem! Thanks for the quick review!
Reviewable status:
pkg/kv/kvclient/kvcoord/dist_sender.go line 206 at r1 (raw file):
Previously, erikgrinaker (Erik Grinaker) wrote…
nit: consider making this more understandable to operators -- they don't know what Send requests or stacks are, and to them a client is a SQL client not a DistSender. Something along the lines of "Number of gateway attempts to proxy a request to an unreachable leaseholder via a follower replica." Same goes for the other descriptions.
I cleaned up this text. Good suggestion.
pkg/ui/workspaces/db-console/src/views/cluster/containers/nodeGraphs/dashboards/networking.tsx line 207 at r2 (raw file):
Previously, erikgrinaker (Erik Grinaker) wrote…
Should we try to present some of these in the same chart? For example, we could group either of proxy/forward or requests/errors into 2 charts rather than having 4 charts.
I had a conversation with the UI team about this, and they recommended to keep seperate graphs. Multiple per-node lines on a single graph are discouraged as they don't scale well
pkg/ui/workspaces/db-console/src/views/cluster/containers/nodeGraphs/dashboards/networking.tsx line 264 at r2 (raw file):
Previously, erikgrinaker (Erik Grinaker) wrote…
Should this be "Proxy forward errors"?
Yep - there were some other small typos I cleaned up as well. Thanks!
Proxy behavior was added in a previous commit. This commit adds 4 new metrics to track the client and server side of proxying and the number of errors that occured as a result of the proxy. These statistics will normally be zero or close to zero. While there is a partial partition the metric will be increased. Epic: none Release note (ops change): Adds four new metrics: distsender.rpc.proxy.sent, distsender.rpc.proxy.err, distsender.rpc.proxy.forward.sent, distsender.rpc.proxy.forward.err to track the number and outcome of proxy requests. Operators should monitor and alert on `distsender.rpc.proxy.sent` as it indicates there is likely a network partition in the system.
Changes the network page to add two new graphs. The first shows the number of proxy requests each node is sending. The second shows any proxy errors that occur. Epic: none Release note (ui change): Adds two new graphs to the network page.
andrewbaptist
force-pushed
the
2024-03-07-proxy-metrics
branch
from
c02032f
to
2fcf790
Compare
|
bors r=erikgrinaker TFTR! |
Proxy behavior was added in a previous commit. This commit adds 4 new
metrics to track the client and server side of proxying and the number
of errors that occured as a result of the proxy. These statistics will
normally be zero or close to zero. While there is a partial partition
the metric will be increased.
Epic: none
Release note: Adds four new metrics:
distsender.rpc.proxy.sent,distsender.rpc.proxy.err,distsender.rpc.proxy.forward.sent,distsender.rpc.proxy.forward.errto track the number and outcome ofproxy requests. Operators should monitor and alert on
distsender.rpc.proxy.sentas it indicates there is likely a networkpartition in the system.