Allow linking Nginx modules dynamically

KFearsoff commented 11 months ago

Issue description

Currently, Nginx allows linking modules only statically:

https://github.com/NixOS/nixpkgs/blob/d0efa70d8114756ca5aeb875b7f3cf6d61543d62/pkgs/servers/http/nginx/generic.nix#L117

However, Nginx doesn't have feature parity between static and dynamic modules. This means that modules that don't support static linking don't work on NixOS.

Steps to reproduce

Apply a patch that reverts #173721 (see reasoning in https://github.com/NixOS/nixpkgs/issues/182935 )
Build Nginx with opentracing module
Try loading the module

Unless I'm missing something, you can't do that: https://github.com/opentracing-contrib/nginx-opentracing/issues/162 Interestingly, nginx-otel module that we don't have in Nixpkgs also can only be installed dynamically: https://github.com/nginxinc/nginx-otel/issues/9

Technical details

Thankfully, the difference in how dynamic and static compiling is done is minimal. Static compilation uses --add-module, while dynamic compilation uses --add-dynamic-module . I haven't verified if it works yet, but I'll run some tests and add an update in a few days unless someone beats me to it.

For the Nginx package, it only requires adding a new optional argument dynamicModules (similar to modules). For the Nginx NixOS module, it requires adding a new option or extending existing one.

RaitoBezarius commented 11 months ago

Please consider sending a PR, I don't think the NGINX maintenance team have the bandwidth to drive this change by themselves. We can surely try to review it.

Also please do not use assignment, we are maintainers, we do not owe feature requests. Just mention us.

KFearsoff commented 11 months ago

Right. Sorry, the Github UI confused me here for a sec, as there are "assignees" and "reviewers" in the same place.

I'll send a PR as soon as I can.

osokin commented 11 months ago

Historically www/nginx and www/nginx-devel ports on FreeBSD built all third-party modules statically only, later all of those modules, including nginx-otel, https://github.com/osokin/nginx-otel/tree/no-cmake, can be built both statically and dynamically.

KFearsoff commented 11 months ago

So uhh.

I did create a patch that allows dynamic linking. The problem is Nginx modules themselves.

https://github.com/opentracing-contrib/nginx-opentracing is a bit old: OpenTracing was merged into OpenTelemetry last year. https://github.com/nginxinc/nginx-otel is seemingly packaged in a legacy way, so it requires the --with-contrib flag which I'm too scared to touch as no module in Nixpkgs is packaged like that. https://github.com/open-telemetry/opentelemetry-cpp-contrib/tree/main/instrumentation/nginx is... tolerable, but it needs to be built with opentracing-cpp which we don't have packaged, and requires and esoteric build flag too.

If anyone is interested in the dynamic linking itself, I'm happy to submit a PR, but my main motivation for getting Opentelemetry thing to completion (which drove me to create this issue) basically vanished lol. Will try to get into Caddy I guess.

leiserfg commented 5 months ago

@KFearsoff did you manage to make nginx-otel work with nix?

KFearsoff commented 5 months ago

@leiserfg No. Truth be told, I haven't looked at it in more depth after the last comment on this issue. From a brief look, it seems like everything stayed the same way. I have also tried running Nginx with Otel in Kubernetes, but it was an extremely janky setup. I'm guessing nobody actually uses Nginx with Otel.

ninaforsyth commented 5 months ago

@KFearsoff Hello, I work at NGINX F5 and would love to make the Otel model less janky. Do you have any feedback on the problems or how we might be able to make it better?

KFearsoff commented 3 months ago

@ninaforsyth Hey, sorry for the late reply. I was meaning to write a comprehensive reply, but it was really hard for the past two months.

The main issue I've had is no support for static linking. Notably, this is very inconvenient for Nixpkgs. From what I can tell, it is still unsupported. This is also a problem for ingress-nginx project: from what I can tell, instead of modifying the basic build image, they instead do some arcane hackery with building a separate container image that compiles the Otel module into some directory that is assumed to be a volume that will also be mounted by the primary Nginx container: https://github.com/kubernetes/ingress-nginx/blob/57d96128b10f8d24308f7055d8f5ed9b79903ffc/charts/ingress-nginx/values.yaml#L706-L729

NixOS / nixpkgs