Arcade: Build errors should flow through Azure Pipelines to GitHub Checks annotations

[rainersigwald]

GitHub Checks have the ability to annotate errors as related to specific lines of code. Arcade-based builds should take advantage of this.

There are several layers of logic required here:

Arcade

The build process must emit messages in such a way that Azure Pipelines can understand them.

The MSBuild and VSBuild pipelines tasks accomplish this by using a custom logger, Microsoft.TeamFoundation.DistributedTask.MSBuild.Logger.

Arcade launches builds using a script rather than those tasks, and it doesn't seem that there's an easy way to get access to the logger DLL from inside an arbitrary script.

Arcade could reimplement the logger--it's a pretty straightforward mapping from the MSBuild BuildErrorEventArgs fields to the metadata required in the console message.

Pipelines

Pipelines then needs to transfer the error-message metadata into a Checks annotation. At the moment, this doesn't appear to be fully functional.

For example, I introduced a compile error into a repo that uses the MSBuild task (and thus the custom logger). Pipelines correctly flags the error (https://dev.azure.com/dnceng/public/_build/results?buildId=94488), but the corresponding Check run says only that the overall build failed.

For this part, we'll need to engage the Azure DevOps folks.

[rainersigwald]

One possibility to get partway there very cheaply is something like https://github.com/Microsoft/msbuild/pull/3606, which doesn't apply to origin line/column number but gets everything highlighted red in Pipelines.

[tmat]

Another thing to point out - the existing logger (Microsoft.TeamFoundation.DistributedTask.MSBuild.Logger) only works for desktop msbuild, we'd need one that runs on Core.

[rainersigwald]

Funny thing . . . .NET Core is pretty good about that these days :)

S:\work\azdo_logging_mess>dotnet msbuild /noconlog /l:Microsoft.TeamFoundation.DistributedTask.MSBuild.Logger.dll test.proj
Microsoft (R) Build Engine version 16.0.225-preview+g5ebeba52a1 for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

##vso[task.logdetail id=a7bc3710-7d55-4466-88f8-0e010696662d;parentid=;name=test.proj;type=Build;starttime=2019-02-13T20:15:42.4586119Z;state=InProgress;]
##vso[task.logissue type=Error;sourcepath=S:\work\azdo_logging_mess\test.proj;linenumber=3;columnnumber=3;code=ABC123;](No message specified)
##vso[task.logdetail id=a7bc3710-7d55-4466-88f8-0e010696662d;parentid=;type=Build;result=Failed;finishtime=2019-02-13T20:15:42.5234418Z;progress=100;state=Completed;parentid=;name=;]

(I haven't tried more complex multi-proc scenarios, and we'd want them to _say_ they supported it, so this probably isn't the final say. But it's promising!)

[markwilkie]

@chcosta - could you reach out to eh AzDO folks about pipeline error msg transport support that @rainersigwald mentioned? This seems cool @rainersigwald - thanks!

[chcosta]

This came up in discussions with Azure DevOps last month.

Here's the note from the discussion around this point.

Allow customer builds to ##VSO-annotate to inform the run about errors, and these annotations would show up in GitHub Checks too. Test infrastructure already has a command like this, but might not match this scenario.

I'll follow up with them to understand the status better.

[chcosta]

I'm continuing the discussion with Azure DevOps about this but delayed by OOF schedules. Likely not to have any update until Mid March.

[alexperovich]

I did some digging. The code that actually hooks up the logger is here https://github.com/Microsoft/azure-pipelines-tasks/blob/3db96d9354c2b0d7a7a060c9fc52e2081ba6329f/Tasks/Common/MSBuildHelpers/InvokeFunctions.ps1#L111 and the logger assembly is referenced here https://github.com/Microsoft/azure-pipelines-tasks/blob/3db96d9354c2b0d7a7a060c9fc52e2081ba6329f/Tasks/Common/MSBuildHelpers/make.json#L5

We could probably just bootstrap that assembly and add the logger to msbuild invocations.

[anurse]

We're looking at this in aspnet/Extensions, fyi: https://github.com/aspnet/Extensions/pull/1226

[markwilkie]

@chcosta - you mentioned an update by mid-March?

[chcosta]

Yes, didn't circle back here (there are a few issues around this particular area), but @anurse 's work and my own investigations show that errors do flow back if you use the proper vso annotations for reporting errors.

[markwilkie]

@rainersigwald - what do you think about next steps here? (especially based on @anurse 's work)

[rainersigwald]

I think next steps are to let the aspnet/Extension stuff bake for a little while (week or two?) and if it continues to work without issue, promote it to everyone.

[anurse]

The main "problem" we're seeing right now is that it seems to sometimes just randomly not work. There's no error, it's just that some jobs don't show detailed error information. It's not consistent between job either, so I'm not sure exactly what's going on.

No problems have arisen so far due to this though so I think that as long nothing comes up this week, we should definitely promote this in to arcade. We don't even have it in all our repos yet, so getting it in to arcade would help us there.

[markwilkie]

What do you think @anurse ?

[anurse]

Seems to be working fine! Like I said, I believe it does still randomly not wire-up events properly, but no errors.

[jaredpar]

Source for the logger is here https://dev.azure.com/mseng/AzureDevOps/_git/AzureDevOps?path=%2FDistributedTask%2FAgent%2FMSBuild.Logger%2FLoggerHelpers.cs&version=GBmaster

[chcosta]

Azure DevOps says they're ok if we want to move the code for the MSBuild Logger to an open source repo or publish / consume a NuGet package of it.

[jaredpar]

@chcosta if that's the case then my vote is move code here. Reason being it's just saner to manage. I'd rather add functionality to arcade, especially when it's this straight forward, than yet another NuPkg (that will invetiably just cause me to swear a lot later).

[markwilkie]

Closing this in favor of #2376 Let me know if there's a gap that didn't get sorted.

[anurse]

Is #2376 expected to include the necessary Arcade changes to wire up the logger and ensure build errors flow to GitHub checks? If so, I'm fine with that.

[rainersigwald]

Agreed; there's two phases:

• build and distribute the code
• wire it into builds

My interpretation is #2376 does the former, and this does the latter. But they could be done together, presumably.

[markwilkie]

ACK - I made an explicit note that #2376 includes hooking it up at least to Arcade. Then others can do the same as they'd like following the model.

[jaredpar]

Still not clear why we closed this. #2376 just adds the powershell / bash functionality. This is about adding an MSBuild logger implementation.

[jaredpar]

Ignore me. I'm mixing up issues in my head.