From 4de6bdb87a8d7e3b33b7cf94a6abd6f8ec8870e6 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Sat, 9 May 2026 01:25:19 +0200 Subject: [PATCH 01/11] Add devcontainer workflow for local builds and integration tests (#60) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a `.devcontainer/` image definition and a thin Makefile + runner script so developers can reproduce the GitLab build + integration-test pipeline locally. The image bakes in the LLVM+musl sysroot, Rust+cbindgen for RUM, uv, and a prebuilt httpd. Two non-obvious bits in the runner script: - `UV_PROJECT_ENVIRONMENT` points outside the bind-mounted repo, so a host-side `uv sync` on darwin can't leave mac wheels in `.venv/` that the container's Linux Python then tries to import. - Pytest options use `--flag=VALUE` — the space-separated form is captured by pytest's early arg parser as a positional test path before conftest registers the custom option, which silently skips conftest. The GitLab `build-ci-image` job's hash input and Dockerfile path are retargeted to the new `.devcontainer/` location. --- {.gitlab => .devcontainer}/Dockerfile | 0 .devcontainer/context.files | 21 +++++ .devcontainer/devcontainer.json | 11 +++ .devcontainer/run-integration-tests.sh | 55 +++++++++++ .github/workflows/CI_IMAGE.md | 44 +++++++++ .github/workflows/dev.yml | 14 +-- .github/workflows/release.yml | 12 +-- .github/workflows/system-tests.yml | 12 +-- .gitignore | 6 ++ .gitlab-ci.yml | 124 ++++++------------------- .gitmodules | 6 +- Makefile | 71 +++++++++++--- 12 files changed, 233 insertions(+), 143 deletions(-) rename {.gitlab => .devcontainer}/Dockerfile (100%) create mode 100644 .devcontainer/context.files create mode 100644 .devcontainer/devcontainer.json create mode 100755 .devcontainer/run-integration-tests.sh create mode 100644 .github/workflows/CI_IMAGE.md diff --git a/.gitlab/Dockerfile b/.devcontainer/Dockerfile similarity index 100% rename from .gitlab/Dockerfile rename to .devcontainer/Dockerfile diff --git a/.devcontainer/context.files b/.devcontainer/context.files new file mode 100644 index 0000000..2f1fcdc --- /dev/null +++ b/.devcontainer/context.files @@ -0,0 +1,21 @@ +# Build-context allowlist for the devcontainer image. One rsync-relative +# path per line; '#' comments and blank lines are stripped before rsync +# sees them. Anything the Dockerfile COPYs has to live under one of these. +# +# Only files whose contents affect the image go in this list — anything +# that's only invoked at runtime via volume mount (e.g. +# run-integration-tests.sh) is intentionally excluded so editing it +# doesn't invalidate the cached image tag. + +# image recipe +.devcontainer/Dockerfile + +# musl/LLVM toolchain inputs (CHECKSUMS, locale.h.diff, alltypes.h.diff, +# Toolchain.cmake., glibc_compat.c) — sourced from the +# nginx-datadog submodule's build_env so the cross-compile sysroot here +# matches what nginx-datadog uses. +deps/nginx-datadog/build_env/ + +# Downloads + builds httpd 2.4 inside the image so test jobs have an +# Apache to load mod_datadog into without rebuilding it per pipeline. +scripts/setup-httpd.py diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json new file mode 100644 index 0000000..8b9cf16 --- /dev/null +++ b/.devcontainer/devcontainer.json @@ -0,0 +1,11 @@ +{ + "name": "httpd-datadog", + "initializeCommand": "make -f .devcontainer/devcontainer.mk .devcontainer-stage-context", + "build": { + "dockerfile": ".staged/.devcontainer/Dockerfile", + "context": ".staged", + "args": { + "ARCH": "${localEnv:ARCH:x86_64}" + } + } +} diff --git a/.devcontainer/run-integration-tests.sh b/.devcontainer/run-integration-tests.sh new file mode 100755 index 0000000..cfcd025 --- /dev/null +++ b/.devcontainer/run-integration-tests.sh @@ -0,0 +1,55 @@ +#!/bin/sh +# Build mod_datadog (with RUM) and run the full integration-test suite. +# Assumes it runs inside the devcontainer image (Alpine + musl sysroot + httpd +# at /httpd/httpd-build/ + uv). +# +# If MODULE_PATH is set in the environment, the cmake build is skipped and +# pytest is pointed at the pre-built module at that path. CI uses this to +# reuse the artifact produced by the build:$ARCH job instead of rebuilding +# from source inside the test job. +set -eux + +REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)" +cd "$REPO_ROOT" + +ARCH="$(uname -m)" + +# Use a container-specific build tree to avoid colliding with host builds +# (host cmake would use Unix Makefiles; the ci-release preset uses Ninja). +BUILD_DIR="$REPO_ROOT/build-container" +DIST_DIR="$REPO_ROOT/dist-container" + +# Repo root is bind-mounted from the host; cmake complains otherwise. +git config --global --add safe.directory "$REPO_ROOT" + +if [ -z "${MODULE_PATH:-}" ]; then + cmake --preset=ci-release \ + -DHTTPD_DATADOG_ENABLE_RUM=ON \ + -DCMAKE_TOOLCHAIN_FILE="/sysroot/${ARCH}-none-linux-musl/Toolchain.cmake" \ + -B "$BUILD_DIR" . + cmake --build "$BUILD_DIR" -j + # Wipe DIST_DIR so a removed/renamed file in the build doesn't + # linger from a previous install and silently get used by tests. + rm -rf "$DIST_DIR" + cmake --install "$BUILD_DIR" --prefix "$DIST_DIR" + MODULE_PATH="$DIST_DIR/lib/mod_datadog.so" +fi + +cd test/integration-test + +# Keep the venv outside the bind-mounted repo so a host-side `uv sync` +# (darwin/arm64 wheels) can't shadow the container's Linux one. +export UV_PROJECT_ENVIRONMENT=/root/.venv-httpd-datadog-tests + +uv sync + +# Pytest's early arg parser runs BEFORE conftest registers `--bin-path`. +# If we pass `--bin-path VALUE` (space-separated), that parser treats the +# value as a positional test path and uses its parent as rootdir, which +# then prevents conftest from loading. The `=` syntax keeps the option and +# its value in a single token and avoids that race. +uv run pytest \ + --bin-path=/httpd/httpd-build/bin/apachectl \ + --module-path="$MODULE_PATH" \ + --log-dir="$REPO_ROOT/logs" \ + -v "$@" diff --git a/.github/workflows/CI_IMAGE.md b/.github/workflows/CI_IMAGE.md new file mode 100644 index 0000000..4c5b056 --- /dev/null +++ b/.github/workflows/CI_IMAGE.md @@ -0,0 +1,44 @@ +# CI image used by GitHub Actions + +The workflows in this directory pin `image:` to +`datadog/docker-library:httpd-datadog-ci-` — a public Docker Hub +mirror of the internal devcontainer image built by GitLab CI from +[`.devcontainer/Dockerfile`](../../.devcontainer/Dockerfile). The build ++ caching pipeline lives in [`.gitlab/devcontainer.yml`](../../.gitlab/devcontainer.yml); +the image-tag hash is the sha256 of the staged build context defined +by [`.devcontainer/context.files`](../../.devcontainer/context.files). + +GitHub-hosted runners can't pull from `registry.ddbuild.io`, so we +periodically copy the latest amd64 variant to Docker Hub and bump the +`image:` reference in each workflow. + +## Bumping the image + +Once GitLab CI has published a new tag (any change to a path listed in +`.devcontainer/context.files` invalidates the cache and triggers a +rebuild), mirror it from the repo root: + +```sh +make mirror-public-image +``` + +The target stages the build context, computes the same 12-char hash +GitLab uses, pulls `registry.ddbuild.io/ci/httpd-datadog/devcontainer:amd64-`, +retags it as `datadog/docker-library:httpd-datadog-ci-`, and +pushes. It then prints the exact `image:` value to copy into: + +- `dev.yml` (build + test jobs) +- `release.yml` (build job) +- `system-tests.yml` (build-artifacts job) + +## Why two image namespaces + +| Consumer | Pull from | Why | +|-----------------|----------------------------------------------------------------------------|-----| +| GitLab CI | `registry.ddbuild.io/ci/httpd-datadog/devcontainer:-` | private, fast, nydus-optimized | +| GitHub Actions | `datadog/docker-library:httpd-datadog-ci-` (mirrored from the above) | public, reachable from GitHub-hosted runners | +| Local `make` | Either of the above (pull `registry.ddbuild.io/...` if you have access; build locally otherwise) | dev convenience | + +The two mirrors stay in sync only as far as the most recent +`make mirror-public-image` run — bump the workflows promptly after +running it. diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 8fcd549..a551fa5 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -19,20 +19,12 @@ jobs: needs: format runs-on: ubuntu-22.04 container: - # See in Makefile where this image comes from. + # See .github/workflows/CI_IMAGE.md — bump via `make mirror-public-image`. image: datadog/docker-library:httpd-datadog-ci-28219c0ef3e00f1e3d5afcab61a73a5e9bd2a9b957d7545556711cce2a6262cd steps: - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 - - name: Add cloned repo as safe - run: sh -c "git config --global --add safe.directory $PWD" - - name: Init required submodules - run: git submodule update --init --depth=1 deps/dd-trace-cpp deps/nginx-datadog - - name: Configure - run: cmake --preset=ci-dev -B build . - name: Build - run: | - cmake --build build -j --verbose - cmake --install build --prefix dist + run: make ci-build PRESET=ci-dev - name: Export library uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: @@ -44,7 +36,7 @@ jobs: needs: build runs-on: ubuntu-22.04 container: - # See in Makefile where this image comes from. + # See .github/workflows/CI_IMAGE.md — bump via `make mirror-public-image`. image: datadog/docker-library:httpd-datadog-ci-28219c0ef3e00f1e3d5afcab61a73a5e9bd2a9b957d7545556711cce2a6262cd env: DD_ENV: ci diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 7a7539b..6514dd7 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -5,20 +5,12 @@ jobs: build: runs-on: ubuntu-22.04 container: - # See in Makefile where this image comes from. + # See .github/workflows/CI_IMAGE.md — bump via `make mirror-public-image`. image: datadog/docker-library:httpd-datadog-ci-28219c0ef3e00f1e3d5afcab61a73a5e9bd2a9b957d7545556711cce2a6262cd steps: - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 - - name: Add cloned repo as safe - run: sh -c "git config --global --add safe.directory $PWD" - - name: Init required submodules - run: git submodule update --init --depth=1 deps/dd-trace-cpp deps/nginx-datadog - - name: Configure - run: cmake --preset ci-release -B build . - name: Build - run: | - cmake --build build -j --verbose - cmake --install build --prefix dist + run: make ci-build PRESET=ci-release - name: Export library uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: diff --git a/.github/workflows/system-tests.yml b/.github/workflows/system-tests.yml index 72414ee..dbde772 100644 --- a/.github/workflows/system-tests.yml +++ b/.github/workflows/system-tests.yml @@ -19,20 +19,12 @@ jobs: build-artifacts: runs-on: ubuntu-22.04 container: - # See in Makefile where this image comes from. + # See .github/workflows/CI_IMAGE.md — bump via `make mirror-public-image`. image: datadog/docker-library:httpd-datadog-ci-28219c0ef3e00f1e3d5afcab61a73a5e9bd2a9b957d7545556711cce2a6262cd steps: - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 - - name: Add cloned repo as safe - run: sh -c "git config --global --add safe.directory $PWD" - - name: Init required submodules - run: git submodule update --init --depth=1 deps/dd-trace-cpp deps/nginx-datadog - - name: Configure - run: cmake --preset=ci-dev -B build . - name: Build - run: | - cmake --build build -j --verbose - cmake --install build --prefix dist + run: make ci-build PRESET=ci-dev - name: Export library uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: diff --git a/.gitignore b/.gitignore index 75feb22..cb252be 100644 --- a/.gitignore +++ b/.gitignore @@ -23,10 +23,13 @@ compile_commands.json __pycache__/ +build-container/ build-rum/ build/ +dist-container/ httpd-*/ httpd/ +/logs/ venv/ test/integration-test/.coverage @@ -37,3 +40,6 @@ test/integration-test/htmlcov/ test/integration-test/log-*/ test/integration-test/logs/ test/integration-test/uv.lock +.devcontainer/.staged/ +.devcontainer/.image-ref +.devcontainer/.image-ref-x86_64 diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 4a328df..9e9b3b6 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,5 +1,15 @@ +include: + - local: .gitlab/devcontainer.yml + variables: - CI_DOCKER_IMAGE_BASE: registry.ddbuild.io/ci/httpd-datadog + # Inputs to .gitlab/devcontainer.yml (the dd-repo-tools shared + # devcontainer template). DEVCONTAINER_PER_ARCH=true publishes + # $IMAGE:amd64-$TAG and $IMAGE:arm64-$TAG; the downstream build / test + # jobs below then pull the matching arch and run on native runners + # (`tags: ["arch:$ARCH"]`). The cmake compile + integration-test suite + # is too slow under qemu to run any other way. + DEVCONTAINER_REPO_NAME: httpd-datadog + DEVCONTAINER_PER_ARCH: "true" GIT_SUBMODULE_STRATEGY: recursive GIT_SUBMODULE_DEPTH: 1 GIT_CONFIG_COUNT: 1 @@ -12,7 +22,6 @@ default: aud: image-integrity stages: - - ci-image - build - test - aws @@ -20,7 +29,9 @@ stages: # Template jobs (hidden, reusable) .build-template: stage: build - image: $CI_DOCKER_IMAGE_BASE:$CI_IMAGE_HASH + needs: + - job: devcontainer_image + artifacts: true script: - git config --global --add safe.directory $PWD - > @@ -42,97 +53,16 @@ stages: .test-rum-template: stage: test - image: $CI_DOCKER_IMAGE_BASE:$CI_IMAGE_HASH + # Same `--flag=VALUE` reasoning as run-integration-tests.sh: pytest's early + # arg parser runs before conftest registers these custom options, so the + # space-separated form gets read as a positional test path and skips + # conftest. script: - - cd $CI_PROJECT_DIR/test/integration-test && uv sync && uv run pytest scenarios --module-path $CI_PROJECT_DIR/artifacts/$ARCH/mod_datadog.so --bin-path /httpd/httpd-build/bin/apachectl --log-dir $CI_PROJECT_DIR/logs -m requires_rum -v - -build-ci-image: - stage: ci-image - tags: ["arch:$ARCH"] - image: registry.ddbuild.io/images/docker:27.3.1 - parallel: - matrix: - - ARCH: amd64 - TOOLCHAIN_ARCH: x86_64 - - ARCH: arm64 - TOOLCHAIN_ARCH: aarch64 - script: - - | - FILES=$(find .gitlab/Dockerfile deps/nginx-datadog/build_env/ scripts/setup-httpd.py -type f | sort) - HASH=$(for f in $FILES; do echo "--- $f ---"; cat "$f"; done | sha256sum | cut -d ' ' -f 1) - - echo "CI_IMAGE_HASH=$HASH" >> ci-image.env - - IMAGE_TAG="$CI_DOCKER_IMAGE_BASE/$ARCH:$HASH" - - | - if docker buildx imagetools inspect "$IMAGE_TAG" > /dev/null 2>&1; then - echo "Image $IMAGE_TAG already exists. Skipping build." - else - docker buildx build \ - --tag $IMAGE_TAG \ - --platform linux/$ARCH \ - --build-arg ARCH=$TOOLCHAIN_ARCH \ - --push \ - --file .gitlab/Dockerfile \ - . - echo "Image $IMAGE_TAG built for $ARCH." - fi - artifacts: - reports: - dotenv: ci-image.env - -create-ci-manifest: - stage: ci-image - needs: - - build-ci-image - tags: ["arch:amd64"] - image: registry.ddbuild.io/images/docker:27.3.1 - script: - - MANIFEST_TAG="$CI_DOCKER_IMAGE_BASE:$CI_IMAGE_HASH" - - | - if docker buildx imagetools inspect "$MANIFEST_TAG" > /dev/null 2>&1; then - echo "Multi-arch manifest $MANIFEST_TAG already exists. Skipping." - else - docker buildx imagetools create \ - --tag "$MANIFEST_TAG" \ - "$CI_DOCKER_IMAGE_BASE/amd64:$CI_IMAGE_HASH" \ - "$CI_DOCKER_IMAGE_BASE/arm64:$CI_IMAGE_HASH" - fi - -nydusify-ci-image: - stage: ci-image - needs: - - build-ci-image - - create-ci-manifest - allow_failure: true - tags: ["arch:amd64"] - image: registry.ddbuild.io/images/nydus:v98111448-6d7c7d0-v2.3.8-dd - variables: - DDSIGN_SKIP_SIGNING: "true" - script: - - | - IMAGE="$CI_DOCKER_IMAGE_BASE:$CI_IMAGE_HASH" - REPO="${IMAGE%:*}" - - # Resolve a platform manifest digest from the multi-arch index - DIGEST=$(crane manifest "$IMAGE" 2>/dev/null | jq -r '(.manifests // [])[0].digest // empty' 2>/dev/null) - if [ -n "$DIGEST" ]; then - MANIFEST=$(crane manifest "${REPO}@${DIGEST}" 2>/dev/null) - else - MANIFEST=$(crane manifest "$IMAGE" 2>/dev/null) - fi - - # Check if the platform manifest already contains nydus layers - if echo "$MANIFEST" | jq -e '[.layers[].mediaType] | any(contains("nydus"))' > /dev/null 2>&1; then - echo "$IMAGE is already nydusified. Skipping." - else - echo "Converting $IMAGE with nydusify..." - nydus-convert "$IMAGE" "$IMAGE" - fi + - cd $CI_PROJECT_DIR/test/integration-test && uv sync && uv run pytest scenarios --module-path=$CI_PROJECT_DIR/artifacts/$ARCH/mod_datadog.so --bin-path=/httpd/httpd-build/bin/apachectl --log-dir=$CI_PROJECT_DIR/logs -m requires_rum -v build:amd64: extends: .build-template - needs: - - build-ci-image - - create-ci-manifest + image: $DEVCONTAINER_IMAGE_AMD64 variables: ARCH: amd64 TOOLCHAIN_ARCH: x86_64 @@ -140,9 +70,7 @@ build:amd64: build:arm64: extends: .build-template - needs: - - build-ci-image - - create-ci-manifest + image: $DEVCONTAINER_IMAGE_ARM64 variables: ARCH: arm64 TOOLCHAIN_ARCH: aarch64 @@ -150,18 +78,24 @@ build:arm64: test-rum:amd64: extends: .test-rum-template + image: $DEVCONTAINER_IMAGE_AMD64 needs: - - build-ci-image + - job: devcontainer_image + artifacts: true - job: "build:amd64" + artifacts: true variables: ARCH: amd64 tags: ["arch:amd64"] test-rum:arm64: extends: .test-rum-template + image: $DEVCONTAINER_IMAGE_ARM64 needs: - - build-ci-image + - job: devcontainer_image + artifacts: true - job: "build:arm64" + artifacts: true variables: ARCH: arm64 tags: ["arch:arm64"] diff --git a/.gitmodules b/.gitmodules index 52a8a84..52183b5 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,10 +1,10 @@ [submodule "deps/dd-trace-cpp"] path = deps/dd-trace-cpp - url = ../dd-trace-cpp.git + url = https://github.com/DataDog/dd-trace-cpp.git [submodule "deps/nginx-datadog"] path = deps/nginx-datadog - url = ../httpd-datadog.git + url = https://github.com/DataDog/httpd-datadog.git branch = mirror-nginx-datadog [submodule "deps/inject-browser-sdk"] path = deps/inject-browser-sdk - url = ../inject-browser-sdk.git + url = https://github.com/DataDog/inject-browser-sdk.git diff --git a/Makefile b/Makefile index 4360495..63610c0 100644 --- a/Makefile +++ b/Makefile @@ -1,17 +1,60 @@ -# The CI image used for some GitHub jobs is built by the GitLab build-ci-image job. -# The hash is computed by this GitLab job from the files used to build the image. +# Pin the image name so it doesn't drift with the worktree directory name +# (the upstream default is derived from $(notdir $(CURDIR)) — fine for a +# canonical clone called `httpd-datadog`, wrong for any feature-branch +# worktree). Matches DEVCONTAINER_IMAGE_NAME in .gitlab-ci.yml. +DEV_CONTAINER_IMAGE_NAME := registry.ddbuild.io/ci/httpd-datadog/devcontainer + +# The devcontainer Dockerfile pulls toolchain files from the +# nginx-datadog submodule's build_env/. Without this guard the staging +# step silently produces a tree missing those files and the resulting +# tag mismatches what CI computes — fail loudly instead. # -# Whenever this image needs to be updated, one should: -# - Get the hash from a run of the GitLab build-ci-image job. -# - Copy this hash here, and in the GitHub workflow files. -# - Run: make replicate-ci-image-for-github. +# Skipped for `ci-build`, which runs its own `git submodule update` +# inline (the GitHub workflows checkout without submodules and init +# them as part of the build). All other targets (dev-image, +# test-integration, mirror-public-image) genuinely need build_env +# already present and keep the guard. +ifneq ($(MAKECMDGOALS),ci-build) +DEV_CONTAINER_REQUIRED_PATHS := deps/nginx-datadog/build_env/Toolchain.cmake.x86_64 +endif + +include .devcontainer/devcontainer.mk + +.PHONY: test-integration +test-integration: dev-image + $(IN_DEVCONTAINER) .devcontainer/run-integration-tests.sh -CI_DOCKER_IMAGE_HASH ?= 28219c0ef3e00f1e3d5afcab61a73a5e9bd2a9b957d7545556711cce2a6262cd -CI_IMAGE_FROM_GITLAB ?= registry.ddbuild.io/ci/httpd-datadog/amd64:$(CI_DOCKER_IMAGE_HASH) -CI_IMAGE_IN_PUBLIC_REPO_FOR_GITHUB ?= datadog/docker-library:httpd-datadog-ci-$(CI_DOCKER_IMAGE_HASH) +# One-shot CI build: trust the workdir, init the submodules cmake +# actually consumes, configure/build/install. Used by every job in +# .github/workflows/ that produces mod_datadog.so. Switch presets via +# PRESET=ci-release. inject-browser-sdk is omitted from the submodule +# list because RUM is off by default (HTTPD_DATADOG_ENABLE_RUM=OFF). +PRESET ?= ci-dev +.PHONY: ci-build +ci-build: + git config --global --add safe.directory $(CURDIR) + git submodule update --init --depth=1 deps/dd-trace-cpp deps/nginx-datadog + cmake --preset=$(PRESET) -B build . + cmake --build build -j --verbose + cmake --install build --prefix dist -.PHONY: replicate-ci-image-for-github -replicate-ci-image-for-github: - docker pull $(CI_IMAGE_FROM_GITLAB) - docker tag $(CI_IMAGE_FROM_GITLAB) $(CI_IMAGE_IN_PUBLIC_REPO_FOR_GITHUB) - docker push $(CI_IMAGE_IN_PUBLIC_REPO_FOR_GITHUB) +# GitHub-hosted runners can't pull from registry.ddbuild.io; the workflows +# in .github/workflows/ pin to a public Docker Hub mirror at +# datadog/docker-library:httpd-datadog-ci-. After a new tag is built +# by .gitlab/devcontainer.yml (any change to .devcontainer/context.files +# inputs invalidates the cache), run this target to copy the amd64 variant +# to Docker Hub and bump the workflows. +# See .github/workflows/CI_IMAGE.md for the bump procedure. +.PHONY: mirror-public-image +mirror-public-image: + @$(MAKE) -s -f .devcontainer/devcontainer.mk .devcontainer-stage-context + @hash=$$($(MAKE) -s -f .devcontainer/devcontainer.mk .devcontainer-image-hash); \ + src="$(DEV_CONTAINER_IMAGE_NAME):amd64-$$hash"; \ + public="datadog/docker-library:httpd-datadog-ci-$$hash"; \ + echo "Mirroring $$src -> $$public"; \ + docker pull --platform linux/amd64 "$$src"; \ + docker tag "$$src" "$$public"; \ + docker push "$$public"; \ + echo ""; \ + echo "Update image: in .github/workflows/{dev,release,system-tests}.yml to:"; \ + echo " $$public" From 21db2db3263f40621ab4ab2eca191a4028a2f3b5 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Sat, 9 May 2026 01:51:19 +0200 Subject: [PATCH 02/11] Fix integration-test hang and stale proxy.conf directive (#63) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fixes that together unwedge the integration suite and make it CI-visible: - **AioHTTPServer / TestAgent cleanup**: make them context managers, daemonize their background threads, bound the stop-time join, fix `_app.cleanup()` → `runner.cleanup()`, and set a `TCPSite` `shutdown_timeout`. Without these, a failed assertion in `test_http_proxy` kept non-daemon threads alive and wedged the process in `__futex_wait` until the container was SIGKILL'd. - **proxy.conf**: drop the orphan `` block left behind by `470db65` (deprecate sampling delegation); it broke apachectl with "Invalid command" for every proxy test. - **CI**: add `test-integration:$ARCH` jobs running the same `run-integration-tests.sh` entrypoint as `make test-integration`, scoped to the matching devcontainer-image matrix slot. The existing `test-rum` job only covered `requires_rum` tests — the rest of the suite was never run in CI. --- .gitlab-ci.yml | 37 +++++++++++++++++ test/integration-test/conftest.py | 40 ++++++++++++------- .../scenarios/conf/proxy.conf | 5 --- test/integration-test/scenarios/test_proxy.py | 36 +++++++++++------ 4 files changed, 85 insertions(+), 33 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 9e9b3b6..b7be329 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -60,6 +60,17 @@ stages: script: - cd $CI_PROJECT_DIR/test/integration-test && uv sync && uv run pytest scenarios --module-path=$CI_PROJECT_DIR/artifacts/$ARCH/mod_datadog.so --bin-path=/httpd/httpd-build/bin/apachectl --log-dir=$CI_PROJECT_DIR/logs -m requires_rum -v +# Runs the full integration-test suite via the same entrypoint local +# developers use (`make test-integration` -> run-integration-tests.sh). +# MODULE_PATH points the script at the build:$ARCH artifact so the cmake +# build step inside the script is skipped and only pytest runs. +.test-integration-template: + stage: test + variables: + MODULE_PATH: $CI_PROJECT_DIR/artifacts/$ARCH/mod_datadog.so + script: + - .devcontainer/run-integration-tests.sh + build:amd64: extends: .build-template image: $DEVCONTAINER_IMAGE_AMD64 @@ -100,6 +111,30 @@ test-rum:arm64: ARCH: arm64 tags: ["arch:arm64"] +test-integration:amd64: + extends: .test-integration-template + image: $DEVCONTAINER_IMAGE_AMD64 + needs: + - job: devcontainer_image + artifacts: true + - job: "build:amd64" + artifacts: true + variables: + ARCH: amd64 + tags: ["arch:amd64"] + +test-integration:arm64: + extends: .test-integration-template + image: $DEVCONTAINER_IMAGE_ARM64 + needs: + - job: devcontainer_image + artifacts: true + - job: "build:arm64" + artifacts: true + variables: + ARCH: arm64 + tags: ["arch:arm64"] + .upload-template: stage: aws needs: @@ -109,6 +144,8 @@ test-rum:arm64: artifacts: true - job: "test-rum:amd64" - job: "test-rum:arm64" + - job: "test-integration:amd64" + - job: "test-integration:arm64" tags: ["arch:amd64"] image: registry.ddbuild.io/images/aws-cli:2.33.12 script: diff --git a/test/integration-test/conftest.py b/test/integration-test/conftest.py index 1be935e..4390031 100644 --- a/test/integration-test/conftest.py +++ b/test/integration-test/conftest.py @@ -94,22 +94,22 @@ def load_configuration(self, conf_path: str) -> bool: # Continue anyway - sometimes apachectl returns non-zero but Apache starts # We'll verify below with HTTP requests - # Wait for Apache to fully start and verify it's running - # Give Apache up to 5 seconds to start + # Probe readiness with a bare TCP connect rather than an HTTP GET — + # mod_datadog's `DatadogTracing Off` inside a is silently + # dropped by the config merge in common_conf.cpp (child Off is ORed + # with parent On), so any HTTP probe would emit a trace and pollute + # the per-test agent session. + import socket max_wait = 5 start_time = time.time() while time.time() - start_time < max_wait: - # Check if Apache is responding by making a simple HTTP request try: - import requests - response = requests.get(self.make_url("/"), timeout=1) - # If we get any response, Apache is running - print(f"[debug] Apache started successfully, responding with status {response.status_code}") - return True - except requests.exceptions.RequestException: - # Apache not ready yet, wait a bit - time.sleep(0.2) + with socket.create_connection((self.host, int(self.port)), timeout=0.5): + print(f"[debug] Apache accepting connections on {self.host}:{self.port}") + return True + except (ConnectionRefusedError, OSError): + time.sleep(0.1) # If we get here, Apache didn't start properly print(f"[error] Apache failed to respond after {max_wait} seconds") @@ -198,19 +198,29 @@ def __init__(self, host: str, port: int) -> None: def internal_run(self) -> None: runner = web.AppRunner(self._app) self.loop.run_until_complete(runner.setup()) - site = web.TCPSite(runner, self.host, self.port) + # shutdown_timeout bounds how long aiohttp will wait for active + # keep-alive connections to close on teardown. The default is 60s; + # Apache workers that got SIGKILL leave sockets lingering and make + # that timeout show up as a 60s hang at end-of-session. + site = web.TCPSite(runner, self.host, self.port, shutdown_timeout=1.0) self.loop.run_until_complete(site.start()) self.loop.run_until_complete(self._stop.wait()) - self.loop.run_until_complete(self._app.cleanup()) + # runner.cleanup() stops sites, closes the TCP listener, and runs the + # app's cleanup handlers. Calling only app.cleanup() leaves the + # listener open and the event loop blocks forever on exit. + self.loop.run_until_complete(runner.cleanup()) self.loop.close() def run(self) -> None: - self._thread = threading.Thread(target=self.internal_run) + # daemon=True so a stuck event loop (e.g. aiohttp refusing to close) + # can't block interpreter shutdown after pytest_sessionfinish. The + # explicit stop() path below is still the intended cleanup. + self._thread = threading.Thread(target=self.internal_run, daemon=True) self._thread.start() def stop(self) -> None: self.loop.call_soon_threadsafe(self._stop.set) - self._thread.join() + self._thread.join(timeout=3.0) def new_session(self, token=None) -> AgentSession: if token is None: diff --git a/test/integration-test/scenarios/conf/proxy.conf b/test/integration-test/scenarios/conf/proxy.conf index 661c8eb..938fa65 100644 --- a/test/integration-test/scenarios/conf/proxy.conf +++ b/test/integration-test/scenarios/conf/proxy.conf @@ -18,8 +18,3 @@ DatadogAddTag foo bar DatadogPropagationStyle datadog ProxyPass "/http" "${upstream_url}" - - - DatadogDelegateSampling On - ProxyPass "${upstream_url}" - diff --git a/test/integration-test/scenarios/test_proxy.py b/test/integration-test/scenarios/test_proxy.py index 569e792..d4ef959 100644 --- a/test/integration-test/scenarios/test_proxy.py +++ b/test/integration-test/scenarios/test_proxy.py @@ -34,11 +34,23 @@ def internal_run(self) -> None: self.loop.close() def run(self) -> None: - self._thread = threading.Thread(target=self.internal_run) + # daemon=True so an early test failure that skips stop() can't keep + # the interpreter alive at pytest shutdown — the __exit__ path below + # is the intended cleanup, this is just a safety net. + self._thread = threading.Thread(target=self.internal_run, daemon=True) self._thread.start() def stop(self) -> None: self.loop.call_soon_threadsafe(self._stop.set) + if self._thread is not None: + self._thread.join(timeout=3.0) + + def __enter__(self) -> "AioHTTPServer": + self.run() + return self + + def __exit__(self, exc_type, exc, tb) -> None: + self.stop() def test_http_proxy(server, agent, log_dir, module_path): @@ -46,7 +58,6 @@ def test_http_proxy(server, agent, log_dir, module_path): Verify proxified HTTP requests propagate tracing context """ - # TODO: support `with` syntax def make_temporary_http_server(host: str, port: int): q = Queue() @@ -62,7 +73,6 @@ async def index(request): host = "127.0.0.1" port = 8081 queue, http_server = make_temporary_http_server(host, port) - http_server.run() config = { "path": relpath("conf/proxy.conf"), @@ -72,20 +82,20 @@ async def index(request): conf_path = os.path.join(log_dir, "httpd.conf") save_configuration(make_configuration(config, log_dir, module_path), conf_path) - assert server.check_configuration(conf_path) - assert server.load_configuration(conf_path) + with http_server: + assert server.check_configuration(conf_path) + assert server.load_configuration(conf_path) - r = requests.get(server.make_url("/http"), timeout=2) - assert r.status_code == 200 + r = requests.get(server.make_url("/http"), timeout=2) + assert r.status_code == 200 - http_server.stop() - server.stop(conf_path) + server.stop(conf_path) - assert not queue.empty() + assert not queue.empty() - upstream_headers = queue.get() - assert "x-datadog-trace-id" in upstream_headers - assert "x-datadog-parent-id" in upstream_headers + upstream_headers = queue.get() + assert "x-datadog-trace-id" in upstream_headers + assert "x-datadog-parent-id" in upstream_headers traces = agent.get_traces(timeout=5) assert len(traces) == 1 From 703e6fe375b041dbd3cd60675476541aa0f7f963 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Sat, 9 May 2026 01:52:31 +0200 Subject: [PATCH 03/11] Rewrite readiness-probe comment to be bug-agnostic (#64) The readiness-probe comment cited a specific bug in the `DatadogTracing Off` scope-merge path to justify using TCP over HTTP. That bug is fixed earlier in this stack, so the comment now reads as if a live bug is still in the tree. Restate the rationale bug-agnostically: any HTTP probe runs through the module's handlers and can emit a trace into the per-test agent session; a bare TCP connect can't. This stays true regardless of future mod_datadog changes. --- test/integration-test/conftest.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/integration-test/conftest.py b/test/integration-test/conftest.py index 4390031..f7fea71 100644 --- a/test/integration-test/conftest.py +++ b/test/integration-test/conftest.py @@ -94,11 +94,11 @@ def load_configuration(self, conf_path: str) -> bool: # Continue anyway - sometimes apachectl returns non-zero but Apache starts # We'll verify below with HTTP requests - # Probe readiness with a bare TCP connect rather than an HTTP GET — - # mod_datadog's `DatadogTracing Off` inside a is silently - # dropped by the config merge in common_conf.cpp (child Off is ORed - # with parent On), so any HTTP probe would emit a trace and pollute - # the per-test agent session. + # Probe readiness with a bare TCP connect rather than an HTTP GET: + # HTTP probes run through mod_datadog's handlers and can emit a + # trace into the per-test agent session, contaminating assertions + # tests make about per-request trace counts. A raw TCP connect + # doesn't reach any request handler, so it stays out of traces. import socket max_wait = 5 start_time = time.time() From 49e7889cd89433343daa7f76c635dc475d77953f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chojnacki?= Date: Sat, 9 May 2026 17:45:41 +0200 Subject: [PATCH 04/11] Simplify CI_IMAGE.md down to the one command that matters --- .github/workflows/CI_IMAGE.md | 46 +++++++---------------------------- 1 file changed, 9 insertions(+), 37 deletions(-) diff --git a/.github/workflows/CI_IMAGE.md b/.github/workflows/CI_IMAGE.md index 4c5b056..46c3493 100644 --- a/.github/workflows/CI_IMAGE.md +++ b/.github/workflows/CI_IMAGE.md @@ -1,44 +1,16 @@ -# CI image used by GitHub Actions +# GitHub Actions CI image -The workflows in this directory pin `image:` to -`datadog/docker-library:httpd-datadog-ci-` — a public Docker Hub -mirror of the internal devcontainer image built by GitLab CI from -[`.devcontainer/Dockerfile`](../../.devcontainer/Dockerfile). The build -+ caching pipeline lives in [`.gitlab/devcontainer.yml`](../../.gitlab/devcontainer.yml); -the image-tag hash is the sha256 of the staged build context defined -by [`.devcontainer/context.files`](../../.devcontainer/context.files). +GitHub-hosted runners can't pull from `registry.ddbuild.io`, so the +workflows here pin `image:` to a public Docker Hub mirror of the +GitLab-built devcontainer image. -GitHub-hosted runners can't pull from `registry.ddbuild.io`, so we -periodically copy the latest amd64 variant to Docker Hub and bump the -`image:` reference in each workflow. - -## Bumping the image - -Once GitLab CI has published a new tag (any change to a path listed in -`.devcontainer/context.files` invalidates the cache and triggers a -rebuild), mirror it from the repo root: +Bump it with: ```sh make mirror-public-image ``` -The target stages the build context, computes the same 12-char hash -GitLab uses, pulls `registry.ddbuild.io/ci/httpd-datadog/devcontainer:amd64-`, -retags it as `datadog/docker-library:httpd-datadog-ci-`, and -pushes. It then prints the exact `image:` value to copy into: - -- `dev.yml` (build + test jobs) -- `release.yml` (build job) -- `system-tests.yml` (build-artifacts job) - -## Why two image namespaces - -| Consumer | Pull from | Why | -|-----------------|----------------------------------------------------------------------------|-----| -| GitLab CI | `registry.ddbuild.io/ci/httpd-datadog/devcontainer:-` | private, fast, nydus-optimized | -| GitHub Actions | `datadog/docker-library:httpd-datadog-ci-` (mirrored from the above) | public, reachable from GitHub-hosted runners | -| Local `make` | Either of the above (pull `registry.ddbuild.io/...` if you have access; build locally otherwise) | dev convenience | - -The two mirrors stay in sync only as far as the most recent -`make mirror-public-image` run — bump the workflows promptly after -running it. +The target pulls the latest amd64 build, retags it under +`datadog/docker-library:httpd-datadog-ci-`, pushes, and prints +the exact `image:` value to paste into `dev.yml`, `release.yml`, and +`system-tests.yml`. From 05fcd11130e038a382bc943373091034ff2d99e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chojnacki?= Date: Sat, 9 May 2026 19:18:20 +0200 Subject: [PATCH 05/11] Drop MAKECMDGOALS workaround for required-paths check dd-repo-tools moved the precondition from a parse-time $(error ...) to a phony prereq wired only on devcontainer-touching targets, so ci-build (which runs its own git submodule update inline) and unrelated targets no longer trip the guard at parse time. The local gate this Makefile carried is no longer needed. --- Makefile | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index 63610c0..608915e 100644 --- a/Makefile +++ b/Makefile @@ -7,16 +7,11 @@ DEV_CONTAINER_IMAGE_NAME := registry.ddbuild.io/ci/httpd-datadog/devcontainer # The devcontainer Dockerfile pulls toolchain files from the # nginx-datadog submodule's build_env/. Without this guard the staging # step silently produces a tree missing those files and the resulting -# tag mismatches what CI computes — fail loudly instead. -# -# Skipped for `ci-build`, which runs its own `git submodule update` -# inline (the GitHub workflows checkout without submodules and init -# them as part of the build). All other targets (dev-image, -# test-integration, mirror-public-image) genuinely need build_env -# already present and keep the guard. -ifneq ($(MAKECMDGOALS),ci-build) +# tag mismatches what CI computes — fail loudly instead. Upstream gates +# this check to devcontainer-touching targets only, so `ci-build` (which +# inits its own submodules in the recipe) and unrelated targets aren't +# blocked at parse time. DEV_CONTAINER_REQUIRED_PATHS := deps/nginx-datadog/build_env/Toolchain.cmake.x86_64 -endif include .devcontainer/devcontainer.mk From d7eb1804b622ae7f54f898b49aa26a133c63e942 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chojnacki?= Date: Mon, 11 May 2026 14:16:59 +0200 Subject: [PATCH 06/11] Address Copilot review on PR #78 - test_proxy.py: AioHTTPServer.internal_run() now calls runner.cleanup() (which stops the TCPSite and runs app cleanup together) instead of app.cleanup() alone, and pins TCPSite shutdown_timeout=1.0. Without this, the listener stayed up past stop() and kept the thread alive, re-introducing the teardown hang the parent #63 commit claimed to fix. - Makefile: quote $(CURDIR) in `git config safe.directory` so a workspace path containing spaces doesn't word-split. --- Makefile | 2 +- test/integration-test/scenarios/test_proxy.py | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 608915e..e2c17ba 100644 --- a/Makefile +++ b/Makefile @@ -27,7 +27,7 @@ test-integration: dev-image PRESET ?= ci-dev .PHONY: ci-build ci-build: - git config --global --add safe.directory $(CURDIR) + git config --global --add safe.directory "$(CURDIR)" git submodule update --init --depth=1 deps/dd-trace-cpp deps/nginx-datadog cmake --preset=$(PRESET) -B build . cmake --build build -j --verbose diff --git a/test/integration-test/scenarios/test_proxy.py b/test/integration-test/scenarios/test_proxy.py index d4ef959..ebb47aa 100644 --- a/test/integration-test/scenarios/test_proxy.py +++ b/test/integration-test/scenarios/test_proxy.py @@ -27,10 +27,13 @@ def __init__(self, app, host, port) -> None: def internal_run(self) -> None: runner = web.AppRunner(self._app) self.loop.run_until_complete(runner.setup()) - site = web.TCPSite(runner, self._host, self._port) + site = web.TCPSite(runner, self._host, self._port, shutdown_timeout=1.0) self.loop.run_until_complete(site.start()) self.loop.run_until_complete(self._stop.wait()) - self.loop.run_until_complete(self._app.cleanup()) + # `runner.cleanup()` stops the site and the app together; calling + # `app.cleanup()` alone leaves the TCPSite listening, which keeps + # the thread alive past stop() and wedges pytest teardown. + self.loop.run_until_complete(runner.cleanup()) self.loop.close() def run(self) -> None: From e6000e0a0b058ccd4d7f1e8154eaf3a85ee50a76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chojnacki?= Date: Mon, 11 May 2026 14:22:16 +0200 Subject: [PATCH 07/11] Trim context.files to the essential 5 lines --- .devcontainer/context.files | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/.devcontainer/context.files b/.devcontainer/context.files index 2f1fcdc..2b945ac 100644 --- a/.devcontainer/context.files +++ b/.devcontainer/context.files @@ -1,21 +1,5 @@ -# Build-context allowlist for the devcontainer image. One rsync-relative -# path per line; '#' comments and blank lines are stripped before rsync -# sees them. Anything the Dockerfile COPYs has to live under one of these. -# -# Only files whose contents affect the image go in this list — anything -# that's only invoked at runtime via volume mount (e.g. -# run-integration-tests.sh) is intentionally excluded so editing it -# doesn't invalidate the cached image tag. - -# image recipe +# rsync --files-from allowlist (anything COPY'd by the Dockerfile). +# Runtime-only files (bind-mounted at `docker run`) stay out of the hash. .devcontainer/Dockerfile - -# musl/LLVM toolchain inputs (CHECKSUMS, locale.h.diff, alltypes.h.diff, -# Toolchain.cmake., glibc_compat.c) — sourced from the -# nginx-datadog submodule's build_env so the cross-compile sysroot here -# matches what nginx-datadog uses. deps/nginx-datadog/build_env/ - -# Downloads + builds httpd 2.4 inside the image so test jobs have an -# Apache to load mod_datadog into without rebuilding it per pipeline. scripts/setup-httpd.py From 27da7fd735263cfd0edfe7cb9a8fabeb6aa25ade Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chojnacki?= Date: Mon, 11 May 2026 14:47:57 +0200 Subject: [PATCH 08/11] Move integration-test runner into the Makefile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit run-integration-tests.sh was a standalone script that only existed to be the body of \`make test-integration\` (and the equivalent CI job). Inline it into the test-integration recipe so there's one source of truth — pytest invocation, RUM-enabled cmake build, UV_PROJECT_ENVIRONMENT hack — and drop the script. MODULE_PATH stays an override: empty (default) builds via \`cmake --preset=ci-release -DHTTPD_DATADOG_ENABLE_RUM=ON …\` into build-container/dist-container; set (CI's case) skips the build and points pytest at a pre-built artifact. .gitlab-ci.yml test-integration template now calls \`make test-integration\` instead of the script. --- .devcontainer/run-integration-tests.sh | 55 -------------------------- .gitlab-ci.yml | 18 ++++----- Makefile | 39 +++++++++++++++++- 3 files changed, 47 insertions(+), 65 deletions(-) delete mode 100755 .devcontainer/run-integration-tests.sh diff --git a/.devcontainer/run-integration-tests.sh b/.devcontainer/run-integration-tests.sh deleted file mode 100755 index cfcd025..0000000 --- a/.devcontainer/run-integration-tests.sh +++ /dev/null @@ -1,55 +0,0 @@ -#!/bin/sh -# Build mod_datadog (with RUM) and run the full integration-test suite. -# Assumes it runs inside the devcontainer image (Alpine + musl sysroot + httpd -# at /httpd/httpd-build/ + uv). -# -# If MODULE_PATH is set in the environment, the cmake build is skipped and -# pytest is pointed at the pre-built module at that path. CI uses this to -# reuse the artifact produced by the build:$ARCH job instead of rebuilding -# from source inside the test job. -set -eux - -REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)" -cd "$REPO_ROOT" - -ARCH="$(uname -m)" - -# Use a container-specific build tree to avoid colliding with host builds -# (host cmake would use Unix Makefiles; the ci-release preset uses Ninja). -BUILD_DIR="$REPO_ROOT/build-container" -DIST_DIR="$REPO_ROOT/dist-container" - -# Repo root is bind-mounted from the host; cmake complains otherwise. -git config --global --add safe.directory "$REPO_ROOT" - -if [ -z "${MODULE_PATH:-}" ]; then - cmake --preset=ci-release \ - -DHTTPD_DATADOG_ENABLE_RUM=ON \ - -DCMAKE_TOOLCHAIN_FILE="/sysroot/${ARCH}-none-linux-musl/Toolchain.cmake" \ - -B "$BUILD_DIR" . - cmake --build "$BUILD_DIR" -j - # Wipe DIST_DIR so a removed/renamed file in the build doesn't - # linger from a previous install and silently get used by tests. - rm -rf "$DIST_DIR" - cmake --install "$BUILD_DIR" --prefix "$DIST_DIR" - MODULE_PATH="$DIST_DIR/lib/mod_datadog.so" -fi - -cd test/integration-test - -# Keep the venv outside the bind-mounted repo so a host-side `uv sync` -# (darwin/arm64 wheels) can't shadow the container's Linux one. -export UV_PROJECT_ENVIRONMENT=/root/.venv-httpd-datadog-tests - -uv sync - -# Pytest's early arg parser runs BEFORE conftest registers `--bin-path`. -# If we pass `--bin-path VALUE` (space-separated), that parser treats the -# value as a positional test path and uses its parent as rootdir, which -# then prevents conftest from loading. The `=` syntax keeps the option and -# its value in a single token and avoids that race. -uv run pytest \ - --bin-path=/httpd/httpd-build/bin/apachectl \ - --module-path="$MODULE_PATH" \ - --log-dir="$REPO_ROOT/logs" \ - -v "$@" diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index b7be329..44a7141 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -53,23 +53,23 @@ stages: .test-rum-template: stage: test - # Same `--flag=VALUE` reasoning as run-integration-tests.sh: pytest's early - # arg parser runs before conftest registers these custom options, so the - # space-separated form gets read as a positional test path and skips - # conftest. + # Same `--flag=VALUE` reasoning as the `make test-integration` recipe: + # pytest's early arg parser runs before conftest registers these custom + # options, so the space-separated form gets read as a positional test + # path and skips conftest. script: - cd $CI_PROJECT_DIR/test/integration-test && uv sync && uv run pytest scenarios --module-path=$CI_PROJECT_DIR/artifacts/$ARCH/mod_datadog.so --bin-path=/httpd/httpd-build/bin/apachectl --log-dir=$CI_PROJECT_DIR/logs -m requires_rum -v -# Runs the full integration-test suite via the same entrypoint local -# developers use (`make test-integration` -> run-integration-tests.sh). -# MODULE_PATH points the script at the build:$ARCH artifact so the cmake -# build step inside the script is skipped and only pytest runs. +# Runs the full integration-test suite via the same `make test-integration` +# entrypoint local developers use. MODULE_PATH points the target at the +# build:$ARCH artifact so the cmake build inside the recipe is skipped +# and only pytest runs. .test-integration-template: stage: test variables: MODULE_PATH: $CI_PROJECT_DIR/artifacts/$ARCH/mod_datadog.so script: - - .devcontainer/run-integration-tests.sh + - make test-integration build:amd64: extends: .build-template diff --git a/Makefile b/Makefile index e2c17ba..7f80ac0 100644 --- a/Makefile +++ b/Makefile @@ -15,9 +15,46 @@ DEV_CONTAINER_REQUIRED_PATHS := deps/nginx-datadog/build_env/Toolchain.cmake.x86 include .devcontainer/devcontainer.mk +# Build mod_datadog (with RUM, against the cross-compile sysroot) inside +# the devcontainer and run the full pytest integration-test suite. +# MODULE_PATH= skips the cmake build and points pytest at a pre-built +# artifact — the .gitlab-ci.yml test-integration:* jobs use this to reuse +# the build:* artifact instead of recompiling per test job. +# +# Pytest options use `--flag=VALUE`: pytest's early arg parser runs before +# conftest registers `--bin-path`/`--module-path`, so the space-separated +# form gets read as a positional test path and silently skips conftest. +# +# UV_PROJECT_ENVIRONMENT points outside the bind-mounted repo so a +# host-side `uv sync` (darwin/arm64 wheels) can't shadow the container's +# Linux venv. +MODULE_PATH ?= + .PHONY: test-integration test-integration: dev-image - $(IN_DEVCONTAINER) .devcontainer/run-integration-tests.sh + $(IN_DEVCONTAINER) sh -ec '\ + arch=$$(uname -m); \ + git config --global --add safe.directory "$$PWD"; \ + module_path="$(MODULE_PATH)"; \ + if [ -z "$$module_path" ]; then \ + cmake --preset=ci-release \ + -DHTTPD_DATADOG_ENABLE_RUM=ON \ + -DCMAKE_TOOLCHAIN_FILE=/sysroot/$${arch}-none-linux-musl/Toolchain.cmake \ + -B build-container .; \ + cmake --build build-container -j; \ + rm -rf dist-container; \ + cmake --install build-container --prefix dist-container; \ + module_path="$$PWD/dist-container/lib/mod_datadog.so"; \ + fi; \ + repo_root=$$PWD; \ + cd test/integration-test; \ + export UV_PROJECT_ENVIRONMENT=/root/.venv-httpd-datadog-tests; \ + uv sync; \ + uv run pytest \ + --bin-path=/httpd/httpd-build/bin/apachectl \ + --module-path="$$module_path" \ + --log-dir="$$repo_root/logs" \ + -v' # One-shot CI build: trust the workdir, init the submodules cmake # actually consumes, configure/build/install. Used by every job in From 59eb7205a139385430888d9229dd5c69354b0db8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chojnacki?= Date: Mon, 11 May 2026 15:09:09 +0200 Subject: [PATCH 09/11] Derive ARCH from `uname -m` instead of a build-arg default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit devcontainer.json can't auto-detect host arch — the previous `"ARCH": "${localEnv:ARCH:x86_64}"` defaulted to x86_64 on arm64 hosts, mismatching docker buildx's host-platform pick. `devcontainer up` then died at `mv /usr/lib/gcc/x86_64-alpine-linux-musl/…` on Apple Silicon (Copilot review #2 from PR #78). Workaround was `export ARCH=aarch64` before opening the container; this commit removes the need for it. The Dockerfile now derives ARCH from `uname -m` inside each RUN that needs it — uname reflects the buildx target platform, so the value is always correct regardless of how the image is invoked. `ARG ARCH` stays declared (as an override) so the dd-repo-tools template's auto-detected build-arg keeps working. Both Toolchain.cmake variants are now staged and the RUN picks the right one (COPY can't take a shell-derived source). devcontainer.json drops `build.args.ARCH` entirely — no more env-var dance for the IDE flow. Verified end-to-end with no `ARCH` env set: - `make test-integration`: 28 passed, 1 skipped in 10.15s - `devcontainer up` + `devcontainer exec . make test-integration`: 28 passed, 1 skipped in 10.12s --- .devcontainer/Dockerfile | 46 ++++++++++++++++++++++----------- .devcontainer/devcontainer.json | 5 +--- 2 files changed, 32 insertions(+), 19 deletions(-) diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index 475bc72..ad4fe28 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -7,8 +7,16 @@ # - Rust toolchain for RUM (inject-browser-sdk requires Rust 1.73+) # - uv (fast Python package manager for tests) # -# Toolchain files are sourced from deps/nginx-datadog/build_env/ +# Toolchain files are sourced from deps/nginx-datadog/build_env/. # +# Arch is auto-derived from `uname -m` inside each RUN (matches the +# buildx --platform the container runs on), so callers don't have to +# pass --build-arg ARCH=. The VSCode "Reopen in Container" flow can't +# auto-detect host arch from devcontainer.json, so relying on uname +# keeps it correct without env-var gymnastics. `ARG ARCH` stays declared +# (as an override) so external callers that already pass it — e.g. the +# dd-repo-tools template's auto-detected build-arg — don't trigger +# "ARG not declared" warnings. FROM alpine:3.20.3 AS sysroot ARG LLVM_VERSION=17.0.6 @@ -16,8 +24,6 @@ ARG ARCH COPY deps/nginx-datadog/build_env/CHECKSUMS /CHECKSUMS -RUN echo "Building LLVM ${LLVM_VERSION} on ${ARCH}" - RUN apk --no-cache add alpine-sdk coreutils sudo bash samurai python3 linux-headers \ compiler-rt clang llvm lld wget cmake make binutils musl-dev git patchelf xz lit RUN wget https://github.com/llvm/llvm-project/releases/download/llvmorg-${LLVM_VERSION}/llvm-project-${LLVM_VERSION}.src.tar.xz && \ @@ -60,22 +66,32 @@ RUN cd /usr/lib && ln -s clang/${LLVM_VERSION%%.*}/lib/linux/libclang_rt.builtin RUN rm -rf /llvm-project-${LLVM_VERSION}.src RUN rm -f llvm-project-${LLVM_VERSION}.src.tar.xz -RUN mkdir -p /sysroot/${ARCH}-none-linux-musl/usr -RUN ln -s /usr/lib /sysroot/${ARCH}-none-linux-musl/usr/ -RUN ln -s /usr/include /sysroot/${ARCH}-none-linux-musl/usr/ -RUN ln -s /lib /sysroot/${ARCH}-none-linux-musl/ -RUN ln -s /usr/lib/llvm${LLVM_VERSION%%.*}/lib/clang/${LLVM_VERSION%%.*}/lib /sysroot/${ARCH}-none-linux-musl/usr/lib/resource_dir/lib - -COPY deps/nginx-datadog/build_env/Toolchain.cmake.${ARCH} /sysroot/${ARCH}-none-linux-musl/Toolchain.cmake +RUN arch=${ARCH:-$(uname -m)} && \ + mkdir -p /sysroot/${arch}-none-linux-musl/usr && \ + ln -s /usr/lib /sysroot/${arch}-none-linux-musl/usr/ && \ + ln -s /usr/include /sysroot/${arch}-none-linux-musl/usr/ && \ + ln -s /lib /sysroot/${arch}-none-linux-musl/ && \ + ln -s /usr/lib/llvm${LLVM_VERSION%%.*}/lib/clang/${LLVM_VERSION%%.*}/lib /sysroot/${arch}-none-linux-musl/usr/lib/resource_dir/lib + +# Both Toolchain.cmake variants are staged; the RUN below picks the one +# matching the build platform. (COPY can't use a shell-derived path, so +# we copy both and select at RUN-time instead.) +COPY deps/nginx-datadog/build_env/Toolchain.cmake.x86_64 /tmp/Toolchain.cmake.x86_64 +COPY deps/nginx-datadog/build_env/Toolchain.cmake.aarch64 /tmp/Toolchain.cmake.aarch64 +RUN arch=${ARCH:-$(uname -m)} && \ + cp /tmp/Toolchain.cmake.${arch} /sysroot/${arch}-none-linux-musl/Toolchain.cmake && \ + rm /tmp/Toolchain.cmake.x86_64 /tmp/Toolchain.cmake.aarch64 # see https://github.com/llvm/llvm-project/issues/60572 -RUN mv /usr/lib/gcc/${ARCH}-alpine-linux-musl/13.2.1/include/stdatomic.h /usr/lib/gcc/${ARCH}-alpine-linux-musl/13.2.1/include/stdatomic.h_ -RUN cp /usr/lib/llvm${LLVM_VERSION%%.*}/lib/clang/${LLVM_VERSION%%.*}/include/stdatomic.h /usr/lib/gcc/${ARCH}-alpine-linux-musl/13.2.1/include/stdatomic.h +RUN arch=${ARCH:-$(uname -m)} && \ + mv /usr/lib/gcc/${arch}-alpine-linux-musl/13.2.1/include/stdatomic.h /usr/lib/gcc/${arch}-alpine-linux-musl/13.2.1/include/stdatomic.h_ && \ + cp /usr/lib/llvm${LLVM_VERSION%%.*}/lib/clang/${LLVM_VERSION%%.*}/include/stdatomic.h /usr/lib/gcc/${arch}-alpine-linux-musl/13.2.1/include/stdatomic.h COPY deps/nginx-datadog/build_env/glibc_compat.c /sysroot/ -RUN clang --sysroot /sysroot/${ARCH}-none-linux-musl/ -fpie -O2 -fno-omit-frame-pointer \ - -ggdb3 -c /sysroot/glibc_compat.c -o /tmp/glibc_compat.o && \ - ar rcs /sysroot/${ARCH}-none-linux-musl/usr/lib/libglibc_compat.a /tmp/glibc_compat.o && \ +RUN arch=${ARCH:-$(uname -m)} && \ + clang --sysroot /sysroot/${arch}-none-linux-musl/ -fpie -O2 -fno-omit-frame-pointer \ + -ggdb3 -c /sysroot/glibc_compat.c -o /tmp/glibc_compat.o && \ + ar rcs /sysroot/${arch}-none-linux-musl/usr/lib/libglibc_compat.a /tmp/glibc_compat.o && \ rm /tmp/glibc_compat.o # Install dependencies for httpd diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 8b9cf16..e8eb326 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -3,9 +3,6 @@ "initializeCommand": "make -f .devcontainer/devcontainer.mk .devcontainer-stage-context", "build": { "dockerfile": ".staged/.devcontainer/Dockerfile", - "context": ".staged", - "args": { - "ARCH": "${localEnv:ARCH:x86_64}" - } + "context": ".staged" } } From 0d30d50f63203449f6a788074d3141c9ab5cc739 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chojnacki?= Date: Wed, 13 May 2026 23:27:55 +0200 Subject: [PATCH 10/11] test/integration-test: strip CI runner DD_* vars GitLab runners set DD_SERVICE=gitlab-runner (and likely DD_ENV/ DD_VERSION) on their job environment to tag the runner's own traces. The pytest subprocess that runs apachectl inherits these, so mod_datadog sees them and overrides the per-test `dd.service` directives that test_default_configurations / test_directive_ configurations assert on. Pop the three vars at conftest import so the in-config values win. --- test/integration-test/conftest.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/integration-test/conftest.py b/test/integration-test/conftest.py index f7fea71..bb38bef 100644 --- a/test/integration-test/conftest.py +++ b/test/integration-test/conftest.py @@ -24,6 +24,11 @@ CWD = os.path.dirname(__file__) +# CI runners pre-set DD_* on the env to tag their own traces; strip so +# the per-test `dd.service` directive in httpd.conf wins. +for _dd_var in ("DD_SERVICE", "DD_ENV", "DD_VERSION"): + os.environ.pop(_dd_var, None) + class DockerProc: def __init__(self, container_name: str) -> None: From 30fa4bf47acc94c33fffaeef8f4899c2636ca16b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chojnacki?= Date: Wed, 13 May 2026 23:39:31 +0200 Subject: [PATCH 11/11] Address xlamorlette review nits on PR #78 - Makefile test-integration: $$HOME instead of hardcoded /root for the uv venv path. Inside the current image $HOME=/root so equivalent today, but adapts if the image ever runs as non-root. - CI_IMAGE.md: spell out that `make mirror-public-image` is run from `main` after a .devcontainer/ change lands. The pipeline publishes the new tag; this target just retags to Docker Hub. --- .github/workflows/CI_IMAGE.md | 4 +++- Makefile | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/CI_IMAGE.md b/.github/workflows/CI_IMAGE.md index 46c3493..4f98b2d 100644 --- a/.github/workflows/CI_IMAGE.md +++ b/.github/workflows/CI_IMAGE.md @@ -4,7 +4,9 @@ GitHub-hosted runners can't pull from `registry.ddbuild.io`, so the workflows here pin `image:` to a public Docker Hub mirror of the GitLab-built devcontainer image. -Bump it with: +Run this **from `main`** after a `.devcontainer/` change has landed +(the GitLab pipeline publishes the new tag; this just retags it to +Docker Hub and updates the workflow `image:` pins): ```sh make mirror-public-image diff --git a/Makefile b/Makefile index 7f80ac0..6686d7a 100644 --- a/Makefile +++ b/Makefile @@ -48,7 +48,7 @@ test-integration: dev-image fi; \ repo_root=$$PWD; \ cd test/integration-test; \ - export UV_PROJECT_ENVIRONMENT=/root/.venv-httpd-datadog-tests; \ + export UV_PROJECT_ENVIRONMENT=$$HOME/.venv-httpd-datadog-tests; \ uv sync; \ uv run pytest \ --bin-path=/httpd/httpd-build/bin/apachectl \