Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixos/prometheus: add ec2_sd_configs section to scrape_configs #54691

Merged
merged 3 commits into from Apr 16, 2019

Conversation

thefloweringash
Copy link
Member

@thefloweringash thefloweringash commented Jan 27, 2019

Motivation for this change

Prometheus's configuration in Nix is exhaustively specified. To use any of the various service discovery mechanisms they must be defined in the module. This adds definitions for the EC2 service discovery according to the Prometheus Documentation for version 1.8, matching the default value of config.services.prometheus.package.

Applies cleanly to 18.09 where I tested it, and is purely additive. If merged, I'd like it to be backported as well.

In the interests of not having to closely track upstream changes, would it be preferable to have an extraConfigs inside scrapeConfigs, that is merged as pure data? For example, if a user wants to use prometheus 2.6, it has a new option in the EC2 service discovery section called filters. As is, this is impossible(?) without updating this module again, but with an extraConfigs there's an escape.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Copy link
Member

@basvandijk basvandijk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thefloweringash note I just merged PR #58255 which adds the prometheus2 service.

I see that the ec2_sd_config options are different between 1.8 and 2.8. So I think it makes sense starting to split the options for these version. Like having both promTypes.ec2_sd_config_v1 and promTypes.ec2_sd_config_v2.

I also agree it would be nice to have a extraConfigs in scrapeConfigs like you suggest.

thefloweringash and others added 3 commits April 16, 2019 13:43
This results in a smaller prometheus.yml config file.

It also allows us to use the same options for both prometheus-1 and
prometheus-2 since the new options for prometheus-2 default to null
and will be filtered out if they are not set.
@basvandijk
Copy link
Member

@GrahamcOfBorg test prometheus prometheus2

@basvandijk basvandijk merged commit 4e86766 into NixOS:master Apr 16, 2019
@thefloweringash thefloweringash deleted the prometheus-ec2 branch May 1, 2019 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants