netdata: facilitate building netdata with cloud support #92782

leotaku · 2020-07-09T10:21:46Z

This PR adds a withCloud option to the netdata package. When enabled it allows the netdata agent to communicate with the netdata cloud.

Other than that, we unconditionally wrap the netdata-claim.sh script with a path to openssl and no longer remove the sbin output as it is required by the script.

Motivation for this change

The current netdata package does not support connecting to the netdata cloud.
There has been a packaging request for netdata with cloud support in #91549.
#92291 was merged in preparation to this PR.

Things done

leotaku · 2020-07-09T10:26:30Z

This PR also adds a new file for the custom version of mosquitto used by netdata. I was unsure if it makes sense for that package to have a meta section, so I have left it out of the initial PR. Also, the maintainer for the Netdata package @lethalman has seemingly been inactive for almost three years.

leotaku · 2020-08-08T17:24:24Z

@jtojnar I really don't want to pester you, but maybe it would be possible for you to review this PR, as you have already approved and merged #92291? Thank you for your hard work on the Nixpkgs project!

pkgs/tools/system/netdata/mosquitto.nix

pkgs/tools/system/netdata/default.nix

jtojnar · 2020-08-08T18:55:22Z

pkgs/tools/system/netdata/default.nix

-    "--sysconfdir=/etc"
-  ];
+  configureFlags = [ "--localstatedir=/var" "--sysconfdir=/etc" ]
+    ++ optional withCloud "--enable-cloud";


Again I will ask how does this affect closure size and if it would not make sense to enable it unconditionally.

Closure size goes from 148743560 to 149251816, so it's basically irrelevant.

I would be very grateful if you could tell me what the policy about adding new configuration options currently is. I generally just see the many e.g. withCups, withDBengine, withIpmi, withNetfilter, withSsl configuration options, many of which seem even less relevant than e.g. withCloud, and assume that adding new options is good practice. But your comments make me think that they might just be there for backwards compatibility and "no options" is preferred?

I do not think there is a policy. There are few legitimate reasons for having the options overridable (when it makes the closure significantly larger, when it adds an unfree dependency to otherwise free package…) but most of the time it looks to me like a cargo-cult.

I typically ask myself: “Would someone want to disable this feature? If there were an overridable option, would someone bother overriding it? Are the benefits commensurate with the added complexity to the expression?” I would only add an option when I answered “Yes” to all three of the questions.

But in the end, the decision is up to the maintainers of each individual package, since they will bear the complexity.

I think withCloud could be interesting to some users that would like to ensure that none of their system analytics are streamed to servers owned by the Netdata company. There seems to be a way to disable the cloud connection at runtime, but it does seem like disabling cloud support at built time is still the only option to ensure no connection is made.

Personally, I think that's useful enough to keep the withCloud option, which, in my opinion, also does not add a lot of complexity to the derivation.

Looks like the cloud disabling is now fixed upstream?

Given that, I propose the package just gets built with cloud support.

jtojnar · 2020-08-08T18:58:28Z

pkgs/tools/system/netdata/default.nix

+    cp -r "${mosquitto}/lib" externaldeps/mosquitto;
+    cp -r "${libwebsockets_3_2}/lib" externaldeps/libwebsockets;


Ugh https://github.com/netdata/netdata/blob/3d33eea007967ffe70f7918e6b617366cb34dd28/CMakeLists.txt#L916-L925

Does not someone (some other distro, pull request on the repo) have a patch that would allow using shared library from the system?

I noticed that this is terrible practice by the upstream project, but I chose to simply adapt the build script for the initial PR.

Checking out other distros (Alpine, Arch, Debian, Fedora, Gentoo) I found a relevant issue:

netdata/netdata#8961

At least some of the developers think that accommodating Linux distributions is generally a good idea, however I am not that confident that we will see actual changes anytime soon. In any case, I have subscribed to the Issue and will of course update the PR if I notice any changes.

According to one of the developers, Netdata will soon be using a new implementation of the Agent-Cloud-Link which would only require unpatched and common dependencies (JSON-C and OpenSSL). I would be more than happy to modify this PR when this change lands, or if it could be agreed to merge this PR in its current "hackish" state, open a new PR to then fix these issues.

@leotaku hi, you are right I am pushing for the new implementation of the link as you mentioned. Called ACLK-NG,

It started originally as a skunkworks project, now it is starting to take shape and we plan to merge it ASAP.
While it works already it will take some time to weatherproof it and catch all possible bugs (It is a huge change and a lot of code written). This is the motive why originally it will be released as a fallback (in case ACLK-Legacy cannot be built e.g. due to missing custom mosquito lib), with the possibility to enforce its use by passing --aclk-ng to the installer.

The main motivation to do this for me was to enable full-featured Netdata in every distribution package manager (e.g. remove the need for bundling libraries and other "hacky" things). As a side effect, it also seems to perform better (throughput doesn't drop with latency as was the case with Legacy).

rissson

When testing this with Netdata 1.25.0 (basically applying your changes on top of nixos-20.09), it fails with

/nix/store/hiwz81i1g3fn3p0acjs042a4h5fri6dh-binutils-2.31.1/bin/ld: externaldeps/libwebsockets/libwebsockets.a(unix-plugins.c.o): undefined reference to symbol 'dlclose@@GLIBC_2.2.5'
  /nix/store/hiwz81i1g3fn3p0acjs042a4h5fri6dh-binutils-2.31.1/bin/ld: /nix/store/bdf8iipzya03h2amgfncqpclf6bmy3a1-glibc-2.32/lib/libdl.so.2: error adding symbols: DSO missing from command line

However, it works fine when building from this commit netdata/netdata@593e1b6 and not using the bundled libwebsockets. I think we can wait for the next Netdata release to be able to use this.

pkgs/tools/system/netdata/mosquitto.nix

pkgs/tools/system/netdata/default.nix

SuperSandro2000 · 2020-11-23T11:03:48Z

Result of nixpkgs-review pr 92782 run on x86_64-darwin 1

1 package built:

netdata

SuperSandro2000 · 2020-12-22T16:36:58Z

pkgs/tools/system/netdata/mosquitto.nix

+  '';
+
+  postInstall = ''
+    cp $out/lib/libmosquitto_static.a $out/lib/libmosquitto.a


Can we use ln here?

Sorry, but I am not quite sure what you mean by "use ln" here, could you maybe elaborate?

I think they mean doing something like this:

ln -s $out/lib/libmosquitto_static.a $out/lib/libmosquitto.a

Apologies, I and l look the same to me in my system variable-width font, so I got confused. It does not seem possible to use symbolic links here (build fails) but hard links seem to be fine.

I'm not sure if hard linking is something that is commonly done in Nixpkgs and whether it has any particular advantages or disadvantages. A very naive grep-based investigation shows that copying files is done much more frequently than hardlinking, so I'd probably just keep the cp.

$ cd nixpkgs $ rg "cp[ ][^(-r)]" | wc -l 1451 $ rg "ln[ ][^(-s)]" | wc -l 29

cp is useful when moving files from source to out but ln should be used when symlinking static libs in out or even mv.

Ok, I have switched the two lines to hardlinking now.

ln should be used when symlinking static libs in out or even mv

You are aware that we are not using symlinking here, right? I am now using hardlinking, which is a different concept which is why I am unsure if I should use it in derivations.

SuperSandro2000 · 2020-12-22T16:41:59Z

Result of nixpkgs-review pr 92782 run on x86_64-linux 1

1 package failed to build:

netdata

  CC       collectors/proc.plugin/proc_uptime.o
  CC       collectors/proc.plugin/sys_kernel_mm_ksm.o
  CC       collectors/proc.plugin/sys_block_zram.o
  CC       collectors/proc.plugin/sys_devices_system_edac_mc.o
  CC       collectors/proc.plugin/sys_devices_system_node.o
  CC       collectors/proc.plugin/sys_fs_btrfs.o
  CC       collectors/proc.plugin/sys_class_power_supply.o
  CC       collectors/proc.plugin/sys_class_infiniband.o
  CC       collectors/tc.plugin/plugin_tc.o
  GEN      netdata
/nix/store/kcp0y7g1ir4dxq2gqz687i4vb7gqy8s0-binutils-2.31.1/bin/ld: externaldeps/libwebsockets/libwebsockets.a(unix-plugins.c.o): undefined reference to symbol 'dlclose@@GLIBC_2.2.5'
/nix/store/kcp0y7g1ir4dxq2gqz687i4vb7gqy8s0-binutils-2.31.1/bin/ld: /nix/store/1yvpgm763b3hvg8q4fzpzmflr5674x4j-glibc-2.32-10/lib/libdl.so.2: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status
make[2]: *** [Makefile:4371: netdata] Error 1
make[2]: Leaving directory '/build/source'
make[1]: *** [Makefile:5506: all-recursive] Error 1
make[1]: Leaving directory '/build/source'
make: *** [Makefile:2987: all] Error 2

rissson · 2021-01-01T13:59:18Z

@leotaku With #107306 merged, would you mind rebasing on master?

This commit adds a `withCloud` option to the netdata package. When enabled it allows the netdata agent to communicate with the netdata cloud. Other than that, we unconditionally wrap the `netdata-claim.sh` script with a path to openssl and no longer remove the `sbin` output as it is required by the script.

Co-authored-by: Marc Schmitt <marc.schmitt@risson.space>

pkgs/tools/system/netdata/mosquitto.nix

SuperSandro2000 · 2021-01-25T00:17:11Z

This is a semi-automatic executed nixpkgs-review which is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 92782 run on x86_64-darwin 1

1 package built:

netdata

SuperSandro2000 · 2021-01-25T00:17:37Z

This is a semi-automatic executed nixpkgs-review which is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 92782 run on x86_64-linux 1

1 package built:

netdata

SuperSandro2000

diff LGTM

SuperSandro2000 · 2021-03-04T02:36:43Z

pkgs/tools/system/netdata/mosquitto.nix

+  postInstall = ''
+    ln $out/lib/libmosquitto_static.a $out/lib/libmosquitto.a
+    ln $out/include/mosquitto.h $out/lib/mosquitto.h
+  '';


Not sure if we want to add a meta section here.

Sorry for keeping you waiting, I must have missed the notifications for your comments. Honestly, I think it should be fine not having the meta section, especially since I would like to remove this package from Nixpkgs as soon as ACLK-NG is present in the Netdata package provided by Nixpkgs. If you think a meta section is necessary, I will of course add it.

Packages require a meta section.

I mean, this package will likely disappear in a few weeks, plus it's only used by netdata.

underhood · 2021-03-22T09:47:49Z

Hi, ACLK-NG has been merged and is present in current nightlies and will be present in the next Netdata release. Currently is a fallback for when ACLK LEgacy can't be built (e.g. missing dependency) or can be enabled by passing --aclk-ng parameter to the installer.

SuperSandro2000 · 2021-03-27T01:42:44Z

@leotaku the build is failing

@ofborg build

SuperSandro2000 · 2021-03-27T02:13:38Z

This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 92782 run on x86_64-linux 1

1 package built:

netdata

leotaku · 2021-04-01T18:17:24Z

The new Netdata release (1.30.0) already has ACLK-NG support. This means that this pull request is no longer relevant.

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Jul 9, 2020

jtojnar reviewed Aug 8, 2020

View reviewed changes

leotaku mentioned this pull request Sep 17, 2020

Allow use of system libwebsockets and mosquitto netdata/netdata#8961

Closed

rissson suggested changes Nov 16, 2020

View reviewed changes

pkgs/tools/system/netdata/mosquitto.nix Outdated Show resolved Hide resolved

pkgs/tools/system/netdata/default.nix Outdated Show resolved Hide resolved

SuperSandro2000 reviewed Nov 23, 2020

View reviewed changes

pkgs/tools/system/netdata/default.nix Show resolved Hide resolved

rissson mentioned this pull request Dec 21, 2020

netdata: 1.26.0 -> 1.28.0 #107306

Merged

10 tasks

SuperSandro2000 reviewed Dec 22, 2020

View reviewed changes

leotaku force-pushed the netdata-cloud branch from deddeed to ff83616 Compare January 6, 2021 02:44

rissson requested review from rissson, SuperSandro2000 and jtojnar January 6, 2021 03:22

SuperSandro2000 added the 2.status: merge conflict label Jan 18, 2021

leotaku and others added 6 commits January 24, 2021 12:49

Reformat configureFlags list to span multiple lines

f41d476

Remove redundant semicolons

b60cf82

Update libmosquitto for new Netdata version

8bce22f

Co-authored-by: Marc Schmitt <marc.schmitt@risson.space>

Update build script for new Netdata version

edc637f

Co-authored-by: Marc Schmitt <marc.schmitt@risson.space>

Add comment to mosquitto.nix file

672612c

leotaku force-pushed the netdata-cloud branch from de325fb to 672612c Compare January 24, 2021 11:54

ofborg bot removed the 2.status: merge conflict label Jan 24, 2021

SuperSandro2000 reviewed Jan 24, 2021

View reviewed changes

pkgs/tools/system/netdata/mosquitto.nix Outdated Show resolved Hide resolved

Replace explicit buildPhase with makeFlags

cb06d3c

Use hardlinks for duplicate files in $out

a2c7bd3

SuperSandro2000 approved these changes Mar 4, 2021

View reviewed changes

leotaku requested a review from SuperSandro2000 March 13, 2021 09:09

rissson approved these changes Mar 24, 2021

View reviewed changes

leotaku closed this Apr 1, 2021

		cp -r "${mosquitto}/lib" externaldeps/mosquitto;
		cp -r "${libwebsockets_3_2}/lib" externaldeps/libwebsockets;

netdata: facilitate building netdata with cloud support #92782

netdata: facilitate building netdata with cloud support #92782

Conversation

leotaku commented Jul 9, 2020 • edited

Motivation for this change

Things done

leotaku commented Jul 9, 2020 • edited

leotaku commented Aug 8, 2020

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

jtojnar Aug 8, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

underhood Feb 26, 2021 • edited

Choose a reason for hiding this comment

rissson left a comment

Choose a reason for hiding this comment

SuperSandro2000 commented Nov 23, 2020

Choose a reason for hiding this comment

leotaku Dec 22, 2020 • edited

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

SuperSandro2000 commented Dec 22, 2020

rissson commented Jan 1, 2021

SuperSandro2000 commented Jan 25, 2021

SuperSandro2000 commented Jan 25, 2021

SuperSandro2000 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leotaku Mar 11, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

underhood commented Mar 22, 2021 • edited

SuperSandro2000 commented Mar 27, 2021

SuperSandro2000 commented Mar 27, 2021

leotaku commented Apr 1, 2021

leotaku commented Jul 9, 2020 •

edited

leotaku commented Jul 9, 2020 •

edited

jtojnar Aug 8, 2020 •

edited

underhood Feb 26, 2021 •

edited

leotaku Dec 22, 2020 •

edited

leotaku Mar 11, 2021 •

edited

underhood commented Mar 22, 2021 •

edited