Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Bug][DORA-change_lead_time_calculator] Wrong deployment reference #8188

Open
3 tasks done
kostas-petrakis opened this issue Nov 12, 2024 · 2 comments · May be fixed by #8206
Open
3 tasks done

[Bug][DORA-change_lead_time_calculator] Wrong deployment reference #8188

kostas-petrakis opened this issue Nov 12, 2024 · 2 comments · May be fixed by #8206
Labels
devops Something about CI/CD (devops) severity/p1 This bug affects functionality or significantly affect ux type/bug This issue is a bug

Comments

@kostas-petrakis
Copy link

kostas-petrakis commented Nov 12, 2024 •
edited
Loading

Search before asking

  • I had searched in the issues and found no similar issues.

What happened

While analyzing the data gathered from our GitHub integration, I stumbled upon an anomaly in a minor subset of deployments. Oddly enough, some deployments were reporting a deployment of incosistent time (in my example 30 weeks).
Given my familiarity with our GitHub operations, I was certain this wasn't accurate, prompting me to dig deeper. My investigation points towards a potential issue with the getDeploymentCommit function, particularly in relation to EnrichPrevSuccessDeploymentCommit.

To illustrate, I've attached an example PR

{
"pull_requests": [
	{
		"id" : "github:GithubPullRequest:4:1806504909",
		"created_at" : "2024-11-12 08:34:13.134",
		"updated_at" : "2024-11-12 08:34:13.134",
		"_raw_data_params" : "{\"ConnectionId\":4,\"Name\":\"********\"}",
		"_raw_data_table" : "_raw_github_api_pull_requests",
		"_raw_data_id" : 4499,
		"_raw_data_remark" : "",
		"base_repo_id" : "github:GithubRepo:4:***********",
		"base_ref" : "main",
		"base_commit_sha" : "a51af0dabd2d9e9bd7b23e432688842cf086c248",
		"head_repo_id" : "github:GithubRepo:4:*******",
		"head_ref" : "feature\***************",
		"head_commit_sha" : "e6a73f3095d1c97d4d4576e3c06719fb07938bd1",
		"merge_commit_sha" : "b297c73092685c4b1a0ee62c2326d77115a86e94",
		"status" : "MERGED",
		"original_status" : "closed",
		"type" : "",
		"component" : "",
		"title" : "****************************",
		"description" : "null",
		"url" : "https:\/\/github.com\/******\/*******\/pull\/1025",
		"author_name" : "***************",
		"author_id" : "github:GithubAccount:4:***********",
		"parent_pr_id" : "",
		"pull_request_key" : 1025,
		"created_date" : "2024-04-04 09:08:03",
		"merged_date" : "2024-04-05 09:00:16",
		"closed_date" : "2024-04-05 09:00:16",
		"additions" : 0,
		"deletions" : 0,
		"merged_by_name" : "",
		"merged_by_id" : "github:GithubAccount:4:0",
		"is_draft" : 0
	}
]}

As you can see this PR was merged 2024-04-05 09:00:16 yet the deployment linked is incorrect, as it belongs to a different deployment.

{
"project_pr_metrics": [
	{
		"id" : "github:GithubPullRequest:4:1806504909",
		"created_at" : "2024-11-12 08:38:29.797",
		"updated_at" : "2024-11-12 08:38:29.797",
		"_raw_data_params" : "",
		"_raw_data_table" : "",
		"_raw_data_id" : 0,
		"_raw_data_remark" : "",
		"project_name" : "*************",
		"first_commit_sha" : "3726ef9fce54e1f95717ab0fbe695403cbf850ff",
		"pr_coding_time" : 1,
		"first_review_id" : "github:GithubPrReview:4:******",
		"pr_pickup_time" : 4,
		"pr_review_time" : 1429,
		"deployment_commit_id" : "github:GithubRun:4:435823930:11612162783:https:\/\/github.com\/*****\/****",
		"pr_deploy_time" : 301201,
		"pr_cycle_time" : 302635,
		"first_commit_authored_date" : "2024-04-04 09:07:51",
		"pr_created_date" : "2024-04-04 09:08:03",
		"first_comment_date" : "2024-04-04 09:11:44",
		"pr_merged_date" : "2024-04-05 09:00:16",
		"pr_deployed_date" : "2024-10-31 13:00:17"
	}
]}

Manually checking in the cicd_deployment_commits for the merge_commit_sha (b297c73092685c4b1a0ee62c2326d77115a86e94) I can see that the deployment is there:

{
"cicd_deployment_commits": [
	{
		"id" : "github:GithubRun:4:435823930:8567339272:https:\/\/github.com\/l*****\/****",
		"created_at" : "2024-11-12 11:17:32.066",
		"updated_at" : "2024-11-12 11:17:33.602",
		"_raw_data_params" : "{\"ConnectionId\":4,\"Name\":\"*****\/****\"}",
		"_raw_data_table" : "_raw_github_api_runs",
		"_raw_data_id" : 2338,
		"_raw_data_remark" : "prev_deployment_commit_id_enricher,",
		"cicd_scope_id" : "github:GithubRepo:4:435823930",
		"cicd_deployment_id" : "github:GithubRun:4:435823930:8567339272",
		"name" : "CI\/CD",
		"result" : "SUCCESS",
		"original_result" : "success",
		"status" : "DONE",
		"original_status" : "completed",
		"environment" : "PRODUCTION",
		"original_environment" : "",
		"created_date" : "2024-04-05 09:00:19",
		"queued_date" : null,
		"queued_duration_sec" : null,
		"started_date" : "2024-04-05 09:00:19",
		"finished_date" : "2024-04-05 09:14:22",
		"duration_sec" : 843.0,
		"commit_sha" : "b297c73092685c4b1a0ee62c2326d77115a86e94",
		"ref_name" : "main",
		"repo_id" : "github:GithubRepo:4:435823930",
		"repo_url" : "https:\/\/github.com\/*****\/****",
		"prev_success_deployment_commit_id" : "",
		"commit_msg" : "",
		"subtask_name" : "dora.generateDeploymentCommits",
		"display_title" : "Merge pull request #1025 from *****\/****",
		"url" : "https:\/\/github.com\/*****\/****/actions\/runs\/8567339272"
	}
]}

Upon inspection, I've noticed that the prev_success_deployment_commit_id field is empty in our data. I suspect this might be influencing the results of the query in the change_lead_time_calculator.go module.

I'm uncertain if the query should return null when faced with this condition, given that prev_success_deployment_commit_id is consistently empty for the first deployment recorded for the cicd_scope_id/repo_url/env grouping, as per code.

Interestingly, the linked "deployment_commit_id" : "github:GithubRun:4:435823930:11612162783 is the final entry in the cicd_deployment_commits table for the cicd_scope_id/repo_url/env grouping.

What do you expect to happen

PR deployment should be linked to the proper run for this example (github:GithubRun:4:435823930:8567339272:https:\/\/github.com\/l*****\/****)

How to reproduce

I will just describe what the status of that repo is, GitHub, deployment through CI/CD workflow, explore for the first PR deployment recorded by Devlake and evaluate the linked deployment.
This issue is only related to the first deployment for each cicd_scope_id/repo_url/env grouping.

Anything else

No response

Version

v1.0.1@e061ef2

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct
@kostas-petrakis kostas-petrakis added the type/bug This issue is a bug label Nov 12, 2024
@dosubot dosubot bot added devops Something about CI/CD (devops) severity/p1 This bug affects functionality or significantly affect ux labels Nov 12, 2024
@dosubot DosuBot
Copy link

dosubot bot commented Nov 12, 2024

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.

@kostas-petrakis kostas-petrakis linked a pull request Nov 20, 2024 that will close this issue
Open
@Startrekzky
Copy link
Contributor

Startrekzky commented Nov 27, 2024 •
edited
Loading

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
devops Something about CI/CD (devops) severity/p1 This bug affects functionality or significantly affect ux type/bug This issue is a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(dora): change lead time calculator kostas-petrakis/incubator-devlake
2 participants
@Startrekzky @kostas-petrakis