otel: Add OpenTelemetry functionality to NGINX Unit #1463

javorszky · 2024-10-18T12:43:04Z

Adds OpenTelemetry implementation via a Rust crate compiled to a static C library and linked into the existing Unit codebase with the necessary build and configuration steps.

Each commit in order builds, runs, and works as intended, each export traces to grafana from 290fffd (configuration items and their validation) onwards
The --otel build argument works from fb15df7 (add build tooling to include otel code) onwards.
Adding settings.telemetry to Unit's config.json works from 290fffd (configuration items and their validation) onwards
Each commit has the Signed-off-by trailers, and all of them are co-authored between @avahahn and myself.

Closes #1283

avahahn · 2024-10-21T15:17:36Z

Opentelemetry performance test results!

The tests use wrk, were ran on a 12c/24t system with 64gb ram. wrk invocation as follows:
./wrk -t22 -c880 -d30s http://127.0.0.1:80/
Here, Unit is serving the welcome page.

With otel change, tracing enabled (sample rate 1.0, batch size 20, HTTP Transport)

Latency:

Avg: 1.55ms
Stdev: 2.67ms, 95.46%

Requests per second:

Avg: 31.70k
Stdev: 5.87k, 79.66%

With otel change, tracing NOT enabled:

Latency:

Avg: 1.00ms
Stdev: 777.66us 75.06%

Requests per second:

Avg: 41.76k
Stdev: 11.33k 55.51%

Built without OpenTelemetry support:

Latency:

Avg: 0.97ms
Stdev: 673.74us, 77.44%

Requests per second:

Avg: 42.25k
Stdev: 4.88k, 76.60%

I conclude that the enclosed change does not measurably impact Unit's performance when not configured. The differences in numbers between the 2nd and 3rd dataset is far smaller than either of their deviance values, and I find the data is always similar across test runs.

I also conclude that in a very high throughput deployment opentelemetry tracing will have a small impact. This test represents a high throughput deployment with the sample ratio cranked up unreasonably high. Best practices in any kind of situation in which unit will be receiving this kind of traffic are to reduce the sampling ratio to minimize the load of opentelemetry span processing and generation. Even with the sampling ratio cranked up to maximum (100%) opentelemetry processing for every single request (over 30k requests per second) merely adds half a millisecond on average.

For more information you can reproduce my tests with these scripts:
https://github.com/avahahn/unit-perf-test

avahahn · 2024-10-21T19:09:46Z

Just wanted to offer a note for configuration. The settings object has been extended with a new telemetry field. This field contains currently 4 items:

endpoint: The endpoint for the OpenTelemetry Collector
This is a required field.
It takes either a URL to either a gRPC or HTTP API.
Example: http://lgtm:4318/v1/traces
protocol: Determines the protocol used to communicate with the endpoint
This is a required field.
Can either be specified in all caps or all lowercase.
One of either "HTTP" or "GRPC"
batch_size: Number of spans to cache before triggering a transaction with configured endpoint
This allows the user to cache up to N spans before the opentelemetry background thread sends spans over network to the collector. It is expected that this scales up with the throughput of the user's environment as well as given any bandwidth constraints the user might have for their network.
Must be a positive integer.
sampling_ratio: percent of requests to trace
This allows the user to only trace anywhere from 0% to 100% of requests that hit Unit. In high throughput environments this percentage will be lower. This allows the user to save space in storing span data, and to collect request metrics like time to decode headers and whatnot without storing massive amounts of duplicate superfluous data.
Must be a positive floating point number.

Example configuration:

curl -X PUT 127.0.0.1:8080/config -d '{
    "settings": {
        "telemetry": {
            "batch_size": 20,
            "endpoint": "http://lgtm:4318/v1/traces",
            "protocol": "http",
            "sampling_ratio": 1
        }
    },
    "listeners": {
        "*:80": {
            "pass": "routes"
        }
    },
    "routes": [
        {
            "match": {
                "headers": {
                    "accept": "*text/html*"
                }
            },
            "action": {
                "share": "/usr/share/unit/welcome/welcome.html"
            }
        },
        {
            "action": {
                "share": "/usr/share/unit/welcome/welcome.md"
            }
        }
    ]
}'

ac000

For the commits with Ava's Co-authored-by, the commit tags should look like

Signed-off-by: Gabor Javorszky <[email protected]>
Co-developed-by: Ava Hahn <[email protected]>
Signed-off-by: Ava Hahn <[email protected]>

I.e. A c-[ad]-b tag should be immediately followed by a s-o-b from the same person.

Note: that the person that last handled the patch should be the last s-o-b, so there's a logical progression of people who where involved on its way into the repo, hence the slight re-ordering.

Also note we use Co-developed-by.

For .editorconfig: fix bracket balance of editorconfig file and docs/openapi: update OpenAPI references the s-o-b's should be swapped around.

ac000

In commit

    otel: add build tooling to include otel code

I think the following would be better in ./configure

diff --git ./auto/make ./auto/make
index f21a2dfc..e7d29ba3 100644
--- ./auto/make
+++ ./auto/make
@@ -7,6 +7,11 @@
 
 $echo "creating $NXT_MAKEFILE"
 
+if [ $NXT_OTEL = "NO" ];  then
+    NXT_OTEL_LIB_LOC=
+    NXT_OTEL_BUILD_FLAG=
+    NXT_OTEL_LIB_DIR=
+fi
 
 cat << END > $NXT_MAKEFILE
 
@@ -138,14 +143,14 @@ cat << END >> $NXT_MAKEFILE
 
 libnxt:        $NXT_BUILD_DIR/lib/$NXT_LIB_SHARED $NXT_BUILD_DIR/lib/$NXT_LIB_STATIC
 
-$NXT_BUILD_DIR/lib/$NXT_LIB_SHARED: \$(NXT_LIB_OBJS)
+$NXT_BUILD_DIR/lib/$NXT_LIB_SHARED: \$(NXT_LIB_OBJS) $NXT_OTEL_LIB_LOC
        \$(PP_LD) \$@
        \$(v)\$(NXT_SHARED_LOCAL_LINK) -o \$@ \$(NXT_LIB_OBJS) \\
-               $NXT_LIBM $NXT_LIBS $NXT_LIB_AUX_LIBS
+               $NXT_LIBM $NXT_LIBS $NXT_LIB_AUX_LIBS $NXT_OTEL_LIB_LOC
 
-$NXT_BUILD_DIR/lib/$NXT_LIB_STATIC: \$(NXT_LIB_OBJS)
+$NXT_BUILD_DIR/lib/$NXT_LIB_STATIC: \$(NXT_LIB_OBJS) $NXT_OTEL_LIB_LOC
        \$(PP_AR) \$@
-       \$(v)$NXT_STATIC_LINK \$@ \$(NXT_LIB_OBJS)
+       \$(v)$NXT_STATIC_LINK \$@ \$(NXT_LIB_OBJS) $NXT_OTEL_LIB_LOC
 
 $NXT_BUILD_DIR/lib/$NXT_LIB_UNIT_STATIC: \$(NXT_LIB_UNIT_OBJS) \\
                $NXT_BUILD_DIR/share/pkgconfig/unit.pc \\
@@ -359,11 +364,11 @@ $echo >> $NXT_MAKEFILE
 cat << END >> $NXT_MAKEFILE
 
 $NXT_BUILD_DIR/sbin/$NXT_DAEMON:       $NXT_BUILD_DIR/lib/$NXT_LIB_STATIC \\
-                               \$(NXT_OBJS)
+                               \$(NXT_OBJS) $NXT_OTEL_LIB_LOC
        \$(PP_LD) \$@
        \$(v)\$(NXT_EXEC_LINK) -o \$@ \$(CFLAGS) \\
                \$(NXT_OBJS) $NXT_BUILD_DIR/lib/$NXT_LIB_STATIC \\
-               $NXT_LIBM $NXT_LIBS $NXT_LIB_AUX_LIBS
+               $NXT_LIBM $NXT_LIBS $NXT_LIB_AUX_LIBS $NXT_OTEL_LIB_LOC
 
 END

Where you only need to adjust the NXT_LIB_AUX_CFLAGS and NXT_LIB_AUX_LIBS variables.

E.g. in my http compression patches I do

diff --git ./configure ./configure
index 6929d41d..f33134b7 100755
--- ./configure
+++ ./configure
@@ -127,6 +127,7 @@ NXT_LIBRT=
 . auto/unix
 . auto/os/conf
 . auto/ssltls
+. auto/compression
 
 if [ $NXT_REGEX = YES ]; then
     . auto/pcre
@@ -169,11 +170,13 @@ END
 
 NXT_LIB_AUX_CFLAGS="$NXT_OPENSSL_CFLAGS $NXT_GNUTLS_CFLAGS \\
                     $NXT_CYASSL_CFLAGS $NXT_POLARSSL_CFLAGS \\
-                    $NXT_PCRE_CFLAGS"
+                    $NXT_PCRE_CFLAGS $NXT_ZLIB_CFLAGS $NXT_ZSTD_CFLAGS \\
+                    $NXT_BROTLI_CFLAGS"
 
 NXT_LIB_AUX_LIBS="$NXT_OPENSSL_LIBS $NXT_GNUTLS_LIBS \\
                     $NXT_CYASSL_LIBS $NXT_POLARSSL_LIBS \\
-                    $NXT_PCRE_LIB"
+                    $NXT_PCRE_LIB $NXT_ZLIB_LIBS $NXT_ZSTD_LIBS \\
+                    $NXT_BROTLI_LIBS"
 
 if [ $NXT_NJS != NO ]; then
     . auto/njs

auto/make

auto/otel

src/nxt_otel.c

auto/otel

src/nxt_otel.c

ac000 · 2024-10-22T16:04:46Z

For

tools/openapi: update OpenAPI references

I would make the prefix docs/openapi as docs/unit-openapi.yaml is the source of truth.

ac000

otel: configuration items and their validation

Adds code responsible for users to apply the `telemetry` configuration
options.

I would like to see example Unit configuration here...

src/nxt_conf_validation.c

src/nxt_router.c

src/nxt_otel.c

src/nxt_otel.h

src/otel/src/lib.rs

src/nxt_otel.c

src/otel/src/lib.rs

docs/unit-openapi.yaml

src/nxt_otel.c

ac000

{
    "settings": {
        "telemetry": {
            "batch_size": 20,
            "endpoint": "http://lgtm:4318/v1/traces",

At some point this will require a DNS lookup.

I assume this happens in the OTEL library in its own thread away from the actual router threads?

Would this lookup happen once at startup for for every datum or batch of data sent?

On a related note is there a pool of connections to the endpoint created or is a new connection established for each bit of data sent?

ac000 · 2024-11-08T06:03:43Z

@ac000 A change of note: I identified a bug caused by using nxt_log in the collector background thread:

I did wonder what might happen here...

(gdb) p *thr
$2 = {log = 0x0, main_log = {level = 0, ident = 0, handler = 0x0, ctx_handler = 0x0, ctx = 0x0}, task = 0x0, tid = 0, 
  handle = 0, link = 0x0, thread_pool = 0x0, time = {now = {realtime = {sec = 0, nsec = 0}, monotonic = 0, update = 0}, 
    last_gmtime = 0, last_localtime = 0, gmtime = {tm_sec = 0, tm_min = 0, tm_hour = 0, tm_mday = 0, tm_mon = 0, 
      tm_year = 0, tm_wday = 0, tm_yday = 0, tm_isdst = 0, tm_gmtoff = 0, tm_zone = 0x0}, localtime = {tm_sec = 0, 
      tm_min = 0, tm_hour = 0, tm_mday = 0, tm_mon = 0, tm_year = 0, tm_wday = 0, tm_yday = 0, tm_isdst = 0, 
      tm_gmtoff = 0, tm_zone = 0x0}, no_cache = 0, signal = 0, nstrings = 0, strings = 0x0}, runtime = 0x0, engine = 0x0, 
  data = 0x0, random = {i = 0 '\000', j = 0 '\000', s = '\000' <repeats 255 times>, count = 0}}
(gdb) p thr->task
$3 = (nxt_task_t *) 0x0

Yes, so the thread-local storage variable has been initialised by the compiler, but we've never called nxt_thread_init() or set thr->task = task in nxt_event_engine_start().

This is understandable because it is a new separate thread that is being managed by Rust/Tokio. So I have removed the nxt_log calls from the code that is running in a background thread, and replaced the one error log with eprintln!()

Hmm, not ideal... but stderr does get directed to the unit log...

The fact eprintln!() panics if it can't write to stderr, is slightly concerning...

But I don't see a good alternative... if we are going to be puttting more rust code in Unit, then we'll probably need to come up with a rust version of nxt_log() at some point.

javorszky · 2024-11-08T11:46:51Z

Since there is an issue of showing credit for the work involved, I insist that we keep Co-authored-by here.

I wanted to double check this, so opened this PR to my own branch with bogus changes for the sole purpose of figuring out whether GitHub also parses the Co-developed-by trailer in the UI or not. It does not: javorszky#1

The important part is this:

And in the commit details:

Whereas the commit with only the Co-developed-by has this:

We could use both tags, but not sure what we'd gain with it. There's a way to customize what trailers we can use and how, but we don't have control over how GitHub specifically works.

ac000 · 2024-11-08T15:01:49Z

Are you guys not seeing the rust warnings?

This patch is needed to get rid of these warnings

warning: type `nxt_uint_t` should have an upper camel case name
  --> src/lib.rs:35:10
   |
35 | pub type nxt_uint_t = ::std::os::raw::c_uint;
   |          ^^^^^^^^^^ help: convert the identifier to upper camel case: `NxtUintT`
   |
   = note: `#[warn(non_camel_case_types)]` on by default
...
warning: constant `NXT_LOG_INFO` is never used
  --> src/lib.rs:26:7
   |
26 | const NXT_LOG_INFO:   u32 = 4;
   |       ^^^^^^^^^^^^
   |
   = note: `#[warn(dead_code)]` on by default

diff --git ./src/otel/src/lib.rs ./src/otel/src/lib.rs
index 7dbed005..dbf8e6f4 100644
--- ./src/otel/src/lib.rs
+++ ./src/otel/src/lib.rs
@@ -1,3 +1,7 @@
+#![allow(
+    non_camel_case_types,
+)]
+
 use opentelemetry::global::BoxedSpan;
 use opentelemetry::trace::{
     Span, SpanBuilder, SpanKind, TraceId, Tracer, TracerProvider,
@@ -15,15 +19,10 @@ use std::sync::{Arc, OnceLock};
 use tokio::sync::mpsc;
 use tokio::sync::mpsc::{Receiver, Sender};
 
-#[allow(
-    non_camel_case_types,
-)]
-
 const TRACEPARENT_HEADER_LEN: u8 = 55;
 const TIMEOUT: time::Duration = std::time::Duration::from_secs(10);
 
-const NXT_LOG_ERR:    u32 = 1;
-const NXT_LOG_INFO:   u32 = 4;
+const NXT_LOG_ERR: u32 = 1;
 
 #[repr(C)]
 pub struct nxt_str_t {

That still leaves

warning: unused variable: `log_callback`
   --> src/lib.rs:144:5
    |
144 |     log_callback: unsafe extern "C" fn(log_level: nxt_uint_t, msg: *con...
    |     ^^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_log_callback`
    |
    = note: `#[warn(unused_variables)]` on by default

Which can be dealt with by (don't see any point leaving it in with an "_")

diff --git ./src/otel/src/lib.rs ./src/otel/src/lib.rs
index 7dbed005..68e5d2b0 100644
--- ./src/otel/src/lib.rs
+++ ./src/otel/src/lib.rs
@@ -118,7 +117,6 @@ unsafe fn nxt_otel_rs_init(
          */
         Ok(_) => {
             std::thread::spawn(move || nxt_otel_rs_runtime(
-                log_callback,
                 ep,
                 proto,
                 batch_size,
@@ -141,7 +139,6 @@ unsafe fn nxt_otel_rs_init(
  */
 #[tokio::main]
 async unsafe fn nxt_otel_rs_runtime(
-    log_callback: unsafe extern "C" fn(log_level: nxt_uint_t, msg: *const c_char),
     endpoint: String,
     proto: Protocol,
     batch_size: f64,

Which needless to say should be applied to commit 39284a2 ("otel: add opentelemetry rust crate code")

javorszky · 2024-11-08T17:24:36Z

This patch is needed to get rid of these warnings

Thanks for catching these. I found a few more things.

I applied the patches with some additions:

#![allow(non_camel_case_types)] goes on the top of the file otherwise rust complains that the inner attribute can't be on line 18. Plus the ! is needed to make it an inner attribute (apply to anything in the same scope), otherwise it would only apply to the thing immediately following it. The patch did exactly this, no need to modify.
fixed a typo: uszie -> usize
also removed the unused NXT_LOG_INFO
changed type of NXT_LOG_ERR from u32 -> usize, rust was complaining the signature of log_callback was different to what we were passing it

ac000 · 2024-11-08T18:45:13Z

This patch is needed to get rid of these warnings

Thanks for catching these. I found a few more things.

I applied the patches with some modifications:
* `#![allow(non_camel_case_types)]` goes on the top of the file otherwise rust complains that the inner attribute can't be on line 18. Plus the `!` is needed to make it an inner attribute (apply to anything in the same scope), otherwise it would only apply to the thing immediately following it.

But that's exactly what the patch does!

* fixed a typo: `uszie` -> `usize`

Oh, right, good catch!

* also removed the unused `NXT_LOG_INFO`

That's also what the patch does...

* changed type of `NXT_LOG_ERR` from `u32` -> `usize`, rust was complaining the signature of `log_callback` was different to what we were passing it

Hmm.. usize will be wrong for x86-64...

What platform are you building on? I guess it isn't x86-64...

NXT_LOG_ERR should actually be using our new nxt_uint_t type, which will be sized according to platform...

javorszky · 2024-11-11T08:52:37Z

But that's exactly what the patch does!

Apologies, I didn't want to take credit for something you did. The reason I reworked is because I assume when you refer to the patch, it's the changeset you added to the comment, ie this bit:

Which looking at it does exactly what you said it does. I'm... unsure why I thought it did something different, there was a version where the same attribute was just moved a couple of lines further up without adding the ! to it. Let me reword / add that clarification to my previous comments.

javorszky · 2024-11-11T08:55:21Z

Hmm.. usize will be wrong for x86-64...

What platform are you building on? I guess it isn't x86-64...

I'm using a dockerfile that begins with FROM debian:stable-slim on a mac M1 (arm64) computer.

The built image has a uname -a of

Linux 58f1643967d4 6.10.11-linuxkit #1 SMP Thu Oct  3 10:17:28 UTC 2024 aarch64 GNU/Linux

My host system has a uname -a of

Darwin G9PQ7J244F 23.6.0 Darwin Kernel Version 23.6.0: Thu Sep 12 23:35:29 PDT 2024; root:xnu-10063.141.1.701.1~1/RELEASE_ARM64_T6000 arm64

Looking at the code, on x86_64 usize shouldn't be used:

#[cfg(target_arch = "x86_64")]
pub type nxt_uint_t = ::std::os::raw::c_uint;

#[cfg(not(target_arch = "x86_64"))]
pub type nxt_uint_t = usize;

// ...

    log_callback: unsafe extern "C" fn(log_level: nxt_uint_t, msg: *const c_char),

This should work on both x86_64 and not x86_64 🤔

I see what you're saying. So basically this patch should also be applied:

- const NXT_LOG_ERR:    usize = 1;
+ const NXT_LOG_ERR:    nxt_uint_t = 1;

javorszky · 2024-11-11T15:29:35Z

NXT_LOG_ERR should actually be using our new nxt_uint_t type, which will be sized according to platform...

Hah, I totally missed this (apologies, I'm having a weird day today.

I applied that patch, nxt log err is now the nxt_uint_t type.

ac000 · 2024-11-12T01:01:58Z

I see what you're saying. So basically this patch should also be applied:
- const NXT_LOG_ERR:    usize = 1;
+ const NXT_LOG_ERR:    nxt_uint_t = 1;

Exactly! That'll be right regardless of architecture. The reason for doing this is to make it match the C side of things...

ac000 · 2024-11-12T01:37:24Z

OK, taking one last look through the code, just a couple of minor things

diff --git ./src/nxt_conf_validation.c ./src/nxt_conf_validation.c
index b0f5c530..0dde39ff 100644
--- ./src/nxt_conf_validation.c
+++ ./src/nxt_conf_validation.c
@@ -1522,7 +1522,7 @@ nxt_otel_validate_batch_size(nxt_conf_validation_t *vldt,
 
     batch_size = nxt_conf_get_number(value);
     if (batch_size <= 0) {
-      return NXT_ERROR;
+        return NXT_ERROR;
     }
 
     return NXT_OK;

Should be applied to e0b640b ("otel: configuration items and their validation")

diff --git ./src/nxt_http.h ./src/nxt_http.h
index 456a96ce..19bbdda3 100644
--- ./src/nxt_http.h
+++ ./src/nxt_http.h
@@ -10,6 +10,7 @@
 #include <nxt_regex.h>
 #include <nxt_otel.h>
 
+
 typedef enum {
     NXT_HTTP_UNSET = -1,
     NXT_HTTP_INVALID = 0,

Should be applied to 209c084 ("otel: add build tooling to include otel code")

javorszky · 2024-11-12T08:47:26Z

just a couple of minor things

These have been applied to their respective commits.

Thank you 🙂

ac000 · 2024-11-12T16:47:09Z

Almost there, lets just get the commit tags right..

For a572a81 ("otel: add opentelemetry rust crate code")

If you're adding my c-a-b, I need to sign-off on it (which I don't mind doing) as a c-a-b should always be immediately followed by a s-o-b.

Seeing as Ava is the author, she doesn't need a c-a-b.

Lastly, seeing as Gabor is the committer his should be the last s-o-b as he was the last one to handle the patch..., so that would make the commit tags look like

    Co-authored-by: Andrew Clayton <[email protected]>
    Signed-off-by: Andrew Clayton <[email protected]>
    Signed-off-by: Ava Hahn <[email protected]>
    Signed-off-by: Gabor Javorszky <[email protected]>

For b1a1009 ("otel: add build tooling to include otel code"), following the above rules

    Signed-off-by: Ava Hahn <[email protected]>
    Co-authored-by: Gabor Javorszky <[email protected]>
    Signed-off-by: Gabor Javorszky <[email protected]>

804fe14 ("otel: add header parsing and test call state")

    Signed-off-by: Ava Hahn <[email protected]>
    Signed-off-by: Gabor Javorszky <[email protected]>

3d2beb4 ("otel: configuration items and their validation")

    Signed-off-by: Ava Hahn <[email protected]>
    Signed-off-by: Gabor Javorszky <[email protected]>

eb9b0ff (".editorconfig: fix bracket balance of editorconfig file")

    Signed-off-by: Ava Hahn <[email protected]>
    Signed-off-by: Gabor Javorszky <[email protected]>

c3f14ca ("docs/openapi: update OpenAPI references")

    Signed-off-by: Ava Hahn <[email protected]>
    Signed-off-by: Gabor Javorszky <[email protected]>

ac000 · 2024-11-12T16:54:12Z

For the first commit I'm equally happy if you just remove my c-a-b, I'm not sure it really warrants it...

This is purely the source code of the rust end of opentelemetry. It does not have build tooling wired up yet, nor is this used from the C code. Signed-off-by: Ava Hahn <[email protected]> Signed-off-by: Gabor Javorszky <[email protected]>

Adds the --otel flag to the configure command and the various build time variables and checks that are needed in this flow. It also includes the nxt_otel.c and nxt_otel.h files that are needed for the rest of Unit to talk to the compiled static library that's generated from the rust crate. Signed-off-by: Ava Hahn <[email protected]> Co-authored-by: Gabor Javorszky <[email protected]> Signed-off-by: Gabor Javorszky <[email protected]>

Enables Unit to parse the tracestate and traceparent headers and add it to the list, as well as calls to nxt_otel_test_and_call_state. Signed-off-by: Ava Hahn <[email protected]> Signed-off-by: Gabor Javorszky <[email protected]>

Adds code responsible for users to apply the `telemetry` configuration options. configuration snippet as follows: { "settings": { "telemetry": { "batch_size": 20, "endpoint": "http://lgtm:4318/v1/traces", "protocol": "http", "sampling_ratio": 1 } }, "listeners": { "*:80": { "pass": "routes" } }, "routes": [ { "match": { "headers": { "accept": "*text/html*" } }, "action": { "share": "/usr/share/unit/welcome/welcome.html" } }, { "action": { "share": "/usr/share/unit/welcome/welcome.md" } } ] } Signed-off-by: Ava Hahn <[email protected]> Signed-off-by: Gabor Javorszky <[email protected]>

Tiny bracket balance fix. Signed-off-by: Ava Hahn <[email protected]> Signed-off-by: Gabor Javorszky <[email protected]>

These changes are generated by the openapi generator through a make command. Signed-off-by: Ava Hahn <[email protected]> Signed-off-by: Gabor Javorszky <[email protected]>

javorszky · 2024-11-12T17:03:01Z

For the first commit I'm equally happy if you just remove my c-a-b, I'm not sure it really warrants it...

I shuffled the trailers around per your comments.

ac000

OK, I think we can call this done and release it into the wild!

I know this took some time but thanks for sticking with it and hopefully you'll agree... it's better for it...

ac000 · 2024-11-12T20:18:42Z

Oh well, all the s-o-b's are now the wrong way round, never mind, maybe next time just have one person doing the committing...

javorszky requested review from avahahn and ac000 October 18, 2024 12:43

javorszky force-pushed the gabor/otel-review-2-check branch from bb5da68 to dac737c Compare October 21, 2024 12:12

ac000 requested changes Oct 21, 2024

View reviewed changes

ac000 requested changes Oct 22, 2024

View reviewed changes

ac000 reviewed Oct 22, 2024

View reviewed changes

src/nxt_otel.c Outdated Show resolved Hide resolved

avahahn force-pushed the gabor/otel-review-2-check branch 2 times, most recently from dac737c to 6ebfe70 Compare October 22, 2024 22:35

ac000 requested changes Oct 22, 2024

View reviewed changes

src/nxt_otel.c Outdated Show resolved Hide resolved

src/nxt_otel.c Outdated Show resolved Hide resolved

ac000 reviewed Oct 23, 2024

View reviewed changes

src/nxt_otel.h Outdated Show resolved Hide resolved

ac000 mentioned this pull request Oct 23, 2024

Use nxt_nitems() instead of sizeof() for strings (arrays) #1467

Merged

avahahn force-pushed the gabor/otel-review-2-check branch from 6ebfe70 to c2d6ce7 Compare October 23, 2024 21:19

This comment was marked as duplicate.

Sign in to view

avahahn force-pushed the gabor/otel-review-2-check branch from c2d6ce7 to ee3b440 Compare October 23, 2024 23:33

ac000 reviewed Oct 25, 2024

View reviewed changes

src/otel/src/lib.rs Outdated Show resolved Hide resolved

ac000 reviewed Oct 25, 2024

View reviewed changes

src/nxt_otel.c Outdated Show resolved Hide resolved

ac000 reviewed Oct 26, 2024

View reviewed changes

src/otel/src/lib.rs Outdated Show resolved Hide resolved

callahad reviewed Oct 28, 2024

View reviewed changes

docs/unit-openapi.yaml Show resolved Hide resolved

avahahn force-pushed the gabor/otel-review-2-check branch 2 times, most recently from 21fd6be to 02f7630 Compare October 28, 2024 23:55

ac000 reviewed Oct 29, 2024

View reviewed changes

src/nxt_otel.c Outdated Show resolved Hide resolved

avahahn force-pushed the gabor/otel-review-2-check branch from 02f7630 to 9a497ec Compare October 30, 2024 23:42

ac000 reviewed Oct 31, 2024

View reviewed changes

avahahn force-pushed the gabor/otel-review-2-check branch 2 times, most recently from 59ad421 to ddea9a8 Compare November 8, 2024 02:07

avahahn requested a review from ac000 November 8, 2024 07:44

javorszky force-pushed the gabor/otel-review-2-check branch from ddea9a8 to 2ba5a0a Compare November 8, 2024 17:21

javorszky force-pushed the gabor/otel-review-2-check branch from 2ba5a0a to 90f1972 Compare November 11, 2024 15:28

javorszky force-pushed the gabor/otel-review-2-check branch from 90f1972 to c3f14ca Compare November 12, 2024 08:47

avahahn and others added 6 commits November 12, 2024 16:54

otel: add opentelemetry rust crate code

1e3f04c

This is purely the source code of the rust end of opentelemetry. It does not have build tooling wired up yet, nor is this used from the C code. Signed-off-by: Ava Hahn <[email protected]> Signed-off-by: Gabor Javorszky <[email protected]>

otel: add header parsing and test call state

6d9eb24

Enables Unit to parse the tracestate and traceparent headers and add it to the list, as well as calls to nxt_otel_test_and_call_state. Signed-off-by: Ava Hahn <[email protected]> Signed-off-by: Gabor Javorszky <[email protected]>

.editorconfig: fix bracket balance of editorconfig file

88c138d

Tiny bracket balance fix. Signed-off-by: Ava Hahn <[email protected]> Signed-off-by: Gabor Javorszky <[email protected]>

docs/openapi: update OpenAPI references

25625b9

These changes are generated by the openapi generator through a make command. Signed-off-by: Ava Hahn <[email protected]> Signed-off-by: Gabor Javorszky <[email protected]>

javorszky force-pushed the gabor/otel-review-2-check branch from c3f14ca to 25625b9 Compare November 12, 2024 16:59

ac000 approved these changes Nov 12, 2024

View reviewed changes

avahahn merged commit e1fd14f into nginx:master Nov 12, 2024
32 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

otel: Add OpenTelemetry functionality to NGINX Unit #1463

otel: Add OpenTelemetry functionality to NGINX Unit #1463

javorszky commented Oct 18, 2024 •

edited

Loading

avahahn commented Oct 21, 2024

avahahn commented Oct 21, 2024

ac000 left a comment •

edited

Loading

ac000 left a comment

ac000 commented Oct 22, 2024

ac000 left a comment

This comment was marked as duplicate.

ac000 left a comment

ac000 commented Nov 8, 2024

javorszky commented Nov 8, 2024

ac000 commented Nov 8, 2024

javorszky commented Nov 8, 2024 •

edited

Loading

ac000 commented Nov 8, 2024

javorszky commented Nov 11, 2024 •

edited

Loading

javorszky commented Nov 11, 2024 •

edited

Loading

javorszky commented Nov 11, 2024

ac000 commented Nov 12, 2024

ac000 commented Nov 12, 2024

javorszky commented Nov 12, 2024

ac000 commented Nov 12, 2024

ac000 commented Nov 12, 2024

javorszky commented Nov 12, 2024

ac000 left a comment

ac000 commented Nov 12, 2024

otel: Add OpenTelemetry functionality to NGINX Unit #1463

otel: Add OpenTelemetry functionality to NGINX Unit #1463

Conversation

javorszky commented Oct 18, 2024 • edited Loading

avahahn commented Oct 21, 2024

Opentelemetry performance test results!

With otel change, tracing enabled (sample rate 1.0, batch size 20, HTTP Transport)

With otel change, tracing NOT enabled:

Built without OpenTelemetry support:

avahahn commented Oct 21, 2024

ac000 left a comment • edited Loading

Choose a reason for hiding this comment

ac000 left a comment

Choose a reason for hiding this comment

ac000 commented Oct 22, 2024

ac000 left a comment

Choose a reason for hiding this comment

This comment was marked as duplicate.

ac000 left a comment

Choose a reason for hiding this comment

ac000 commented Nov 8, 2024

javorszky commented Nov 8, 2024

ac000 commented Nov 8, 2024

javorszky commented Nov 8, 2024 • edited Loading

ac000 commented Nov 8, 2024

javorszky commented Nov 11, 2024 • edited Loading

javorszky commented Nov 11, 2024 • edited Loading

javorszky commented Nov 11, 2024

ac000 commented Nov 12, 2024

ac000 commented Nov 12, 2024

javorszky commented Nov 12, 2024

ac000 commented Nov 12, 2024

ac000 commented Nov 12, 2024

javorszky commented Nov 12, 2024

ac000 left a comment

Choose a reason for hiding this comment

ac000 commented Nov 12, 2024

javorszky commented Oct 18, 2024 •

edited

Loading

ac000 left a comment •

edited

Loading

javorszky commented Nov 8, 2024 •

edited

Loading

javorszky commented Nov 11, 2024 •

edited

Loading

javorszky commented Nov 11, 2024 •

edited

Loading