Improve sockstat descriptions. Fixes #1856.

[Hello71]

Signed-off-by: Alex Xu (Hello71) [email protected]

[Hello71]

v2: return error instead of panicking when sockstat has unexpected format

[Hello71]

v3: fix indentation

[discordianfish]

Not sure we want to maintain such metric name <-> description mapping. My suggestion in #1856 was to just use a g eneric 'value from ' description instead. But open for arguments. @SuperQ wdyt?

[roidelapluie]

I think we can have better metrics names. value from <> makes the user explicly look for extra documentation, while providing direct info avoids that extra indirection.

[discordianfish]

@roidelapluie I'm open to that but if we go down that route of having a mapping of metrics to descriptions, we can also have a mapping for the metric names to make them follow our naming best practices..

[Hello71]

we can also have a mapping for the metric names to make them follow our naming best practices..

according to the comment,

	// Previously these metric names were generated directly from the file output.
	// In order to keep the same level of compatibility, we must map the fields
	// to their correct names.

iiuc, you want to rename node_sockstat_TCP_tw to, say, node_time_wait_sockets. how will you maintain backwards compatibility?

[discordianfish]

I think this deserve a more generic discussion first. Currently we explicitly require collectors to not maintain such mapping as in https://github.com/prometheus/node_exporter/blob/master/CONTRIBUTING.md#collector-implementation-guidelines So for now I think we should close this.

[Hello71]

Currently we explicitly require collectors to not maintain such mapping as in https://github.com/prometheus/node_exporter/blob/master/CONTRIBUTING.md#collector-implementation-guidelines

This policy says "The metrics should not get transformed"; in this PR, I never suggested changing the metric name or value, only the help text.

If custom help texts are banned, should we remove all of the help texts from https://github.com/prometheus/node_exporter/blob/master/collector/slabinfo_linux.go, https://github.com/prometheus/node_exporter/blob/master/collector/zfs_freebsd.go, https://github.com/prometheus/node_exporter/blob/master/collector/xfs_linux.go, etc? How would that improve the user experience or save maintainer time?

If we look at the context of the policy (#321), "vendor based mappings" means things like SMART values, not Linux proc definitions. This interpretation makes much more sense: it is hardly any more work to add some help text to each already-Linux-specific proc file collection, whereas it would be much more work to keep a list of SMART values for each disk vendor on top of the Linux SMART collection.

[SuperQ]

I agree with @Hello71. The intention of the "not transformed" is about the value, not the naming or help text. Really, we want to provide as much help text as possible for the raw data coming from the kernel. Adding help text makes metrics easier to use.

Clarifying the policy is needed, but I think this PR is worth accepting.