From 25bb1c99f4bdfb734c67b0d66b4ed73b9e2667a2 Mon Sep 17 00:00:00 2001 From: proost Date: Thu, 20 Aug 2026 00:26:31 +0900 Subject: [PATCH 1/2] fix: required user agent header --- .github/workflows/webserver.yml | 54 ++--- .../otel-webserver-module/build.gradle | 19 ++ .../docker/almalinux8/Dockerfile | 8 +- .../docker/centos7/Dockerfile | 6 +- .../docker/ubuntu20.04/Dockerfile | 2 +- .../src/nginx/ngx_http_opentelemetry_module.c | 13 +- .../issue_474_user_agent_regression_test.sh | 221 ++++++++++++++++++ 7 files changed, 272 insertions(+), 51 deletions(-) create mode 100755 instrumentation/otel-webserver-module/test/nginx/issue_474_user_agent_regression_test.sh diff --git a/.github/workflows/webserver.yml b/.github/workflows/webserver.yml index cf042be42..9e26471e4 100644 --- a/.github/workflows/webserver.yml +++ b/.github/workflows/webserver.yml @@ -1,6 +1,7 @@ name: webserver instrumentation CI on: + workflow_dispatch: push: branches: [ main ] paths: @@ -18,7 +19,7 @@ permissions: jobs: webserver-build-test-ubuntu: name: webserver-ubuntu-build - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 steps: - name: checkout otel webserver uses: actions/checkout@v7.0.1 @@ -31,9 +32,9 @@ jobs: uses: actions/cache@v6 with: path: /tmp/buildx-cache/ - key: apache-ubuntu-20.04-${{ github.sha }} + key: apache-ubuntu-22.04-${{ github.sha }} restore-keys: | - apache-ubuntu-20.04 + apache-ubuntu-22.04 - name: setup docker image run: | cd instrumentation/otel-webserver-module @@ -55,6 +56,11 @@ jobs: ./gradlew assembleWebServerModule -DtargetSystem=ubuntu' + - name: unit test + run: | + docker exec --workdir /otel-webserver-module apache_ubuntu_container \ + ./gradlew runTests -DtargetSystem=ubuntu + - name: update cache run: | rm -rf /tmp/buildx-cache/apache_ubuntu @@ -62,7 +68,7 @@ jobs: webserver-build-test-centos7: name: webserver-centos7-build - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 steps: - name: checkout otel webserver uses: actions/checkout@v7.0.1 @@ -99,8 +105,8 @@ jobs: ./gradlew assembleWebServerModule' - name: unit test run: | - docker exec apache_centos7_container bash -c \ - 'cd /otel-webserver-module; ./gradlew runUnitTest' + docker exec --workdir /otel-webserver-module apache_centos7_container \ + ./gradlew runTests # - name: update cache # run: | # rm -rf /tmp/buildx-cache/apache_centos7 @@ -127,11 +133,9 @@ jobs: # sleep 30 # ./gradlew :test:integration:integrationTests -i # curl http://localhost:9411/api/v2/spans?serviceName=demoservice - - webserver-build-test-almalinux8: name: webserver-almalinux8-build - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 steps: - name: checkout otel webserver uses: actions/checkout@v7.0.1 @@ -168,8 +172,8 @@ jobs: ./gradlew assembleWebServerModule' - name: unit test run: | - docker exec apache_almalinux8_container bash -c \ - 'cd /otel-webserver-module; ./gradlew runUnitTest' + docker exec --workdir /otel-webserver-module apache_almalinux8_container \ + ./gradlew runTests # - name: update cache # run: | # rm -rf /tmp/buildx-cache/apache_almalinux8 @@ -196,31 +200,3 @@ jobs: # sleep 30 # ./gradlew :test:integration:integrationTests -i # curl http://localhost:9411/api/v2/spans?serviceName=demoservice - - - Codeql-build: - permissions: - security-events: write # for github/codeql-action/analyze to upload SARIF results - name: static-analysis - runs-on: ubuntu-20.04 - steps: - - name: checkout otel webserver - uses: actions/checkout@v7.0.1 - - name: setup environment - run: | - cd .. - cp opentelemetry-cpp-contrib/instrumentation/otel-webserver-module/codeql-env.sh . - sudo ./codeql-env.sh - - name: Initialize CodeQL - uses: github/codeql-action/init@v4.37.6 - with: - languages: cpp - - name: build - run: | - cd instrumentation/otel-webserver-module - cp -r /dependencies ../otel-webserver-module/ - cp -r /build-dependencies ../otel-webserver-module/ - ./gradlew assembleWebserverModule -DtargetSystem=ubuntu --info - - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v4.37.6 - diff --git a/instrumentation/otel-webserver-module/build.gradle b/instrumentation/otel-webserver-module/build.gradle index 5f8ea4a41..1c1eaf259 100644 --- a/instrumentation/otel-webserver-module/build.gradle +++ b/instrumentation/otel-webserver-module/build.gradle @@ -539,6 +539,25 @@ task runApacheServer(type: Exec) { commandLine './ApacheTesting.sh', "${target_system}" } +task runNginxIssue474Test(type: Exec) { + group = 'verification' + description = 'Run the NGINX issue #474 User-Agent regression test' + + dependsOn assembleNginxModule + + workingDir 'test/nginx' + commandLine './issue_474_user_agent_regression_test.sh', + "${buildDir}/opentelemetry-webserver-sdk-${osArch}-${osName}.tgz", + '1.26.0' +} + +task runTests { + group = 'verification' + description = 'Run all webserver module tests' + + dependsOn runUnitTest, runNginxIssue474Test +} + // Code Coverage task lcovCapture(type: Exec) { diff --git a/instrumentation/otel-webserver-module/docker/almalinux8/Dockerfile b/instrumentation/otel-webserver-module/docker/almalinux8/Dockerfile index 2e3d01d33..30863b9d8 100644 --- a/instrumentation/otel-webserver-module/docker/almalinux8/Dockerfile +++ b/instrumentation/otel-webserver-module/docker/almalinux8/Dockerfile @@ -144,14 +144,14 @@ RUN wget --no-check-certificate https://ftp.gnu.org/gnu/automake/automake-${AUTO && cd .. && rm -rf automake-${AUTOMAKE_VERSION}.tar.gz # install libtool -RUN wget --no-check-certificate https://ftpmirror.gnu.org/libtool/libtool-${LIBTOOL_VERSION}.tar.gz \ +RUN wget --no-check-certificate https://ftp.gnu.org/gnu/libtool/libtool-${LIBTOOL_VERSION}.tar.gz \ && tar xzf libtool-${LIBTOOL_VERSION}.tar.gz \ && cd libtool-${LIBTOOL_VERSION} \ && ./configure --prefix=/usr \ && make -j 6 \ && make install \ && libtool --version \ - && cd .. && rm -rf libtool--${LIBTOOL_VERSION}.tar.gz + && cd .. && rm -rf libtool-${LIBTOOL_VERSION}.tar.gz #install log4cxx RUN mkdir -p dependencies/apache-log4cxx/${LOG4CXX_VERSION} \ @@ -254,9 +254,9 @@ RUN cd /otel-webserver-module/build \ RUN cp /otel-webserver-module/conf/nginx/opentelemetry_module.conf /opt/ \ - && sed -i '8i load_module /opt/opentelemetry-webserver-sdk/WebServerModule/Nginx/1.26.2/ngx_http_opentelemetry_module.so;' /etc/nginx/nginx.conf \ + && sed -i "8i load_module /opt/opentelemetry-webserver-sdk/WebServerModule/Nginx/${NGINX_VERSION}/ngx_http_opentelemetry_module.so;" /etc/nginx/nginx.conf \ && sed -i '33i include /opt/opentelemetry_module.conf;' /etc/nginx/nginx.conf \ && cd / COPY entrypoint.sh /usr/local/bin/ -ENTRYPOINT ["entrypoint.sh"] \ No newline at end of file +ENTRYPOINT ["entrypoint.sh"] diff --git a/instrumentation/otel-webserver-module/docker/centos7/Dockerfile b/instrumentation/otel-webserver-module/docker/centos7/Dockerfile index b8571d35d..056bb3ed5 100644 --- a/instrumentation/otel-webserver-module/docker/centos7/Dockerfile +++ b/instrumentation/otel-webserver-module/docker/centos7/Dockerfile @@ -185,14 +185,14 @@ RUN wget --no-check-certificate https://ftp.gnu.org/gnu/automake/automake-${AUTO && cd .. && rm -rf automake-${AUTOMAKE_VERSION}.tar.gz # install libtool -RUN wget --no-check-certificate https://ftpmirror.gnu.org/libtool/libtool-${LIBTOOL_VERSION}.tar.gz \ +RUN wget --no-check-certificate https://ftp.gnu.org/gnu/libtool/libtool-${LIBTOOL_VERSION}.tar.gz \ && tar xzf libtool-${LIBTOOL_VERSION}.tar.gz \ && cd libtool-${LIBTOOL_VERSION} \ && ./configure --prefix=/usr \ && make -j 6 \ && make install \ && libtool --version \ - && cd .. && rm -rf libtool--${LIBTOOL_VERSION}.tar.gz + && cd .. && rm -rf libtool-${LIBTOOL_VERSION}.tar.gz #install log4cxx RUN mkdir -p dependencies/apache-log4cxx/${LOG4CXX_VERSION} \ @@ -303,4 +303,4 @@ RUN rm -rf grpc && rm -rf autoconf-${AUTOCONF_VERSION} && rm -rf automake-${AUTO && rm -f httpd-2.2.31.tar.gz && rm -f httpd-2.4.23.tar.gz COPY entrypoint.sh /usr/local/bin/ -ENTRYPOINT ["entrypoint.sh"] \ No newline at end of file +ENTRYPOINT ["entrypoint.sh"] diff --git a/instrumentation/otel-webserver-module/docker/ubuntu20.04/Dockerfile b/instrumentation/otel-webserver-module/docker/ubuntu20.04/Dockerfile index 34e524c45..94b8faaad 100644 --- a/instrumentation/otel-webserver-module/docker/ubuntu20.04/Dockerfile +++ b/instrumentation/otel-webserver-module/docker/ubuntu20.04/Dockerfile @@ -190,7 +190,7 @@ RUN echo "deb [signed-by=/usr/share/keyrings/nginx-archive-keyring.gpg] \ | tee /etc/apt/sources.list.d/nginx.list \ && echo "Package: *\nPin: origin nginx.org\nPin: release o=nginx\nPin-Priority: 900\n" \ | tee /etc/apt/preferences.d/99nginx \ - && apt update -y && apt install nginx -y + && apt update -y && apt install "nginx=${NGINX_VERSION}-1~focal" -y # Build Webserver Module COPY . /otel-webserver-module diff --git a/instrumentation/otel-webserver-module/src/nginx/ngx_http_opentelemetry_module.c b/instrumentation/otel-webserver-module/src/nginx/ngx_http_opentelemetry_module.c index da6914f53..1363381a9 100644 --- a/instrumentation/otel-webserver-module/src/nginx/ngx_http_opentelemetry_module.c +++ b/instrumentation/otel-webserver-module/src/nginx/ngx_http_opentelemetry_module.c @@ -2056,10 +2056,15 @@ static void fillRequestPayload(request_payload* req_payload, ngx_http_request_t* temp_request_method[(r->method_name).len]='\0'; req_payload->request_method = temp_request_method; - char *temp_user_agent = ngx_pcalloc(r->pool, r->headers_in.user_agent->value.len +1); - strcpy(temp_user_agent,(const char*)(r->headers_in.user_agent->value.data)); - temp_user_agent[r->headers_in.user_agent->value.len]='\0'; - req_payload->user_agent = temp_user_agent; + ngx_table_elt_t *user_agent = r->headers_in.user_agent; + if (user_agent == NULL) { + req_payload->user_agent = ""; + } else { + char *temp_user_agent = ngx_pcalloc(r->pool, r->headers_in.user_agent->value.len+1); + strcpy(temp_user_agent, (const char*)(r->headers_in.user_agent->value.data)); + temp_user_agent[r->headers_in.user_agent->value.len]='\0'; + req_payload->user_agent = temp_user_agent; + } ngx_uint_t remote_port = 0; if (r->connection != NULL) { diff --git a/instrumentation/otel-webserver-module/test/nginx/issue_474_user_agent_regression_test.sh b/instrumentation/otel-webserver-module/test/nginx/issue_474_user_agent_regression_test.sh new file mode 100755 index 000000000..28d7014ef --- /dev/null +++ b/instrumentation/otel-webserver-module/test/nginx/issue_474_user_agent_regression_test.sh @@ -0,0 +1,221 @@ +#!/usr/bin/env bash + +set -Eeuo pipefail + +if [[ $# -ne 2 ]]; then + echo "Usage: $0 " >&2 + exit 2 +fi + +archive_path="$1" +nginx_version="$2" +nginx_binary="${NGINX_BINARY:-nginx}" +test_port="${NGINX_ISSUE_474_TEST_PORT:-18080}" + +for command in curl ps tar "$nginx_binary"; do + if ! command -v "$command" >/dev/null 2>&1; then + echo "Required command not found: $command" >&2 + exit 1 + fi +done + +if [[ ! -f "$archive_path" ]]; then + echo "Webserver module archive not found: $archive_path" >&2 + exit 1 +fi + +runtime_version="$($nginx_binary -v 2>&1)" +if [[ "$runtime_version" != *"nginx/$nginx_version"* ]]; then + echo "Expected nginx/$nginx_version, but found: $runtime_version" >&2 + exit 1 +fi + +test_root="$(mktemp -d /tmp/otel-nginx-issue-474.XXXXXX)" +nginx_prefix="$test_root/nginx" +nginx_config="$nginx_prefix/nginx.conf" +nginx_error_log="$nginx_prefix/error.log" +nginx_pid_file="$nginx_prefix/nginx.pid" +nginx_pid="" +current_test="startup" + +cleanup() { + local exit_code=$? + trap - EXIT + + if [[ -n "$nginx_pid" ]] && kill -0 "$nginx_pid" >/dev/null 2>&1; then + "$nginx_binary" -p "$nginx_prefix/" -c "$nginx_config" -s quit >/dev/null 2>&1 || true + for _ in {1..50}; do + if ! kill -0 "$nginx_pid" >/dev/null 2>&1; then + break + fi + sleep 0.1 + done + if kill -0 "$nginx_pid" >/dev/null 2>&1; then + kill "$nginx_pid" >/dev/null 2>&1 || true + fi + fi + + if [[ $exit_code -ne 0 ]]; then + echo "FAILED: $current_test" >&2 + if [[ -f "$nginx_error_log" ]]; then + echo "NGINX error log:" >&2 + cat "$nginx_error_log" >&2 + fi + fi + + case "$test_root" in + /tmp/otel-nginx-issue-474.*) + rm -rf -- "$test_root" + ;; + esac + + exit "$exit_code" +} +trap cleanup EXIT + +mkdir -p "$nginx_prefix" +tar -xzf "$archive_path" -C "$test_root" + +sdk_root="$test_root/opentelemetry-webserver-sdk" +module_path="$sdk_root/WebServerModule/Nginx/$nginx_version/ngx_http_opentelemetry_module.so" +sdk_library_path="$sdk_root/sdk_lib/lib" + +if [[ ! -f "$module_path" ]]; then + echo "NGINX module not found in archive: $module_path" >&2 + exit 1 +fi + +if [[ ! -x "$sdk_root/install.sh" ]]; then + echo "SDK installer not found in archive: $sdk_root/install.sh" >&2 + exit 1 +fi + +"$sdk_root/install.sh" --ignore-permissions + +export LD_LIBRARY_PATH="$sdk_library_path${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}" +export OTEL_SDK_LOG_CONFIG_PATH="$sdk_root/conf/opentelemetry_sdk_log4cxx.xml" + +cat >"$nginx_config" </dev/null; then + break + fi + fi + sleep 0.1 +done + +if [[ -z "$nginx_pid" ]] || ! kill -0 "$nginx_pid" >/dev/null 2>&1; then + echo "NGINX did not start" >&2 + exit 1 +fi + +worker_pid() { + ps -eo pid=,ppid= | awk -v parent="$nginx_pid" '$2 == parent { print $1; exit }' +} + +assert_no_worker_crash() { + if grep -Eiq 'worker process .*exited on signal|segmentation fault|core dumped' "$nginx_error_log"; then + echo "NGINX worker crash detected" >&2 + return 1 + fi +} + +run_test_case() { + local test_name="$1" + shift + + current_test="$test_name" + echo "RUN: $test_name" + + local worker_before + local worker_after + local http_status + + worker_before="$(worker_pid)" + if [[ -z "$worker_before" ]]; then + echo "Unable to find the NGINX worker before $test_name" >&2 + return 1 + fi + + if ! http_status="$(curl --silent --show-error --output /dev/null \ + --write-out '%{http_code}' --max-time 5 --http1.1 \ + "$@" "http://127.0.0.1:$test_port/issue-474")"; then + echo "Request failed for $test_name" >&2 + return 1 + fi + + if [[ "$http_status" != "200" ]]; then + echo "Expected HTTP 200 for $test_name, received $http_status" >&2 + return 1 + fi + + worker_after="$(worker_pid)" + if [[ "$worker_after" != "$worker_before" ]]; then + echo "NGINX worker changed during $test_name: $worker_before -> ${worker_after:-missing}" >&2 + return 1 + fi + + assert_no_worker_crash + echo "PASS: $test_name" +} + +test_non_empty_user_agent() { + run_test_case "non-empty User-Agent" -H 'User-Agent: otel-regression-test' +} + +test_omitted_user_agent() { + run_test_case "omitted User-Agent" -H 'User-Agent:' +} + +test_empty_user_agent() { + run_test_case "empty User-Agent" -H 'User-Agent;' +} + +test_non_empty_user_agent +test_omitted_user_agent +test_empty_user_agent + +current_test="complete" +echo "All NGINX issue #474 regression tests passed" From 64d4d4fdcbccc8c8673953be4e6eb55754273c52 Mon Sep 17 00:00:00 2001 From: proost Date: Fri, 21 Aug 2026 17:43:14 +0900 Subject: [PATCH 2/2] refactor: remove strcopy --- .../src/nginx/ngx_http_opentelemetry_module.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/instrumentation/otel-webserver-module/src/nginx/ngx_http_opentelemetry_module.c b/instrumentation/otel-webserver-module/src/nginx/ngx_http_opentelemetry_module.c index 1363381a9..e70061030 100644 --- a/instrumentation/otel-webserver-module/src/nginx/ngx_http_opentelemetry_module.c +++ b/instrumentation/otel-webserver-module/src/nginx/ngx_http_opentelemetry_module.c @@ -2060,9 +2060,8 @@ static void fillRequestPayload(request_payload* req_payload, ngx_http_request_t* if (user_agent == NULL) { req_payload->user_agent = ""; } else { - char *temp_user_agent = ngx_pcalloc(r->pool, r->headers_in.user_agent->value.len+1); - strcpy(temp_user_agent, (const char*)(r->headers_in.user_agent->value.data)); - temp_user_agent[r->headers_in.user_agent->value.len]='\0'; + char *temp_user_agent = ngx_pcalloc(r->pool, user_agent->value.len + 1); + ngx_memcpy(temp_user_agent, user_agent->value.data, user_agent->value.len); req_payload->user_agent = temp_user_agent; }