diff --git a/.gitlab/Dockerfile b/.devcontainer/Dockerfile similarity index 64% rename from .gitlab/Dockerfile rename to .devcontainer/Dockerfile index 475bc72f..ad4fe282 100644 --- a/.gitlab/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/context.files b/.devcontainer/context.files new file mode 100644 index 00000000..2b945ac0 --- /dev/null +++ b/.devcontainer/context.files @@ -0,0 +1,5 @@ +# 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 +deps/nginx-datadog/build_env/ +scripts/setup-httpd.py diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json new file mode 100644 index 00000000..e8eb326b --- /dev/null +++ b/.devcontainer/devcontainer.json @@ -0,0 +1,8 @@ +{ + "name": "httpd-datadog", + "initializeCommand": "make -f .devcontainer/devcontainer.mk .devcontainer-stage-context", + "build": { + "dockerfile": ".staged/.devcontainer/Dockerfile", + "context": ".staged" + } +} diff --git a/.github/workflows/CI_IMAGE.md b/.github/workflows/CI_IMAGE.md new file mode 100644 index 00000000..4f98b2db --- /dev/null +++ b/.github/workflows/CI_IMAGE.md @@ -0,0 +1,18 @@ +# GitHub Actions CI image + +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. + +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 +``` + +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`. diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 8fcd549e..a551fa50 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 7a7539b1..6514dd73 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 72414eec..dbde7728 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 75feb22b..cb252be8 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 4a328df2..44a7141c 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,27 @@ stages: .test-rum-template: stage: test - image: $CI_DOCKER_IMAGE_BASE:$CI_IMAGE_HASH + # 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 - -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 + - 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 -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 +# 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: - DDSIGN_SKIP_SIGNING: "true" + MODULE_PATH: $CI_PROJECT_DIR/artifacts/$ARCH/mod_datadog.so 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 + - make test-integration 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 +81,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 +89,48 @@ 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"] + +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"] @@ -175,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/.gitmodules b/.gitmodules index 52a8a84d..52183b5c 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 43604958..6686d7a5 100644 --- a/Makefile +++ b/Makefile @@ -1,17 +1,92 @@ -# 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. 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 + +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. # -# 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. +# 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) 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=$$HOME/.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' -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" diff --git a/test/integration-test/conftest.py b/test/integration-test/conftest.py index 1be935e2..bb38befb 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: @@ -94,22 +99,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: + # 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() 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 +203,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 661c8eb4..938fa65d 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 569e7923..ebb47aa5 100644 --- a/test/integration-test/scenarios/test_proxy.py +++ b/test/integration-test/scenarios/test_proxy.py @@ -27,18 +27,33 @@ 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: - 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 +61,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 +76,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 +85,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