[CI] Add commit-only fallback when resolving serverless/single-step Azure build artifacts#8782
Merged
Merged
Conversation
This comment has been minimized.
This comment has been minimized.
NachoEchevarria
changed the title
NachoEchevarria
added
the
area:builds
NachoEchevarria
added
the
AI Generated
BenchmarksBenchmark execution time: 2026-06-29 10:49:00 Comparing candidate commit 871ee93 in PR branch Found 0 performance improvements and 1 performance regressions! Performance is the same for 71 metrics, 0 unstable metrics, 58 known flaky benchmarks, 68 flaky benchmarks without significant changes.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c172248b7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # So match on the commit SHA first, across all branches. A build "carries" the commit when | ||
| # it is a PR build with pr.sourceSha == SHA, or any build with sourceVersion == SHA. Prefer | ||
| # non-scheduled builds, but fall back to a scheduled build for the same SHA. | ||
| allBuildsUrl="https://dev.azure.com/datadoghq/dd-trace-dotnet/_apis/build/builds?api-version=7.1&definitions=54&\$top=200&queryOrder=queueTimeDescending" |
There was a problem hiding this comment.
With this URL the script only searches the latest 200 builds across the entire definition before exiting. In a GitLab pipeline for a release/hotfix/PR branch whose matching Azure build has aged past the global top 200, this now reports no build even though the previous branch-scoped query (branchName=$branchName&$top=10) would still find one because Azure filters by branch before applying $top. This can break artifact downloads for quieter branches during periods with many master/PR builds; keeping the old branch-keyed lookup as a fallback avoids that regression. The same pattern is duplicated in download-single-step-artifacts.sh.
Useful? React with 👍 / 👎.
| # non-scheduled builds, but fall back to a scheduled build for the same SHA. | ||
| allBuildsUrl="https://dev.azure.com/datadoghq/dd-trace-dotnet/_apis/build/builds?api-version=7.1&definitions=54&\$top=200&queryOrder=queueTimeDescending" | ||
| buildId=$(curl -sS "$allBuildsUrl" | jq -r --arg version "$CI_COMMIT_SHA" ' | ||
| [ .value[] | select((.triggerInfo["pr.sourceSha"] == $version) or (.sourceVersion == $version)) ] |
There was a problem hiding this comment.
This SHA-only predicate can select any PR validation build whose source branch happened to point at the same commit, before the branch build for the current ref. In the inspected Azure workflow, PR jobs use steps/clone-repo.yml to merge the PR source with its target branch before building, so those artifacts are not necessarily the raw CI_COMMIT_SHA tree; if another PR with the same source SHA targets a different or newer base, this can download artifacts containing code from that unrelated target branch. Keep PR matches scoped to pr.sourceBranch == $CI_COMMIT_BRANCH, or prefer exact sourceVersion branch builds before cross-branch PR builds.
Useful? React with 👍 / 👎.
andrewlock
reviewed
Jun 11, 2026
Comment on lines
+32
to
+33
| # Artifacts are identified by the commit SHA, not the branch: the same SHA always produces | ||
| # the same artifacts regardless of which ref the pipeline ran on. A branch-keyed lookup |
Member
There was a problem hiding this comment.
I'm not sure this is technically true, because we can (and do) configure Azure Devops with extra variables that change the build (e.g. enable debug mode). While that should be an edge case we can ignore, it does make me a little nervous, given the single-step-artifacts download is downloading the "real" artifacts for publish... Maybe we should reverse the fallback here - e.g. try to find by pr first, then try to find for build, and then try to find by commit only? 🤔 WDYT?
Collaborator
Author
There was a problem hiding this comment.
Good point, agreed. I've modified it to try PR first, then the branch build, and only fall back to a commit-only (any-branch) match if those don't find anything. That keeps the exact build for the normal case and just rescues the mis-attributed-branch situation. Thanks!
Co-authored-by: Andrew Lock <[email protected]>
NachoEchevarria
changed the title
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8782) and master. ✅ No regressions detected - check the details below Full Metrics ComparisonFakeDbCommand
HttpMessageHandler
Comparison explanationExecution-time benchmarks measure the whole time it takes to execute a program, and are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are highlighted in **red**. The following thresholds were used for comparing the execution times:
Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard. Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph). Duration chartsFakeDbCommand (.NET Framework 4.8)gantt
title Execution time (ms) FakeDbCommand (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8782) - mean (70ms) : 68, 73
master - mean (74ms) : 63, 85
section Bailout
This PR (8782) - mean (74ms) : 72, 76
master - mean (75ms) : 73, 77
section CallTarget+Inlining+NGEN
This PR (8782) - mean (1,085ms) : 1036, 1134
master - mean (1,081ms) : 1036, 1127
FakeDbCommand (.NET Core 3.1)gantt
title Execution time (ms) FakeDbCommand (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8782) - mean (109ms) : 106, 112
master - mean (112ms) : 106, 119
section Bailout
This PR (8782) - mean (110ms) : 109, 112
master - mean (111ms) : 109, 112
section CallTarget+Inlining+NGEN
This PR (8782) - mean (776ms) : 749, 803
master - mean (775ms) : 751, 798
FakeDbCommand (.NET 6)gantt
title Execution time (ms) FakeDbCommand (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8782) - mean (97ms) : 94, 99
master - mean (99ms) : 93, 105
section Bailout
This PR (8782) - mean (97ms) : 96, 99
master - mean (102ms) : 97, 106
section CallTarget+Inlining+NGEN
This PR (8782) - mean (937ms) : 897, 976
master - mean (942ms) : 897, 986
FakeDbCommand (.NET 8)gantt
title Execution time (ms) FakeDbCommand (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8782) - mean (98ms) : 93, 103
master - mean (95ms) : 93, 97
section Bailout
This PR (8782) - mean (98ms) : 93, 102
master - mean (97ms) : 91, 103
section CallTarget+Inlining+NGEN
This PR (8782) - mean (810ms) : 775, 846
master - mean (811ms) : 779, 843
HttpMessageHandler (.NET Framework 4.8)gantt
title Execution time (ms) HttpMessageHandler (.NET Framework 4.8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8782) - mean (203ms) : 198, 208
master - mean (204ms) : 201, 207
section Bailout
This PR (8782) - mean (206ms) : 201, 211
master - mean (207ms) : 203, 210
section CallTarget+Inlining+NGEN
This PR (8782) - mean (1,205ms) : 1163, 1247
master - mean (1,214ms) : 1165, 1263
HttpMessageHandler (.NET Core 3.1)gantt
title Execution time (ms) HttpMessageHandler (.NET Core 3.1)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8782) - mean (292ms) : 287, 298
master - mean (293ms) : 288, 299
section Bailout
This PR (8782) - mean (290ms) : 287, 294
master - mean (293ms) : 289, 296
section CallTarget+Inlining+NGEN
This PR (8782) - mean (975ms) : 950, 1000
master - mean (979ms) : 954, 1004
HttpMessageHandler (.NET 6)gantt
title Execution time (ms) HttpMessageHandler (.NET 6)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8782) - mean (286ms) : 280, 292
master - mean (287ms) : 281, 293
section Bailout
This PR (8782) - mean (286ms) : 281, 290
master - mean (286ms) : 280, 292
section CallTarget+Inlining+NGEN
This PR (8782) - mean (1,170ms) : 1124, 1215
master - mean (1,173ms) : 1134, 1211
HttpMessageHandler (.NET 8)gantt
title Execution time (ms) HttpMessageHandler (.NET 8)
dateFormat x
axisFormat %Q
todayMarker off
section Baseline
This PR (8782) - mean (284ms) : 276, 293
master - mean (283ms) : 276, 290
section Bailout
This PR (8782) - mean (284ms) : 279, 288
master - mean (282ms) : 276, 288
section CallTarget+Inlining+NGEN
This PR (8782) - mean (1,047ms) : 1001, 1092
master - mean (1,050ms) : 1003, 1096
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
andrewlock
approved these changes
Jun 26, 2026
|
|
||
| if [ -z "${buildId}" ]; then | ||
| echo "No build found for commit '$CI_COMMIT_SHA' on branch '$branchName' (including PRs)" | ||
| echo "No build found carrying commit '$CI_COMMIT_SHA' (branch '$branchName') in the recent build history" |
Member
There was a problem hiding this comment.
The other script still says "No build found for commit..." (which sounds better to me than "carrying").
Not critical, but it would be nice to keep consistency across the scripts if possible (maybe event refactor the duplicated code into a share file someday?).
Collaborator
Author
There was a problem hiding this comment.
Corrected. Thanks!
lucaspimentel
approved these changes
Jun 26, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Improves the GitLab CI artifact download helpers for serverless and single-step jobs by making Azure DevOps build resolution more resilient when a GitLab branch pipeline points at a commit whose artifacts were produced on a different Azure branch (e.g., due to rebase/force-push behavior).
Changes:
- Reworks build lookup to be most-specific-first (PR build → manual/individualCI build → commit-only fallback across branches).
- Adds a tier-3 jq selector intended to prefer successful builds to avoid long waits on builds that never publish artifacts.
- Updates logging/error messages to reflect the new resolution strategy.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| .gitlab/download-single-step-artifacts.sh | Adds tier-3 commit-only fallback build resolution to locate ssi-artifacts when branch+SHA lookup fails. |
| .gitlab/download-serverless-artifacts.sh | Adds the same tier-3 commit-only fallback build resolution to locate serverless-artifacts when branch+SHA lookup fails. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+80
to
+86
| buildId=$(curl -sS "$allBuildsUrl" | jq -r --arg version "$CI_COMMIT_SHA" ' | ||
| [ .value[] | select((.triggerInfo["pr.sourceSha"] == $version) or (.sourceVersion == $version)) ] | ||
| | ( map(select(.reason != "schedule" and .result == "succeeded")) | ||
| + map(select(.reason == "schedule" and .result == "succeeded")) | ||
| + map(select(.reason != "schedule")) | ||
| + map(select(.reason == "schedule")) ) | ||
| | .[0].id // empty') |
Collaborator
Author
There was a problem hiding this comment.
Preferring an in-progress build over a failed-but-complete one could make us wait needlessly. Verdict: common case already handled; the extra .status reordering isn't clearly an improvement.
Comment on lines
62
to
63
| allBuildsForPrUrl="https://dev.azure.com/datadoghq/dd-trace-dotnet/_apis/build/builds?api-version=7.1&definitions=54&\$top=100&queryOrder=queueTimeDescending&reasonFilter=pullRequest" | ||
| buildId=$(curl -sS $allBuildsForPrUrl | jq --arg version $CI_COMMIT_SHA --arg branch $CI_COMMIT_BRANCH '.value[] | select(.triggerInfo["pr.sourceBranch"] == $branch and .triggerInfo["pr.sourceSha"] == $version) | .id' | head -n 1) |
Collaborator
Author
There was a problem hiding this comment.
pr.sourceBranch in triggerInfo is the short branch name, so matching it against CI_COMMIT_BRANCH (also short) is correct. The refs/heads/$CI_COMMIT_BRANCH form is only used for the tier-2 branchName= URL param, which genuinely requires the full ref. The code uses the right format for each field.
Comment on lines
68
to
69
| allBuildsForBranchUrl="https://dev.azure.com/datadoghq/dd-trace-dotnet/_apis/build/builds?api-version=7.1&definitions=54&\$top=10&queryOrder=queueTimeDescending&branchName=$branchName&reasonFilter=manual,individualCI" | ||
| buildId=$(curl -sS $allBuildsForBranchUrl | jq --arg version $CI_COMMIT_SHA '.value[] | select(.sourceVersion == $version and .reason != "schedule") | .id' | head -n 1) |
Comment on lines
+61
to
+67
| buildId=$(curl -sS "$allBuildsUrl" | jq -r --arg version "$CI_COMMIT_SHA" ' | ||
| [ .value[] | select((.triggerInfo["pr.sourceSha"] == $version) or (.sourceVersion == $version)) ] | ||
| | ( map(select(.reason != "schedule" and .result == "succeeded")) | ||
| + map(select(.reason == "schedule" and .result == "succeeded")) | ||
| + map(select(.reason != "schedule")) | ||
| + map(select(.reason == "schedule")) ) | ||
| | .[0].id // empty') |
Comment on lines
+42
to
44
| # 1. PR build for this branch + commit | ||
| allBuildsForPrUrl="https://dev.azure.com/datadoghq/dd-trace-dotnet/_apis/build/builds?api-version=7.1&definitions=54&\$top=100&queryOrder=queueTimeDescending&reasonFilter=pullRequest" | ||
| buildId=$(curl -sS $allBuildsForPrUrl | jq --arg version $CI_COMMIT_SHA --arg branch $CI_COMMIT_BRANCH '.value[] | select(.triggerInfo["pr.sourceBranch"] == $branch and .triggerInfo["pr.sourceSha"] == $version) | .id' | head -n 1) |
Comment on lines
49
to
50
| allBuildsForBranchUrl="https://dev.azure.com/datadoghq/dd-trace-dotnet/_apis/build/builds?api-version=7.1&definitions=54&\$top=10&queryOrder=queueTimeDescending&branchName=$branchName&reasonFilter=manual,individualCI" | ||
| buildId=$(curl -sS $allBuildsForBranchUrl | jq --arg version $CI_COMMIT_SHA '.value[] | select(.sourceVersion == $version and .reason != "schedule") | .id' | head -n 1) |
Collaborator
Author
|
Thanks for the feedback and reviews |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The serverless and single-step artifact download scripts (
.gitlab/download-serverless-artifacts.sh,.gitlab/download-single-step-artifacts.sh) resolved the Azure DevOps build by matching branch name + commit SHA. This intermittently failed:Root cause:
cf138c11is a normalmastercommit whose artifacts live on themasterAzure builds. But the GitLab pipeline ran withCI_COMMIT_BRANCH=dudik/bounded-capture-filters— a feature branch whose tip transiently parked on this commit during a rebase/force-push, firing a branch pipeline for a(branch, SHA)pair that has no Azure build. The branch-keyed lookups never checkedmaster, where the artifacts actually exist →exit 1.Change
Resolve the build most-specific-first, stopping at the first hit:
Why most-specific-first: Azure builds can be parameterized (e.g. debug mode), so when the branch is correct we still pick the exact
(branch, commit)build, preserving today's behavior. Tier 3 only fires when the precise build genuinely can't be found.Why prefer completed-successful in tier 3: when several runs carry the same SHA, this avoids locking onto a queued/canceled/failed build that never reaches the artifact-publish stage and polling it for 40 minutes before failing. Applied only to tier 3 — tiers 1–2 intentionally wait on an in-progress build for the exact branch. If no successful build exists, tier 3 still falls back to an in-progress one rather than failing.
Test coverage
Validated against the live Azure DevOps API for the failing inputs (
branch=dudik/bounded-capture-filters,sha=cf138c11):0matches → reproduces the failure.203307, with bothserverless-artifactsandssi-artifactsconfirmed present.exit 1);bash -npasses on both scripts.Notes
check-azure-pipelinejobs do an equivalent lookup but delegate to an externalwait-for-pipeline.sh(out of scope here); flagged separately for those owners.