OTEL build fixes #1527

ac000 · 2025-01-07T00:38:55Z

This series of commits address a number of build issues with the OTEL support in Unit.

The first commit f1afcf5b auto/otel: Make use of nxt_feature_name fixes an issue with systems that don't have Linux's sched_getaffinity(2).

The second commit 3b5542ed auto/make: s/NXT_OTEL_LIB_LOC/NXT_OTEL_LIB_STATIC/ is a preparatory cleanup patch that renames a variable to better match existing naming conventions.

The third commit e2c2c901 auto/make, otel: fix linking on macOS and Ubuntu fixes an issue with how the OTEL static lib was being linked in. This is a patch from @thresheek

The fourth commit e40a3ca8 auto/make: Add missing NXT_OTEL_LIB_STATIC to some C tests fixes an issue with building some of the C tests, it's somewhat related to the above and is a patch from @remicollet

The fifth commit 99068620 auto/otel: don't look for OpenSSL on Darwin fixes an issue with building OTEL on macOS. This is a patch from @thresheek

The sixth commit 0397301e auto/make: Make GNU make mandatory fixes an issue with building OTEL with non-GNU make, by making GNU make mandatory (it's really the only sane path forward)

The following changes since commit 706b994fb586330828ff7d445c60e39390cc35c6:

  Version bump (2024-12-19 20:56:24 +0000)

are available in the Git repository at:

  [email protected]:ac000/unit.git otel-build-fixes

for you to fetch changes up to 0397301e12857acf1dc9c843ede5617847c16649:

  auto/make: Make GNU make mandatory (2025-01-07 00:17:37 +0000)

----------------------------------------------------------------
Andrew Clayton (3):
      auto/otel: Make use of nxt_feature_name
      auto/make: s/NXT_OTEL_LIB_LOC/NXT_OTEL_LIB_STATIC/
      auto/make: Make GNU make mandatory

Konstantin Pavlov (2):
      auto/make, otel: fix linking on macOS and Ubuntu
      auto/otel: don't look for OpenSSL on Darwin

Remi Collet (1):
      auto/make: Add missing NXT_OTEL_LIB_STATIC to some C tests

 auto/make | 43 +++++++++++++++++++------------------------
 auto/otel | 56 +++++++++++++++++++++++++++++++++-----------------------
 2 files changed, 52 insertions(+), 47 deletions(-)

ac000 · 2025-01-07T00:49:43Z

This supplants #1520

Build tested on Fedora, Debian, FreeBSD & macOS with and without --otel

thresheek · 2025-01-07T00:55:58Z

Also tested on all supported platforms in pkg/

thresheek

Looks good to me. I'm not sure about the gmake dependency part, though, but it's better to be explicit rather than fail during the build process.

Maybe @osokin has some ideas on how to fix that properly.

avahahn

Not sure if this is expected behavior or not: make tests does not seem to be building libotel.a by default anymore, so if you execute the following you will get an error:

./configure --otel --openssl --tests
make -j8 tests

However if you make first then you will not have issues with make tests.
This was found on an Ubuntu arm64 VM.

ac000 · 2025-01-07T04:22:09Z

Fix ./configure --otel --tests ... && make tests. I.e make tests without building unit etc first...

$ git range-diff 0397301e...70d0683f
1:  e40a3ca8 ! 1:  da917411 auto/make: Add missing NXT_OTEL_LIB_STATIC to some C tests
    @@ Commit message
         Signed-off-by: Andrew Clayton <[email protected]>
     
      ## auto/make ##
    -@@ auto/make: $NXT_BUILD_DIR/tests: \$(NXT_TEST_OBJS) \\
    +@@ auto/make: tests:               $NXT_BUILD_DIR/tests $NXT_BUILD_DIR/utf8_file_name_test \\
    +                   $NXT_BUILD_DIR/unit_websocket_echo
    + 
    + $NXT_BUILD_DIR/tests: \$(NXT_TEST_OBJS) \\
    +-                  $NXT_BUILD_DIR/lib/$NXT_LIB_STATIC
    ++                  $NXT_BUILD_DIR/lib/$NXT_LIB_STATIC \\
    ++                  \$(NXT_OTEL_LIB_STATIC)
        \$(PP_LD) \$@
        \$(v)\$(NXT_EXEC_LINK) -o $NXT_BUILD_DIR/tests \\
                \$(CFLAGS) \$(NXT_TEST_OBJS) \\
    @@ auto/make: $NXT_BUILD_DIR/tests: \$(NXT_TEST_OBJS) \\
                $NXT_LD_OPT $NXT_LIBM $NXT_LIBS $NXT_LIB_AUX_LIBS
      
      $NXT_BUILD_DIR/utf8_file_name_test: $NXT_LIB_UTF8_FILE_NAME_TEST_SRCS \\
    -@@ auto/make: $NXT_BUILD_DIR/utf8_file_name_test: $NXT_LIB_UTF8_FILE_NAME_TEST_SRCS \\
    +-                  $NXT_BUILD_DIR/lib/$NXT_LIB_STATIC
    ++                  $NXT_BUILD_DIR/lib/$NXT_LIB_STATIC \\
    ++                  \$(NXT_OTEL_LIB_STATIC)
    +   \$(PP_LD) \$@
        \$(v)\$(CC) \$(CFLAGS) \$(NXT_LIB_INCS) $NXT_LIB_AUX_CFLAGS \\
                -o $NXT_BUILD_DIR/utf8_file_name_test \\
                $NXT_LIB_UTF8_FILE_NAME_TEST_SRCS \\
    @@ auto/make: $NXT_BUILD_DIR/utf8_file_name_test: $NXT_LIB_UTF8_FILE_NAME_TEST_SRCS
                $NXT_LD_OPT $NXT_LIBM $NXT_LIBS $NXT_LIB_AUX_LIBS
      
      $NXT_BUILD_DIR/ncq_test: $NXT_BUILD_DIR/src/test/nxt_ncq_test.o \\
    -@@ auto/make: $NXT_BUILD_DIR/ncq_test: $NXT_BUILD_DIR/src/test/nxt_ncq_test.o \\
    +-                  $NXT_BUILD_DIR/lib/$NXT_LIB_STATIC
    ++                  $NXT_BUILD_DIR/lib/$NXT_LIB_STATIC \\
    ++                  \$(NXT_OTEL_LIB_STATIC)
        \$(PP_LD) \$@
        \$(v)\$(NXT_EXEC_LINK) -o $NXT_BUILD_DIR/ncq_test \\
                \$(CFLAGS) $NXT_BUILD_DIR/src/test/nxt_ncq_test.o \\
    @@ auto/make: $NXT_BUILD_DIR/ncq_test: $NXT_BUILD_DIR/src/test/nxt_ncq_test.o \\
                $NXT_LD_OPT $NXT_LIBM $NXT_LIBS $NXT_LIB_AUX_LIBS
      
      $NXT_BUILD_DIR/vbcq_test: $NXT_BUILD_DIR/src/test/nxt_vbcq_test.o \\
    -@@ auto/make: $NXT_BUILD_DIR/vbcq_test: $NXT_BUILD_DIR/src/test/nxt_vbcq_test.o \\
    +-                  $NXT_BUILD_DIR/lib/$NXT_LIB_STATIC
    ++                  $NXT_BUILD_DIR/lib/$NXT_LIB_STATIC \\
    ++                  \$(NXT_OTEL_LIB_STATIC)
        \$(PP_LD) \$@
        \$(v)\$(NXT_EXEC_LINK) -o $NXT_BUILD_DIR/vbcq_test \\
                \$(CFLAGS) $NXT_BUILD_DIR/src/test/nxt_vbcq_test.o \\
2:  99068620 = 2:  6c005a00 auto/otel: don't look for OpenSSL on Darwin
3:  0397301e = 3:  70d0683f auto/make: Make GNU make mandatory

ac000 · 2025-01-09T03:33:18Z

OK, so here's another issue!...

On master

$ make -j8 D=1
...
cargo build --debug --manifest-path src/otel/Cargo.toml
error: unexpected argument '--debug' found

  tip: `--debug` is the default for `cargo build`; instead `--release` is supported

Usage: cargo build [OPTIONS]

For more information, try '--help'.

Anyway as I mentioned to @osokin I have an idea to keep BSD make working and will get rid of this issue... It'll mean doing a --release build of otel by default, and if you want to do a --debug build you'll need to do it using GNU make...

ac000 · 2025-01-09T05:28:27Z

Re-do the last commit to keep BSD make working

$ git range-diff --creation-factor=100 70d0683f...4fe4f788
1:  70d0683f ! 1:  4fe4f788 auto/make: Make GNU make mandatory
    @@ Metadata
     Author: Andrew Clayton <[email protected]>
     
      ## Commit message ##
    -    auto/make: Make GNU make mandatory
    +    auto/make: Fix various issues with building OTEL
     
    -    There is breakage on non-GNU make systems due to the OTEL stuff using
    -    'ifeq'.
    +    There were at least a couple of issues with building OTEL support.
     
    -    We had previously handled this by only using such constructs (and
    -    enabling certain features, such as D= & E=) when using GNU make.
    +    It only worked with GNU make due to the use of ifeq, even gmake had some
    +    issues.
     
    -    OTEL added this unconditionally. It's used to set the target build
    -    directory appropriately depending on whether we are doing debug or
    -    release builds.
    +    Debug builds were broken due to trying to pass --debug to cargo which is
    +    the default and isn't a valid option.
     
    -    Unfortunately it seems there is no way to control this via cargo/rustc
    -    command line arguments or environment variables.
    +    This 'fixes' things by doing 'release' builds of OTEL by default.
     
    -    For BSD make we could use '.if' etc but then trying to handle these two
    -    systems (and possibly more) in the build scripts is going to be
    -    unwieldy.
    +    Passing D=1 to make will generate 'debug' builds but this as previously
    +    with D= etc, only works with GNU make.
     
    -    Let's just bite the bullet and make (no pun intended) GNU make mandatory
    -    to build Unit, it's the default on Linux (I'm sure there's an exception
    -    somewhere...) and it seems the default on macOS. On other systems it's
    -    readily available via gmake(1).
    +    We make use the of the '--emit link=' rustc option to place the
    +    libotel.a static library into build/lib
     
    -    This also brings build feature parity to all systems out the box.
    +    This is good, it consolidates the static libraries into one place and it
    +    simplifies the build scripts.
     
    -    As it happens, this also fixes issues with using gmake...
    +    While we're at it pretty print the cargo command by default.
     
         Fixes: 9d3dcb800 ("otel: add build tooling to include otel code")
         Link: <https://github.com/nginx/unit/pull/1520#issuecomment-2556265063>
         Signed-off-by: Andrew Clayton <[email protected]>
     
      ## auto/make ##
    -@@ auto/make: manpage: $NXT_BUILD_DIR/share/man/man8/unitd.8
    +@@ auto/make: PP_AR                := @echo '  AR    '
    + PP_LD             := @echo '  LD    '
    + PP_VER            := @echo '  VER   '
    + PP_SED            := @echo '  SED   '
    ++PP_CR             := @echo '  CR    '
      
    - END
    + CC =                      $CC
    + AR =                      $AR
      
    --
    --NXT_OS=$(uname -s)
    --NXT_GNU_MAKE=$(make --version | grep GNU || true)
    --
    --# Requires GNU make. On OpenIndiana at least we have to use gmake
    --if [ -n "$NXT_GNU_MAKE" ] || [ $NXT_OS = "SunOS" ]; then
    --
    --  cat << END >> $NXT_MAKEFILE
    -+cat << END >> $NXT_MAKEFILE
    - # By default compiler etc output is hidden, use
    - #   make V=1 ...
    - # to show it.
    -@@ auto/make: endif
    + EXTRA_CFLAGS =
    + CFLAGS =          $NXT_CFLAGS $NXT_CC_OPT $CFLAGS \$(EXTRA_CFLAGS)
    +-RUST_FLAGS =
    +-NXT_OTEL_LIB_STATIC =
      
    - END
    + NXT_EXEC_LINK =           $NXT_EXEC_LINK $NXT_LD_OPT
    + NXT_SHARED_LOCAL_LINK =   $NXT_SHARED_LOCAL_LINK $NXT_LD_OPT
    + NXT_MODULE_LINK = $NXT_MODULE_LINK
      
    --fi
    --
    - # potentially set otel lib location
    - if [ $NXT_OTEL = YES ]; then
    ++NXT_OTEL_LIB_STATIC =
     +
    - cat << END >> $NXT_MAKEFILE
    + all: $NXT_DAEMON manpage
    + 
    + .PHONY: $NXT_DAEMON manpage
    +@@ auto/make: manpage: $NXT_BUILD_DIR/share/man/man8/unitd.8
    + END
    + 
    + 
    ++if [ $NXT_OTEL = YES ]; then
    ++
    ++  cat << END >> $NXT_MAKEFILE
    ++
    ++RUST_FLAGS =              --release
    ++NXT_OTEL_LIB_STATIC =     $NXT_BUILD_DIR/lib/libotel.a
    ++
    ++END
    ++
    ++fi
    ++
    ++
    + NXT_OS=$(uname -s)
    + NXT_GNU_MAKE=$(make --version | grep GNU || true)
    + 
    +@@ auto/make: D := 0
      
      ifeq (\$D,1)
    -@@ auto/make: else
    +         CFLAGS += -O0
    +-        RUST_FLAGS += --debug
    +-else
    +-        RUST_FLAGS += --release
    ++        RUST_FLAGS =
      endif
      
    - END
    -+
    + # Optionally disable -Werror with
    +@@ auto/make: END
    + 
      fi
      
    +-# potentially set otel lib location
    +-if [ $NXT_OTEL = YES ]; then
    +-cat << END >> $NXT_MAKEFILE
    +-
    +-ifeq (\$D,1)
    +-  NXT_OTEL_LIB_STATIC = $NXT_OTEL_LIB_DIR/target/debug/libotel.a
    +-else
    +-  NXT_OTEL_LIB_STATIC = $NXT_OTEL_LIB_DIR/target/release/libotel.a
    +-endif
    +-
    +-END
    +-fi
    + 
      # The include paths list.
    + 
    +@@ auto/make: NXT_OTEL_DEPS=" \
    +     cat << END >> $NXT_MAKEFILE
    + 
    + \$(NXT_OTEL_LIB_STATIC): $NXT_OTEL_DEPS
    +-  cargo build \$(RUST_FLAGS) --manifest-path $NXT_OTEL_LIB_DIR/Cargo.toml
    ++  \$(PP_CR) \$@
    ++  \$(v)cargo rustc \$(RUST_FLAGS) \\
    ++          --manifest-path $NXT_OTEL_LIB_DIR/Cargo.toml \\
    ++          -- --emit link=../../$NXT_BUILD_DIR/lib/libotel.a
    + END
    + fi

osokin · 2025-01-09T14:45:42Z

Re-do the last commit to keep BSD make working

This looks good to me, thank you.

Another issue is related to cargo build procedure. FreeBSD uses poudriere(8) package builder to build ports, and network connectivity is unavailable after the fetch phase (phases are: fetch -> patch -> configure -> build -> install, all have pre- and post- targets). I do believe we may follow a similar to unit-java module procedure for the case. I'm glad to hear your thoughts on that.

Thank you in advance.

remicollet · 2025-01-09T15:28:18Z

and network connectivity is unavailable after the fetch

Same issue with RPM build (which are done offline, especially required for reproducible builds)
A solution is to vendor cargo dependencies (not yet done for unit, but already used in some other packages)

osokin · 2025-01-09T16:06:57Z

and network connectivity is unavailable after the fetch

Same issue with RPM build (which are done offline, especially required for reproducible builds) A solution is to vendor cargo dependencies (not yet done for unit, but already used in some other packages)

Yeah, so I've created a separate port specifically for the Unit's OTel, devel/unit-otel.

thresheek

If it's good enough for @osokin it's good enough for me 🙂

I've checked the builds for Linux packages we have, and it's also good. Same on macOS side.

ac000 · 2025-01-10T04:15:26Z

@osokin @remicollet

I've created a new issue to track the off-line builds issue and added your comments there...

When building with --otel on macOS for example I was seeing compile failures with the cpu_set_t stuff which should only be used under Linux. It turned out that despite checking for Linux sched_getaffinity() ... not found we were getting #ifndef NXT_HAVE_LINUX_SCHED_GETAFFINITY #define NXT_HAVE_LINUX_SCHED_GETAFFINITY 1 #endif in build/include/nxt_auto_config.h It seems this was due to the . auto/feature in auto/otel, this check happens right after the above. Without having nxt_feature_name=NXT_HAVE_OTEL set. Instead we were adding the define for that manually. Doing auto/feature without having a nxt_feature_name must have used the last set one and enabled it. Set nxt_feature_name and remove the manual editing of nxt_auto_config.h Fixes: 9d3dcb8 ("otel: add build tooling to include otel code") Signed-off-by: Andrew Clayton <[email protected]>

This better matches existing naming convention, e.g NXT_LIB_STATIC Fixes: 9d3dcb8 ("otel: add build tooling to include otel code") Signed-off-by: Andrew Clayton <[email protected]>

The static library is supposed to be specified prior to its dependencies. Also, no need to put an otel static library inside libnxt static library, as we explicitely link unit binary with otel static library anyway. This fixes the following build problems: - macOS: Finished `release` profile [optimized] target(s) in 58.07s AR build/lib/libnxt.a LD build/sbin/unitd ld: archive member 'libotel.a' not a mach-o file in '/private/tmp/unit-20241219-8965-yb46xp/build/lib/libnxt.a' clang: error: linker command failed with exit code 1 (use -v to see invocation) - Ubuntu 22 (./configure --otel): LD build/sbin/unitd cc -Wl,-E -o build/sbin/unitd -pipe -fPIC -fvisibility=hidden -fno-strict-overflow -funsigned-char -std=gnu11 -O -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -fno-strict-aliasing -Wmissing-prototypes -Werror -g \ build/src/nxt_main.o build/lib/libnxt.a \ -lm -lrt -lpthread \ \ -lpcre2-8 -lssl -lcrypto src/otel/target/release/libotel.a /usr/bin/ld: src/otel/target/release/libotel.a(reqwest-97d1376dfb77d784.reqwest.cb371ce8e1e3945e-cgu.04.rcgu.o): in function `core::ptr::drop_in_place<alloc::vec::Vec<reqwest::tls::Certificate>>': reqwest.cb371ce8e1e3945e-cgu.04:(.text._ZN4core3ptr69drop_in_place$LT$alloc..vec..Vec$LT$reqwest..tls..Certificate$GT$$GT$17h9b62679cc7161be5E+0x30): undefined reference to `X509_free' Fixes: 9d3dcb8 ("otel: add build tooling to include otel code") [ Tweaked subject prefix. s/NXT_OTEL_LIB_LOC/NXT_OTEL_LIB_STATIC/ - Andrew ] Signed-off-by: Andrew Clayton <[email protected]>

@thresheek

Fixes: 9d3dcb8 ("otel: add build tooling to include otel code") [ Commit subject, s/NXT_OTEL_LIB_LOC/NXT_OTEL_LIB_STATIC/ and placement of NXT_OTEL_LIB_STATIC tweaked as per @thresheek - Andrew ] Signed-off-by: Andrew Clayton <[email protected]>

Rust code relies on macOS-provided frameworks for TLS. Fixes: 9d3dcb8 ("otel: add build tooling to include otel code") [ Tweaked subject prefix. Some minor tweaks for current changes. - Andrew ] Signed-off-by: Andrew Clayton <[email protected]>

There were at least a couple of issues with building OTEL support. It only worked with GNU make due to the use of ifeq, even gmake had some issues. Debug builds were broken due to trying to pass --debug to cargo which is the default and isn't a valid option. This 'fixes' things by doing 'release' builds of OTEL by default. Passing D=1 to make will generate 'debug' builds but this as previously with D= etc, only works with GNU make. We make use of the '--emit link=' rustc option to place the libotel.a static library into build/lib This is good, it consolidates the static libraries into one place and it simplifies the build scripts. While we're at it pretty print the cargo command by default. Fixes: 9d3dcb8 ("otel: add build tooling to include otel code") Link: <nginx#1520 (comment)> Signed-off-by: Andrew Clayton <[email protected]>

ac000 · 2025-01-10T04:17:04Z

Rebased with master

$ git range-diff 857b85cc...d0dbba3b
-:  -------- > 1:  d699fb9d otel: fix segfaults when otel not configured
-:  -------- > 2:  753e2984 otel: remove deadcode
1:  f1afcf5b = 3:  1149d9ac auto/otel: Make use of nxt_feature_name
2:  3b5542ed = 4:  b9232c75 auto/make: s/NXT_OTEL_LIB_LOC/NXT_OTEL_LIB_STATIC/
3:  e2c2c901 = 5:  854a52ce auto/make, otel: fix linking on macOS and Ubuntu
4:  da917411 = 6:  bfb86ed0 auto/make: Add missing NXT_OTEL_LIB_STATIC to some C tests
5:  6c005a00 = 7:  1ac4e409 auto/otel: don't look for OpenSSL on Darwin
6:  857b85cc = 8:  d0dbba3b auto/make: Fix various issues with building OTEL

osokin · 2025-01-10T04:17:15Z

@osokin @remicollet

I've created a new issue to track the off-line builds issue and added your comments there...

Very good! Thank you so much!

ac000 · 2025-01-10T04:25:07Z

Failing checks are because we need to update for Ruby 3.4...

This was referenced Jan 7, 2025

add libotel for tests #1521

Closed

auto/otel: Fix breakage on non-Linux platforms #1520

Closed

ac000 marked this pull request as ready for review January 7, 2025 00:48

ac000 requested review from thresheek and avahahn January 7, 2025 00:48

avahahn approved these changes Jan 7, 2025

View reviewed changes

thresheek approved these changes Jan 7, 2025

View reviewed changes

avahahn requested changes Jan 7, 2025

View reviewed changes

ac000 force-pushed the otel-build-fixes branch from 0397301 to 70d0683 Compare January 7, 2025 04:18

ac000 requested a review from avahahn January 7, 2025 04:24

avahahn approved these changes Jan 7, 2025

View reviewed changes

ac000 force-pushed the otel-build-fixes branch 2 times, most recently from d3cb989 to 4fe4f78 Compare January 9, 2025 05:13

ac000 requested review from avahahn and thresheek January 9, 2025 05:37

ac000 force-pushed the otel-build-fixes branch from 4fe4f78 to 857b85c Compare January 9, 2025 05:40

thresheek approved these changes Jan 10, 2025

View reviewed changes

ac000 mentioned this pull request Jan 10, 2025

cargo/rust, off-line and reproduceable builds #1537

Open

ac000 and others added 3 commits January 10, 2025 04:16

auto/make: s/NXT_OTEL_LIB_LOC/NXT_OTEL_LIB_STATIC/

b9232c7

This better matches existing naming convention, e.g NXT_LIB_STATIC Fixes: 9d3dcb8 ("otel: add build tooling to include otel code") Signed-off-by: Andrew Clayton <[email protected]>

remicollet and others added 3 commits January 10, 2025 04:16

ac000 force-pushed the otel-build-fixes branch from 857b85c to d0dbba3 Compare January 10, 2025 04:16

ac000 merged commit d0dbba3 into nginx:master Jan 10, 2025
21 of 23 checks passed

ac000 deleted the otel-build-fixes branch January 10, 2025 04:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OTEL build fixes #1527

OTEL build fixes #1527

ac000 commented Jan 7, 2025

ac000 commented Jan 7, 2025

thresheek commented Jan 7, 2025

thresheek left a comment

avahahn left a comment

ac000 commented Jan 7, 2025

ac000 commented Jan 9, 2025

ac000 commented Jan 9, 2025

osokin commented Jan 9, 2025

remicollet commented Jan 9, 2025

osokin commented Jan 9, 2025

thresheek left a comment

ac000 commented Jan 10, 2025

ac000 commented Jan 10, 2025

osokin commented Jan 10, 2025

ac000 commented Jan 10, 2025

OTEL build fixes #1527

OTEL build fixes #1527

Conversation

ac000 commented Jan 7, 2025

ac000 commented Jan 7, 2025

thresheek commented Jan 7, 2025

thresheek left a comment

Choose a reason for hiding this comment

avahahn left a comment

Choose a reason for hiding this comment

ac000 commented Jan 7, 2025

ac000 commented Jan 9, 2025

ac000 commented Jan 9, 2025

osokin commented Jan 9, 2025

remicollet commented Jan 9, 2025

osokin commented Jan 9, 2025

thresheek left a comment

Choose a reason for hiding this comment

ac000 commented Jan 10, 2025

ac000 commented Jan 10, 2025

osokin commented Jan 10, 2025

ac000 commented Jan 10, 2025