Repository navigation
Code coverage for Java Profiler - #858
Conversation
CI Test ResultsRun: #40558283618 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Summary: Total: 32 | Passed: 32 | Failed: 0 Updated: 2026-10-27 18:49:20 UTC |
…rage tasks - Package the `coverage` native library jar and consumable configuration when -PenableCoverage is set, so :ddprof-test:testCoverage can resolve it (still excluded from publishing). - gtestCoverageReport now also depends on :ddprof-test:testCoverage and merges the gtest and Java-test .gcda data into one gcovr report; negative hit counts are warned about instead of failing the report. - Build the coverage config with -fprofile-update=atomic so concurrent profiler threads don't corrupt gcov counters, and link it on Linux with -Wl,-z,nodelete like the debug config. - Clear stale .gcda files before each gtestCoverage_<test> and testCoverage run, and derive the gcov tool from the configured compiler instead of the gcov on PATH. - Enable JaCoCo on testDebug/testSlowDebug only when jacocoTestReport is requested (drops -PdisableJacoco). When a coverage report is requested, test failures are deferred rather than ignored: the report is generated, then the build fails listing the failed test tasks. - Add NativeBuildConfig so debug-only tests also run against the coverage build. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Build the coverage config on configureDebug and add only the
--coverage flags, so it measures the same binary as testDebug.
- Add a BuildConfiguration.publishable flag (false for coverage) and
filter packaging/publishing on it instead of the "coverage" name.
- Pick Apple's llvm-cov on macOS only when the compiler is Apple clang,
so a Homebrew g++-<N> gets its matching gcov-<N>.
- Always rerun testCoverage/testSlowCoverage, since cleanLibCoverageData
always clears the .gcda counts (the musl Exec tasks could otherwise be
UP-TO-DATE).
- Gate the coverage test tasks and cleanLibCoverageData on
:ddprof-lib:checkCoverageTools.
- Point jacocoTestReport at the known .exec files instead of walking the
build directory, and use tasks.named { } instead of tasks.matching { }.
- Treat the coverage build as a debug build in LazyTraceIdJfrValidationTest,
ReferenceChainAssertions and BaseContextWallClockTest.
- Install llvm in the Ubuntu test image for the unversioned llvm-cov.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7cc60dd1f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![datadog-prod-us1-6[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
Ordinary test tasks keep JaCoCo disabled, avoiding instrumentation changes to profiler assertions. No actionable defects were identified; runtime generation of the C++ and Java coverage reports remains unverified.
🤖 Bits Code Review · Commit b7cc60d
There was a problem hiding this comment.
🟡 Changes recommended
Preflight dependency checks and report failure deferral do not consistently respect the tasks actually scheduled.
2 open issues
What changed in this PR
Adds opt-in native and Java coverage reporting to the profiler’s build and test infrastructure, without publishing coverage artifacts.
Changes:
- Adds a coverage build configuration, tool preflight checks, and combined gcovr/JaCoCo reports.
- Instruments coverage test JVMs and child processes, with deferred test-failure reporting.
- Updates test build detection and container tooling for coverage runs.
| File | Description |
|---|---|
| utils/run-containers-tests.sh | Installs coverage tools in test containers. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/BaseContextWallClockTest.java | Includes coverage builds in distribution assertions. |
| ddprof-test/src/test/java/com/datadoghq/profiler/UnwindFailuresTest.java | Recognizes coverage as DEBUG-instrumented. |
| ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainAssertions.java | Uses shared build detection. |
| ddprof-test/src/test/java/com/datadoghq/profiler/NativeBuildConfig.java | Centralizes native build classification. |
| ddprof-test/src/test/java/com/datadoghq/profiler/JVMAccessTest.java | Enables debug-gated tests for coverage. |
| ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerTest.java | Enables debug-gated tests for coverage. |
| ddprof-test/src/test/java/com/datadoghq/profiler/cpu/LazyTraceIdJfrValidationTest.java | Allows validation in coverage builds. |
| ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProfilerTest.java | Applies debug-build handling to coverage. |
| ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProcessProfilerTest.java | Forwards JaCoCo instrumentation to child JVMs. |
| ddprof-test/build.gradle.kts | Configures Java coverage collection and reporting. |
| ddprof-lib/build.gradle.kts | Adds preflight checks, combined reports, and publication filtering. |
| build-logic/conventions/src/main/kotlin/com/datadoghq/profiler/ProfilerTestPlugin.kt | Supports additional JVM arguments for musl tests. |
| build-logic/conventions/src/main/kotlin/com/datadoghq/native/util/PlatformUtils.kt | Shares Google Test availability detection. |
| build-logic/conventions/src/main/kotlin/com/datadoghq/native/model/BuildConfiguration.kt | Adds configuration publishability control. |
| build-logic/conventions/src/main/kotlin/com/datadoghq/native/gtest/GtestPlugin.kt | Reuses shared Google Test detection. |
| build-logic/conventions/src/main/kotlin/com/datadoghq/native/config/ConfigurationPresets.kt | Defines the opt-in coverage preset. |
Review effort: Balanced
# Conflicts: # ddprof-lib/build.gradle.kts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e7918bd86
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
rkennke
left a comment
rkennke
left a comment
There was a problem hiding this comment.
Nice, carefully commented work. Coverage is truly opt-in (inactive config, not publishable, JaCoCo disabled for every non-coverage test task), and the atomic counter flag, gcov/compiler matching and up-front tool check are all good decisions. Inline comments are mostly about robustness. I've skipped points the other bots already raised and replied in their threads where I had more to add or disagreed.
Note that CI never passes -PenableCoverage, so the green runs only show that the default build is unaffected; none of the new coverage paths are exercised.
Questions:
- Has a full
coverageReportrun with at least one failing test been tried, on both the glibc (Test) and musl (Exec) paths? That would confirm the deferral and its ordering (see the comment onddprof-test/build.gradle.ktsL159). - What wall-clock time and coverage numbers does a full run give? That would help decide whether this becomes a nightly job later.
rkennke
left a comment
rkennke
left a comment
There was a problem hiding this comment.
Thank you, looks good to me!
kaahos
left a comment
kaahos
left a comment
There was a problem hiding this comment.
This looks good to me thanks!
What does this PR do?:
Adds comprehensive code coverage support for the Java Profiler, enabling both C++ coverage (via gcov/llvm-cov + gcovr) and Java coverage (via JaCoCo).
Key features:
coveragebuild configuration (opt-in via-PenableCoverage)coverageReporttask generates both C++ and Java HTML reportscheckCoverageToolspreflight task validates all required tools before the ~30-minute build/test cycle-fprofile-update=atomic) to prevent GCC bug 68080 corruption from concurrent threadsAbstractProcessProfilerTest.launch()Motivation:
Enable code coverage measurement for the profiler's native and Java code to identify untested code paths and improve test coverage.
Additional Notes:
coverageconfiguration is not publishable (excluded from artifacts)NativeBuildConfig.isDebugInstrumented()consolidates debug/coverage/sanitizer build detectionHow to test the change?:
Reports are generated at:
ddprof-lib/build/reports/coverage/cpp/index.htmlddprof-lib/build/reports/coverage/java/html/index.htmlFor Datadog employees:
credentials of any kind, I've requested a security review (run the
dd:platform-security-reviewskill, or file a request via the PSEC review form).
bewairealso runs automatically on every PR.Unsure? Have a question? Request a review!