Skip to content

docs(claude): fix stale CI and profiler notes #4243

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Closed
morrisonlevi wants to merge 1 commit into from
+32 −27
Closed

docs(claude): fix stale CI and profiler notes #4243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .claude/ci/appsec-gradle-integration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,8 @@ If you need to inspect sidecar/helper or PHP issues:
- The `test` task itself is disabled (`tasks['test'].enabled = false`). Use versioned tasks like `test8.3-debug`.
- Docker images are pulled from `docker.io/datadog/dd-appsec-php-ci`. Without `-PfloatingImageTags`, images are resolved by SHA256 digest from `gradle/tag_mappings.gradle`. If a digest is not locally available, Docker will pull it.
- `buildPortableLibdatadogPhp` uses the
`nginx-fpm-php-8.5-release-musl` image with nightly Rust. The image must
`nginx-fpm-php-8.5-release-musl` image with stable Rust (`compile_rust.sh`
sets `RUSTC_BOOTSTRAP=1` for `-Zbuild-std`). The image must
Comment on lines 275 to +277

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Name the image actually used by the portable build task

Correct the image named here: buildPortableLibdatadogPhp sets imageTag: 'php-buildonly-rust' in appsec/tests/integration/build.gradle, while nginx-fpm-php-8.5-release-musl belongs to a separate runtime-image task. As written, someone troubleshooting a missing portable-build image will pull or inspect the wrong image and still be unable to run this Gradle task.

Useful? React with 👍 / 👎.

be available locally or pullable.
- On first run, Gradle downloads its wrapper, dependencies, and Docker images. Expect 5-10 minutes. Subsequent runs with warm caches take ~20-50 seconds for a single test.
- **c-ares DNS failure in Alpine containers.** Alpine's `curl` and `git` use
Expand Down
26 changes: 15 additions & 11 deletions .claude/ci/github-actions-profiler.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@
|--------|--------|-------------|
| `Profiling correctness / prof-correctness ({ver}, nts)` | `ubuntu-24.04` | Builds profiler + runs NTS correctness test cases |
| `Profiling correctness / prof-correctness ({ver}, zts)` | `ubuntu-24.04` | Same + `exceptions_zts` (requires `parallel` PECL extension) |
| `Profiling ASAN Tests / prof-asan ({ver}, {arch})` | `arm-8core-linux` / `ubuntu-8-core-latest` | Builds profiler with ASAN + runs `.phpt` profiling tests |
| `Profiling ASAN/UBSAN Tests / PHP 8.5 {nts,zts} UBSAN ({arch})` | `arm-8core-linux` / `ubuntu-8-core-latest` | Builds profiler with UBSAN + runs `.phpt` profiling tests |

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restore the ASAN job row

Keep a separate ASAN entry when adding UBSAN. The referenced workflow still defines prof-asan for PHP 8.3–8.5 across nts-asan and debug-zts-asan, but this replacement row documents only the PHP 8.5 UBSAN job, leaving the CI Jobs table incomplete even though the ASAN matrix immediately below remains documented.

Useful? React with 👍 / 👎.


Correctness matrix: PHP 8.0+ × {nts, zts}.
ASAN matrix: PHP 8.3+ × {arm64, amd64}.
ASAN matrix: PHP 8.3+ × {nts-asan, debug-zts-asan} × {arm64, amd64}.
UBSAN matrix: PHP 8.5 × {nts, zts} × {arm64, amd64}.

## What It Tests

Expand All @@ -36,7 +37,8 @@ ZTS adds: `exceptions_zts`.

Use `.claude/ci/dockerh` with the `datadog/dd-trace-ci:php-<VERSION>_bookworm-{N}` image
matching the PHP version under test (see `index.md` for image version and contents). The CI
uses clang-19 on ubuntu-24.04; clang-17 in the image works fine.
uses clang-20 (`LLVM_VERSION` in `prof_correctness.yml`) on ubuntu-24.04; clang-21 in the
image works fine.

Actions jobs use `shivammathur/setup-php` instead, but the same `dd-trace-ci`
image is a suitable local substitute.
Expand All @@ -63,13 +65,13 @@ overlaid `/project/dd-trace-php/target` fixes it.

```bash
# NTS example (PHP 8.3)
dockerh --cache profiler-8.3-nts --php nts datadog/dd-trace-ci:php-8.3_bookworm-6 -- bash -c '
dockerh --cache profiler-8.3-nts --php nts datadog/dd-trace-ci:php-8.3_bookworm-11 -- bash -c '
export CARGO_TARGET_DIR=/project/dd-trace-php/target
cd profiling && cargo rustc --features=trigger_time_sample --profile profiler-release --crate-type=cdylib
'

# ZTS example (PHP 8.1) — note --php zts, matching image version, and separate cache name
dockerh --cache profiler-8.1-zts --php zts datadog/dd-trace-ci:php-8.1_bookworm-6 -- bash -c '
dockerh --cache profiler-8.1-zts --php zts datadog/dd-trace-ci:php-8.1_bookworm-11 -- bash -c '
export CARGO_TARGET_DIR=/project/dd-trace-php/target
cd profiling && cargo rustc --features=trigger_time_sample --profile profiler-release --crate-type=cdylib
'
Expand All @@ -88,7 +90,7 @@ write pprof output there — no extra mounts needed:

```bash
dockerh --cache profiler-8.3-nts --php nts \
datadog/dd-trace-ci:php-8.3_bookworm-6 -- bash -c '
datadog/dd-trace-ci:php-8.3_bookworm-11 -- bash -c '
export CARGO_TARGET_DIR=/project/dd-trace-php/target
export DD_PROFILING_LOG_LEVEL=warn # use "trace" only when debugging — trace is verbose and slows execution
export DD_PROFILING_EXPERIMENTAL_FEATURES_ENABLED=1
Expand Down Expand Up @@ -134,7 +136,7 @@ The pprof files are zstd-compressed protobuf. Use `go tool pprof` (available in
dd-trace-ci image) to inspect them. Pass `--user root` so `apt-get install` works:

```bash
dockerh --cache profiler-8.3-nts --php nts datadog/dd-trace-ci:php-7.3_bookworm-6 --user root -- bash -c '
dockerh --cache profiler-8.3-nts --php nts datadog/dd-trace-ci:php-7.3_bookworm-11 --user root -- bash -c '
apt-get update -qq > /dev/null 2>&1 && apt-get install -y -qq zstd > /dev/null 2>&1

PPROF_DIR=/project/dd-trace-php/tmp/correctness/allocations
Expand Down Expand Up @@ -231,7 +233,7 @@ and clang-17, then runs the `.phpt` test suite with `--asan`.

```bash
dockerh --cache profiler-asan-8.3-nts --php nts-asan \
datadog/dd-trace-ci:php-8.3_bookworm-6 --user root --privileged -- bash -c '
datadog/dd-trace-ci:php-8.3_bookworm-11 --user root --privileged -- bash -c '
Comment on lines 235 to +236

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Match the ASAN commands to the bookworm-11 toolchain

After switching this example to bookworm-11, the command below still exports CC=clang-17 and RUSTC_LINKER=lld-17. The image's Dockerfile installs only clang/lld 21, and the corresponding ASAN workflow also uses version 21, so this local reproduction stops with a missing compiler or linker before building. Update the hard-coded toolchain values to 21 along with the image tag.

Useful? React with 👍 / 👎.

export CARGO_TARGET_DIR=/project/dd-trace-php/target
export CC=clang-17
export CFLAGS="-fsanitize=address -fno-omit-frame-pointer"
Expand Down Expand Up @@ -266,10 +268,12 @@ the workflow file for the current pinned version.

## Gotchas

- **Expected ASAN test counts:** 39 total, ~27 pass, ~12 skip (30%), 0 fail. The skips are normal
- **Expected test counts (PHP 8.5):** ASAN 47 total, 32 pass, 15 skip, 0 fail; UBSAN nts
47 total, 35 pass, 12 skip, 0 fail. The skips are normal
(platform/env conditions). A non-zero fail count indicates a real problem.
Comment on lines +271 to 273

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Document separate ASAN counts for NTS and ZTS

Qualify this ASAN count by build type and add the other matrix result. The workflow runs both nts-asan and debug-zts-asan, and their skip totals necessarily differ: zts_01.phpt runs only under ZTS, while exceptions_zts_01.phpt also runs there because the bookworm ZTS image installs ext-parallel; both skip under NTS. A single unqualified 32 pass, 15 skip expectation therefore misclassifies normal output for one of the ASAN variants.

Useful? React with 👍 / 👎.

- The `profiler-release` profile is defined in the workspace root `Cargo.toml`, not in
`profiling/Cargo.toml`. It inherits from `release` with `panic = "abort"`.
- The profiler is built from the root `datadog-php` crate (`Cargo.toml`, `--features profiling`);
there is no `profiling/Cargo.toml`. The `profiler-release` profile is defined there too and
inherits from `release` with `panic = "abort"`.
Comment on lines +274 to +276

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Use the profiler-only flags in reproduction commands

Complete the root-crate migration in the reproduction commands: the direct Cargo examples still omit --no-default-features, so the root crate's default tracer feature remains enabled and standalone_profiler is not selected. They also try to load/copy libdatadog_php_profiling.so, while the consolidated crate produces libdatadog_php.so (as reflected by config.m4). Consequently, users following the NTS, ZTS, debug, or ASAN examples cannot use the documented output path; update the commands to build profiling,trigger_time_sample without defaults and adjust the artifact paths.

Useful? React with 👍 / 👎.

- `dockerh` runs the container as your host UID so cache dirs are writable without any
permission tricks. Pass `--user root` after the image name if you need to install
packages with `apt-get`.
Expand Down
14 changes: 7 additions & 7 deletions .claude/ci/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ CI images are tagged `datadog/dd-trace-ci:php-{version}_bookworm-{N}` where `N`
is an iteration number shared across all GitLab appsec jobs. Find the current
value by searching for `bookworm-` in `.gitlab/generate-appsec.php`
.
The `php-8.3_bookworm-{N}` image contains: Rust (see
`profiling/rust-toolchain.toml` for the pinned version), clang-17, Go, and
The `php-8.3_bookworm-{N}` image contains: Rust (see `rust-toolchain.toml`
for the pinned version), clang-21, Go, and
multiple PHP builds under `/opt/php/` (nts, zts, debug, etc.). Use `--php nts`
(or another variant) with `dockerh` to select the right build — see the `--php`
section above.
Expand All @@ -122,7 +122,7 @@ in CI scripts are mirrors of `datadog/dd-trace-ci:TAG` on Docker Hub. Pull them
directly without authentication — no registry login or image export/import needed:

```bash
docker pull datadog/dd-trace-ci:php-8.3_bookworm-6
docker pull datadog/dd-trace-ci:php-8.3_bookworm-11
```

(The exception is registry.ddbuild.io/images/mirror/b1o7r7e0/nginx_musl_toolchain,
Expand Down Expand Up @@ -255,7 +255,7 @@ this file instead of duplicating build commands.
### Group A — Native Linux unit and extension tests

Runner: `arch:amd64` + `arch:arm64`
Image: `datadog/dd-trace-ci:php-{version}_bookworm-6`
Image: `datadog/dd-trace-ci:php-{version}_bookworm-11`
No Docker daemon — tests run directly in the container.

→ **[appsec-native-tests.md](appsec-native-tests.md)**
Expand All @@ -276,7 +276,7 @@ Covers: `Unit tests`, `PHP Language Tests`, `test_c`, `ASAN test_c`, `Opcache te
### Group B — Native Linux web framework tests

Runner: `arch:amd64`
Image: `datadog/dd-trace-ci:php-{version}_bookworm-6`
Image: `datadog/dd-trace-ci:php-{version}_bookworm-11`
GitLab service containers: test-agent, httpbin, request-replayer

→ **[tracer-web-tests.md](tracer-web-tests.md)**
Expand All @@ -291,7 +291,7 @@ Covers: `test_web_laravel_*`, `test_web_symfony_*`, `test_web_wordpress_*`,
### Group C — Native Linux service integration tests

Runner: `arch:amd64`
Image: `datadog/dd-trace-ci:php-{version}_bookworm-6`
Image: `datadog/dd-trace-ci:php-{version}_bookworm-11`
GitLab service containers: MySQL, Redis, Kafka, Elasticsearch, MongoDB, etc.

→ **[tracer-integration-tests.md](tracer-integration-tests.md)**
Expand All @@ -310,7 +310,7 @@ Covers: `test_integrations_amqp*`, `test_integrations_curl`, `test_integrations_
### Group D — Native Linux compile / artifact build

Runner: `arch:amd64` + `arch:arm64`
Image: `datadog/dd-trace-ci:php-{version}_bookworm-6`
Image: `datadog/dd-trace-ci:php-{version}_bookworm-11`
Produces `.so` artifacts consumed by Groups B, C, H.

→ **[compile-artifacts.md](compile-artifacts.md)**
Expand Down
2 changes: 1 addition & 1 deletion .claude/project/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ request crashes and is shared across language tracers. See
`compile_rust.sh` (invoked from the Makefile) drives it. Minimize FFI surface;
headers are generated with cbindgen (see [components.md](components.md)).
Toolchain is pinned — see `Cargo.toml` (`rust-version`) and
`profiling/rust-toolchain.toml`, not a hardcoded version.
`rust-toolchain.toml`, not a hardcoded version.
Comment on lines 87 to +88

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep the scoped profiler toolchain reference

Do not replace the profiler toolchain pointer with only the root file: profiling/rust-toolchain.toml still exists, and Rustup selects it rather than the root rust-toolchain.toml whenever commands run from profiling/, as the profiler reproduction guide instructs. The two pins happen to match now, but this documentation directs maintainers to the wrong controlling file and will become incorrect as soon as either pin changes.

Useful? React with 👍 / 👎.


## Configuration & INI

Expand Down
14 changes: 7 additions & 7 deletions .claude/project/profiling.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ agent.

## Key files & dirs

- `profiling/Cargo.toml` — `cdylib`; depends on `libdd-profiling`/`alloc`/
`common` from libdatadog.
- `profiling/rust-toolchain.toml` — pins Rust (distinct from the workspace
toolchain).
- There is no separate profiler crate: it is built from the root
`datadog-php` crate (`Cargo.toml`) with `--no-default-features --features
profiling`, which pulls in `libdd-profiling`/`libdd-alloc` from libdatadog.
`components-rs/lib.rs` includes `profiling/src/lib.rs` as the `profiling`
module, and `components-rs/build.rs` includes `profiling/build.rs`.
- `profiling/src/lib.rs` — module entry: `minit`/`rinit`/`prshutdown`, Zend
interrupt registration.
- `profiling/src/profiling/` — `mod.rs` (`Profiler` + `SampleValues`),
Expand All @@ -33,12 +34,11 @@ pprof via a background thread; `prshutdown` flushes.
Builds independently from the tracer; depends on libdatadog for pprof. The
main tracer looks up `ddog_php_prof_interrupt_function` by symbol to call on
interrupt (see [tracer.md](tracer.md)). Not sidecar-dependent — it uploads
directly (contrast with [sidecar.md](sidecar.md)). `build.rs` reads
`../VERSION`.
directly (contrast with [sidecar.md](sidecar.md)). `profiling/build.rs` reads
the repo-root `VERSION`.

## Gotchas

- Pinned `rust-toolchain.toml`, not the workspace default.
- `CARGO_TARGET_DIR` must be set (the Makefile uses `tmp/build_profiler`).
- `cdylib` is release-only — phpt tests fail on debug builds.
- Allocation hook differs: PHP 8.4+ vs ≤8.3 use different modules.
Expand Down
Loading