Notify BuildStepListeners before and after a BuildStep is performed

[Aurel]

Context

When a BuildStep is typically run as part of AbstractBuild.perform, BuildStepListeners are notified that a BuildStep is starting and typically notified when one has finished, even if failed. There are a some other plugins that use BuildStepListeners to analyze which step in the process actually failed, but at the moment no notification is sent to those plugins when a BuildStep is run as part of a BuildStepRunner.

This means that instead of seeing what step in a process actually failed, many of those plugins might see something like the ConditionalBuilder or some other Builder wrapping a BuildStepRunner. This means that once a Build Step is invoked through this flow, it will never be able to report its metrics upstream to whatever listener might be watching steps as they roll in.

Solution

It seems that the solution is as easy as

I've added my proposed solution to this problem, but I'm still fairly new to the world of Jenkins plugins, so if there's a reason why this is not a good idea, I'm curious to understand more. I'm not sure if anyone else is currently manually invoking BuildStepListener.started/BuildStepListener.finished on nested BuildSteps, but I feel it is the right thing to do.

I have also wrapped the BuildStepRunner's perform method in a try/finally so that we can at least know that the Step is finished, even if threw an exception, but allowing the Exception to fall through to the appropriate scope.

Please let me know if this seems to be an appropriate suggestion.

Potential Concerns

There does exist one minor concern here - I personally don't know how BuildStepListeners are implemented in various plugins - and if there is an implicit expectation that all calls to the started / finished methods are always root level objects. By doing this change we'd basically be calling started on the ConditionalBuilder, for instance, and then calling started on whatever step is being run by the BuildStepRunner here, before we ever finish the wrapping call. This might be perfectly harmless, but is worthy to consider in case any plugins operate on the assumption that nested BuildSteps are never notified about.

[Aurel]

Is there somebody out there who can actually merge this in or take a look at it?