Skip to content

Commit 082a402

Browse files
bujjibabukattaBujjibabukattaklesh
authored
fix(dora): use two-phase deployment lookup to prevent first-deploymen… (apache#8942)
* fix(dora): use two-phase deployment lookup to prevent first-deployment over-mapping in lead time calculator * test(dora): add e2e coverage for Phase 1 direct deployment match (issue apache#8790) * test(dora): add direct-match deployment case to change lead time e2e test * fix(dora): use straight double quotes in doc comment to satisfy gofmt --------- Co-authored-by: Bujjibabukatta <bujjubabukatta6@gmail.com> Co-authored-by: Klesh Wong <klesh@qq.com>
1 parent f93895f commit 082a402

6 files changed

Lines changed: 37 additions & 24 deletions

File tree

backend/plugins/dora/e2e/change_lead_time/cicd_deployment_commits.csv

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,5 @@ id,result,started_date,duration_sec,cicd_deployment_id,cicd_scope_id,repo_url,en
1414
13,SUCCESS,2023-04-13T07:56:39.000+00:00,60,pipeline7,cicd2,REPO111,PRODUCTION,3,,commit13,2023-4-13 7:56:39,2023-04-13T07:57:39.000+00:00
1515
14,FAILURE,2023-04-13T07:57:26.000+00:00,60,pipeline8,cicd3,REPO111,PRODUCTION,,,commit14,2023-4-13 7:57:26,2023-04-13T07:58:26.000+00:00
1616
15,SUCCESS,2023-04-13T07:57:45.000+00:00,60,pipeline9,cicd3,REPO111,PRODUCTION,,,commit15,2023-4-13 7:57:45,2023-04-13T07:58:45.000+00:00
17-
16,SUCCESS,2023-04-13T07:58:24.000+00:00,60,pipeline10,cicd3,REPO333,,,,commit16,2023-4-13 7:58:24,2023-04-13T07:59:24.000+00:00
17+
16,SUCCESS,2023-04-13T07:58:24.000+00:00,60,pipeline10,cicd3,REPO333,,,,commit16,2023-4-13 7:58:24,2023-04-13T07:59:24.000+00:00
18+
17,SUCCESS,2023-04-13T07:59:00.000+00:00,60,pipeline11,cicd1,REPO111,PRODUCTION,,project1,direct_commit1,2023-4-13 7:59:00,2023-04-13T08:00:00.000+00:00

backend/plugins/dora/e2e/change_lead_time/project_pr_metrics.csv

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,4 @@ pr2,project1,2537845559d8db99e9cda6190f32b50ec979c722,,comment04,1,60,5,1538,159
55
pr3,project1,55f445997abbd5918da59d202d28762cd56fbd44,5883,comment07,,5760,6,,10203,2023-04-07T04:51:47.000+00:00,2023-04-10T06:53:51.000+00:00,2023-04-11T06:53:51.000+00:00,2023-04-14T06:53:51.000+00:00,2023-04-13T07:30:34.000+00:00
66
pr4,project1,5ad0c09c447c19338f1dfbb65d89a3728962b3b7,11704,comment10,1500,,,,11764,2023-04-05T04:51:47.000+00:00,2023-04-14T08:55:01.000+00:00,2023-04-13T07:55:01.000+00:00,2023-04-13T08:55:01.000+00:00,
77
pr5,project1,62535543802631a0d3daf0b0b78c6a7e05e508fb,13144,comment12,,313068,,,13204,2023-04-04T04:51:47.000+00:00,2022-09-07T23:07:13.000+00:00,2023-04-13T07:55:01.000+00:00,2023-04-13T08:55:01.000+00:00,
8+
pr7,project1,pr7_commit0,1440,comment13,30,30,9,1433,2933,2023-04-11T07:00:00.000+00:00,2023-04-12T07:30:00.000+00:00,2023-04-12T07:00:00.000+00:00,2023-04-12T08:00:00.000+00:00,2023-04-13T07:52:26.000+00:00

backend/plugins/dora/e2e/change_lead_time/pull_request_comments.csv

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,4 @@ comment09,pr3,2023-4-12 6:53:51,i
1212
comment10,pr4,2023-4-14 8:55:01,j
1313
comment11,pr4,2023-4-14 8:55:01,k
1414
comment12,pr5,2022-09-07 23:07:13,l
15+
comment13,pr7,2023-4-12 7:30:00,m

backend/plugins/dora/e2e/change_lead_time/pull_request_commits.csv

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,4 @@ pr0_commit0,pr0,2022-1-10 4:51:47,
1212
56b895f0443730c6d7abfbc51a05ab35abd2971f,pr4,2023-4-06 4:51:47,
1313
5ad0c09c447c19338f1dfbb65d89a3728962b3b7,pr4,2023-4-05 4:51:47,
1414
62535543802631a0d3daf0b0b78c6a7e05e508fb,pr5,2023-4-04 4:51:47,
15+
pr7_commit0,pr7,2023-4-11 7:00:00,

backend/plugins/dora/e2e/change_lead_time/pull_requests.csv

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,4 @@ pr3,repo1,a,pr_merge_commit3,2023-4-11 6:53:51,2023-4-14 6:53:51,deployment_comm
66
pr4,repo1,,pr_merge_commit4,2023-4-13 7:55:01,2023-4-13 8:55:01,,,
77
pr5,repo1,,pr_merge_commit5,2023-4-13 7:55:01,2023-4-13 8:55:01,,,
88
pr6,repo1,,pr_merge_commit6,2023-4-13 7:55:01,,,,
9+
pr7,repo1,a,commit9,2023-4-12 7:00:00,2023-4-12 8:00:00,,,

backend/plugins/dora/tasks/change_lead_time_calculator.go

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -277,47 +277,55 @@ func batchFetchFirstReviews(projectName string, db dal.Dal) (map[string]*code.Pu
277277
// batchFetchDeployments retrieves deployment commits for all merge commits in the given project.
278278
// Returns a map indexed by merge commit SHA for O(1) lookup performance.
279279
//
280-
// The query finds the first successful production deployment for each merge commit by:
281-
// 1. Finding deployment commits that have a previous successful deployment
282-
// 2. Joining with commits_diffs to find which deployment included each merge commit
283-
// 3. Filtering for successful production deployments
284-
// 4. Ordering by started_date to get the earliest deployment
280+
// Uses a two-phase strategy to avoid the "first deployment over-mapping" problem:
285281
//
286-
// The map is indexed by merge_sha (from commits_diffs), not by deployment commit_sha,
287-
// because the caller needs to look up deployments by PR merge_commit_sha.
282+
// Phase 1 - Direct match: find successful PRODUCTION deployments whose commit_sha
283+
// directly equals a PR's merge_commit_sha. Safe even for the very first deployment.
284+
//
285+
// Phase 2 - Diff-based fallback: use the commits_diffs join strategy, but deliberately
286+
// skip the first deployment (prev_success_deployment_commit_id == "") to avoid over-mapping.
288287
func batchFetchDeployments(projectName string, db dal.Dal) (map[string]*devops.CicdDeploymentCommit, errors.Error) {
289-
var results []*deploymentCommitWithMergeSha
290-
291-
// Query finds the first deployment for each merge commit by using a window function
292-
// to rank deployments by started_date, then filtering to keep only rank 1.
288+
deploymentMap := make(map[string]*devops.CicdDeploymentCommit)
289+
var directResults []*devops.CicdDeploymentCommit
293290
err := db.All(
294-
&results,
291+
&directResults,
292+
dal.Select("dc.*"),
293+
dal.From("cicd_deployment_commits dc"),
294+
dal.Join("LEFT JOIN project_mapping pm ON pm.table = 'cicd_scopes' AND pm.row_id = dc.cicd_scope_id"),
295+
dal.Where("dc.environment = 'PRODUCTION'"), // TODO: remove when multi-environment is supported
296+
dal.Where("dc.result = ? AND pm.project_name = ?", devops.RESULT_SUCCESS, projectName),
297+
dal.Orderby("dc.started_date ASC, dc.id ASC"),
298+
)
299+
if err != nil {
300+
return nil, errors.Default.Wrap(err, "failed to batch fetch direct deployments")
301+
}
302+
for _, dc := range directResults {
303+
if _, exists := deploymentMap[dc.CommitSha]; !exists {
304+
deploymentCopy := *dc
305+
deploymentMap[dc.CommitSha] = &deploymentCopy
306+
}
307+
}
308+
var diffResults []*deploymentCommitWithMergeSha
309+
err = db.All(
310+
&diffResults,
295311
dal.Select("dc.*, cd.commit_sha as merge_sha"),
296312
dal.From("cicd_deployment_commits dc"),
297313
dal.Join("LEFT JOIN cicd_deployment_commits p ON dc.prev_success_deployment_commit_id = p.id"),
298314
dal.Join("INNER JOIN commits_diffs cd ON cd.new_commit_sha = dc.commit_sha AND cd.old_commit_sha = COALESCE(p.commit_sha, '')"),
299315
dal.Join("LEFT JOIN project_mapping pm ON pm.table = 'cicd_scopes' AND pm.row_id = dc.cicd_scope_id"),
300316
dal.Where("dc.prev_success_deployment_commit_id <> ''"),
301-
dal.Where("dc.environment = 'PRODUCTION'"), // TODO: remove this when multi-environment is supported
317+
dal.Where("dc.environment = 'PRODUCTION'"), // TODO: remove when multi-environment is supported
302318
dal.Where("dc.result = ? AND pm.project_name = ?", devops.RESULT_SUCCESS, projectName),
303319
dal.Orderby("cd.commit_sha, dc.started_date ASC, dc.id ASC"),
304320
)
305-
306321
if err != nil {
307-
return nil, errors.Default.Wrap(err, "failed to batch fetch deployments")
322+
return nil, errors.Default.Wrap(err, "failed to batch fetch diff-based deployments")
308323
}
309-
310-
// Build the map indexed by merge_sha for O(1) lookup.
311-
// Keep only the first deployment for each merge commit (earliest by started_date).
312-
deploymentMap := make(map[string]*devops.CicdDeploymentCommit, len(results))
313-
for _, result := range results {
314-
// Only keep the first deployment for each merge_sha
324+
for _, result := range diffResults {
315325
if _, exists := deploymentMap[result.MergeSha]; !exists {
316-
// Copy the CicdDeploymentCommit without the MergeSha field
317326
deploymentCopy := result.CicdDeploymentCommit
318327
deploymentMap[result.MergeSha] = &deploymentCopy
319328
}
320329
}
321-
322330
return deploymentMap, nil
323331
}

0 commit comments

Comments
 (0)