[Breaking Change] Design & track metrics for backlog burndown (APEX metrics)

[konrad-jamrozik]

As originally discussed on REST API Tools planning meeting on 11/1/2023, we should devise and track metrics showing progress on burning down backlog of issues related to Spec PR tools. Specifically, at first we want to show improvement with issues related to breaking changes tooling.

Initial metrics design proposal as of 11/1/2023

Let's keep track of the number of new issues filed per month that are labelled Breaking Changes and (Breaking Changes + bug). Let's also keep track of number of such issues closed per month. We want to show that, month by month, less new issues are opened, and issues are getting closed, hence showing burndown to zero.

As a secondary measure, we could track how many PRs are affected by each of these issues. This way we could roughly claim which of the closed issues were very impactful vs minor.

We could also label each PR that has problem with breaking change tooling with BreakingChange-ToolingIssue (we probably want to stay aligned with format proposed by #6374). Such label should be added to PR where the breaking change tooling resulted in false positives (the tooling failed while it shouldn't), in false negatives (the tooling should have reported breaking change, but didn't), and where the errors and logs provided by the tooling were hard to interpret. If a PR has such a label, it should always be linked to appropriate backlog issue.

Finally, we could also track occurrences of the labels proposed in #6374, once implemented.

Required implementation work

We need to implement a set of queries and charts to pull the relevant data and visualize it. The opened issues by month, closed opened issues by month, PRs labelled appropriately per month, and charts over time.

The labelling doesn't require any implementation changes, beyond adding the label to GitHub repo. I have added the label to azure-rest-api-specs and azure-rest-api-specs-pr repos. The breaking change reviewers need to start labelling PRs appropriately.

There is also implementation work captured in #6374 that is prerequisite for tracking that data.

[konrad-jamrozik]

Since we discussed these metrics I spent some time addressing the most pressing issues related to breaking changes, according to people most interested in it, primarily Mike Kistler and Jeffrey Richter. The issue with the metrics we proposed is that they don't really reflect that well what is going on. I am working primarily on the following big fundamental issues:

• [Breaking Change] Bypass review of BreakingChange/NewApiVersionRequired for PRs to dev branches #5838
• [Breaking Change] Tooling reports many duplicate errors #7242
• [Breaking Change] Improve error reporting in the GitHub check pane. Notably, make it easy to find all errors #7239
• [Breaking Change][PR Workflow] Use more granular labels for Breaking Changes approvals #6374

It is hard to measure benefits of such work via issues opened/closed

What I can propose instead is to report back the telemetry log statements I am adding along the way. They would say things like:

• This is how many duplicate reports we have avoided, saving time.
• This is how many extra/unnecessary breaking change reviews we have avoided, saving time.
• This is how many breaking change suppressions have been added to the repo, saving time.

Queries to measure impact

Important

The queries below have parameterized start and end time. To get full logs, you must run the queries multiple times, covering the desired time ranges, and union them together, remembering to deduplicate the PR URLs across partitions of the union. It is not possible to run given query in one go for the entire time range of interest, as given query reads raw diagnostic build logs dump, which contains huge amounts of logs, hence it is not possible to have too wide of a time range. The query would time out.

You can determine since when given query applies by doing git blame on relevant source code, linked below. The code usually was deployed the same day it was merged to the codebase.

Measure #5838

To measure:

• [Breaking Change] Bypass review of BreakingChange/NewApiVersionRequired for PRs to dev branches #5838

We emit this log:

console.log(`processBreakingChangeLabels: PR: ${prKey.toUrl()}, targetBranch: ${prContext.targetBranch}. ` 
+ `The addition of 'BreakingChangesReviewRequired' or 'VersioningReviewRequired' labels has been prevented `
+ `because we checked that the PR is not targeting a production branch.`);

And measure it with this Kusto query.

Note the query also covers an earlier version of this log, without the PR info.

Measure #7242

To measure:

• [Breaking Change] Tooling reports many duplicate errors #7242

We emit this log:

console.log(`UnifiedPipeLineStore.appendOadMessages: PR: ${prKey.toUrl()}, baseBranch: ${baseBranch}, `
+ `oadMessages.length: ${oadMessages.length}, duplicateOadMessages.length: ${duplicateOadMessages.length}, `
+ `_oadMessageCache.length: ${this._oadMessageCache.length}.`)

And measure it with this Kusto query.

Measure #7239

To measure:

• [Breaking Change] Improve error reporting in the GitHub check pane. Notably, make it easy to find all errors #7239

We emit this log:

const logPrefix =
  `buildCompletedBreakingChangeCheckText: PR: ${prUrl}: `;

await logger.logInfo(logPrefix + `ENTERING. checkShowName: ${checkShowName}, pipelineTaskResult.checkRunUrl: ${pipelineTaskResult.checkRunUrl}`);

And measure it with this Kusto query.

Measure #6374

To measure:

• [Breaking Change][PR Workflow] Use more granular labels for Breaking Changes approvals #6374

We observe all the occurrences when the new approval labels have been used. They can be observed by using following GitHub queries:

• aka.ms/azsdk/specs-brch-approvals: new fine-grained approval labels usage in azure-rest-api-specs repo
• aka.ms/azsdk/private-specs-brch-approvals: new fine-grained approval labels usage in azure-rest-api-specs-pr repo

Measure #6396

To measure:

• [Breaking Change][PR Workflow] Breaking Changes should not flag breaks from prior preview as errors #6396

We emit this log

if (process.exitCode == 0 && oadViolationsCnt > 0) {
  // We are using this log as a metric to track and measure impact of the work on improving "breaking changes" tooling. Log statement added around 2/22/2024.
  // See: https://github.com/Azure/azure-sdk-tools/issues/7223#issuecomment-1839830834
  console.log(
    `detectBreakingChange: ` +
      `Prevented spurious failure of breaking change check. prKey: ${detector.prKey}, ` +
      `comparisonType: ${comparisonType}, oadViolationsCnt: ${oadViolationsCnt}, ` +
      `process.exitCode: ${process.exitCode}.`
  );
}

And measure it with this Kusto query.