Receive: Reload tenant limit configuration on file change

[douglascamata]

Signed-off-by: Douglas Camata 159076+douglascamata@users.noreply.github.com

• I added CHANGELOG entry for this change.
• Change is not relevant to the end user.

Changes

• Isolate the limiting logic in Receive into its own package pkg/receive/limits.
• Tenant limit configuration reload when it changes without having to restart Receive.
  • Adds an fsnotify (filesystem watcher) based mechanism to reload configuration based on any PathOrContent. If this works well enough it can be extracted into https://github.com/efficientgo/tools/tree/main/extkingpin and imported here.

Future work

• After this is merged and this new function is proven to be working well, an issue can be created to track the migration of the logic into https://github.com/efficientgo/tools/tree/main/extkingpin and its usage in all other configuration reloads throughout the project.
• Add file hash checking to ensure we only reload the configuration if the contents of the file are changed.
• Contribute some fixes (i.e. the reload skip logic fix) and features (i.e. the debouncer) to Prometheus file discovery

Verification

• Unit tests
• Local tests

[matej-g]

Thanks for addressing everything @douglascamata ❤️, approving with my minor objections (#5673 (review)), which is fine since we agreed to tackle it separately 👍

[douglascamata]

Hey @matej-g, can you re-review please? I had to fix duplicated metrics collector register errors when config reloads were triggered by using the UnRegisterer (see fd98829). Related to the upgrade to Prometheus v2.39.

[bwplotka]

Nice work! 💪🏽

[bwplotka]

My only concerns is as @matej-g mentioned - what we do about

thanos/pkg/receive/config.go

Line 50 in 862b7a3

type ConfigWatcher struct {

- sounds like with this PR we have two ways of doing the same thing (reloading from file). What's the direction here to unify this? (:

[douglascamata]

• sounds like with this PR we have two ways of doing the same thing (reloading from file). What's the direction here to unify this? (:

@bwplotka as mentioned in the Future Work section & comments of this PR and some of the Thanos Community Meetings, the plan is to unify all the implementations over time. First I would like to confirm this one is correct and stable, which is difficult considering the complexity & gotchas of all the different backends of fsnotify but not impossible. Then, in case everything goes well, smaller PRs can follow migrating every other configuration reload system.

I do not want to open a single huge PR migrating everything to a brand new system that didn't prove itself yet. Even I would possibly not approve such PR. 😄