DataDog / nginx-datadog

Enhance NGINX Observability and Security with Datadog's Module
https://www.datadoghq.com
Apache License 2.0
22 stars 9 forks source link

openresty incompatibility #30

Open sterchelen opened 1 year ago

sterchelen commented 1 year ago

We want to run openresty nginx with nginx-datadog, though the 1.0.0 release mentions

OpenResty's Debian-based Docker image cannot load the module due to a build flag incompatibility.

Could you elaborate what is the build flag incompatibility? They seem to use --with-compat flag.

dgoffredo commented 1 year ago

That incompatibility comes from https://github.com/DataDog/nginx-datadog/pull/7.

I haven't revisited OpenResty since then.

It looks like OpenResty does use --with-compat now:

david@ein:~$ docker run -it --entrypoint=/bin/sh openresty/openresty:alpine-apk
Unable to find image 'openresty/openresty:alpine-apk' locally
alpine-apk: Pulling from openresty/openresty
0cdfa0c98ed7: Pull complete 
2ba00672bb2b: Pull complete 
e41d33bf378e: Pull complete 
24db290953b3: Pull complete 
Digest: sha256:35c1de1b33768acc7cc0a5d1fff3cfe73192718da74c607cfe3bb2b5b64176d1
Status: Downloaded newer image for openresty/openresty:alpine-apk
/ # which nginx
/usr/local/openresty/nginx/sbin/nginx
/ # nginx -V 2>&1 | grep arguments: | sed 's/ --/\n--/g'
configure arguments:
--prefix=/usr/local/openresty/nginx
--with-cc-opt='-O2 -DNGX_LUA_ABORT_AT_PANIC -I/usr/local/openresty/zlib/include -I/usr/local/openresty/pcre/include -I/usr/local/openresty/openssl111/include'
--add-module=../ngx_devel_kit-0.3.1
--add-module=../echo-nginx-module-0.62
--add-module=../xss-nginx-module-0.06
--add-module=../ngx_coolkit-0.2
--add-module=../set-misc-nginx-module-0.33
--add-module=../form-input-nginx-module-0.12
--add-module=../encrypted-session-nginx-module-0.09
--add-module=../srcache-nginx-module-0.32
--add-module=../ngx_lua-0.10.21
--add-module=../ngx_lua_upstream-0.07
--add-module=../headers-more-nginx-module-0.33
--add-module=../array-var-nginx-module-0.05
--add-module=../memc-nginx-module-0.19
--add-module=../redis2-nginx-module-0.15
--add-module=../redis-nginx-module-0.3.9
--add-module=../ngx_stream_lua-0.0.11
--with-ld-opt='-Wl,-rpath,/usr/local/openresty/luajit/lib -L/usr/local/openresty/zlib/lib -L/usr/local/openresty/pcre/lib -L/usr/local/openresty/openssl111/lib -Wl,-rpath,/usr/local/openresty/zlib/lib:/usr/local/openresty/pcre/lib:/usr/local/openresty/openssl111/lib'
--with-cc='ccache gcc -fdiagnostics-color=always -g3'
--with-pcre-jit
--with-stream
--with-stream_ssl_module
--with-stream_ssl_preread_module
--with-http_v2_module
--without-mail_pop3_module
--without-mail_imap_module
--without-mail_smtp_module
--with-http_stub_status_module
--with-http_realip_module
--with-http_addition_module
--with-http_auth_request_module
--with-http_secure_link_module
--with-http_random_index_module
--with-http_gzip_static_module
--with-http_sub_module
--with-http_dav_module
--with-http_flv_module
--with-http_mp4_module
--with-http_gunzip_module
--with-threads
--with-compat
--with-stream
--with-http_ssl_module
/ # 

It was added here: https://github.com/openresty/docker-openresty/blob/a997b75d58d0d7903a563720b762e8d747049092/CHANGELOG.md?plain=1#L302

due to this: https://github.com/openresty/docker-openresty/issues/114

But that's much earlier than when I played with it. Perhaps the relevant customer was using an older OpenResty.

Anyway, you can try using one of our release builds, but OpenResty is not yet supported. We're still figuring out what to do with the multiplicity of nginx builds out in the wild.

For the particular case of OpenResty, which is widely used, I might add another dimension to the build matrix that tracks these images: https://hub.docker.com/r/openresty/openresty. Was waiting for the need to arise.

If the closest current binary release doesn't work, then something like https://github.com/DataDog/nginx-datadog/pull/29 might.

We plan to add similar custom-build support for nginx installations not included in our binary releases, but that work has not yet begun.

sterchelen commented 1 year ago

Anyway, you can try using one of our release builds, but OpenResty is not yet supported.

we did try, but without any success. nginx ends in segmentation fault.

If the closest current binary release doesn't work, then something like https://github.com/DataDog/nginx-datadog/pull/29 might.

but we can't count on #29 as you said in the comment section right?

We plan to add similar custom-build support for nginx installations not included in our binary releases, but that work has not yet begun.

do you have an eta?

dgoffredo commented 1 year ago

we did try, but without any success. nginx ends in segmentation fault.

Bummer.

but we can't count on https://github.com/DataDog/nginx-datadog/pull/29 as you said in the comment section right?

The problem with OpenResty is that they patch the nginx source code. So, in general, nginx modules built against the upstream nginx source tree are incompatible with nginx modules built against the post-patch OpenResty version of the sources. In terms of binary compatibility, they really only differ in a few ways (explored in the issue linked above), but better would be to build against the patched sources, and with the same nginx ./configure flags as are used when building OpenResty.

do you have an eta?

I don't. And I don't mean to sound dismissive. We did not recognize the need to address OpenResty until just now, after our quarter 3 planning. I'll see what I can get done in the next couple of months, but adding a new build dimension has previously been a lot of work (mostly testing and CI-bashing).

dudo commented 1 year ago

We ran into this issue as well. We currently use the opentracing plugin, but want to switch to the datadog specific library in order to use the per request datadog_sample_rate tooling.

dgoffredo commented 1 year ago

@sterchelen @dudo

Alright, let me play with this. But first, I need to know the precise OpenResty setup you're using.

Please get the following information:

Thanks.

dudo commented 1 year ago
$ uname -a
Linux 80d24dff7006 5.15.49-linuxkit-pr #1 SMP Thu May 25 07:17:40 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

$ cat /etc/os-release
NAME="Ubuntu"
VERSION="20.04.6 LTS (Focal Fossa)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 20.04.6 LTS"
VERSION_ID="20.04"
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
VERSION_CODENAME=focal
UBUNTU_CODENAME=focal

$ nginx -V
nginx version: openresty/1.19.9.1
built by gcc 9.4.0 (Ubuntu 9.4.0-1ubuntu1~20.04.1) 
built with OpenSSL 1.1.1l  24 Aug 2021
TLS SNI support enabled
configure arguments: --prefix=/usr/local/openresty/nginx --with-cc-opt=-O2 --add-module=../ngx_devel_kit-0.3.1 --add-module=../echo-nginx-module-0.62 --add-module=../xss-nginx-module-0.06 --add-module=../ngx_coolkit-0.2 --add-module=../set-misc-nginx-module-0.32 --add-module=../form-input-nginx-module-0.12 --add-module=../encrypted-session-nginx-module-0.08 --add-module=../srcache-nginx-module-0.32 --add-module=../ngx_lua-0.10.20 --add-module=../ngx_lua_upstream-0.07 --add-module=../headers-more-nginx-module-0.33 --add-module=../array-var-nginx-module-0.05 --add-module=../memc-nginx-module-0.19 --add-module=../redis2-nginx-module-0.15 --add-module=../redis-nginx-module-0.3.7 --add-module=../rds-json-nginx-module-0.15 --add-module=../rds-csv-nginx-module-0.09 --add-module=../ngx_stream_lua-0.0.10 --with-ld-opt=-Wl,-rpath,/usr/local/openresty/luajit/lib --with-openssl=/tmp/openssl-1.1.1l --with-pcre=/tmp/pcre-8.44 --with-file-aio --with-http_addition_module --with-http_auth_request_module --with-http_dav_module --with-http_flv_module --with-http_geoip_module=dynamic --with-http_gunzip_module --with-http_gzip_static_module --with-http_image_filter_module=dynamic --with-http_mp4_module --with-http_random_index_module --with-http_realip_module --with-http_secure_link_module --with-http_slice_module --with-http_ssl_module --with-http_stub_status_module --with-http_sub_module --with-http_v2_module --with-http_xslt_module=dynamic --with-ipv6 --with-mail --with-mail_ssl_module --with-md5-asm --with-pcre-jit --with-sha1-asm --with-stream --with-stream_ssl_module --with-threads --add-dynamic-module=/usr/local/nginx-opentracing/opentracing --with-openssl-opt=-g --with-pcre-opt=-g --with-stream --with-stream_ssl_preread_module
dgoffredo commented 1 year ago

There is a rabbit hole to go down.

Here's what I tried:

Dockerfile

from openresty/openresty:1.19.9.1-focal

run apt-get install -y wget && \
  cd /tmp && \
  wget 'https://github.com/DataDog/nginx-datadog/releases/download/v1.0.1/nginx_1.19.9-ngx_http_datadog_module.so.tgz' && \
  tar -xzf nginx_1.19.9-ngx_http_datadog_module.so.tgz && \
  mv ngx_http_datadog_module.so /usr/local/openresty/nginx/modules

docker-compose.yaml

services:

  nginx:
    build:
      context: ./
      dockerfile: ./Dockerfile
    ports:
      - '127.0.0.1:8080:80'
    volumes:
      - './nginx.conf:/usr/local/openresty/nginx/conf/nginx.conf:ro'
    environment:
      - DD_ENV=prod
      - DD_AGENT_HOST=dd-agent
    depends_on:
      - dd-agent

  dd-agent:
    image: 'datadog/agent'
    volumes:
      - '/var/run/docker.sock:/var/run/docker.sock:ro'
      - '/run/user:/run/user:ro'
      - '/proc/:/host/proc/:ro'
      - '/sys/fs/cgroup/:/host/sys/fs/cgroup:ro'
    environment:
      - 'DD_APM_ENABLED=true'
      - DD_API_KEY
      - DOCKER_HOST
      - 'DD_LOG_LEVEL=ERROR'

nginx.conf

load_module modules/ngx_http_datadog_module.so;

# TODO: The Datadog module's technique for automatically forwarding environment
# variables to worker processes is not working in OpenResty.
# env POOP;
# env DD_AGENT_HOST;

events {
    worker_connections  1024;
}

http {
    include       mime.types;
    default_type  application/octet-stream;
    client_body_temp_path /var/run/openresty/nginx-client-body;
    proxy_temp_path       /var/run/openresty/nginx-proxy;
    fastcgi_temp_path     /var/run/openresty/nginx-fastcgi;
    uwsgi_temp_path       /var/run/openresty/nginx-uwsgi;
    scgi_temp_path        /var/run/openresty/nginx-scgi;
    sendfile        on;
    keepalive_timeout  65;
    include /etc/nginx/conf.d/*.conf;

    datadog_agent_url http://dd-agent:8126;

    server {
        listen       80;

        location / {
            root   /usr/local/openresty/nginx/html;
            index  index.html index.htm;
        }
    }
}

That works, but the module is not forwarding the environment variables DD_ENV and DD_AGENT_HOST to the worker processes.

If you remove the datadog_agent_url directive in nginx.conf, then the tracer will not send traces to Datadog, because it will try to connect to localhost instead of to dd-agent.

Here's the kicker. If you uncomment the env directives in nginx.conf, then openresty segfaults. In fact, any env directive causes openresty to segfault.

I have no idea why. I'll have to look into this more another time. For now, OpenResty support is a feature request.

jiz4oh commented 11 months ago

Hi @dgoffredo @dudo, we also find the interesting mention, finally we successfully run openresty nginx with nginx-datadog, the only thing we have meeting is openresty is not realized environment correctly that dgoffredo mentioned and we resolve it by envsubst.

docker-entrypoint.sh

#!/usr/bin/env sh

set -eu

export DD_AGENT_HOST="${DD_AGENT_HOST:-127.0.0.1}"
export DD_ENV="${DD_ENV:unknown}"
export DD_SERVICE="${DD_SERVICE:unknown}"

envsubst '${DD_AGENT_HOST} ${DD_ENV} ${DD_SERVICE}' </etc/nginx/conf.d/default.conf.template >/usr/local/openresty/nginx/conf/nginx.conf

exec "$@"

/etc/nginx/conf.d/default.conf.template

...
http {
    datadog_agent_url http://${DD_AGENT_HOST}:8126;
    datadog_environment ${DD_ENV};
    datadog_service_name ${DD_SERVICE};
...
dgoffredo commented 11 months ago

Hi @jiz4oh,

Whatever causes env to crash could be indicative of a deeper issue with nginx-datadog + OpenResty. Use it at your own risk, as it is not yet supported. OpenResty support is one of the things that we'd like to add, but it doesn't look like it will make it into our project planning for what remains of the year.

joshuabaird commented 8 months ago

Hi @dgoffredo! Any movement on official support for OpenResty?

dgoffredo commented 8 months ago

Nothing yet, Josh, but thanks for checking in.

The work was not prioritized in our planning for the first quarter of 2024, but it's possible that someone on the team will be able to revisit it.

msarm-1debit commented 4 months ago

Hi @dgoffredo! Any movement on official support for OpenResty?

dgoffredo commented 4 months ago

You'll have to ask @dmehala, because I'm no longer a maintainer of this project.

dmehala commented 3 months ago

Hi @msarm-1debit

OpenResty support is not prioritized for the current quarter. Instead, I planned to increase the feature set of our Kong plugin by using dd-trace-cpp bindings. This has the positive side effect of also providing bindings to LuaJIT, which could be reused to support OpenResty.

I'll experiment with OpenResty to assess the feasibility. If it's successful, priority might shift. IMO, it's better to sink your teeth into a new integration than something that's already working and could have a negative impact on customers.

I strongly suggest to open a feature request here for visibility.

I'll keep you posted.