Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add libotel for tests #1521

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

remicollet
Copy link
Contributor

When otel support is enable, libnxt.a needs libotel.a

Without this patch, when building tests

 LD     build/unit_websocket_chat
/usr/bin/ld: build/lib/libnxt.a(nxt_router.o): in function `nxt_router_conf_create':
/builddir/build/BUILD/unit-1.34.0-build/unit-27bde184dedcbf687db2f314c60c037623318a8d/src/nxt_router.c:2217:(.text+0x73c8): undefined reference to `nxt_otel_rs_init'
/usr/bin/ld: /builddir/build/BUILD/unit-1.34.0-build/unit-27bde184dedcbf687db2f314c60c037623318a8d/src/nxt_router.c:2221:(.text+0x755f): undefined reference to `nxt_otel_rs_uninit'
/usr/bin/ld: build/lib/libnxt.a(nxt_http_request.o): in function `nxt_http_request_create':
/builddir/build/BUILD/unit-1.34.0-build/unit-27bde184dedcbf687db2f314c60c037623318a8d/src/nxt_http_request.c:289:(.text+0x9dc): undefined reference to `nxt_otel_rs_is_init'
/usr/bin/ld: build/lib/libnxt.a(nxt_otel.o): in function `nxt_otel_propagate_header':
/builddir/build/BUILD/unit-1.34.0-build/unit-27bde184dedcbf687db2f314c60c037623318a8d/src/nxt_otel.c:72:(.text+0xe8): undefined reference to `nxt_otel_rs_copy_traceparent'
/usr/bin/ld: /builddir/build/BUILD/unit-1.34.0-build/unit-27bde184dedcbf687db2f314c60c037623318a8d/src/nxt_otel.c:93:(.text+0x14d): undefined reference to `nxt_otel_rs_add_event_to_trace'
/usr/bin/ld: build/lib/libnxt.a(nxt_otel.o): in function `nxt_otel_trace_and_span_init':
/builddir/build/BUILD/unit-1.34.0-build/unit-27bde184dedcbf687db2f314c60c037623318a8d/src/nxt_otel.c:270:(.text+0x280): undefined reference to `nxt_otel_rs_get_or_create_trace'
/usr/bin/ld: build/lib/libnxt.a(nxt_otel.o): in function `nxt_otel_span_add_headers':
/builddir/build/BUILD/unit-1.34.0-build/unit-27bde184dedcbf687db2f314c60c037623318a8d/src/nxt_otel.c:146:(.text+0x340): undefined reference to `nxt_otel_rs_add_event_to_trace'
/usr/bin/ld: /builddir/build/BUILD/unit-1.34.0-build/unit-27bde184dedcbf687db2f314c60c037623318a8d/src/nxt_otel.c:150:(.text+0x382): undefined reference to `nxt_otel_rs_add_event_to_trace'
/usr/bin/ld: /builddir/build/BUILD/unit-1.34.0-build/unit-27bde184dedcbf687db2f314c60c037623318a8d/src/nxt_otel.c:152:(.text+0x3af): undefined reference to `nxt_otel_rs_add_event_to_trace'
/usr/bin/ld: build/lib/libnxt.a(nxt_otel.o): in function `nxt_otel_span_add_body':
/builddir/build/BUILD/unit-1.34.0-build/unit-27bde184dedcbf687db2f314c60c037623318a8d/src/nxt_otel.c:204:(.text+0x48f): undefined reference to `nxt_otel_rs_add_event_to_trace'
/usr/bin/ld: build/lib/libnxt.a(nxt_otel.o): in function `nxt_otel_span_collect':
/builddir/build/BUILD/unit-1.34.0-build/unit-27bde184dedcbf687db2f314c60c037623318a8d/src/nxt_otel.c:245:(.text+0x4dc): undefined reference to `nxt_otel_rs_send_trace'
/usr/bin/ld: build/lib/libnxt.a(nxt_otel.o): in function `nxt_otel_span_add_status':
/builddir/build/BUILD/unit-1.34.0-build/unit-27bde184dedcbf687db2f314c60c037623318a8d/src/nxt_otel.c:230:(.text+0x553): undefined reference to `nxt_otel_rs_add_event_to_trace'

@remicollet
Copy link
Contributor Author

Notice: this is related to RPM build where -Wl,-z,now is used, so requiring all symbols to be resolved at link time

@ac000
Copy link
Member

ac000 commented Dec 20, 2024

Yeah, we noticed this...

There are a number of issues with --otel

@remicollet
Copy link
Contributor Author

remicollet commented Dec 20, 2024

There are a number of issues with --otel

Do you count as issue

  • size of sources x 33 (with vendored rust libraries to allow offline build)
  • size of binary x 8 (1.3M to 11 M for unitd)
  • time of build x 7 (without download)

;)

@ac000
Copy link
Member

ac000 commented Dec 20, 2024

It is certainly less than ideal.

Unfortunately there is no C API for OTEL, there is a C++ API, but that is supposedly horrendous...

In my mind, it would have been possible to create a thin C wrapper (exposing just the bits we need) around the C++ API...

@thresheek
Copy link
Member

And for C++ libraries, it's also less than ideal due to the amount of dependencies they have. It's at least:

  • abseil-cpp
  • protobuf
  • grpc

It's not that different size-wise in a resulting binary from the Rust version, and build times are even worse. We have the exact same problem in NGINX otel implementation.

@ac000
Copy link
Member

ac000 commented Dec 20, 2024

But at least there are distro packages for those things already so no need to build any of that, it would just be building the C wrapper...

@thresheek
Copy link
Member

It really, really depends on a distro and its version. It's going to get better though, that's for sure.

@thresheek
Copy link
Member

I think this patch is good on it's own, although I'd move $(NXT_OTEL_LIB_LOC) to the position right after $NXT_BUILD_DIR/lib/$NXT_LIB_STATIC (probably next line), since it's out responsibility to provide proper dependency resolution order here.

@ac000
Copy link
Member

ac000 commented Dec 20, 2024

Commit subject and message would also need tweaking to better describe the actual issue...

Actually this should probably be combined with thresheek@18381b1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants