chenwei791129 opened a new issue, #9222: URL: https://github.com/apache/devlake/issues/9222
### Search before asking - [X] I had searched in the [issues](https://github.com/apache/devlake/issues?q=is%3Aissue) and found no similar feature requirement. ### Use case As a tech lead, I want DORA Lead Time for Changes (LTC) to only count the pull requests that represent an actual product change, so that the metric reflects how long a feature or fix takes to reach production instead of being skewed by MRs that are not changes. Our GitLab workflow: 1. Every feature / fix lives on its own branch (`feat/...`, `fix/...`) and is merged into a long-lived `release-uat` branch via MR. 2. The customer verifies on the UAT environment. 3. Once verified, a **single** MR merges `release-uat` into `production`, which triggers the production pipeline. Branch naming follows the Conventional Commits type prefix: | Branch prefix | Meaning | Represents a product change? | | --- | --- | --- | | `feat/...` | New feature | yes | | `fix/...` | Bug fix | yes | | `chore/...` | Dependency bumps, CI config, misc | no | | `docs/...` | Documentation | no | | `release-uat` → `production` | Release / promotion MR | no | ```text time ─────────────────────────────────────────────────────────────▶ feat/A a1──a2──a3 \ feat/B \ b1──b2 \ \ chore/X \ \ x1 \ \ \ release-uat ─────────────M1────────M2───────M3──────────┐ \ production ───────────────────────────────────────────────R ──▶ 🚀 PRODUCTION deployment M1/M2/M3 = feature MRs merged into release-uat (merge commits) R = merge commit of the release MR (release-uat → production) ``` For us, a "change" in the DORA sense is a `feat/` or `fix/` MR. `chore/` and `docs/` MRs are not product changes, and the release MR in step 3 is only the act of promoting already-finished work; it is not a change by itself. ### Description #### What DevLake does today **Mapping feature MRs to the production deployment already works.** `batchFetchDeployments` in `backend/plugins/dora/tasks/change_lead_time_calculator.go` has two phases: - Phase 1 (direct match): `cicd_deployment_commits.commit_sha == pr.merge_commit_sha` → matches `R`. - Phase 2 (diff-based): via `commits_diffs` (computed by refdiff from `commit_parents` between the previous successful production deployment and the current one) → `M1`, `M2`, `M3` are ancestors of `R`, so feat/A, feat/B and chore/X are all mapped to the same production deployment. So each feature MR gets a correct LTC: *its own first commit → merged into release-uat → production deployment finished* (given gitextractor is enabled for the repo, the promotion MR is not squash-merged, and it is not the very first production deployment). **The problem: every merged MR is unconditionally written to `project_pr_metrics`.** `calculateChangeLeadTime` selects all PRs with `merged_date IS NOT NULL` and has no filter at all. This produces two different kinds of distortion: **A. The release MR is counted as a change, and its numbers are wrong.** - GitLab's `merge_requests/:iid/commits` returns **all** commits a1…x1 for `R`. - `first_commit = MIN(commit_authored_date)` picks `a1`, the first commit of the *earliest* feature branch. - `R`'s `pr_coding_time` therefore spans `a1 → R created`, and its `pr_cycle_time` is roughly the length of the whole release cycle. - `R`'s cycle time is always ≥ the longest feature MR in that release, so every release cycle adds one data point at the top of the distribution and pushes the median up. `avg`-based panels are affected more. - The "PR Details" panel lists `R` with a first-commit SHA that belongs to a different MR. This is independent of branch naming; any team using a "staging → production promotion MR" flow hits it. **B. `chore/` and `docs/` MRs are mixed in and drag LTC down.** These MRs are usually fast and skip UAT. Their numbers are correct, but they do not represent the lead time of a product change. ```text project_pr_metrics (illustrative, one release cycle) PR head_ref first_commit merged deployed pr_cycle_time should count? ──────── ─────────── ──────────── ─────── ──────── ───────────── ───────────── feat/A feat/A a1 (day 0) day 3 day 14 14d ✔ feat/B feat/B b1 (day 4) day 7 day 14 10d ✔ chore/X chore/X x1 (day 12) day 12 day 14 2d ✘ pulls down R release-uat a1 (day 0) ◀── day 14 day 14 14d ✘ pulls up, first commit belongs to feat/A ``` #### There is no filtering point anywhere in the pipeline ```text GitLab API DevLake backend Grafana ────────── ─────────────── ─────── all MRs ──▶ mr_extractor ──▶ _tool_gitlab_merge_requests │ ▼ mr_convertor ──────▶ pull_requests (no filter) ├ head_ref = 'feat/login' ├ base_ref = 'release-uat' └ type = (labels only) │ ▼ dora: calculateChangeLeadTime WHERE merged_date IS NOT NULL ──▶ project_pr_metrics (no filter) │ ▼ DORA dashboards _pr_stats CTE (no filter) ``` `pull_requests` already has `head_ref` / `base_ref`; the data is there, nothing uses it. What existing configuration can and cannot do: - **Scope config `prType` / `prComponent`** (e.g. `backend/plugins/gitlab/models/scope_config.go`): regex against MR **labels**, not branch names, and they only populate `type` / `component`; they never exclude an MR. There is no `headRef` / `baseRef` / branch pattern at all. - **DORA plugin options** (`backend/plugins/dora/tasks/task_data.go`): only `projectName` / `scopeId`. - **Grafana**: `DORA.json`, `dora-details-lead-timefor-changes.json`, `dora-by-team.json`, `engineering-throughput-and-cycle-time.json` do not filter on `type` / `head_ref` / `base_ref`. #### Expected behaviour A configuration that defines "which MRs are changes in the DORA sense", at least an include / exclude regex on `head_ref` (source branch) and/or `base_ref` (target branch). With our workflow, a single `head_ref` allow-list such as `^(feat|fix)/` would address both A and B: the release MR's `head_ref` is `release-uat`, which simply does not match. #### Open questions 1. **Where should the filter live?** - A. **Per-plugin scope config** (e.g. a `prBranchPattern` on `GitlabScopeConfig`) applied in `mr_convertor`, so filtered MRs never reach `pull_requests`. - Granularity: per repo. - Side effect: `pull_requests` loses those MRs entirely, so code-review dashboards (throughput, review time) shrink too. - Plugin-specific: `common.ScopeConfig` only has `Entities` / `ConnectionId` / `Name`; fields like `prType` / `deploymentPattern` are defined separately by each plugin, and plugins cannot import each other. Supporting GitHub / Bitbucket / Azure DevOps means repeating the struct field, migration, converter and config-ui change per plugin. - B. **DORA plugin option** (project metrics `pluginOption`), applied in `calculateChangeLeadTime` against domain-layer `pull_requests.head_ref` / `base_ref`, affecting only `project_pr_metrics`. - Granularity: per DevLake project. - Side effect: none on `pull_requests`; semantically "these MRs exist, they are just not changes". - One change covers every source plugin, because the domain layer is source-agnostic. - Precedent: `linker` already stores `pluginOption.prToIssueRegexp`, with UI in `config-ui/src/routes/project/detail/settings-panel.tsx`; DORA's `pluginOption` is currently `{}`. - C. **Tag only in the domain layer** (e.g. write the branch prefix into `pull_requests.type`) and let each dashboard filter. - Least invasive, but every dashboard that reads `project_pr_metrics` needs SQL changes. 2. **Granularity**: per repo (A) or per DevLake project (B)? Repos inside one project may use different naming conventions; supporting that requires A, or B plus a per-repo override. 3. **Cross-plugin consistency**: with A, should GitHub / Bitbucket / Azure DevOps be done in the same change? With B it is automatic. 4. **Include or exclude?** - `head_ref` allow-list `^(feat|fix)/`: branches that do not follow the convention are excluded by default; also covers the release MR (problem A). - `head_ref` deny-list `^(chore|docs)/`: included by default; does **not** exclude the release MR unless `^release-uat$` is added or a `base_ref` deny-list `^production$` is supported. - Which one(s) to support decides whether problem A is solved as a side effect. #### Related code - `backend/plugins/dora/tasks/change_lead_time_calculator.go` - `CalculateChangeLeadTime`: unfiltered PR query - `batchFetchFirstCommits`: `MIN(commit_authored_date)` - `batchFetchDeployments`: Phase 1 / Phase 2 mapping - `backend/plugins/dora/tasks/task_data.go`: DORA options - `backend/plugins/dora/impl/impl.go` `MakeMetricPluginPipelinePlanV200`: refdiff scheduling inside the DORA plan - `backend/plugins/refdiff/tasks/deployment_commit_diff_calculator.go`: source of `commits_diffs` - `backend/plugins/gitlab/models/scope_config.go`: `prType` / `prComponent` - `backend/plugins/gitlab/tasks/mr_extractor.go`: where the label regexes are applied - `backend/plugins/gitlab/tasks/mr_convertor.go`: `HeadRef` / `BaseRef` written to `pull_requests` - `backend/plugins/gitlab/tasks/mr_commit_collector.go`: `merge_requests/{iid}/commits` (source of problem A's first commit) - `backend/core/models/domainlayer/code/pull_request.go`: `pull_requests` columns - `config-ui/src/plugins/register/gitlab/transformation.tsx`: scope config UI - `grafana/dashboards/mysql/DORA.json`, `dora-details-lead-timefor-changes.json` ### Related issues Searched `apache/devlake` for "lead time for changes", "project_pr_metrics", "head_ref", "exclude pull requests dora", "release branch lead time". Nothing requests branch-based filtering of LTC; the closest prior art is author-based (bot) exclusion: - #6984 `[Feature][DORA Metrics][Median Lead time for changes] Filter PRs based on tags` — closed as stale (2024). Same symptom from a different angle (Renovate PRs dragging LTC down); asked for a tag/regex filter in the GitLab scope config (option A above). The reporter's workaround was editing the Grafana SQL with `head_ref NOT LIKE 'renovate/%'`. Never implemented. - #8381 `[Question][DORA] Ignoring Automated VCS Changes` — closed as stale. Discussion converged on an `accounts.is_bot` flag; maintainers noted that the DORA plugin's internal logic producing `project_pr_metrics` should honour the flag, not just dashboard SQL. - #8974 `[Question][Metrics] How to remove automation from project_pr_metrics?` — fixed by #9000, which added `accounts.is_bot` and excludes bot comments from the first-review lookup (PR Pickup Time). It does not exclude any PR from `project_pr_metrics`. - #8188 / #8790 `[Bug][DORA-change_lead_time_calculator] Wrong deployment reference` — fixed by #8942 (the two-phase direct-match / diff-based deployment mapping described above). Related context only: it is why feature MRs are already mapped to the right production deployment. This request is complementary to the `is_bot` work: that one filters by **who** authored the PR, this one by **which branch** the PR comes from (its role in the workflow). A release MR is authored by a human and is not a bot PR. ### Are you willing to submit a PR? - [X] Yes I am willing to submit a PR! I have a preferred approach (option B above) and am happy to adjust to the maintainers' preference on the open questions before starting. ### Code of Conduct - [X] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
