This is an automated email from the ASF dual-hosted git repository.
klesh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/devlake.git
The following commit(s) were added to refs/heads/main by this push:
new 082a40242 fix(dora): use two-phase deployment lookup to prevent
first-deploymen… (#8942)
082a40242 is described below
commit 082a40242db3244ab9d24598d3b9ee4b3e799207
Author: bujjibabukatta <[email protected]>
AuthorDate: Wed Jul 15 04:20:13 2026 -0400
fix(dora): use two-phase deployment lookup to prevent first-deploymen…
(#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
#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 <[email protected]>
Co-authored-by: Klesh Wong <[email protected]>
---
.../change_lead_time/cicd_deployment_commits.csv | 3 +-
.../e2e/change_lead_time/project_pr_metrics.csv | 1 +
.../e2e/change_lead_time/pull_request_comments.csv | 1 +
.../e2e/change_lead_time/pull_request_commits.csv | 1 +
.../dora/e2e/change_lead_time/pull_requests.csv | 1 +
.../dora/tasks/change_lead_time_calculator.go | 54 +++++++++++++---------
6 files changed, 37 insertions(+), 24 deletions(-)
diff --git
a/backend/plugins/dora/e2e/change_lead_time/cicd_deployment_commits.csv
b/backend/plugins/dora/e2e/change_lead_time/cicd_deployment_commits.csv
index 9517c2520..9d1e488f0 100644
--- a/backend/plugins/dora/e2e/change_lead_time/cicd_deployment_commits.csv
+++ b/backend/plugins/dora/e2e/change_lead_time/cicd_deployment_commits.csv
@@ -14,4 +14,5 @@
id,result,started_date,duration_sec,cicd_deployment_id,cicd_scope_id,repo_url,en
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
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
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
-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
\ No newline at end of file
+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,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
\ No newline at end of file
diff --git a/backend/plugins/dora/e2e/change_lead_time/project_pr_metrics.csv
b/backend/plugins/dora/e2e/change_lead_time/project_pr_metrics.csv
index 1c1e045f5..1d9c736a5 100644
--- a/backend/plugins/dora/e2e/change_lead_time/project_pr_metrics.csv
+++ b/backend/plugins/dora/e2e/change_lead_time/project_pr_metrics.csv
@@ -5,3 +5,4 @@
pr2,project1,2537845559d8db99e9cda6190f32b50ec979c722,,comment04,1,60,5,1538,159
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
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,
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,
+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
\ No newline at end of file
diff --git
a/backend/plugins/dora/e2e/change_lead_time/pull_request_comments.csv
b/backend/plugins/dora/e2e/change_lead_time/pull_request_comments.csv
index 916a6f57d..513e617b0 100644
--- a/backend/plugins/dora/e2e/change_lead_time/pull_request_comments.csv
+++ b/backend/plugins/dora/e2e/change_lead_time/pull_request_comments.csv
@@ -12,3 +12,4 @@ comment09,pr3,2023-4-12 6:53:51,i
comment10,pr4,2023-4-14 8:55:01,j
comment11,pr4,2023-4-14 8:55:01,k
comment12,pr5,2022-09-07 23:07:13,l
+comment13,pr7,2023-4-12 7:30:00,m
\ No newline at end of file
diff --git a/backend/plugins/dora/e2e/change_lead_time/pull_request_commits.csv
b/backend/plugins/dora/e2e/change_lead_time/pull_request_commits.csv
index a6e112d9f..e183d14cc 100644
--- a/backend/plugins/dora/e2e/change_lead_time/pull_request_commits.csv
+++ b/backend/plugins/dora/e2e/change_lead_time/pull_request_commits.csv
@@ -12,3 +12,4 @@ pr0_commit0,pr0,2022-1-10 4:51:47,
56b895f0443730c6d7abfbc51a05ab35abd2971f,pr4,2023-4-06 4:51:47,
5ad0c09c447c19338f1dfbb65d89a3728962b3b7,pr4,2023-4-05 4:51:47,
62535543802631a0d3daf0b0b78c6a7e05e508fb,pr5,2023-4-04 4:51:47,
+pr7_commit0,pr7,2023-4-11 7:00:00,
\ No newline at end of file
diff --git a/backend/plugins/dora/e2e/change_lead_time/pull_requests.csv
b/backend/plugins/dora/e2e/change_lead_time/pull_requests.csv
index 46a205077..3a3ec7ae9 100644
--- a/backend/plugins/dora/e2e/change_lead_time/pull_requests.csv
+++ b/backend/plugins/dora/e2e/change_lead_time/pull_requests.csv
@@ -6,3 +6,4 @@ pr3,repo1,a,pr_merge_commit3,2023-4-11 6:53:51,2023-4-14
6:53:51,deployment_comm
pr4,repo1,,pr_merge_commit4,2023-4-13 7:55:01,2023-4-13 8:55:01,,,
pr5,repo1,,pr_merge_commit5,2023-4-13 7:55:01,2023-4-13 8:55:01,,,
pr6,repo1,,pr_merge_commit6,2023-4-13 7:55:01,,,,
+pr7,repo1,a,commit9,2023-4-12 7:00:00,2023-4-12 8:00:00,,,
diff --git a/backend/plugins/dora/tasks/change_lead_time_calculator.go
b/backend/plugins/dora/tasks/change_lead_time_calculator.go
index 47749b1af..9861a5c7a 100644
--- a/backend/plugins/dora/tasks/change_lead_time_calculator.go
+++ b/backend/plugins/dora/tasks/change_lead_time_calculator.go
@@ -277,47 +277,55 @@ func batchFetchFirstReviews(projectName string, db
dal.Dal) (map[string]*code.Pu
// batchFetchDeployments retrieves deployment commits for all merge commits in
the given project.
// Returns a map indexed by merge commit SHA for O(1) lookup performance.
//
-// The query finds the first successful production deployment for each merge
commit by:
-// 1. Finding deployment commits that have a previous successful deployment
-// 2. Joining with commits_diffs to find which deployment included each merge
commit
-// 3. Filtering for successful production deployments
-// 4. Ordering by started_date to get the earliest deployment
+// Uses a two-phase strategy to avoid the "first deployment over-mapping"
problem:
//
-// The map is indexed by merge_sha (from commits_diffs), not by deployment
commit_sha,
-// because the caller needs to look up deployments by PR merge_commit_sha.
+// Phase 1 - Direct match: find successful PRODUCTION deployments whose
commit_sha
+// directly equals a PR's merge_commit_sha. Safe even for the very first
deployment.
+//
+// Phase 2 - Diff-based fallback: use the commits_diffs join strategy, but
deliberately
+// skip the first deployment (prev_success_deployment_commit_id == "") to
avoid over-mapping.
func batchFetchDeployments(projectName string, db dal.Dal)
(map[string]*devops.CicdDeploymentCommit, errors.Error) {
- var results []*deploymentCommitWithMergeSha
-
- // Query finds the first deployment for each merge commit by using a
window function
- // to rank deployments by started_date, then filtering to keep only
rank 1.
+ deploymentMap := make(map[string]*devops.CicdDeploymentCommit)
+ var directResults []*devops.CicdDeploymentCommit
err := db.All(
- &results,
+ &directResults,
+ dal.Select("dc.*"),
+ dal.From("cicd_deployment_commits dc"),
+ dal.Join("LEFT JOIN project_mapping pm ON pm.table =
'cicd_scopes' AND pm.row_id = dc.cicd_scope_id"),
+ dal.Where("dc.environment = 'PRODUCTION'"), // TODO: remove
when multi-environment is supported
+ dal.Where("dc.result = ? AND pm.project_name = ?",
devops.RESULT_SUCCESS, projectName),
+ dal.Orderby("dc.started_date ASC, dc.id ASC"),
+ )
+ if err != nil {
+ return nil, errors.Default.Wrap(err, "failed to batch fetch
direct deployments")
+ }
+ for _, dc := range directResults {
+ if _, exists := deploymentMap[dc.CommitSha]; !exists {
+ deploymentCopy := *dc
+ deploymentMap[dc.CommitSha] = &deploymentCopy
+ }
+ }
+ var diffResults []*deploymentCommitWithMergeSha
+ err = db.All(
+ &diffResults,
dal.Select("dc.*, cd.commit_sha as merge_sha"),
dal.From("cicd_deployment_commits dc"),
dal.Join("LEFT JOIN cicd_deployment_commits p ON
dc.prev_success_deployment_commit_id = p.id"),
dal.Join("INNER JOIN commits_diffs cd ON cd.new_commit_sha =
dc.commit_sha AND cd.old_commit_sha = COALESCE(p.commit_sha, '')"),
dal.Join("LEFT JOIN project_mapping pm ON pm.table =
'cicd_scopes' AND pm.row_id = dc.cicd_scope_id"),
dal.Where("dc.prev_success_deployment_commit_id <> ''"),
- dal.Where("dc.environment = 'PRODUCTION'"), // TODO: remove
this when multi-environment is supported
+ dal.Where("dc.environment = 'PRODUCTION'"), // TODO: remove
when multi-environment is supported
dal.Where("dc.result = ? AND pm.project_name = ?",
devops.RESULT_SUCCESS, projectName),
dal.Orderby("cd.commit_sha, dc.started_date ASC, dc.id ASC"),
)
-
if err != nil {
- return nil, errors.Default.Wrap(err, "failed to batch fetch
deployments")
+ return nil, errors.Default.Wrap(err, "failed to batch fetch
diff-based deployments")
}
-
- // Build the map indexed by merge_sha for O(1) lookup.
- // Keep only the first deployment for each merge commit (earliest by
started_date).
- deploymentMap := make(map[string]*devops.CicdDeploymentCommit,
len(results))
- for _, result := range results {
- // Only keep the first deployment for each merge_sha
+ for _, result := range diffResults {
if _, exists := deploymentMap[result.MergeSha]; !exists {
- // Copy the CicdDeploymentCommit without the MergeSha
field
deploymentCopy := result.CicdDeploymentCommit
deploymentMap[result.MergeSha] = &deploymentCopy
}
}
-
return deploymentMap, nil
}
