initial minimal h2o webserver integration #14585

underhood · 2023-02-22T10:49:39Z

Summary

Uses h2o as webserver for netdata. Currently will run alongside (on port 19998) the old webserver (port 19999) if enabled.
By default disabled. Enable by netdata.conf:

[httpd]
  enable = yes

Recommended to use SSL with:

[httpd]
  ssl = yes

In such case if you do https://127.0.0.1:19998 you will get HTTP 2 support (which works only over SSL).

Currently biggest limitation is no support for streaming. I will have to investigate how to do this without modifying h2o.

TODO:

children support
strange bug with content type not correct after refresh
CI/packaging fixes - i redid it to not use external cmake and build libh2o.a ourselves instead

This also uncovered multiple bugs on local dashboard code e.g.:

sometimes there is double slash in URL 127.0.0.1//api/v1/info that is requested by dashboar
if child dashboard is requested by url host/uuid instead of host/uuid/ our new webserver handles both url correctly (by sending index page for a chile) but dashboard will send subsequent request wrong if former URL form is used.

Test Plan

Additional Information

For users: How does this change affect me?

stelfrag · 2023-02-22T15:00:03Z

To enable it as I see its [httpd] section with enabled = yes

underhood · 2023-02-23T05:07:47Z

Yep i have to also fix couple of bugs and child support

stelfrag · 2023-02-24T11:18:11Z

Yep i have to also fix couple of bugs and child support

Can you add a small list of pending tasks in the description so that we can follow along?

underhood · 2023-02-25T06:29:37Z

Yep i have to also fix couple of bugs and child support

Can you add a small list of pending tasks in the description so that we can follow along?

added, child support is working now (I redid it with "uberhandler" as opposed to registering paths for every child as I had it working previously, this way it is more effective)

underhood · 2023-02-25T11:29:34Z

~~@Ferroin looks like I will need a bit of a help with packaging stuff here~~
I am changing the so that we build the libh2o ourselves instead of calling their cmake build.

underhood · 2023-02-27T11:00:12Z

the only remaining single failure in CI is Build / Build Static (ppc64le) (pull_request) I am ignoring it because when I install alpine in QEMU emulating ppc64le netdata builds successfuly.

andrewm4894 · 2023-02-27T16:21:31Z

@underhood would it make sense to add this to buildinfo or anything like that if there could be some period of time where both old and new webserver can be run in parallel by users?

Ferroin · 2023-02-27T16:32:38Z

the only remaining single failure in CI is Build / Build Static (ppc64le) (pull_request) I am ignoring it because when I install alpine in QEMU emulating ppc64le netdata builds successfuly.

It’s failing on 64-bit ARM due to undefined function references, not on POWER (the POWER build job is just getting cancelled due to the 64-bit ARM build failing). Link to the failing logs for the most recent build as-of this comment: https://github.com/netdata/netdata/actions/runs/4279728307/jobs/7455048379

That said, this needs to be fixed before this gets merged, period. Without the all the static builds working, we do not publish any static builds for nightlies or releases (or any source tarballs for that matter), due to limitations in how GitHub Actions dependency handling between jobs works.

Ferroin

Is there some reason the Makefile components need to be in the main Makefile instead of a sub-directory for the new web server?
Is there a particular reason that we are not even trying to support using a system copy of libh2o? I get that this may be a bit tricky, but I was under the impression that it was understood at this point that we need to have such support for any dependency that is both widely available and does not involve a custom patches (and AFAICT, libh2o fits both requirements) so that the few cases of third-party packaging that we do functionally support will actually build the relevant features. The fact that libh2o is going to ultimately be a mandatory dependency does not sidestep that issue with vendoring.

.gitmodules

underhood · 2023-02-28T02:32:41Z

the only remaining single failure in CI is Build / Build Static (ppc64le) (pull_request) I am ignoring it because when I install alpine in QEMU emulating ppc64le netdata builds successfuly.

It’s failing on 64-bit ARM due to undefined function references, not on POWER (the POWER build job is just getting cancelled due to the 64-bit ARM build failing). Link to the failing logs for the most recent build as-of this comment: https://github.com/netdata/netdata/actions/runs/4279728307/jobs/7455048379

That said, this needs to be fixed before this gets merged, period. Without the all the static builds working, we do not publish any static builds for nightlies or releases (or any source tarballs for that matter), due to limitations in how GitHub Actions dependency handling between jobs works.

EDIT: just noticed it is failing 64-bit arm now, previously it was always failing ppc64le. so yep I will have to investigate this and fix.

Yes I see the failure in CI. That is why I installed Alpine in QEMU on ppc64le and tried to reproduce it. It built successfully. Also that error would indicate missing zlib however then we should fail at:

test "${with_zlib}" = "yes" -a "${have_zlib}" != "yes" && AC_MSG_ERROR([zlib required but not found. Try installing 'zlib1g-dev' or 'zlib-devel'.])

~~so I do not know why this fails and why only on PPC~~

underhood · 2023-02-28T03:00:38Z

Is there a particular reason that we are not even trying to support using a system copy of libh2o? I get that this may be a bit tricky, but I was under the impression that it was understood at this point that we need to have such support for any dependency that is both widely available and does not involve a custom patches (and AFAICT, libh2o fits both requirements) so that the few cases of third-party packaging that we do functionally support will actually build the relevant features. The fact that libh2o is going to ultimately be a mandatory dependency does not sidestep that issue with vendoring.

I plan to add that in following PR. For now I went with what h2o itself recommends and that is static linking and intgrating h2o into the project

underhood · 2023-03-01T02:38:24Z

@Ferroin how to see here https://github.com/netdata/netdata/actions/runs/4279728307/jobs/7455048379
how netdata was configured (how the ./configure was called) and its output?
In past this was relatively easy to see, but now it is unclear to me what exactly this build step is doing/executing and also looking at .github files it is quite hard to follow.

underhood · 2023-03-01T05:00:23Z

after latest changes I seem to have correct behavior:

arch/distro	zlib-dev installed	--with-zlib (1) / without (0)	build result	netdata buildinfo HTTPD	Correct result
aarch64(RPi4) ubuntu	1	1	1	1	✔️
aarch64(RPi4) ubuntu	1	0	1	0	✔️
aarch64(RPi4) ubuntu	0	1	0 (configure fails with zlib req but not present - not link error)	N/A	✔️
aarch64(RPi4) ubuntu	0	0	1	0	✔️
ppc64le(qemu) alpine-3.17.2	1	1	1	1	✔️
ppc64le(qemu) alpine-3.17.2	1	0	1	0	✔️
ppc64le(qemu) alpine-3.17.2	0	1	0 (configure fails with zlib req but not present - not link error)	N/A	✔️
ppc64le(qemu) alpine-3.17.2	0	0	1	0	✔️

Since I am not really sure what CI is doing here as it is using some caches etc. (by looking at the output it seems it is starting build from middle) and I dont see a way how to actually see commands used to build netdata and/or output of configure script I am not sure how to diagnose this.

underhood · 2023-03-01T08:19:03Z

@Ferroin @tkatsoulas
I am confused here. In qemu PPC64LE i get expected behaviour in all cases (see table above) with/without zlib yet it fails in CI.

I can't really follow the CI process here as it seems to use some kind of cache for the ppc64le. Can the problem be there, how was that cache generated? It looks like we are not doing full build here. How do I reproduce this locally?

thiagoftsm · 2023-03-01T12:18:54Z

Hello @underhood ,

I am starting testing your PR today, and I observed that we are having different warning when we compile with CFLAGS:

CFLAGS="-Og -ggdb -Wall -Wextra -fno-omit-frame-pointer -Wformat-signedness -fstack-protector-all -Wformat-truncation=2 -Wunused-result -DHAVE_BACKTRACE=1 -DNETDATA_TRACE_ALLOCATIONS=1 -DNETDATA_DEV_MODE=1 -DNETDATA_VERIFY_LOCKS=1"

In this file you have what was written in my stderr during compilation.

Of course the majority, if not all, are from H2O. Can we hide this?

underhood · 2023-03-01T12:51:35Z

Hello @underhood ,

I am starting testing your PR today, and I observed that we are having different warning when we compile with CFLAGS:
CFLAGS="-Og -ggdb -Wall -Wextra -fno-omit-frame-pointer -Wformat-signedness -fstack-protector-all -Wformat-truncation=2 -Wunused-result -DHAVE_BACKTRACE=1 -DNETDATA_TRACE_ALLOCATIONS=1 -DNETDATA_DEV_MODE=1 -DNETDATA_VERIFY_LOCKS=1"
In this file you have what was written in my stderr during compilation.

Of course the majority, if not all, are from H2O. Can we hide this?

I am already hiding some (mostly ssl 3 deprecation warnings) i can add

daemon/common.h

httpd/http_server.c

This reverts commit f18d57e.

underhood · 2023-04-21T14:52:12Z

@stelfrag fixed merge conflict caused by ML

BUT the WEBRTC PR went in meanwhile and changed the struct web_client so I will have to do some changes

underhood · 2023-04-21T15:03:18Z

ok should be updated

thiagoftsm

I only could retest this PR after disable ML, but issue is not related to it, LGTM!

underhood · 2023-04-27T12:29:19Z

thanks for review I will merge immediately after new release (to not delay current one in case of trouble)

github-actions bot added area/build Build system (autotools and cmake). area/daemon area/packaging Packaging and operating systems support labels Feb 22, 2023

underhood force-pushed the h2o_minimal_step1 branch from 81fcb36 to 2c6e6c6 Compare February 25, 2023 06:26

github-actions bot added area/ci and removed area/packaging Packaging and operating systems support labels Feb 27, 2023

underhood changed the title ~~[WIP] introduce h2o webserver~~ initial minimal h2o webserver integration Feb 27, 2023

underhood marked this pull request as ready for review February 27, 2023 06:52

underhood requested review from Ferroin, tkatsoulas, thiagoftsm and vkalintiris as code owners February 27, 2023 06:52

Ferroin reviewed Feb 27, 2023

View reviewed changes

.gitmodules Show resolved Hide resolved

thiagoftsm reviewed Mar 1, 2023

View reviewed changes

daemon/common.h Show resolved Hide resolved

thiagoftsm reviewed Mar 1, 2023

View reviewed changes

httpd/http_server.c Outdated Show resolved Hide resolved

underhood added 19 commits April 21, 2023 16:45

h2o a shallow copy by default

94a58c5

add HTTPD to buildinfo

04b367a

fix conf.ac

8adfd98

move HTTP related defines to libnetdata

5085888

use http coodes from httpd_defs.h

77d62af

use UUID_STR_LEN

c139fec

include only when built

497a170

usel TLS on newer SSL

6be2fbd

reduce amount of warnning produced by h2o compilation

6a61318

fix build error

7f5248d

Revert "h2o a shallow copy by default"

bbd0369

This reverts commit f18d57e.

try ipv6 if cant parse as ipv4

9e94f47

fix rebase mistake

b15d96a

sitch to evloop

7597908

internal evloop

619bc01

add missing stuff from merge

5acf239

codacy fix - reduce scope of rc

ff83daf

remove some warnings

5d36e89

minor - add log

cd29c78

underhood dismissed thiagoftsm’s stale review via cd29c78 April 21, 2023 14:49

underhood force-pushed the h2o_minimal_step1 branch from a6e15d9 to cd29c78 Compare April 21, 2023 14:49

update to new struct web_client

783a3d3

underhood requested review from thiagoftsm and stelfrag April 21, 2023 15:03

thiagoftsm approved these changes Apr 22, 2023

View reviewed changes

stelfrag approved these changes Apr 27, 2023

View reviewed changes

underhood merged commit 66c4735 into netdata:master May 10, 2023
132 of 133 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

initial minimal h2o webserver integration #14585

initial minimal h2o webserver integration #14585

underhood commented Feb 22, 2023 •

edited

stelfrag commented Feb 22, 2023

underhood commented Feb 23, 2023

stelfrag commented Feb 24, 2023

underhood commented Feb 25, 2023

underhood commented Feb 25, 2023 •

edited

underhood commented Feb 27, 2023 •

edited

andrewm4894 commented Feb 27, 2023

Ferroin commented Feb 27, 2023 •

edited

Ferroin left a comment

underhood commented Feb 28, 2023 •

edited

underhood commented Feb 28, 2023

underhood commented Mar 1, 2023 •

edited

underhood commented Mar 1, 2023 •

edited

underhood commented Mar 1, 2023 •

edited

thiagoftsm commented Mar 1, 2023 •

edited

underhood commented Mar 1, 2023

underhood commented Apr 21, 2023

underhood commented Apr 21, 2023

thiagoftsm left a comment •

edited

underhood commented Apr 27, 2023

initial minimal h2o webserver integration #14585

initial minimal h2o webserver integration #14585

Conversation

underhood commented Feb 22, 2023 • edited

Summary

Test Plan

Additional Information

stelfrag commented Feb 22, 2023

underhood commented Feb 23, 2023

stelfrag commented Feb 24, 2023

underhood commented Feb 25, 2023

underhood commented Feb 25, 2023 • edited

underhood commented Feb 27, 2023 • edited

andrewm4894 commented Feb 27, 2023

Ferroin commented Feb 27, 2023 • edited

Ferroin left a comment

Choose a reason for hiding this comment

underhood commented Feb 28, 2023 • edited

underhood commented Feb 28, 2023

underhood commented Mar 1, 2023 • edited

underhood commented Mar 1, 2023 • edited

underhood commented Mar 1, 2023 • edited

thiagoftsm commented Mar 1, 2023 • edited

underhood commented Mar 1, 2023

underhood commented Apr 21, 2023

underhood commented Apr 21, 2023

thiagoftsm left a comment • edited

Choose a reason for hiding this comment

underhood commented Apr 27, 2023

underhood commented Feb 22, 2023 •

edited

underhood commented Feb 25, 2023 •

edited

underhood commented Feb 27, 2023 •

edited

Ferroin commented Feb 27, 2023 •

edited

underhood commented Feb 28, 2023 •

edited

underhood commented Mar 1, 2023 •

edited

underhood commented Mar 1, 2023 •

edited

underhood commented Mar 1, 2023 •

edited

thiagoftsm commented Mar 1, 2023 •

edited

thiagoftsm left a comment •

edited