Support per-share metrics #25
Conversation
phlogistonjohn
left a comment
phlogistonjohn
left a comment
There was a problem hiding this comment.
Looking at it from largely a golang perspecitive only it looks generally good, I have a couple of minor nitpicks but I'm also ok with approving any time.
synarete
force-pushed
the
ss-per-share-metrics
branch
from
0eae003 to
f7b7a69
Compare
avanthakkar
left a comment
avanthakkar
left a comment
There was a problem hiding this comment.
LGTM overall
Just one note—not directly related to this PR, but something worth tracking so we don’t forget: the netbios label is currently only present on the smb_metrics_status metric. I think we’ll eventually need this label on all SMB related metrics (including per-share), not just status. When Ceph Dashboard builds Grafana dashboards, alerts, etc.. they’ll require a unique identifier like netbios to join exposed metrics with related metadata metric (to be exposed in Ceph prometheus module), which will have this label too.
avanthakkar
left a comment
avanthakkar
left a comment
There was a problem hiding this comment.
Might be worth updating the README to include examples of per-share labeled metrics too?
Good point. Will fix it in a follow-up PR. |
Indeed. Will fix. |
Digging a bit more into it, I realized that we should probably also add the client-ip as label. Although it is not a requirement for now, we have it now (with this PR), so why not add it? adding another commit soon, will ask for another review. |
When per-share metrics mode is enabled, smbstatus dumps fine-grained profile counters. Parse it. Signed-off-by: Shachar Sharon <ssharon@redhat.com>
Parse per-share profile key to extract share-name and connected client IP as a pair of strings. Signed-off-by: Shachar Sharon <ssharon@redhat.com>
When exporting per-share metrics provide net-bios name, share-name and client-ip as labels. Signed-off-by: Shachar Sharon <ssharon@redhat.com>
Added a (subset) of additional metrics when per-share profile counters are enabled. Signed-off-by: Shachar Sharon <ssharon@redhat.com>
synarete
force-pushed
the
ss-per-share-metrics
branch
from
f7b7a69 to
edc14f8
Compare
|
Added netbios and client as labels + updated README. @avanthakkar Please take another look. |
3 participants
With per-share profile metrics now on Samba's vfs_ceph_new master, support this mode also in Prometheus metrics using additional label.