[Bug][Dora-Cahnge_lead_time_calculator] incorrect linking of commits to deployments when using azuredevops and webhook

[wouldd]

Search before asking

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

What happened

I assume that the issue I'm seeing is somehow unique to me using the azuredevops_go plugin and the webhook for reporting deployments from octopus. However the 'bug' I've identified looks to me like it would effect everyone. So probably the real issue is somewhere else but all I can say is that for me the DORA metrics are all wildly out because it relates every commit that has ever been made against a repo to the most recent deploy. This means the cycle time calculation for each commit is showing as just the age of that commit until the most recent deploy.
I believe the offending line is this one: https://github.com/apache/incubator-devlake/blob/cb69f35b5319f57dd3d6c8534121a6ca069cd175/backend/plugins/dora/tasks/change_lead_time_calculator.go#L215C2-L215C3
the commits_diffs table has 3 columns, commit_sha, old_commit_sha and new_commit_sha. For me I need the filter in this query to filter where the mergeSha = cd.new_commit_sha in order to work correctly.
I've tested this in my setup and that change has the expected behaviour. However, I would obviously expect this to be broken for everyone doing DORA metrics at all, and it seems unlikely no one else noticed this. So I guess maybe the intention of that commits_diffs table is to have information in it differently and perhaps the azure plugin is populating it incorrectly?

What do you expect to happen

In my case I have lots of quite old 4+years minimum repos. but I've only just started feeding deploy events in. so I would expect only to have data for the relatively few new commits since this went into place. and I would expect the cycle time calculations for those to be right.

How to reproduce

configure a long standing repo with lots of historical commtis against it. create a webhook call for the most recent commit/commtis in teh repo and expect to see dora calculations for those commits only. However you should see that it links all of the old commits to the new deployment.
Indeed when I run with a version in which I changed the above filter check then I get the behavior I expect. all the numbers make sense and the pr_cycle_time calculations show up numbers for only commits that have been reported as part of deployments

Anything else

Happens every time for all commits.

Version

master

Are you willing to submit PR?

• Yes I am willing to submit a PR!

Code of Conduct

• I agree to follow this project's Code of Conduct

[Startrekzky]

Hi @wouldd , I'm trying to understand what you have encountered. Correct me if I'm wrong:

1. You collected commits and deployments from Azure DevOps Go plugin, not from other Git plugins or Azure DevOps python plugin.
2. You found that DORA's change lead time was incorrect. The time was much longer than you expected.
3. You found that the records in the commits_diff table were incorrect. All commits rather than new commits were associated with the latest deployments.
4. You thought the third point might be a bug. This bug only impacts the data integrity of change lead time calculated from the raw data from the Azure DevOps Go plugin?

[wouldd]

@Startrekzky pretty much. 1) yes usig azure devops go plugin which is what is pulling commits/pr information. and using teh 'webhook' to post deployment information that includes the shas of the commits being deployed.
2) yes, change lead time had been calculated for all commits in a repo and the time calculated for all of them was essentiaially from their moment of commit until the date of the recent deployment (which in reality only deployed 3 commits)
3) the commits_diffs table seems to just be a relationship table between commits, I'm not super clear on what it represents, but yes the 'new' commit that was included in the deployment had an entyr in here linking it back to the first commit ever in the repository
4) as mentioned if I modify the query in the lead time calculator to filter on 'new commit sha' then I appear to get sensible values restricted to those commits that were actually part of my deploy.. The odd thing is that if hte query is just wrong and it really should be filtering on new_commit_sha then I would expect this bug to impact all users of any datasource and dora is just broken. But since I would assume it would be pretty obvious if it wasn't working for everyone, I must conclude that the shape of this table is not being populated by the azure plugin as expected by the system at large.

[Startrekzky]

Hi @wouldd This seems to be a problem related to the refdiff. I'm not sure if the refdiff calling logics are the same among all plugins. I'll forward it to @klesh to take a look.

[klesh]

I suspect that the merge_commit_sha might be wrongly picked.
https://learn.microsoft.com/en-us/rest/api/azure/devops/git/pull-requests/get-pull-requests?view=azure-devops-rest-7.1&tabs=HTTP

Currently, both azuredevops plugins are using lastMergeCommit as the merge commit sha, but maybe it should be lastMergeTargetCommit ?

For Change Lead Time to be calculated correctly, the following requirements must be met:

1. Deployment Commits (for a repo) could form a Linear History. It means any newer Deployment Commit should contain all older Deployment Commits
2. pr.merge_commit_sha must be found in the Linear History

loop in @mr-ks

[wouldd]

@klesh that certainly sounds plausible. I'm trying to wrap my head around your point 1 about all deployment commits forming a linear history 'newer deployment commit should contain all older deployment commits' I'm not sure I understand why that would be desired?
I would assume that for change cycle time you need

1. a deployment id with it's date of deployment, result, environment.
2. a set of commit shas that are linked to that deployment id as the vehichle that deployed them,
3. a lookup of all commits with their commit date and a link to the PR that merged them.

So you would process a deployment row, link it's deployment commits and join to their commit records and pr record. then calculate the various time deltas for commit->pr create->pr-merge->deployment to production and record that cycle tyime against those commits.
So any commit that has no associated deployment would have no cycle time values (the vast majority of our commit history was deployed long ago and we won't be back porting that information just starting from now)
dora calcs /boards would work on only those commits that have non-null values for cycle time
I'm not sure I see in this why a deployment commit would be linked to older deployments at all?

[klesh]

@wouldd It is needed to determine which deployment a PR belongs to.

If I understand your assumption correctly, you are connecting the PR to deployment by commit dates? no, it won't work, because the deployment happened after a commit got committed not necessarily including the commit. For example, one may start committing to a feature branch for the next version while a deployment on the release branch for the current version is taking place.

[wouldd]

@klesh I don't think we need to relate things by date at all.
I think you have a deployment record provided by the webhook where that record says 'these are teh commits that were deployed in this deployment'
I think you have a PR record provided by the azure devops api that says 'these are the commits that were part of this PR'
so from ad eployment you can select the commits it deployed and join them to the pr commits to find the pr(s) that they were merged in.

So between these you can link all the data required to caluclate cycle time. you know exactly which commits where in a deployment and can look up which PR those commits were associated with to reference those timings and look up what date that commit was originally made to calculate how long it took from that point to the point it was deployed.

Why would a deployment commit be linked to all previous deployment commits?

[klesh]

@klesh I don't think we need to relate things by date at all. I think you have a deployment record provided by the webhook where that record says 'these are teh commits that were deployed in this deployment' I think you have a PR

No, this is not the case. The webhook, or any other data sources would have only 1 commit sha for each repo per deployment stored in the cicd_deployment_commits table, so we have to calculate the commits between deployments, which is covered by https://github.com/apache/incubator-devlake/blob/49f276f558a2ea7124dfdc680bccbef27bf0b581/backend/plugins/refdiff/tasks/deployment_commit_diff_calculator.go

Let's assume that you have a release branch, you merge PR to it then deploy, at any point of time, you pick the latest commit of the branch, and search all its ancestors, you can find every single deployment commits up to the point, then you are guaranteed to have a correct PRs and deployments relationship. This is important, because one may merge a couple of PRs before a deployment.

Sorry, I shouldn't use the term Linear Commit History, it is not accurate, we just need all the deployment commits gets included in the history.

record provided by the azure devops api that says 'these are the commits that were part of this PR' so from ad eployment you can select the commits it deployed and join them to the pr commits to find the pr(s) that they were merged in.

That is the idea but not the whole story, in fact, commits of a PR morally don't end up in the deployment commit history., especially, when most projects tend to use Squash and Merge which no PR.commits would ever end up in the release branch. To address this, we use merge_commit_sha ( or squash_commit_sha or head_commit_sha) which would definitely appear in the commits history to represent if a PR is included in a deployment.

So between these you can link all the data required to caluclate cycle time. you know exactly which commits where in a deployment and can look up which PR those commits were associated with to reference those timings and look up what date that commit was originally made to calculate how long it took from that point to the point it was deployed.

Why would a deployment commit be linked to all previous deployment commits?

Let me know if I missed anything.

[wouldd]

@klesh Sorry for the slow response, I was on vacation for the last two weeks then getting ontop of other work stuff.
I saw that you had put in a fix related to this issue and I tried applying that fix. however it does not help. the dora reports still calculates every commit ever agfainst my respository against the only deployment it has been told about.
I suspect there is a fundamental presumption in this that this system will always have full information about every deployment from the start of the repository? however that is not going to be the case for us (or I assume most users). so we might have a 3 year old rep owith thousands of commits against it that were deployed historicaly hoever we have no way and no intention to try to tell devlake about those historical deployments. We just want information as it pertains to new deployments and the commits there.
I saw you added a build url to the cicdbuild converter in your fix, which I assume was hoped would limit the scope of the queries to just things that had a populated build url? The other issue here is that our deployments are not happening from build pipelines. they're handled by a completely separate tool.(hence the use of the webhook)

unfortunately as it stands this makes the dora metrics unusable for us, and those would be the main thing we're interested in.

[nicolavolpini]

I might be encountering the same bug. If not, please feel free to delete this comment as I don't want to stray the discussion.

I created a project with a single Github repo, using the GitHub connection for commits and PRs. A webhook is used for deployments.
The DORA Details - Lead Time for Changes board shows the correct PR averages but the 2. PR details table shows a ridiculously high change_lead_time (see screenshots).
It appears to be associating all the PRs in the project to the same and currently only webhook commit (notice how the deployment_id is the same for all PRs).

List of PRs in board:

Correct averages:

This is also visible in the project_pr_metric table, where all PRs match the same deployment_commit_id (in this case the webhook id).

I'm on 1.0.0b4.

Here's the sanitized CURL for the webhook:

      "deploymentCommits":[
        {
        "commit_sha":"<sha>",
        "repo_url":"https://github.com/<org>/<repo>"
        }
      ],
      "create_time":"2024-04-11T11:39:33+00:00",
      "start_time":"2024-04-11T11:39:33+00:00",
    "end_time":"2024-04-16T11:51:35+00:00",
    "result": "SUCCESS"
     }'

Thanks!

[wouldd]

@Startrekzky I'm curious about the characterisation of the bug priority you set being 'does not affect the functionality or isn't evident' from my point of view this renders one of the core dora metrics unusable. I would consider that to be a very evident impact on function and a key metric we would be interested in.