Add Prometheus 2 service in parallel with 1.x version #49802

azazel75 · 2018-11-05T20:44:13Z

Motivation for this change

Cleaned up version of #46069. The reason for this is that there's no clean upgrade path from 1.x to 2.x, the suggested solution by the Prometheus team is that of let them run in parallel an use the 1.x instance as source for the 2.x.

in the original PR someone suggested a more copy and paste approach. I don't know, I don't really need Prometheus 1.x. So is up to you to decide

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)
Fits CONTRIBUTING.md.

benley · 2018-11-05T21:00:37Z

nixos/modules/services/monitoring/prometheus/default.nix

  promUser = "prometheus";
  promGroup = "prometheus";
+  prom2User = "prometheus2";


I suspect it would be acceptable (preferable, even?) to have prometheus and prometheus2 share the same UID/GID, unless there's a reason that won't work.

@benley It's centainly preferable to have just one user. As of now the data dir /var/lib/prometheus(2) is also the home of the user and that's is used to create the dir and to ensure that the dir is automatically owned by it. I have yet to find out what's the best way to proceed if the data dirs are two. What you advise me to do?

Hrm, good question. I suppose you could put prometheus 2 data in a subdirectory of prometheus 1's data, but that introduces some danger when someone goes to delete their old prometheus 1 data dir once they're finished migrating. Maybe the systemd unit could create the new data dir before starting the prometheus daemon?

see systemd.tmpfiles.rules in other modules to create dirs.

You can also create the directory with

serviceConfig = { StateDirectory = "prometheus2" };

in the systemd config

eskytthe · 2018-11-08T18:05:06Z

nixos/modules/services/monitoring/prometheus/default.nix

+
+  cmdlineArgs2 = cfg2.extraFlags ++ [
+    "--storage.tsdb.path=${cfg2.dataDir}/data/"
+    "--config.file=${prometheusYml}"


Suggested change

"--config.file=${prometheusYml}"

"--config.file=${prometheus2Yml}"

Did some test of your new module - seems to be a error here - new prometheus2 configuration will not be generated. Thanks for your work! BR Erik

This is was integrated

ivan · 2019-02-12T06:09:28Z

I would like to test this, could you please update it for master?

As the configuration for the exporters and alertmanager is unchanged between the two major versions this patch tries to minimize duplication while at the same time as there's no upgrade path from 1.x to 2.x, it allows running the two services in parallel. See also NixOS#56037

azazel75 · 2019-02-21T14:55:09Z

@ivan I've rebased the patch and integrated all the changes that happened on master, can/will you test it out?

eonpatapon · 2019-02-21T15:38:27Z

There is some syntax error in the alertmanager module

syntax error, unexpected '=', expecting ';', at /.../nixpkgs/nixos/modules/services/monitoring/prometheus/alertmanager.nix:140:14

azazel75 · 2019-02-21T15:50:35Z

@eonpatapon should be fixed now

griff · 2019-02-21T17:07:56Z

@azazel75 why did you also make 2 services for each of the exporters? They look to be the exact same services but configured to be under services.prometheus.exportes and services.prometheus2.exportes. Just because I switch to running both prometheus 1 & 2 I would not need to run e.g. both services.prometheus.exportes.node and services.prometheus2.exportes.node so why have them as separate services?

azazel75 · 2019-02-21T18:27:53Z

Sorry @griff but I don't understand precisely what your arguments against two set of exporters configurations are. their configuration modules access their own configuration stored as services.prometheus.exporters.foo or services.prometheus2.exporters.foo. Maybe for the "node" exporter can't be more than one exporter configured in a system, but generally speaking maybe in a situation where both prometheus1 and 2 are running there could be the need for concurrent running of two exporters of the same type. I don't have a concrete example though.

griff · 2019-02-21T18:53:14Z

@azazel75 I may have missunderstod how the exporters work but let me try and explain my point better with my understanding.

As such exporters are http servers that will get called by the prometheus server at an interval. My understanding is that there is no problem with two prometheus servers calling the same exporter at different intervals. i.e. they don't disturb each other and state is not reset on a scrape. If this is correct running the same exporter twice is just a waste of CPU and RAM since just one can be scraped from multiple servers and having multiple services that allow you to run the same exporter twice signals that you need to run them twice if running both prometheus 1 & 2.

azazel75 · 2019-02-21T19:18:31Z

>>>> "Brian" == Brian Olsen ***@***.***> writes:

Brian> @azazel75 I may have missunderstod how the exporters work but let me try and explain my point better with my understanding. Brian> As such exporters are http servers that will get called by the Brian> prometheus server at an interval. My understanding is that there is no Brian> problem with two prometheus servers calling the same exporter at Brian> different intervals. i.e. they don't disturb each other and state is Brian> not reset on a scrape. If this is correct running the same exporter Brian> twice is just a waste of CPU and RAM since just one can be scraped Brian> from multiple servers and having multiple services that allow you to Brian> run the same exporter twice signals that you need to run them twice if Brian> running both prometheus 1 & 2. I think that your reasoning has some has a weakness. I think that there can be some reasons why two exporters of the same type run e.g: - the two prometheus monitor a set of different service in the process of being upgraded, where the services whose data is collected by exporter foo-A are different (and so a different config is needed) by those whose data is collected by foo-B, where the first is collected by prometheus1 and the second by prometheus2 - the exporter needs to present the data to prometheus1 and 2 in different way (different aggregation, different precision) to the two monitoring agents, so one quick way may be to configure two of them, with different configuration parameters, and dismiss the one connected to prometheus1 when it will be... deleted. but anyway, it isn't a great complication in the config files of the exporters, they just take in their parent configuration attr as a variable instead of as a constant. It may well become useful if there will be a Prometheus3 someday. At the same time I'm against over engineering the configuration so if you (you that have merge permission) think that it's too much, it surely can be simplified. In that case I think the configuration should emphasize that the configuration of the exporters is somewhat independent from the two prometheus services and that it doesn't belong to one in particular.

bachp · 2019-02-21T22:09:20Z

@azazel75 I think a clean copy and past of prometheus would be easier to maintain if we say that we want to get rid of prometheus 1 in the long run.

I'm with @griff for the exporters. From my experience they are completly independet of the prometheus version you are running. So I would not add additional services for them.

griff · 2019-02-22T18:44:17Z

@azazel75 I am sorry if I wrongly gave the impression that I am any kind of authority on this subject or that I have merge permissions. I am just a normal user like yourself that wants NixOS to be as great as possible and therefore offered my personal opinions.

As for your response of wanting to run different exporters with different parameters the question I have is why then only allow 2? The prometheus exporter format is becoming pretty standard and supported by other metrics solutions (like telegraf and DataDog to mention just 2) so if the argument is that we want to support different scrapers hitting the same exporter type run with different arguments why not allow me to run the same exporter 3 times or as many times as I want?

benley · 2019-02-22T20:48:17Z

why not allow me to run the same exporter 3 times or as many times as I want?

This is an issue that affects variety of services in NixOS, and probably not one we should try to solve in this PR. The existing pattern of services.<thing>.enable doesn't lend itself well to supporting multiple instances of a given service, and I'm not sure what the best approach is to improve upon it.

eonpatapon · 2019-02-24T10:12:38Z

There is still an issue with alertmanager module, here's the patch to fix it:

diff --git a/nixos/modules/services/monitoring/prometheus/alertmanager.nix b/nixos/modules/services/monitoring/promethe
index f19dd709a21..e9127068175 100644
--- a/nixos/modules/services/monitoring/prometheus/alertmanager.nix
+++ b/nixos/modules/services/monitoring/prometheus/alertmanager.nix
@@ -152,7 +152,7 @@ let
           after    = [ "network.target" ];
           script = ''
             ${amCfg.package}/bin/alertmanager \
-              ${concatStringsSep " \\\n  " cmdlineArgs}
+              ${concatStringsSep " \\\n  " (mkCmdlineArgs amCfg)}
           '';
           serviceConfig = {
             User = amCfg.user;

azazel75 · 2019-02-25T16:45:16Z

Thanks @eonpatapon. @griff there's no need to worry. It's just that I don't have the power to merge this either but to answer your example of running more that two sets of exporters, it seems out of scope. Anyway I'm running out of time that I want to spend on this, feel free to take what you need and propose your own or directly modify this ( If I can do something about that, just tell me and I wil do It)

fpletz · 2019-03-03T12:03:57Z

nixos/modules/services/monitoring/prometheus/exporters.nix

@@ -4,8 +4,10 @@ with lib;

 let
  cfg = config.services.prometheus.exporters;
+  cfg2 = config.services.prometheus2.exporters;


Having two sets of exports does not make any sense. Please remove these changes.

fpletz · 2019-03-03T12:07:58Z

nixos/modules/services/monitoring/prometheus/alertmanager.nix

-in {
-  options = {
-    services.prometheus.alertmanager = {
+  cfg2 = config.services.prometheus2.alertmanager;


A second instance/version of alertmanager does not make sense. Prometheus sends queries to alertmanager to trigger alerts, so one instance is enough.

fpletz · 2019-03-03T12:09:02Z

nixos/modules/misc/ids.nix

@@ -339,6 +339,7 @@
      rss2email = 312;
      cockroachdb = 313;
      zoneminder = 314;
+      prometheus2 = 315;


One user for both prometheus instances would be enough. Especially because we are going to remove prometheus1 soon.

disassembler · 2019-03-22T01:57:45Z

@azazel75 are you able to finish up the requested changes? If not, @jbgi can take this over as IOHK needs this PR as well for a project we're working on.

azazel75 · 2019-03-25T11:06:59Z

@disassembler, @jbgi please go for it. The only detail you have to pay attention to is the creation of the working directory if you are going to use just one user for both the services in such case, one directory is created as home of the user, the other needs to be created when the service is enabled. The proposed solutions involve the use of systemd-tmpfiles subsystem or the StateDirectory service config entry. From my pow the former while effective seems like using something that was intented for other purposes ( see man tmpfiles.d) and the latter while elegant and effective need for the working directory to be located under /var/lib.

arianvp · 2019-03-25T11:45:27Z

I would in both cases use StateDirectory instead of relying on home. System users in general shouldn't have a home directory unless for compatibility reasons (the service is hard coded to look stuff up in $HOME). I'm pretty sure prometheus doesn't do this so not allocating a home directory should be safe.

This also makes it possible to later switch to DynamicUser instead.

jbgi · 2019-03-25T13:59:51Z

Added some more commits in #58255 to address the points raised.

basvandijk · 2019-04-08T14:14:30Z

Let's close this PR in favour of #58255 which includes the commits of this PR and fixes the remaining issues.

GrahamcOfBorg added 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Nov 5, 2018

benley reviewed Nov 5, 2018

View reviewed changes

azazel75 mentioned this pull request Nov 7, 2018

Prometheus: upgrade configuration module to use the 2.x package #46069

Closed

9 tasks

eskytthe reviewed Nov 8, 2018

View reviewed changes

bachp mentioned this pull request Dec 8, 2018

Prometheus 2.x support for 18.03 #34166

Closed

bachp mentioned this pull request Feb 19, 2019

nixos/prometheus: support v2 #56037

Closed

10 tasks

azazel75 force-pushed the prometheus2 branch from fefe775 to 978f25c Compare February 21, 2019 14:52

azazel75 requested a review from infinisil as a code owner February 21, 2019 14:52

Make it pass a minimal test

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

e324c7e

GrahamcOfBorg added 10.rebuild-linux: 1-10 and removed 10.rebuild-linux: 0 labels Feb 21, 2019

Fix alertmanager service definition. Thanks to @eonpatapon

6f638b0

fpletz requested changes Mar 3, 2019

View reviewed changes

jbgi mentioned this pull request Mar 20, 2019

prometheus: fix version 2 errors #57910

Closed

10 tasks

jbgi mentioned this pull request Mar 25, 2019

Add Prometheus 2 service in parallel with 1.x version (continuation) #58255

Merged

10 tasks

basvandijk closed this Apr 8, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Prometheus 2 service in parallel with 1.x version #49802

Add Prometheus 2 service in parallel with 1.x version #49802

azazel75 commented Nov 5, 2018

benley Nov 5, 2018

azazel75 Nov 5, 2018

benley Nov 7, 2018

globin Nov 8, 2018

arianvp Nov 12, 2018

eskytthe Nov 8, 2018

eskytthe Nov 8, 2018

azazel75 Feb 21, 2019

ivan commented Feb 12, 2019

azazel75 commented Feb 21, 2019

eonpatapon commented Feb 21, 2019 •

edited

Loading

azazel75 commented Feb 21, 2019

griff commented Feb 21, 2019

azazel75 commented Feb 21, 2019

griff commented Feb 21, 2019

azazel75 commented Feb 21, 2019 via email

bachp commented Feb 21, 2019

griff commented Feb 22, 2019

benley commented Feb 22, 2019

eonpatapon commented Feb 24, 2019

azazel75 commented Feb 25, 2019

fpletz Mar 3, 2019

fpletz Mar 3, 2019

fpletz Mar 3, 2019

bachp Mar 3, 2019

disassembler commented Mar 22, 2019

azazel75 commented Mar 25, 2019

arianvp commented Mar 25, 2019

jbgi commented Mar 25, 2019

basvandijk commented Apr 8, 2019

	"--config.file=${prometheusYml}"
	"--config.file=${prometheus2Yml}"

Add Prometheus 2 service in parallel with 1.x version #49802

Add Prometheus 2 service in parallel with 1.x version #49802

Conversation

azazel75 commented Nov 5, 2018

Motivation for this change

Things done

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivan commented Feb 12, 2019

azazel75 commented Feb 21, 2019

eonpatapon commented Feb 21, 2019 • edited Loading

azazel75 commented Feb 21, 2019

griff commented Feb 21, 2019

azazel75 commented Feb 21, 2019

griff commented Feb 21, 2019

azazel75 commented Feb 21, 2019 via email

bachp commented Feb 21, 2019

griff commented Feb 22, 2019

benley commented Feb 22, 2019

eonpatapon commented Feb 24, 2019

azazel75 commented Feb 25, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

disassembler commented Mar 22, 2019

azazel75 commented Mar 25, 2019

arianvp commented Mar 25, 2019

jbgi commented Mar 25, 2019

basvandijk commented Apr 8, 2019

eonpatapon commented Feb 21, 2019 •

edited

Loading