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 9517c252010..9d1e488f072 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 1c1e045f5ec..bb1944e38c7 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,71a5b8c9d0e1f2a3b4c5d6e7f8a9b0c1d2e3f4a5,5,,,,17,2,15,2023-04-13T07:45:00.000+00:00,,2023-04-13T07:50:00.000+00:00,2023-04-13T07:58:00.000+00:00,2023-04-13T08:00:00.00₀+₀₀:₀₀ \ 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 a6e112d9fef..939f49f6557 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, +71a5b8c9d0e1f2a3b4c5d6e7f8a9b0c1d2e3f4a5,pr7,2023-4-13 7:45: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 46a2050777d..ca71d5dc338 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,direct_commit1,2023-4-13 7:50:00,2023-4-13 7:58:00,direct match - merge_commit_sha equals deployment commit_sha,, diff --git a/backend/plugins/dora/tasks/change_lead_time_calculator.go b/backend/plugins/dora/tasks/change_lead_time_calculator.go index 47749b1af3f..f426c7bf668 100644 --- a/backend/plugins/dora/tasks/change_lead_time_calculator.go +++ b/backend/plugins/dora/tasks/change_lead_time_calculator.go @@ -277,47 +277,61 @@ 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) + // Phase 1: direct match — deployment.commit_sha == PR.merge_commit_sha. + // Safe to include even the first deployment; no risk of over-mapping. + 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 + } + } + // Phase 2: diff-based mapping — deliberately excludes deployments with no + // prev_success_deployment_commit_id to prevent the first deployment from + // absorbing all historical PRs. + 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 { + // Phase 1 takes priority; only fill gaps not already resolved by direct match. if _, exists := deploymentMap[result.MergeSha]; !exists { - // Copy the CicdDeploymentCommit without the MergeSha field deploymentCopy := result.CicdDeploymentCommit deploymentMap[result.MergeSha] = &deploymentCopy } } - return deploymentMap, nil }