Fill out the template below. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
The pull request must only fix one issue at the time.
The pull request must update the test suite to demonstrate the changed functionality.
After you create the pull request, all status checks must be pass before a maintainer reviews your contribution. For more details, please see CONTRIBUTING.
What does this PR do?
This PR fixes logic that propagates errors from pipeline nodes to their parents.
The problem is that currently errors are propagated unconditionally from a node to its ancestors until the root node is reached.
This approach is not always correct, as an error in the pipeline might be caught and suppressed, in which case Jenkins can report as successful the stage that caught the error (along with all its parents).
The desired state is to have statuses of the spans sent to Datadog exactly the same as the statuses of corresponding Jenkins pipeline stages, which means the logic should be adjusted to stop propagation if the error was caught.
The motivation to change this now is that there is a customer complaining about the statuses disparity.
Description of the Change
There are 3 ways to catch and suppress an error:
try/catch block in a scripted pipeline
catchError block in a declarative pipeline
warnError block in a declarative pipeline
For the latter two BuildPipelineNode class was augmented with an additional field that stores the status that should be applied if an error occurs in that node (with catchError this status can be configured, with warnError the status is always unstable).
For the first one, existing field BuildPipelineNode#errorObj is examined: if an error is caught and suppressed, the node that caught it will have null in that field.
Alternate Designs
Possible Drawbacks
Verification Process
Verified manually with a local Jenkins instance and DD staging, and covered with integration tests that contain examples of pipelines where errors are caught using different ways.
Additional Notes
Release Notes
Review checklist (to be filled by reviewers)
[ ] Feature or bug fix MUST have appropriate tests (unit, integration, etc...)
[ ] PR title must be written as a CHANGELOG entry (see why)
[ ] Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
[ ] PR must have one changelog/ label attached. If applicable it should have the backward-incompatible label attached.
[ ] PR should not have do-not-merge/ label attached.
[ ] If Applicable, issue must have kind/ and severity/ labels attached at least.
Requirements for Contributing to this repository
What does this PR do?
This PR fixes logic that propagates errors from pipeline nodes to their parents. The problem is that currently errors are propagated unconditionally from a node to its ancestors until the root node is reached. This approach is not always correct, as an error in the pipeline might be caught and suppressed, in which case Jenkins can report as successful the stage that caught the error (along with all its parents). The desired state is to have statuses of the spans sent to Datadog exactly the same as the statuses of corresponding Jenkins pipeline stages, which means the logic should be adjusted to stop propagation if the error was caught.
The motivation to change this now is that there is a customer complaining about the statuses disparity.
Description of the Change
There are 3 ways to catch and suppress an error:
try
/catch
block in a scripted pipelinecatchError
block in a declarative pipelinewarnError
block in a declarative pipelineFor the latter two
BuildPipelineNode
class was augmented with an additional field that stores the status that should be applied if an error occurs in that node (withcatchError
this status can be configured, withwarnError
the status is alwaysunstable
). For the first one, existing fieldBuildPipelineNode#errorObj
is examined: if an error is caught and suppressed, the node that caught it will havenull
in that field.Alternate Designs
Possible Drawbacks
Verification Process
Verified manually with a local Jenkins instance and DD staging, and covered with integration tests that contain examples of pipelines where errors are caught using different ways.
Additional Notes
Release Notes
Review checklist (to be filled by reviewers)
changelog/
label attached. If applicable it should have thebackward-incompatible
label attached.do-not-merge/
label attached.kind/
andseverity/
labels attached at least.