diff --git a/backend/plugins/dora/e2e/calculate_change_lead_time_test.go b/backend/plugins/dora/e2e/calculate_change_lead_time_test.go index 1c591108a32..828c36091b6 100644 --- a/backend/plugins/dora/e2e/calculate_change_lead_time_test.go +++ b/backend/plugins/dora/e2e/calculate_change_lead_time_test.go @@ -18,6 +18,7 @@ limitations under the License. package e2e import ( + "regexp" "testing" "github.com/apache/devlake/core/models/common" @@ -59,3 +60,35 @@ func TestCalculateCLTimeDataFlow(t *testing.T) { IgnoreTypes: []interface{}{common.NoPKModel{}}, }) } + +func TestCalculateCLTimeWithPrHeadRefPatternDataFlow(t *testing.T) { + var plugin impl.Dora + dataflowTester := e2ehelper.NewDataFlowTester(t, "dora", plugin) + + taskData := &tasks.DoraTaskData{ + Options: &tasks.DoraOptions{ + ProjectName: "project1", + PrHeadRefPattern: "^(feat|fix)/", + }, + PrHeadRefRegexp: regexp.MustCompile("^(feat|fix)/"), + } + + dataflowTester.FlushTabler(&code.PullRequest{}) + + dataflowTester.ImportCsvIntoTabler("./change_lead_time/project_mapping.csv", &crossdomain.ProjectMapping{}) + dataflowTester.ImportCsvIntoTabler("./change_lead_time/repos.csv", &code.Repo{}) + dataflowTester.ImportCsvIntoTabler("./change_lead_time/cicd_scopes.csv", &devops.CicdScope{}) + dataflowTester.ImportCsvIntoTabler("./change_lead_time/pull_requests.csv", &code.PullRequest{}) + dataflowTester.ImportCsvIntoTabler("./change_lead_time/cicd_deployment_commits.csv", &devops.CicdDeploymentCommit{}) + dataflowTester.ImportNullableCsvIntoTabler("./change_lead_time/commits_diffs.csv", &code.CommitsDiff{}) + dataflowTester.ImportCsvIntoTabler("./change_lead_time/pull_request_comments.csv", &code.PullRequestComment{}) + dataflowTester.ImportCsvIntoTabler("./change_lead_time/pull_request_commits.csv", &code.PullRequestCommit{}) + + // only pr0 (feat/a), pr1 (fix/b) and pr4 (refs/heads/feat/d) match the pattern + dataflowTester.FlushTabler(&crossdomain.ProjectPrMetric{}) + dataflowTester.Subtask(tasks.CalculateChangeLeadTimeMeta, taskData) + dataflowTester.VerifyTableWithOptions(&crossdomain.ProjectPrMetric{}, e2ehelper.TableOptions{ + CSVRelPath: "./change_lead_time/project_pr_metrics_pr_head_ref_pattern.csv", + IgnoreTypes: []interface{}{common.NoPKModel{}}, + }) +} diff --git a/backend/plugins/dora/e2e/change_lead_time/project_pr_metrics_pr_head_ref_pattern.csv b/backend/plugins/dora/e2e/change_lead_time/project_pr_metrics_pr_head_ref_pattern.csv new file mode 100644 index 00000000000..b41c2ada610 --- /dev/null +++ b/backend/plugins/dora/e2e/change_lead_time/project_pr_metrics_pr_head_ref_pattern.csv @@ -0,0 +1,4 @@ +id,project_name,first_commit_sha,pr_coding_time,first_review_id,pr_pickup_time,pr_review_time,deployment_commit_id,pr_deploy_time,pr_cycle_time,first_commit_authored_date,first_comment_date,pr_created_date,pr_merged_date,pr_deployed_date +pr0,project1,pr0_commit0,1440,,,,,,44640,2022-01-10T04:51:47.000+00:00,,2022-01-11T04:51:47.000+00:00,2022-02-10T04:51:47.000+00:00, +pr1,project1,08d2f2b6de0fa8de4d0e2b55b4b9a2e244214029,1440,comment02,5,55,5,2978,4478,2023-04-10T04:51:47.000+00:00,2023-04-11T04:56:47.000+00:00,2023-04-11T04:51:47.000+00:00,2023-04-11T05:51:47.000+00:00,2023-04-13T07:29:14.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, 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 3a3ec7ae9af..a83370ab06f 100644 --- a/backend/plugins/dora/e2e/change_lead_time/pull_requests.csv +++ b/backend/plugins/dora/e2e/change_lead_time/pull_requests.csv @@ -1,9 +1,9 @@ -id,base_repo_id,author_id,merge_commit_sha,created_date,merged_date,_raw_data_remark,base_commit_sha,head_commit_sha -pr0,repo1,a,pr_merge_commit0,2022-1-11 4:51:47,2022-2-10 4:51:47,deployment_commit 0,, -pr1,repo1,a,pr_merge_commit1,2023-4-11 4:51:47,2023-4-11 5:51:47,deployment_commit 5,, -pr2,repo1,a,pr_merge_commit2,2023-4-12 4:51:47,2023-4-12 5:51:47,deployment_commit 5,, -pr3,repo1,a,pr_merge_commit3,2023-4-11 6:53:51,2023-4-14 6:53:51,deployment_commit 6,, -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,,, +id,base_repo_id,author_id,merge_commit_sha,created_date,merged_date,_raw_data_remark,base_commit_sha,head_commit_sha,head_ref +pr0,repo1,a,pr_merge_commit0,2022-1-11 4:51:47,2022-2-10 4:51:47,deployment_commit 0,,,feat/a +pr1,repo1,a,pr_merge_commit1,2023-4-11 4:51:47,2023-4-11 5:51:47,deployment_commit 5,,,fix/b +pr2,repo1,a,pr_merge_commit2,2023-4-12 4:51:47,2023-4-12 5:51:47,deployment_commit 5,,,chore/c +pr3,repo1,a,pr_merge_commit3,2023-4-11 6:53:51,2023-4-14 6:53:51,deployment_commit 6,,,release-uat +pr4,repo1,,pr_merge_commit4,2023-4-13 7:55:01,2023-4-13 8:55:01,,,,refs/heads/feat/d +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,,,,,feat/e +pr7,repo1,a,commit9,2023-4-12 7:00:00,2023-4-12 8:00:00,,,,hotfix/g diff --git a/backend/plugins/dora/impl/impl.go b/backend/plugins/dora/impl/impl.go index 92802a1bdc5..2b7315d16d7 100644 --- a/backend/plugins/dora/impl/impl.go +++ b/backend/plugins/dora/impl/impl.go @@ -104,8 +104,13 @@ func (p Dora) PrepareTaskData(taskCtx plugin.TaskContext, options map[string]int if err != nil { return nil, err } + prHeadRefRegexp, err := tasks.CompilePrHeadRefPattern(op.PrHeadRefPattern) + if err != nil { + return nil, err + } return &tasks.DoraTaskData{ - Options: op, + Options: op, + PrHeadRefRegexp: prHeadRefRegexp, }, nil } @@ -126,6 +131,17 @@ func (p Dora) MakeMetricPluginPipelinePlanV200(projectName string, options json. return nil, errors.Default.WrapRaw(err) } } + // validate early so that an invalid pattern fails when the plan is generated + prHeadRefRegexp, err := tasks.CompilePrHeadRefPattern(op.PrHeadRefPattern) + if err != nil { + return nil, err + } + changeLeadTimeOptions := map[string]interface{}{ + "projectName": projectName, + } + if prHeadRefRegexp != nil { + changeLeadTimeOptions["prHeadRefPattern"] = prHeadRefRegexp.String() + } plan := coreModels.PipelinePlan{ { @@ -154,10 +170,8 @@ func (p Dora) MakeMetricPluginPipelinePlanV200(projectName string, options json. }, { { - Plugin: "dora", - Options: map[string]interface{}{ - "projectName": projectName, - }, + Plugin: "dora", + Options: changeLeadTimeOptions, Subtasks: []string{ "calculateChangeLeadTime", tasks.IssuesToIncidentsMeta.Name, diff --git a/backend/plugins/dora/impl/impl_test.go b/backend/plugins/dora/impl/impl_test.go index 3fcdc58e44e..6ca36abe700 100644 --- a/backend/plugins/dora/impl/impl_test.go +++ b/backend/plugins/dora/impl/impl_test.go @@ -22,6 +22,7 @@ import ( "github.com/apache/devlake/plugins/dora/tasks" "testing" + "github.com/apache/devlake/core/errors" coreModels "github.com/apache/devlake/core/models" "github.com/stretchr/testify/assert" ) @@ -70,3 +71,58 @@ func TestMakeMetricPluginPipelinePlanV200(t *testing.T) { } assert.Equal(t, doraOutputPlan, plan) } + +func TestMakeMetricPluginPipelinePlanV200WithPrHeadRefPattern(t *testing.T) { + var dora Dora + const projectName = "TestMakePlanV200-project" + optionJson, err := json.Marshal(map[string]interface{}{ + "prHeadRefPattern": " ^(feat|fix)/ ", + }) + assert.Nil(t, err) + plan, err := dora.MakeMetricPluginPipelinePlanV200(projectName, optionJson) + assert.Nil(t, err) + assert.Len(t, plan, 3) + assert.Equal(t, map[string]interface{}{"projectName": projectName}, plan[0][0].Options) + assert.Equal(t, map[string]interface{}{"projectName": projectName}, plan[1][0].Options) + assert.Equal(t, map[string]interface{}{ + "projectName": projectName, + "prHeadRefPattern": "^(feat|fix)/", + }, plan[2][0].Options) +} + +func TestMakeMetricPluginPipelinePlanV200WithBlankPrHeadRefPattern(t *testing.T) { + var dora Dora + const projectName = "TestMakePlanV200-project" + plan, err := dora.MakeMetricPluginPipelinePlanV200(projectName, json.RawMessage(`{"prHeadRefPattern":" "}`)) + assert.Nil(t, err) + assert.Equal(t, map[string]interface{}{"projectName": projectName}, plan[2][0].Options) +} + +func TestPrepareTaskDataPrHeadRefPattern(t *testing.T) { + var dora Dora + + data, err := dora.PrepareTaskData(nil, map[string]interface{}{"projectName": "p", "prHeadRefPattern": "^feat/"}) + assert.Nil(t, err) + if re := data.(*tasks.DoraTaskData).PrHeadRefRegexp; assert.NotNil(t, re) { + assert.Equal(t, "^feat/", re.String()) + } + + data, err = dora.PrepareTaskData(nil, map[string]interface{}{"projectName": "p"}) + assert.Nil(t, err) + assert.Nil(t, data.(*tasks.DoraTaskData).PrHeadRefRegexp) + + data, err = dora.PrepareTaskData(nil, map[string]interface{}{"projectName": "p", "prHeadRefPattern": "^(feat"}) + assert.Nil(t, data) + if assert.NotNil(t, err) { + assert.Equal(t, errors.BadInput, err.GetType()) + } +} + +func TestMakeMetricPluginPipelinePlanV200WithInvalidPrHeadRefPattern(t *testing.T) { + var dora Dora + plan, err := dora.MakeMetricPluginPipelinePlanV200("p", json.RawMessage(`{"prHeadRefPattern":"^(feat"}`)) + assert.Nil(t, plan) + if assert.NotNil(t, err) { + assert.Equal(t, errors.BadInput, err.GetType()) + } +} diff --git a/backend/plugins/dora/tasks/change_lead_time_calculator.go b/backend/plugins/dora/tasks/change_lead_time_calculator.go index 9a86c6ed10b..fdcf628b59e 100644 --- a/backend/plugins/dora/tasks/change_lead_time_calculator.go +++ b/backend/plugins/dora/tasks/change_lead_time_calculator.go @@ -79,7 +79,7 @@ func CalculateChangeLeadTime(taskCtx plugin.SubTaskContext) errors.Error { // Get pull requests by repo project_name var clauses = []dal.Clause{ - dal.Select("pr.id, pr.pull_request_key, pr.author_id, pr.merge_commit_sha, pr.created_date, pr.merged_date"), + dal.Select("pr.id, pr.pull_request_key, pr.author_id, pr.merge_commit_sha, pr.created_date, pr.merged_date, pr.head_ref"), dal.From("pull_requests pr"), dal.Join(`LEFT JOIN project_mapping pm ON (pm.row_id = pr.base_repo_id)`), dal.Where("pr.merged_date IS NOT NULL AND pm.project_name = ? AND pm.table = 'repos'", data.Options.ProjectName), @@ -90,6 +90,7 @@ func CalculateChangeLeadTime(taskCtx plugin.SubTaskContext) errors.Error { } defer cursor.Close() + var total, excluded int converter, err := api.NewDataConverter(api.DataConverterArgs{ RawDataSubTaskArgs: api.RawDataSubTaskArgs{ Ctx: taskCtx, @@ -104,6 +105,11 @@ func CalculateChangeLeadTime(taskCtx plugin.SubTaskContext) errors.Error { Input: cursor, Convert: func(inputRow interface{}) ([]interface{}, errors.Error) { pr := inputRow.(*code.PullRequest) + total++ + if !MatchPrHeadRef(data.PrHeadRefRegexp, pr.HeadRef) { + excluded++ + return nil, nil + } // Initialize a new ProjectPrMetric projectPrMetric := &crossdomain.ProjectPrMetric{} projectPrMetric.Id = pr.Id @@ -165,7 +171,13 @@ func CalculateChangeLeadTime(taskCtx plugin.SubTaskContext) errors.Error { return err } // Execute the data converter - return converter.Execute() + if err := converter.Execute(); err != nil { + return err + } + if data.PrHeadRefRegexp != nil { + logger.Info("prHeadRefPattern %q excluded %d of %d merged pull requests", data.PrHeadRefRegexp.String(), excluded, total) + } + return nil } func computeTimeSpan(start, end *time.Time) *int64 { diff --git a/backend/plugins/dora/tasks/task_data.go b/backend/plugins/dora/tasks/task_data.go index 83fadc49687..55a05629fed 100644 --- a/backend/plugins/dora/tasks/task_data.go +++ b/backend/plugins/dora/tasks/task_data.go @@ -18,6 +18,9 @@ limitations under the License. package tasks import ( + "regexp" + "strings" + "github.com/apache/devlake/core/errors" helper "github.com/apache/devlake/helpers/pluginhelper/api" ) @@ -31,11 +34,15 @@ type DoraOptions struct { Since string ProjectName string `json:"projectName"` ScopeId *string `json:"scopeId,omitempty"` + // PrHeadRefPattern is a Go RE2 regex; when set, only merged pull requests whose + // source branch (head_ref) matches it are counted by calculateChangeLeadTime. + PrHeadRefPattern string `json:"prHeadRefPattern,omitempty"` } type DoraTaskData struct { Options *DoraOptions DisableIssueToIncidentGenerator bool + PrHeadRefRegexp *regexp.Regexp } func DecodeAndValidateTaskOptions(options map[string]interface{}) (*DoraOptions, errors.Error) { @@ -47,3 +54,26 @@ func DecodeAndValidateTaskOptions(options map[string]interface{}) (*DoraOptions, return &op, nil } + +// CompilePrHeadRefPattern compiles the prHeadRefPattern option. +// A blank pattern means no filtering and yields a nil regexp. +func CompilePrHeadRefPattern(pattern string) (*regexp.Regexp, errors.Error) { + pattern = strings.TrimSpace(pattern) + if pattern == "" { + return nil, nil + } + re, err := regexp.Compile(pattern) + if err != nil { + return nil, errors.BadInput.Wrap(err, "invalid prHeadRefPattern") + } + return re, nil +} + +// MatchPrHeadRef reports whether a pull request source branch matches the pattern. +// A nil regexp matches everything. The "refs/heads/" prefix is ignored. +func MatchPrHeadRef(re *regexp.Regexp, headRef string) bool { + if re == nil { + return true + } + return re.MatchString(strings.TrimPrefix(headRef, "refs/heads/")) +} diff --git a/backend/plugins/dora/tasks/task_data_test.go b/backend/plugins/dora/tasks/task_data_test.go new file mode 100644 index 00000000000..10245bdd251 --- /dev/null +++ b/backend/plugins/dora/tasks/task_data_test.go @@ -0,0 +1,62 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tasks + +import ( + "testing" + + "github.com/apache/devlake/core/errors" + "github.com/stretchr/testify/assert" +) + +func TestCompilePrHeadRefPattern(t *testing.T) { + for _, pattern := range []string{"", " "} { + re, err := CompilePrHeadRefPattern(pattern) + assert.Nil(t, err) + assert.Nil(t, re) + } + + re, err := CompilePrHeadRefPattern("^(feat") + assert.Nil(t, re) + if assert.NotNil(t, err) { + assert.Equal(t, errors.BadInput, err.GetType()) + } + + re, err = CompilePrHeadRefPattern(" ^(feat|fix)/ ") + assert.Nil(t, err) + if assert.NotNil(t, re) { + assert.Equal(t, "^(feat|fix)/", re.String()) + } +} + +func TestMatchPrHeadRef(t *testing.T) { + assert.True(t, MatchPrHeadRef(nil, "")) + assert.True(t, MatchPrHeadRef(nil, "chore/a")) + + re, err := CompilePrHeadRefPattern("^(feat|fix)/") + assert.Nil(t, err) + assert.True(t, MatchPrHeadRef(re, "feat/a")) + assert.True(t, MatchPrHeadRef(re, "refs/heads/fix/b")) + assert.False(t, MatchPrHeadRef(re, "chore/a")) + assert.False(t, MatchPrHeadRef(re, "")) + assert.False(t, MatchPrHeadRef(re, "FEAT/x")) + + re, err = CompilePrHeadRefPattern("(?i)^feat/") + assert.Nil(t, err) + assert.True(t, MatchPrHeadRef(re, "FEAT/x")) +} diff --git a/config-ui/src/routes/project/detail/settings-panel.tsx b/config-ui/src/routes/project/detail/settings-panel.tsx index 39832ca3695..7b62c620efb 100644 --- a/config-ui/src/routes/project/detail/settings-panel.tsx +++ b/config-ui/src/routes/project/detail/settings-panel.tsx @@ -55,6 +55,7 @@ export const SettingsPanel = ({ project, onRefresh }: Props) => { const [name, setName] = useState(''); const [dora, setDora] = useState({ enable: false, + prHeadRefPattern: '', }); const [linker, setLinker] = useState({ enable: false, @@ -81,6 +82,7 @@ export const SettingsPanel = ({ project, onRefresh }: Props) => { setName(project.name); setDora({ enable: dora?.enable ?? false, + prHeadRefPattern: dora?.pluginOption?.prHeadRefPattern ?? '', }); setLinker({ enable: linker?.enable ?? false, @@ -132,7 +134,9 @@ export const SettingsPanel = ({ project, onRefresh }: Props) => { metrics: [ { pluginName: 'dora', - pluginOption: {}, + pluginOption: { + prHeadRefPattern: dora.prHeadRefPattern.trim(), + }, enable: dora.enable, }, { @@ -208,12 +212,38 @@ export const SettingsPanel = ({ project, onRefresh }: Props) => { setDora({ enable: e.target.checked })}> + setDora({ ...dora, enable: e.target.checked })}> Enable DORA Metrics } - description="DORA metrics are four widely-adopted metrics for measuring software delivery performance." - /> + description={ + + DORA metrics are four widely-adopted metrics for measuring software delivery performance. Optionally, + only count pull requests whose source branch matches this regex as changes for Lead Time for Changes and + the PR cycle time metrics. Leave empty to count all merged pull requests. + +
Example 1 - Only feature and fix branches: ^(feat|fix)/
+
Example 2 - Same, case-insensitive, plus hotfix: (?i)^(feat|fix|hotfix)/
+
Go RE2 syntax; lookahead such as (?!...) is not supported.
+
The refs/heads/ prefix (Azure DevOps) is ignored when matching.
+ + } + /> + + } + > + {dora.enable && ( + setDora({ ...dora, prHeadRefPattern: e.target.value })} + /> + )} + setLinker({ ...linker, enable: e.target.checked })}>