fix(dora): use two-phase deployment lookup to prevent first-deploymen…#8942
Open
bujjibabukatta wants to merge 1 commit into
Open
fix(dora): use two-phase deployment lookup to prevent first-deploymen…#8942bujjibabukatta wants to merge 1 commit into
bujjibabukatta wants to merge 1 commit into
Conversation
…t over-mapping in lead time calculator
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
1 participant
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
Fixes the "wrong deployment reference" bug where PRs were incorrectly
associated with old deployments, causing massively skewed median lead
time for change metrics.
Root cause
batchFetchDeploymentsused only diff-based mapping, whichdeliberately excludes the very first deployment in a collected window
(via
prev_success_deployment_commit_id <> ''). This guard is correctfor the diff path — without it, the first deployment would absorb every
historical PR via
commits_diffs. However, it also blocked a legitimatecase: when a deployment's own
commit_shadirectly equals a PR'smerge_commit_sha, that association is precise and safe, even for thefirst deployment.
This caused the scenario reported in #8790: projects using the GitLab
connector to onboard historical deployments for the first time would see
the first deployment incorrectly pull in all old MRs.
Fix
Two-phase strategy in
batchFetchDeployments:Phase 1 — direct match (new):
Query all successful PRODUCTION deployments in the project, keyed by
commit_sha. If a deployment'scommit_shaequals the PR'smerge_commit_sha, associate them. Nocommits_diffsjoin needed —the match is exact, so there is no over-mapping risk.
Phase 2 — diff-based fallback (unchanged):
Retain the existing
commits_diffsjoin with theprev_success_deployment_commit_id <> ''guard. Only fills in PRs notalready resolved by Phase 1.
Test coverage
All existing e2e test cases in
calculate_change_lead_time_test.gopass without change. In the existing fixtures, PR
merge_commit_shavalues are distinct from deployment
commit_shavalues, so Phase 1produces no matches and Phase 2 runs identically to before — verifying
no regression.
The bug scenario (Phase 1 actually matching) is the real-world case
described in #8790 and #8188 and is not yet covered by a fixture test.
A follow-up test case with
deployment.commit_sha == pr.merge_commit_shawould be a good addition.
Related issues
Closes #8790
Related: #8188, #8249, #7193