Skip to content

Conversation

0xf09f95b4
Copy link

@0xf09f95b4 0xf09f95b4 commented Aug 4, 2025

Hi,

this PR attempts to address #2805.

The issue we encountered is a case in which a path is bind-mounted to itself, the same issue you can see in this latest comment.

NixOS bind-mounts the /nix/store path to /nix/store to remount the same mount read-only and (nosuid, nodev), as you can see here. If the /nix/store path is already a mount from some device, this will lead to metrics being collected twice for the same (mountPoint, device) combination if the mount options differ.

This issue only occurs, when the filesystem options of the two mounts are different. The core issue is, that the existing filter that wants to prevent this issue here takes the entire labels struct into account, including mount options.

The mount options themselves are not used or exposed by node_exporter. So this minimal fix only removes the filesystem options after they are used to extract the ro gauge. This way, the seen := map[filesystemLabels]bool{} filter works correctly (as the options are always "").

This has a side-effect: Previously, the ro gauge was exported twice in this case, if the two mounts were read only and read-write (once ro, once rw). Now, the gauge is exported only once and in the example, it would be exported rw, even though the mount actually used by the kernel (the upper mount), is ro.

Additional options we came up with to fix this:

  • Filter this earlier, like the previous commit in this PR tried.
  • Remove the options from fileSystemLabels alltogether (only used in Linux anyway).
  • Add more logic to export the "correct" ro state.
  • Add the ro option to gauges as a label.

@discordianfish You were involved in the original issue. What do you think?

@0xf09f95b4 0xf09f95b4 force-pushed the filter-repeated-mountpoints branch from c347b31 to 83efcfe Compare August 4, 2025 09:17
@0xf09f95b4
Copy link
Author

@SuperQ do you have an opinion on this? Do you think this minimal fix is a good approach or would you like a more obvious/clearer implementation here?

@SuperQ SuperQ requested a review from discordianfish August 15, 2025 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant