diff --git a/.github/scripts/extract_coverage_artifact.py b/.github/scripts/extract_coverage_artifact.py new file mode 100644 index 000000000..12a74b84f --- /dev/null +++ b/.github/scripts/extract_coverage_artifact.py @@ -0,0 +1,77 @@ +"""Copy one expected coverage report from a ZIP without extracting archive paths.""" + +import argparse +from pathlib import Path, PurePosixPath +import stat +import zipfile + +MAX_ARCHIVE_FILES = 10_000 +MAX_ARCHIVE_BYTES = 256 * 1024 * 1024 +MAX_REPORT_BYTES = 64 * 1024 * 1024 + + +def select(archive, kind): + members = archive.infolist() + if ( + len(members) > MAX_ARCHIVE_FILES + or sum(member.file_size for member in members) > MAX_ARCHIVE_BYTES + ): + raise ValueError("Coverage artifact exceeds size limits") + + candidates = [] + for member in members: + path = PurePosixPath(member.filename) + if ( + member.is_dir() + or stat.S_ISDIR(member.external_attr >> 16) + or path.is_absolute() + or ".." in path.parts + or "\\" in member.filename + or member.flag_bits & 1 + or stat.S_ISLNK(member.external_attr >> 16) + or member.file_size > MAX_REPORT_BYTES + ): + continue + if kind == "html" and path.name == "index.html" and "Code Coverage Report" in str(path): + candidates.append((0, member)) + elif kind == "xml" and path.suffix.lower() == ".xml": + name = path.name.lower() + if str(path).endswith("unified-coverage/coverage.xml"): + priority = 0 + elif name == "coverage.xml": + priority = 1 + elif "coverage" in name: + priority = 2 + else: + continue + candidates.append((priority, member)) + + if not candidates: + raise ValueError(f"No coverage {kind} report found") + priority = min(item[0] for item in candidates) + selected = [member for rank, member in candidates if rank == priority] + return selected + + +def copy_report(archive_path, output, kind): + if Path(archive_path).stat().st_size > MAX_ARCHIVE_BYTES: + raise ValueError("Coverage archive exceeds size limit") + with zipfile.ZipFile(archive_path) as archive: + selected = select(archive, kind) + data = archive.read(selected[0]) + if any(archive.read(member) != data for member in selected[1:]): + raise ValueError(f"Conflicting coverage {kind} reports") + Path(output).write_bytes(data) + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("kind", choices=("html", "xml")) + parser.add_argument("archive", type=Path) + parser.add_argument("output", type=Path) + args = parser.parse_args() + copy_report(args.archive, args.output, args.kind) + + +if __name__ == "__main__": + main() diff --git a/.github/scripts/post_profiler_comment.py b/.github/scripts/post_profiler_comment.py new file mode 100644 index 000000000..6b015f0cf --- /dev/null +++ b/.github/scripts/post_profiler_comment.py @@ -0,0 +1,365 @@ +"""Read public ADO artifacts as data and update a SHA-bound PR performance comment.""" + +import argparse +from http.client import HTTPException +import json +import os +from pathlib import Path +import re +import sys +import time +from urllib.error import URLError +from urllib.parse import urlencode, urlparse +from urllib.request import HTTPRedirectHandler, Request, build_opener + +sys.path.insert(0, str(Path(__file__).resolve().parents[2])) +from eng.profiler_benchmarks import report as reporting + +ROOT = Path(__file__).resolve().parents[2] +ADO = "https://dev.azure.com/sqlclientdrivers/public/_apis/build" +REPOSITORY = "microsoft/mssql-python" +HEADER = f"{reporting.MARKER}\n## PR Performance Report\n\n" +# Allow a 160-minute ADO job plus queueing; the workflow reserves publication time. +WAIT_MINUTES = 220 +COMPLETED_RESULTS = {"succeeded", "partiallySucceeded", "failed"} + + +def allowed_url(url): + parsed = urlparse(url) + host = parsed.hostname or "" + if ( + parsed.scheme != "https" + or parsed.username + or parsed.password + or parsed.port not in (None, 443) + ): + return False + return host in ("api.github.com", "dev.azure.com", "sqlclientdrivers.visualstudio.com") or ( + host.endswith(".vsblob.vsassets.io") + or host.endswith(".blob.core.windows.net") + or host.endswith(".artifacts.visualstudio.com") + ) + + +class SafeRedirect(HTTPRedirectHandler): + def redirect_request(self, req, fp, code, msg, headers, newurl): + if not allowed_url(newurl): + raise ValueError("Artifact redirect outside permitted hosts") + redirected = super().redirect_request(req, fp, code, msg, headers, newurl) + if urlparse(req.full_url).hostname != urlparse(newurl).hostname: + redirected.remove_header("Authorization") + return redirected + + +def fetch(url, token=None, method=None, data=None, limit=4 * 1024 * 1024): + if not allowed_url(url): + raise ValueError("URL outside permitted hosts") + headers = {"Accept": "application/json", "User-Agent": "mssql-python-profiler-ci"} + if token: + if urlparse(url).hostname != "api.github.com": + raise ValueError("GitHub credentials must not be sent to artifact hosts") + headers["Authorization"] = "Bearer " + token + payload = None if data is None else json.dumps(data).encode() + if payload is not None: + headers["Content-Type"] = "application/json" + request = Request(url, headers=headers, method=method, data=payload) + try: + with build_opener(SafeRedirect()).open(request, timeout=30) as response: + body = response.read(limit + 1) + except (HTTPException, ConnectionError) as error: + raise URLError("Incomplete HTTP response") from error + if len(body) > limit: + raise ValueError("Response exceeds size limit") + return body + + +def api(url, **kwargs): + return json.loads(fetch(url, **kwargs).decode("utf-8-sig")) + + +def github(path, **kwargs): + return api( + f"https://api.github.com/repos/{REPOSITORY}/{path}", token=os.environ["GH_TOKEN"], **kwargs + ) + + +def publish(pr_number, head, body, base=None): + def current(): + pr = github(f"pulls/{pr_number}") + return ( + pr["state"] == "open" + and pr["head"]["sha"] == head + and (base is None or pr["base"]["sha"] == base) + ) + + if not current(): + print("Not publishing stale performance results") + return + page = 1 + comment = None + while True: + comments = github(f"issues/{pr_number}/comments?per_page=100&page={page}") + comment = next( + ( + c + for c in comments + if c["user"]["login"] == "github-actions[bot]" + and c["body"].startswith(reporting.MARKER) + ), + comment, + ) + if len(comments) < 100: + break + page += 1 + if comment: + if not current(): + return + github(f"issues/comments/{comment['id']}", method="PATCH", data={"body": body}) + else: + if not current(): + return + github(f"issues/{pr_number}/comments", method="POST", data={"body": body}) + + +def publish_with_retry(pr_number, head, body, base=None, attempts=3): + for attempt in range(attempts): + try: + publish(pr_number, head, body, base) + return + except (KeyError, TypeError, ValueError, TimeoutError, URLError): + if attempt + 1 == attempts: + raise + time.sleep(5) + + +def find_build(builds, number, head): + return next( + ( + build + for build in builds + if isinstance(build, dict) + and isinstance(build.get("definition"), dict) + and isinstance(build.get("repository"), dict) + and isinstance(build["repository"].get("id"), str) + and isinstance(build.get("triggerInfo"), dict) + and build.get("definition", {}).get("id") == 2128 + and build.get("repository", {}).get("id", "").lower() == REPOSITORY + and build.get("sourceBranch") == f"refs/pull/{number}/merge" + and build.get("triggerInfo", {}).get("pr.sourceSha") == head + and build.get("triggerInfo", {}).get("pr.number") == str(number) + ), + None, + ) + + +def build_items(response): + items = response.get("value") if isinstance(response, dict) else None + if not isinstance(items, list) or not all( + isinstance(build, dict) + and isinstance(build.get("id"), int) + and isinstance(build.get("status"), str) + and isinstance(build.get("definition"), dict) + and isinstance(build.get("repository"), dict) + and isinstance(build["repository"].get("id"), str) + and isinstance(build.get("triggerInfo"), dict) + and isinstance(build.get("sourceBranch"), str) + for build in items + ): + raise ValueError("Invalid build list") + return items + + +def artifact_items(response): + items = response.get("value") if isinstance(response, dict) else None + if not isinstance(items, list) or not all( + isinstance(item, dict) + and isinstance(item.get("name"), str) + and isinstance(item.get("resource"), dict) + for item in items + ): + raise ValueError("Invalid artifact list") + return items + + +def unavailable(number, head, reason, base=None): + publish_with_retry( + number, + head, + HEADER + "**Performance could not be assessed.**\n\n" + reason + " No result is available.", + base, + ) + + +def run(number, head, wait_minutes): + publish_with_retry( + number, + head, + HEADER + + "**Performance assessment pending.**\n\n" + + f"Waiting for the matching performance run for head `{head}`.", + ) + deadline = time.monotonic() + wait_minutes * 60 + build = None + pr_base = None + failures = 0 + while time.monotonic() < deadline: + try: + pr = github(f"pulls/{number}") + if ( + not isinstance(pr, dict) + or not isinstance(pr.get("state"), str) + or not isinstance(pr.get("head"), dict) + or not isinstance(pr.get("base"), dict) + ): + raise ValueError + current_head = pr["head"].get("sha") + current_base = pr["base"].get("sha") + pr_base = current_base + query = urlencode( + { + "definitions": 2128, + "branchName": f"refs/pull/{number}/merge", + "queryOrder": "queueTimeDescending", + "$top": 50, + "api-version": "7.1", + } + ) + build = find_build(build_items(api(f"{ADO}/builds?{query}")), number, head) + if pr["state"] != "open" or current_head != head: + return + if ( + build is not None + and build.get("status") == "completed" + and build.get("result") not in COMPLETED_RESULTS | {"canceled"} + ): + unavailable( + number, + head, + "Performance run completed with an unsupported result.", + pr_base, + ) + return + complete = ( + build is not None + and build.get("status") == "completed" + and build.get("result") in COMPLETED_RESULTS + ) + except (ValueError, KeyError, TypeError, URLError, TimeoutError): + failures += 1 + if failures >= 5: + unavailable(number, head, "Performance data services failed repeatedly.", pr_base) + return + time.sleep(30) + continue + failures = 0 + if complete: + break + time.sleep(30) + if ( + build is None + or build.get("status") != "completed" + or build.get("result") not in COMPLETED_RESULTS + ): + unavailable( + number, + head, + f"No matching performance run completed within the {wait_minutes}-minute wait " + f"for `{head}`.", + pr_base, + ) + return + build_id = build["id"] + source = build.get("sourceVersion") + try: + if ( + type(build_id) is not int + or build_id <= 0 + or not re.fullmatch(r"[0-9a-f]{40}", source) + or not re.fullmatch(r"[0-9a-f]{40}", pr_base or "") + ): + raise ValueError + commit = github(f"git/commits/{source}") + base = pr_base + base_commit = github(f"git/commits/{base}") + if not isinstance(commit, dict) or not isinstance(base_commit, dict): + raise ValueError + source_tree_info = commit.get("tree") + base_tree_info = base_commit.get("tree") + if not isinstance(source_tree_info, dict) or not isinstance(base_tree_info, dict): + raise ValueError + source_tree_sha = source_tree_info.get("sha") + base_tree_sha = base_tree_info.get("sha") + if not re.fullmatch(r"[0-9a-f]{40}", source_tree_sha or "") or not re.fullmatch( + r"[0-9a-f]{40}", base_tree_sha or "" + ): + raise ValueError + source_tree = github(f"git/trees/{source_tree_sha}?recursive=1") + base_tree = github(f"git/trees/{base_tree_sha}?recursive=1") + except (ValueError, KeyError, TypeError, URLError, TimeoutError): + unavailable(number, head, "Build provenance validation failed.", pr_base) + return + artifacts = None + failures = 0 + while time.monotonic() < deadline: + try: + artifacts = artifact_items(api(f"{ADO}/builds/{build_id}/artifacts?api-version=7.1")) + failures = 0 + if {"profiler-" + leg for leg in reporting.LEGS} <= { + item["name"] for item in artifacts + }: + break + except (ValueError, KeyError, TypeError, URLError, TimeoutError): + failures += 1 + if failures >= 5: + artifacts = None + break + time.sleep(30) + if artifacts is None: + unavailable(number, head, "Performance artifacts remained unavailable.", pr_base) + return + artifact_urls, issues = {}, [] + for leg in reporting.LEGS: + matching = [item for item in artifacts if item["name"] == "profiler-" + leg] + if len(matching) != 1: + issues.append(leg + " (missing)") + continue + url = matching[0]["resource"].get("downloadUrl") + if not isinstance(url, str): + issues.append(leg + " (invalid artifact)") + continue + artifact_urls[leg] = url + + def load_artifact(url): + try: + return fetch(url, limit=32 * 1024 * 1024) + except (TimeoutError, URLError) as error: + raise ValueError("Artifact download failed") from error + + evidence = reporting.AssessmentEvidence( + build=build, + head=head, + base=base, + merge_commit=commit, + base_commit=base_commit, + source_tree=source_tree, + base_tree=base_tree, + trusted_root=ROOT, + ) + publish_with_retry( + number, head, reporting.assess(evidence, artifact_urls, load_artifact, issues), base + ) + + +if __name__ == "__main__": + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--pr", type=int, required=True) + parser.add_argument("--head", required=True) + parser.add_argument("--wait-minutes", type=int, default=WAIT_MINUTES) + args = parser.parse_args() + if ( + args.pr <= 0 + or not re.fullmatch(r"[0-9a-f]{40}", args.head) + or not 1 <= args.wait_minutes <= WAIT_MINUTES + ): + parser.error("Invalid PR, head SHA or wait limit") + run(args.pr, args.head, args.wait_minutes) diff --git a/.github/workflows/pr-code-coverage.yml b/.github/workflows/pr-code-coverage.yml index 123a8bf1d..909950571 100644 --- a/.github/workflows/pr-code-coverage.yml +++ b/.github/workflows/pr-code-coverage.yml @@ -8,9 +8,14 @@ on: permissions: contents: read +concurrency: + group: pr-code-coverage-${{ github.event.pull_request.number }} + cancel-in-progress: true + jobs: coverage-report: runs-on: ubuntu-latest + timeout-minutes: 245 permissions: pull-requests: write contents: read @@ -34,30 +39,44 @@ jobs: git show-ref --verify refs/remotes/origin/main || echo "Warning: origin/main not found" - name: Wait for ADO build to start + env: + PR_NUMBER: ${{ github.event.pull_request.number }} + PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} run: | - PR_NUMBER=${{ github.event.pull_request.number }} - API_URL="https://dev.azure.com/sqlclientdrivers/public/_apis/build/builds?definitions=2128&queryOrder=queueTimeDescending&%24top=10&api-version=7.1-preview.7" + PR_BRANCH="refs/pull/$PR_NUMBER/merge" + API_URL="https://dev.azure.com/sqlclientdrivers/public/_apis/build/builds?definitions=2128&branchName=refs%2Fpull%2F${PR_NUMBER}%2Fmerge&queryOrder=queueTimeDescending&%24top=100&api-version=7.1-preview.7" + DEADLINE=$((SECONDS + 15 * 60)) + API_FAILURES=0 + BUILD_ID="" - echo "Waiting for Azure DevOps build to start for PR #$PR_NUMBER ..." + echo "Waiting up to 15 minutes for Azure DevOps build for PR #$PR_NUMBER at $PR_HEAD_SHA..." - for i in {1..30}; do - echo "Attempt $i/30: Checking if build has started..." - - # Fetch API response with error handling - API_RESPONSE=$(curl -s "$API_URL") - - # Check if response is valid JSON - if ! echo "$API_RESPONSE" | jq . >/dev/null 2>&1; then - echo "❌ Invalid JSON response from Azure DevOps API" - echo "Response received: $API_RESPONSE" - echo "This usually indicates the Azure DevOps pipeline has failed or API is unavailable" - exit 1 + while (( SECONDS < DEADLINE )); do + REQUEST_TIMEOUT=$((DEADLINE - SECONDS)) + if (( REQUEST_TIMEOUT <= 0 )); then break; fi + if (( REQUEST_TIMEOUT > 30 )); then REQUEST_TIMEOUT=30; fi + if API_RESPONSE=$(curl --fail --silent --show-error --connect-timeout 10 --max-time "$REQUEST_TIMEOUT" "$API_URL") && + jq -e 'type == "object" and (.value | type == "array")' <<< "$API_RESPONSE" >/dev/null 2>&1; then + API_FAILURES=0 + else + API_FAILURES=$((API_FAILURES + 1)) + echo "⚠️ Build API HTTP/JSON error ($API_FAILURES/5)" + if (( API_FAILURES >= 5 )); then + echo "❌ Azure DevOps build API unavailable after 5 consecutive failures" + exit 1 + fi + API_RESPONSE='{"value":[]}' fi - - # Parse build info safely - BUILD_INFO=$(echo "$API_RESPONSE" | jq -c --arg PR "$PR_NUMBER" '[.value[]? | select(.triggerInfo["pr.number"]?==$PR)] | .[0] // empty' 2>/dev/null) - - if [[ -n "$BUILD_INFO" && "$BUILD_INFO" != "null" && "$BUILD_INFO" != "empty" ]]; then + + # The merge ref is shared across revisions; match the actual PR head as well. + BUILD_INFO=$(jq -c --arg PR "$PR_NUMBER" --arg SHA "$PR_HEAD_SHA" --arg BRANCH "$PR_BRANCH" ' + [.value[]? | select( + .definition.id == 2128 and .sourceBranch == $BRANCH and + (.triggerInfo["pr.number"] | tostring) == $PR and + .triggerInfo["pr.sourceSha"] == $SHA + )] | .[0] // empty' <<< "$API_RESPONSE") + + if [[ -n "$BUILD_INFO" ]]; then STATUS=$(echo "$BUILD_INFO" | jq -r '.status // "unknown"') RESULT=$(echo "$BUILD_INFO" | jq -r '.result // "unknown"') BUILD_ID=$(echo "$BUILD_INFO" | jq -r '.id // "unknown"') @@ -74,98 +93,162 @@ jobs: echo "✅ Found build: ID=$BUILD_ID, Status=$STATUS, Result=$RESULT" echo "🔗 Build URL: $WEB_URL" - echo "ADO_URL=$WEB_URL" >> $GITHUB_ENV - echo "BUILD_ID=$BUILD_ID" >> $GITHUB_ENV - - # Check if build has failed early - if [[ "$STATUS" == "completed" && "$RESULT" == "failed" ]]; then - echo "❌ Azure DevOps build $BUILD_ID failed early" - echo "This coverage workflow cannot proceed when the main build fails." - exit 1 - fi - - echo "🚀 Build has started, proceeding to poll for coverage artifacts..." - break - else - echo "⏳ No build found for PR #$PR_NUMBER yet... (attempt $i/30)" - fi + echo "ADO_URL=$WEB_URL" >> "$GITHUB_ENV" + echo "BUILD_ID=$BUILD_ID" >> "$GITHUB_ENV" + echo "PR_NUMBER=$PR_NUMBER" >> "$GITHUB_ENV" + echo "PR_HEAD_SHA=$PR_HEAD_SHA" >> "$GITHUB_ENV" - if [[ $i -eq 30 ]]; then - echo "❌ Timeout: No build found for PR #$PR_NUMBER after 30 attempts" - echo "This may indicate the Azure DevOps pipeline was not triggered" - exit 1 + # A failed matrix leg does not invalidate a successful coverage artifact. + echo "🚀 Build found, proceeding to poll for coverage artifacts..." + break fi - sleep 10 + echo "⏳ No matching build found for PR #$PR_NUMBER at $PR_HEAD_SHA yet..." + SLEEP_SECONDS=$((DEADLINE - SECONDS)) + if (( SLEEP_SECONDS > 30 )); then SLEEP_SECONDS=30; fi + if (( SLEEP_SECONDS > 0 )); then sleep "$SLEEP_SECONDS"; fi done + if [[ -z "$BUILD_ID" ]]; then + echo "❌ Timeout: No build found for PR #$PR_NUMBER at $PR_HEAD_SHA within 15 minutes" + exit 1 + fi + - name: Download and parse coverage report run: | - BUILD_ID=${{ env.BUILD_ID }} + BUILD_URL="https://dev.azure.com/SqlClientDrivers/public/_apis/build/builds/$BUILD_ID?api-version=7.1-preview.7" ARTIFACTS_URL="https://dev.azure.com/SqlClientDrivers/public/_apis/build/builds/$BUILD_ID/artifacts?api-version=7.1-preview.5" - - echo "📥 Polling for coverage artifacts for build $BUILD_ID..." - - # Poll for coverage artifacts with retry logic + BUILDS_URL="https://dev.azure.com/sqlclientdrivers/public/_apis/build/builds?definitions=2128&branchName=refs%2Fpull%2F${PR_NUMBER}%2Fmerge&queryOrder=queueTimeDescending&%24top=100&api-version=7.1-preview.7" + PR_BRANCH="refs/pull/$PR_NUMBER/merge" + # Coverage may queue behind 160-minute benchmark jobs before its own run. + DEADLINE=$((SECONDS + 220 * 60)) + COMPLETED_AT=-1 + ARTIFACT_FAILURES=0 + BUILD_FAILURES=0 COVERAGE_ARTIFACT="" - for i in {1..60}; do - echo "Attempt $i/60: Checking for coverage artifacts..." - - # Fetch artifacts with error handling - ARTIFACTS_RESPONSE=$(curl -s "$ARTIFACTS_URL") - - # Check if response is valid JSON - if ! echo "$ARTIFACTS_RESPONSE" | jq . >/dev/null 2>&1; then - echo "⚠️ Invalid JSON response from artifacts API (attempt $i/60)" - if [[ $i -eq 60 ]]; then - echo "❌ Persistent API issues after 60 attempts" - echo "Response received: $ARTIFACTS_RESPONSE" + COVERAGE_ARTIFACT_APPROVED=false + echo "📥 Waiting up to 220 minutes for coverage artifacts for build $BUILD_ID..." + + while (( SECONDS < DEADLINE )); do + COVERAGE_ARTIFACT="" + REQUEST_TIMEOUT=$((DEADLINE - SECONDS)) + if (( REQUEST_TIMEOUT <= 0 )); then break; fi + if (( REQUEST_TIMEOUT > 30 )); then REQUEST_TIMEOUT=30; fi + if ARTIFACTS_RESPONSE=$(curl --fail --silent --show-error --connect-timeout 10 --max-time "$REQUEST_TIMEOUT" "$ARTIFACTS_URL") && + jq -e 'type == "object" and (.value | type == "array")' <<< "$ARTIFACTS_RESPONSE" >/dev/null 2>&1; then + ARTIFACT_FAILURES=0 + COVERAGE_ARTIFACT=$(jq -r ' + [.value[]? | select(.name | test("Code Coverage Report")) | + .resource.downloadUrl | select(type == "string" and length > 0)] | + .[0] // empty' <<< "$ARTIFACTS_RESPONSE") + else + ARTIFACT_FAILURES=$((ARTIFACT_FAILURES + 1)) + echo "⚠️ Artifacts API HTTP/JSON error ($ARTIFACT_FAILURES/5)" + if (( ARTIFACT_FAILURES >= 5 )); then + echo "❌ Azure DevOps artifacts API unavailable after 5 consecutive failures" exit 1 fi - sleep 30 - continue fi - - # Show available artifacts for debugging - echo "🔍 Available artifacts:" - echo "$ARTIFACTS_RESPONSE" | jq -r '.value[]?.name // "No artifacts found"' - - # Find the coverage report artifact - COVERAGE_ARTIFACT=$(echo "$ARTIFACTS_RESPONSE" | jq -r '.value[]? | select(.name | test("Code Coverage Report")) | .resource.downloadUrl // empty' 2>/dev/null) - - if [[ -n "$COVERAGE_ARTIFACT" && "$COVERAGE_ARTIFACT" != "null" && "$COVERAGE_ARTIFACT" != "empty" ]]; then - echo "✅ Found coverage artifact on attempt $i!" - break + + # Inspect lifecycle, not aggregate result: independent matrix jobs can fail. + REQUEST_TIMEOUT=$((DEADLINE - SECONDS)) + if (( REQUEST_TIMEOUT <= 0 )); then break; fi + if (( REQUEST_TIMEOUT > 30 )); then REQUEST_TIMEOUT=30; fi + if BUILD_RESPONSE=$(curl --fail --silent --show-error --connect-timeout 10 --max-time "$REQUEST_TIMEOUT" "$BUILD_URL") && + jq -e --arg ID "$BUILD_ID" '(.id | tostring) == $ID and + (.status | type == "string")' <<< "$BUILD_RESPONSE" >/dev/null 2>&1; then + BUILD_FAILURES=0 + STATUS=$(jq -r '.status' <<< "$BUILD_RESPONSE") + RESULT=$(jq -r '.result // "unknown"' <<< "$BUILD_RESPONSE") + if [[ "$STATUS" == "completed" && "$RESULT" == "canceled" ]]; then + if REPLACEMENTS=$(curl --fail --silent --show-error --connect-timeout 10 --max-time "$REQUEST_TIMEOUT" "$BUILDS_URL") && + REPLACEMENT=$(jq -ce --arg PR "$PR_NUMBER" --arg SHA "$PR_HEAD_SHA" \ + --arg BRANCH "$PR_BRANCH" --arg ID "$BUILD_ID" ' + [.value[]? | select( + .definition.id == 2128 and .sourceBranch == $BRANCH and + (.triggerInfo["pr.number"] | tostring) == $PR and + .triggerInfo["pr.sourceSha"] == $SHA and + .id > ($ID | tonumber) and + (.status != "completed" or .result != "canceled") + )] | .[0]' <<< "$REPLACEMENTS"); then + BUILD_ID=$(jq -r '.id' <<< "$REPLACEMENT") + [[ "$BUILD_ID" =~ ^[0-9]+$ ]] || { + echo "Invalid replacement Azure DevOps build ID" + exit 1 + } + BUILD_URL="https://dev.azure.com/SqlClientDrivers/public/_apis/build/builds/$BUILD_ID?api-version=7.1-preview.7" + ARTIFACTS_URL="https://dev.azure.com/SqlClientDrivers/public/_apis/build/builds/$BUILD_ID/artifacts?api-version=7.1-preview.5" + ADO_URL="https://dev.azure.com/sqlclientdrivers/public/_build/results?buildId=$BUILD_ID" + echo "BUILD_ID=$BUILD_ID" >> "$GITHUB_ENV" + echo "ADO_URL=$ADO_URL" >> "$GITHUB_ENV" + COMPLETED_AT=-1 + ARTIFACT_FAILURES=0 + BUILD_FAILURES=0 + COVERAGE_ARTIFACT="" + COVERAGE_ARTIFACT_APPROVED=false + echo "Selected ADO run was canceled; continuing with replacement build $BUILD_ID" + continue + fi + ARTIFACT_FAILURES=0 + echo "Canceled build $BUILD_ID has no replacement yet..." + SLEEP_SECONDS=$((DEADLINE - SECONDS)) + if (( SLEEP_SECONDS > 30 )); then SLEEP_SECONDS=30; fi + if (( SLEEP_SECONDS > 0 )); then sleep "$SLEEP_SECONDS"; fi + continue + fi + if [[ -n "$COVERAGE_ARTIFACT" ]] && + { [[ "$STATUS" == "inProgress" ]] || + { [[ "$STATUS" == "completed" ]] && + [[ "$RESULT" =~ ^(succeeded|partiallySucceeded|failed)$ ]]; }; }; then + COVERAGE_ARTIFACT_APPROVED=true + echo "✅ Found coverage artifact!" + break + fi + if [[ "$STATUS" == "completed" ]] && (( COMPLETED_AT < 0 )); then + COMPLETED_AT=$SECONDS + echo "Build completed ($RESULT); allowing 2 minutes for artifact propagation..." + fi else - echo "⏳ Coverage report not ready yet (attempt $i/60)..." - if [[ $i -eq 60 ]]; then - echo "❌ Timeout: Coverage report artifact not found after 60 attempts" - echo "Available artifacts:" - echo "$ARTIFACTS_RESPONSE" | jq -r '.value[]?.name // "No artifacts found"' + BUILD_FAILURES=$((BUILD_FAILURES + 1)) + echo "⚠️ Build lifecycle API HTTP/JSON error ($BUILD_FAILURES/5)" + if (( BUILD_FAILURES >= 5 )); then + echo "❌ Azure DevOps build lifecycle API unavailable after 5 consecutive failures" exit 1 fi - sleep 30 fi + + if (( COMPLETED_AT >= 0 && SECONDS - COMPLETED_AT >= 120 )); then + echo "❌ Build $BUILD_ID completed but coverage artifact is still unavailable after propagation grace" + exit 1 + fi + echo "⏳ Coverage report not ready yet..." + SLEEP_SECONDS=$((DEADLINE - SECONDS)) + if (( SLEEP_SECONDS > 30 )); then SLEEP_SECONDS=30; fi + if (( SLEEP_SECONDS > 0 )); then sleep "$SLEEP_SECONDS"; fi done - + + if [[ "$COVERAGE_ARTIFACT_APPROVED" != true || -z "$COVERAGE_ARTIFACT" ]]; then + echo "❌ Timeout: Coverage report artifact not found within 220 minutes" + exit 1 + fi + if [[ -n "$COVERAGE_ARTIFACT" && "$COVERAGE_ARTIFACT" != "null" && "$COVERAGE_ARTIFACT" != "empty" ]]; then echo "📊 Downloading coverage report..." - if ! curl -L "$COVERAGE_ARTIFACT" -o coverage-report.zip --fail --silent; then + COVERAGE_ARCHIVE="$RUNNER_TEMP/coverage-report.zip" + if ! curl -L "$COVERAGE_ARTIFACT" -o "$COVERAGE_ARCHIVE" --fail --silent --show-error \ + --connect-timeout 10 --max-time 60 --max-filesize 268435456 \ + --retry 2 --retry-delay 5 --retry-max-time 180; then echo "❌ Failed to download coverage report from Azure DevOps" echo "This indicates the coverage artifacts may not be available or accessible" exit 1 fi - if ! unzip -o -q coverage-report.zip; then - echo "❌ Failed to extract coverage artifacts" - echo "Trying to extract with verbose output for debugging..." - unzip -l coverage-report.zip || echo "Failed to list archive contents" + INDEX_FILE="$RUNNER_TEMP/coverage-index.html" + if ! python .github/scripts/extract_coverage_artifact.py html "$COVERAGE_ARCHIVE" "$INDEX_FILE"; then + echo "❌ Failed to read the coverage HTML artifact" exit 1 fi - - # Find the main index.html file - INDEX_FILE=$(find . -name "index.html" -path "*/Code Coverage Report*" | head -1) - + if [[ -f "$INDEX_FILE" ]]; then echo "🔍 Parsing coverage data from $INDEX_FILE..." @@ -232,8 +315,6 @@ jobs: echo "✅ Coverage data extracted successfully" else echo "❌ Could not find index.html in coverage report" - echo "Available files in the coverage report:" - find . -name "*.html" | head -10 || echo "No HTML files found" exit 1 fi else @@ -252,15 +333,18 @@ jobs: echo "📥 Fetching artifacts for build $BUILD_ID to find coverage files..." - # Fetch artifacts with error handling - ARTIFACTS_RESPONSE=$(curl -s "$ARTIFACTS_URL") - - # Check if response is valid JSON - if ! echo "$ARTIFACTS_RESPONSE" | jq . >/dev/null 2>&1; then - echo "❌ Invalid JSON response from artifacts API" - echo "Response received: $ARTIFACTS_RESPONSE" - exit 1 - fi + for i in {1..5}; do + if ARTIFACTS_RESPONSE=$(curl --fail --silent --show-error --connect-timeout 10 --max-time 30 "$ARTIFACTS_URL") && + jq -e 'type == "object" and (.value | type == "array")' <<< "$ARTIFACTS_RESPONSE" >/dev/null 2>&1; then + break + fi + echo "⚠️ Artifacts API HTTP/JSON error ($i/5)" + if [[ $i -eq 5 ]]; then + echo "❌ Azure DevOps artifacts API unavailable after 5 attempts" + exit 1 + fi + sleep 5 + done echo "🔍 Available artifacts:" echo "$ARTIFACTS_RESPONSE" | jq -r '.value[]?.name // "No artifacts found"' @@ -270,44 +354,21 @@ jobs: if [[ -n "$COVERAGE_XML_ARTIFACT" && "$COVERAGE_XML_ARTIFACT" != "null" && "$COVERAGE_XML_ARTIFACT" != "empty" ]]; then echo "📊 Downloading coverage artifact from: $COVERAGE_XML_ARTIFACT" - if ! curl -L "$COVERAGE_XML_ARTIFACT" -o coverage-artifacts.zip --fail --silent; then + COVERAGE_XML_ARCHIVE="$RUNNER_TEMP/coverage-artifacts.zip" + if ! curl -L "$COVERAGE_XML_ARTIFACT" -o "$COVERAGE_XML_ARCHIVE" --fail --silent --show-error \ + --connect-timeout 10 --max-time 60 --max-filesize 268435456 \ + --retry 2 --retry-delay 5 --retry-max-time 180; then echo "❌ Failed to download coverage artifacts" exit 1 fi - if ! unzip -o -q coverage-artifacts.zip; then - echo "❌ Failed to extract coverage artifacts" - echo "Trying to extract with verbose output for debugging..." - unzip -l coverage-artifacts.zip || echo "Failed to list archive contents" + COVERAGE_XML="$RUNNER_TEMP/coverage.xml" + if ! python .github/scripts/extract_coverage_artifact.py xml "$COVERAGE_XML_ARCHIVE" "$COVERAGE_XML"; then + echo "❌ Failed to read the coverage XML artifact" exit 1 fi - - echo "🔍 Looking for coverage XML files in extracted artifacts..." - find . -name "*.xml" -type f | head -10 - - # Look for the main coverage.xml file in unified-coverage directory or any coverage XML - if [[ -f "unified-coverage/coverage.xml" ]]; then - echo "✅ Found unified coverage file at unified-coverage/coverage.xml" - cp "unified-coverage/coverage.xml" ./coverage.xml - elif [[ -f "coverage.xml" ]]; then - echo "✅ Found coverage.xml in root directory" - # Already in the right place - else - # Try to find any coverage XML file - COVERAGE_FILE=$(find . -name "*coverage*.xml" -type f | head -1) - if [[ -n "$COVERAGE_FILE" ]]; then - echo "✅ Found coverage file: $COVERAGE_FILE" - cp "$COVERAGE_FILE" ./coverage.xml - else - echo "❌ No coverage XML file found in artifacts" - echo "Available files:" - find . -name "*.xml" -type f - exit 1 - fi - fi - - echo "✅ Coverage XML file is ready at ./coverage.xml" - ls -la ./coverage.xml + echo "✅ Coverage XML file is ready at $COVERAGE_XML" + ls -la "$COVERAGE_XML" else echo "❌ Could not find coverage artifacts" echo "This indicates the Azure DevOps CodeCoverageReport job may not have run successfully" @@ -315,22 +376,24 @@ jobs: fi - name: Generate patch coverage report + env: + COVERAGE_XML: ${{ runner.temp }}/coverage.xml run: | # Install dependencies pip install diff-cover jq sudo apt-get update && sudo apt-get install -y libxml2-utils # Verify coverage.xml exists before proceeding - if [[ ! -f coverage.xml ]]; then + if [[ ! -f "$COVERAGE_XML" ]]; then echo "❌ coverage.xml not found in current directory" echo "Available files:" ls -la | head -20 exit 1 fi - echo "✅ coverage.xml found, size: $(wc -c < coverage.xml) bytes" + echo "✅ coverage.xml found, size: $(wc -c < "$COVERAGE_XML") bytes" echo "🔍 Coverage file preview (first 10 lines):" - head -10 coverage.xml + head -10 "$COVERAGE_XML" # Generate diff coverage report using the new command format echo "🚀 Generating patch coverage report..." @@ -354,27 +417,27 @@ jobs: # Debug: Check coverage.xml content for specific files echo "🔍 Coverage.xml analysis:" echo "Python files mentioned in coverage.xml:" - grep -o 'filename="[^"]*\.py"' coverage.xml | head -10 || echo "Could not extract filenames" + grep -o 'filename="[^"]*\.py"' "$COVERAGE_XML" | head -10 || echo "Could not extract filenames" echo "Sample coverage data:" - head -20 coverage.xml + head -20 "$COVERAGE_XML" # Use the new format for diff-cover commands echo "🚀 Running diff-cover..." - diff-cover coverage.xml \ + diff-cover "$COVERAGE_XML" \ --compare-branch=main \ --html-report patch-coverage.html \ --json-report patch-coverage.json \ --markdown-report patch-coverage.md || { echo "❌ diff-cover failed with exit code $?" echo "Checking if coverage.xml is valid XML..." - if ! xmllint --noout coverage.xml 2>/dev/null; then + if ! xmllint --noout "$COVERAGE_XML" 2>/dev/null; then echo "❌ coverage.xml is not valid XML" echo "First 50 lines of coverage.xml:" - head -50 coverage.xml + head -50 "$COVERAGE_XML" else echo "✅ coverage.xml is valid XML" echo "🔍 diff-cover verbose output:" - diff-cover coverage.xml --compare-branch=main --markdown-report debug-patch-coverage.md -v || echo "Verbose diff-cover also failed" + diff-cover "$COVERAGE_XML" --compare-branch=main --markdown-report debug-patch-coverage.md -v || echo "Verbose diff-cover also failed" fi # Don't exit here, let's see what files were created } diff --git a/.github/workflows/pr-profiler-report.yml b/.github/workflows/pr-profiler-report.yml new file mode 100644 index 000000000..d14dd5770 --- /dev/null +++ b/.github/workflows/pr-profiler-report.yml @@ -0,0 +1,34 @@ +name: PR Performance Report + +# Privileged reporting only. No PR checkout, builds, or artifact execution here. +on: + pull_request_target: + branches: [main] + types: [opened, synchronize, reopened, ready_for_review] + +permissions: + contents: read + pull-requests: write + +concurrency: + group: profiler-report-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + report: + runs-on: ubuntu-latest + timeout-minutes: 230 + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + with: + ref: ${{ github.event.pull_request.base.sha }} + persist-credentials: false + - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 + with: + python-version: "3.13" + - name: Publish validated paired benchmark results + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.pull_request.number }} + PR_HEAD: ${{ github.event.pull_request.head.sha }} + run: python .github/scripts/post_profiler_comment.py --pr "$PR_NUMBER" --head "$PR_HEAD" diff --git a/benchmarks/README.md b/benchmarks/README.md index ce0480057..4a79375bd 100644 --- a/benchmarks/README.md +++ b/benchmarks/README.md @@ -8,7 +8,15 @@ This directory contains benchmark scripts for testing the performance of various Comprehensive benchmarks using the richbench framework for detailed performance analysis. ### 2. `perf-benchmarking.py` - Real-World Query Benchmarks -Standalone script that tests real-world queries against AdventureWorks2022 database with statistical analysis. + +Direct `pyodbc` and `mssql_python` comparisons against AdventureWorks2022. + +### 3. Profiler benchmark comparisons + +Profiler benchmarks are engineering infrastructure, separate from these standalone +scripts and from the runtime profiler. See +[`eng/profiler_benchmarks/README.md`](../eng/profiler_benchmarks/README.md). +Their reviewer-facing output is the impact-first **PR Performance Report**. ## Why Benchmarks? - To measure the efficiency of `pyodbc` and `mssql_python` in handling database operations. diff --git a/eng/pipelines/pr-validation-pipeline.yml b/eng/pipelines/pr-validation-pipeline.yml index d0d3311d1..6e5038d89 100644 --- a/eng/pipelines/pr-validation-pipeline.yml +++ b/eng/pipelines/pr-validation-pipeline.yml @@ -51,6 +51,7 @@ jobs: - job: pytestonwindows displayName: 'Windows x64' + timeoutInMinutes: 160 pool: vmImage: 'windows-latest' @@ -67,6 +68,8 @@ jobs: pythonVersion: '3.14' steps: + - checkout: self + fetchDepth: 0 - task: UsePythonVersion@0 inputs: versionSpec: '$(pythonVersion)' @@ -235,10 +238,23 @@ jobs: env: DB_PASSWORD: $(DB_PASSWORD) + - script: | + cd mssql_python\pybind + build.bat x64 + displayName: 'Build profiling .pyd file' + condition: and(succeeded(), eq(variables['Build.Reason'], 'PullRequest'), ne(variables['sqlVersion'], 'LocalDB')) + env: + ENABLE_PROFILING: 1 + - script: | cd mssql_python\pybind build.bat x64 displayName: 'Build .pyd file' + condition: and(succeeded(), or(ne(variables['Build.Reason'], 'PullRequest'), eq(variables['sqlVersion'], 'LocalDB'))) + + - script: python -m eng.profiler_benchmarks.controller --check-build on + displayName: 'Verify native configuration and recording OFF before pytest' + condition: and(succeeded(), eq(variables['Build.Reason'], 'PullRequest'), ne(variables['sqlVersion'], 'LocalDB')) - template: steps/install-mssql-py-core.yml parameters: @@ -306,33 +322,6 @@ jobs: env: DB_PASSWORD: $(DB_PASSWORD) - # Download baseline from latest main run (for PR comparison) - - task: DownloadPipelineArtifact@2 - inputs: - source: specific - project: $(System.TeamProjectId) - pipeline: $(System.DefinitionId) - runVersion: latestFromBranch - runBranch: refs/heads/main - artifact: 'perf-baseline-$(sqlVersion)' - path: $(Build.SourcesDirectory) - displayName: 'Download baseline from main' - condition: and(succeeded(), ne(variables['Build.SourceBranch'], 'refs/heads/main'), or(eq(variables['sqlVersion'], 'SQL2022'), eq(variables['sqlVersion'], 'SQL2025'))) - continueOnError: true - - # Rename downloaded baseline so the script finds it (artifact may be in a subfolder) - - powershell: | - $found = Get-ChildItem -Path "$(Build.SourcesDirectory)" -Filter "benchmark_results.json" -Recurse -File -ErrorAction SilentlyContinue | Select-Object -First 1 - if ($null -ne $found) { - Copy-Item $found.FullName "benchmark_baseline.json" -Force - Write-Host "Baseline file ready: benchmark_baseline.json (from $($found.FullName))" - } else { - Write-Host "No baseline file downloaded (first run or artifact missing)" - } - displayName: 'Prepare baseline file' - condition: and(succeeded(), ne(variables['Build.SourceBranch'], 'refs/heads/main'), or(eq(variables['sqlVersion'], 'SQL2022'), eq(variables['sqlVersion'], 'SQL2025'))) - continueOnError: true - # Run performance benchmarks on SQL Server 2022 - powershell: | Write-Host "Checking and installing ODBC Driver 18 for SQL Server..." @@ -416,24 +405,23 @@ jobs: exit 1 } - Write-Host "`nInstalling pyodbc..." - pip install pyodbc - - Write-Host "`nRunning performance benchmarks..." - python benchmarks/perf-benchmarking.py --baseline benchmark_baseline.json --json benchmark_results.json - displayName: 'Run performance benchmarks on SQL Server 2022/2025' - condition: and(succeeded(), or(eq(variables['sqlVersion'], 'SQL2022'), eq(variables['sqlVersion'], 'SQL2025'))) + python -m eng.profiler_benchmarks.controller --reuse-candidate --leg "Windows-$(sqlVersion)" --output profiler-results + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + displayName: 'Compare profiling builds on SQL Server 2022/2025' + condition: and(succeeded(), eq(variables['Build.Reason'], 'PullRequest'), or(eq(variables['sqlVersion'], 'SQL2022'), eq(variables['sqlVersion'], 'SQL2025'))) continueOnError: true + timeoutInMinutes: 100 env: + SYSTEM_PULLREQUEST_SOURCECOMMITID: $(System.PullRequest.SourceCommitId) DB_CONNECTION_STRING: 'Server=localhost;Database=AdventureWorks2022;Uid=sa;Pwd=$(DB_PASSWORD);TrustServerCertificate=yes' - # Publish benchmark results as artifact (consumed as baseline by future PR runs) + # Publish partial reports and failure logs for PR assessment. - task: PublishPipelineArtifact@1 inputs: - targetPath: benchmark_results.json - artifact: 'perf-baseline-$(sqlVersion)' - displayName: 'Publish benchmark baseline' - condition: and(succeeded(), eq(variables['Build.SourceBranch'], 'refs/heads/main'), or(eq(variables['sqlVersion'], 'SQL2022'), eq(variables['sqlVersion'], 'SQL2025'))) + targetPath: profiler-results + artifact: 'profiler-Windows-$(sqlVersion)' + displayName: 'Publish paired profiler measurements' + condition: and(succeededOrFailed(), eq(variables['Build.Reason'], 'PullRequest'), or(eq(variables['sqlVersion'], 'SQL2022'), eq(variables['sqlVersion'], 'SQL2025'))) continueOnError: true - task: CopyFiles@2 @@ -450,12 +438,21 @@ jobs: TargetFolder: '$(Build.ArtifactStagingDirectory)' displayName: 'Copy pdb file to staging' + - task: PublishBuildArtifacts@1 + inputs: + PathtoPublish: '$(Build.ArtifactStagingDirectory)' + ArtifactName: 'ddbc_bindings-profiling-$(sqlVersion)' + publishLocation: 'Container' + displayName: 'Publish profiling build artifacts' + condition: and(succeeded(), eq(variables['Build.Reason'], 'PullRequest'), ne(variables['sqlVersion'], 'LocalDB')) + - task: PublishBuildArtifacts@1 inputs: PathtoPublish: '$(Build.ArtifactStagingDirectory)' ArtifactName: 'ddbc_bindings' publishLocation: 'Container' displayName: 'Publish build artifacts' + condition: and(succeeded(), or(ne(variables['Build.Reason'], 'PullRequest'), eq(variables['sqlVersion'], 'LocalDB'))) - task: PublishTestResults@2 condition: succeededOrFailed() @@ -471,11 +468,9 @@ jobs: - job: PytestOnMacOS displayName: 'macOS x86_64' - # Colima + SQL Server container setup averages ~12.5 min but has a long tail - # (setup has been observed at 17-39 min). The ADO default job timeout is 60 min, - # which the setup tail plus tests plus the 20-min benchmark step can exceed, - # getting the job killed mid-step. Give the job enough headroom for the tail. - timeoutInMinutes: 90 + # Reserve 60 minutes outside the 100-minute benchmark step for Colima/SQL + # setup, pytest, fixture restore and artifact publication. + timeoutInMinutes: 160 pool: vmImage: 'macos-latest' @@ -501,6 +496,9 @@ jobs: pythonVersion: '3.14' steps: + - checkout: self + fetchDepth: 0 + - task: UsePythonVersion@0 inputs: versionSpec: '$(pythonVersion)' @@ -570,7 +568,11 @@ jobs: pip install -r requirements.txt echo "Building pybind bindings (.so) (overlapped with container setup)..." - ( cd mssql_python/pybind && ./build.sh ) + PROFILER_BUILD=0 + if [ "$(Build.Reason)" = "PullRequest" ]; then + PROFILER_BUILD=1 + fi + ( cd mssql_python/pybind && ENABLE_PROFILING="$PROFILER_BUILD" ./build.sh ) echo "Waiting for container setup (Colima + SQL Server) to finish..." SQL_STATUS=0 @@ -583,6 +585,10 @@ jobs: env: DB_PASSWORD: $(DB_PASSWORD) + - script: python -m eng.profiler_benchmarks.controller --check-build on + displayName: 'Verify native configuration and recording OFF before pytest' + condition: and(succeeded(), eq(variables['Build.Reason'], 'PullRequest')) + - template: steps/install-mssql-py-core.yml parameters: platform: unix @@ -659,56 +665,30 @@ jobs: env: DB_PASSWORD: $(DB_PASSWORD) - # Download macOS baseline from latest main run (for PR comparison) - - task: DownloadPipelineArtifact@2 - inputs: - source: specific - project: $(System.TeamProjectId) - pipeline: $(System.DefinitionId) - runVersion: latestFromBranch - runBranch: refs/heads/main - artifact: 'perf-baseline-macOS-$(sqlVersion)' - path: $(Build.SourcesDirectory) - displayName: 'Download macOS baseline from main' - condition: and(succeeded(), ne(variables['Build.SourceBranch'], 'refs/heads/main'), or(eq(variables['sqlVersion'], 'SQL2022'), eq(variables['sqlVersion'], 'SQL2025'))) - continueOnError: true - - - script: | - found=$(find "$(Build.SourcesDirectory)" -name benchmark_results.json -type f 2>/dev/null | head -1) - if [ -n "$found" ]; then - cp "$found" benchmark_baseline.json - echo "Baseline file ready: benchmark_baseline.json (from $found)" - else - echo "No baseline file downloaded (first run or artifact missing)" - fi - displayName: 'Prepare macOS baseline file' - condition: and(succeeded(), ne(variables['Build.SourceBranch'], 'refs/heads/main'), or(eq(variables['sqlVersion'], 'SQL2022'), eq(variables['sqlVersion'], 'SQL2025'))) - continueOnError: true - # Run performance benchmarks on macOS - script: | - echo "Installing ODBC Driver 18 for pyodbc..." + set -euo pipefail + echo "Restoring build dependencies for isolated profiling builds..." brew tap microsoft/mssql-release https://github.com/Microsoft/homebrew-mssql-release # Newer Homebrew refuses to load formulae from third-party taps unless the tap is trusted brew trust microsoft/mssql-release || echo "brew trust failed; attempting install anyway" - HOMEBREW_ACCEPT_EULA=Y brew install msodbcsql18 || echo "ODBC Driver 18 install failed — pyodbc benchmarks will be skipped" - pip install pyodbc - echo "Running performance benchmarks..." - python benchmarks/perf-benchmarking.py --baseline benchmark_baseline.json --json benchmark_results.json - displayName: 'Run performance benchmarks on macOS $(sqlVersion)' - condition: and(succeeded(), or(eq(variables['sqlVersion'], 'SQL2022'), eq(variables['sqlVersion'], 'SQL2025'))) - timeoutInMinutes: 20 + HOMEBREW_ACCEPT_EULA=Y brew install msodbcsql18 + python -m eng.profiler_benchmarks.controller --reuse-candidate --leg "macOS-$(sqlVersion)" --output profiler-results + displayName: 'Compare profiling builds on macOS $(sqlVersion)' + condition: and(succeeded(), eq(variables['Build.Reason'], 'PullRequest'), or(eq(variables['sqlVersion'], 'SQL2022'), eq(variables['sqlVersion'], 'SQL2025'))) + timeoutInMinutes: 100 continueOnError: true env: + SYSTEM_PULLREQUEST_SOURCECOMMITID: $(System.PullRequest.SourceCommitId) DB_CONNECTION_STRING: 'Server=tcp:127.0.0.1,1433;Database=AdventureWorks2022;Uid=SA;Pwd=$(DB_PASSWORD);TrustServerCertificate=yes' - # Publish benchmark results as artifact on main merges + # Both revisions and samples travel together; no historical baseline lookup. - task: PublishPipelineArtifact@1 inputs: - targetPath: benchmark_results.json - artifact: 'perf-baseline-macOS-$(sqlVersion)' - displayName: 'Publish macOS benchmark baseline' - condition: and(succeeded(), eq(variables['Build.SourceBranch'], 'refs/heads/main'), or(eq(variables['sqlVersion'], 'SQL2022'), eq(variables['sqlVersion'], 'SQL2025'))) + targetPath: profiler-results + artifact: 'profiler-macOS-$(sqlVersion)' + displayName: 'Publish paired profiler measurements' + condition: and(succeededOrFailed(), eq(variables['Build.Reason'], 'PullRequest'), or(eq(variables['sqlVersion'], 'SQL2022'), eq(variables['sqlVersion'], 'SQL2025'))) continueOnError: true - script: | @@ -722,6 +702,7 @@ jobs: - job: PytestOnLinux displayName: 'Linux x86_64' + timeoutInMinutes: 160 pool: vmImage: 'ubuntu-latest' @@ -755,6 +736,9 @@ jobs: useAzureSQL: 'false' steps: + - checkout: self + fetchDepth: 0 + - script: | # Create a Docker container for testing docker run -d --name test-container-$(distroName) \ @@ -847,12 +831,25 @@ jobs: - script: | # Build pybind bindings in the container - docker exec test-container-$(distroName) bash -c " + PROFILER_BUILD=0 + if [ "$(Build.Reason)" = "PullRequest" ] && [ "$(distroName)" = "Ubuntu" ]; then + PROFILER_BUILD=1 + fi + docker exec -e ENABLE_PROFILING="$PROFILER_BUILD" test-container-$(distroName) bash -c " + set -e source /opt/venv/bin/activate cd mssql_python/pybind chmod +x build.sh ./build.sh " + if [ "$PROFILER_BUILD" = "1" ]; then + docker exec test-container-$(distroName) bash -c " + set -e + source /opt/venv/bin/activate + cd /workspace + python -m eng.profiler_benchmarks.controller --check-build on + " + fi displayName: 'Build pybind bindings (.so) in $(distroName) container' - template: steps/install-mssql-py-core.yml @@ -896,7 +893,7 @@ jobs: " else # Local SQL Server testing - SQLSERVER_IP=$(docker inspect sqlserver-$(distroName) --format='{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}') + export SQLSERVER_IP=$(docker inspect sqlserver-$(distroName) --format='{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}') echo "SQL Server IP: $SQLSERVER_IP" docker exec \ @@ -950,59 +947,47 @@ jobs: - script: | # Run performance benchmarks on Ubuntu with SQL Server 2022 only if [ "$(distroName)" = "Ubuntu" ] && [ "$(useAzureSQL)" = "false" ]; then - SQLSERVER_IP=$(docker inspect sqlserver-$(distroName) --format='{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}') + export SQLSERVER_IP=$(docker inspect sqlserver-$(distroName) --format='{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}') echo "Running performance benchmarks on Ubuntu with SQL Server IP: $SQLSERVER_IP" docker exec \ - -e DB_CONNECTION_STRING="Server=$SQLSERVER_IP;Database=AdventureWorks2022;Uid=SA;Pwd=$(DB_PASSWORD);TrustServerCertificate=yes" \ - test-container-$(distroName) bash -c " + -e BUILD_BUILDID \ + -e SYSTEM_PULLREQUEST_SOURCECOMMITID \ + -e DB_PASSWORD \ + -e SQLSERVER_IP \ + test-container-$(distroName) bash -c ' + set -euo pipefail source /opt/venv/bin/activate - - echo 'Reinstalling ODBC Driver for benchmarking...' + export DB_CONNECTION_STRING="Server=$SQLSERVER_IP;Database=AdventureWorks2022;Uid=SA;Pwd=$DB_PASSWORD;Encrypt=no;TrustServerCertificate=yes" export DEBIAN_FRONTEND=noninteractive - - # Remove duplicate repository sources if they exist - rm -f /etc/apt/sources.list.d/microsoft-prod.list - - # Add Microsoft repository - curl -sSL https://packages.microsoft.com/keys/microsoft.asc | apt-key add - - curl -sSL https://packages.microsoft.com/config/ubuntu/24.04/prod.list > /etc/apt/sources.list.d/mssql-release.list - - # Update package lists + # The Microsoft repository was configured by the earlier build step. + # Restore the ODBC headers and library link removed before pytest. apt-get update -qq - - # Install unixodbc and its dependencies first (provides libodbcinst.so.2 needed by msodbcsql18) - echo 'Installing unixODBC dependencies...' - apt-get install -y --no-install-recommends unixodbc unixodbc-dev libodbc1 odbcinst odbcinst1debian2 - - # Verify libodbcinst.so.2 is available - ldconfig - ls -la /usr/lib/x86_64-linux-gnu/libodbcinst.so.2 || echo 'Warning: libodbcinst.so.2 not found' - - # Install ODBC Driver 18 - echo 'Installing msodbcsql18...' - ACCEPT_EULA=Y apt-get install -y msodbcsql18 - - # Verify ODBC driver installation - odbcinst -q -d -n 'ODBC Driver 18 for SQL Server' || echo 'Warning: ODBC Driver 18 not registered' - - echo 'Installing pyodbc for benchmarking...' - pip install pyodbc - echo 'Running performance benchmarks on $(distroName)' - if [ -f benchmark_baseline.json ]; then - python benchmarks/perf-benchmarking.py --baseline benchmark_baseline.json --json benchmark_results.json || echo 'Performance benchmark failed or database not available' - else - python benchmarks/perf-benchmarking.py --json benchmark_results.json || echo 'Performance benchmark failed or database not available' - fi - " + apt-get install -y --reinstall libodbcinst2 + ACCEPT_EULA=Y apt-get install -y --no-install-recommends git unixodbc unixodbc-dev libodbc2 libodbcinst2 odbcinst msodbcsql18 + git config --global --add safe.directory /workspace + odbcinst -q -d -n "ODBC Driver 18 for SQL Server" + python -m eng.profiler_benchmarks.controller --reuse-candidate --leg Linux-SQL2022 --output profiler-results + ' else echo "Skipping performance benchmarks on $(distroName) (only runs on Ubuntu with local SQL Server)" fi - displayName: 'Run performance benchmarks in $(distroName) container' - condition: and(succeeded(), eq(variables['distroName'], 'Ubuntu'), eq(variables['useAzureSQL'], 'false')) + displayName: 'Compare profiling builds in $(distroName) container' + condition: and(succeeded(), eq(variables['Build.Reason'], 'PullRequest'), eq(variables['distroName'], 'Ubuntu'), eq(variables['useAzureSQL'], 'false')) continueOnError: true + timeoutInMinutes: 100 env: DB_PASSWORD: $(DB_PASSWORD) + BUILD_BUILDID: $(Build.BuildId) + SYSTEM_PULLREQUEST_SOURCECOMMITID: $(System.PullRequest.SourceCommitId) + + - task: PublishPipelineArtifact@1 + inputs: + targetPath: profiler-results + artifact: profiler-Linux-SQL2022 + displayName: 'Publish paired profiler measurements' + condition: and(succeededOrFailed(), eq(variables['Build.Reason'], 'PullRequest'), eq(variables['distroName'], 'Ubuntu'), eq(variables['useAzureSQL'], 'false')) + continueOnError: true - script: | # Copy test results from container to host diff --git a/eng/profiler_benchmarks/README.md b/eng/profiler_benchmarks/README.md new file mode 100644 index 000000000..490802036 --- /dev/null +++ b/eng/profiler_benchmarks/README.md @@ -0,0 +1,38 @@ +# Profiler Benchmarks + +This source-only package powers the impact-first **PR Performance Report**. It is +separate from the runtime profiler in `profiler/` and standalone `benchmarks/`. + +## Local use + +```bash +# Requires build dependencies, pyarrow, and an AdventureWorks2022 connection. +python -m eng.profiler_benchmarks.controller --base main --candidate HEAD \ + --leg Linux-SQL2022 --output profiler-results +python -m eng.profiler_benchmarks.report profiler-results/report.json +``` + +The fixed registry has 20 tasks. `--scenarios` runs a local subset, but subset +reports remain incomplete and cannot produce a verdict. + +## Measurement contract + +CI uses the PR merge's first parent as the exact base. It reuses the +profiling-enabled candidate build after pytest and builds the base separately. +Fresh processes run five measured pairs after one warmup pair in alternating order. + +Workers have six minutes each. CI allows 90 minutes inside a 100-minute step and +160-minute job; local runs receive 105 minutes because they build both revisions. +Partial results never produce a verdict. + +## Publication + +Five environments publish raw samples: Windows and macOS on SQL Server 2022/2025, +and Ubuntu on SQL Server 2022. The privileged publisher runs trusted base code, +authenticates benchmark producers, validates bounded artifacts, and ignores stale +heads. A failed aggregate build can still publish when its authenticated artifacts +validate. Missing, malformed, canceled, incomplete, or invalid data remains unavailable. + +The publisher waits up to 220 minutes inside a 230-minute workflow. The first main +comparison after introduction may be incomplete because its parent lacks this +infrastructure. A fresh ADO-only retry also requires rerunning the GitHub publisher. diff --git a/eng/profiler_benchmarks/__init__.py b/eng/profiler_benchmarks/__init__.py new file mode 100644 index 000000000..a3a7b9735 --- /dev/null +++ b/eng/profiler_benchmarks/__init__.py @@ -0,0 +1 @@ +"""Paired profiler benchmark measurement and reporting.""" diff --git a/eng/profiler_benchmarks/controller.py b/eng/profiler_benchmarks/controller.py new file mode 100644 index 000000000..97376d1c2 --- /dev/null +++ b/eng/profiler_benchmarks/controller.py @@ -0,0 +1,345 @@ +"""Build and measure base/candidate in isolated directories on the same CI agent.""" + +import argparse +import contextlib +import faulthandler +import importlib.util +import io +import json +import os +from pathlib import Path, PurePosixPath +import platform +import re +import signal +import subprocess +import sys +import tarfile +import tempfile +import time + +from .report import LEGS, suite_hash +from . import workloads + +ROOT = Path(__file__).resolve().parents[2] +SHA = re.compile(r"[0-9a-f]{40}") +# Twelve six-minute passes plus a 15-minute base build and preflight need 88 +# minutes. Local runs build both revisions and receive another 15 minutes. +BENCHMARK_TIMEOUT = 90 * 60 +LOCAL_BENCHMARK_TIMEOUT = 105 * 60 +WORKER_TIMEOUT = 6 * 60 +WINDOWS = os.name == "nt" + + +def git(*args): + return subprocess.check_output(["git", "-C", str(ROOT), *args], text=True).strip() + + +def resolve_revisions(base, candidate): + candidate = git("rev-parse", "--verify", "--end-of-options", f"{candidate}^{{commit}}") + # ADO validates refs/pull/N/merge. Its first parent is the exact target snapshot, + # not whichever main build happened to finish most recently. + base = git( + "rev-parse", "--verify", "--end-of-options", f"{base or candidate + '^1'}^{{commit}}" + ) + return base, candidate + + +def checkout(revision, path): + with tempfile.TemporaryFile() as archive: + subprocess.run(["git", "-C", str(ROOT), "archive", revision], stdout=archive, check=True) + archive.seek(0) + with tarfile.open(fileobj=archive) as tar: + if sys.version_info >= (3, 12): + tar.extractall(path, filter="data") + return + members = tar.getmembers() + for member in members: + member_path = PurePosixPath(member.name) + if ( + not member.name + or member_path.is_absolute() + or ".." in member_path.parts + or "\\" in member.name + or re.match(r"^[A-Za-z]:", member.name) + or not (member.isfile() or member.isdir()) + ): + raise ValueError("Unsafe git archive member") + tar.extractall(path, members=members) + + +def terminate_process_tree(process): + if WINDOWS: + result = subprocess.run( + ["taskkill", "/PID", str(process.pid), "/T", "/F"], + capture_output=True, + text=True, + ) + if result.returncode: + if process.poll() is not None: + return + process.kill() + process.wait() + raise RuntimeError(f"Failed to terminate build process tree: {result.stdout.strip()}") + process.wait(timeout=5) + return + + try: + os.killpg(process.pid, signal.SIGKILL) + except ProcessLookupError: + pass + process.wait(timeout=5) + + +def build(path, log, timeout=900): + env = dict(os.environ, ENABLE_PROFILING="1") + # build scripts find Python via PATH; keep the controller's interpreter. + env["PATH"] = str(Path(sys.executable).parent) + os.pathsep + env["PATH"] + command = ["cmd", "/c", "build.bat"] if os.name == "nt" else ["bash", "build.sh"] + with log.open("w", encoding="utf-8") as output: + process = subprocess.Popen( + command, + cwd=path / "mssql_python/pybind", + env=env, + stdout=output, + stderr=subprocess.STDOUT, + start_new_session=not WINDOWS, + creationflags=subprocess.CREATE_NEW_PROCESS_GROUP if WINDOWS else 0, + ) + try: + returncode = process.wait(timeout=timeout) + except subprocess.TimeoutExpired: + terminate_process_tree(process) + raise + if returncode: + raise subprocess.CalledProcessError(returncode, command) + + +def check_build(source_root, profiling): + sys.path.insert(0, str(source_root)) + import mssql_python + import mssql_python_odbc + from mssql_python import ddbc_bindings, perf_timer + + for module in (mssql_python, ddbc_bindings, mssql_python_odbc): + if not Path(module.__file__).resolve().is_relative_to(source_root.resolve()): + raise RuntimeError("Imported driver outside the selected checkout") + if hasattr(ddbc_bindings, "profiling") != profiling: + raise RuntimeError("Native profiling build configuration mismatch") + if perf_timer.is_enabled() or (profiling and ddbc_bindings.profiling.is_enabled()): + raise RuntimeError("Profiling must default to recording OFF") + + +def load_suite(): + # Load only the common profiler package by path. Keep the revision checkout + # first on sys.path so lazy provider imports cannot select candidate binaries. + spec = importlib.util.spec_from_file_location( + "profiler", + ROOT / "profiler/__init__.py", + submodule_search_locations=[str(ROOT / "profiler")], + ) + module = importlib.util.module_from_spec(spec) + sys.modules["profiler"] = module + spec.loader.exec_module(module) + import profiler.core as core + + return core, workloads + + +def worker(args): + # Import the chosen driver FIRST, then the SAME workload/controller for both + # revisions. Never mix two native extensions into one interpreter. + check_build(args.source_root, profiling=True) + core, workloads = load_suite() + + cases = workloads.registry() + chosen = args.scenarios or list(cases) + if set(chosen) - set(cases): + raise ValueError("Unknown benchmark scenario") + core.SCENARIOS = cases + # The runner owns enable/disable/cleanup, just as in the documented CLI. + with core.Profiler() as profiler: + output = {} + for name in chosen: + print(f"Starting scenario: {name}", flush=True) + args.output.write_text( + json.dumps(dict(status="running", active_scenario=name, scenarios=output)), + encoding="utf-8", + ) + # Keep phase tables out of logs, but never hide which workload stalled. + with contextlib.redirect_stdout(io.StringIO()): + result = profiler.run(name)[0] + if result["cpp"] is None or result["py"] is None: + raise RuntimeError(f"Scenario {name} was skipped") + if not result["cpp"]: + raise RuntimeError(f"Scenario {name} has no native samples") + output[name] = {key: result[key] for key in ("wall_ms", "cpp", "py")} + output[name]["work"] = result.get("detail", "Connection: 1").split(" (")[0] + args.output.write_text( + json.dumps( + dict(status="running", active_scenario=None, scenarios=output), allow_nan=False + ), + encoding="utf-8", + ) + print(f"Completed scenario: {name} ({result['wall_ms']:.3f} ms)", flush=True) + print("Collecting server metadata", flush=True) + profiler._ensure_connection() + with profiler._conn.cursor() as cursor: + cursor.execute("SELECT CAST(SERVERPROPERTY('ProductVersion') AS VARCHAR(80))") + sql_version = cursor.fetchone()[0] + environment = dict( + os=platform.system(), + architecture=platform.machine().lower(), + python=platform.python_version(), + sql_version=sql_version, + ) + args.output.write_text( + json.dumps(dict(environment=environment, scenarios=output), allow_nan=False), + encoding="utf-8", + ) + + +def measure(path, output, scenarios, timeout=WORKER_TIMEOUT): + command = [ + sys.executable, + "-u", + "-m", + "eng.profiler_benchmarks.controller", + "--worker", + "--source-root", + str(path), + "--output", + str(output), + ] + if scenarios: + command += ["--scenarios", *scenarios] + output.unlink(missing_ok=True) + with output.with_suffix(".log").open("w", encoding="utf-8") as log: + subprocess.run(command, stdout=log, stderr=subprocess.STDOUT, timeout=timeout, check=True) + return json.loads(output.read_text(encoding="utf-8")) + + +def remaining(deadline, limit): + seconds = deadline - time.monotonic() + if seconds <= 0: + raise TimeoutError("Profiler benchmarks exhausted their overall build/measurement budget") + return min(seconds, limit) + + +def run(args): + base, candidate = resolve_revisions(args.base, args.candidate) + args.output.mkdir(parents=True, exist_ok=True) + report_path = args.output / "report.json" + report_path.unlink(missing_ok=True) + head = os.environ.get("SYSTEM_PULLREQUEST_SOURCECOMMITID", candidate) + if not SHA.fullmatch(head): + head = candidate + report = dict( + schema_version=1, + status="incomplete", + leg=args.leg, + base_commit=base, + source_commit=candidate, + head_commit=head, + build_id=int(os.environ.get("BUILD_BUILDID", "0")), + suite_hash=suite_hash(ROOT), + samples=args.samples, + warmups=args.warmups, + pairs=[], + ) + report_path.write_text(json.dumps(report), encoding="utf-8") + timeout = BENCHMARK_TIMEOUT if args.reuse_candidate else LOCAL_BENCHMARK_TIMEOUT + deadline = time.monotonic() + timeout + # CI reuses the profiling build already exercised by pytest. The base always + # has its own checkout and process. Local runs can build both sides instead. + with tempfile.TemporaryDirectory(prefix="profiler-ci-") as directory: + paths = {side: Path(directory) / side for side in ("base", "candidate")} + for side, revision in (("base", base), ("candidate", candidate)): + if side == "candidate" and args.reuse_candidate: + if candidate != git("rev-parse", "HEAD"): + raise ValueError("--reuse-candidate requires candidate to be checkout HEAD") + paths[side] = ROOT + subprocess.run( + [ + sys.executable, + "-m", + "eng.profiler_benchmarks.controller", + "--check-build", + "on", + ], + check=True, + timeout=remaining(deadline, 60), + ) + continue + checkout(revision, paths[side]) + print(f"Building profiling {side}: {revision}", flush=True) + build(paths[side], args.output / f"build-{side}.log", remaining(deadline, 900)) + for sample in range(args.warmups + args.samples): + pair = {} + order = ("base", "candidate") if sample % 2 == 0 else ("candidate", "base") + for side in order: + print(f"Measuring pair {sample + 1}: {side}", flush=True) + pair[side] = measure( + paths[side], + args.output / f"{side}-{sample}.json", + args.scenarios, + remaining(deadline, WORKER_TIMEOUT), + ) + if pair["base"]["environment"] != pair["candidate"]["environment"]: + raise RuntimeError("Base and candidate environments differ") + if sample >= args.warmups: + report["pairs"].append(pair) + report_path.write_text(json.dumps(report, allow_nan=False), encoding="utf-8") + if args.scenarios is None: + report["status"] = "complete" + report_path.write_text(json.dumps(report, allow_nan=False), encoding="utf-8") + print(f"Paired profiler report: {report_path}", flush=True) + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--base", help="Exact base revision; defaults to candidate first parent") + parser.add_argument("--candidate", default="HEAD") + parser.add_argument("--leg", choices=LEGS) + parser.add_argument("--output", type=Path) + parser.add_argument("--samples", type=int, default=5) + parser.add_argument("--warmups", type=int, default=1) + parser.add_argument("--scenarios", nargs="+", help="Local subset; CI runs the full registry") + parser.add_argument("--worker", action="store_true", help=argparse.SUPPRESS) + parser.add_argument("--source-root", type=Path, help=argparse.SUPPRESS) + parser.add_argument( + "--reuse-candidate", + action="store_true", + help="Reuse checkout HEAD's profiling build after pytest", + ) + parser.add_argument( + "--check-build", + choices=("on", "off"), + help="Verify native compile configuration and recording OFF, then exit", + ) + args = parser.parse_args() + if args.check_build: + check_build(ROOT, profiling=args.check_build == "on") + elif args.worker: + if args.source_root is None or args.output is None: + parser.error("--worker requires --source-root and --output") + # Dumps contain stack locations, not locals or connection strings. The + # parent still kills/reaps the worker at its deadline if it cannot finish. + faulthandler.enable() + faulthandler.dump_traceback_later(60, repeat=True) + try: + worker(args) + finally: + faulthandler.cancel_dump_traceback_later() + else: + if ( + not args.output + or not args.leg + or not 3 <= args.samples <= 15 + or not 1 <= args.warmups <= 3 + ): + parser.error("Choose a leg, 3-15 measured pairs and 1-3 warmup pairs") + run(args) + + +if __name__ == "__main__": + main() diff --git a/eng/profiler_benchmarks/report.py b/eng/profiler_benchmarks/report.py new file mode 100644 index 000000000..b5768861e --- /dev/null +++ b/eng/profiler_benchmarks/report.py @@ -0,0 +1,662 @@ +"""Validate bounded profiler data and render an advisory, per-platform comparison.""" + +import argparse +from dataclasses import dataclass +import hashlib +import html +import io +import json +import math +from pathlib import Path, PurePosixPath +import re +import stat +import statistics +import zipfile +import zlib + +LEGS = ("Windows-SQL2022", "Windows-SQL2025", "macOS-SQL2022", "macOS-SQL2025", "Linux-SQL2022") +TASK_NAMES = { + "connect": "Connection opening", + "select": "SELECT queries", + "insert": "Row insertion", + "executemany": "Executemany inserts", + "fetchall": "Fetch-all queries", + "fetchone": "Row-by-row fetching", + "fetchmany": "Batched row fetching", + "commit_rollback": "Transaction commit and rollback", + "arrow": "Arrow row fetching", + "insertmanyvalues": "100,000-row insertion", + "fetchmany_100": "Row fetching in batches of 100", + "fetchmany_10000": "Row fetching in batches of 10,000", + "prepared_qmark": "Repeated positional queries", + "prepared_named": "Repeated named-parameter queries", + "legacy_insertmany": "Legacy 100,000-row insertion", + "setinputsizes": "Insertion with explicit input sizes", + "join_aggregation": "Joined aggregation queries", + "large_fetch": "Large joined-result fetching", + "fetch_1_2m": "1.2-million-row fetching", + "cte": "Common table expression queries", +} +CASES = tuple(TASK_NAMES) +MAX_BYTES = 8 * 1024 * 1024 +MAX_COMMENT_CHARS = 60000 +MAX_DIAGNOSTIC_ROWS = 20 +MARKER = "" +THRESHOLD = 0.20 +MIN_DELTA_MS = 1.0 + + +def suite_paths(root): + root = Path(root) + return [ + root / "eng/pipelines/pr-validation-pipeline.yml", + root / "eng/profiler_benchmarks/__init__.py", + root / "eng/profiler_benchmarks/controller.py", + root / "eng/profiler_benchmarks/report.py", + root / "eng/profiler_benchmarks/workloads.py", + root / "eng/scripts/setup_sql_container.py", + root / "requirements.txt", + *sorted((root / "profiler").glob("*.py")), + ] + + +def suite_hash(root): + digest = hashlib.sha256() + for file in suite_paths(root): + digest.update(file.name.encode()) + digest.update(file.read_bytes().replace(b"\r\n", b"\n")) + return digest.hexdigest() + + +@dataclass(frozen=True) +class AssessmentEvidence: + build: dict + head: str + base: str + merge_commit: dict + base_commit: dict + source_tree: dict + base_tree: dict + trusted_root: Path + + +def artifact_report(raw): + """Read exactly one bounded JSON member; never extract or execute artifact files.""" + with zipfile.ZipFile(io.BytesIO(raw)) as archive: + members = archive.infolist() + if len(members) > 200 or sum(member.file_size for member in members) > 64 * 1024 * 1024: + raise ValueError("Oversized artifact") + reports = [ + member for member in members if PurePosixPath(member.filename).name == "report.json" + ] + if len(reports) != 1: + raise ValueError("Expected exactly one report.json") + member = reports[0] + path = PurePosixPath(member.filename) + if ( + path.is_absolute() + or ".." in path.parts + or "\\" in member.filename + or stat.S_ISLNK(member.external_attr >> 16) + or member.file_size > MAX_BYTES + ): + raise ValueError("Invalid report member") + if member.flag_bits & 1: + raise ValueError("Encrypted performance artifacts are unsupported") + return json.loads(archive.read(member).decode("utf-8")) + + +def suite_blobs(tree, root): + if ( + not isinstance(tree, dict) + or tree.get("truncated") is not False + or not isinstance(tree.get("tree"), list) + or not all(isinstance(entry, dict) for entry in tree["tree"]) + ): + raise ValueError("Incomplete commit tree") + expected = {path.relative_to(root).as_posix() for path in suite_paths(root)} + blobs = { + entry.get("path"): entry.get("sha") + for entry in tree["tree"] + if entry.get("type") == "blob" and entry.get("path") in expected + } + if set(blobs) != expected or any( + not re.fullmatch(r"[0-9a-f]{40}", sha or "") for sha in blobs.values() + ): + raise ValueError("Benchmark suite missing from commit tree") + return blobs + + +def unavailable(reason): + return ( + f"{MARKER}\n## PR Performance Report\n\n" + f"**Performance could not be assessed.**\n\n{reason} No result is available." + ) + + +def number(value, maximum=1e12): + if type(value) not in (float, int) or not 0 <= value <= maximum: + raise ValueError("Invalid performance measurement") + return value + + +def text(value, limit=160): + if not isinstance(value, str) or not value or len(value) > limit: + raise ValueError("Invalid performance label") + if any(ord(char) < 32 or ord(char) == 127 for char in value): + raise ValueError("Control character in performance label") + return value + + +def validate(report, build_id=None, head=None, source=None, base=None, suite=None): + try: + return _validate(report, build_id, head, source, base, suite) + except KeyError as error: + raise ValueError(f"Missing performance report field: {error.args[0]}") from error + + +def _validate(report, build_id=None, head=None, source=None, base=None, suite=None): + if not isinstance(report, dict) or report.get("schema_version") != 1: + raise ValueError("Unsupported report schema") + if report.get("leg") not in LEGS or report.get("status") not in ("complete", "incomplete"): + raise ValueError("Invalid report status or leg") + if type(report.get("build_id")) is not int or report["build_id"] < 0: + raise ValueError("Invalid build_id") + for key, expected in ( + ("build_id", build_id), + ("head_commit", head), + ("source_commit", source), + ("base_commit", base), + ("suite_hash", suite), + ): + if expected is not None and report.get(key) != expected: + raise ValueError(f"Report provenance mismatch: {key}") + for key in ("head_commit", "source_commit", "base_commit"): + if not re.fullmatch(r"[0-9a-f]{40}", report.get(key, "")): + raise ValueError("Invalid commit identity") + if not re.fullmatch(r"[0-9a-f]{64}", report.get("suite_hash", "")): + raise ValueError("Invalid workload identity") + samples = report.get("samples") + if type(samples) is not int or not 3 <= samples <= 15: + raise ValueError("Insufficient or excessive samples") + if type(report.get("warmups")) is not int or not 1 <= report["warmups"] <= 3: + raise ValueError("Invalid warmup count") + pairs = report.get("pairs") + if not isinstance(pairs, list) or len(pairs) > samples: + raise ValueError("Invalid sample pairs") + if report["status"] == "incomplete": + return report + if len(pairs) != samples: + raise ValueError("Incomplete sample pairs") + environment = None + work = {} + for pair in pairs: + if not isinstance(pair, dict) or set(pair) != {"base", "candidate"}: + raise ValueError("Invalid paired sample") + for side in ("base", "candidate"): + sample = pair[side] + if not isinstance(sample, dict): + raise ValueError("Invalid sample") + env = sample["environment"] + if not isinstance(env, dict) or set(env) != { + "os", + "architecture", + "python", + "sql_version", + }: + raise ValueError("Invalid environment") + for value in env.values(): + text(value) + expected_os, sql = report["leg"].split("-") + if env["os"] != {"macOS": "Darwin"}.get(expected_os, expected_os): + raise ValueError("Artifact platform does not match its leg") + if not env["sql_version"].startswith({"SQL2022": "16.", "SQL2025": "17."}[sql]): + raise ValueError("Artifact SQL version does not match its leg") + if environment is not None and environment != env: + raise ValueError("Environment changed between measurements") + environment = env + scenarios = sample["scenarios"] + if not isinstance(scenarios, dict): + raise ValueError("Invalid scenarios object") + if set(scenarios) != set(CASES): + raise ValueError("Scenario set incomplete or changed") + for name, scenario in scenarios.items(): + if not isinstance(scenario, dict): + raise ValueError("Invalid scenario") + number(scenario["wall_ms"]) + if scenario["wall_ms"] <= 0: + raise ValueError("Zero workload time") + identity = text(scenario["work"]) + if name in work and work[name] != identity: + raise ValueError(f"Workload changed for {name}") + work[name] = identity + for layer in ("cpp", "py"): + stats = scenario[layer] + if ( + not isinstance(stats, dict) + or len(stats) > 300 + or (layer == "cpp" and not stats) + ): + raise ValueError("Missing or oversized profiling data") + for label, counter in stats.items(): + text(label) + if not label.startswith("ddbc::" if layer == "cpp" else "py::"): + raise ValueError("Invalid phase prefix") + if not isinstance(counter, dict): + raise ValueError("Invalid phase counter") + calls = counter["calls"] + if type(calls) is not int or not 1 <= calls <= 100_000_000: + raise ValueError("Invalid call count") + for field in ("total_us", "min_us", "max_us"): + number(counter[field]) + if not counter["min_us"] <= counter["max_us"] <= counter["total_us"]: + raise ValueError("Inconsistent phase totals") + return report + + +def assess(evidence, artifact_urls, load_artifact, issues=()): + issues = list(issues) + try: + if ( + not isinstance(evidence.build, dict) + or not isinstance(evidence.merge_commit, dict) + or not isinstance(evidence.base_commit, dict) + or not isinstance(evidence.source_tree, dict) + or not isinstance(evidence.base_tree, dict) + ): + raise ValueError + build_id = evidence.build.get("id") + source = evidence.build.get("sourceVersion") + if ( + type(build_id) is not int + or build_id <= 0 + or not re.fullmatch(r"[0-9a-f]{40}", source or "") + or not re.fullmatch(r"[0-9a-f]{40}", evidence.head) + or not re.fullmatch(r"[0-9a-f]{40}", evidence.base) + or evidence.merge_commit.get("sha") != source + or evidence.base_commit.get("sha") != evidence.base + or [parent["sha"] for parent in evidence.merge_commit["parents"]] + != [evidence.base, evidence.head] + ): + raise ValueError + source_tree_sha = evidence.merge_commit["tree"]["sha"] + base_tree_sha = evidence.base_commit["tree"]["sha"] + if ( + not re.fullmatch(r"[0-9a-f]{40}", source_tree_sha) + or not re.fullmatch(r"[0-9a-f]{40}", base_tree_sha) + or evidence.source_tree.get("sha") != source_tree_sha + or evidence.base_tree.get("sha") != base_tree_sha + ): + raise ValueError + except (KeyError, TypeError, ValueError): + return unavailable("Build provenance validation failed.") + + try: + suite_unchanged = suite_blobs(evidence.source_tree, evidence.trusted_root) == suite_blobs( + evidence.base_tree, evidence.trusted_root + ) + trusted_suite = suite_hash(evidence.trusted_root) + except (KeyError, TypeError, ValueError): + return unavailable("Benchmark suite validation failed because a required file changed.") + + reports = [] + for leg, url in artifact_urls.items(): + try: + report = validate( + artifact_report(load_artifact(url)), + build_id, + evidence.head, + source, + evidence.base, + ) + if report["leg"] != leg: + raise ValueError("Artifact leg mismatch") + reports.append(report) + except ( + KeyError, + RecursionError, + TypeError, + ValueError, + zipfile.BadZipFile, + zlib.error, + ): + issues.append(leg + " (invalid artifact)") + + if not suite_unchanged or any(report["suite_hash"] != trusted_suite for report in reports): + reports = [] + issues.append("workload version differs from trusted base") + try: + return render(reports, evidence.head, build_id, issues) + except ValueError: + return unavailable("Performance report rendering failed.") + + +def comparisons(report): + """Do not add inclusive phase totals together or treat them as wall-clock time.""" + output = [] + for name in CASES: + base = [pair["base"]["scenarios"][name] for pair in report["pairs"]] + candidate = [pair["candidate"]["scenarios"][name] for pair in report["pairs"]] + ratios = [new["wall_ms"] / old["wall_ms"] for old, new in zip(base, candidate)] + old = statistics.median(s["wall_ms"] for s in base) + new = statistics.median(s["wall_ms"] for s in candidate) + ratio = statistics.median(ratios) + # Requiring 80% of paired samples to agree avoids flagging one noisy pass. + agrees = sum(r > 1 + THRESHOLD for r in ratios) >= math.ceil(len(ratios) * 0.8) + status = ( + "regression" + if ratio > 1 + THRESHOLD and new - old >= MIN_DELTA_MS and agrees + else ("noisy" if ratio > 1 + THRESHOLD and new - old >= MIN_DELTA_MS else "ok") + ) + phases = [] + changed_counts = [] + for layer in ("cpp", "py"): + labels = set().union(*(s[layer] for s in base + candidate)) + for label in labels: + before = [s[layer].get(label) for s in base] + after = [s[layer].get(label) for s in candidate] + if not all(before) or not all(after): + changed_counts.append(f"{label} (added, removed, or intermittent)") + continue + before_calls = statistics.median(s["calls"] for s in before) + after_calls = statistics.median(s["calls"] for s in after) + if before_calls != after_calls: + changed_counts.append(f"{label} ({before_calls:g} -> {after_calls:g} calls)") + delta = ( + statistics.median(s["total_us"] for s in after) + - statistics.median(s["total_us"] for s in before) + ) / 1000 + if delta > 0: + phases.append((delta, label)) + output.append( + dict( + name=name, + base_ms=old, + candidate_ms=new, + change_pct=(ratio - 1) * 100, + status=status, + phases=sorted(phases, reverse=True)[:3], + counts=sorted(changed_counts)[:3], + ) + ) + return output + + +def escape(value): + value = html.escape(value, quote=True) + for char in "\\|`[]()*_~@": + value = value.replace(char, f"&#{ord(char)};") + return value + + +def environment_name(leg): + operating_system, sql = leg.split("-") + return f"{operating_system} / SQL Server {sql.removeprefix('SQL')}" + + +def issue_reason(leg, issues): + prefix = leg + " (" + for issue in issues: + if issue.startswith(prefix) and issue.endswith(")"): + return issue[len(prefix) : -1] + global_issues = [issue for issue in issues if not any(issue.startswith(x + " (") for x in LEGS)] + return global_issues[0] if global_issues else "incomplete benchmark" + + +def render(reports, head, build_id, issues=()): + url = f"https://dev.azure.com/sqlclientdrivers/public/_build/results?buildId={build_id}" + by_leg = {r["leg"]: r for r in reports} + if len(by_leg) != len(reports): + raise ValueError("Duplicate performance report leg") + completed = { + leg: (report, comparisons(report)) + for leg in LEGS + if (report := by_leg.get(leg)) is not None and report["status"] == "complete" + } + regressions = [ + (leg, row) + for leg, (_, rows) in completed.items() + for row in rows + if row["status"] == "regression" + ] + noisy = [ + (leg, row) + for leg, (_, rows) in completed.items() + for row in rows + if row["status"] == "noisy" + ] + missing = len(LEGS) - len(completed) + + if len(regressions) == 1: + leg, row = regressions[0] + opening = ( + f"This PR consistently slows {TASK_NAMES[row['name']].lower()} on " + f"{environment_name(leg)} by {row['change_pct']:.1f}%." + ) + elif regressions: + tasks = len({row["name"] for _, row in regressions}) + environments = len({leg for leg, _ in regressions}) + opening = ( + f"This PR has {len(regressions)} consistent slowdown signals across " + f"{tasks} database tasks and {environments} environments." + ) + elif noisy: + if len(noisy) == 1: + leg, row = noisy[0] + opening = ( + f"{TASK_NAMES[row['name']]} was slower on {environment_name(leg)}, " + "but the repeated comparisons were inconsistent." + ) + else: + tasks = len({row["name"] for _, row in noisy}) + environments = len({leg for leg, _ in noisy}) + opening = ( + f"No consistent slowdowns detected. {len(noisy)} inconsistent comparisons " + f"need review across {tasks} database tasks and {environments} environments." + ) + elif not completed: + opening = ( + "Performance could not be assessed because no environment produced a complete result." + ) + elif not missing: + opening = f"No consistent slowdowns detected across all {len(LEGS)} environments." + else: + completed_label = "environment" if len(completed) == 1 else "environments" + missing_label = "environment" if missing == 1 else "environments" + opening = ( + f"No consistent slowdowns in the {len(completed)} completed {completed_label}. " + f"No result is available for {missing} {missing_label}." + ) + + lines = [MARKER, "## PR Performance Report", "", f"**{opening}**", ""] + highlighted = regressions or noisy + if highlighted: + if not regressions: + lines += ["Inconsistent slowdowns to review:", ""] + lines += [ + "| Environment | Affected task | Before | After | Change |", + "|---|---|---:|---:|---:|", + ] + for leg, row in highlighted: + lines.append( + f"| {environment_name(leg)} | {TASK_NAMES[row['name']]} | " + f"{row['base_ms']:.3f} ms | {row['candidate_ms']:.3f} ms | " + f"{row['change_pct']:+.1f}% |" + ) + lines.append("") + if regressions: + lines.append( + "The largest recorded phase increases for these tasks are shown below. " + "Phase timings are supporting evidence, not root-cause proof." + ) + if noisy: + lines.append( + f"{len(noisy)} additional inconsistent slowdown" + f"{'s' if len(noisy) != 1 else ''} also need review." + ) + lines.append("") + + lines += [ + f"**Coverage:** {len(completed)} of {len(LEGS)} environments completed. " + "Advisory result; does not block merging.", + "", + "| Environment | Status |", + "|---|---|", + ] + for leg in LEGS: + report = by_leg.get(leg) + status = ( + "Completed" + if leg in completed + else f"No result available ({escape(issue_reason(leg, issues))})" + ) + lines.append(f"| {environment_name(leg)} | {status} |") + + diagnostics_start = len(lines) + lines += [ + "", + "
", + "Affected phases and call counts", + "", + "Phase times are inclusive diagnostics and must not be added together. " + "They identify where measured time changed, not why it changed.", + ] + diagnostics = 0 + total_diagnostics = 0 + for leg, (_, rows) in completed.items(): + relevant = [row for row in rows if row["status"] != "ok" or row["counts"]] + total_diagnostics += len(relevant) + visible = relevant[: max(0, MAX_DIAGNOSTIC_ROWS - diagnostics)] + if not visible: + continue + lines += ["", f"### {environment_name(leg)}"] + for row in visible: + diagnostics += 1 + phases = "; ".join(f"{escape(label)} +{delta:.3f} ms" for delta, label in row["phases"]) + counts = "; ".join(escape(label) for label in row["counts"]) + detail = phases or "no positive phase delta" + if counts: + detail += f". Call changes: {counts}" + lines.append(f"**{TASK_NAMES[row['name']]}:** {detail}.") + if not diagnostics: + lines += ["", "No affected phases or call-count changes were recorded."] + elif total_diagnostics > diagnostics: + lines += [ + "", + f"{total_diagnostics - diagnostics} additional diagnostic rows are available " + "in the raw ADO artifacts.", + ] + lines += [ + "", + "
", + ] + diagnostics_end = len(lines) + lines += [ + "", + "
", + "All database tasks and timings", + ] + + for leg, (report, rows) in completed.items(): + lines += [ + "", + f"### {environment_name(leg)}", + "| Database task | Before | After | Paired change | Result |", + "|---|---:|---:|---:|---|", + ] + for row in rows: + result = { + "regression": "consistent slowdown", + "noisy": "inconsistent slowdown", + "ok": "no signal", + }[row["status"]] + lines.append( + f"| {TASK_NAMES[row['name']]} | {row['base_ms']:.3f} ms | " + f"{row['candidate_ms']:.3f} ms | {row['change_pct']:+.1f}% | {result} |" + ) + lines += [ + "", + "
", + "", + "
", + "Build, commits and measurement details", + "", + ] + lines += [ + f"[ADO build {build_id}]({url})", + "", + f"PR head: `{head}`", + ] + if completed: + first = next(iter(completed.values()))[0] + lines += [ + f"Base: `{first['base_commit']}`", + f"Measured merge: `{first['source_commit']}`", + "", + ] + for leg, (report, _) in completed.items(): + env = report["pairs"][0]["base"]["environment"] + lines.append( + f"- {environment_name(leg)}: Python {escape(env['python'])}, " + f"{escape(env['architecture'])}, SQL {escape(env['sql_version'])}; " + f"{report['samples']} paired comparisons and {report['warmups']} warmup." + ) + lines += [ + "", + "A consistent slowdown requires more than 20% median paired slowdown, at least " + "1 ms between the median runtimes, and at least 80% of pairs exceeding the " + "relative threshold. An inconsistent slowdown crosses the first two thresholds " + "without enough pair agreement.", + "", + "The displayed change is the median of paired before-and-after ratios. It is not " + "recalculated from the two displayed median runtimes.", + ] + if issues: + lines += ["", "Unavailable or rejected data: " + ", ".join(escape(x) for x in issues)] + lines += [ + "", + "Both revisions use profiling-enabled builds on the same agent and database, " + "with alternating order and discarded warmups. Results are diagnostic and do " + "not represent production-wheel latency.", + "", + "Raw samples and logs are attached to the ADO run as `profiler-*` artifacts.", + "", + "
", + ] + body = "\n".join(lines) + if len(body) > MAX_COMMENT_CHARS: + lines[diagnostics_start:diagnostics_end] = [ + "", + "
", + "Affected phases and call counts", + "", + f"{total_diagnostics} diagnostic rows are available in the raw ADO artifacts.", + "", + "
", + ] + body = "\n".join(lines) + if len(body) > MAX_COMMENT_CHARS: + raise ValueError("Performance comment exceeds its size budget") + return body + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("reports", nargs="+", type=Path) + args = parser.parse_args() + reports = [validate(json.loads(path.read_text(encoding="utf-8"))) for path in args.reports] + first = reports[0] + for report in reports[1:]: + validate( + report, + build_id=first["build_id"], + head=first["head_commit"], + source=first["source_commit"], + base=first["base_commit"], + suite=first["suite_hash"], + ) + print(render(reports, first["head_commit"], first["build_id"])) + + +if __name__ == "__main__": + main() diff --git a/eng/profiler_benchmarks/workloads.py b/eng/profiler_benchmarks/workloads.py new file mode 100644 index 000000000..324600860 --- /dev/null +++ b/eng/profiler_benchmarks/workloads.py @@ -0,0 +1,148 @@ +"""Fixed workloads shared by the base and candidate profiler benchmark builds.""" + +from functools import partial +import time + +from profiler import scenarios + +# Preserve the AdventureWorks workloads from the previous CI benchmark. +QUERIES = { + "join_aggregation": """ + SELECT p.ProductID, p.Name AS ProductName, pc.Name AS Category, + psc.Name AS Subcategory, COUNT(sod.SalesOrderDetailID) AS TotalOrders, + SUM(sod.OrderQty) AS TotalQuantity, SUM(sod.LineTotal) AS TotalRevenue, + AVG(sod.UnitPrice) AS AvgPrice + FROM Sales.SalesOrderDetail sod + INNER JOIN Production.Product p ON sod.ProductID = p.ProductID + INNER JOIN Production.ProductSubcategory psc ON p.ProductSubcategoryID = psc.ProductSubcategoryID + INNER JOIN Production.ProductCategory pc ON psc.ProductCategoryID = pc.ProductCategoryID + GROUP BY p.ProductID, p.Name, pc.Name, psc.Name + HAVING SUM(sod.LineTotal) > 10000 ORDER BY TotalRevenue DESC + """, + "large_fetch": """ + SELECT soh.SalesOrderID, soh.OrderDate, soh.DueDate, soh.ShipDate, soh.Status, + soh.SubTotal, soh.TaxAmt, soh.Freight, soh.TotalDue, c.CustomerID, + p.FirstName, p.LastName, a.AddressLine1, a.City, + sp.Name AS StateProvince, cr.Name AS Country + FROM Sales.SalesOrderHeader soh + INNER JOIN Sales.Customer c ON soh.CustomerID = c.CustomerID + INNER JOIN Person.Person p ON c.PersonID = p.BusinessEntityID + INNER JOIN Person.BusinessEntityAddress bea ON p.BusinessEntityID = bea.BusinessEntityID + INNER JOIN Person.Address a ON bea.AddressID = a.AddressID + INNER JOIN Person.StateProvince sp ON a.StateProvinceID = sp.StateProvinceID + INNER JOIN Person.CountryRegion cr ON sp.CountryRegionCode = cr.CountryRegionCode + WHERE soh.OrderDate >= '2013-01-01' + """, + "fetch_1_2m": """ + SELECT sod.SalesOrderID, sod.SalesOrderDetailID, sod.ProductID, + sod.OrderQty, sod.UnitPrice, sod.LineTotal, + p.Name AS ProductName, p.ProductNumber, p.Color, p.ListPrice, + n1.number AS RowMultiplier1 + FROM Sales.SalesOrderDetail sod + CROSS JOIN (SELECT TOP 10 ROW_NUMBER() OVER (ORDER BY (SELECT NULL)) AS number + FROM Sales.SalesOrderDetail) n1 + INNER JOIN Production.Product p ON sod.ProductID = p.ProductID + """, + "cte": """ + WITH SalesSummary AS ( + SELECT soh.SalesPersonID, YEAR(soh.OrderDate) AS OrderYear, + SUM(soh.TotalDue) AS YearlyTotal + FROM Sales.SalesOrderHeader soh WHERE soh.SalesPersonID IS NOT NULL + GROUP BY soh.SalesPersonID, YEAR(soh.OrderDate) + ), RankedSales AS ( + SELECT SalesPersonID, OrderYear, YearlyTotal, + RANK() OVER (PARTITION BY OrderYear ORDER BY YearlyTotal DESC) AS SalesRank + FROM SalesSummary + ) + SELECT rs.SalesPersonID, p.FirstName, p.LastName, + rs.OrderYear, rs.YearlyTotal, rs.SalesRank + FROM RankedSales rs INNER JOIN Person.Person p ON rs.SalesPersonID = p.BusinessEntityID + WHERE rs.SalesRank <= 10 ORDER BY rs.OrderYear DESC, rs.SalesRank + """, +} + + +def query(conn, ctx, sql): + with conn.cursor() as cursor: + ctx.enable() + try: + start = time.perf_counter() + cursor.execute(sql) + rows = cursor.fetchall() + wall_ms = (time.perf_counter() - start) * 1000 + cpp, py = ctx.collect() + return dict( + title="AdventureWorks query", + wall_ms=wall_ms, + cpp=cpp, + py=py, + detail=f"Rows: {len(rows)}", + ) + finally: + ctx.disable() + + +def parameter_execution(conn, ctx, named=False): + with conn.cursor() as cursor: + ctx.enable() + try: + start = time.perf_counter() + for value in range(100): + if named: + cursor.execute("SELECT %(value)s", {"value": value}) + else: + cursor.execute("SELECT ?", (value,)) + assert cursor.fetchone()[0] == value + wall_ms = (time.perf_counter() - start) * 1000 + cpp, py = ctx.collect() + return dict( + title="Parameterized execution", wall_ms=wall_ms, cpp=cpp, py=py, detail="Rows: 100" + ) + finally: + ctx.disable() + + +def legacy_insertmany(conn, ctx, input_sizes=False): + from mssql_python import SQL_INTEGER, SQL_VARCHAR + + sql = "INSERT INTO #ci_insert VALUES " + ",".join(["(?,?)"] * 1000) + batches = [ + [value for i in range(start, start + 1000) for value in (i, f"value_{i}")] + for start in range(0, 100_000, 1000) + ] + with conn.cursor() as cursor: + try: + cursor.execute( + "DROP TABLE IF EXISTS #ci_insert; " + "CREATE TABLE #ci_insert (id INT, val VARCHAR(100))" + ) + sizes = [(SQL_INTEGER, 0, 0), (SQL_VARCHAR, 100, 0)] * 1000 + ctx.enable() + start = time.perf_counter() + for params in batches: + if input_sizes: + cursor.setinputsizes(sizes) + cursor.execute(sql, params) + wall_ms = (time.perf_counter() - start) * 1000 + cpp, py = ctx.collect() + return dict( + title="Batched insert", wall_ms=wall_ms, cpp=cpp, py=py, detail="Rows: 100000" + ) + finally: + ctx.disable() + conn.rollback() + + +def registry(): + """Keep every PR #552 scenario, including its existing timing boundaries.""" + result = dict(scenarios.SCENARIOS) + result.update( + fetchmany_100=(partial(scenarios.fetchmany, batch_size=100), True), + fetchmany_10000=(partial(scenarios.fetchmany, batch_size=10000), True), + prepared_qmark=(parameter_execution, False), + prepared_named=(partial(parameter_execution, named=True), False), + legacy_insertmany=(legacy_insertmany, False), + setinputsizes=(partial(legacy_insertmany, input_sizes=True), False), + ) + result.update((name, (partial(query, sql=sql), False)) for name, sql in QUERIES.items()) + return result diff --git a/profiler/README.md b/profiler/README.md index 3521ed9b3..8c0662b14 100644 --- a/profiler/README.md +++ b/profiler/README.md @@ -21,8 +21,9 @@ context manager whose end-to-end cost is within run-to-run noise. Runtime-instrumentation tests remain part of the driver test suite. Tests that require the dev-only `profiler/` package skip when it is absent from an installed -wheel. Broader profiler testing and profiling-enabled CI builds are deferred to -follow-up work. +wheel. The [profiler benchmark guide](../eng/profiler_benchmarks/README.md) describes +isolated profiling builds, scenario coverage and advisory PR regression comments. +Broader profiler testing remains follow-up work. Use controlled diagnostic workloads with one owner of the process-wide profiling state: enable, run the workload, wait for worker threads to finish, then collect. diff --git a/tests/test_036_profiler_ci.py b/tests/test_036_profiler_ci.py new file mode 100644 index 000000000..c5223d106 --- /dev/null +++ b/tests/test_036_profiler_ci.py @@ -0,0 +1,1316 @@ +"""Contract tests for paired performance comparisons and data-only PR reporting.""" + +import copy +from http.client import IncompleteRead +import importlib.util +import io +import json +import os +from pathlib import Path +import re +import signal +import subprocess +import sys +import tarfile +import time +from types import SimpleNamespace +from unittest.mock import MagicMock +from urllib.error import URLError +import zipfile +import zlib + +import pytest + +ROOT = Path(__file__).resolve().parents[1] +if not (ROOT / ".github/scripts/post_profiler_comment.py").is_file(): + pytest.skip("CI reporting tools are not installed in driver wheels", allow_module_level=True) + +from eng.profiler_benchmarks import controller +from eng.profiler_benchmarks import report as reporting +from eng.profiler_benchmarks import workloads as benchmark_workloads + + +def load(name, path): + spec = importlib.util.spec_from_file_location(name, ROOT / path) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +publisher = load("post_profiler_comment", ".github/scripts/post_profiler_comment.py") +extractor = load("extract_coverage_artifact", ".github/scripts/extract_coverage_artifact.py") + + +def ado_build(**values): + build = dict( + id=42, + status="completed", + result="failed", + definition={"id": 2128}, + repository={"id": "microsoft/mssql-python"}, + sourceBranch="refs/pull/123/merge", + sourceVersion="b" * 40, + triggerInfo={"pr.number": "123", "pr.sourceSha": "c" * 40}, + ) + build.update(values) + return build + + +def pr_topology(head="c" * 40, base="a" * 40, merge_base=None): + def response(path): + if path.startswith("pulls/"): + return {"state": "open", "head": {"sha": head}, "base": {"sha": base}} + if path.startswith("git/commits/"): + commit_sha = path.removeprefix("git/commits/") + source = commit_sha == "b" * 40 + return { + "sha": commit_sha, + "parents": [{"sha": merge_base or base}, {"sha": head}] if source else [], + "tree": {"sha": ("d" if source else "e") * 40}, + } + if path.startswith("git/trees/"): + tree_sha = path.removeprefix("git/trees/").split("?", 1)[0] + return { + "sha": tree_sha, + "truncated": False, + "tree": [ + { + "path": file.relative_to(ROOT).as_posix(), + "type": "blob", + "sha": f"{index + 1:040x}", + } + for index, file in enumerate(reporting.suite_paths(ROOT)) + ], + } + raise AssertionError(f"Unexpected GitHub path: {path}") + + return response + + +@pytest.fixture +def report(): + def sample(scale): + counter = dict(calls=1, total_us=1000, min_us=1000, max_us=1000) + return dict( + environment=dict( + os="Linux", architecture="x86_64", python="3.13.7", sql_version="16.0" + ), + scenarios={ + name: dict( + wall_ms=10 * scale, work="Rows: 100", cpp={"ddbc::query": counter}, py={} + ) + for name in reporting.CASES + }, + ) + + return dict( + schema_version=1, + status="complete", + leg="Linux-SQL2022", + base_commit="a" * 40, + source_commit="b" * 40, + head_commit="c" * 40, + suite_hash="d" * 64, + build_id=42, + samples=5, + warmups=1, + pairs=[dict(base=sample(1), candidate=sample(1.3)) for _ in range(5)], + ) + + +def test_consistent_slowdown_is_advisory_regression(report): + reporting.validate(report, 42, "c" * 40, "b" * 40, "a" * 40) + rows = reporting.comparisons(report) + assert all( + row["status"] == "regression" and row["change_pct"] == pytest.approx(30) for row in rows + ) + body = reporting.render([report], "c" * 40, 42) + assert "20 consistent slowdown signals" in body + assert "| Linux / SQL Server 2022 | Connection opening |" in body + assert "| macOS / SQL Server 2022 | No result available" in body + assert body.index("consistent slowdown signals") < body.index( + "Build, commits and measurement details" + ) + + +def test_noisy_slowdown_and_submillisecond_change_are_not_regressions(report): + for pair in report["pairs"][:2]: + pair["candidate"]["scenarios"]["select"]["wall_ms"] = 8 + report["pairs"][0]["candidate"]["scenarios"]["connect"]["wall_ms"] = 1000 + assert reporting.comparisons(report)[1]["status"] == "noisy" + for pair in report["pairs"]: + pair["base"]["scenarios"]["insert"]["wall_ms"] = 0.1 + pair["candidate"]["scenarios"]["insert"]["wall_ms"] = 0.2 + assert reporting.comparisons(report)[2]["status"] == "ok" + + +def test_phase_call_changes_are_reported_without_summing_nested_totals(report): + for pair in report["pairs"]: + pair["candidate"]["scenarios"]["select"]["cpp"] = { + "ddbc::query": dict(calls=2, total_us=5000, min_us=2000, max_us=3000), + } + row = reporting.comparisons(report)[1] + assert row["counts"] == ["ddbc::query (1 -> 2 calls)"] + assert row["phases"] == [(4.0, "ddbc::query")] + + +@pytest.mark.parametrize("case", ["nan", "missing", "environment", "work", "few", "prefix", "zero"]) +def test_reject_invalid_or_incomparable_data(report, case): + sample = report["pairs"][0]["candidate"] + if case == "nan": + sample["scenarios"]["select"]["wall_ms"] = float("nan") + elif case == "missing": + del sample["scenarios"]["select"] + elif case == "environment": + sample["environment"]["sql_version"] = "other" + elif case == "work": + sample["scenarios"]["select"]["work"] = "Rows: 200" + elif case == "few": + report["pairs"].pop() + elif case == "zero": + sample["scenarios"]["select"]["wall_ms"] = 0 + elif case == "prefix": + sample["scenarios"]["select"]["cpp"] = { + "not-native": sample["scenarios"]["select"]["cpp"]["ddbc::query"] + } + with pytest.raises(ValueError): + reporting.validate(report) + + +@pytest.mark.parametrize("field", ["environment", "scenarios", "cpp", "calls"]) +def test_missing_report_fields_are_normalized_to_value_error(report, field): + sample = report["pairs"][0]["base"] + if field in ("environment", "scenarios"): + del sample[field] + elif field == "cpp": + del sample["scenarios"]["select"][field] + else: + del sample["scenarios"]["select"]["cpp"]["ddbc::query"][field] + with pytest.raises(ValueError, match="Missing performance report field"): + reporting.validate(report) + + +def test_reject_wrong_commit_and_preserve_incomplete_status(report): + with pytest.raises(ValueError, match="provenance"): + reporting.validate(report, head="e" * 40) + report["status"] = "incomplete" + report["pairs"] = [] + reporting.validate(report) + assert "Performance could not be assessed" in reporting.render([report], "c" * 40, 42) + + +@pytest.mark.parametrize("build_id", [None, True, -1]) +def test_reject_invalid_build_id(report, build_id): + report["build_id"] = build_id + with pytest.raises(ValueError, match="build_id"): + reporting.validate(report) + + +def set_leg(report, leg): + report = copy.deepcopy(report) + report["leg"] = leg + operating_system, sql = leg.split("-") + for pair in report["pairs"]: + for sample in pair.values(): + sample["environment"]["os"] = {"macOS": "Darwin"}.get( + operating_system, operating_system + ) + sample["environment"]["sql_version"] = "16.0" if sql == "SQL2022" else "17.0" + return report + + +@pytest.mark.parametrize( + "key,value", + [ + ("build_id", 43), + ("head_commit", "e" * 40), + ("source_commit", "e" * 40), + ("base_commit", "e" * 40), + ("suite_hash", "e" * 64), + ], +) +def test_standalone_report_rejects_mixed_provenance(report, tmp_path, monkeypatch, key, value): + first = tmp_path / "linux.json" + second = tmp_path / "windows.json" + first.write_text(json.dumps(report), encoding="utf-8") + other = set_leg(report, "Windows-SQL2022") + other[key] = value + second.write_text(json.dumps(other), encoding="utf-8") + monkeypatch.setattr(sys, "argv", ["report", str(first), str(second)]) + with pytest.raises(ValueError, match=key): + reporting.main() + + +def test_render_rejects_duplicate_legs(report): + with pytest.raises(ValueError, match="Duplicate"): + reporting.render([report, copy.deepcopy(report)], "c" * 40, 42) + + +def test_render_bounds_schema_valid_diagnostics(report): + reports = [set_leg(copy.deepcopy(report), leg) for leg in reporting.LEGS] + labels = ["ddbc::" + str(index) + "_" * 152 for index in range(3)] + for item in reports: + for pair in item["pairs"]: + for name in reporting.CASES: + for side, calls in (("base", 1), ("candidate", 2)): + pair[side]["scenarios"][name]["cpp"] = { + label: dict(calls=calls, total_us=2000, min_us=1000, max_us=1000) + for label in labels + } + reporting.validate(item) + body = reporting.render(reports, "c" * 40, 42) + assert len(body) <= 60000 + assert "100 diagnostic rows are available in the raw ADO artifacts" in body + assert "All database tasks and timings" in body + assert "Build, commits and measurement details" in body + + +@pytest.mark.parametrize("invalid", ["source commit", "base commit", "source tree"]) +def test_assessment_binds_all_evidence_to_authenticated_commits(invalid): + evidence = reporting.AssessmentEvidence( + build=ado_build(), + head="c" * 40, + base="a" * 40, + merge_commit={ + "sha": "b" * 40, + "parents": [{"sha": "a" * 40}, {"sha": "c" * 40}], + "tree": {"sha": "d" * 40}, + }, + base_commit={"sha": "a" * 40, "tree": {"sha": "e" * 40}}, + source_tree={"sha": "d" * 40, "truncated": False, "tree": []}, + base_tree={"sha": "e" * 40, "truncated": False, "tree": []}, + trusted_root=ROOT, + ) + if invalid == "source commit": + evidence.merge_commit["sha"] = "f" * 40 + elif invalid == "base commit": + evidence.base_commit["sha"] = "f" * 40 + else: + evidence = reporting.AssessmentEvidence(**{**evidence.__dict__, "source_tree": []}) + body = reporting.assess(evidence, {}, lambda url: pytest.fail("must not download")) + assert "Performance could not be assessed" in body + assert "Build provenance validation failed" in body + + +def clear_slowdowns(report): + for pair in report["pairs"]: + for name in reporting.CASES: + pair["candidate"]["scenarios"][name]["wall_ms"] = pair["base"]["scenarios"][name][ + "wall_ms" + ] + return report + + +def test_impact_summary_handles_single_inconsistent_and_complete_clean_results(report): + clean = clear_slowdowns(copy.deepcopy(report)) + for pair, scale in zip(clean["pairs"], (1.3, 1.3, 1.3, 0.8, 0.8)): + pair["candidate"]["scenarios"]["fetchone"]["wall_ms"] *= scale + noisy = reporting.render([clean], "c" * 40, 42) + assert ( + "**Row-by-row fetching was slower on Linux / SQL Server 2022, " + "but the repeated comparisons were inconsistent.**" + ) in noisy + assert "Inconsistent slowdowns to review:" in noisy + + complete = [set_leg(clear_slowdowns(copy.deepcopy(report)), leg) for leg in reporting.LEGS] + clean_body = reporting.render(complete, "c" * 40, 42) + assert "**No consistent slowdowns detected across all 5 environments.**" in clean_body + assert "**Coverage:** 5 of 5 environments completed." in clean_body + + +def test_impact_summary_handles_single_regression_partial_and_no_results(report): + single = clear_slowdowns(copy.deepcopy(report)) + for pair in single["pairs"]: + pair["candidate"]["scenarios"]["fetchone"]["wall_ms"] *= 1.3 + body = reporting.render([single], "c" * 40, 42) + assert ( + "**This PR consistently slows row-by-row fetching on Linux / SQL Server 2022 " "by 30.0%.**" + ) in body + assert "Affected phases and call counts" in body + assert "All database tasks and timings" in body + assert "Build, commits and measurement details" in body + assert "median of paired before-and-after ratios" in body + + partial = reporting.render( + [clear_slowdowns(copy.deepcopy(report))], + "c" * 40, + 42, + ["Windows-SQL2022 (missing)"], + ) + assert "No consistent slowdowns in the 1 completed environment." in partial + assert "No result is available for 4 environments." in partial + assert "| Windows / SQL Server 2022 | No result available (missing) |" in partial + assert "pending" not in partial.lower() + + unavailable = reporting.render([], "c" * 40, 42, ["Linux-SQL2022 (invalid artifact)"]) + assert "Performance could not be assessed" in unavailable + assert "No consistent slowdowns" not in unavailable + + +@pytest.mark.parametrize( + "path", + [ + ("pairs", 0, "candidate"), + ("pairs", 0, "candidate", "environment"), + ("pairs", 0, "candidate", "scenarios"), + ("pairs", 0, "candidate", "scenarios", "select"), + ("pairs", 0, "candidate", "scenarios", "select", "cpp"), + ("pairs", 0, "candidate", "scenarios", "select", "cpp", "ddbc::query"), + ], +) +@pytest.mark.parametrize("as_list", [False, True]) +def test_reject_non_object_sample_containers(report, path, as_list): + parent = report + for key in path[:-1]: + parent = parent[key] + key = path[-1] + parent[key] = list(parent[key]) if as_list else None + with pytest.raises(ValueError): + reporting.validate(report) + + +def zip_data(entries): + out = io.BytesIO() + with zipfile.ZipFile(out, "w") as archive: + for name, data in entries: + archive.writestr(name, data) + return out.getvalue() + + +@pytest.mark.parametrize( + "kind,member,data", + [ + ("html", "Code Coverage Report_1/index.html", b"coverage"), + ("xml", "unified-coverage/coverage.xml", b""), + ], +) +def test_coverage_artifact_reader_copies_only_expected_report(tmp_path, kind, member, data): + archive = tmp_path / "coverage.zip" + archive.write_bytes( + zip_data( + [ + (".github/actions/post-coverage-comment/action.yml", "malicious"), + ("../outside.txt", "escape"), + (member, data), + ] + ) + ) + output = tmp_path / f"report.{kind}" + extractor.copy_report(archive, output, kind) + assert output.read_bytes() == data + assert not (tmp_path.parent / "outside.txt").exists() + assert not (tmp_path / ".github").exists() + + +@pytest.mark.parametrize("second,valid", [("", True), ("", False)]) +def test_coverage_artifact_reader_accepts_only_identical_duplicate_reports(tmp_path, second, valid): + archive = tmp_path / "coverage.zip" + output = tmp_path / "coverage.xml" + archive.write_bytes( + zip_data( + [ + ("first/coverage.xml", ""), + ("second/coverage.xml", second), + ] + ) + ) + if valid: + extractor.copy_report(archive, output, "xml") + assert output.read_text() == "" + else: + with pytest.raises(ValueError, match="Conflicting"): + extractor.copy_report(archive, output, "xml") + + +def test_coverage_artifact_reader_rejects_oversized_or_unrelated_archives(tmp_path): + archive = tmp_path / "coverage.zip" + archive.write_bytes(zip_data([("test-results.xml", "")])) + with pytest.raises(ValueError, match="No coverage xml"): + extractor.copy_report(archive, tmp_path / "coverage.xml", "xml") + with archive.open("wb") as stream: + stream.seek(extractor.MAX_ARCHIVE_BYTES) + stream.write(b"x") + with pytest.raises(ValueError, match="archive exceeds"): + extractor.copy_report(archive, tmp_path / "coverage.xml", "xml") + archive.write_bytes(zip_data([("coverage.xml/", b"")])) + with pytest.raises(ValueError, match="No coverage xml"): + extractor.copy_report(archive, tmp_path / "coverage.xml", "xml") + directory = zipfile.ZipInfo("coverage.xml") + directory.create_system = 3 + directory.external_attr = 0o40755 << 16 + archive.write_bytes(zip_data([(directory, b"")])) + with pytest.raises(ValueError, match="No coverage xml"): + extractor.copy_report(archive, tmp_path / "coverage.xml", "xml") + + +def test_artifact_read_never_extracts_paths(report): + raw = json.dumps(report) + assert ( + reporting.artifact_report(zip_data([("profiler-Linux-SQL2022/report.json", raw)])) == report + ) + for entries in [ + [("../report.json", raw)], + [("/report.json", raw)], + [("a/report.json", raw), ("b/report.json", raw)], + [("logs.txt", "no report")], + ]: + with pytest.raises(ValueError): + reporting.artifact_report(zip_data(entries)) + + +def test_untrusted_labels_cannot_inject_links_mentions_or_markdown(): + assert reporting.escape("[click](https://example.com) @everyone | `code`") == ( + "[click](https://example.com) @everyone | `code`" + ) + assert not publisher.allowed_url("https://example.com/artifact") + assert not publisher.allowed_url("http://dev.azure.com/artifact") + assert not publisher.allowed_url("https://dev.azure.com@evil.example/artifact") + assert publisher.allowed_url("https://dev.azure.com/sqlclientdrivers/public/") + assert publisher.allowed_url( + "https://artprodcus3.artifacts.visualstudio.com/A1/_apis/artifact/" + ) + assert not publisher.allowed_url("https://artifacts.visualstudio.com.evil.example/artifact") + + +@pytest.mark.parametrize("error", [IncompleteRead(b"partial"), ConnectionResetError("reset")]) +def test_incomplete_http_response_is_normalized_for_terminal_fallback(monkeypatch, error): + opener = MagicMock() + opener.open.return_value.__enter__.return_value.read.side_effect = error + monkeypatch.setattr(publisher, "build_opener", lambda *args: opener) + with pytest.raises(URLError, match="Incomplete HTTP response"): + publisher.fetch("https://api.github.com/repos/microsoft/mssql-python") + + +def test_build_selection_requires_exact_pr_head(): + build = ado_build() + assert publisher.find_build([build], 123, "c" * 40) is build + for key in ("pr.number", "pr.sourceSha"): + bad = copy.deepcopy(build) + bad["triggerInfo"][key] = "different" + assert publisher.find_build([bad], 123, "c" * 40) is None + + +def test_publisher_does_not_post_stale_head(monkeypatch): + calls = [] + + def api(path, **kwargs): + calls.append((path, kwargs)) + return {"state": "open", "head": {"sha": "new-head"}} + + monkeypatch.setattr(publisher, "github", api) + publisher.publish(123, "old-head", "anything") + assert len(calls) == 1 and calls[0][1] == {} + + +def test_publisher_retries_transient_comment_failures(monkeypatch): + publish = MagicMock(side_effect=[URLError("temporary"), None]) + sleeps = [] + monkeypatch.setattr(publisher, "publish", publish) + monkeypatch.setattr(publisher.time, "sleep", sleeps.append) + publisher.publish_with_retry(123, "a" * 40, "body") + assert publish.call_count == 2 + assert sleeps == [5] + + +def test_revisions_use_exact_first_parent(monkeypatch): + calls = [] + + def git(*args): + calls.append(args) + return "b" * 40 if len(calls) == 1 else "a" * 40 + + monkeypatch.setattr(controller, "git", git) + assert controller.resolve_revisions(None, "HEAD") == ("a" * 40, "b" * 40) + assert calls[1][-1] == "b" * 40 + "^1^{commit}" + + +@pytest.mark.parametrize("name", ["safe.txt", "../outside.txt", "C:/outside.txt"]) +def test_checkout_is_safe_and_compatible_with_python_310(tmp_path, monkeypatch, name): + archive = io.BytesIO() + with tarfile.open(fileobj=archive, mode="w") as tar: + member = tarfile.TarInfo(name) + member.size = 4 + tar.addfile(member, io.BytesIO(b"data")) + archive.seek(0) + + class Archive: + def __enter__(self): + return archive + + def __exit__(self, *args): + return None + + monkeypatch.setattr(controller.sys, "version_info", (3, 10)) + monkeypatch.setattr(controller.tempfile, "TemporaryFile", Archive) + monkeypatch.setattr(controller.subprocess, "run", lambda *a, **kw: None) + if name == "safe.txt": + controller.checkout("a" * 40, tmp_path) + assert (tmp_path / name).read_bytes() == b"data" + else: + with pytest.raises(ValueError, match="Unsafe"): + controller.checkout("a" * 40, tmp_path) + assert not (tmp_path.parent / "outside.txt").exists() + + +def test_report_cases_match_the_executed_workload_registry(): + _, workloads = controller.load_suite() + assert tuple(workloads.registry()) == reporting.CASES + assert ROOT / "eng/profiler_benchmarks/__init__.py" in reporting.suite_paths(ROOT) + assert ROOT / "eng/profiler_benchmarks/report.py" in reporting.suite_paths(ROOT) + assert ROOT / "eng/pipelines/pr-validation-pipeline.yml" in reporting.suite_paths(ROOT) + assert ROOT / "eng/scripts/setup_sql_container.py" in reporting.suite_paths(ROOT) + assert ROOT / "requirements.txt" in reporting.suite_paths(ROOT) + + +def test_query_workload_executes_and_collects(monkeypatch): + cursor = MagicMock() + cursor.fetchall.return_value = [(1,), (2,)] + connection = MagicMock() + connection.cursor.return_value.__enter__.return_value = cursor + context = MagicMock() + context.collect.return_value = ({"cpp": {}}, {"py": {}}) + monkeypatch.setattr(benchmark_workloads.time, "perf_counter", MagicMock(side_effect=[1, 1.1])) + result = benchmark_workloads.query(connection, context, "SELECT 1") + cursor.execute.assert_called_once_with("SELECT 1") + assert result["detail"] == "Rows: 2" + context.enable.assert_called_once() + context.disable.assert_called_once() + + +@pytest.mark.parametrize("named", [False, True]) +def test_parameter_workload_executes_both_binding_forms(named): + cursor = MagicMock() + cursor.fetchone.side_effect = [(value,) for value in range(100)] + connection = MagicMock() + connection.cursor.return_value.__enter__.return_value = cursor + context = MagicMock() + context.collect.return_value = ({}, {}) + result = benchmark_workloads.parameter_execution(connection, context, named=named) + expected = ("SELECT %(value)s", {"value": 0}) if named else ("SELECT ?", (0,)) + assert cursor.execute.call_args_list[0].args == expected + assert cursor.execute.call_count == 100 + assert result["detail"] == "Rows: 100" + context.disable.assert_called_once() + + +@pytest.mark.parametrize("input_sizes", [False, True]) +def test_legacy_insert_workload_executes_both_variants(input_sizes): + cursor = MagicMock() + connection = MagicMock() + connection.cursor.return_value.__enter__.return_value = cursor + context = MagicMock() + context.collect.return_value = ({}, {}) + result = benchmark_workloads.legacy_insertmany(connection, context, input_sizes=input_sizes) + assert cursor.execute.call_count == 101 + assert cursor.setinputsizes.call_count == (100 if input_sizes else 0) + assert result["detail"] == "Rows: 100000" + connection.rollback.assert_called_once() + context.disable.assert_called_once() + + +def test_suite_blobs_require_complete_authenticated_tree(): + expected = [path.relative_to(ROOT).as_posix() for path in reporting.suite_paths(ROOT)] + tree = { + "truncated": False, + "tree": [ + {"path": path, "type": "blob", "sha": f"{index + 1:040x}"} + for index, path in enumerate(expected) + ], + } + assert set(reporting.suite_blobs(tree, ROOT)) == set(expected) + tree["tree"].pop() + with pytest.raises(ValueError, match="missing"): + reporting.suite_blobs(tree, ROOT) + tree["tree"].append(None) + with pytest.raises(ValueError, match="Incomplete"): + reporting.suite_blobs(tree, ROOT) + + +def test_publisher_finishes_unavailable_when_checked_suite_file_moves(monkeypatch): + posted = [] + build = ado_build() + monkeypatch.setattr( + publisher, "publish", lambda number, head, body, base=None: posted.append(body) + ) + monkeypatch.setattr(publisher, "github", pr_topology()) + artifacts = [ + {"name": "profiler-" + leg, "resource": {"downloadUrl": "https://dev.azure.com/" + leg}} + for leg in reporting.LEGS + ] + monkeypatch.setattr( + publisher, + "api", + lambda url: {"value": artifacts if "/artifacts?" in url else [build]}, + ) + monkeypatch.setattr( + reporting, + "suite_blobs", + MagicMock(side_effect=ValueError("Benchmark suite missing from commit tree")), + ) + publisher.run(123, "c" * 40, 1) + assert len(posted) == 2 + assert "Performance assessment pending" in posted[0] + assert "Performance could not be assessed" in posted[1] + assert "required file changed" in posted[1] + + +@pytest.mark.parametrize("fail", [False, True]) +def test_worker_checkpoints_completed_and_active_scenarios(tmp_path, monkeypatch, capsys, fail): + output = tmp_path / "base-0.json" + result = dict(wall_ms=10.0, cpp={"ddbc::run": {}}, py={}, detail="Rows: 10") + profiler = MagicMock() + profiler.__enter__.return_value = profiler + profiler._conn.cursor.return_value.__enter__.return_value.fetchone.return_value = ("16.0",) + + def run(name): + partial = json.loads(output.read_text()) + assert partial["active_scenario"] == name + if name == "second": + assert set(partial["scenarios"]) == {"first"} + if fail: + raise RuntimeError("workload failure") + return [result] + + profiler.run.side_effect = run + core = SimpleNamespace(Profiler=lambda: profiler) + workloads = SimpleNamespace(registry=lambda: {"first": None, "second": None}) + monkeypatch.setattr(controller, "check_build", lambda *a, **kw: None) + monkeypatch.setattr(controller, "load_suite", lambda: (core, workloads)) + args = SimpleNamespace(source_root=tmp_path, scenarios=None, output=output) + if fail: + with pytest.raises(RuntimeError, match="workload failure"): + controller.worker(args) + assert json.loads(output.read_text())["active_scenario"] == "second" + else: + controller.worker(args) + final = json.loads(output.read_text()) + assert final["environment"]["sql_version"] == "16.0" + assert set(final["scenarios"]) == {"first", "second"} + assert "active_scenario" not in final + assert "Starting scenario: second" in capsys.readouterr().out + profiler.__exit__.assert_called_once() + + +def test_measure_timeout_retains_partial_results_and_log(tmp_path, monkeypatch): + output = tmp_path / "base-0.json" + output.write_text('{"stale": true}') + + def timeout(command, **kwargs): + assert not output.exists() + assert command[1] == "-u" + assert kwargs["timeout"] == 3 + output.write_text('{"status":"running","active_scenario":"fetchone"}') + kwargs["stdout"].write("Starting scenario: fetchone\n") + raise subprocess.TimeoutExpired(command, 3) + + monkeypatch.setattr(controller.subprocess, "run", timeout) + with pytest.raises(subprocess.TimeoutExpired): + controller.measure(tmp_path, output, ["fetchone"], timeout=3) + assert json.loads(output.read_text())["active_scenario"] == "fetchone" + assert "Starting scenario: fetchone" in output.with_suffix(".log").read_text() + + +@pytest.mark.skipif(os.name == "nt", reason="exercises POSIX process groups") +def test_build_timeout_terminates_descendants(tmp_path, monkeypatch): + pybind = tmp_path / "mssql_python/pybind" + pybind.mkdir(parents=True) + pid_file = tmp_path / "descendant.pid" + monkeypatch.setenv("DESCENDANT_PID", str(pid_file)) + (pybind / "build.sh").write_text( + "#!/usr/bin/env bash\n" + f'"{sys.executable}" -c "import time; time.sleep(60)" &\n' + 'echo "$!" > "$DESCENDANT_PID"\n' + "wait\n", + encoding="utf-8", + ) + + descendant = None + try: + with pytest.raises(subprocess.TimeoutExpired): + controller.build(tmp_path, tmp_path / "build.log", timeout=1) + descendant = int(pid_file.read_text(encoding="utf-8")) + deadline = time.monotonic() + 5 + while time.monotonic() < deadline: + try: + os.kill(descendant, 0) + except ProcessLookupError: + break + stat = Path(f"/proc/{descendant}/stat") + if stat.is_file() and stat.read_text(encoding="utf-8").split()[2] == "Z": + break + time.sleep(0.05) + else: + pytest.fail("build descendant survived timeout cleanup") + finally: + if descendant is not None: + try: + os.kill(descendant, signal.SIGKILL) + except ProcessLookupError: + pass + + +def test_windows_process_tree_cleanup_uses_taskkill(monkeypatch): + process = MagicMock(pid=123) + taskkill = MagicMock(return_value=subprocess.CompletedProcess([], 0, "")) + monkeypatch.setattr(controller, "WINDOWS", True) + monkeypatch.setattr(controller.subprocess, "run", taskkill) + controller.terminate_process_tree(process) + taskkill.assert_called_once_with( + ["taskkill", "/PID", "123", "/T", "/F"], + capture_output=True, + text=True, + ) + process.wait.assert_called_once_with(timeout=5) + + +def test_windows_process_tree_cleanup_accepts_already_exited_process(monkeypatch): + process = MagicMock(pid=123) + process.poll.return_value = 0 + monkeypatch.setattr(controller, "WINDOWS", True) + monkeypatch.setattr( + controller.subprocess, + "run", + MagicMock(return_value=subprocess.CompletedProcess([], 128, "", "not found")), + ) + controller.terminate_process_tree(process) + process.kill.assert_not_called() + + +def test_overall_budget_caps_build_and_worker_time(monkeypatch): + monkeypatch.setattr(controller.time, "monotonic", lambda: 100) + assert controller.remaining(110, controller.WORKER_TIMEOUT) == 10 + assert controller.remaining(1000, 60) == 60 + with pytest.raises(TimeoutError, match="overall"): + controller.remaining(100, controller.WORKER_TIMEOUT) + + +@pytest.mark.parametrize("scenarios,status", [(None, "complete"), (["select"], "incomplete")]) +def test_full_sample_budget_fits_slow_hosted_workers( + report, tmp_path, monkeypatch, scenarios, status +): + # Run 174385 completed workers in 169-285s. Budget twelve five-minute + # passes plus the full base-build/preflight allowance, not just measured pairs. + clock = [0] + measured = [] + monkeypatch.setattr(controller.time, "monotonic", lambda: clock[0]) + monkeypatch.setattr(controller, "resolve_revisions", lambda *a: ("a" * 40, "b" * 40)) + monkeypatch.setattr(controller, "git", lambda *a: "b" * 40) + monkeypatch.setattr(controller, "checkout", lambda *a: None) + monkeypatch.setenv("BUILD_BUILDID", "42") + monkeypatch.setenv("SYSTEM_PULLREQUEST_SOURCECOMMITID", "c" * 40) + + def build(path, log, timeout): + assert timeout >= 900 + clock[0] += 900 + + def preflight(command, **kwargs): + assert command[1:3] == ["-m", "eng.profiler_benchmarks.controller"] + assert "--check-build" in command and kwargs["timeout"] == 60 + clock[0] += 60 + + def measure(path, output, scenarios, timeout): + assert scenarios == args.scenarios + if timeout < 300: + raise subprocess.TimeoutExpired("hosted worker replay", timeout) + clock[0] += 300 + measured.append(output.name) + return copy.deepcopy(report["pairs"][0]["base"]) + + monkeypatch.setattr(controller, "build", build) + monkeypatch.setattr(controller.subprocess, "run", preflight) + monkeypatch.setattr(controller, "measure", measure) + args = SimpleNamespace( + base=None, + candidate="HEAD", + output=tmp_path, + leg=report["leg"], + samples=5, + warmups=1, + reuse_candidate=True, + scenarios=scenarios, + ) + controller.run(args) + result = reporting.validate(json.loads((tmp_path / "report.json").read_text())) + assert result["status"] == status and len(result["pairs"]) == 5 + assert measured == [ + f"{side}-{index}.json" + for index in range(6) + for side in (("base", "candidate") if index % 2 == 0 else ("candidate", "base")) + ] + assert clock[0] == 76 * 60 + assert clock[0] < controller.BENCHMARK_TIMEOUT + + +def test_ci_deadlines_include_setup_queueing_and_publication(): + pipeline = (ROOT / "eng/pipelines/pr-validation-pipeline.yml").read_text(encoding="utf-8") + for job in ("pytestonwindows", "PytestOnMacOS", "PytestOnLinux"): + section = pipeline.split(f"- job: {job}\n", 1)[1].split("\n- job:", 1)[0] + job_minutes = int(re.search(r"^ timeoutInMinutes: (\d+)$", section, re.M)[1]) + benchmark_step = section.split( + "python -m eng.profiler_benchmarks.controller --reuse-candidate", 1 + )[1] + step_minutes = int(re.search(r"^ timeoutInMinutes: (\d+)$", benchmark_step, re.M)[1]) + assert step_minutes * 60 >= controller.BENCHMARK_TIMEOUT + 10 * 60 + assert job_minutes >= step_minutes + 60 + assert publisher.WAIT_MINUTES >= job_minutes + 60 + workflow = (ROOT / ".github/workflows/pr-profiler-report.yml").read_text(encoding="utf-8") + workflow_minutes = int(re.search(r"timeout-minutes: (\d+)", workflow)[1]) + assert workflow_minutes >= publisher.WAIT_MINUTES + 10 + assert controller.LOCAL_BENCHMARK_TIMEOUT >= ( + 2 * 15 * 60 + 2 * (5 + 1) * controller.WORKER_TIMEOUT + ) + + +def test_linux_profiler_step_does_not_put_database_password_on_command_line(): + pipeline = (ROOT / "eng/pipelines/pr-validation-pipeline.yml").read_text(encoding="utf-8") + benchmark = pipeline.split("# Run performance benchmarks on Ubuntu", 1)[1] + benchmark = benchmark.split("displayName: 'Compare profiling builds", 1)[0] + assert "-e DB_PASSWORD \\" in benchmark + assert "Pwd=$(DB_PASSWORD)" not in benchmark + assert "Pwd=$DB_PASSWORD" in benchmark + + +def test_build_check_rejects_foreign_provider_and_enabled_recording(tmp_path, monkeypatch): + native = SimpleNamespace( + __file__=str(tmp_path / "binding.so"), profiling=SimpleNamespace(is_enabled=lambda: False) + ) + timer = SimpleNamespace(is_enabled=lambda: False) + package = SimpleNamespace( + __file__=str(tmp_path / "mssql_python/__init__.py"), ddbc_bindings=native, perf_timer=timer + ) + provider = SimpleNamespace(__file__=str(tmp_path / "mssql_python_odbc/__init__.py")) + monkeypatch.setitem(sys.modules, "mssql_python", package) + monkeypatch.setitem(sys.modules, "mssql_python_odbc", provider) + monkeypatch.setattr(sys, "path", sys.path[:]) + controller.check_build(tmp_path, True) + provider.__file__ = str(tmp_path.parent / "candidate-provider/__init__.py") + with pytest.raises(RuntimeError, match="outside"): + controller.check_build(tmp_path, True) + provider.__file__ = str(tmp_path / "provider/__init__.py") + timer.is_enabled = lambda: True + with pytest.raises(RuntimeError, match="recording OFF"): + controller.check_build(tmp_path, True) + + +def test_head_moving_while_listing_comments_prevents_publish(monkeypatch): + calls = [] + reads = 0 + + def api(path, **kwargs): + nonlocal reads + calls.append((path, kwargs)) + assert not kwargs, "No write allowed after head moved" + if path.startswith("pulls/"): + reads += 1 + return {"state": "open", "head": {"sha": "old" if reads == 1 else "new"}} + return [] + + monkeypatch.setattr(publisher, "github", api) + publisher.publish(1, "old", "data") + assert reads == 2 and len(calls) == 3 + + +def test_base_moving_while_listing_comments_prevents_publish(monkeypatch): + calls = [] + reads = 0 + + def api(path, **kwargs): + nonlocal reads + calls.append((path, kwargs)) + assert not kwargs, "No write allowed after base moved" + if path.startswith("pulls/"): + reads += 1 + return { + "state": "open", + "head": {"sha": "head"}, + "base": {"sha": "base" if reads == 1 else "new-base"}, + } + return [] + + monkeypatch.setattr(publisher, "github", api) + publisher.publish(1, "head", "data", "base") + assert reads == 2 and len(calls) == 3 + + +@pytest.mark.parametrize( + "corrupt", + [ + None, + "zip", + "timeout", + "scenarios", + "suite", + "source", + "base", + "provenance", + "recursion", + "deflate", + "delayed", + ], +) +def test_publisher_renders_validated_artifact_and_marks_missing_legs(report, monkeypatch, corrupt): + posted = [] + windows = copy.deepcopy(report) + windows["leg"] = "Windows-SQL2022" + for pair in windows["pairs"]: + for sample in pair.values(): + sample["environment"]["os"] = "Windows" + if corrupt == "scenarios": + report["pairs"][0]["candidate"]["scenarios"] = list(reporting.CASES) + elif corrupt == "suite": + report["suite_hash"] = "e" * 64 + data = { + "Windows-SQL2022": zip_data([("report.json", json.dumps(windows))]), + "Linux-SQL2022": ( + b"invalid ZIP" + if corrupt == "zip" + else zip_data( + [ + ( + "report.json", + ( + "[" * 2000 + "0" + "]" * 2000 + if corrupt == "recursion" + else json.dumps(report) + ), + ) + ] + ) + ), + } + build = ado_build() + if corrupt == "provenance": + del build["sourceVersion"] + artifacts = [ + { + "name": "profiler-" + leg, + "resource": {"downloadUrl": "https://dev.azure.com/" + leg}, + } + for leg in data + ] + artifact_responses = [[], artifacts] if corrupt == "delayed" else [artifacts] + clock = [0] + + def api(url): + if "/artifacts?" not in url: + return {"value": [build]} + response = ( + artifact_responses.pop(0) if len(artifact_responses) > 1 else artifact_responses[0] + ) + return {"value": response} + + monkeypatch.setattr( + publisher, "publish", lambda number, head, body, base=None: posted.append(body) + ) + monkeypatch.setattr(reporting, "suite_hash", lambda root: "d" * 64) + suite_versions = iter(({"suite": "source"}, {"suite": "base"})) + monkeypatch.setattr( + reporting, + "suite_blobs", + lambda *args: next(suite_versions) if corrupt == "source" else {"suite": "same"}, + ) + monkeypatch.setattr( + publisher, + "github", + pr_topology(base="e" * 40, merge_base="a" * 40) if corrupt == "base" else pr_topology(), + ) + monkeypatch.setattr(publisher, "api", api) + + def fetch(url, **kwargs): + leg = url.rsplit("/", 1)[-1] + if corrupt == "timeout" and leg == "Linux-SQL2022": + raise TimeoutError("timed out") + return data[leg] + + monkeypatch.setattr(publisher, "fetch", fetch) + if corrupt == "deflate": + artifact_report = reporting.artifact_report + + def corrupt_deflate(raw): + if raw == data["Linux-SQL2022"]: + raise zlib.error("corrupt deflate stream") + return artifact_report(raw) + + monkeypatch.setattr(reporting, "artifact_report", corrupt_deflate) + monkeypatch.setattr(publisher.time, "monotonic", lambda: clock[0]) + monkeypatch.setattr( + publisher.time, "sleep", lambda seconds: clock.__setitem__(0, clock[0] + seconds) + ) + publisher.run(123, "c" * 40, 1) + assert len(posted) == 2 + assert posted[0].startswith(reporting.MARKER) + if corrupt in ("base", "provenance"): + assert "Build provenance validation failed" in posted[1] + return + assert "| macOS / SQL Server 2022 | No result available" in posted[1] + if corrupt in ("suite", "source"): + assert "workload version differs from trusted base" in posted[1] + assert "consistent slowdown signals" not in posted[1] + elif corrupt in ("zip", "timeout", "scenarios", "recursion", "deflate"): + assert "### Windows / SQL Server 2022" in posted[1] + assert reporting.escape("Linux-SQL2022 (invalid artifact)") in posted[1] + assert "| Linux / SQL Server 2022 | No result available (invalid artifact) |" in posted[1] + assert posted[1].count("20 consistent slowdown signals") == 1 + else: + assert "### Windows / SQL Server 2022" in posted[1] + assert posted[1].count("40 consistent slowdown signals") == 1 + + +def test_publisher_waits_for_newer_run_after_exact_head_build_is_canceled(report, monkeypatch): + canceled = ado_build(id=41, result="canceled") + replacement = {**canceled, "id": 42, "result": "failed"} + builds = iter(([canceled], [replacement])) + posted = [] + clock = [0] + + def api(url): + return {"value": next(builds)} if "/builds?" in url else {"value": []} + + monkeypatch.setattr(publisher, "api", api) + monkeypatch.setattr(publisher, "github", pr_topology()) + monkeypatch.setattr( + publisher, "publish", lambda number, head, body, base=None: posted.append(body) + ) + monkeypatch.setattr(reporting, "suite_blobs", lambda *args: {"suite": "same"}) + monkeypatch.setattr(publisher.time, "monotonic", lambda: clock[0]) + monkeypatch.setattr( + publisher.time, "sleep", lambda seconds: clock.__setitem__(0, clock[0] + seconds) + ) + publisher.run(123, "c" * 40, 4) + assert clock[0] == 240 + assert len(posted) == 2 + assert "buildId=42" in posted[1] + + +@pytest.mark.parametrize("result", [None, "unknown"]) +def test_publisher_rejects_unsupported_completed_results(monkeypatch, result): + posted = [] + build = ado_build(result=result) + + def api(url): + assert "/builds?" in url, "Unsupported builds must not query artifacts" + return {"value": [build]} + + monkeypatch.setattr(publisher, "api", api) + monkeypatch.setattr(publisher, "github", pr_topology()) + monkeypatch.setattr( + publisher, "publish", lambda number, head, body, base=None: posted.append(body) + ) + publisher.run(123, "c" * 40, 1) + assert len(posted) == 2 + assert "unsupported result" in posted[1] + + +@pytest.mark.parametrize("status", [None, "notStarted", "inProgress"]) +def test_publisher_deadline_finishes_without_reading_unfinished_build_metadata(monkeypatch, status): + posted = [] + clock = [0] + build = ado_build(status=status, sourceVersion=None) + + def github(path): + assert path == "pulls/123", "Unfinished builds must not query merge topology" + return {"state": "open", "head": {"sha": "c" * 40}, "base": {"sha": "a" * 40}} + + def api(url): + assert "/builds?" in url, "Unfinished builds must not query artifacts" + return {"value": [] if status is None else [build]} + + def sleep(seconds): + clock[0] += seconds + + monkeypatch.setattr(publisher.time, "monotonic", lambda: clock[0]) + monkeypatch.setattr(publisher.time, "sleep", sleep) + monkeypatch.setattr(publisher, "github", github) + monkeypatch.setattr(publisher, "api", api) + monkeypatch.setattr( + publisher, "publish", lambda number, head, body, base=None: posted.append(body) + ) + publisher.run(123, "c" * 40, 1) + assert clock[0] == 60 and len(posted) == 2 + assert "Performance assessment pending" in posted[0] + assert "Performance assessment pending" not in posted[1] + assert "1-minute wait" in posted[1] + assert "Performance could not be assessed" in posted[1] + + +def test_publisher_retries_transient_polling_failures_before_finalizing(monkeypatch): + posted = [] + clock = [0] + responses = iter((URLError("temporary"), ValueError("bad JSON"), {"value": []})) + monkeypatch.setattr( + publisher, "publish", lambda number, head, body, base=None: posted.append(body) + ) + monkeypatch.setattr(publisher, "github", pr_topology()) + monkeypatch.setattr(publisher, "api", lambda url: next(responses)) + monkeypatch.setattr(publisher.time, "monotonic", lambda: clock[0]) + monkeypatch.setattr( + publisher.time, "sleep", lambda seconds: clock.__setitem__(0, clock[0] + seconds) + ) + publisher.run(123, "c" * 40, 1) + assert clock[0] == 60 + assert len(posted) == 2 + assert "Performance could not be assessed" in posted[1] + + +def test_publisher_retries_malformed_pr_and_artifact_responses(monkeypatch): + posted = [] + clock = [0] + prs = iter(({}, pr_topology()("pulls/123"))) + monkeypatch.setattr( + publisher, "publish", lambda number, head, body, base=None: posted.append(body) + ) + monkeypatch.setattr(publisher, "github", lambda path: next(prs)) + monkeypatch.setattr(publisher, "api", lambda url: {"value": []}) + monkeypatch.setattr(publisher.time, "monotonic", lambda: clock[0]) + monkeypatch.setattr( + publisher.time, "sleep", lambda seconds: clock.__setitem__(0, clock[0] + seconds) + ) + publisher.run(123, "c" * 40, 1) + assert len(posted) == 2 and "Performance could not be assessed" in posted[1] + with pytest.raises(ValueError, match="artifact list"): + publisher.artifact_items({"value": [None]}) + with pytest.raises(ValueError, match="build list"): + publisher.build_items({"value": [{}]}) + malformed = ado_build() + malformed["repository"]["id"] = None + with pytest.raises(ValueError, match="build list"): + publisher.build_items({"value": [malformed]}) + + +def test_artifact_polling_uses_remaining_publication_budget(monkeypatch): + posted = [] + clock = [0] + build = ado_build() + artifacts = [ + {"name": "profiler-" + leg, "resource": {"downloadUrl": "https://dev.azure.com/" + leg}} + for leg in reporting.LEGS + ] + artifact_responses = iter([[]] * 5 + [artifacts]) + + def api(url): + return {"value": next(artifact_responses)} if "/artifacts?" in url else {"value": [build]} + + monkeypatch.setattr(publisher, "api", api) + monkeypatch.setattr(publisher, "github", pr_topology()) + monkeypatch.setattr( + publisher, "publish", lambda number, head, body, base=None: posted.append(body) + ) + monkeypatch.setattr(reporting, "assess", lambda *args: "final report") + monkeypatch.setattr(publisher.time, "monotonic", lambda: clock[0]) + monkeypatch.setattr( + publisher.time, "sleep", lambda seconds: clock.__setitem__(0, clock[0] + seconds) + ) + publisher.run(123, "c" * 40, 4) + assert clock[0] == 150 + assert posted == [ + publisher.HEADER + + "**Performance assessment pending.**\n\n" + + f"Waiting for the matching performance run for head `{'c' * 40}`.", + "final report", + ] + + +def test_artifact_symlink_and_oversized_json_are_rejected(): + symlink = zipfile.ZipInfo("report.json") + symlink.create_system = 3 + symlink.external_attr = 0o120777 << 16 + with pytest.raises(ValueError, match="Invalid report"): + reporting.artifact_report(zip_data([(symlink, "{}")])) + with pytest.raises(ValueError, match="Invalid report"): + reporting.artifact_report(zip_data([("report.json", " " * (reporting.MAX_BYTES + 1))])) + + +@pytest.mark.parametrize("environment", [{"os": "Windows"}, {"sql_version": "17.0"}]) +def test_report_leg_must_match_measured_environment(report, environment): + for pair in report["pairs"]: + for sample in pair.values(): + sample["environment"].update(environment) + with pytest.raises(ValueError, match="leg"): + reporting.validate(report) + + +def test_ci_reuses_profiling_builds_without_changing_release_defaults(): + pipeline = (ROOT / "eng/pipelines/pr-validation-pipeline.yml").read_text(encoding="utf-8") + assert "benchmarks/perf-benchmarking.py" not in pipeline + assert pipeline.count("python -m eng.profiler_benchmarks.controller --reuse-candidate") == 3 + profiler_conditions = re.findall( + r"displayName: '(?:Compare profiling builds[^']*|Publish paired profiler measurements)'\n" + r" condition: ([^\n]+)", + pipeline, + ) + assert len(profiler_conditions) == 6 + assert all( + "eq(variables['Build.Reason'], 'PullRequest')" in condition + for condition in profiler_conditions + ) + for release in (ROOT / "OneBranchPipelines").rglob("*.yml"): + assert "ENABLE_PROFILING" not in release.read_text(encoding="utf-8") + windows = pipeline.split("- job: pytestonwindows\n", 1)[1].split("\n- job:", 1)[0] + assert "##vso[task.setvariable" not in windows + assert "ENABLE_PROFILING: 1" in windows + assert "ArtifactName: 'ddbc_bindings-profiling-$(sqlVersion)'" in windows + assert "ArtifactName: 'ddbc_bindings'" in windows + assert ( + "condition: and(succeeded(), eq(variables['Build.Reason'], 'PullRequest'), " + "ne(variables['sqlVersion'], 'LocalDB'))" + ) in windows + assert ( + "condition: and(succeeded(), or(ne(variables['Build.Reason'], 'PullRequest'), " + "eq(variables['sqlVersion'], 'LocalDB')))" + ) in windows + macos = pipeline.split("- job: PytestOnMacOS\n", 1)[1].split("\n- job:", 1)[0] + assert 'if [ "$(Build.Reason)" = "PullRequest" ]; then' in macos + assert 'ENABLE_PROFILING="$PROFILER_BUILD" ./build.sh' in macos + assert "condition: and(succeeded(), eq(variables['Build.Reason'], 'PullRequest'))" in macos + linux = pipeline.split("- job: PytestOnLinux\n", 1)[1].split("\n- job:", 1)[0] + assert ( + 'if [ "$(Build.Reason)" = "PullRequest" ] && [ "$(distroName)" = "Ubuntu" ]; then' in linux + ) + assert 'if [ "$PROFILER_BUILD" = "1" ]; then' in linux + assert "python -m eng.profiler_benchmarks.controller --check-build on" in linux + benchmark = linux.split("# Run performance benchmarks on Ubuntu", 1)[1] + assert "-e BUILD_BUILDID \\" in benchmark + assert "BUILD_BUILDID: $(Build.BuildId)" in benchmark + assert "git config --global --add safe.directory /workspace" in benchmark + assert "apt-get install -y --reinstall libodbcinst2" in benchmark + assert benchmark.index("apt-get install -y --reinstall libodbcinst2") < benchmark.index( + "ACCEPT_EULA=Y apt-get install" + ) + assert "libodbc1 " not in benchmark and "odbcinst1debian2" not in benchmark + + +def test_profiler_documentation_preserves_standalone_benchmarks_and_failed_build_contract(): + benchmarks = (ROOT / "benchmarks/README.md").read_text(encoding="utf-8") + assert "perf-benchmarking.py" in benchmarks + assert "Profiler benchmark comparisons" in benchmarks + contract = (ROOT / "eng/profiler_benchmarks/README.md").read_text(encoding="utf-8") + assert "failed aggregate build can still publish" in contract + + +def test_comment_workflow_executes_only_trusted_base_code(): + workflow = (ROOT / ".github/workflows/pr-profiler-report.yml").read_text(encoding="utf-8") + assert "pull_request_target:" in workflow + assert "ref: ${{ github.event.pull_request.base.sha }}" in workflow + assert "persist-credentials: false" in workflow + assert "actions/checkout@11d5960a326750d5838078e36cf38b85af677262" in workflow + assert "actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065" in workflow + coverage = (ROOT / ".github/workflows/pr-code-coverage.yml").read_text(encoding="utf-8") + assert coverage.count("extract_coverage_artifact.py") == 2 + assert coverage.count("--max-filesize 268435456") == 2 + assert "unzip -o" not in coverage + assert '-o "$COVERAGE_ARCHIVE"' in coverage + assert '-o "$COVERAGE_XML_ARCHIVE"' in coverage + assert "-o coverage-report.zip" not in coverage + assert "-o coverage-artifacts.zip" not in coverage + assert coverage.count("._links.web.href") == 1 + assert ( + 'ADO_URL="https://dev.azure.com/sqlclientdrivers/public/_build/results?buildId=$BUILD_ID"' + in coverage + ) + assert 'cp "$COVERAGE_XML"' not in coverage + assert 'diff-cover "$COVERAGE_XML"' in coverage + assert "COVERAGE_XML: ${{ runner.temp }}/coverage.xml" in coverage + assert ( + "head.ref" not in workflow + and "head.sha }}" not in workflow.split("ref:", 1)[1].split("persist", 1)[0] + ) diff --git a/tests/test_pr_code_coverage_workflow.py b/tests/test_pr_code_coverage_workflow.py new file mode 100644 index 000000000..9f116b6db --- /dev/null +++ b/tests/test_pr_code_coverage_workflow.py @@ -0,0 +1,302 @@ +"""Run the workflow's polling shell with local curl fixtures and an accelerated clock.""" + +import json +import os +from pathlib import Path +import shutil +import subprocess +import sys +import textwrap + +import pytest + +WORKFLOW = Path(__file__).resolve().parents[1] / ".github/workflows/pr-code-coverage.yml" +pytestmark = pytest.mark.skipif( + not WORKFLOW.is_file() + or sys.platform == "win32" + or not shutil.which("bash") + or not shutil.which("jq"), + reason="requires a source checkout, Bash and jq (as on the coverage runner)", +) +SHA = "a" * 40 +ARTIFACT_URL = "https://dev.azure.com/SqlClientDrivers/public/coverage.zip" +EMPTY = {"value": []} +ARTIFACT = { + "value": [{"name": "Code Coverage Report_1", "resource": {"downloadUrl": ARTIFACT_URL}}] +} + + +def _build(build_id=174262, sha=SHA, pr="779", branch="refs/pull/779/merge", definition=2128): + return { + "id": build_id, + "definition": {"id": definition}, + "sourceBranch": branch, + "triggerInfo": {"pr.number": pr, "pr.sourceSha": sha}, + "status": "inProgress", + "_links": { + "web": { + "href": ( + "https://dev.azure.com/SqlClientDrivers/public/_build/results" + f"?buildId={build_id}" + ) + } + }, + } + + +def _script(step): + section = WORKFLOW.read_text(encoding="utf-8").split(f" - name: {step}\n", 1)[1] + section = section.split("\n - name:", 1)[0] + return textwrap.dedent(section.split(" run: |\n", 1)[1]) + + +def _run(tmp_path, script, fixtures, clock_scale=1): + for kind, responses in fixtures.items(): + (tmp_path / f"{kind}.count").write_text(str(len(responses)), encoding="utf-8") + (tmp_path / f"{kind}.next").write_text("0", encoding="utf-8") + for index, response in enumerate(responses): + code, body = response if isinstance(response, tuple) else (0, response) + body = body if isinstance(body, str) else json.dumps(body) + (tmp_path / f"{kind}.{index}.body").write_text(body, encoding="utf-8") + (tmp_path / f"{kind}.{index}.code").write_text(str(code), encoding="utf-8") + + prefix = r""" +SECONDS=0 +trap 'printf "%s\n" "$SECONDS" > "$FIXTURE_DIR/elapsed"' EXIT +sleep() { + printf "%s\n" "$1" >> "$FIXTURE_DIR/sleeps" + SECONDS=$((SECONDS + $1 * CLOCK_SCALE)) +} +curl() { + local url="${@: -1}" kind index count code + printf "%s\n" "$*" >> "$FIXTURE_DIR/requests" + case "$url" in + *"/artifacts?"*) kind=artifacts ;; + *"/builds?"*) kind=builds ;; + *"/builds/"*"?api-version="*) kind=build ;; + *) echo "Unexpected URL: $url" >&2; return 99 ;; + esac + if [[ ! -f "$FIXTURE_DIR/$kind.count" ]]; then + echo "Unexpected request: $kind" >&2 + return 99 + fi + index=$(< "$FIXTURE_DIR/$kind.next") + count=$(< "$FIXTURE_DIR/$kind.count") + printf "%s\n" "$((index + 1))" > "$FIXTURE_DIR/$kind.next" + if (( index >= count )); then index=$((count - 1)); fi + code=$(< "$FIXTURE_DIR/$kind.$index.code") + cat "$FIXTURE_DIR/$kind.$index.body" + return "$code" +} +""" + env = { + **os.environ, + "FIXTURE_DIR": str(tmp_path), + "GITHUB_ENV": str(tmp_path / "github-env"), + "PR_NUMBER": "779", + "PR_HEAD_SHA": SHA, + "BUILD_ID": "174262", + "CLOCK_SCALE": str(clock_scale), + } + result = subprocess.run( + ["bash", "--noprofile", "--norc", "-eo", "pipefail", "-c", prefix + script], + env=env, + cwd=tmp_path, + capture_output=True, + text=True, + timeout=45, + ) + requests = (tmp_path / "requests").read_text(encoding="utf-8").splitlines() + for request in requests: + assert "--fail" in request + assert "--connect-timeout 10" in request + assert "--max-time " in request + timeout = int(request.split("--max-time ", 1)[1].split()[0]) + assert 0 < timeout <= 30 + assert "Unexpected request" not in result.stderr + assert "Unexpected URL" not in result.stderr + return result + + +def _poll(tmp_path, artifacts, builds, replacements=(EMPTY,), clock_scale=1): + script = _script("Download and parse coverage report") + # Only execute discovery; downloaded report contents are never executed by these tests. + script = script.split('\nif [[ -n "$COVERAGE_ARTIFACT" &&', 1)[0] + script += '\nprintf "COVERAGE_ARTIFACT=%s\\n" "$COVERAGE_ARTIFACT"\n' + return _run( + tmp_path, + script, + {"artifacts": artifacts, "build": builds, "builds": replacements}, + clock_scale, + ) + + +def test_selects_exact_head_pr_branch_and_definition_even_when_build_failed(tmp_path): + matching = {**_build(), "status": "completed", "result": "failed"} + builds = [ + _build(174267, sha="b" * 40), + _build(174266, pr="780"), + _build(174265, branch="refs/heads/main"), + _build(174264, definition=9999), + {**_build(174263, sha=None), "sourceVersion": SHA}, + matching, + _build(174261), + ] + result = _run(tmp_path, _script("Wait for ADO build to start"), {"builds": [{"value": builds}]}) + assert result.returncode == 0, result.stdout + result.stderr + exported = (tmp_path / "github-env").read_text(encoding="utf-8") + assert "BUILD_ID=174262\n" in exported + assert f"ADO_URL={matching['_links']['web']['href']}\n" in exported + request = (tmp_path / "requests").read_text(encoding="utf-8") + assert "definitions=2128&branchName=refs%2Fpull%2F779%2Fmerge" in request + assert "queryOrder=queueTimeDescending" in request + + +def test_ignores_old_head_until_exact_build_appears_and_retries_bad_responses(tmp_path): + result = _run( + tmp_path, + _script("Wait for ADO build to start"), + { + "builds": [ + {"value": [_build(174261, sha="b" * 40)]}, + (22, "HTTP 503"), + "gateway error", + {"value": {}}, + {"value": [_build()]}, + ] + }, + ) + assert result.returncode == 0, result.stdout + result.stderr + assert "BUILD_ID=174262\n" in (tmp_path / "github-env").read_text(encoding="utf-8") + + +def test_accepts_late_artifact_after_failed_aggregate_completes(tmp_path): + result = _poll( + tmp_path, + [EMPTY] * 32 + [ARTIFACT], + [_build()] * 31 + [{**_build(), "status": "completed", "result": "failed"}], + clock_scale=10, + ) + assert result.returncode == 0, result.stdout + result.stderr + assert f"COVERAGE_ARTIFACT={ARTIFACT_URL}" in result.stdout + assert "Build completed (failed)" in result.stdout + assert int((tmp_path / "elapsed").read_text()) > 150 * 60 + + +@pytest.mark.parametrize("result", ["succeeded", "failed"]) +def test_completed_without_artifact_stops_after_short_grace(tmp_path, result): + completed = {**_build(), "status": "completed", "result": result} + run = _poll(tmp_path, [EMPTY], [completed]) + assert run.returncode != 0 + assert "after propagation grace" in run.stdout + assert 120 <= int((tmp_path / "elapsed").read_text()) < 180 + + +def test_canceled_without_replacement_obeys_wall_clock_budget(tmp_path): + canceled = {**_build(), "status": "completed", "result": "canceled"} + run = _poll(tmp_path, [(22, "not found")], [canceled], clock_scale=10) + assert run.returncode != 0 + assert "has no replacement yet" in run.stdout + assert "Timeout:" in run.stdout + assert 220 * 60 <= int((tmp_path / "elapsed").read_text()) < 225 * 60 + + +def test_canceled_visible_artifact_without_replacement_is_rejected(tmp_path): + canceled = {**_build(), "status": "completed", "result": "canceled"} + run = _poll(tmp_path, [ARTIFACT], [canceled], clock_scale=10) + assert run.returncode != 0 + assert "has no replacement yet" in run.stdout + assert "Timeout:" in run.stdout + + +def test_immediately_available_artifact_is_accepted_after_lifecycle_check(tmp_path): + result = _poll(tmp_path, [ARTIFACT], [_build()]) + assert result.returncode == 0, result.stdout + result.stderr + assert f"COVERAGE_ARTIFACT={ARTIFACT_URL}" in result.stdout + assert (tmp_path / "build.next").read_text().strip() == "1" + + +def test_switches_from_canceled_run_to_newer_exact_head_build(tmp_path): + older = {**_build(174000), "status": "completed", "result": "succeeded"} + replacement = _build(175449) + result = _poll( + tmp_path, + [ARTIFACT, ARTIFACT], + [{**_build(), "status": "completed", "result": "canceled"}, replacement], + [{"value": [older, replacement]}], + ) + assert result.returncode == 0, result.stdout + result.stderr + assert "continuing with replacement build 175449" in result.stdout + exported = (tmp_path / "github-env").read_text(encoding="utf-8") + assert "BUILD_ID=175449\n" in exported + assert "buildId=175449\n" in exported + + +def test_artifact_and_lifecycle_http_json_errors_are_retried(tmp_path): + result = _poll( + tmp_path, + [(22, "HTTP 502"), "{invalid", {"value": None}, EMPTY, ARTIFACT], + [(28, ""), "not json", {"status": "completed"}, _build()], + ) + assert result.returncode == 0, result.stdout + result.stderr + assert f"COVERAGE_ARTIFACT={ARTIFACT_URL}" in result.stdout + + +def test_xml_artifact_refresh_retries_http_and_json_errors(tmp_path): + script = _script("Download coverage XML from ADO") + script = script.replace("BUILD_ID=${{ env.BUILD_ID }}\n", "") + script = script.split('\necho "🔍 Available artifacts:"', 1)[0] + result = _run( + tmp_path, + script, + {"artifacts": [(22, "HTTP 503"), "invalid JSON", ARTIFACT]}, + ) + assert result.returncode == 0, result.stdout + result.stderr + assert (tmp_path / "artifacts.next").read_text().strip() == "3" + + +@pytest.mark.parametrize("failing_api", ["builds", "artifacts", "build"]) +def test_persistent_api_errors_have_finite_retries(tmp_path, failing_api): + if failing_api == "builds": + result = _run(tmp_path, _script("Wait for ADO build to start"), {"builds": ["not JSON"]}) + else: + result = _poll( + tmp_path, + ["not JSON"] if failing_api == "artifacts" else [EMPTY], + ["not JSON"] if failing_api == "build" else [_build()], + ) + assert result.returncode != 0 + assert "5 consecutive failures" in result.stdout + assert int((tmp_path / f"{failing_api}.next").read_text()) == 5 + assert int((tmp_path / "elapsed").read_text()) < 180 + + +@pytest.mark.parametrize("step,budget", [("build", 15 * 60), ("artifact", 220 * 60)]) +def test_missing_build_or_queued_coverage_obeys_wall_clock_budget(tmp_path, step, budget): + if step == "build": + result = _run( + tmp_path, + _script("Wait for ADO build to start"), + {"builds": [{"value": [_build(174261, sha="b" * 40)]}]}, + ) + else: + result = _poll( + tmp_path, + [EMPTY], + [{**_build(), "status": "notStarted"}], + clock_scale=10, + ) + assert result.returncode != 0 + assert "Timeout:" in result.stdout + assert budget <= int((tmp_path / "elapsed").read_text()) < budget + 300 + + +def test_job_budget_leaves_time_for_downloads_and_publishing(): + workflow = WORKFLOW.read_text(encoding="utf-8") + assert ( + "concurrency:\n" + " group: pr-code-coverage-${{ github.event.pull_request.number }}\n" + " cancel-in-progress: true\n" + ) in workflow + assert " timeout-minutes: 245\n" in workflow + assert "PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }}" in workflow