Horde node warnings affecting suspect commit calculation on errors

Development Pipeline & Plugins
, ,
1

Hey everyone,

We have been using Horde’s slack integration pretty heavily in order to notify users asap on build breakage and it has been working great. We’re running a lot of validator templates for perpetually cooking, compiling blueprints etc so we’re catching issues and triaging them very quickly. But I’ve noticed that when errors happen, nodes that contain warnings are worse at figuring out the correct suspect commit compared to nodes that had runs with no warnings

I’ve confirmed that Horde is properly including the file information on the fingerprints and having the asset’s relativePath on the event properties for example. But from what I can tell, due to this code on IssueService.cs:

IJobStepRef? prevJob = await _jobStepRefs.GetPrevStepForNodeAsync(job.StreamId, job.TemplateId, node.Name, job.CommitId, JobStepOutcome.Success, true);

only job steps with JobStepOutcome.Success will be considered for suspect detection, so for example since our CompileBlueprints validation step always had warnings, it’s completely skipping the suspect detection

Due to our validation nodes having a lot of warnings, we use the workflow option `“triageWarnings”: false` in order to focus on errors for triaging, and even the `“CreateWarningIssues”: “false”` annotation option so no warning issue is created from this, but there’s no setting from what I can tell to affect the above functionality. The only way I found was setting `NotifyOnWarnings=“false”` on the node level in buildgraph which downgrades all LogEvents from Warning to Information.

This seems to work, but it’s scary as it makes warnings much less visible (job step appearing green) and it’s pretty cumbersome as it can’t be set on the Template/Workflow level and instead needs to be provided to each Node on the Buildgraph level (even though its value can be handled by an option and provided by the template)

Let me know if I’ve missed something. We’re building Horde from the ue5-main git branch so it’s the latest changes

Thank you

2

Steps to Reproduce

  1. Make a buildgraph with a node that supports proper fingerprint handling, like `<Commandlet Name=“CompileAllBlueprints”` which gets properly handled by ContentIssueHandler.cs
  2. Have warnings on that Node and see that the job step’s outcome is marked as “Warnings”
  3. Introduce a breaking change on a Blueprint, that makes the CompileAllBlueprints node throw an error

Horde will be able to annotate the file and knows what the suspect file is, but due to IssueService.cs:

IJobStepRef? prevJob = await _jobStepRefs.GetPrevStepForNodeAsync(job.StreamId, job.TemplateId, node.Name, job.CommitId, JobStepOutcome.Success, true);

only looking at successful job steps, it will ignore job steps that had warnings, going further back than necessary when looking for suspect commits that introduced an error . If all previous job executions had warnings then prevJob will be null and no suspect detection will execute

3

Hey Austin, thank you for the response

Yeah this makes sense. We’ve also are working on either solving or silencing warnings little by little but yeah it’s a slow process.

In the meantime setting NotifyOnWarnings=“false” on the Buildgraph nodes has been working well, as it still gives us the warnings in the logs, so people who hunt warnings can still find and squash them, while not affecting the suspect logic. The only thing is that the labels still appear green on the overview, but that’s something we can train people to keep an eye out for

So the current flow works, just wanted to make sure I’m not missing anything. Cheers!