Add mountpoint to mountstats collector.

[dsnt02518]

I think this will address #2097

This is also useful for Hammerspace where the same 'device' may be mounted via the same protocol to differing locations, but is actually still a unique mount.

As per @SuperQ 's comment in #2097 , only the first unique (Device, Protocol, MountAddress, MountPoint) will be returned, as per the existing logic with (Device, Protocol, MountAddress).

[discordianfish]

You need to sign-off and fix the tests

[dsnt02518]

A

You need to sign-off and fix the tests

Apologies - missed the make usage instead of make test - will get that fixed up.

[dsnt02518]

Tests are fixed and passing, however they do slightly lose the meaning originally intended regarding the 'duplicates' since the extra mountpoint label means the /mnt/nfs/test-dupe mounts are no longer duplicates.
Let me know if that is considered an issue and I can attempt to re-work them.

[discordianfish]

Yeah good point re/ dups. I guess with the mountpoint there can't be dups anymore? @SuperQ remember the context?

[SuperQ]

Yea, I'm not sure we want to do it this way. The mountpoint, while useful, is going to lead to a lot of duplicate cardinality for some users.

For example, NFS mounted homedirs with an automount for each user. It could be the same device for each mount point for /home/foo and /home/bar.

The real question I have is, in the real world, are the metrics per mountpoint different? If they are, it would be a compelling case to add the label.

Otherwise, perhaps a node_nfs_mountpoint_info metric which would limit the M:N cardinality would be better.

I don't really have access to this kind of setup anymore to test.

[discordianfish]

The real question I have is, in the real world, are the metrics per mountpoint different? If they are, it would be a compelling case to add the label.

Yeah thats the key question. If yes, let's to this here. If not, let's use node_nfs_mountpoint_info

[SuperQ]

@dsnt02518 Would you mind providing some real-world example mounstats output here? Feel free to redact any private data, but the real-world metrics data would be useful to see.

[eugene-eeo]

@SuperQ one datapoint is mounting the same NFS volume using nosharecache to get per-container NFS client metrics; currently this means that due to using .Device as a key we lose that information -- the alternative is for us to use procfs library ourselves.

the point on cardinality is still valid though, so maybe this is best hidden behind a flag.

[nicolastakashi]

Thanks for the patience on this one — it is the original attempt at surfacing the NFS mountpoint. The direction discussed here (a mountpoint_info join metric instead of a per-series mountpoint label, to avoid the M:N cardinality problem) has since been implemented and approved in #3554. Suggest consolidating on #3554.