prometheus / prometheus/node_exporter

Proposed minor changes to defaults for collector.diskstats.ignored-devices, collector.filesystem.ignored-fs-types

Open
#1,562 0 comments 9 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Go
Stars
13.8k
Forks
2.7k
Avg merge
1d 23h
Merged PRs (30d)
8

Description

Host operating system: output of uname -a
Linux ldex-mon2 4.15.0-70-generic #79-Ubuntu SMP Tue Nov 12 10:36:11 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
node_exporter version: output of node_exporter --version
node_exporter, version 0.18.1 (branch: HEAD, revision: 3db77732e925c08f675d7404a8c46466b2ece83e)
  build user:       root@b50852a1acba
  build date:       20190604-16:41:18
  go version:       go1.12.5
node_exporter command line flags

See below

Are you running node_exporter in Docker?

No

Proposal

node_exporter runs with defaults which include:

      --collector.diskstats.ignored-devices="^(ram|loop|fd|(h|s|v|xv)d[a-z]|nvme\\d+n\\d+p)\\d+$"
                                Regexp of devices to ignore for diskstats.
      --collector.filesystem.ignored-mount-points="^/(dev|proc|sys|var/lib/docker/.+)($|/)"
                                Regexp of mount points to ignore for filesystem collector.
      --collector.filesystem.ignored-fs-types="^(autofs|binfmt_misc|bpf|cgroup2?|configfs|debugfs|devpts|devtmpfs|fusectl|hugetlbfs|mqueue|nsfs|overlay|proc|procfs|pstore|rpc_pipefs|securityfs|selinuxfs|squashfs|sysfs|tracefs)$"
                                Regexp of filesystem types to ignore for filesystem collector.
      --collector.systemd.unit-blacklist=".+\\.(automount|device|mount|scope|slice)"
                                Regexp of systemd units to blacklist. Units must both match whitelist and not match blacklist to be included.

I would like to make a few suggestions for tweaks to this list. I am not making a PR right now, so that these suggestions can be assessed individually.

1. ignore partitions like /dev/sdaa1

When you have more than 26 drives, Linux creates device names like /dev/sdaa, /dev/sdab etc. I propose changing collector.diskstats.ignored-devices as follows:

from
^(ram|loop|fd|(h|s|v|xv)d[a-z]|nvme\\d+n\\d+p)\\d+$
to
^(ram|loop|fd|(h|s|v|xv)d[a-z]+|nvme\\d+n\\d+p)\\d+$

(note the + after [a-z]). Or if you want to be slightly more restrictive:

^(ram|loop|fd|(h|s|v|xv)d[a-z][a-z]?|nvme\\d+n\\d+p)\\d+$

I think this should be uncontroversial.

2. ignore aufs (and fuse.lxcfs? rootfs?)

ignored-fs-types already excludes a number of docker-related filesystems (e.g. overlay). In an initial deployment of node_exporter I found the following filesystems scraped:

  22124       "fstype": "aufs",
      1       "fstype": "btrfs",
      9       "fstype": "ext2",
    289       "fstype": "ext4",
     27       "fstype": "fuse.glusterfs",
     44       "fstype": "fuse.lxcfs",
      5       "fstype": "nfs",
    854       "fstype": "nfs4",
     32       "fstype": "rootfs",
   2224       "fstype": "tmpfs",
      6       "fstype": "vfat",
     61       "fstype": "xfs",
     47       "fstype": "zfs",

aufs is another union filesystem used by some versions of docker, and I suggest adding this into the default list of excluded filesystems, as this was generating by far the largest amount of noise.

I see little value in monitoring fuse.lxcfs either, as it's a virtual filesystem similar to /proc.

rootfs is seen on some older machines, and is effectively an overlay which just duplicates the root filesystem, e.g. I get these timeseries

node_filesystem_readonly{device="rootfs",fstype="rootfs",instance="storage1",job="node",mountpoint="/"} 0
node_filesystem_readonly{device="/dev/sdx5",fstype="ext4",instance="storage1",job="node",mountpoint="/"} 0

I think it can be excluded.

In my own configuration I am also going to filter out tmpfs (which is mainly lots of random docker and kubernetes mounts), but I can understand why some people might want to keep that in certain circumstances, and I am fine with leaving it enabled by default so people can make their own choices, perhaps by filtering on mountpoint instead.

So my proposal is: add aufs, fuse.lxcfs and rootfs to collector.filesystem.ignored-fs-types:

from
^(autofs|binfmt_misc|bpf|cgroup2?|configfs|debugfs|devpts|devtmpfs|fusectl|hugetlbfs|mqueue|nsfs|overlay|proc|procfs|pstore|rpc_pipefs|securityfs|selinuxfs|squashfs|sysfs|tracefs)$
to
^(aufs|autofs|binfmt_misc|bpf|cgroup2?|configfs|debugfs|devpts|devtmpfs|fuse[.]lxcfs|fusectl|hugetlbfs|mqueue|nsfs|overlay|proc|procfs|pstore|rootfs|rpc_pipefs|securityfs|selinuxfs|squashfs|sysfs|tracefs)$

3. Remove ambiguity around backslashes

The use of \\d makes it tricky to modify these defaults by pasting then back into a shell file or environment file, which may be subject to additional levels of unescaping backslashes.

For example, suppose you have a systemd unit file:

EnvironmentFile=/etc/default/node_exporter
ExecStart=/usr/local/bin/node_exporter $OPTIONS

and you want to modify the behaviour, so you paste back a modified version of the default regex:

OPTIONS='--web.disable-exporter-metrics --collector.diskstats.ignored-devices="^(ram|loop|fd|(h|s|v|xv)d[a-z]+|nvme\\d+n\\d+p)\\d+$"'

Is this correct??[^1] I found it highly unclear how many levels of backslash escaping are required here. It becomes even more confusing when you look at the Go source and realise that the string constant "\\" is only a single backslash character (and kingpin is converting it to two blackslashes for display purposes)

Some older systems may be using upstart (/etc/init/*.conf), in which case the interpretation of backslashes may be different again, since it's actually going via the shell:

script
    . /etc/default/node_exporter
    exec /usr/local/bin/node_exporter $OPTIONS
end script

However, rewriting the regexs to avoid backslashes sidesteps the problem, and can be done easily by changing \\d to [0-9] and \\. to [.].

This would mean changing collector.diskstats.ignored-devices:

from
^(ram|loop|fd|(h|s|v|xv)d[a-z]|nvme\\d+n\\d+p)\\d+$
to
^(ram|loop|fd|(h|s|v|xv)d[a-z]|nvme[0-9]+n[0-9]+p)[0-9]+$

and collector.systemd.unit-blacklist:

from
.+\\.(automount|device|mount|scope|slice)
to
.+[.](automount|device|mount|scope|slice)

The functionality is unchanged.


[^1] Answer: even when the regexs have been rewritten to avoid backslashes, the example I have is still incorrect. You also have to remove the double-quotes because they are treated as part of the argument data by systemd. The working solution is:

OPTIONS='--web.disable-exporter-metrics --collector.diskstats.ignored-devices=^(ram|loop|fd|(h|s|v|xv)d[a-z]+|nvme[0-9]+n[0-9]+p)[0-9]+$'

I had to debug this using cat /proc/<pid>/cmdline | hexdump -C. However, at least removing backslashes avoids one of the pitfalls!

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by locating the collector.diskstats.ignored-devices, collector.filesystem.ignored-fs-types, and collector.systemd.unit-blacklist defaults in the Go source. Review the proposed regex changes and their systemd/upstart implications, then assess each suggestion individually; done means the accepted defaults and their escaping behavior are covered consistently.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, linux
Domain
observability-sre
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.