diff --git a/.github/scripts/flake_report.py b/.github/scripts/flake_report.py index 67a6cc8ade..a28594592f 100755 --- a/.github/scripts/flake_report.py +++ b/.github/scripts/flake_report.py @@ -224,16 +224,23 @@ def cmd_report(args): other_task_failures = non_test_task_failures(args.attempt_log, args.test_task_pattern) - # Two independent signs that the final attempt stopped early rather than - # running to completion and failing tests: Gradle saying so in the log, and - # the attempt having reached fewer tests than another attempt managed. The - # latter needs more than one attempt to compare against, which slow suites - # (MAX_ATTEMPTS=1) do not have, so the log is the primary signal. + # Three independent signs that the final attempt stopped early rather than + # running to completion and failing tests: Gradle saying so in the log, the + # command dying to a signal, and the attempt having reached fewer tests than + # another attempt managed. The last needs more than one attempt to compare + # against, which slow suites (MAX_ATTEMPTS=1) do not have. A signal that + # kills Gradle itself leaves nothing alive to log a marker, so the exit + # status is the only evidence in that case. # Cell names are ---[-slow] (quarantine.KNOWN_LIBCS); # the leading token is the only part cut_short_marker needs. final_attempt_cut_short = cut_short_marker( args.attempt_log, musl=args.cell.split("-", 1)[0] == "musl" ) + # Shells report death by signal N as 128+N; Gradle never exits above 128 on + # its own, however many tests fail. + final_attempt_signal = None + if args.final_attempt_exit_code is not None and args.final_attempt_exit_code > 128: + final_attempt_signal = args.final_attempt_exit_code - 128 observed_shortfall = None if final_attempt_ran and ran > 1: best_observed = max(len(seen) for _, seen, _ in attempts) @@ -264,8 +271,9 @@ def cmd_report(args): # must not gate. # final attempt exited -> gate only with evidence that it was cut # non-zero and was cut short short: Gradle reporting a dead test JVM, - # or the attempt having reached fewer - # tests than another attempt managed. A + # the command dying to a signal, or the + # attempt having reached fewer tests + # than another attempt managed. A # quarantined test that fails makes Gradle # exit non-zero all by itself, so treating # every non-zero exit as a crash would @@ -312,6 +320,13 @@ def cmd_report(args): "short ({!r}), so the tests missing from its results cannot be " "read as quarantined" ).format(args.final_attempt_exit_code, final_attempt_cut_short) + elif final_attempt_signal is not None: + gates = True + gate_reason = ( + "the final attempt exited {} (killed by signal {}), so it did not " + "run to completion and the tests missing from its results cannot " + "be read as quarantined" + ).format(args.final_attempt_exit_code, final_attempt_signal) elif args.final_attempt_exit_code not in (None, 0) and observed_shortfall: gates = True gate_reason = ( @@ -351,6 +366,7 @@ def cmd_report(args): "final_attempt_failure_count": final_attempt_failure_count, "other_task_failures": other_task_failures, "final_attempt_cut_short": final_attempt_cut_short, + "final_attempt_signal": final_attempt_signal, "final_attempt_observed_shortfall": observed_shortfall, "final_attempt_exit_code": args.final_attempt_exit_code, "gates": gates, @@ -381,7 +397,9 @@ def cmd_report(args): def main(): parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("--list", default=quarantine.DEFAULT_LIST) - sub = parser.add_subparsers(dest="command", required=True) + # required=True on add_subparsers() needs Python 3.7+; this also has to + # run under Python 3.6 (EL7's base-repo python3), so the check is manual. + sub = parser.add_subparsers(dest="command") count = sub.add_parser("count", help="print the number of distinct failed tests") count.add_argument("--dir", required=True) @@ -408,6 +426,8 @@ def main(): report.set_defaults(func=cmd_report) args = parser.parse_args() + if args.command is None: + parser.error("a command is required") return args.func(args) diff --git a/.github/scripts/quarantine.py b/.github/scripts/quarantine.py index fb048073f1..d0e14d8639 100755 --- a/.github/scripts/quarantine.py +++ b/.github/scripts/quarantine.py @@ -27,6 +27,7 @@ TICKET_RE = re.compile(r"^PROF-\d+$") DATE_RE = re.compile(r"^\d{4}-\d{2}-\d{2}$") FIELDS = ("test", "ticket", "added", "review_by", "cells", "reason") + # Long enough not to be busywork, short enough that a quarantine outlives # neither the release it was added in nor the memory of why. DEFAULT_REVIEW_DAYS = 90 @@ -44,8 +45,15 @@ # Cell names are ---. Only libc and arch are a closed # set -- jdk and config come from the workflow inputs and grow without warning # -- so those two are the only axes worth checking a glob against. +# +# "el7" names the GitLab functional job's Oracle Linux 7 runtime, which is +# technically glibc but must not share the "glibc" token: that job runs a +# different environment (container, kernel, network stack) than the GitHub +# Actions "glibc" matrix (Ubuntu), and the two can fail the same test for +# unrelated reasons. Sharing a cell string would let an entry meant to excuse +# one silently excuse the other. KNOWN_ARCHES = ("amd64", "aarch64") -KNOWN_LIBCS = ("glibc", "musl") +KNOWN_LIBCS = ("glibc", "musl", "el7") # Anything that reads like an architecture. A glob naming one that CI never # builds silently quarantines nothing, which is how "*arm64*" shipped in this # file's own example: the arch is spelled aarch64. @@ -182,6 +190,15 @@ def covers(entry, test_id): return test_id == pattern +def _parse_date(s): + """A YYYY-MM-DD string as a date, or raises ValueError. + + date.fromisoformat() needs Python 3.7+; this also has to run under + Python 3.6 (EL7's base-repo python3). + """ + return datetime.datetime.strptime(s, "%Y-%m-%d").date() + + def is_expired(entry, today=None): """True once this entry's review_by date has passed. @@ -199,7 +216,7 @@ def is_expired(entry, today=None): # entry that can be. An expiry that cannot be read has passed. return True try: - due = datetime.date.fromisoformat(review_by) + due = _parse_date(review_by) except ValueError: return True return due < (today or datetime.date.today()) @@ -311,7 +328,7 @@ def complain(line, message): if entry["added"] and DATE_RE.match(entry["added"]): try: - datetime.date.fromisoformat(entry["added"]) + _parse_date(entry["added"]) except ValueError: complain(line, "added '{}' is not a real calendar date".format(entry["added"])) @@ -338,7 +355,7 @@ def complain(line, message): if DATE_RE.match(entry["review_by"]): try: - due = datetime.date.fromisoformat(entry["review_by"]) + due = _parse_date(entry["review_by"]) except ValueError: complain(line, "review_by '{}' is not a real calendar date".format( entry["review_by"])) @@ -370,12 +387,16 @@ def complain(line, message): def main(): parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("--list", default=DEFAULT_LIST) - sub = parser.add_subparsers(dest="command", required=True) + # required=True on add_subparsers() needs Python 3.7+; this also has to run + # under Python 3.6 (EL7's base-repo python3), so the check is manual. + sub = parser.add_subparsers(dest="command") validate = sub.add_parser("validate", help="check the list's format and review dates") validate.set_defaults(func=cmd_validate) args = parser.parse_args() + if args.command is None: + parser.error("a command is required") return args.func(args) diff --git a/.github/scripts/tests/test_quarantine.sh b/.github/scripts/tests/test_quarantine.sh index 078e5bc238..c2406138a2 100755 --- a/.github/scripts/tests/test_quarantine.sh +++ b/.github/scripts/tests/test_quarantine.sh @@ -675,6 +675,29 @@ assert d['final_attempt_cut_short'], d " "$CASE/out.json" || fail "a dead test JVM was excused by the quarantine list" pass "a cut-short final attempt gates despite its failures being quarantined" +# A signal that kills Gradle itself (e.g. the OOM killer taking the --no-daemon +# build JVM) leaves nothing alive to print a marker, and with a single attempt +# there is no earlier run to measure a shortfall against. The exit status is +# then the only evidence the suite never finished. +CASE="$TEMP_DIR/case-quarantined-killed-by-signal" +mkdir -p "$CASE/flake-evidence/attempt-1" +write_failure_xml "$CASE/flake-evidence/attempt-1" "com.dd.WobblyTest" "sometimesFails" "boom" +cat > "$CASE/attempt.log" <<'EOS' +> Task :ddprof-test:testRelease +EOS +write_list "$CASE/list.txt" "$(entry com.dd.WobblyTest.sometimesFails PROF-1 "$(day_offset 30)")" +python3 "$SCRIPTS/flake_report.py" --list "$CASE/list.txt" report \ + --cell "el7-8-release-amd64" --evidence-dir "$CASE/flake-evidence" \ + --final-attempt 1 --attempt-log "$CASE/attempt.log" \ + --final-attempt-exit-code 137 --test-task-pattern test \ + --out "$CASE/out.json" >/dev/null 2>&1 +python3 -c " +import json,sys +d = json.load(open(sys.argv[1])) +assert d['gates'] is True, 'a final attempt killed by a signal must gate even with every named failure quarantined: %r' % d['gate_reason'] +" "$CASE/out.json" || fail "a final attempt killed by SIGKILL was excused by the quarantine list" +pass "a final attempt killed by a signal gates despite its failures being quarantined" + # musl runs ProfilerTestRunner through a plain Exec task, not Gradle's native # Test task: Gradle prints the exact same "finished with non-zero exit value" # line whenever that process exits non-zero for *any* reason, including an diff --git a/.gitlab/base/el7/Dockerfile b/.gitlab/base/el7/Dockerfile index 5c6f261199..0dafec4fa9 100644 --- a/.gitlab/base/el7/Dockerfile +++ b/.gitlab/base/el7/Dockerfile @@ -23,7 +23,11 @@ ARG BASE_IMAGE=oraclelinux:7 FROM ${BASE_IMAGE} as base WORKDIR /root -RUN yum -y install git make zip unzip which wget curl jq gcc-c++ \ +# python3 (3.6.8 on this base repo -- no newer python3/SCL repo exists for +# Oracle Linux 7) runs the quarantine classifier (.github/scripts/ +# run_tests_with_retry.sh, quarantine.py, flake_report.py) that +# functional-tests.sh wraps its test run in. +RUN yum -y install git make zip unzip which wget curl jq gcc-c++ python3 \ && yum -y clean all RUN curl -s "https://get.sdkman.io" | bash && \ diff --git a/.gitlab/build-deploy/.gitlab-ci.yml b/.gitlab/build-deploy/.gitlab-ci.yml index 6a6455ae02..5727a68990 100644 --- a/.gitlab/build-deploy/.gitlab-ci.yml +++ b/.gitlab/build-deploy/.gitlab-ci.yml @@ -9,8 +9,8 @@ variables: BUILD_IMAGE_X64_GLIBC: registry.ddbuild.io/ci/async-profiler-build:v137281788-x64-glibc-base@sha256:270a9101716a0865071182e0ef0e40e98f874535b90722949b33a4a8998fe08d # Generated by https://gitlab.ddbuild.io/DataDog/java-profiler/-/jobs/2040944172 BUILD_IMAGE_ARM64_GLIBC: registry.ddbuild.io/ci/async-profiler-build:v137281788-arm64-glibc-base@sha256:511d051d37ed99b2ca9cd3af797aa9de63e4b4b952af1e738b2cea913bbcc057 - # Generated by pipeline 138438605 - BUILD_IMAGE_X64_EL7: registry.ddbuild.io/ci/async-profiler-build:v138438605-x64-el7-base@sha256:2f4d2852979d7b190dcb997bbc833eb795f2f0db9905300ec239516480e8599d + # Generated by pipeline 139300629 + BUILD_IMAGE_X64_EL7: registry.ddbuild.io/ci/async-profiler-build:v139300629-x64-el7-base@sha256:b3a77ea5db8bd245a3d455e38b7a0e9579f35c5c8099cc0e39fc8de29fb07b47 # Generated by https://gitlab.ddbuild.io/DataDog/java-profiler/-/jobs/2040944172 BUILD_IMAGE_X64_MUSL: registry.ddbuild.io/ci/async-profiler-build:v137281788-x64-musl-base@sha256:fd3803695771de3eabf44e4953c0474273f9dbb97d88c22e99ac507b88572481 # Generated by https://gitlab.ddbuild.io/DataDog/java-profiler/-/jobs/2040944172 @@ -228,6 +228,13 @@ stresstest:arm64-musl: paths: - functional/$TARGET/jdk$TEST_JDK/reports - functional/$TARGET/jdk$TEST_JDK/logs + # run_tests_with_retry.sh's own evidence: each attempt's JUnit XML + # snapshot, the classifier's gating decision, and the final attempt's + # raw log -- without these, a quarantine decision on this job has + # nothing to show for it besides the console log. + - flake-evidence + - ci-outcome + - build/logs/attempt.log expire_in: 2 weeks # Runs on every pipeline (not just nightly): JDK 8 is the oldest/most diff --git a/.gitlab/scripts/functional-tests.sh b/.gitlab/scripts/functional-tests.sh index 3ad233a191..9f04779971 100755 --- a/.gitlab/scripts/functional-tests.sh +++ b/.gitlab/scripts/functional-tests.sh @@ -68,6 +68,23 @@ function onexit { trap onexit EXIT -./gradlew -Pddprof_version="$(get_version)" -Pskip-native=ddprof-lib,malloc-shim -Pwith-libs="$(pwd)/libs" -PCI \ - -PtestMaxHeap=1536m \ +# "el7", not "glibc": this runtime (container, kernel, network stack) is a +# different environment than the GitHub Actions glibc/Ubuntu matrix and can +# fail the same test for unrelated reasons, so it needs its own cell token +# (quarantine.py's KNOWN_LIBCS) -- sharing "glibc" would let an entry meant to +# excuse one silently excuse the other. +CELL="el7-${TEST_JDK}-$(printf '%s' "${TEST_CONFIG}" | tr '[:upper:]' '[:lower:]')-amd64" + +# Default to no retry: a flaky-vs-broken retry means a full second suite run, +# and this runner has already OOMKilled the container outright at the default +# 2 attempts (see PR #805). Overridable via the job's own variables. +export MAX_ATTEMPTS="${MAX_ATTEMPTS:-1}" + +# Compile build-logic inside the Gradle process: the default Kotlin compile +# daemon (~660MB RSS) otherwise stays resident through the whole test run in +# this memory-limited pod. Not set globally -- under CodeQL's Kotlin extractor +# an in-process compile thrashes Gradle's default 512m heap. +.github/scripts/run_tests_with_retry.sh "${CELL}" -- \ + ./gradlew -Pddprof_version="$(get_version)" -Pskip-native=ddprof-lib,malloc-shim -Pwith-libs="$(pwd)/libs" -PCI \ + -PtestMaxHeap=1536m -Pkotlin.compiler.execution.strategy=in-process \ ":ddprof-test:test${TEST_CONFIG}" --max-workers=1 --build-cache --stacktrace --info --no-watch-fs --no-daemon diff --git a/build-logic/conventions/src/main/kotlin/com/datadoghq/native/util/PlatformUtils.kt b/build-logic/conventions/src/main/kotlin/com/datadoghq/native/util/PlatformUtils.kt index f48055ac98..f6c4bbb44f 100644 --- a/build-logic/conventions/src/main/kotlin/com/datadoghq/native/util/PlatformUtils.kt +++ b/build-logic/conventions/src/main/kotlin/com/datadoghq/native/util/PlatformUtils.kt @@ -377,9 +377,11 @@ object PlatformUtils { /** * Whether native compilation should be skipped for [project]. * - * `-Pskip-native` (bare, no value) skips it everywhere — the long-standing - * behavior, used when consuming a prebuilt shipped library via -Pwith-libs - * and never touching a compiler. + * `-Pskip-native` (bare, no value) or `skip-native=true` (the form + * gradle.properties.template documents) skips it everywhere — the + * long-standing behavior, used when consuming a prebuilt shipped library + * via -Pwith-libs and never touching a compiler. `skip-native=false` skips + * nothing. * * `-Pskip-native=` skips it only for those * projects. This lets a caller substitute the prebuilt *shipped* library @@ -392,10 +394,13 @@ object PlatformUtils { if (!project.hasProperty("skip-native")) { return false } - val value = project.property("skip-native") as? String - if (value.isNullOrEmpty()) { + val value = (project.property("skip-native") as? String)?.trim() + if (value.isNullOrEmpty() || value.equals("true", ignoreCase = true)) { return true } + if (value.equals("false", ignoreCase = true)) { + return false + } return value.split(",").map { it.trim() }.contains(project.name) } diff --git a/ddprof-test/quarantine.txt b/ddprof-test/quarantine.txt index 7fbd2121da..6944b1e105 100644 --- a/ddprof-test/quarantine.txt +++ b/ddprof-test/quarantine.txt @@ -42,5 +42,32 @@ # When CI sees a flake it prints a ready-made line in the PR comment. The # ticket and the judgement are still yours. # -# Example (delete when the first real entry lands): +# Example: # com.datadoghq.profiler.cpu.CpuSamplingTest.testSampling | PROF-12345 | 2026-09-02 | 2026-12-02 | *aarch64* | Under-samples on emulated aarch64; 2 of 40 runs +# +# NativeSocketSampler's hooks install and fire correctly on this cell +# (confirmed via a debug-config diagnostic build -- send/recv/write/read all +# called, PLT patching succeeds); the rate limiter then rejects nearly every +# sample because doTcpTransfer()'s "the send buffer fills and blocks after +# ~32 iterations" assumption does not hold on this runner's network stack, so +# calls never get slow enough for the time-weighted sampler to keep them. +# Scoped to el7-8-release-amd64 (the only cell this has been observed on) -- +# widen only with fresh evidence from the nightly el7-17/-21 cells. +# +# Every one of these classes' single test method is @RetryingTest, which (like +# @ParameterizedTest/@TestTemplate) reports as "Class.[1]", "[2]", ... in the +# XML -- no method name at all -- so the entry has to be class-wide; an exact +# method name here would never match anything. NativeSocketUdpExcludedTest is +# deliberately not listed: it asserts *zero* events, which the bug's symptom +# (near-zero events everywhere) would make pass, not fail, so a failure there +# has a different, unconfirmed cause and quarantining it on this evidence +# risks hiding an unrelated regression later. +com.datadoghq.profiler.nativesocket.NativeSocketBytesAccuracyTest.* | PROF-16014 | 2026-09-21 | 2026-12-20 | el7-8-release-amd64 | NativeSocketEvent count 0: doTcpTransfer() never blocks long enough to be sampled on this runner +com.datadoghq.profiler.nativesocket.NativeSocketEnabledTest.* | PROF-16014 | 2026-09-21 | 2026-12-20 | el7-8-release-amd64 | NativeSocketEvent count 0: doTcpTransfer() never blocks long enough to be sampled on this runner +com.datadoghq.profiler.nativesocket.NativeSocketEventFieldsTest.* | PROF-16014 | 2026-09-21 | 2026-12-20 | el7-8-release-amd64 | NativeSocketEvent count 0: doTcpTransfer() never blocks long enough to be sampled on this runner +com.datadoghq.profiler.nativesocket.NativeSocketEventThreadTest.* | PROF-16014 | 2026-09-21 | 2026-12-20 | el7-8-release-amd64 | NativeSocketEvent count 0: doTcpTransfer() never blocks long enough to be sampled on this runner +com.datadoghq.profiler.nativesocket.NativeSocketRateLimitTest.* | PROF-16014 | 2026-09-21 | 2026-12-20 | el7-8-release-amd64 | NativeSocketEvent count 0: doTcpTransfer() never blocks long enough to be sampled on this runner +com.datadoghq.profiler.nativesocket.NativeSocketRemoteAddressTest.* | PROF-16014 | 2026-09-21 | 2026-12-20 | el7-8-release-amd64 | NativeSocketEvent count 0: doTcpTransfer() never blocks long enough to be sampled on this runner +com.datadoghq.profiler.nativesocket.NativeSocketRestartTest.* | PROF-16014 | 2026-09-21 | 2026-12-20 | el7-8-release-amd64 | NativeSocketEvent count 0: doTcpTransfer() never blocks long enough to be sampled on this runner +com.datadoghq.profiler.nativesocket.NativeSocketSendRecvSeparateTest.* | PROF-16014 | 2026-09-21 | 2026-12-20 | el7-8-release-amd64 | NativeSocketEvent count 0: doTcpTransfer() never blocks long enough to be sampled on this runner +com.datadoghq.profiler.nativesocket.NativeSocketStackTraceTest.* | PROF-16014 | 2026-09-21 | 2026-12-20 | el7-8-release-amd64 | @TestTemplate invocations (cstack=vm/vmx/dwarf/fp) have no method name in the XML; same root cause as the rest of the package