[darc/maestro++] Notify on subscription update failures

[mmitche]

Maestro should notify (open issues) when there are subscription updates that fail to execute properly. This is just for internal infrastructure issues, and not failures in pull requests.

[mmitche]

/cc @alexperovich @jcagme

[jcagme]

@tmat informed that Roslyn was not getting arcade updates since there was a PR which was created on 01/17 and has not been merged since there are merge conflicts.

This PR has 19 commits so the part where Maestro updates a PR is working fine but there is no way to know if something is not working correctly. I quick checked the history of its subscription and found no related "errors".

I think (need to confirm) we stop trying to auto-merge when we find there are conflicts which is perfectly fine but also I think we should have a way to notify repo owners when a PR is in this state so action is taken and the PR can be manually merged.

[epananth]

Spoke to @riarenas and got the following update

1. What kind of notification? - Create an issue in github/core-eng
2. How to differentiate between infrastructure error vs error in pull request? - Any error that happens before creating a PR is considered as infrastructure error and after the PR is created then its considered as PR error.

In case of both the errors github issue is created. PR error can be assigned to the respective repo owners to take a look at.

[riarenas]

How to differentiate between infrastructure error vs error in pull request? - Any error that happens before creating a PR is considered as infrastructure error and after the PR is created then its considered as PR error.

I'd add that in cases where there is a PR opened already, but we fail to update an existing PR with a new batch of dependencies, this also counts as one of the errors we want to report.

[epananth]

@riarenas @mmitche

I looked into the flow and this is what I propose.

The code for creating and updating the PR exists in PullRequestActor. The idea is to catch the exception and create a github issue if the PR does not get created /updated.

So I was planning to create a method to create a github pull request.

What will the method capture?

1) Error connecting to github while creating the PR / updating the PR
2) Error connecting to Azdo 
3) Any other error in calling the method CreatePullRequestAsync should go create a git hub issue. 
    4) Error while creating a new branch 
    5) Error while trying to commit to the branch

What will the error contain?

The issue will be created in github/core-eng . The error will contain
a) Source repo,
b) Target repo and
c) Dependency update version( old and new)
d) Actual error details

For Eg: Error while updating "x" dependency from source repo to target repo. HTTP 404 : Unable to connect to https://github.com/maestro-auth-test/maestro-test2

Where should this method be called from?

   1)  In the PullRequestActor, there is a method called CreatePullRequestAsync. I was planning to call this method from the catch block. 
2) Also the part where is says something bad happened but it did not create an exception this can be called from that place too. But here no exception is thrown so there will be no exception details in this issue. ( This might be a bad idea since we will not know what actually went wrong, but instead of just deleting the branch it will be nice to know the branch was deleted and no update went through) 

Please let me know your thoughts.

[epananth]

Spoke to @riarenas, we discussed that there are categories of errors in this which we cannot pin point at this particular time. So I am going to list the errors and categorize the errors and depending on the category it will create a different type of github issue( I have to think about this too) .

[mmitche]

Looks good to me. Where are you thinking to create issues for errors on PRs to AzDO repos? Maybe core-eng as a backup?

[epananth]

I categorized the list errors to start with, there might be more to this.

Error	Error category	Github	Notes
Azdo unable to connect	Azdo Error	Single Github issue	Need to check what is the error we get when Azdo is down.   Eg : Token expiration, Azdo 404 , 500   If one of the subscriptions failed and an issue was created in github then for the same error another issue should not be created.   Need a way to look up the last few issues created in core-eng say last hour or so then have to validate if we are creating an issue with the same info.   Not sure if this is even possible.
Wrong parameters to the API request	Parameter Error	Single Github issue	There are many calls to different apis in PullRequestActor class.   Eg: wrong name of the repo, subscription, source repo, target repo
Logic Error	Dependency Update Error	Single Github issue	Eg : Unable to create target branch, unable to find the target repo, unable to find the valid subscription, target branch was renamed, Deployment broke dependency update
Github down	Github Error	Single Github issue	Eg: Github API throttling   Should create only one issue.
Any other errors	Unknown Error ( don’t know what type of error this should be)	Single Github issue	Before deleting the branch, it just creates an issue. (prob catch the exception and add this in the description)

So I was thinking all the issues will be created in core-eng as default.

[mmitche]

So I was thinking all the issues will be created in core-eng as default.
Works for me.

[epananth]

For Error diagnosis and capturing the error as github issue, we have 2 categories to look into

• SubscriptionUpdateHistory table - For non-batched updates - we can use the existing API - /api/subscriptions/{id}/history
• RepositoryBranchUpdateHistory - For Batched updates - we can use the existing API - /api/repo-config/history

We can run a scheduled flow to check if there are any errors registered in the above tables. The scheduler can be configurable to run every hour or day. We already have some data in the table, but we need to improve these error msgs in them.

SubscriptionUpdateHistory table has data which says it only some update went through. If we can add better error msgs then it will be useful for this scenario. Probably add more into like source, target repo, success message and error message. Right now all the success and error message as logged under errorMessage column, we can update this to some column say Message that will hold a json with the success/ error message.

RepositoryBranchUpdateHistory table definitely has better data which we can use.

Q's -

• Is unable to merge because of merge conflict considered as an error?
• There are places in RepositoryBranchUpdateHistory where there is an error and after retry, the update is successful. I am not sure why we are not updating this record since the try was successful. I was thinking we can add another column say RetryCount, we can keep count for the number of retries. We can do the same for the SubscriptionUpdateHistory. This will reduce duplication of the data.
• Also not sure who is using this data. If we make changes to these tables what gets affected?

[riarenas]

Do you think the errors we are registering in the tables cover all the categories that you analyzed earlier? I like the idea of using these existing APIs as a source of truth for creating the issues, but depending of how much we record right now, we'll have to make sure we add enough details to make things actionable.

Is unable to merge because of merge conflict considered as an error?

I think this is out of scope, we are only looking at informing on errors that happen before we can create or update the PR.

There are places in RepositoryBranchUpdateHistory where there is an error and after retry, the update is successful. I am not sure why we are not updating this record since the try was successful. I was thinking we can add another column say RetryCount, we can keep count for the number of retries. We can do the same for the SubscriptionUpdateHistory. This will reduce duplication of the data.

I'm a bit lost here, isn't there a new record for the successful update? Are you suggesting we stop doing that, or are you asking why we do it like that?

Also not sure who is using this data. If we make changes to these tables what gets affected?

Nothing that I'm aware of. We used to provide a darc command for just the subscription history, but removed it when we introduced the batched subscriptions and the repository history API.