Hello @kostas-petrakis! I'm here to help you with any bugs, questions, or contributions you have. Let me know how I can assist you while we wait for a human maintainer.

The issue you're encountering seems to be related to how the getDeploymentCommit function handles cases where the prev_success_deployment_commit_id is empty. This situation typically occurs for the first deployment in a cicd_scope_id/repo_url/env grouping, as the EnrichPrevSuccessDeploymentCommit function resets this field to an empty string for the first deployment in a new set of consecutive deployments ^[1].

In the change_lead_time_calculator.go module, the logic for selecting the deployment commit might not be correctly handling cases where prev_success_deployment_commit_id is empty, leading to incorrect deployment references being linked. This could explain why the deployment time is inaccurately reported as 30 weeks.

To address this, you might need to adjust the logic in the getDeploymentCommit function to ensure it correctly identifies and links the first deployment in such groupings. This could involve modifying the query to account for cases where prev_success_deployment_commit_id is empty and ensuring it selects the appropriate deployment commit for the first deployment scenario.

Since this issue is specific to the first deployment in each grouping, reviewing and potentially updating the logic in both the getDeploymentCommit and EnrichPrevSuccessDeploymentCommit functions to handle these edge cases more effectively would be a good starting point.

To continue talking to Dosu, mention @dosu.

Hi @kostas-petrakis , I saw your PR #8206

One of DevLake's strategies of PR-deployment_commit mapping is to skip the mapping of the very first deployment_commit in DevLake. Why?

Because in many cases,

1. Users will collect both PRs and deployment_commits within a selected timeframe.
2. When finding a PR's associated deployment_commit, we need to diff the commits between every consecutive deployment_commits, which means we don't know the precise commits deployed by the very first deployment_commit collected by DevLake.
3. Hence, we don't know the PRs deployed by the very first deployment_commit.

In your case, the PR github:GithubPullRequest:4:1806504909 might be deployed by the first deployement_commit github:GithubRun:4:435823930:8567339272:https:\/\/github.com\/l*****\/**** for sure.

However, if we allow the first deployment_commit mapped to PRs. If the deploy time between the prior deployment to the first deployment collected by DevLake, many irrelevant PRs merged during this period will be mis-mapped to the deployment commit (some other users had encountered the problem).

Finally, we adopted the above strategy of excluding the very first deployment when mapping to PR.

@Startrekzky thanks for the explanation. However, in my specific use case (and I assume to others looking at the upvotes), I've observed that PRs associated with the initial recorded commit are being mapped to an incorrect deployment. This seems to introduce a different kind of mis-mapping issue, which affects the accuracy of our metrics (as explained with the data above).
I'm wondering if there might be a way to refactor the PR-deployment mapping process to handle this situation more accurately.

This issue has been automatically marked as stale because it has been inactive for 60 days. It will be closed in next 7 days if no further activity occurs.

We've been tracking this and it's still an issue for us! Although we're transitioning over to Github Actions we still have a lot of teams using Github for commits and an old Jenkins set up for deploy. Where devlake is finding PRs within Github but can't find a corresponding deploy it's choosing (we think) the first and so earliest deploy available to it, creating massive data skews of 10s of weeks.

If there is a PR that says it's been deployed but devlake can't find a deploy job we'd expect it to just not include that PR at all

We are in the process of analyzing some of the PRs that have been wrongly mapped to non relevant deployments. This is likely due to the previously mentioned query, which appears to be producing inaccurate results during transformation. We are currently investigating affected PRs.

Appreciate it @kostas-petrakis ! We'll keep an eye on this thread, thanks

Hello @kostas-petrakis , we are also keeping track of this ticket since we are affected.
Have you found out anything more about your affected PRs? Thanks.

Following up on this issue now that we've resolved the more urgent missing DORA calculations.
I can confirm that we're still seeing the same behavior on some of the newly onboarded services. As @hera-a-glitch pointed out, the behavior isn't entirely consistent—likely due to recalculation of DORA? (need to check)
From what I’ve observed, this issue only affects the first batch of collected commits. The current logic attempts to find a previous deployment; if none is found, it links the PR deployment to the latest available one (if I’m recalling the behavior correctly).
I'll dig deeper into this and explore potential solutions. Thanks for your patience!

This issue has been automatically marked as stale because it has been inactive for 60 days. It will be closed in next 7 days if no further activity occurs.

This issue has been closed because it has been inactive for a long time. You can reopen it if you encounter the similar problem in the future.

I am re-opening this as it is still affecting some of our repositories and raising questions from engineering teams.
I took some time to think about a possible approach which would possibly reduce the effect.
@Startrekzky
I am thinking of the following approach:

Adjust getDeploymentCommit to use a two-phase strategy:

• Direct match: if there exists a successful PRODUCTION deployment in the project whose commit_sha equals the PR’s merge commit, return it. This is precise and does not risk the first-deployment over-mapping problem.
• Fallback to diff-based mapping: retain the current strategy (including the filter dc.prev_success_deployment_commit_id <> '') to avoid mapping to the first deployment via diffs.

The following is how this translates to the implementation (adding here for discussion before opening a PR):

// getDeploymentCommit takes a merge commit SHA, a repository ID, a list of deployment pairs, and a database connection as input.
// It returns the deployment pair related to the merge commit, or nil if not found.
func getDeploymentCommit(mergeSha string, projectName string, db dal.Dal) (*devops.CicdDeploymentCommit, errors.Error) {
	// Phase 1: direct mapping when the deployment run's commit_sha equals the PR's merge commit.
	// This is precise and safe, and should be allowed even if it's the first deployment in the collected window.
	direct := make([]*devops.CicdDeploymentCommit, 0, 1)
	err := db.All(
		&direct,
		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("pm.project_name = ? AND dc.commit_sha = ? AND dc.result = ? AND dc.environment = 'PRODUCTION'", projectName, mergeSha, devops.RESULT_SUCCESS),
		dal.Orderby("dc.started_date, dc.id ASC"),
		dal.Limit(1),
	)
	if err != nil {
		return nil, err
	}
	if len(direct) > 0 {
		return direct[0], nil
	}

	// Phase 2: fall back to diff-based mapping (skipping the first deployment to avoid over-mapping).
	deploymentCommits := make([]*devops.CicdDeploymentCommit, 0, 1)
	// do not use `.First` method since gorm would append ORDER BY ID to the query which leads to an error
	err = db.All(
		&deploymentCommits,
		dal.Select("dc.*"),
		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("LEFT JOIN project_mapping pm ON (pm.table = 'cicd_scopes' AND pm.row_id = dc.cicd_scope_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.Where("dc.prev_success_deployment_commit_id <> ''"),
		dal.Where("dc.environment = 'PRODUCTION'"), // TODO: remove this when multi-environment is supported
		dal.Where("pm.project_name = ? AND cd.commit_sha = ? AND dc.RESULT = ?", projectName, mergeSha, devops.RESULT_SUCCESS),
		dal.Orderby("dc.started_date, dc.id ASC"),
		dal.Limit(1),
	)
	if err != nil {
		return nil, err
	}
	if len(deploymentCommits) == 0 {
		return nil, nil
	}
	return deploymentCommits[0], nil
}

Hi @petkostas , thanks for the proposal. I'll discuss with @klesh to figure out how to optimize this part.

This issue has been automatically marked as stale because it has been inactive for 60 days. It will be closed in next 7 days if no further activity occurs.

Will test next week